Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopt Go Modules #4146

Merged
merged 4 commits into from
Oct 10, 2019
Merged

Adopt Go Modules #4146

merged 4 commits into from
Oct 10, 2019

Conversation

mangalaman93
Copy link
Contributor

@mangalaman93 mangalaman93 commented Oct 9, 2019

This change is Reviewable

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ A review job has been created and sent to the PullRequest network.


@mangalaman93 you can click here to see the review status or cancel the code review job.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

1 Message
It looks like the description for this pull request is either blank or very short. Adding a high-level summary will help our reviewers provide better feedback. Feel free to include questions for PullRequest reviewers and make specific feedback requests.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

⚠️ Warning

PullRequest detected a force-push on this branch. This may have caused some information to be lost, and additional time may be required to complete review of the code. Read More

Copy link
Contributor

@campoy campoy left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2448 of 2448 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @manishrjain, @martinmr, and @pawanrawal)

@mangalaman93 mangalaman93 merged commit 0e73a57 into master Oct 10, 2019
@mangalaman93 mangalaman93 deleted the aman/mod branch October 10, 2019 04:21
@adg
Copy link

adg commented Oct 10, 2019

This change appears to be bad.

The go.sum file contains at least one entry that differs to the actual hash of the content.

To reproduce:

$ git clone https://github.com/dgraph-io/dgraph
$ cd dgraph/chunker
$ go test
go: downloading go.etcd.io/etcd v0.0.0-20190228193606-a943ad0ee4c9
go: downloading github.com/dgraph-io/ristretto v0.0.0-20190903064322-eb48d2f7ca30
verifying github.com/dgraph-io/ristretto@v0.0.0-20190903064322-eb48d2f7ca30: checksum mismatch
	downloaded: h1:2ymRTEtPu60vzv+YTIHHlJ3713Bl3G/MPc2BOc/OkzM=
	go.sum:     h1:FkdGlqxPjfHKdVUzS5dOSMeOkGjEG7PnR1RLbcEqw88=

SECURITY ERROR
This download does NOT match an earlier download recorded in go.sum.
The bits may have been replaced on the origin server, or an attacker may
have intercepted the download attempt.

For more information, see 'go help module-auth'.

@adg
Copy link

adg commented Oct 10, 2019

@mangalaman93
Copy link
Contributor Author

@adg thanks for pointing out. Fixed in #4153

@dihmeetree
Copy link
Contributor

dihmeetree commented Oct 10, 2019

@mangalaman93 @campoy I've updated my Dgraph version to master, but i'm still getting the error.

github.com/dgraph-io/dgraph/protos/pb
../../../go/pkg/mod/github.com/dgraph-io/dgraph@v1.1.1-0.20191010194828-771f1589e129/protos/pb/pb.pb.go:7146:29: m.Kv[iNdEx].MarshalToSizedBuffer undefined (type *pb.KV has no field or method MarshalToSizedBuffer)
../../../go/pkg/mod/github.com/dgraph-io/dgraph@v1.1.1-0.20191010194828-771f1589e129/protos/pb/pb.pb.go:7209:29: m.Kv[iNdEx].MarshalToSizedBuffer undefined (type *pb.KV has no field or method MarshalToSizedBuffer)

And I definitely have the newest version, as proved in my protos/Makefile being:
--gofast_out=plugins=grpc,Mapi.proto=github.com/dgraph-io/dgo/v2/protos/api:pb \ pb.proto

Any ideas?

@mangalaman93
Copy link
Contributor Author

Could you check if you had the protos/pb/pb.go file unmodified? We have seen this issue before and it happens when protos/pb/pb.go is generated using an old version of go-protobuf.

@dihmeetree
Copy link
Contributor

I see api "github.com/dgraph-io/dgo/v2/protos/api" in the /pb/pb.go file.. does that mean it's updated?

@mangalaman93
Copy link
Contributor Author

Check whether the file that you have locally is same as the on in master branch.

@dihmeetree
Copy link
Contributor

dihmeetree commented Oct 11, 2019

I've confirmed that my pb/pb.pb.go file is the same that's on the master branch @mangalaman93 Can you verify for me once again on your end.. and see if you can replicate my issue from a fresh install of Dgraph?

@dihmeetree
Copy link
Contributor

dihmeetree commented Oct 12, 2019

@mangalaman93 @adg

PROTO_PATH := ${PROTO_PATH}:${GOPATH}/src/github.com/dgraph-io/dgo/protos

Should this be PROTO_PATH := ${PROTO_PATH}:${GOPATH}/src/github.com/dgraph-io/dgo/v2/protos instead?

@mangalaman93
Copy link
Contributor Author

I think we assume that GOPATH is defined, we need to fix this. I think @adg has already created GitHub issues for this. May be, it is using dgo in GOPATH that has older version. I will look into it when I get a chance.

@dihmeetree
Copy link
Contributor

Thank you @mangalaman93 Hope we can get this fixed soon.. really want to start using Go modules with Dgraph soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants