Merge pull request #4398 from ipfs/fix/hamt-bug

fix hamt delete issue

Author: whyrusleeping

.travis.yml

@@ -10,7 +10,7 @@ os:
 language: go
 
 go:
-    - 1.8
+    - 1.9
 
 env:
   - TEST_NO_FUSE=1 TEST_VERBOSE=1 TEST_SUITE=test_go_expensive

README.md

@@ -104,7 +104,7 @@ With snap, in any of the [supported Linux distributions](https://snapcraft.io/do
 
 #### Install Go
 
-The build process for ipfs requires Go 1.8 or higher. If you don't have it: [Download Go 1.8+](https://golang.org/dl/).
+The build process for ipfs requires Go 1.9 or higher. If you don't have it: [Download Go 1.9+](https://golang.org/dl/).
 
 
 You'll need to add Go's bin directories to your `$PATH` environment variable e.g., by adding these lines to your `/etc/profile` (for a system-wide installation) or `$HOME/.profile`:
@@ -148,7 +148,7 @@ mismatched APIs.
 * Also, [instructions for OpenBSD](docs/openbsd.md).
 * `git` is required in order for `go get` to fetch all dependencies.
 * Package managers often contain out-of-date `golang` packages.
-  Ensure that `go version` reports at least 1.8. See above for how to install go.
+  Ensure that `go version` reports at least 1.9. See above for how to install go.
 * If you are interested in development, please install the development
 dependencies as well.
 * *WARNING: Older versions of OSX FUSE (for Mac OS X) can cause kernel panics when mounting!*

bin/Rules.mk

@@ -1,9 +1,9 @@
 include mk/header.mk
 
-dist_root_$(d)=/ipfs/QmR27Do9gqx9VmuQTEX1UGXETSWYJTQzPzxS5FNUnySCv1
+dist_root_$(d)=/ipfs/QmT3CLJKJzWPuN4NAN4LLy69UpKskMF3AuYhXstKdn8V43
 
-$(d)/gx: $(d)/gx-v0.12.0
-$(d)/gx-go: $(d)/gx-go-v1.5.0
+$(d)/gx: $(d)/gx-v0.12.1
+$(d)/gx-go: $(d)/gx-go-v1.6.0
 
 TGTS_$(d) := $(d)/gx $(d)/gx-go
 DISTCLEAN += $(wildcard $(d)/gx-v*) $(wildcard $(d)/gx-go-v*) $(d)/tmp

ci/Dockerfile.buildenv

@@ -1,4 +1,4 @@
-FROM golang:1.8
+FROM golang:1.9
 MAINTAINER Jakub Sztandera <[email protected]>
 
 

circle.yml

@@ -12,8 +12,8 @@ machine:
 
   post:
     - sudo rm -rf /usr/local/go
-    - if [ ! -e go1.8.3.linux-amd64.tar.gz ]; then curl -o go1.8.3.linux-amd64.tar.gz https://storage.googleapis.com/golang/go1.8.3.linux-amd64.tar.gz; fi
-    - sudo tar -C /usr/local -xzf go1.8.3.linux-amd64.tar.gz
+    - if [ ! -e go1.9.2.linux-amd64.tar.gz ]; then curl -o go1.9.2.linux-amd64.tar.gz https://storage.googleapis.com/golang/go1.9.2.linux-amd64.tar.gz; fi
+    - sudo tar -C /usr/local -xzf go1.9.2.linux-amd64.tar.gz
 
   services:
     - docker
@@ -30,7 +30,7 @@ dependencies:
     - cd "$HOME/.go_workspace/src/$IMPORT_PATH" && make deps
 
   cache_directories:
-    - ~/go1.8.3.linux-amd64.tar.gz
+    - ~/go1.9.2.linux-amd64.tar.gz
     - "$HOME/.go_workspace/src/gx/ipfs"
 
 test:

mk/golang.mk

@@ -1,5 +1,5 @@
 # golang utilities
-GO_MIN_VERSION = 1.8
+GO_MIN_VERSION = 1.9
 
 # pre-definitions
 GOCC ?= go

package.json

@@ -4,7 +4,7 @@
   },
   "gx": {
     "dvcsimport": "github.com/ipfs/go-ipfs",
-    "goversion": "1.8"
+    "goversion": "1.9"
   },
   "gxDependencies": [
     {

unixfs/hamt/hamt.go

@@ -492,39 +492,44 @@ func (ds *HamtShard) modifyValue(ctx context.Context, hv *hashBits, key string,
 
 		return nil
 	case *shardValue:
-		switch {
-		case val == nil: // passing a nil value signifies a 'delete'
-			ds.bitfield.SetBit(ds.bitfield, idx, 0)
-			return ds.rmChild(cindex)
+		if child.key == key {
+			// value modification
+			if val == nil {
+				ds.bitfield.SetBit(ds.bitfield, idx, 0)
+				return ds.rmChild(cindex)
+			}
 
-		case child.key == key: // value modification
 			child.val = val
 			return nil
+		}
 
-		default: // replace value with another shard, one level deeper
-			ns, err := NewHamtShard(ds.dserv, ds.tableSize)
-			if err != nil {
-				return err
-			}
-			ns.prefix = ds.prefix
-			chhv := &hashBits{
-				b:        hash([]byte(child.key)),
-				consumed: hv.consumed,
-			}
+		if val == nil {
+			return os.ErrNotExist
+		}
 
-			err = ns.modifyValue(ctx, hv, key, val)
-			if err != nil {
-				return err
-			}
+		// replace value with another shard, one level deeper
+		ns, err := NewHamtShard(ds.dserv, ds.tableSize)
+		if err != nil {
+			return err
+		}
+		ns.prefix = ds.prefix
+		chhv := &hashBits{
+			b:        hash([]byte(child.key)),
+			consumed: hv.consumed,
+		}
 
-			err = ns.modifyValue(ctx, chhv, child.key, child.val)
-			if err != nil {
-				return err
-			}
+		err = ns.modifyValue(ctx, hv, key, val)
+		if err != nil {
+			return err
+		}
 
-			ds.setChild(cindex, ns)
-			return nil
+		err = ns.modifyValue(ctx, chhv, child.key, child.val)
+		if err != nil {
+			return err
 		}
+
+		ds.setChild(cindex, ns)
+		return nil
 	default:
 		return fmt.Errorf("unexpected type for child: %#v", child)
 	}

unixfs/hamt/hamt_test.go

@@ -222,6 +222,20 @@ func TestRemoveElems(t *testing.T) {
 	}
 	ctx := context.Background()
 
+	for i := 0; i < 100; i++ {
+		err := s.Remove(ctx, fmt.Sprintf("NOTEXIST%d", rand.Int()))
+		if err != os.ErrNotExist {
+			t.Fatal("shouldnt be able to remove things that don't exist")
+		}
+	}
+
+	for _, d := range dirs {
+		_, err := s.Find(ctx, d)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
 	shuffle(time.Now().UnixNano(), dirs)
 
 	for _, d := range dirs {

unixfs/hamt/util.go

@@ -2,6 +2,7 @@ package hamt
 
 import (
 	"math/big"
+	"math/bits"
 )
 
 // hashBits is a helper that allows the reading of the 'next n bits' as an integer.
@@ -39,25 +40,10 @@ func (hb *hashBits) Next(i int) int {
 	}
 }
 
-const (
-	m1  = 0x5555555555555555 //binary: 0101...
-	m2  = 0x3333333333333333 //binary: 00110011..
-	m4  = 0x0f0f0f0f0f0f0f0f //binary:  4 zeros,  4 ones ...
-	h01 = 0x0101010101010101 //the sum of 256 to the power of 0,1,2,3...
-)
-
-// from https://en.wikipedia.org/wiki/Hamming_weight
-func popCountUint64(x uint64) int {
-	x -= (x >> 1) & m1             //put count of each 2 bits into those 2 bits
-	x = (x & m2) + ((x >> 2) & m2) //put count of each 4 bits into those 4 bits
-	x = (x + (x >> 4)) & m4        //put count of each 8 bits into those 8 bits
-	return int((x * h01) >> 56)
-}
-
 func popCount(i *big.Int) int {
 	var n int
 	for _, v := range i.Bits() {
-		n += popCountUint64(uint64(v))
+		n += bits.OnesCount64(uint64(v))
 	}
 	return n
 }