From e36f28d58190e071b652009b4c2fa1b082f28e68 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Fri, 1 Apr 2022 00:41:33 +0200 Subject: [PATCH 1/3] chore: bump go-ipld-format v0.4.0 and fix related sharness tests --- go.mod | 4 ++-- go.sum | 7 ++++--- test/sharness/t0050-block.sh | 4 ++-- test/sharness/t0054-dag-car-import-export.sh | 4 ++-- test/sharness/t0080-repo.sh | 2 +- test/sharness/t0081-repo-pinning.sh | 2 +- test/sharness/t0085-pins.sh | 4 ++-- 7 files changed, 14 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 1fd95dee40d..33f254478cf 100644 --- a/go.mod +++ b/go.mod @@ -42,7 +42,7 @@ require ( github.com/ipfs/go-ipfs-routing v0.2.1 github.com/ipfs/go-ipfs-util v0.0.2 github.com/ipfs/go-ipld-cbor v0.0.5 - github.com/ipfs/go-ipld-format v0.3.0 + github.com/ipfs/go-ipld-format v0.4.0 github.com/ipfs/go-ipld-git v0.1.1 github.com/ipfs/go-ipld-legacy v0.1.0 github.com/ipfs/go-ipns v0.1.2 @@ -57,7 +57,7 @@ require ( github.com/ipfs/go-unixfs v0.3.1 github.com/ipfs/go-unixfsnode v1.1.3 github.com/ipfs/go-verifcid v0.0.1 - github.com/ipfs/interface-go-ipfs-core v0.6.0 + github.com/ipfs/interface-go-ipfs-core v0.6.2 github.com/ipfs/tar-utils v0.0.2 github.com/ipld/go-car v0.3.2 github.com/ipld/go-codec-dagpb v1.3.0 diff --git a/go.sum b/go.sum index 43f65b68404..aa6b4a17159 100644 --- a/go.sum +++ b/go.sum @@ -519,8 +519,9 @@ github.com/ipfs/go-ipld-cbor v0.0.5/go.mod h1:BkCduEx3XBCO6t2Sfo5BaHzuok7hbhdMm9 github.com/ipfs/go-ipld-format v0.0.1/go.mod h1:kyJtbkDALmFHv3QR6et67i35QzO3S0dCDnkOJhcZkms= github.com/ipfs/go-ipld-format v0.0.2/go.mod h1:4B6+FM2u9OJ9zCV+kSbgFAZlOrv1Hqbf0INGQgiKf9k= github.com/ipfs/go-ipld-format v0.2.0/go.mod h1:3l3C1uKoadTPbeNfrDi+xMInYKlx2Cvg1BuydPSdzQs= -github.com/ipfs/go-ipld-format v0.3.0 h1:Mwm2oRLzIuUwEPewWAWyMuuBQUsn3awfFEYVb8akMOQ= github.com/ipfs/go-ipld-format v0.3.0/go.mod h1:co/SdBE8h99968X0hViiw1MNlh6fvxxnHpvVLnH7jSM= +github.com/ipfs/go-ipld-format v0.4.0 h1:yqJSaJftjmjc9jEOFYlpkwOLVKv68OD27jFLlSghBlQ= +github.com/ipfs/go-ipld-format v0.4.0/go.mod h1:co/SdBE8h99968X0hViiw1MNlh6fvxxnHpvVLnH7jSM= github.com/ipfs/go-ipld-git v0.1.1 h1:TWGnZjS0htmEmlMFEkA3ogrNCqWjIxwr16x1OsdhG+Y= github.com/ipfs/go-ipld-git v0.1.1/go.mod h1:+VyMqF5lMcJh4rwEppV0e6g4nCCHXThLYYDpKUkJubI= github.com/ipfs/go-ipld-legacy v0.1.0 h1:wxkkc4k8cnvIGIjPO0waJCe7SHEyFgl+yQdafdjGrpA= @@ -577,8 +578,8 @@ github.com/ipfs/go-unixfsnode v1.1.3/go.mod h1:ZZxUM5wXBC+G0Co9FjrYTOm+UlhZTjxLf github.com/ipfs/go-verifcid v0.0.1 h1:m2HI7zIuR5TFyQ1b79Da5N9dnnCP1vcu2QqawmWlK2E= github.com/ipfs/go-verifcid v0.0.1/go.mod h1:5Hrva5KBeIog4A+UpqlaIU+DEstipcJYQQZc0g37pY0= github.com/ipfs/interface-go-ipfs-core v0.4.0/go.mod h1:UJBcU6iNennuI05amq3FQ7g0JHUkibHFAfhfUIy927o= -github.com/ipfs/interface-go-ipfs-core v0.6.0 h1:a43QNc3CNayuMjZM+4D9SeyiSF2nKxBaG8qbCAelMNs= -github.com/ipfs/interface-go-ipfs-core v0.6.0/go.mod h1:h3NuO3wzv2KuKazt0zDF2/i8AFRqiKHusyh5DUQQdPA= +github.com/ipfs/interface-go-ipfs-core v0.6.2 h1:nnkq9zhb5O8lPzkZeynEymc83RqkTRqfYH4x5JNUkT4= +github.com/ipfs/interface-go-ipfs-core v0.6.2/go.mod h1:h3NuO3wzv2KuKazt0zDF2/i8AFRqiKHusyh5DUQQdPA= github.com/ipfs/tar-utils v0.0.2 h1:UNgHB4x/PPzbMkmJi+7EqC9LNMPDztOVSnx1HAqSNg4= github.com/ipfs/tar-utils v0.0.2/go.mod h1:4qlnRWgTVljIMhSG2SqRYn66NT+3wrv/kZt9V+eqxDM= github.com/ipld/go-car v0.3.2 h1:V9wt/80FNfbMRWSD98W5br6fyjUAyVgI2lDOTZX16Lg= diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index 4e809876b73..2b58c7efe1a 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -134,9 +134,9 @@ test_expect_success "multi-block 'ipfs block rm '" ' ' test_expect_success "multi-block 'ipfs block rm ' output looks good" ' - echo "cannot remove $RANDOMHASH: $RANDOMHASH not found" >> expect_mixed_rm && + echo "cannot remove $RANDOMHASH: ipld: could not find $RANDOMHASH" >> expect_mixed_rm && echo "removed $TESTHASH" >> expect_mixed_rm && - echo "cannot remove $RANDOMHASH: $RANDOMHASH not found" >> expect_mixed_rm && + echo "cannot remove $RANDOMHASH: ipld: could not find $RANDOMHASH" >> expect_mixed_rm && echo "Error: some blocks not removed" >> expect_mixed_rm test_cmp actual_mixed_rm expect_mixed_rm ' diff --git a/test/sharness/t0054-dag-car-import-export.sh b/test/sharness/t0054-dag-car-import-export.sh index d1ec0d2c52b..d795c151d8e 100755 --- a/test/sharness/t0054-dag-car-import-export.sh +++ b/test/sharness/t0054-dag-car-import-export.sh @@ -67,7 +67,7 @@ EOE # Explainer: # naked_root_import_json_expected output is produced by dag import of combined_naked_roots_genesis_and_128.car # executed when roots are already present in the repo - thus the BlockCount=0 - # (if blocks were not present in the repo, blockstore: block not found would be returned) + # (if blocks were not present in the repo, ipld: could not find would be returned) cat >naked_root_import_json_expected < offline_fetch_error_expected +echo "Error: block was not found locally (offline): ipld: could not find QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX (currently offline, perhaps retry after attaching to the network)" > offline_fetch_error_expected test_expect_success "basic offline export of nonexistent cid" ' ! ipfs dag export QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX 2> offline_fetch_error_actual >/dev/null ' diff --git a/test/sharness/t0080-repo.sh b/test/sharness/t0080-repo.sh index 521b8db43d2..2cdaac474b1 100755 --- a/test/sharness/t0080-repo.sh +++ b/test/sharness/t0080-repo.sh @@ -64,7 +64,7 @@ test_expect_success "'ipfs repo gc --silent' succeeds (no output)" ' ipfs repo gc --silent >gc_out_empty && test_cmp /dev/null gc_out_empty && test_must_fail ipfs cat "$HASH2" 2>err_expected1 && - grep "Error: $HASH2 not found" err_expected1 + grep "Error: ipld: could not find $HASH2" err_expected1 ' test_kill_ipfs_daemon diff --git a/test/sharness/t0081-repo-pinning.sh b/test/sharness/t0081-repo-pinning.sh index 656698fae06..030f3fa3d06 100755 --- a/test/sharness/t0081-repo-pinning.sh +++ b/test/sharness/t0081-repo-pinning.sh @@ -238,7 +238,7 @@ test_expect_success "some are no longer there" ' test_launch_ipfs_daemon_without_network test_expect_success "recursive pin fails without objects" ' test_must_fail ipfs pin add -r "$HASH_DIR1" 2>err_expected8 && - grep "not found" err_expected8 || + grep "ipld: could not find" err_expected8 || test_fsh cat err_expected8 ' diff --git a/test/sharness/t0085-pins.sh b/test/sharness/t0085-pins.sh index 90f3b0d8acd..c83c513682b 100755 --- a/test/sharness/t0085-pins.sh +++ b/test/sharness/t0085-pins.sh @@ -115,7 +115,7 @@ test_pins_error_reporting() { test_expect_success "'ipfs pin add $PIN_ARGS' on non-existent hash should fail" ' test_must_fail ipfs pin add $PIN_ARGS $RANDOM_HASH 2> err && - grep -q "not found" err + grep -q "ipld: could not find" err ' } @@ -147,7 +147,7 @@ test_pin_dag() { test_expect_success "pin file, should fail" ' test_must_fail ipfs pin add --recursive=true $HASH 2> err && cat err && - grep -q "not found" err + grep -q "ipld: could not find" err ' } From f72110c2d8676ec770722aad44547d6169216eec Mon Sep 17 00:00:00 2001 From: Jorropo Date: Fri, 1 Apr 2022 01:18:05 +0200 Subject: [PATCH 2/3] fix: use error instead of strings as error in blockstoreutil/remove --- blocks/blockstoreutil/remove.go | 27 +++++++++++++------------ core/commands/block.go | 35 ++++++++++++++++++++++++++++----- core/coreapi/block.go | 6 ++---- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/blocks/blockstoreutil/remove.go b/blocks/blockstoreutil/remove.go index b4e944fdfcb..992da720b73 100644 --- a/blocks/blockstoreutil/remove.go +++ b/blocks/blockstoreutil/remove.go @@ -3,6 +3,7 @@ package blockstoreutil import ( "context" + "errors" "fmt" "io" @@ -13,14 +14,14 @@ import ( ) // RemovedBlock is used to represent the result of removing a block. -// If a block was removed successfully, then the Error string will be -// empty. If a block could not be removed, then Error will contain the +// If a block was removed successfully, then the Error will be empty. +// If a block could not be removed, then Error will contain the // reason the block could not be removed. If the removal was aborted // due to a fatal error, Hash will be empty, Error will contain the // reason, and no more results will be sent. type RemovedBlock struct { - Hash string `json:",omitempty"` - Error string `json:",omitempty"` + Hash string + Error error } // RmBlocksOpts is used to wrap options for RmBlocks(). @@ -51,17 +52,17 @@ func RmBlocks(ctx context.Context, blocks bs.GCBlockstore, pins pin.Pinner, cids // remove this sometime in the future. has, err := blocks.Has(ctx, c) if err != nil { - out <- &RemovedBlock{Hash: c.String(), Error: err.Error()} + out <- &RemovedBlock{Hash: c.String(), Error: err} continue } if !has && !opts.Force { - out <- &RemovedBlock{Hash: c.String(), Error: format.ErrNotFound{Cid: c}.Error()} + out <- &RemovedBlock{Hash: c.String(), Error: format.ErrNotFound{Cid: c}} continue } err = blocks.DeleteBlock(ctx, c) if err != nil { - out <- &RemovedBlock{Hash: c.String(), Error: err.Error()} + out <- &RemovedBlock{Hash: c.String(), Error: err} } else if !opts.Quiet { out <- &RemovedBlock{Hash: c.String()} } @@ -79,7 +80,7 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{}, stillOkay := make([]cid.Cid, 0, len(cids)) res, err := pins.CheckIfPinned(ctx, cids...) if err != nil { - out <- &RemovedBlock{Error: fmt.Sprintf("pin check failed: %s", err)} + out <- &RemovedBlock{Error: fmt.Errorf("pin check failed: %w", err)} return nil } for _, r := range res { @@ -88,7 +89,7 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{}, } else { out <- &RemovedBlock{ Hash: r.Key.String(), - Error: r.String(), + Error: errors.New(r.String()), } } } @@ -107,11 +108,11 @@ func ProcRmOutput(next func() (interface{}, error), sout io.Writer, serr io.Writ return err } r := res.(*RemovedBlock) - if r.Hash == "" && r.Error != "" { - return fmt.Errorf("aborted: %s", r.Error) - } else if r.Error != "" { + if r.Hash == "" && r.Error != nil { + return fmt.Errorf("aborted: %w", r.Error) + } else if r.Error != nil { someFailed = true - fmt.Fprintf(serr, "cannot remove %s: %s\n", r.Hash, r.Error) + fmt.Fprintf(serr, "cannot remove %s: %v\n", r.Hash, r.Error) } else { fmt.Fprintf(sout, "removed %s\n", r.Hash) } diff --git a/core/commands/block.go b/core/commands/block.go index 4cfe1fc6ef9..945dbf64f80 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -8,7 +8,6 @@ import ( files "github.com/ipfs/go-ipfs-files" - util "github.com/ipfs/go-ipfs/blocks/blockstoreutil" cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv" "github.com/ipfs/go-ipfs/core/commands/cmdutils" @@ -213,6 +212,11 @@ const ( blockQuietOptionName = "quiet" ) +type removedBlock struct { + Hash string `json:",omitempty"` + Error string `json:",omitempty"` +} + var blockRmCmd = &cmds.Command{ Helptext: cmds.HelpText{ Tagline: "Remove IPFS block(s).", @@ -246,7 +250,7 @@ It takes a list of base58 encoded multihashes to remove. err = api.Block().Rm(req.Context, rp, options.Block.Force(force)) if err != nil { - if err := res.Emit(&util.RemovedBlock{ + if err := res.Emit(&removedBlock{ Hash: rp.Cid().String(), Error: err.Error(), }); err != nil { @@ -256,7 +260,7 @@ It takes a list of base58 encoded multihashes to remove. } if !quiet { - err := res.Emit(&util.RemovedBlock{ + err := res.Emit(&removedBlock{ Hash: rp.Cid().String(), }) if err != nil { @@ -269,8 +273,29 @@ It takes a list of base58 encoded multihashes to remove. }, PostRun: cmds.PostRunMap{ cmds.CLI: func(res cmds.Response, re cmds.ResponseEmitter) error { - return util.ProcRmOutput(res.Next, os.Stdout, os.Stderr) + someFailed := false + for { + res, err := res.Next() + if err == io.EOF { + break + } else if err != nil { + return err + } + r := res.(*removedBlock) + if r.Hash == "" && r.Error != "" { + return fmt.Errorf("aborted: %s", r.Error) + } else if r.Error != "" { + someFailed = true + fmt.Fprintf(os.Stderr, "cannot remove %s: %s\n", r.Hash, r.Error) + } else { + fmt.Fprintf(os.Stdout, "removed %s\n", r.Hash) + } + } + if someFailed { + return fmt.Errorf("some blocks not removed") + } + return nil }, }, - Type: util.RemovedBlock{}, + Type: removedBlock{}, } diff --git a/core/coreapi/block.go b/core/coreapi/block.go index a1d5984d412..0b103a78a32 100644 --- a/core/coreapi/block.go +++ b/core/coreapi/block.go @@ -107,10 +107,8 @@ func (api *BlockAPI) Rm(ctx context.Context, p path.Path, opts ...caopts.BlockRm return errors.New("got unexpected output from util.RmBlocks") } - // Because errors come as strings we lose information about - // the error type. - if remBlock.Error != "" { - return errors.New(remBlock.Error) + if remBlock.Error != nil { + return remBlock.Error } return nil case <-ctx.Done(): From ac297a829e5e4bbc42b2640cf285c5413da70443 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Fri, 1 Apr 2022 15:59:41 +0200 Subject: [PATCH 3/3] chore: block/blockstoreutil remove unused ProcRmOutput function --- blocks/blockstoreutil/remove.go | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/blocks/blockstoreutil/remove.go b/blocks/blockstoreutil/remove.go index 992da720b73..4440c2a6538 100644 --- a/blocks/blockstoreutil/remove.go +++ b/blocks/blockstoreutil/remove.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "io" cid "github.com/ipfs/go-cid" bs "github.com/ipfs/go-ipfs-blockstore" @@ -95,30 +94,3 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{}, } return stillOkay } - -// ProcRmOutput takes a function which returns a result from RmBlocks or EOF if there is no input. -// It then writes to stdout/stderr according to the RemovedBlock object returned from the function. -func ProcRmOutput(next func() (interface{}, error), sout io.Writer, serr io.Writer) error { - someFailed := false - for { - res, err := next() - if err == io.EOF { - break - } else if err != nil { - return err - } - r := res.(*RemovedBlock) - if r.Hash == "" && r.Error != nil { - return fmt.Errorf("aborted: %w", r.Error) - } else if r.Error != nil { - someFailed = true - fmt.Fprintf(serr, "cannot remove %s: %v\n", r.Hash, r.Error) - } else { - fmt.Fprintf(sout, "removed %s\n", r.Hash) - } - } - if someFailed { - return fmt.Errorf("some blocks not removed") - } - return nil -}