Skip to content
This repository has been archived by the owner on Dec 2, 2021. It is now read-only.

EVMCC & Fab3 Dependency Split #20

Merged
merged 3 commits into from
Nov 20, 2019

Conversation

swetharepakula
Copy link
Contributor

@swetharepakula swetharepakula commented Nov 12, 2019

@swetharepakula swetharepakula changed the title Dummy commit to create PR EVMCC & Fab3 Dependency Split Nov 12, 2019
@swetharepakula swetharepakula force-pushed the restructure branch 2 times, most recently from 6ed74de to 72fbb28 Compare November 12, 2019 19:08
Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some failed merges to resolve. There are files that have conflicts inside, as well as extra files that need to be deleted.

go1.13 finds some gosum hashing issues, but I'm not sure if it's due to the merge or something else

are we excising dep completely? Is it still used for anything?

I need to check on replace directives for modules.

It looks like there's some stuff that doesn't use the same versions as it used to, including using lesser versions now. Probably an artifact of them mostly being v0.x versions.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
ci/azure-pipelines.yml Show resolved Hide resolved
evmcc/go.sum Outdated Show resolved Hide resolved
evmcc/vendor/modules.txt Show resolved Hide resolved
fab3/go.mod Show resolved Hide resolved
gotools.mk Outdated
@@ -22,19 +22,14 @@ gotools-clean:
-@rm -rf $(BUILD_DIR)/gotools

# Special override for ginkgo since we want to use the version vendored with the project
gotool.dep: GINKGO_VERSION ?= "v1.10.2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotool.dep? Huh? should match gotool.ginkgo, I think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the version, I feel like we were tied to the version in fabric, previously, so that's pretty cool if it's not anymore.

gotools.mk Show resolved Hide resolved
gotools.mk Outdated Show resolved Hide resolved
- since fabric chaincode environments do not support go modules yet,
evmcc needs a local vendor directory
- this commit one in a series of commits to split the evmcc and fab3
dependencies

Signed-off-by: Swetha Repakula <srepaku@us.ibm.com>
@swetharepakula swetharepakula force-pushed the restructure branch 3 times, most recently from 7b7fbe7 to f2345c6 Compare November 12, 2019 20:53
Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think only nits.

Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved

[[override]]
name = "github.com/go-kit/kit"
version = "0.8.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this being the version of go-kit we had before this change.

integration/go.mod Show resolved Hide resolved
integration/go.mod Outdated Show resolved Hide resolved
)

for i in "${arr[@]}"
declare -a goModules=(
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably used in a later commit?

)

declare -a goModules=(
"./fab3"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see it now, 2nd commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea not the cleanest division of the commits

exit 1
fi
done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something something shell magic to merge these two arrays and remove the duplication.

needs research, I don't know off the top of my head.

scripts/golinter.sh Outdated Show resolved Hide resolved
scripts/goListFiles.sh Outdated Show resolved Hide resolved
 - use go modules for entire repo
 - Fab3 requires $GO111MODULE to be set to on to run, test, or build as
 there is no local vendor directory for the module
 - integration still needs a vendor directory as it uses Fabric
 repository which has not been converted to modules yet
 - remove dep make targets
 - modify scripts to enable go modules when dealing with fab3

Signed-off-by: Swetha Repakula <srepaku@us.ibm.com>
The script is not used anywhere in the repo so remove it

Signed-off-by: Swetha Repakula <srepaku@us.ibm.com>
Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine now if the nits are the only thing that's changed. Did not do a full review. Difficult to diff.

@MHBauer MHBauer merged commit bf3fbd5 into hyperledger-archives:master Nov 20, 2019
@MHBauer MHBauer deleted the restructure branch November 20, 2019 23:01
@MHBauer
Copy link
Contributor

MHBauer commented Nov 20, 2019

I didn't delete anything. I thought we turned that off?

@swetharepakula
Copy link
Contributor Author

No, I don't think we turned that off.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants