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

Fix building ijar_cc_binary and singlejar_cc_bin without sandbox by removing dependency on @bazel_tools//tools/cpp:malloc #8954

Conversation

meteorcloudy
Copy link
Member

@meteorcloudy meteorcloudy commented Jul 23, 2019

As @laszlocsomor explained, building ijar_cc_binary and singlejar_cc_bin on Windows (where sandbox is not available yet) will have header conflict due to the same headers are shipped in both @bazel_tools and @local_java_tools.

ijar_cc_binary and singlejar_cc_bin only declare headers in @local_java_tools as dependency, but it will search external/bazel_tools first because every cc targets depend on @bazel_tools//tools/cpp:malloc implicitly.

This change remove the dependency on @bazel_tools//tools/cpp:malloc to avoid this confusion.

@lberki I believe cc targets depending on @bazel_tools//tools/cpp:malloc is a behavior that exists for a long time, is it still necessary? Looks like it's leaking /Iexternal/tools to all cc compilation and causing some header conflicts in non-sandboxed build. This is not the first time I've seen such error.
FYI @oquenchil

This change replaces #8945 and we no longer have to do a patch release for Bazel, just a new release for the java tools.

@lberki
Copy link
Contributor

lberki commented Jul 23, 2019

/cc

@lberki lberki requested review from iirina and removed request for lberki July 23, 2019 11:33
@lberki
Copy link
Contributor

lberki commented Jul 23, 2019

Erm, mousing fail. I menat to remove myself and not @iirina from the reviewer list.

Copy link
Contributor

@iirina iirina left a comment

Choose a reason for hiding this comment

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

Woohoo all tests pass \o/

@lberki I believe cc targets depending on @bazel_tools//tools/cpp:malloc is a behavior that exists for a long time, is it still necessary? Looks like it's leaking /Iexternal/tools to all cc compilation and causing some header conflicts in non-sandboxed build. This is not the first time I've seen such error.
FYI @oquenchil

Since Lukacs seems busy now I suggest opening an issue with the C++ team to investigate this further.

@iirina
Copy link
Contributor

iirina commented Jul 23, 2019

This is awesome, thank you! Let's get it in \o/

@meteorcloudy
Copy link
Member Author

Since Lukacs seems busy now I suggest opening an issue with the C++ team to investigate this further.

I agree! Let's merge this!

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

Successfully merging this pull request may close these issues.

4 participants