Skip to content

bazel: fix the compilation of .proto on Darwin #63879

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
Merged
merged 1 commit into from
Jun 29, 2019
Merged

bazel: fix the compilation of .proto on Darwin #63879

merged 1 commit into from
Jun 29, 2019
+251 −5

Conversation

kalbasit
Copy link
Member

@kalbasit kalbasit commented Jun 28, 2019 •
edited
Loading

Motivation for this change

On Darwin, the last argument to GCC is coming up as an empty string. This is breaking the build of proto_library targets. However, I was not able to reproduce with the example cpp project.

This commit patches the cc_wrapper of Bazel that gets installed on
Darwin to remove the last argument if it's an empty string. This is
not a problem on Linux.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @guibou @Profpatsch

Sorry, something went wrong.

@kalbasit kalbasit requested a review from Profpatsch as a code owner June 28, 2019 02:28
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jun 28, 2019
@kalbasit kalbasit added the 2.status: work-in-progress This PR isn't done label Jun 28, 2019
@kalbasit kalbasit changed the title Nixpkgs bazel protobuf darwin [WIP] bazel: fix Protocol Buffers on Darwin Jun 28, 2019
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jun 28, 2019
@kalbasit
Copy link
Member Author

@GrahamcOfBorg build bazel.tests

@kalbasit
Copy link
Member Author

kalbasit commented Jun 28, 2019 •
edited
Loading

@guibou @Profpatsch as you can see in the ofborg result, this works well on Linux but breaks on Darwin. This test replicates the environment that I run Bazel in my proprietary project.

I can't see the full error here as it gets snipped, but on my project, I can see this (snipped unrelated args):

external/local_config_cc/cc_wrapper.sh -o bazel-out/host/bin/external/com_google_protobuf/protoc [ ..snip.. ] -lpthread -lm -Wl,-S -undefined dynamic_lookup -headerpad_max_install_names -lstdc++ -lm ''

The issue is happening at the very last argument -lm ''. I remove the empty string denoted by '', I'm able to compile correctly. Why is it getting an empty string still?

I'd love to figure out how to fix this and include this test in nixpkgs to help us determine the C/C++ environment for NixOS/Darwin.

@kalbasit
Copy link
Member Author

kalbasit commented Jun 28, 2019 •
edited
Loading

I just pushed a change enabling --verbose_failures. Now, we can see the same error I see on the other project.

@GrahamcOfBorg build bazel.tests

  (cd /private/var/tmp/_bazel_yl/e20faadaf9a0f54dcd282d21d9753065/sandbox/processwrapper-sandbox/3/execroot/our_workspace && \
  exec env - \
    PATH=/nix/store/2p5k5g7byv3vnswwlj5gramgrh31zdgm-clang-wrapper-7.1.0/bin:/nix/store/28g4y6vvypdqgfij86r8vzff3j6bb1w5-clang-7.1.0/bin:/nix/store/5ibk8xxh2ly7gypv14dyjrb4c5ly5306-coreutils-8.31/bin:/nix/store/1l2xnahjr23k1zgwfiv89c6w88jddvlz-cctools-binutils-darwin-wrapper/bin:/nix/store/b3yssmqyfc8yid9vyfq41g0m0gf38dj1-cctools-binutils-darwin/bin:/nix/store/5ibk8xxh2ly7gypv14dyjrb4c5ly5306-coreutils-8.31/bin:/nix/store/6lx2gmb5hk4bnyxrm7ldda6yg7rrpbgy-python3-3.7.3/bin:/nix/store/5ibk8xxh2ly7gypv14dyjrb4c5ly5306-coreutils-8.31/bin:/nix/store/5j429ifcj56whma1h565f9f8iw7iv6dj-findutils-4.6.0/bin:/nix/store/9rls681q6pzzrjrizpiigm29d87sgril-diffutils-3.7/bin:/nix/store/fah3qi1j15and1rdi3b9s42qfimvnq2i-gnused-4.7/bin:/nix/store/i7hnsnag89v55c9f6gzsmjm7hs3c5ygy-gnugrep-3.3/bin:/nix/store/xl08nd4gshlwxkrjz3h0sgyknznqhk4g-gawk-4.2.1/bin:/nix/store/7s4rfk8m58ic458was3ma2m3ricrsn5y-gnutar-1.32/bin:/nix/store/dchmhy3mlc3ivjnmhiq1x0a4b57y7mba-gzip-1.10/bin:/nix/store/8g5abpbdn1pa2cd359fydm87hi792z4j-bzip2-1.0.6.0.1-bin/bin:/nix/store/kpx5d5nwbccqr6jsnq2g6ladv8aknqcm-gnumake-4.2.1/bin:/nix/store/mlsjkws5zc9dja87npqy8g1vrfx1rf23-bash-4.4-p23/bin:/nix/store/zyv7avf4zcidnmds1j4b6qyv83hzr25w-patch-2.7.6/bin:/nix/store/lkjx3bfy5fimyxgzazzqna1dq6n1q6kh-xz-5.2.4-bin/bin:/nix/store/mlsjkws5zc9dja87npqy8g1vrfx1rf23-bash-4.4-p23/bin:/nix/store/5ibk8xxh2ly7gypv14dyjrb4c5ly5306-coreutils-8.31/bin:/nix/store/5j429ifcj56whma1h565f9f8iw7iv6dj-findutils-4.6.0/bin:/nix/store/xl08nd4gshlwxkrjz3h0sgyknznqhk4g-gawk-4.2.1/bin:/nix/store/i7hnsnag89v55c9f6gzsmjm7hs3c5ygy-gnugrep-3.3/bin:/nix/store/j1c5z9bf19943m7dwp6d94hgsf847y3d-gnutar-1.32/bin:/nix/store/fah3qi1j15and1rdi3b9s42qfimvnq2i-gnused-4.7/bin:/nix/store/dchmhy3mlc3ivjnmhiq1x0a4b57y7mba-gzip-1.10/bin:/nix/store/3dzl9y7ii4ba8vjfz3h5wh8m1kgci0nh-which-2.21/bin:/nix/store/xx360szy4vblxn8zw1cfrgivg38xqalz-unzip-6.0/bin \
    PWD=/proc/self/cwd \
  external/local_config_cc/cc_wrapper.sh -o bazel-out/host/bin/external/com_google_protobuf/protoc bazel-out/host/bin/external/com_google_protobuf/_objs/protoc/main.o bazel-out/host/bin/external/com_google_protobuf/libprotoc_lib.a bazel-out/host/bin/external/com_google_protobuf/libprotobuf.a bazel-out/host/bin/external/com_google_protobuf/libprotobuf_lite.a bazel-out/host/bin/external/net_zlib/libzlib.a -lpthread -lm -lpthread -lm -lpthread -lm -lpthread -lm -Wl,-S -undefined dynamic_lookup -headerpad_max_install_names -lstdc++ -lm '')
Execution platform: @bazel_tools//platforms:host_platform

Use --sandbox_debug to see verbose messages from the sandbox
/nix/store/3307a5slhaq4jnpibhkih5k681wksbhy-bash/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.ISO-8859-1): No such file or directory
/nix/store/mlsjkws5zc9dja87npqy8g1vrfx1rf23-bash-4.4-p23/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.ISO-8859-1): No such file or directory
gcc: error: : No such file or directory

@kalbasit kalbasit force-pushed the nixpkgs_bazel_protobuf_darwin branch from 89a948b to 2eb1991 Compare June 28, 2019 04:46
@kalbasit kalbasit changed the title [WIP] bazel: fix Protocol Buffers on Darwin bazel: fix the compilation of .proto on Darwin Jun 28, 2019
@kalbasit kalbasit force-pushed the nixpkgs_bazel_protobuf_darwin branch 5 times, most recently from b3a32c5 to 564a711 Compare June 28, 2019 04:53
@kalbasit
Copy link
Member Author

@GrahamcOfBorg build bazel.tests

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jun 28, 2019
@kalbasit kalbasit removed the 2.status: work-in-progress This PR isn't done label Jun 28, 2019
@kalbasit kalbasit force-pushed the nixpkgs_bazel_protobuf_darwin branch from 564a711 to ee68277 Compare June 28, 2019 19:34
@kalbasit
Copy link
Member Author

@GrahamcOfBorg build bazel.tests

@kalbasit kalbasit changed the title bazel: fix the compilation of .proto on Darwin [WIP] bazel: fix the compilation of .proto on Darwin Jun 29, 2019
@kalbasit kalbasit added the 2.status: work-in-progress This PR isn't done label Jun 29, 2019
@kalbasit
Copy link
Member Author

kalbasit commented Jun 29, 2019 •
edited
Loading

It turned out to be still broken for the java_proto_library. I can replicate within my proprietary project all the way back to 0.22 in 555734e

EDIT: I'll merge this one without the java test and file a separate PR to deal with it.

@kalbasit kalbasit changed the title [WIP] bazel: fix the compilation of .proto on Darwin bazel: fix the compilation of .proto on Darwin Jun 29, 2019
@kalbasit kalbasit removed the 2.status: work-in-progress This PR isn't done label Jun 29, 2019
@kalbasit
bazel: fix the compilation of .proto on Darwin

Verified

This commit was signed with the committer’s verified signature. The key has expired.
kalbasit Wael Nasreddine
GPG key ID: FD437548E0BF0F5F
Expired
Verified
Learn about vigilant mode
a6650e2 
On Darwin, the last argument to GCC is coming up as an empty string.
This is breaking the build of proto_library targets. However, I was not
able to reproduce with the example cpp project[0].

This commit patches the cc_wrapper of Bazel that gets installed on
Darwin to remove the last argument if it's an empty string. This is
not a probem on Linux.

[0]: https://github.com/bazelbuild/examples/tree/master/cpp-tutorial/stage3
@kalbasit kalbasit force-pushed the nixpkgs_bazel_protobuf_darwin branch from 57a8d0f to a6650e2 Compare June 29, 2019 16:55
@kalbasit
Copy link
Member Author

@GrahamcOfBorg build bazel.tests

@kalbasit kalbasit merged commit c49b7f6 into NixOS:master Jun 29, 2019
@kalbasit kalbasit deleted the nixpkgs_bazel_protobuf_darwin branch June 29, 2019 18:07
@Profpatsch
Copy link
Member

Thanks, great work!

It does look suspiciously like a bazel bug though, can you open an upstream issue?

@aherrmann-da aherrmann-da mentioned this pull request Aug 19, 2019
Merged
6 tasks
@kalbasit kalbasit mentioned this pull request Oct 25, 2019
Closed
@flokli
Copy link
Contributor

flokli commented Oct 25, 2019

@kalbasit did you open an issue upstream? I don't see anything linked here…

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

@Profpatsch Profpatsch

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kalbasit @Profpatsch @flokli