Skip to content

Commit 4df7b79

Browse files
JorropoJorropo
authored
Merge pull request #8838 from Jorropo/fix/ErrNotFound-hopefully-final
chore: bump go-ipld-format v0.4.0 and fix related sharness tests
2 parents f855bfe + ac297a8 commit 4df7b79

File tree

10 files changed

+56
-59
lines changed

10 files changed

+56
-59
lines changed

‎blocks/blockstoreutil/remove.go

+10-37
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ package blockstoreutil
33

44
import (
55
"context"
6+
"errors"
67
"fmt"
7-
"io"
88

99
cid "github.com/ipfs/go-cid"
1010
bs "github.com/ipfs/go-ipfs-blockstore"
@@ -13,14 +13,14 @@ import (
1313
)
1414

1515
// RemovedBlock is used to represent the result of removing a block.
16-
// If a block was removed successfully, then the Error string will be
17-
// empty. If a block could not be removed, then Error will contain the
16+
// If a block was removed successfully, then the Error will be empty.
17+
// If a block could not be removed, then Error will contain the
1818
// reason the block could not be removed. If the removal was aborted
1919
// due to a fatal error, Hash will be empty, Error will contain the
2020
// reason, and no more results will be sent.
2121
type RemovedBlock struct {
22-
Hash string `json:",omitempty"`
23-
Error string `json:",omitempty"`
22+
Hash string
23+
Error error
2424
}
2525

2626
// RmBlocksOpts is used to wrap options for RmBlocks().
@@ -51,17 +51,17 @@ func RmBlocks(ctx context.Context, blocks bs.GCBlockstore, pins pin.Pinner, cids
5151
// remove this sometime in the future.
5252
has, err := blocks.Has(ctx, c)
5353
if err != nil {
54-
out <- &RemovedBlock{Hash: c.String(), Error: err.Error()}
54+
out <- &RemovedBlock{Hash: c.String(), Error: err}
5555
continue
5656
}
5757
if !has && !opts.Force {
58-
out <- &RemovedBlock{Hash: c.String(), Error: format.ErrNotFound{Cid: c}.Error()}
58+
out <- &RemovedBlock{Hash: c.String(), Error: format.ErrNotFound{Cid: c}}
5959
continue
6060
}
6161

6262
err = blocks.DeleteBlock(ctx, c)
6363
if err != nil {
64-
out <- &RemovedBlock{Hash: c.String(), Error: err.Error()}
64+
out <- &RemovedBlock{Hash: c.String(), Error: err}
6565
} else if !opts.Quiet {
6666
out <- &RemovedBlock{Hash: c.String()}
6767
}
@@ -79,7 +79,7 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{},
7979
stillOkay := make([]cid.Cid, 0, len(cids))
8080
res, err := pins.CheckIfPinned(ctx, cids...)
8181
if err != nil {
82-
out <- &RemovedBlock{Error: fmt.Sprintf("pin check failed: %s", err)}
82+
out <- &RemovedBlock{Error: fmt.Errorf("pin check failed: %w", err)}
8383
return nil
8484
}
8585
for _, r := range res {
@@ -88,36 +88,9 @@ func FilterPinned(ctx context.Context, pins pin.Pinner, out chan<- interface{},
8888
} else {
8989
out <- &RemovedBlock{
9090
Hash: r.Key.String(),
91-
Error: r.String(),
91+
Error: errors.New(r.String()),
9292
}
9393
}
9494
}
9595
return stillOkay
9696
}
97-
98-
// ProcRmOutput takes a function which returns a result from RmBlocks or EOF if there is no input.
99-
// It then writes to stdout/stderr according to the RemovedBlock object returned from the function.
100-
func ProcRmOutput(next func() (interface{}, error), sout io.Writer, serr io.Writer) error {
101-
someFailed := false
102-
for {
103-
res, err := next()
104-
if err == io.EOF {
105-
break
106-
} else if err != nil {
107-
return err
108-
}
109-
r := res.(*RemovedBlock)
110-
if r.Hash == "" && r.Error != "" {
111-
return fmt.Errorf("aborted: %s", r.Error)
112-
} else if r.Error != "" {
113-
someFailed = true
114-
fmt.Fprintf(serr, "cannot remove %s: %s\n", r.Hash, r.Error)
115-
} else {
116-
fmt.Fprintf(sout, "removed %s\n", r.Hash)
117-
}
118-
}
119-
if someFailed {
120-
return fmt.Errorf("some blocks not removed")
121-
}
122-
return nil
123-
}

‎core/commands/block.go

+30-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
files "github.com/ipfs/go-ipfs-files"
1010

11-
util "github.com/ipfs/go-ipfs/blocks/blockstoreutil"
1211
cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv"
1312
"github.com/ipfs/go-ipfs/core/commands/cmdutils"
1413

@@ -213,6 +212,11 @@ const (
213212
blockQuietOptionName = "quiet"
214213
)
215214

215+
type removedBlock struct {
216+
Hash string `json:",omitempty"`
217+
Error string `json:",omitempty"`
218+
}
219+
216220
var blockRmCmd = &cmds.Command{
217221
Helptext: cmds.HelpText{
218222
Tagline: "Remove IPFS block(s).",
@@ -246,7 +250,7 @@ It takes a list of base58 encoded multihashes to remove.
246250

247251
err = api.Block().Rm(req.Context, rp, options.Block.Force(force))
248252
if err != nil {
249-
if err := res.Emit(&util.RemovedBlock{
253+
if err := res.Emit(&removedBlock{
250254
Hash: rp.Cid().String(),
251255
Error: err.Error(),
252256
}); err != nil {
@@ -256,7 +260,7 @@ It takes a list of base58 encoded multihashes to remove.
256260
}
257261

258262
if !quiet {
259-
err := res.Emit(&util.RemovedBlock{
263+
err := res.Emit(&removedBlock{
260264
Hash: rp.Cid().String(),
261265
})
262266
if err != nil {
@@ -269,8 +273,29 @@ It takes a list of base58 encoded multihashes to remove.
269273
},
270274
PostRun: cmds.PostRunMap{
271275
cmds.CLI: func(res cmds.Response, re cmds.ResponseEmitter) error {
272-
return util.ProcRmOutput(res.Next, os.Stdout, os.Stderr)
276+
someFailed := false
277+
for {
278+
res, err := res.Next()
279+
if err == io.EOF {
280+
break
281+
} else if err != nil {
282+
return err
283+
}
284+
r := res.(*removedBlock)
285+
if r.Hash == "" && r.Error != "" {
286+
return fmt.Errorf("aborted: %s", r.Error)
287+
} else if r.Error != "" {
288+
someFailed = true
289+
fmt.Fprintf(os.Stderr, "cannot remove %s: %s\n", r.Hash, r.Error)
290+
} else {
291+
fmt.Fprintf(os.Stdout, "removed %s\n", r.Hash)
292+
}
293+
}
294+
if someFailed {
295+
return fmt.Errorf("some blocks not removed")
296+
}
297+
return nil
273298
},
274299
},
275-
Type: util.RemovedBlock{},
300+
Type: removedBlock{},
276301
}

‎core/coreapi/block.go

+2-4
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,8 @@ func (api *BlockAPI) Rm(ctx context.Context, p path.Path, opts ...caopts.BlockRm
118118
return errors.New("got unexpected output from util.RmBlocks")
119119
}
120120

121-
// Because errors come as strings we lose information about
122-
// the error type.
123-
if remBlock.Error != "" {
124-
return errors.New(remBlock.Error)
121+
if remBlock.Error != nil {
122+
return remBlock.Error
125123
}
126124
return nil
127125
case <-ctx.Done():

‎go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ require (
4242
github.com/ipfs/go-ipfs-routing v0.2.1
4343
github.com/ipfs/go-ipfs-util v0.0.2
4444
github.com/ipfs/go-ipld-cbor v0.0.5
45-
github.com/ipfs/go-ipld-format v0.3.0
45+
github.com/ipfs/go-ipld-format v0.4.0
4646
github.com/ipfs/go-ipld-git v0.1.1
4747
github.com/ipfs/go-ipld-legacy v0.1.0
4848
github.com/ipfs/go-ipns v0.1.2
@@ -57,7 +57,7 @@ require (
5757
github.com/ipfs/go-unixfs v0.3.1
5858
github.com/ipfs/go-unixfsnode v1.1.3
5959
github.com/ipfs/go-verifcid v0.0.1
60-
github.com/ipfs/interface-go-ipfs-core v0.6.0
60+
github.com/ipfs/interface-go-ipfs-core v0.6.2
6161
github.com/ipfs/tar-utils v0.0.2
6262
github.com/ipld/go-car v0.3.2
6363
github.com/ipld/go-codec-dagpb v1.3.0

‎go.sum

+4-3
Original file line numberDiff line numberDiff line change
@@ -529,8 +529,9 @@ github.com/ipfs/go-ipld-cbor v0.0.5/go.mod h1:BkCduEx3XBCO6t2Sfo5BaHzuok7hbhdMm9
529529
github.com/ipfs/go-ipld-format v0.0.1/go.mod h1:kyJtbkDALmFHv3QR6et67i35QzO3S0dCDnkOJhcZkms=
530530
github.com/ipfs/go-ipld-format v0.0.2/go.mod h1:4B6+FM2u9OJ9zCV+kSbgFAZlOrv1Hqbf0INGQgiKf9k=
531531
github.com/ipfs/go-ipld-format v0.2.0/go.mod h1:3l3C1uKoadTPbeNfrDi+xMInYKlx2Cvg1BuydPSdzQs=
532-
github.com/ipfs/go-ipld-format v0.3.0 h1:Mwm2oRLzIuUwEPewWAWyMuuBQUsn3awfFEYVb8akMOQ=
533532
github.com/ipfs/go-ipld-format v0.3.0/go.mod h1:co/SdBE8h99968X0hViiw1MNlh6fvxxnHpvVLnH7jSM=
533+
github.com/ipfs/go-ipld-format v0.4.0 h1:yqJSaJftjmjc9jEOFYlpkwOLVKv68OD27jFLlSghBlQ=
534+
github.com/ipfs/go-ipld-format v0.4.0/go.mod h1:co/SdBE8h99968X0hViiw1MNlh6fvxxnHpvVLnH7jSM=
534535
github.com/ipfs/go-ipld-git v0.1.1 h1:TWGnZjS0htmEmlMFEkA3ogrNCqWjIxwr16x1OsdhG+Y=
535536
github.com/ipfs/go-ipld-git v0.1.1/go.mod h1:+VyMqF5lMcJh4rwEppV0e6g4nCCHXThLYYDpKUkJubI=
536537
github.com/ipfs/go-ipld-legacy v0.1.0 h1:wxkkc4k8cnvIGIjPO0waJCe7SHEyFgl+yQdafdjGrpA=
@@ -587,8 +588,8 @@ github.com/ipfs/go-unixfsnode v1.1.3/go.mod h1:ZZxUM5wXBC+G0Co9FjrYTOm+UlhZTjxLf
587588
github.com/ipfs/go-verifcid v0.0.1 h1:m2HI7zIuR5TFyQ1b79Da5N9dnnCP1vcu2QqawmWlK2E=
588589
github.com/ipfs/go-verifcid v0.0.1/go.mod h1:5Hrva5KBeIog4A+UpqlaIU+DEstipcJYQQZc0g37pY0=
589590
github.com/ipfs/interface-go-ipfs-core v0.4.0/go.mod h1:UJBcU6iNennuI05amq3FQ7g0JHUkibHFAfhfUIy927o=
590-
github.com/ipfs/interface-go-ipfs-core v0.6.0 h1:a43QNc3CNayuMjZM+4D9SeyiSF2nKxBaG8qbCAelMNs=
591-
github.com/ipfs/interface-go-ipfs-core v0.6.0/go.mod h1:h3NuO3wzv2KuKazt0zDF2/i8AFRqiKHusyh5DUQQdPA=
591+
github.com/ipfs/interface-go-ipfs-core v0.6.2 h1:nnkq9zhb5O8lPzkZeynEymc83RqkTRqfYH4x5JNUkT4=
592+
github.com/ipfs/interface-go-ipfs-core v0.6.2/go.mod h1:h3NuO3wzv2KuKazt0zDF2/i8AFRqiKHusyh5DUQQdPA=
592593
github.com/ipfs/tar-utils v0.0.2 h1:UNgHB4x/PPzbMkmJi+7EqC9LNMPDztOVSnx1HAqSNg4=
593594
github.com/ipfs/tar-utils v0.0.2/go.mod h1:4qlnRWgTVljIMhSG2SqRYn66NT+3wrv/kZt9V+eqxDM=
594595
github.com/ipld/go-car v0.3.2 h1:V9wt/80FNfbMRWSD98W5br6fyjUAyVgI2lDOTZX16Lg=

‎test/sharness/t0050-block.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ test_expect_success "multi-block 'ipfs block rm <invalid> <valid> <invalid>'" '
134134
'
135135

136136
test_expect_success "multi-block 'ipfs block rm <invalid> <valid> <invalid>' output looks good" '
137-
echo "cannot remove $RANDOMHASH: $RANDOMHASH not found" >> expect_mixed_rm &&
137+
echo "cannot remove $RANDOMHASH: ipld: could not find $RANDOMHASH" >> expect_mixed_rm &&
138138
echo "removed $TESTHASH" >> expect_mixed_rm &&
139-
echo "cannot remove $RANDOMHASH: $RANDOMHASH not found" >> expect_mixed_rm &&
139+
echo "cannot remove $RANDOMHASH: ipld: could not find $RANDOMHASH" >> expect_mixed_rm &&
140140
echo "Error: some blocks not removed" >> expect_mixed_rm
141141
test_cmp actual_mixed_rm expect_mixed_rm
142142
'

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ EOE
6767
# Explainer:
6868
# naked_root_import_json_expected output is produced by dag import of combined_naked_roots_genesis_and_128.car
6969
# executed when roots are already present in the repo - thus the BlockCount=0
70-
# (if blocks were not present in the repo, blockstore: block not found would be returned)
70+
# (if blocks were not present in the repo, ipld: could not find <CID> would be returned)
7171
cat >naked_root_import_json_expected <<EOE
7272
{"Root":{"Cid":{"/":"bafy2bzaceaxm23epjsmh75yvzcecsrbavlmkcxnva66bkdebdcnyw3bjrc74u"},"PinErrorMsg":""}}
7373
{"Root":{"Cid":{"/":"bafy2bzaced4ueelaegfs5fqu4tzsh6ywbbpfk3cxppupmxfdhbpbhzawfw5oy"},"PinErrorMsg":""}}
@@ -179,7 +179,7 @@ test_expect_success "basic offline export of 'getting started' dag works" '
179179
'
180180

181181

182-
echo "Error: block was not found locally (offline): QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX not found (currently offline, perhaps retry after attaching to the network)" > offline_fetch_error_expected
182+
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
183183
test_expect_success "basic offline export of nonexistent cid" '
184184
! ipfs dag export QmYwAPJXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX 2> offline_fetch_error_actual >/dev/null
185185
'

‎test/sharness/t0080-repo.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ test_expect_success "'ipfs repo gc --silent' succeeds (no output)" '
6464
ipfs repo gc --silent >gc_out_empty &&
6565
test_cmp /dev/null gc_out_empty &&
6666
test_must_fail ipfs cat "$HASH2" 2>err_expected1 &&
67-
grep "Error: $HASH2 not found" err_expected1
67+
grep "Error: ipld: could not find $HASH2" err_expected1
6868
'
6969

7070
test_kill_ipfs_daemon

‎test/sharness/t0081-repo-pinning.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ test_expect_success "some are no longer there" '
238238
test_launch_ipfs_daemon_without_network
239239
test_expect_success "recursive pin fails without objects" '
240240
test_must_fail ipfs pin add -r "$HASH_DIR1" 2>err_expected8 &&
241-
grep "not found" err_expected8 ||
241+
grep "ipld: could not find" err_expected8 ||
242242
test_fsh cat err_expected8
243243
'
244244

‎test/sharness/t0085-pins.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ test_pins_error_reporting() {
115115

116116
test_expect_success "'ipfs pin add $PIN_ARGS' on non-existent hash should fail" '
117117
test_must_fail ipfs pin add $PIN_ARGS $RANDOM_HASH 2> err &&
118-
grep -q "not found" err
118+
grep -q "ipld: could not find" err
119119
'
120120
}
121121

@@ -147,7 +147,7 @@ test_pin_dag() {
147147
test_expect_success "pin file, should fail" '
148148
test_must_fail ipfs pin add --recursive=true $HASH 2> err &&
149149
cat err &&
150-
grep -q "not found" err
150+
grep -q "ipld: could not find" err
151151
'
152152
}
153153

0 commit comments

Comments
 (0)