diff --git a/core/commands/pin.go b/core/commands/pin.go index f4a3a110ecf..c8ac2d56431 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -194,6 +194,7 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.) }, Options: []cmdkit.Option{ cmdkit.BoolOption(pinRecursiveOptionName, "r", "Recursively unpin the object linked to by the specified object(s).").WithDefault(true), + cmdkit.BoolOption("explain", "e", "Check for other pinned objects which could cause specified object(s) to be indirectly pinned").WithDefault(false), }, Type: PinOutput{}, Run: func(req cmds.Request, res cmds.Response) { @@ -216,7 +217,13 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.) return } - removed, err := corerepo.Unpin(n, api, req.Context(), req.Arguments(), recursive) + explain, _, err := req.Option("explain").Bool() + if err != nil { + res.SetError(err, cmdkit.ErrNormal) + return + } + + removed, err := corerepo.Unpin(n, api, req.Context(), req.Arguments(), recursive, explain) if err != nil { res.SetError(err, cmdkit.ErrNormal) return diff --git a/core/coreapi/pin.go b/core/coreapi/pin.go index d3513e41e55..dda4c7c5318 100644 --- a/core/coreapi/pin.go +++ b/core/coreapi/pin.go @@ -53,12 +53,12 @@ func (api *PinAPI) Ls(ctx context.Context, opts ...caopts.PinLsOption) ([]coreif } func (api *PinAPI) Rm(ctx context.Context, p coreiface.Path) error { - _, err := corerepo.Unpin(api.node, api.core(), ctx, []string{p.String()}, true) + _, err := corerepo.Unpin(api.node, api.core(), ctx, []string{p.String()}, true, true) if err != nil { return err } - return api.node.Pinning.Flush() + return nil } func (api *PinAPI) Update(ctx context.Context, from coreiface.Path, to coreiface.Path, opts ...caopts.PinUpdateOption) error { diff --git a/core/corerepo/pinning.go b/core/corerepo/pinning.go index c943bf3fd63..776042816b9 100644 --- a/core/corerepo/pinning.go +++ b/core/corerepo/pinning.go @@ -51,7 +51,7 @@ func Pin(n *core.IpfsNode, api iface.CoreAPI, ctx context.Context, paths []strin return out, nil } -func Unpin(n *core.IpfsNode, api iface.CoreAPI, ctx context.Context, paths []string, recursive bool) ([]cid.Cid, error) { +func Unpin(n *core.IpfsNode, api iface.CoreAPI, ctx context.Context, paths []string, recursive bool, explain bool) ([]*cid.Cid, error) { unpinned := make([]cid.Cid, len(paths)) for i, p := range paths { @@ -65,7 +65,7 @@ func Unpin(n *core.IpfsNode, api iface.CoreAPI, ctx context.Context, paths []str return nil, err } - err = n.Pinning.Unpin(ctx, k.Cid(), recursive) + err = n.Pinning.Unpin(ctx, k.Cid(), recursive, explain) if err != nil { return nil, err } diff --git a/core/coreunix/add.go b/core/coreunix/add.go index 9e3625a4402..73aa31c9fb5 100644 --- a/core/coreunix/add.go +++ b/core/coreunix/add.go @@ -178,7 +178,7 @@ func (adder *Adder) PinRoot() error { } if adder.tempRoot.Defined() { - err := adder.pinning.Unpin(adder.ctx, adder.tempRoot, true) + err := adder.pinning.Unpin(adder.ctx, adder.tempRoot, true, false) if err != nil { return err } diff --git a/pin/pin.go b/pin/pin.go index 4b91fa4072e..f12cbfb6c62 100644 --- a/pin/pin.go +++ b/pin/pin.go @@ -116,7 +116,7 @@ type Pinner interface { // Unpin the given cid. If recursive is true, removes either a recursive or // a direct pin. If recursive is false, only removes a direct pin. - Unpin(ctx context.Context, cid cid.Cid, recursive bool) error + Unpin(ctx context.Context, cid cid.Cid, recursive bool, explain bool) error // Update updates a recursive pin from one cid to another // this is more efficient than simply pinning the new one and unpinning the @@ -266,29 +266,59 @@ func (p *pinner) Pin(ctx context.Context, node ipld.Node, recurse bool) error { var ErrNotPinned = fmt.Errorf("not pinned") // Unpin a given key -func (p *pinner) Unpin(ctx context.Context, c cid.Cid, recursive bool) error { +func (p *pinner) Unpin(ctx context.Context, c cid.Cid, recursive bool, explain bool) error { p.lock.Lock() defer p.lock.Unlock() - reason, pinned, err := p.isPinnedWithType(c, Any) + + var pinMode Mode + if recursive { + pinMode = Recursive + } else { + pinMode = Direct + } + _, pinned, err := p.isPinnedWithType(c, pinMode) if err != nil { return err } - if !pinned { - return ErrNotPinned - } - switch reason { - case "recursive": + if pinned { if recursive { p.recursePin.Remove(c) return nil + } else { + p.directPin.Remove(c) + return nil } - return fmt.Errorf("%s is pinned recursively", c) - case "direct": - p.directPin.Remove(c) - return nil - default: - return fmt.Errorf("%s is pinned indirectly under %s", c, reason) + } else if recursive { + _, pinned, err := p.isPinnedWithType(c, Direct) + if err != nil { + return err + } + if pinned { + p.directPin.Remove(c) + return nil + } + } + + if explain { + reason, pinned, err := p.isPinnedWithType(c, Any) + if err != nil { + return err + } + if !pinned { + return ErrNotPinned + } + switch reason { + case "recursive": + return fmt.Errorf("%s is pinned recursively", c) + default: + return fmt.Errorf("%s is pinned indirectly under %s", c, reason) + } + } else if recursive { + return fmt.Errorf("%s is not pinned recursively or directly", c) + } else { + return fmt.Errorf("%s is not pinned directly", c) } + } func (p *pinner) isInternalPin(c cid.Cid) bool { diff --git a/pin/pin_test.go b/pin/pin_test.go index 91efc24b661..478b743822c 100644 --- a/pin/pin_test.go +++ b/pin/pin_test.go @@ -137,7 +137,7 @@ func TestPinnerBasic(t *testing.T) { assertPinned(t, p, dk, "pinned node not found.") // Test recursive unpin - err = p.Unpin(ctx, dk, true) + err = p.Unpin(ctx, dk, true, false) if err != nil { t.Fatal(err) } @@ -261,7 +261,7 @@ func TestIsPinnedLookup(t *testing.T) { assertPinned(t, p, bk, "B should be pinned") // Unpin A5 recursively - if err := p.Unpin(ctx, aKeys[5], true); err != nil { + if err := p.Unpin(ctx, aKeys[5], true, false); err != nil { t.Fatal(err) } @@ -269,7 +269,7 @@ func TestIsPinnedLookup(t *testing.T) { assertUnpinned(t, p, aKeys[4], "A4 should be unpinned") // Unpin B recursively - if err := p.Unpin(ctx, bk, true); err != nil { + if err := p.Unpin(ctx, bk, true, false); err != nil { t.Fatal(err) } assertUnpinned(t, p, bk, "B should be unpinned") diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh index 0147f4ea985..79b102baeed 100755 --- a/test/sharness/t0080-repo.sh +++ b/test/sharness/t0080-repo.sh @@ -86,11 +86,17 @@ test_expect_success "pinning directly should fail now" ' ' test_expect_success "'ipfs pin rm -r=false ' should fail" ' - echo "Error: $HASH is pinned recursively" >expected4 && + echo "Error: $HASH is not pinned directly" >expected4 && test_must_fail ipfs pin rm -r=false "$HASH" 2>actual4 && test_cmp expected4 actual4 ' +test_expect_success "'ipfs pin rm -e -r=false ' should fail and explain why" ' + echo "Error: $HASH is pinned recursively" >expected11 && + test_must_fail ipfs pin rm -e -r=false "$HASH" 2>actual11 && + test_cmp expected11 actual11 +' + test_expect_success "remove recursive pin, add direct" ' echo "unpinned $HASH" >expected5 && ipfs pin rm -r "$HASH" >actual5 && @@ -100,6 +106,13 @@ test_expect_success "remove recursive pin, add direct" ' test_expect_success "remove direct pin" ' echo "unpinned $HASH" >expected6 && + ipfs pin rm -r=false "$HASH" >actual6 && + test_cmp expected6 actual6 +' + +test_expect_success "remove direct pin with pin rm -r" ' + echo "unpinned $HASH" >expected6 && + ipfs pin add -r=false "$HASH" ipfs pin rm "$HASH" >actual6 && test_cmp expected6 actual6 ' diff --git a/test/sharness/t0252-files-gc.sh b/test/sharness/t0252-files-gc.sh index 3666d7a481f..eeb2552f80a 100755 --- a/test/sharness/t0252-files-gc.sh +++ b/test/sharness/t0252-files-gc.sh @@ -34,7 +34,7 @@ test_expect_success "gc okay after adding incomplete node -- prep" ' ipfs repo gc && # will remove /adir/file1 and /adir/file2 but not /adir test_must_fail ipfs cat $FILE1_HASH && ipfs files cp /ipfs/$ADIR_HASH /adir && - ipfs pin rm $ADIR_HASH + ipfs pin rm -r=false $ADIR_HASH ' test_expect_success "gc okay after adding incomplete node" '