Pull goleveldb from fork with bug fix #2463
Conversation
go.mod
Outdated
| @@ -68,3 +68,5 @@ require ( | |||
| gopkg.in/yaml.v2 v2.4.0 | |||
| gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect | |||
| ) | |||
|
|
|||
| replace github.com/syndtr/goleveldb => github.com/manish-sethi/goleveldb v1.0.1-0.20210305020748-912e73990f22 | |||
There was a problem hiding this comment.
The replace approach seems reasonable in this scenario, let's double check with @sykesm
There was a problem hiding this comment.
Shouldn't be a need for a replace of a fork here. The replace directive for gomega should be sufficient.
diff --git a/go.mod b/go.mod
index bc8f68448..a4a00a18a 100644
--- a/go.mod
+++ b/go.mod
@@ -2,6 +2,8 @@ module github.com/hyperledger/fabric
go 1.14
+replace github.com/onsi/gomega => github.com/onsi/gomega v1.9.0
+
require (
code.cloudfoundry.org/clock v1.0.0
github.com/DataDog/zstd v1.4.5 // indirect
@@ -36,8 +38,8 @@ require (
github.com/mattn/go-runewidth v0.0.4 // indirect
github.com/miekg/pkcs11 v1.0.3
github.com/mitchellh/mapstructure v1.3.2
- github.com/onsi/ginkgo v1.8.0
- github.com/onsi/gomega v1.9.0
+ github.com/onsi/ginkgo v1.14.0
+ github.com/onsi/gomega v1.10.1
github.com/opencontainers/runc v1.0.0-rc8 // indirect
github.com/pelletier/go-toml v1.8.0 // indirect
github.com/pierrec/lz4 v2.6.0+incompatible // indirect
@@ -53,7 +55,7 @@ require (
github.com/stretchr/objx v0.3.0 // indirect
github.com/stretchr/testify v1.7.1-0.20210116013205-6990a05d54c2 // includes ErrorContains
github.com/sykesm/zap-logfmt v0.0.2
- github.com/syndtr/goleveldb v1.0.1-0.20190625010220-02440ea7a285
+ github.com/syndtr/goleveldb v1.0.1-0.20210305035536-64b5b1c73954
github.com/tedsuo/ifrit v0.0.0-20180802180643-bea94bb476cc
github.com/willf/bitset v1.1.10
go.etcd.io/etcd v0.5.0-alpha.5.0.20181228115726-23731bf9ba55
Edit: s/require/replace/
There was a problem hiding this comment.
The replace was mainly for picking up the actual fix (about manifest corruption). I just noticed that the goleveldb maintainer merged my PR last night shortly after I submitted this PR and hence perhaps you misunderstood the intention of this PR. With the current state, yes, it makes sense to undo this PR and just replace for the lowering down the gomega dependency.
There was a problem hiding this comment.
Does that mean you’re planning to do that or does someone else need to take an action? We obviously want to pick up the fix; it’s just a matter of picking it up from upstream instead of your fork.
There was a problem hiding this comment.
During our scrum Manish said he will do it.
There was a problem hiding this comment.
Does that mean you’re planning to do that or does someone else need to take an action? We obviously want to pick up the fix; it’s just a matter of picking it up from upstream instead of your fork.
Yes, picking from upstream was always the preference. However, the goleveldb did not merge my PR for 2 weeks. Moreover, there were other PRs pending since last six months. So, we were kind of left with not much alternate. Pushed the latest patch to upgrade goleveldb.
|
I'll let the ledger experts comment on the tech. details, I'm just wondering whether we might not want to move the dependency in under an hyperledger github account? |
This commit upgrades goleveldb. This upgraded version includes a fix for the manifest file corruption issue [FAB-18304]. The latest goleveldb requires gomega v1.10.1, which in turn requires protobuf 1.4.2. Fabric has some compatibilities issues with protobuf 1.4.2 [FAB-18363], so, sticking with gomega v1.9.0 which otherwise works with goleveldb. Signed-off-by: manish <manish.sethi@gmail.com>
manish-sethi
force-pushed
the
goleveldb_with_fix_fsync_manifest
branch
from
fa756ab
to
47864a0
Compare
Signed-off-by: manish manish.sethi@gmail.com
Type of change
Description
This PR upgrades goleveldb. This upgraded version includes a fix for the manifest file corruption issue [FAB-18304].
The latest goleveldb requires gomega v1.10.1, which in turn requires protobuf 1.4.2. Fabric has some compatibilities issues with protobuf 1.4.2 [FAB-18363], so, sticking with gomega v1.9.0, which otherwise works with goleveldb.
Related issues
FAB-18304
FAB-18363