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.

Sign up for GitHub

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

Jump to bottom

bazel: Update to 0.27.0 #7391

Merged
merged 13 commits into from
Jun 26, 2019
Merged

bazel: Update to 0.27.0 #7391

merged 13 commits into from
Jun 26, 2019

Conversation

keith
Copy link
Member

@keith keith commented Jun 24, 2019

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

@keith
bazel: Update to 0.27.0
b6d7b9c 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith mentioned this pull request Jun 24, 2019
Closed
@keith
Force some tools to run with python 2
dc2a204 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
test: update socket_passing.py to python3
09cf0c6 
This script is used for the hotrestart_test target, this makes it run
with python3 instead of python2

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Copy link
Member Author

keith commented Jun 25, 2019 •
edited
Loading

This depends on grailbio/bazel-compilation-database#30

@keith
Add python_version to tap2pcap_test
097f439 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@htuch htuch self-assigned this Jun 25, 2019
@keith
Update gcovr for Python 3.5+ support
c7d192c 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Temporarily change compilation database URL
c0aecb6 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Downgrade gcovr because of breaking changes
8a577ea 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Update version
c89d0b1 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith mentioned this pull request Jun 25, 2019
Merged
@keith
Update bazel-compilation-database again
62f7775 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
htuch
htuch approved these changes Jun 25, 2019
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks!

bazel/gen_compilation_database.sh Outdated Show resolved Hide resolved
htuch
htuch suggested changes Jun 25, 2019
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

(undo approval)

@htuch htuch added the waiting label Jun 25, 2019
@keith
Copy link
Member Author

keith commented Jun 25, 2019

Hrm I'm not sure about the coverage failure here, any ideas?

This was referenced Jun 25, 2019
Closed
Closed
Closed
@keith
Go back to upstream version with merged PR
269d4ad 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Go back to tags
953284d 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@htuch
Copy link
Member

htuch commented Jun 26, 2019

@keith this looks like it's a Python 3 thing in gcovr. I'm a little surprised that a change to Bazel could have this impact, as gcovr is run outside of Bazel, see https://github.com/envoyproxy/envoy/blob/master/test/run_envoy_bazel_coverage.sh#L95

@moderation
Copy link
Contributor

bazel-compilation-database 0.3.5 is released

@keith
Copy link
Member Author

keith commented Jun 26, 2019

I've updated to that version here

htuch
htuch reviewed Jun 26, 2019
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM modulo the coverage issue.
/wait

@keith
Copy link
Member Author

keith commented Jun 26, 2019

@htuch we do build it with bazel though https://github.com/keith/envoy/blob/953284d56f42dbe9fe89805441f9b0fc7479e362/ci/do_ci.sh#L247-L251

@keith
Default gcovr to PY2
c2f0a2b 
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Force py2 for gcovr
d04a8da 
Without both of these subpar thinks the main and src files don't match
because the generated files end up in different subpaths of bazel-out
because of the differences in toolchains

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith
Copy link
Member Author

keith commented Jun 26, 2019

Ok I think I finally got par_binary to use the right python version with 0.27.0, we'll see tomorrow

@keith
Copy link
Member Author

keith commented Jun 26, 2019

Finally green 🎉

@keith keith mentioned this pull request Jun 26, 2019
Closed
@htuch htuch merged commit c37b3ab into envoyproxy:master Jun 26, 2019
@keith keith deleted the ks/update-bazel branch June 26, 2019 20:00
brandjon
brandjon reviewed Jun 26, 2019
View reviewed changes
ci/do_ci.sh
@@ -246,7 +246,7 @@ elif [[ "$CI_TARGET" == "bazel.coverage" ]]; then

# gcovr is a pain to run with `bazel run`, so package it up into a
# relocatable and hermetic-ish .par file.
bazel build @com_github_gcovr_gcovr//:gcovr.par
bazel build --python_version=PY2 @com_github_gcovr_gcovr//:gcovr.par
Copy link

Choose a reason for hiding this comment

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

FYI, the Python version is always set by either --host_force_python (for host-configured targets) or the python_version attr of py_binary/py_test targets. If these are not explicitly specified they default to PY3.

So setting --python_version on the command line has no effect on executable Python targets. Its only use is to cause other targets (e.g. a py_library, or some non-Python rule) to have PY2 or PY3 mode for the purposes of evaluating select()s on the Python version and for deciding whether to use the -py2 output directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

@brandjon note this is required because of the genrule that outputs the source file. Without this, when forcing py2 on this par_binary target, subpar believes that the main file is not contained in the srcs because it's not nested in the -py2 output directory.

You can reproduce this by running this command with and without this flag, without it subpar fails here https://github.com/google/subpar/blob/35bb9f0092f71ea56b742a520602da9b3638a24f/subpar.bzl#L29-L30

Copy link

Choose a reason for hiding this comment

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

I see, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandjon brandjon brandjon left review comments

@htuch htuch htuch approved these changes

Assignees

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@keith @htuch @moderation @brandjon