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 <oli@tableflip.io>
Oli Evans committed
Nov 25, 2019 at 11:38 UTC
25694d0238f57b18d3576bf0fcc4e0fdf496e01b
2 files changed
+44
-20
core/commands/files.go
+36
-20
@@ -17,14 +17,14 @@ import (
17
bservice "github.com/ipfs/go-blockservice"
18
cid "github.com/ipfs/go-cid"
19
cidenc "github.com/ipfs/go-cidutil/cidenc"
20
- "github.com/ipfs/go-ipfs-cmds"
21
- "github.com/ipfs/go-ipfs-exchange-offline"
20
+ cmds "github.com/ipfs/go-ipfs-cmds"
21
+ offline "github.com/ipfs/go-ipfs-exchange-offline"
22
ipld "github.com/ipfs/go-ipld-format"
23
logging "github.com/ipfs/go-log"
24
dag "github.com/ipfs/go-merkledag"
25
"github.com/ipfs/go-mfs"
26
ft "github.com/ipfs/go-unixfs"
27
- "github.com/ipfs/interface-go-ipfs-core"
27
+ iface "github.com/ipfs/interface-go-ipfs-core"
28
path "github.com/ipfs/interface-go-ipfs-core/path"
29
mh "github.com/multiformats/go-multihash"
30
)
@@ -997,26 +997,33 @@ Remove files or directories.
997
path = path[:len(path)-1]
998
}
999
1000
+ // if '--force' specified, it will remove anything else,
1001
+ // including file, directory, corrupted node, etc
1002
+ force, _ := req.Options[forceOptionName].(bool)
1003
+
1004
dir, name := gopath.Split(path)
1001
- parent, err := mfs.Lookup(nd.FilesRoot, dir)
1005
+
1006
+ pdir, err := getParentDir(nd.FilesRoot, dir)
1007
if err != nil {
1008
+ if force {
1009
+ switch err {
1010
+ case os.ErrNotExist:
1011
+ return nil
1012
+ }
1013
+ }
1014
return fmt.Errorf("parent lookup: %s", err)
1015
}
1016
1006
- pdir, ok := parent.(*mfs.Directory)
1007
- if !ok {
1008
- return fmt.Errorf("no such file or directory: %s", path)
1009
- }
1010
-
1011
- // if '--force' specified, it will remove anything else,
1012
- // including file, directory, corrupted node, etc
1013
- force, _ := req.Options[forceOptionName].(bool)
1017
if force {
1018
err := pdir.Unlink(name)
1019
if err != nil {
1017
- return err
1020
+ switch err {
1021
+ case os.ErrNotExist:
1022
+ return nil
1023
+ default:
1024
+ return err
1025
+ }
1026
}
1019
-
1027
return pdir.Flush()
1028
}
1029
@@ -1133,15 +1140,11 @@ func getFileHandle(r *mfs.Root, path string, create bool, builder cid.Builder) (
1140
1141
// if create is specified and the file doesnt exist, we create the file
1142
dirname, fname := gopath.Split(path)
1136
- pdiri, err := mfs.Lookup(r, dirname)
1143
+ pdir, err := getParentDir(r, dirname)
1144
if err != nil {
1138
- flog.Error("lookupfail ", dirname)
1145
return nil, err
1146
}
1141
- pdir, ok := pdiri.(*mfs.Directory)
1142
- if !ok {
1143
- return nil, fmt.Errorf("%s was not a directory", dirname)
1144
- }
1147
+
1148
if builder == nil {
1149
builder = pdir.GetCidBuilder()
1150
}
@@ -1184,3 +1187,16 @@ func checkPath(p string) (string, error) {
1187
}
1188
return cleaned, nil
1189
}
1190
+
1191
+func getParentDir(root *mfs.Root, dir string) (*mfs.Directory, error) {
1192
+ parent, err := mfs.Lookup(root, dir)
1193
+ if err != nil {
1194
+ return nil, err
1195
+ }
1196
+
1197
+ pdir, ok := parent.(*mfs.Directory)
1198
+ if !ok {
1199
+ return nil, errors.New("expected *mfs.Directory, didnt get it. This is likely a race condition")
1200
+ }
1201
+ return pdir, nil
1202
+}
test/sharness/t0250-files-api.sh
+8
@@ -683,6 +683,14 @@ test_files_api() {
683
ipfs files rm --force /forcibly-dir &&
684
verify_dir_contents /
685
'
686
+
687
+ test_expect_success "remove nonexistant path forcibly" '
688
+ ipfs files rm --force /nonexistant
689
+ '
690
+
691
+ test_expect_success "remove deeply nonexistant path forcibly" '
692
+ ipfs files rm --force /deeply/nonexistant
693
+ '
694
}
695
696
# test offline and online