From 453be22dfb0f2fe83d8025e077f7360ce2b3438e Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Wed, 13 Jul 2016 16:01:22 -0400 Subject: [PATCH 1/7] Add "ipfs block rm" command. License: MIT Signed-off-by: Kevin Atkinson --- core/commands/block.go | 99 ++++++++++++++++++++++++++++++++++++ test/sharness/t0050-block.sh | 69 ++++++++++++++++++++++++- 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/core/commands/block.go b/core/commands/block.go index 7a0e61a717f..74f7f98b197 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -9,8 +9,10 @@ import ( "strings" "github.com/ipfs/go-ipfs/blocks" + bs "github.com/ipfs/go-ipfs/blocks/blockstore" key "github.com/ipfs/go-ipfs/blocks/key" cmds "github.com/ipfs/go-ipfs/commands" + "github.com/ipfs/go-ipfs/pin" mh "gx/ipfs/QmYf7ng2hG5XBtJA3tN34DQ2GUN5HNksEw1rLDkmr6vGku/go-multihash" u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util" ) @@ -38,6 +40,7 @@ multihash. "stat": blockStatCmd, "get": blockGetCmd, "put": blockPutCmd, + "rm": blockRmCmd, }, } @@ -185,3 +188,99 @@ func getBlockForKey(req cmds.Request, skey string) (blocks.Block, error) { log.Debugf("ipfs block: got block with key: %q", b.Key()) return b, nil } + +var blockRmCmd = &cmds.Command{ + Helptext: cmds.HelpText{ + Tagline: "Remove IPFS block(s).", + ShortDescription: ` +'ipfs block rm' is a plumbing command for removing raw ipfs blocks. +It takes a list of base58 encoded multihashs to remove. +`, + }, + Arguments: []cmds.Argument{ + cmds.StringArg("hash", true, true, "Bash58 encoded multihash of block(s) to remove."), + }, + Options: []cmds.Option{ + cmds.BoolOption("ignore-pins", "Ignore pins.").Default(false), + }, + Run: func(req cmds.Request, res cmds.Response) { + ignorePins, _, err := req.Option("ignore-pins").Bool() + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } + n, err := req.InvocContext().GetNode() + if err != nil { + res.SetError(err, cmds.ErrNormal) + return + } + hashes := req.Arguments() + keys := make([]key.Key, 0, len(hashes)) + for _, hash := range hashes { + k := key.B58KeyDecode(hash) + keys = append(keys, k) + } + rdr, wtr := io.Pipe() + go func() { + pinning := n.Pinning + if ignorePins { + pinning = nil + } + err := rmBlocks(n.Blockstore, pinning, wtr, keys) + if err != nil { + wtr.CloseWithError(fmt.Errorf("Some blocks not deleted: %s", err)) + } else { + wtr.Close() + } + }() + res.SetOutput(rdr) + return + }, + Marshalers: cmds.MarshalerMap{ + cmds.Text: func(res cmds.Response) (io.Reader, error) { + return res.(io.Reader), nil + }, + }, +} + +// pins may be nil +func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out io.Writer, keys []key.Key) error { + var unlocker bs.Unlocker + defer func() { + if unlocker != nil { + unlocker.Unlock() + } + }() + if pins != nil { + // Need to make sure that some operation that is + // finishing with a pin is ocurr simultaneously. + unlocker = blocks.GCLock() + err := checkIfPinned(pins, keys) + if err != nil { + return err + } + } + for _, k := range keys { + err := blocks.DeleteBlock(k) + if err != nil { + return fmt.Errorf("%s: %s", k, err) + } + if out != nil { + fmt.Fprintf(out, "deleted %s\n", k) + } + } + return nil +} + +func checkIfPinned(pins pin.Pinner, keys []key.Key) error { + for _, k := range keys { + reason, pinned, err := pins.IsPinned(k) + if err != nil { + return err + } + if pinned { + return fmt.Errorf("%s pinned via %s", k, reason) + } + } + return nil +} diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index c4f00e04888..53fee125401 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -33,12 +33,79 @@ test_expect_success "'ipfs block stat' succeeds" ' ipfs block stat $HASH >actual_stat ' -test_expect_success "'ipfs block get' output looks good" ' +test_expect_success "'ipfs block stat' output looks good" ' echo "Key: $HASH" >expected_stat && echo "Size: 12" >>expected_stat && test_cmp expected_stat actual_stat ' +test_expect_success "'ipfs block rm' succeeds" ' + ipfs block rm $HASH >actual_rm +' + +test_expect_success "'ipfs block rm' output looks good" ' + echo "deleted $HASH" > expected_rm && + test_cmp expected_rm actual_rm +' + +test_expect_success "'ipfs block rm' block actually removed" ' + test_must_fail ipfs block stat $HASH +' + +DIRHASH=QmdWmVmM6W2abTgkEfpbtA1CJyTWS2rhuUB9uP1xV8Uwtf +FILE1HASH=Qmae3RedM7SNkWGsdzYzsr6svmsFdsva4WoTvYYsWhUSVz +FILE2HASH=QmUtkGLvPf63NwVzLPKPUYgwhn8ZYPWF6vKWN3fZ2amfJF +FILE3HASH=Qmesmmf1EEG1orJb6XdK6DabxexsseJnCfw8pqWgonbkoj + +test_expect_success "add and pin directory" ' + mkdir adir && + echo "file1" > adir/file1 && + echo "file2" > adir/file2 && + echo "file3" > adir/file3 && + ipfs add -r adir + ipfs pin add -r $DIRHASH +' + +test_expect_success "can't remove pinned block" ' + test_must_fail ipfs block rm $DIRHASH 2> block_rm_err +' + +test_expect_success "can't remove pinned block: output looks good" ' + grep -q "$DIRHASH pinned via recursive" block_rm_err +' + +test_expect_success "can't remove indirectly pinned block" ' + test_must_fail ipfs block rm $FILE1HASH 2> block_rm_err +' + +test_expect_success "can't remove indirectly pinned block: output looks good" ' + grep -q "$FILE1HASH pinned via $DIRHASH" block_rm_err +' + +test_expect_success "multi-block 'ipfs block rm --ignore-pins' succeeds" ' + ipfs block rm --ignore-pins $DIRHASH >actual_rm +' + +test_expect_success "multi-block 'ipfs block rm --ignore-pins' output looks good" ' + echo "deleted $DIRHASH" > expected_rm && + test_cmp expected_rm actual_rm +' + +test_expect_success "fix up pins" ' + ipfs pin rm -r $DIRHASH +' + +test_expect_success "multi-block 'ipfs block rm' succeeds" ' + ipfs block rm $FILE1HASH $FILE2HASH $FILE3HASH > actual_rm +' + +test_expect_success "multi-block 'ipfs block rm' output looks good" ' + echo "deleted $FILE1HASH" > expected_rm && + echo "deleted $FILE2HASH" >> expected_rm && + echo "deleted $FILE3HASH" >> expected_rm && + test_cmp expected_rm actual_rm +' + test_expect_success "'ipfs block stat' with nothing from stdin doesnt crash" ' test_expect_code 1 ipfs block stat < /dev/null 2> stat_out ' From 6ad497bcf06a831c5017c05a817140a250d04d69 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Wed, 10 Aug 2016 16:53:20 -0400 Subject: [PATCH 2/7] "block rm": use channel instead of pipe / don't abort on non-fatal error License: MIT Signed-off-by: Kevin Atkinson --- core/commands/block.go | 82 +++++++++++++++++++++++------------- test/sharness/t0050-block.sh | 14 +++--- 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/core/commands/block.go b/core/commands/block.go index 74f7f98b197..d2f3de5c7cb 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -220,67 +220,91 @@ It takes a list of base58 encoded multihashs to remove. k := key.B58KeyDecode(hash) keys = append(keys, k) } - rdr, wtr := io.Pipe() + outChan := make(chan interface{}) + res.SetOutput((<-chan interface{})(outChan)) go func() { + defer close(outChan) pinning := n.Pinning if ignorePins { pinning = nil } - err := rmBlocks(n.Blockstore, pinning, wtr, keys) - if err != nil { - wtr.CloseWithError(fmt.Errorf("Some blocks not deleted: %s", err)) - } else { - wtr.Close() - } + rmBlocks(n.Blockstore, pinning, outChan, keys) }() - res.SetOutput(rdr) return }, - Marshalers: cmds.MarshalerMap{ - cmds.Text: func(res cmds.Response) (io.Reader, error) { - return res.(io.Reader), nil - }, + PostRun: func(req cmds.Request, res cmds.Response) { + if res.Error() != nil { + return + } + outChan, ok := res.Output().(<-chan interface{}) + if !ok { + res.SetError(u.ErrCast(), cmds.ErrNormal) + return + } + res.SetOutput(nil) + + someFailed := false + for out := range outChan { + o := out.(*RemovedBlock) + if o.Error != "" { + someFailed = true + fmt.Fprintf(res.Stderr(), "cannot remove %s: %s\n", o.Hash, o.Error) + } else { + fmt.Fprintf(res.Stdout(), "removed %s\n", o.Hash) + } + } + if someFailed { + res.SetError(fmt.Errorf("some blocks not removed"), cmds.ErrNormal) + } }, + Type: RemovedBlock{}, +} + +type RemovedBlock struct { + Hash string + Error string `json:",omitempty"` } // pins may be nil -func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out io.Writer, keys []key.Key) error { +func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) { var unlocker bs.Unlocker defer func() { if unlocker != nil { unlocker.Unlock() } }() + stillOkay := keys if pins != nil { // Need to make sure that some operation that is // finishing with a pin is ocurr simultaneously. unlocker = blocks.GCLock() - err := checkIfPinned(pins, keys) - if err != nil { - return err - } + stillOkay = checkIfPinned(pins, keys, out) } - for _, k := range keys { + for _, k := range stillOkay { err := blocks.DeleteBlock(k) if err != nil { - return fmt.Errorf("%s: %s", k, err) - } - if out != nil { - fmt.Fprintf(out, "deleted %s\n", k) + out <- &RemovedBlock{ Hash: k.String(), Error: err.Error()} + } else { + out <- &RemovedBlock{ Hash: k.String() } } } - return nil } -func checkIfPinned(pins pin.Pinner, keys []key.Key) error { +func checkIfPinned(pins pin.Pinner, keys []key.Key, out chan<- interface{}) []key.Key { + stillOkay := make([]key.Key, 0, len(keys)) for _, k := range keys { reason, pinned, err := pins.IsPinned(k) if err != nil { - return err - } - if pinned { - return fmt.Errorf("%s pinned via %s", k, reason) + out <- &RemovedBlock { + Hash: k.String(), + Error: fmt.Sprintf("pin check failed %s", err.Error()) } + } else if pinned { + out <- &RemovedBlock { + Hash: k.String(), + Error: fmt.Sprintf("pinned via %s", reason) } + } else { + stillOkay = append(stillOkay, k) } } - return nil + return stillOkay } diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index 53fee125401..966c2531051 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -44,7 +44,7 @@ test_expect_success "'ipfs block rm' succeeds" ' ' test_expect_success "'ipfs block rm' output looks good" ' - echo "deleted $HASH" > expected_rm && + echo "removed $HASH" > expected_rm && test_cmp expected_rm actual_rm ' @@ -71,7 +71,7 @@ test_expect_success "can't remove pinned block" ' ' test_expect_success "can't remove pinned block: output looks good" ' - grep -q "$DIRHASH pinned via recursive" block_rm_err + grep -q "$DIRHASH: pinned via recursive" block_rm_err ' test_expect_success "can't remove indirectly pinned block" ' @@ -79,7 +79,7 @@ test_expect_success "can't remove indirectly pinned block" ' ' test_expect_success "can't remove indirectly pinned block: output looks good" ' - grep -q "$FILE1HASH pinned via $DIRHASH" block_rm_err + grep -q "$FILE1HASH: pinned via $DIRHASH" block_rm_err ' test_expect_success "multi-block 'ipfs block rm --ignore-pins' succeeds" ' @@ -87,7 +87,7 @@ test_expect_success "multi-block 'ipfs block rm --ignore-pins' succeeds" ' ' test_expect_success "multi-block 'ipfs block rm --ignore-pins' output looks good" ' - echo "deleted $DIRHASH" > expected_rm && + echo "removed $DIRHASH" > expected_rm && test_cmp expected_rm actual_rm ' @@ -100,9 +100,9 @@ test_expect_success "multi-block 'ipfs block rm' succeeds" ' ' test_expect_success "multi-block 'ipfs block rm' output looks good" ' - echo "deleted $FILE1HASH" > expected_rm && - echo "deleted $FILE2HASH" >> expected_rm && - echo "deleted $FILE3HASH" >> expected_rm && + echo "removed $FILE1HASH" > expected_rm && + echo "removed $FILE2HASH" >> expected_rm && + echo "removed $FILE3HASH" >> expected_rm && test_cmp expected_rm actual_rm ' From efeb789878c358093b2659894fa536185697d8e3 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Thu, 11 Aug 2016 17:45:22 -0400 Subject: [PATCH 3/7] Check for multiple pinned blocks in a single pass. Provide a new method, Pinner.CheckIfPinned(), which will check if any of the arguments are pinned. Previously IsPinned would need to be called once for each block. The new method will speed up the checking of multiple pinned blocks from O(p*n) to O(p) (where p is the number of pinned blocks and n is the number of blocks to be check) Use the new method in "block rm". License: MIT Signed-off-by: Kevin Atkinson --- core/commands/block.go | 59 ++++++++++++++++++---------- pin/pin.go | 74 ++++++++++++++++++++++++++++++++++++ test/sharness/t0050-block.sh | 11 +++--- 3 files changed, 117 insertions(+), 27 deletions(-) diff --git a/core/commands/block.go b/core/commands/block.go index d2f3de5c7cb..2fc324c5ea5 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -228,7 +228,10 @@ It takes a list of base58 encoded multihashs to remove. if ignorePins { pinning = nil } - rmBlocks(n.Blockstore, pinning, outChan, keys) + err := rmBlocks(n.Blockstore, pinning, outChan, keys) + if err != nil { + outChan <- &RemovedBlock{Error: err.Error()} + } }() return }, @@ -246,7 +249,10 @@ It takes a list of base58 encoded multihashs to remove. someFailed := false for out := range outChan { o := out.(*RemovedBlock) - if o.Error != "" { + if o.Hash == "" && o.Error != "" { + res.SetError(fmt.Errorf("aborted: %s", o.Error), cmds.ErrNormal) + return + } else if o.Error != "" { someFailed = true fmt.Fprintf(res.Stderr(), "cannot remove %s: %s\n", o.Hash, o.Error) } else { @@ -261,12 +267,12 @@ It takes a list of base58 encoded multihashs to remove. } type RemovedBlock struct { - Hash string + Hash string `json:",omitempty"` Error string `json:",omitempty"` } // pins may be nil -func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) { +func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) error { var unlocker bs.Unlocker defer func() { if unlocker != nil { @@ -278,33 +284,44 @@ func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, k // Need to make sure that some operation that is // finishing with a pin is ocurr simultaneously. unlocker = blocks.GCLock() - stillOkay = checkIfPinned(pins, keys, out) + var err error + stillOkay, err = checkIfPinned(pins, keys, out) + if err != nil { + return fmt.Errorf("pin check failed: %s", err) + } } for _, k := range stillOkay { err := blocks.DeleteBlock(k) if err != nil { - out <- &RemovedBlock{ Hash: k.String(), Error: err.Error()} + out <- &RemovedBlock{Hash: k.String(), Error: err.Error()} } else { - out <- &RemovedBlock{ Hash: k.String() } + out <- &RemovedBlock{Hash: k.String()} } } + return nil } -func checkIfPinned(pins pin.Pinner, keys []key.Key, out chan<- interface{}) []key.Key { +func checkIfPinned(pins pin.Pinner, keys []key.Key, out chan<- interface{}) ([]key.Key, error) { stillOkay := make([]key.Key, 0, len(keys)) - for _, k := range keys { - reason, pinned, err := pins.IsPinned(k) - if err != nil { - out <- &RemovedBlock { - Hash: k.String(), - Error: fmt.Sprintf("pin check failed %s", err.Error()) } - } else if pinned { - out <- &RemovedBlock { - Hash: k.String(), - Error: fmt.Sprintf("pinned via %s", reason) } - } else { - stillOkay = append(stillOkay, k) + res, err := pins.CheckIfPinned(keys...) + if err != nil { + return nil, err + } + for _, r := range res { + switch r.Mode { + case pin.NotPinned: + stillOkay = append(stillOkay, r.Key) + case pin.Indirect: + out <- &RemovedBlock{ + Hash: r.Key.String(), + Error: fmt.Sprintf("pinned via %s", r.Via)} + default: + modeStr, _ := pin.PinModeToString(r.Mode) + out <- &RemovedBlock{ + Hash: r.Key.String(), + Error: fmt.Sprintf("pinned: %s", modeStr)} + } } - return stillOkay + return stillOkay, nil } diff --git a/pin/pin.go b/pin/pin.go index fa2af39dbb7..49c5a8dd1e0 100644 --- a/pin/pin.go +++ b/pin/pin.go @@ -75,6 +75,10 @@ type Pinner interface { Pin(context.Context, *mdag.Node, bool) error Unpin(context.Context, key.Key, bool) error + // Check if a set of keys are pinned, more efficient than + // calling IsPinned for each key + CheckIfPinned(keys ...key.Key) ([]Pinned, error) + // PinWithMode is for manually editing the pin structure. Use with // care! If used improperly, garbage collection may not be // successful. @@ -90,6 +94,12 @@ type Pinner interface { InternalPins() []key.Key } +type Pinned struct { + Key key.Key + Mode PinMode + Via key.Key +} + // pinner implements the Pinner interface type pinner struct { lock sync.RWMutex @@ -255,6 +265,70 @@ func (p *pinner) isPinnedWithType(k key.Key, mode PinMode) (string, bool, error) return "", false, nil } +func (p *pinner) CheckIfPinned(keys ...key.Key) ([]Pinned, error) { + p.lock.RLock() + defer p.lock.RUnlock() + pinned := make([]Pinned, 0, len(keys)) + toCheck := make(map[key.Key]struct{}) + + // First check for non-Indirect pins directly + for _, k := range keys { + if p.recursePin.HasKey(k) { + pinned = append(pinned, Pinned{Key: k, Mode: Recursive}) + } else if p.directPin.HasKey(k) { + pinned = append(pinned, Pinned{Key: k, Mode: Direct}) + } else if p.isInternalPin(k) { + pinned = append(pinned, Pinned{Key: k, Mode: Internal}) + } else { + toCheck[k] = struct{}{} + } + } + + // Now walk all recursive pins to check for indirect pins + var checkChildren func(key.Key, key.Key) error + checkChildren = func(rk key.Key, parentKey key.Key) error { + parent, err := p.dserv.Get(context.Background(), parentKey) + if err != nil { + return err + } + for _, lnk := range parent.Links { + k := key.Key(lnk.Hash) + + if _, found := toCheck[k]; found { + pinned = append(pinned, + Pinned{Key: k, Mode: Indirect, Via: rk}) + delete(toCheck, k) + } + + err := checkChildren(rk, k) + if err != nil { + return err + } + + if len(toCheck) == 0 { + return nil + } + } + return nil + } + for _, rk := range p.recursePin.GetKeys() { + err := checkChildren(rk, rk) + if err != nil { + return nil, err + } + if len(toCheck) == 0 { + break + } + } + + // Anything left in toCheck is not pinned + for k, _ := range toCheck { + pinned = append(pinned, Pinned{Key: k, Mode: NotPinned}) + } + + return pinned, nil +} + func (p *pinner) RemovePinWithMode(key key.Key, mode PinMode) { p.lock.Lock() defer p.lock.Unlock() diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index 966c2531051..cb1deb0ff14 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -71,7 +71,7 @@ test_expect_success "can't remove pinned block" ' ' test_expect_success "can't remove pinned block: output looks good" ' - grep -q "$DIRHASH: pinned via recursive" block_rm_err + grep -q "$DIRHASH: pinned: recursive" block_rm_err ' test_expect_success "can't remove indirectly pinned block" ' @@ -79,7 +79,7 @@ test_expect_success "can't remove indirectly pinned block" ' ' test_expect_success "can't remove indirectly pinned block: output looks good" ' - grep -q "$FILE1HASH: pinned via $DIRHASH" block_rm_err + grep -q "$FILE1HASH: pinned via $DIRHASH" block_rm_err ' test_expect_success "multi-block 'ipfs block rm --ignore-pins' succeeds" ' @@ -100,10 +100,9 @@ test_expect_success "multi-block 'ipfs block rm' succeeds" ' ' test_expect_success "multi-block 'ipfs block rm' output looks good" ' - echo "removed $FILE1HASH" > expected_rm && - echo "removed $FILE2HASH" >> expected_rm && - echo "removed $FILE3HASH" >> expected_rm && - test_cmp expected_rm actual_rm + grep -F -q "removed $FILE1HASH" actual_rm && + grep -F -q "removed $FILE2HASH" actual_rm && + grep -F -q "removed $FILE3HASH" actual_rm ' test_expect_success "'ipfs block stat' with nothing from stdin doesnt crash" ' From c88b4b0941ae240a0de70f78c435a6e4339d3f40 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Sun, 14 Aug 2016 23:46:10 -0400 Subject: [PATCH 4/7] "block rm": remove the option to ignore pins License: MIT Signed-off-by: Kevin Atkinson --- core/commands/block.go | 35 +++++++---------------------------- test/sharness/t0050-block.sh | 11 +---------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/core/commands/block.go b/core/commands/block.go index 2fc324c5ea5..754a6919830 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -200,15 +200,7 @@ It takes a list of base58 encoded multihashs to remove. Arguments: []cmds.Argument{ cmds.StringArg("hash", true, true, "Bash58 encoded multihash of block(s) to remove."), }, - Options: []cmds.Option{ - cmds.BoolOption("ignore-pins", "Ignore pins.").Default(false), - }, Run: func(req cmds.Request, res cmds.Response) { - ignorePins, _, err := req.Option("ignore-pins").Bool() - if err != nil { - res.SetError(err, cmds.ErrNormal) - return - } n, err := req.InvocContext().GetNode() if err != nil { res.SetError(err, cmds.ErrNormal) @@ -225,9 +217,6 @@ It takes a list of base58 encoded multihashs to remove. go func() { defer close(outChan) pinning := n.Pinning - if ignorePins { - pinning = nil - } err := rmBlocks(n.Blockstore, pinning, outChan, keys) if err != nil { outChan <- &RemovedBlock{Error: err.Error()} @@ -271,25 +260,15 @@ type RemovedBlock struct { Error string `json:",omitempty"` } -// pins may be nil func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) error { - var unlocker bs.Unlocker - defer func() { - if unlocker != nil { - unlocker.Unlock() - } - }() - stillOkay := keys - if pins != nil { - // Need to make sure that some operation that is - // finishing with a pin is ocurr simultaneously. - unlocker = blocks.GCLock() - var err error - stillOkay, err = checkIfPinned(pins, keys, out) - if err != nil { - return fmt.Errorf("pin check failed: %s", err) - } + unlocker := blocks.GCLock() + defer unlocker.Unlock() + + stillOkay, err := checkIfPinned(pins, keys, out) + if err != nil { + return fmt.Errorf("pin check failed: %s", err) } + for _, k := range stillOkay { err := blocks.DeleteBlock(k) if err != nil { diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index cb1deb0ff14..9190cdd6516 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -82,16 +82,7 @@ test_expect_success "can't remove indirectly pinned block: output looks good" ' grep -q "$FILE1HASH: pinned via $DIRHASH" block_rm_err ' -test_expect_success "multi-block 'ipfs block rm --ignore-pins' succeeds" ' - ipfs block rm --ignore-pins $DIRHASH >actual_rm -' - -test_expect_success "multi-block 'ipfs block rm --ignore-pins' output looks good" ' - echo "removed $DIRHASH" > expected_rm && - test_cmp expected_rm actual_rm -' - -test_expect_success "fix up pins" ' +test_expect_success "remove pin" ' ipfs pin rm -r $DIRHASH ' From bf7c5b303770674fbd6731e94bb1c549f68687ca Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Tue, 16 Aug 2016 19:26:17 -0400 Subject: [PATCH 5/7] Fix bug in arccache.DeleteBlock() method. License: MIT Signed-off-by: Kevin Atkinson --- blocks/blockstore/arc_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blocks/blockstore/arc_cache.go b/blocks/blockstore/arc_cache.go index 63253ef9c00..2b6aa04e29b 100644 --- a/blocks/blockstore/arc_cache.go +++ b/blocks/blockstore/arc_cache.go @@ -32,7 +32,7 @@ func (b *arccache) DeleteBlock(k key.Key) error { switch err { case nil, ds.ErrNotFound, ErrNotFound: b.arc.Add(k, false) - return nil + return err default: return err } From 92f6747a95d83e5be0d4d1eaa7a191bbe644b25a Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Tue, 16 Aug 2016 19:51:41 -0400 Subject: [PATCH 6/7] "block rm": test case for removing pinned, valid, and non-existent blocks Add test that removes a combination of pinned, valid, and non-existent blocks in one command. License: MIT Signed-off-by: Kevin Atkinson --- test/sharness/t0050-block.sh | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index 9190cdd6516..bb9ecbb8055 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -10,13 +10,14 @@ test_description="Test block command" test_init_ipfs +HASH="QmRKqGMAM6EZngbpjSqrvYzq5Qd8b1bSWymjSUY9zQSNDk" + test_expect_success "'ipfs block put' succeeds" ' echo "Hello Mars!" >expected_in && ipfs block put actual_out ' test_expect_success "'ipfs block put' output looks good" ' - HASH="QmRKqGMAM6EZngbpjSqrvYzq5Qd8b1bSWymjSUY9zQSNDk" && echo "$HASH" >expected_out && test_cmp expected_out actual_out ' @@ -96,6 +97,38 @@ test_expect_success "multi-block 'ipfs block rm' output looks good" ' grep -F -q "removed $FILE3HASH" actual_rm ' +test_expect_success "'add some blocks' succeeds" ' + echo "Hello Mars!" | ipfs block put && + echo "Hello Venus!" | ipfs block put +' + +test_expect_success "add and pin directory" ' + ipfs add -r adir + ipfs pin add -r $DIRHASH +' + +HASH=QmRKqGMAM6EZngbpjSqrvYzq5Qd8b1bSWymjSUY9zQSNDk +HASH2=QmdnpnsaEj69isdw5sNzp3h3HkaDz7xKq7BmvFFBzNr5e7 +RANDOMHASH=QRmKqGMAM6EbngbZjSqrvYzq5Qd8b1bSWymjSUY9zQSNDq + +test_expect_success "multi-block 'ipfs block rm' mixed" ' + test_must_fail ipfs block rm $FILE1HASH $DIRHASH $HASH $FILE3HASH $RANDOMHASH $HASH2 2> block_rm_err +' + +test_expect_success "pinned block not removed" ' + ipfs block stat $FILE1HASH && + ipfs block stat $FILE3HASH +' + +test_expect_success "non-pinned blocks removed" ' + test_must_fail ipfs block stat $HASH && + test_must_fail ipfs block stat $HASH2 +' + +test_expect_success "error reported on removing non-existent block" ' + grep -q "cannot remove $RANDOMHASH" block_rm_err +' + test_expect_success "'ipfs block stat' with nothing from stdin doesnt crash" ' test_expect_code 1 ipfs block stat < /dev/null 2> stat_out ' From 8679af7a02fd356c251e4bdd9092ec5e87fe1261 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Wed, 17 Aug 2016 01:29:40 -0400 Subject: [PATCH 7/7] "block rm": add "--force" and "--quiet" option License: MIT Signed-off-by: Kevin Atkinson --- core/commands/block.go | 25 ++++++++++++++++---- test/sharness/t0050-block.sh | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/core/commands/block.go b/core/commands/block.go index 754a6919830..42bd10c6dce 100644 --- a/core/commands/block.go +++ b/core/commands/block.go @@ -13,6 +13,7 @@ import ( key "github.com/ipfs/go-ipfs/blocks/key" cmds "github.com/ipfs/go-ipfs/commands" "github.com/ipfs/go-ipfs/pin" + ds "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore" mh "gx/ipfs/QmYf7ng2hG5XBtJA3tN34DQ2GUN5HNksEw1rLDkmr6vGku/go-multihash" u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util" ) @@ -200,6 +201,10 @@ It takes a list of base58 encoded multihashs to remove. Arguments: []cmds.Argument{ cmds.StringArg("hash", true, true, "Bash58 encoded multihash of block(s) to remove."), }, + Options: []cmds.Option{ + cmds.BoolOption("force", "f", "Ignore nonexistent blocks.").Default(false), + cmds.BoolOption("quiet", "q", "Write minimal output.").Default(false), + }, Run: func(req cmds.Request, res cmds.Response) { n, err := req.InvocContext().GetNode() if err != nil { @@ -207,6 +212,8 @@ It takes a list of base58 encoded multihashs to remove. return } hashes := req.Arguments() + force, _, _ := req.Option("force").Bool() + quiet, _, _ := req.Option("quiet").Bool() keys := make([]key.Key, 0, len(hashes)) for _, hash := range hashes { k := key.B58KeyDecode(hash) @@ -217,7 +224,10 @@ It takes a list of base58 encoded multihashs to remove. go func() { defer close(outChan) pinning := n.Pinning - err := rmBlocks(n.Blockstore, pinning, outChan, keys) + err := rmBlocks(n.Blockstore, pinning, outChan, keys, rmBlocksOpts{ + quiet: quiet, + force: force, + }) if err != nil { outChan <- &RemovedBlock{Error: err.Error()} } @@ -260,7 +270,12 @@ type RemovedBlock struct { Error string `json:",omitempty"` } -func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key) error { +type rmBlocksOpts struct { + quiet bool + force bool +} + +func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, keys []key.Key, opts rmBlocksOpts) error { unlocker := blocks.GCLock() defer unlocker.Unlock() @@ -271,9 +286,11 @@ func rmBlocks(blocks bs.GCBlockstore, pins pin.Pinner, out chan<- interface{}, k for _, k := range stillOkay { err := blocks.DeleteBlock(k) - if err != nil { + if err != nil && opts.force && (err == bs.ErrNotFound || err == ds.ErrNotFound) { + // ignore non-existent blocks + } else if err != nil { out <- &RemovedBlock{Hash: k.String(), Error: err.Error()} - } else { + } else if !opts.quiet { out <- &RemovedBlock{Hash: k.String()} } } diff --git a/test/sharness/t0050-block.sh b/test/sharness/t0050-block.sh index bb9ecbb8055..3cb0aeaca6c 100755 --- a/test/sharness/t0050-block.sh +++ b/test/sharness/t0050-block.sh @@ -12,6 +12,10 @@ test_init_ipfs HASH="QmRKqGMAM6EZngbpjSqrvYzq5Qd8b1bSWymjSUY9zQSNDk" +# +# "block put tests" +# + test_expect_success "'ipfs block put' succeeds" ' echo "Hello Mars!" >expected_in && ipfs block put actual_out @@ -22,6 +26,10 @@ test_expect_success "'ipfs block put' output looks good" ' test_cmp expected_out actual_out ' +# +# "block get" tests +# + test_expect_success "'ipfs block get' succeeds" ' ipfs block get $HASH >actual_in ' @@ -30,6 +38,10 @@ test_expect_success "'ipfs block get' output looks good" ' test_cmp expected_in actual_in ' +# +# "block stat" tests +# + test_expect_success "'ipfs block stat' succeeds" ' ipfs block stat $HASH >actual_stat ' @@ -40,6 +52,10 @@ test_expect_success "'ipfs block stat' output looks good" ' test_cmp expected_stat actual_stat ' +# +# "block rm" tests +# + test_expect_success "'ipfs block rm' succeeds" ' ipfs block rm $HASH >actual_rm ' @@ -129,6 +145,34 @@ test_expect_success "error reported on removing non-existent block" ' grep -q "cannot remove $RANDOMHASH" block_rm_err ' +test_expect_success "'add some blocks' succeeds" ' + echo "Hello Mars!" | ipfs block put && + echo "Hello Venus!" | ipfs block put +' + +test_expect_success "multi-block 'ipfs block rm -f' with non existent blocks succeed" ' + ipfs block rm -f $HASH $RANDOMHASH $HASH2 +' + +test_expect_success "existent blocks removed" ' + test_must_fail ipfs block stat $HASH && + test_must_fail ipfs block stat $HASH2 +' + +test_expect_success "'add some blocks' succeeds" ' + echo "Hello Mars!" | ipfs block put && + echo "Hello Venus!" | ipfs block put +' + +test_expect_success "multi-block 'ipfs block rm -q' produces no output" ' + ipfs block rm -q $HASH $HASH2 > block_rm_out && + test ! -s block_rm_out +' + +# +# Misc tests +# + test_expect_success "'ipfs block stat' with nothing from stdin doesnt crash" ' test_expect_code 1 ipfs block stat < /dev/null 2> stat_out '