From 25694d0238f57b18d3576bf0fcc4e0fdf496e01b Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Mon, 25 Nov 2019 11:38:29 +0000 Subject: [PATCH 1/2] fix: ignore nonexistant when force rm - Make `ipfs files rm --force /nonexistant` succeed when the path does not exist. - Add shaness test for removing nonexistant paths - Refactor duplicated code to find a parent dir into a function I've been writing scripts against the files api, and having to stat things before removing them is a pain. So this PR aims to make --force do what I'd expect it to. License: MIT Signed-off-by: Oli Evans --- core/commands/files.go | 56 ++++++++++++++++++++------------ test/sharness/t0250-files-api.sh | 8 +++++ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/core/commands/files.go b/core/commands/files.go index 36ba60e2cdb..9a8131e6c1c 100644 --- a/core/commands/files.go +++ b/core/commands/files.go @@ -17,14 +17,14 @@ import ( bservice "github.com/ipfs/go-blockservice" cid "github.com/ipfs/go-cid" cidenc "github.com/ipfs/go-cidutil/cidenc" - "github.com/ipfs/go-ipfs-cmds" - "github.com/ipfs/go-ipfs-exchange-offline" + cmds "github.com/ipfs/go-ipfs-cmds" + offline "github.com/ipfs/go-ipfs-exchange-offline" ipld "github.com/ipfs/go-ipld-format" logging "github.com/ipfs/go-log" dag "github.com/ipfs/go-merkledag" "github.com/ipfs/go-mfs" ft "github.com/ipfs/go-unixfs" - "github.com/ipfs/interface-go-ipfs-core" + iface "github.com/ipfs/interface-go-ipfs-core" path "github.com/ipfs/interface-go-ipfs-core/path" mh "github.com/multiformats/go-multihash" ) @@ -997,26 +997,33 @@ Remove files or directories. path = path[:len(path)-1] } + // if '--force' specified, it will remove anything else, + // including file, directory, corrupted node, etc + force, _ := req.Options[forceOptionName].(bool) + dir, name := gopath.Split(path) - parent, err := mfs.Lookup(nd.FilesRoot, dir) + + pdir, err := getParentDir(nd.FilesRoot, dir) if err != nil { + if force { + switch err { + case os.ErrNotExist: + return nil + } + } return fmt.Errorf("parent lookup: %s", err) } - pdir, ok := parent.(*mfs.Directory) - if !ok { - return fmt.Errorf("no such file or directory: %s", path) - } - - // if '--force' specified, it will remove anything else, - // including file, directory, corrupted node, etc - force, _ := req.Options[forceOptionName].(bool) if force { err := pdir.Unlink(name) if err != nil { - return err + switch err { + case os.ErrNotExist: + return nil + default: + return err + } } - return pdir.Flush() } @@ -1133,15 +1140,11 @@ func getFileHandle(r *mfs.Root, path string, create bool, builder cid.Builder) ( // if create is specified and the file doesnt exist, we create the file dirname, fname := gopath.Split(path) - pdiri, err := mfs.Lookup(r, dirname) + pdir, err := getParentDir(r, dirname) if err != nil { - flog.Error("lookupfail ", dirname) return nil, err } - pdir, ok := pdiri.(*mfs.Directory) - if !ok { - return nil, fmt.Errorf("%s was not a directory", dirname) - } + if builder == nil { builder = pdir.GetCidBuilder() } @@ -1184,3 +1187,16 @@ func checkPath(p string) (string, error) { } return cleaned, nil } + +func getParentDir(root *mfs.Root, dir string) (*mfs.Directory, error) { + parent, err := mfs.Lookup(root, dir) + if err != nil { + return nil, err + } + + pdir, ok := parent.(*mfs.Directory) + if !ok { + return nil, errors.New("expected *mfs.Directory, didnt get it. This is likely a race condition") + } + return pdir, nil +} diff --git a/test/sharness/t0250-files-api.sh b/test/sharness/t0250-files-api.sh index 24980966a72..aa6c3e4457f 100755 --- a/test/sharness/t0250-files-api.sh +++ b/test/sharness/t0250-files-api.sh @@ -683,6 +683,14 @@ test_files_api() { ipfs files rm --force /forcibly-dir && verify_dir_contents / ' + + test_expect_success "remove nonexistant path forcibly" ' + ipfs files rm --force /nonexistant + ' + + test_expect_success "remove deeply nonexistant path forcibly" ' + ipfs files rm --force /deeply/nonexistant + ' } # test offline and online From 7fcf40eeeef1f210878ad069dcf65241699358d9 Mon Sep 17 00:00:00 2001 From: Oli Evans Date: Mon, 25 Nov 2019 14:51:30 +0000 Subject: [PATCH 2/2] fix: use if over switch License: MIT Signed-off-by: Oli Evans --- core/commands/files.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/core/commands/files.go b/core/commands/files.go index 9a8131e6c1c..e568657d644 100644 --- a/core/commands/files.go +++ b/core/commands/files.go @@ -1005,11 +1005,8 @@ Remove files or directories. pdir, err := getParentDir(nd.FilesRoot, dir) if err != nil { - if force { - switch err { - case os.ErrNotExist: - return nil - } + if force && err == os.ErrNotExist { + return nil } return fmt.Errorf("parent lookup: %s", err) } @@ -1017,12 +1014,10 @@ Remove files or directories. if force { err := pdir.Unlink(name) if err != nil { - switch err { - case os.ErrNotExist: + if err == os.ErrNotExist { return nil - default: - return err } + return err } return pdir.Flush() }