cmds/dag/import: pin roots by default (#9966)

This is a partial revert of b685355.
Closes #9765 with compromise agreed in #9765 (comment)

core/commands/dag/dag.go

@@ -176,16 +176,13 @@ var DagImportCmd = &cmds.Command{
 		Tagline: "Import the contents of .car files",
 		ShortDescription: `
 'ipfs dag import' imports all blocks present in supplied .car
-( Content Address aRchive ) files, optionally recursively pinning any
-roots specified in the CAR file headers if --pin-roots is set.
+( Content Address aRchive ) files, recursively pinning any roots
+specified in the CAR file headers, unless --pin-roots is set to false.
 
 Note:
   This command will import all blocks in the CAR file, not just those
-  reachable from the specified roots. However, when using --pin-roots,
-  these other blocks will not be pinned and may be garbage collected
-  later. When not using --pin-roots, all blocks imported may be garbage
-  collected if no other pin operation is performed on them, or a root
-  that references them.
+  reachable from the specified roots. However, these other blocks will
+  not be pinned and may be garbage collected later.
 
   The pinning of the roots happens after all car files are processed,
   permitting import of DAGs spanning multiple files.
@@ -203,7 +200,7 @@ Specification of CAR formats: https://ipld.io/specs/transport/car/
 		cmds.FileArg("path", true, true, "The path of a .car file.").EnableStdin(),
 	},
 	Options: []cmds.Option{
-		cmds.BoolOption(pinRootsOptionName, "Pin optional roots listed in the .car headers after importing."),
+		cmds.BoolOption(pinRootsOptionName, "Pin optional roots listed in the .car headers after importing.").WithDefault(true),
 		cmds.BoolOption(silentOptionName, "No output."),
 		cmds.BoolOption(statsOptionName, "Output stats."),
 		cmdutils.AllowBigBlockOption,

docs/changelogs/v0.22.md

@@ -6,35 +6,13 @@
 
 - [Overview](#overview)
 - [🔦 Highlights](#-highlights)
-  - [`ipfs dag import` no longer pins by default](#ipfs-dag-import-no-longer-pins-by-default)
 - [📝 Changelog](#-changelog)
 - [👨‍👩‍👧‍👦 Contributors](#-contributors)
 
 ### Overview
 
 ### 🔦 Highlights
 
-#### `ipfs dag import` no longer pins by default
-
-With the gateway now capable of handling partial CAR exports
-([IPIP-402](https://github.com/ipfs/specs/pull/402)) and incomplete DAG CARs
-becoming more prevalent, there have been changes to the pinning mode when using
-`ipfs dag import`.
-
-Recursive pinning of the entire DAG within an imported CAR is now optional.  To
-explicitly attempt pinning the DAG referenced by any roots present in the CAR,
-you can opt in by using the `--pin-roots` option.
-
-Pinning incomplete DAG will produce an error:
-
-```console
-$ curl 'http://127.0.0.1:8080/ipns/docs.ipfs.tech?format=car&dag-scope=entity' > ./partial-entity.car # Kubo 0.21.0 with IPIP-402 (only root block of unixfs dir)
-$ ipfs dag import --stats --pin-roots=true ./partial-entity.car
-Error pinning	QmPDC11yLAbVw3dX5jMeEuSdk4BiVjSd9X87zaYRdVjzW3	FAILED: block was not found locally (offline): ipld: could not find QmPDvrDAz2aHeLjPVQ4uh1neyknUmDpf1GsBzAbpFhS8ro
-Imported 1 blocks (1618 bytes)
-[exit code 1]
-```
-
 ### 📝 Changelog
 
 ### 👨‍👩‍👧‍👦 Contributors

test/sharness/t0054-dag-car-import-export.sh

@@ -41,7 +41,7 @@ do_import() {
       while [[ -e spin.gc ]]; do ipfsi "$node" repo gc &>/dev/null; done &
       while [[ -e spin.gc ]]; do ipfsi "$node" repo gc &>/dev/null; done &
 
-      ipfsi "$node" dag import --pin-roots "$@" 2>&1 && ipfsi "$node" repo verify &>/dev/null
+      ipfsi "$node" dag import "$@" 2>&1 && ipfsi "$node" repo verify &>/dev/null
       result=$?
 
       rm -f spin.gc &>/dev/null
@@ -117,7 +117,7 @@ EOE
   '
 
   test_expect_success "import/pin naked roots only, relying on local blockstore having all the data" '
-    ipfsi 1 dag import --stats --enc=json --pin-roots ../t0054-dag-car-import-export-data/combined_naked_roots_genesis_and_128.car \
+    ipfsi 1 dag import --stats --enc=json ../t0054-dag-car-import-export-data/combined_naked_roots_genesis_and_128.car \
       > naked_import_result_json_actual
   '
 
@@ -197,14 +197,14 @@ EOE
 head -3 multiroot_import_json_stats_expected > multiroot_import_json_expected
 
 test_expect_success "multiroot import works (--enc=json)" '
-  ipfs dag import --enc=json --pin-roots ../t0054-dag-car-import-export-data/lotus_testnet_export_256_multiroot.car > multiroot_import_json_actual
+  ipfs dag import --enc=json ../t0054-dag-car-import-export-data/lotus_testnet_export_256_multiroot.car > multiroot_import_json_actual
 '
 test_expect_success "multiroot import expected output" '
   test_cmp_sorted multiroot_import_json_expected multiroot_import_json_actual
 '
 
 test_expect_success "multiroot import works with --stats" '
-  ipfs dag import --stats --enc=json --pin-roots ../t0054-dag-car-import-export-data/lotus_testnet_export_256_multiroot.car > multiroot_import_json_actual
+  ipfs dag import --stats --enc=json ../t0054-dag-car-import-export-data/lotus_testnet_export_256_multiroot.car > multiroot_import_json_actual
 '
 test_expect_success "multiroot import expected output" '
   test_cmp_sorted multiroot_import_json_stats_expected multiroot_import_json_actual
@@ -215,18 +215,18 @@ cat >pin_import_expected << EOE
 {"Stats":{"BlockCount":1198,"BlockBytesCount":468513}}
 EOE
 test_expect_success "pin-less import works" '
-  ipfs dag import --stats --enc=json \
+  ipfs dag import --stats --enc=json --pin-roots=false \
   ../t0054-dag-car-import-export-data/lotus_devnet_genesis.car \
   ../t0054-dag-car-import-export-data/lotus_testnet_export_128.car \
     > no-pin_import_actual
 '
-test_expect_success "expected no pins on" '
+test_expect_success "expected no pins on --pin-roots=false" '
   test_cmp pin_import_expected no-pin_import_actual
 '
 
 
 test_expect_success "naked root import works" '
-  ipfs dag import --stats --enc=json --pin-roots ../t0054-dag-car-import-export-data/combined_naked_roots_genesis_and_128.car \
+  ipfs dag import --stats --enc=json ../t0054-dag-car-import-export-data/combined_naked_roots_genesis_and_128.car \
     > naked_root_import_json_actual
 '
 test_expect_success "naked root import expected output" '
@@ -253,7 +253,7 @@ cat > version_2_import_expected << EOE
 EOE
 
 test_expect_success "version 2 import" '
-  ipfs dag import --stats --enc=json --pin-roots \
+  ipfs dag import --stats --enc=json \
     ../t0054-dag-car-import-export-data/lotus_testnet_export_128_v2.car \
     ../t0054-dag-car-import-export-data/lotus_devnet_genesis_v2.car \
   > version_2_import_actual
@@ -299,17 +299,14 @@ test_expect_success "'ipfs dag import' without pinning works fine with incomplet
     ipfs dag import --stats --enc=json --pin-roots=false ../t0054-dag-car-import-export-data/partial-dag-scope-entity.car >partial_nopin_import_out 2>&1 &&
     test_cmp partial_nopin_import_expected partial_nopin_import_out
 '
-test_expect_success "'ipfs dag import' with no params in CLI mode produces exit code 0 (unixfs dir exported as dag-scope=entity from IPIP-402)" '
-    test_expect_code 0 ipfs dag import ../t0054-dag-car-import-export-data/partial-dag-scope-entity.car
-'
 
 test_expect_success "'ipfs dag import' with pinning errors due to incomplete DAG (unixfs dir exported as dag-scope=entity from IPIP-402)" '
     ipfs dag import --stats --enc=json --pin-roots=true ../t0054-dag-car-import-export-data/partial-dag-scope-entity.car >partial_pin_import_out 2>&1 &&
     test_should_contain "\"PinErrorMsg\":\"block was not found locally" partial_pin_import_out
 '
 
 test_expect_success "'ipfs dag import' pin error in default CLI mode produces exit code 1 (unixfs dir exported as dag-scope=entity from IPIP-402)" '
-    test_expect_code 1 ipfs dag import --pin-roots ../t0054-dag-car-import-export-data/partial-dag-scope-entity.car >partial_pin_import_out 2>&1 &&
+    test_expect_code 1 ipfs dag import ../t0054-dag-car-import-export-data/partial-dag-scope-entity.car >partial_pin_import_out 2>&1 &&
     test_should_contain "Error: pinning root \"QmPDC11yLAbVw3dX5jMeEuSdk4BiVjSd9X87zaYRdVjzW3\" FAILED: block was not found locally" partial_pin_import_out
 '
 

test/sharness/t0124-gateway-ipns-record.sh

@@ -12,7 +12,7 @@ test_launch_ipfs_daemon
 IPNS_KEY=k51qzi5uqu5dh71qgwangrt6r0nd4094i88nsady6qgd1dhjcyfsaqmpp143ab
 FILE_CID=bafkreidfdrlkeq4m4xnxuyx6iae76fdm4wgl5d4xzsb77ixhyqwumhz244 # A file containing Hello IPFS
 test_expect_success "Add the test directory & IPNS records" '
-  ipfs dag import --pin-roots ../t0124-gateway-ipns-record/fixtures.car &&
+  ipfs dag import ../t0124-gateway-ipns-record/fixtures.car &&
   ipfs routing put /ipns/${IPNS_KEY} ../t0124-gateway-ipns-record/${IPNS_KEY}.ipns-record
 '
 

test/sharness/t0400-api-no-gateway.sh

@@ -13,7 +13,7 @@ test_init_ipfs
 # Import test case
 # See the static fixtures in ./t0400-api-no-gateway/
 test_expect_success "Add the test directory" '
-  ipfs dag import --pin-roots ../t0400-api-no-gateway/fixtures.car
+  ipfs dag import ../t0400-api-no-gateway/fixtures.car
 '
 HASH=QmNYERzV2LfD2kkfahtfv44ocHzEFK1sLBaE7zdcYT2GAZ # a file containing the string "testing"