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

Update rules_go, bazel_gazelle, and go-ethereum to Support Go 1.11 #490

Merged

Conversation

prestonvanloon
Copy link
Member

@prestonvanloon prestonvanloon commented Sep 9, 2018

The latest version of rules_go and bazel_gazelle support go 1.11 which has a lot of nice features.

I don't foresee any issues, but please checkout this branch and build it on your local machine before clicking approve.

Need

  • Approval build from MacOS
  • Approval build from Linux

I've built and ran tests for ubuntu, but I haven't tested on MacOS.

@prestonvanloon prestonvanloon added the Enhancement New feature or request label Sep 9, 2018
@prestonvanloon prestonvanloon added this to the Backlog milestone Sep 9, 2018
@codecov
Copy link

codecov bot commented Sep 9, 2018

Codecov Report

Merging #490 into master will decrease coverage by 0.35%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #490      +/-   ##
==========================================
- Coverage   78.27%   77.91%   -0.36%     
==========================================
  Files          39       39              
  Lines        2536     2536              
==========================================
- Hits         1985     1976       -9     
- Misses        378      385       +7     
- Partials      173      175       +2
Impacted Files Coverage Δ
beacon-chain/rpc/service.go 92.85% <0%> (-4.77%) ⬇️
beacon-chain/sync/initial-sync/service.go 68.33% <0%> (-4.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab3c0ef...86e8a81. Read the comment docs.

Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

I got the following errors from bazel run //:gazelle

git≠prestonvanloon-update-rules-go-and-gazelle)❱✔≻ bazel run //:gazelle 
Starting local Bazel server and connecting to it...
..............................
Analyzing: target //:gazelle (7 packages loaded)
Analyzing: target //:gazelle (7 packages loaded)
ERROR: /private/var/tmp/_bazel_terencetsao/3136447ae55d487a443f50acb4025ea7/external/go_sdk/BUILD.bazel:20:1: @go_sdk//:srcs: invalid label 'src/cmd/go/testdata/mod/rsc.io_!c!g!o_v1.0.0.txt' in element 813 of attribute 'srcs' in 'filegroup' rule: invalid target name 'src/cmd/go/testdata/mod/rsc.io_!c!g!o_v1.0.0.txt': target names may not contain '!'
ERROR: /private/var/tmp/_bazel_terencetsao/3136447ae55d487a443f50acb4025ea7/external/go_sdk/BUILD.bazel:57:1: @go_sdk//:files: invalid label 'src/cmd/go/testdata/mod/rsc.io_!c!g!o_v1.0.0.txt' in element 1242 of attribute 'srcs' in 'filegroup' rule: invalid target name 'src/cmd/go/testdata/mod/rsc.io_!c!g!o_v1.0.0.txt': target names may not contain '!'
ERROR: Analysis of target '//:gazelle' failed; build aborted: error loading package '@go_sdk//': Package '' contains errors

@prestonvanloon
Copy link
Member Author

Oh boy! Ok, I'll look into that.

Meanwhile, I've created #491 to have travis catch these types of things.

@prestonvanloon prestonvanloon self-assigned this Sep 10, 2018
Copy link
Member

@terencechain terencechain left a comment

Choose a reason for hiding this comment

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

Works on a mac with bazel-0.16.1
Was able to build validator and run gazelle.

Copy link
Member

@nisdas nisdas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rauljordan rauljordan left a comment

Choose a reason for hiding this comment

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

Confirmed works on ubuntu 16.04

@rauljordan rauljordan merged commit 78c7633 into prysmaticlabs:master Sep 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants