Make pin rm not try to find pinned ancestors by default

core/commands/pin.go

@@ -184,6 +184,7 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.)
 	},
 	Options: []cmdkit.Option{
 		cmdkit.BoolOption("recursive", "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) {
@@ -200,7 +201,13 @@ collected if needed. (By default, recursively. Use -r=false for direct pins.)
 			return
 		}
 
-		removed, err := corerepo.Unpin(n, 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, req.Context(), req.Arguments(), recursive, explain)
 		if err != nil {
 			res.SetError(err, cmdkit.ErrNormal)
 			return

core/coreapi/pin.go

@@ -53,7 +53,7 @@ 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, ctx, []string{p.String()}, true)
+	_, err := corerepo.Unpin(api.node, ctx, []string{p.String()}, true, true)
 	if err != nil {
 		return err
 	}

core/corerepo/pinning.go

@@ -58,7 +58,7 @@ func Pin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool)
 	return out, nil
 }
 
-func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool) ([]*cid.Cid, error) {
+func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool, explain bool) ([]*cid.Cid, error) {
 	unpinned := make([]*cid.Cid, len(paths))
 
 	r := &resolver.Resolver{
@@ -77,7 +77,7 @@ func Unpin(n *core.IpfsNode, ctx context.Context, paths []string, recursive bool
 			return nil, err
 		}
 
-		err = n.Pinning.Unpin(ctx, k, recursive)
+		err = n.Pinning.Unpin(ctx, k, recursive, explain)
 		if err != nil {
 			return nil, err
 		}

core/coreunix/add.go

@@ -183,7 +183,7 @@ func (adder *Adder) PinRoot() error {
 	}
 
 	if adder.tempRoot != nil {
-		err := adder.pinning.Unpin(adder.ctx, adder.tempRoot, true)
+		err := adder.pinning.Unpin(adder.ctx, adder.tempRoot, true, false)
 		if err != nil {
 			return err
 		}

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
@@ -254,29 +254,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 {

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,15 +261,15 @@ 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)
 	}
 
 	assertPinned(t, p, aKeys[0], "A0 should still be pinned through B")
 	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")

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 <hash>' 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 <hash>' 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
 '

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" '