search

bazelbuild / rules_cc

C++ Rules for Bazel
https://bazel.build
Apache License 2.0
182 stars 90 forks source link

Test allowing diamond inheritance of shared lib #97

Closed jlaxson closed 3 years ago

jlaxson commented 3 years ago

This is a test of the scenario described in bazelbuild/bazel#12981. Fails as described with the release version, and passes with HEAD now.

cc @oquenchil

oquenchil commented 3 years ago

The failing presubmits is from something that was rolled back earlier this week. If you merge from head they should start passing.

jlaxson commented 3 years ago

@oquenchil Updated. It looks like one of the Ubuntu builds is pulling a ~2 week old bazel build and so (properly) failing

oquenchil commented 3 years ago

Ah yes, the reason for that is that since they are different repos, we can only test this with bazel at head. The solution is to add the new package you added here at the bottom: https://github.com/bazelbuild/rules_cc/blob/master/.bazelci/presubmit.yml

I know we have test_cc_shared_library/... tested there but it doesn't seem to include subpackages.

Alternatively you can add your tests to the package one level above which would already be included in presubmit.yml, and you wouldn't have to modify that file.

Thank you.

jlaxson commented 3 years ago

@oquenchil not sure I'm following. I did add it explicitly to the test, but I think the problem is that the bazel it's pulling is

bazel --nomaster_bazelrc --bazelrc=/dev/null version
--
  | 2021/02/12 18:43:11 Using unreleased version at commit 748c442c7062ba40fa667824b8c3bad767a0c5ca

Which is from several weeks ago and doesn't have the fix merged into bazel HEAD a few days ago

% git log 748c442c7062ba40fa667824b8c3bad767a0c5ca
commit 748c442c7062ba40fa667824b8c3bad767a0c5ca
Author: Dave Nicponski <dave.nicponski@gmail.com>
Date:   Tue Jan 26 14:20:05 2021 -0800
oquenchil commented 3 years ago

It might be because you are missing /... in the presubmit file just like for the entry above.

jlaxson commented 3 years ago

@oquenchil Tried that and still fails. It looks like the last_downstream_green build is quite old as a number of rules repositories are failing on bazel HEAD. I switched it to last_green and it passes now. Seems like last_green might be what you want anyway to isolate from effects of other rules repos failing anyway, but not sure. Could keep this if you like the change, or I can put it back and just wait for last_downstream_green to update.

oquenchil commented 3 years ago

Yes, that makes sense, thanks!