From 3406c8ad3e4975f46be2334232f1b90602ff162f Mon Sep 17 00:00:00 2001 From: Asmir Avdicevic Date: Thu, 9 Apr 2020 11:57:15 +0200 Subject: [PATCH] fix(cmd): checkout supports specifying a directory --- cmd/checkout.go | 20 +++++++++++++++----- cmd/checkout_test.go | 30 ++++++++++++++++++++++++++++++ fsi/fsi.go | 28 ++++++++-------------------- lib/fsi.go | 10 +++++----- 4 files changed, 58 insertions(+), 30 deletions(-) create mode 100644 cmd/checkout_test.go diff --git a/cmd/checkout.go b/cmd/checkout.go index fe20c4f2c..2ffcfa103 100644 --- a/cmd/checkout.go +++ b/cmd/checkout.go @@ -24,7 +24,7 @@ func NewCheckoutCommand(f Factory, ioStreams ioes.IOStreams) *cobra.Command { Annotations: map[string]string{ "group": "workdir", }, - Args: cobra.ExactArgs(1), + Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { if err := o.Complete(f, args); err != nil { return err @@ -43,6 +43,8 @@ type CheckoutOptions struct { Refs *RefSelect FSIMethods *lib.FSIMethods + + Dir string } // Complete configures the checkout command @@ -57,6 +59,12 @@ func (o *CheckoutOptions) Complete(f Factory, args []string) (err error) { return err } + if len(args) == 2 { + o.Dir = args[1] + } else { + o.Dir = "" + } + return nil } @@ -72,17 +80,19 @@ func (o *CheckoutOptions) Run() (err error) { if pos == -1 { return fmt.Errorf("expect '/' in dataset ref") } - folderName := varName.CreateVarNameFromString(ref[pos+1:]) + if o.Dir == "" { + o.Dir = varName.CreateVarNameFromString(ref[pos+1:]) + } - if err = qfs.AbsPath(&folderName); err != nil { + if err = qfs.AbsPath(&o.Dir); err != nil { return err } var res string - err = o.FSIMethods.Checkout(&lib.CheckoutParams{Dir: folderName, Ref: ref}, &res) + err = o.FSIMethods.Checkout(&lib.CheckoutParams{Dir: o.Dir, Ref: ref}, &res) if err != nil { return err } - printSuccess(o.Out, "created and linked working directory %s for existing dataset", folderName) + printSuccess(o.Out, "created and linked working directory %s for existing dataset", o.Dir) return nil } diff --git a/cmd/checkout_test.go b/cmd/checkout_test.go new file mode 100644 index 000000000..cac639152 --- /dev/null +++ b/cmd/checkout_test.go @@ -0,0 +1,30 @@ +package cmd + +import ( + "testing" +) + +func TestDoubleCheckout(t *testing.T) { + runner := NewFSITestRunner(t, "double_checkout") + defer runner.Delete() + + _ = runner.CreateAndChdirToWorkDir("checkout_dir") + + // Init as a linked directory + if err := runner.ExecCommand("qri init --name double_checkout --format csv"); err != nil { + t.Fatal(err.Error()) + } + + // Save a version of the dataset + if err := runner.ExecCommand("qri save"); err != nil { + t.Fatal(err.Error()) + } + + // Move to different work directory + runner.ChdirToRoot() + + // Checkout should fail as the dataset already linked + if err := runner.ExecCommand("qri checkout me/double_checkout ./checkout_test"); err == nil { + t.Fatal("`qri checkout` should fail if already linked, but did not fail") + } +} diff --git a/fsi/fsi.go b/fsi/fsi.go index ee470cffb..aa6f61982 100644 --- a/fsi/fsi.go +++ b/fsi/fsi.go @@ -99,32 +99,18 @@ func (fsi *FSI) LinkedRefs(offset, limit int) ([]reporef.DatasetRef, error) { return refs, nil } -// HasLink checks if a refString already has an existing link on the file system -func (fsi *FSI) HasLink(refStr string) (hasLink bool, err error) { - ref, err := repo.ParseDatasetRef(refStr) - if err != nil { - return false, err - } - err = repo.CanonicalizeDatasetRef(fsi.repo, &ref) - if err != nil && err != repo.ErrNotFound && err != repo.ErrNoHistory { - return false, err - } - - return fsi.RefHasLink(ref) -} - -// RefHasLink checks if a ref already has an existing link on the file system -func (fsi *FSI) RefHasLink(ref reporef.DatasetRef) (hasLink bool, err error) { - if stored, err := fsi.repo.GetRef(ref); err == nil { +// EnsureRefNotLinked checks if a ref already has an existing link on the file system +func (fsi *FSI) EnsureRefNotLinked(ref *reporef.DatasetRef) error { + if stored, err := fsi.repo.GetRef(*ref); err == nil { if stored.FSIPath != "" { // There is already a link for this dataset, see if that link still exists. targetPath := filepath.Join(stored.FSIPath, QriRefFilename) if _, err := os.Stat(targetPath); err == nil { - return true, fmt.Errorf("'%s' is already linked to %s", ref.AliasString(), stored.FSIPath) + return fmt.Errorf("'%s' is already linked to %s", ref.AliasString(), stored.FSIPath) } } } - return false, nil + return nil } // CreateLink links a working directory to a dataset. Returns the reference alias, and a @@ -141,7 +127,9 @@ func (fsi *FSI) CreateLink(dirPath, refStr string) (alias string, rollback func( return ref.String(), rollback, err } - if _, err := fsi.RefHasLink(ref); err != nil { + // todo(arqu): should utilize rollback as other operations bellow + // can fail too + if err := fsi.EnsureRefNotLinked(&ref); err != nil { return "", rollback, err } diff --git a/lib/fsi.go b/lib/fsi.go index fd015292a..b759746d1 100644 --- a/lib/fsi.go +++ b/lib/fsi.go @@ -183,6 +183,11 @@ func (m *FSIMethods) Checkout(p *CheckoutParams, out *string) (err error) { log.Debugf("Checkout for ref %q", ref) + // Fail early if link already exists + if err := m.inst.fsi.EnsureRefNotLinked(ref); err != nil { + return err + } + // Load dataset that is being checked out. ds, err := dsfs.LoadDataset(ctx, m.inst.repo.Store(), ref.Path) if err != nil { @@ -197,11 +202,6 @@ func (m *FSIMethods) Checkout(p *CheckoutParams, out *string) (err error) { } log.Debugf("Checkout loaded dataset %q", ref) - // Fail early if link already exists - if _, err := m.inst.fsi.HasLink(p.Ref); err != nil { - return err - } - // Create a directory. if err := os.Mkdir(p.Dir, os.ModePerm); err != nil { log.Debugf("Checkout, Mkdir failed, error: %s", ref)