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

Hard-wire --experimental_sibling_repository_layout to false. #21785

Closed
wants to merge 1 commit into from

Conversation

lberki
Copy link
Contributor

@lberki lberki commented Mar 25, 2024

According to the discussion
#20500, no one is too attached to it and the project, sadly, failed: given the current climate, it doesn't look like we'll ever have enough political will to go through with the necessary migration to flip the default value of the flag, as good an idea as it is on its own merits.

This change is designed to minimize the number of lines changed so that it's easy to roll back if need be.

According to the discussion
#20500, no one is too
attached to it and the project, sadly, failed: given the current
climate, it doesn't look like we'll ever have enough political will to
go through with the necessary migration to flip the default value of the
flag, as good an idea as it is on its own merits.

This change is designed to minimize the number of lines changed so that
it's easy to roll back if need be.
@lberki lberki requested review from meisterT, Wyverald and comius March 25, 2024 12:48
@lberki lberki requested review from a team and oquenchil as code owners March 25, 2024 12:48
@lberki lberki requested review from aranguyen and removed request for a team March 25, 2024 12:48
@github-actions github-actions bot added team-Configurability platforms, toolchains, cquery, select(), config transitions team-Rules-CPP Issues for C++ rules team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Mar 25, 2024
@lberki lberki removed request for a team, oquenchil and aranguyen March 25, 2024 13:09
@lberki
Copy link
Contributor Author

lberki commented Mar 27, 2024

In a last, desperate attempt, I ran the HEAD + downstream projects pipeline and it's surprisingly green: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3753#018e7f5a-9256-492e-8dcf-ccbe3e587943

What fails is the following:

  • Genuine domain-specific issues:
    • Bazel (in gathering BUILD.bazel file for @bazel_tools, I believe, seems fixable)
    • IntelliJ plugin (seems to assume the external/ path segment)
    • rules_dotnet (hard-wires assumptions about paths that wouldn't hold anymore)
    • rules_haskell (HaskellBuildLibrary is broken)
    • rules_jvm_external (due to the change to the layout of the runfiles tree)
    • rules_perl: actually crashes Bazel
    • rules_python: genuine breakage, but seems to be fixable
    • rules_foreign_cc (a test that checks an exact path that changes + "file not found issues")
    • rules_testing: an issue in Skydoc
    • rules_webtesting: an issue in Skydoc
  • C++ issues:
    • Bazel examples
    • Cloud Robotics (it says "missing input file @llvm_toolchain//:bin/llvm-dwp, so it looks fixable in O(1) time)
    • rules_go (zlib didn't compile -- this is alarming, if zlib didn't compile what else is broken although it might just be .d file parsing issue that could be fixed in Bazel)
    • Protobuf ("this rule is missing dependency declarations", so may be an issue with .d file parsing)
  • Probably false positives:
    • rules_kotlin (I fail to see how "No such package @rules_kotlin//kotlin might be caused by this, but maybe my imagination is not rich enough)
    • Skylib (looked like an artifact of how I ran the experiment)
    • Kythe (an unrelated Java failure, may be red at HEAD, too)
    • rules_graalvm (seems very unrelated, I didn't check if it's green at HEAD but I'd no be surprised if it was red)

@comius
Copy link
Contributor

comius commented May 15, 2024

@lberki Is this ready for a review? Does it need 3 reviewers? Can you import it? I find internal reviews more pleasant than GitHub.

@comius comius added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 15, 2024
@lberki
Copy link
Contributor Author

lberki commented May 16, 2024

If all we do is internal reviews, we are not really an open source project, are we?

My understanding is that we decided to flip the flag instead and start a Gardening project to fix the downstream projects so I'll close this pull request without merging it.

@lberki lberki closed this May 16, 2024
@iancha1992 iancha1992 deleted the lberki-disable-sibling-repository-layout branch September 16, 2024 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-user-response Awaiting a response from the author team-Configurability platforms, toolchains, cquery, select(), config transitions team-Remote-Exec Issues and PRs for the Execution (Remote) team team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants