fix(pin): wait till after fetching to remove direct pin

Otherwise, we could abort while fetching the graph and stay in a state where the
direct pin is removed.

fixes #4650

pin/pin.go

@@ -225,9 +225,6 @@ func (p *pinner) Pin(ctx context.Context, node ipld.Node, recurse bool) error {
 			return nil
 		}
 
-		if p.directPin.Has(c) {
-			p.directPin.Remove(c)
-		}
 		p.lock.Unlock()
 		// fetch entire graph
 		err := mdag.FetchGraph(ctx, c, p.dserv)

test/sharness/t0081-repo-pinning.sh

@@ -237,13 +237,21 @@ test_expect_success "some are no longer there" '
   test_must_fail ipfs ls "$HASH_DIR3"
 '
 
+test_launch_ipfs_daemon --offline
 test_expect_success "recursive pin fails without objects" '
-  ipfs pin rm -r=false "$HASH_DIR1" &&
   test_must_fail ipfs pin add -r "$HASH_DIR1" 2>err_expected8 &&
   grep "pin: merkledag: not found" err_expected8 ||
   test_fsh cat err_expected8
 '
 
+# Regression test for https://github.com/ipfs/go-ipfs/issues/4650
+# This test requires the daemon. Otherwise, the pin changes are reverted when
+# the pin fails in the previous test.
+test_expect_success "failed recursive pin does not remove direct pin" '
+  test_pin_flag "$HASH_DIR1" direct true
+'
+test_kill_ipfs_daemon
+
 test_expect_success "test add nopin file" '
   echo "test nopin data" > test_nopin_data &&
   NOPINHASH=$(ipfs add -q --pin=false test_nopin_data) &&