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

Breakage with Bazel HEAD due to 00805727b867d33fd922e63ca82b0d9825ad79fe #2464

Closed
lberki opened this issue Feb 16, 2021 · 8 comments
Closed
Labels
cleanup Tech debt, resolving it improves our own velocity

Comments

@lberki
Copy link
Contributor

lberki commented Feb 16, 2021

This breakage was reported by our continuous integration.

https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/1906#44b4eb78-3a05-4ecf-abdb-dfa16b20ff86

It is probably caused by bazelbuild/bazel@0080572 that calls test binaries in external repositories not using their legacy external/$REPO/$REPO_RELATIVE_PATH path but the canonical ../$REPO/$REPO_RELATIVE_PATH one.

I haven't looked into what the mechanism of the breakage is; my guess is that test binaries try to find data files relative to $0 and that logic needs to be updated.

This bug is also available in our continuous integration repository as bazelbuild/continuous-integration#1093 .

@lberki
Copy link
Contributor Author

lberki commented Feb 16, 2021

/cc @philwo @gyias @c-parsons

@mattem mattem added the cleanup Tech debt, resolving it improves our own velocity label Feb 16, 2021
@alexeagle
Copy link
Collaborator

Is it possibly related to our use of --nolegacy_external_runfiles? https://github.com/bazelbuild/rules_nodejs/blob/stable/common.bazelrc#L60-L62

@alexeagle alexeagle reopened this Feb 17, 2021
@gyias
Copy link

gyias commented Feb 17, 2021

Probably not. The triggering change is part of a project to deprecate the legacy external runfiles path.

@gyias
Copy link

gyias commented Feb 17, 2021

Hmm, I now think the commit isn't actually the culprit. I ran a CI with a revert, and it's still broken.

@alexeagle
Copy link
Collaborator

alexeagle commented Feb 18, 2021 via email

@meteorcloudy
Copy link
Collaborator

@meteorcloudy
Copy link
Collaborator

And this can only be reproduced with remote caching (or just --disk_cache=/tmp/bazel_disk_cache). I'm suspecting this is a bug in Bazel.

bazel-io pushed a commit to bazelbuild/bazel that referenced this issue Feb 19, 2021
Fixes a bunch of downstream breakages:

bazelbuild/continuous-integration#1093
bazelbuild/continuous-integration#1094
bazel-contrib/rules_nodejs#2464
bazelbuild/rules_python#419

Turns out, the assertion that "Merkle tree computation uses `ActionInput.getExecPath()`" was only mostly correct: there was a place where the key of the input map was used instead.

I'm somewhat surprised that this did not show up in our test battery, although, admittedly, "unsound directory as an input file in an external repository" doesn't sound like the most common use case.

RELNOTES: None.
PiperOrigin-RevId: 358366246
@comius
Copy link
Contributor

comius commented Feb 19, 2021

@mattem mattem closed this as completed Feb 23, 2021
coeuvre pushed a commit to coeuvre/bazel that referenced this issue Jul 15, 2021
… .

Fixes a bunch of downstream breakages:

bazelbuild/continuous-integration#1093
bazelbuild/continuous-integration#1094
bazel-contrib/rules_nodejs#2464
bazelbuild/rules_python#419

Turns out, the assertion that "Merkle tree computation uses `ActionInput.getExecPath()`" was only mostly correct: there was a place where the key of the input map was used instead.

I'm somewhat surprised that this did not show up in our test battery, although, admittedly, "unsound directory as an input file in an external repository" doesn't sound like the most common use case.

RELNOTES: None.
PiperOrigin-RevId: 358366246
coeuvre pushed a commit to coeuvre/bazel that referenced this issue Jul 15, 2021
… .

Fixes a bunch of downstream breakages:

bazelbuild/continuous-integration#1093
bazelbuild/continuous-integration#1094
bazel-contrib/rules_nodejs#2464
bazelbuild/rules_python#419

Turns out, the assertion that "Merkle tree computation uses `ActionInput.getExecPath()`" was only mostly correct: there was a place where the key of the input map was used instead.

I'm somewhat surprised that this did not show up in our test battery, although, admittedly, "unsound directory as an input file in an external repository" doesn't sound like the most common use case.

RELNOTES: None.
PiperOrigin-RevId: 358366246
coeuvre pushed a commit to coeuvre/bazel that referenced this issue Jul 16, 2021
… .

Fixes a bunch of downstream breakages:

bazelbuild/continuous-integration#1093
bazelbuild/continuous-integration#1094
bazel-contrib/rules_nodejs#2464
bazelbuild/rules_python#419

Turns out, the assertion that "Merkle tree computation uses `ActionInput.getExecPath()`" was only mostly correct: there was a place where the key of the input map was used instead.

I'm somewhat surprised that this did not show up in our test battery, although, admittedly, "unsound directory as an input file in an external repository" doesn't sound like the most common use case.

RELNOTES: None.
PiperOrigin-RevId: 358366246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Tech debt, resolving it improves our own velocity
Projects
None yet
Development

No branches or pull requests

6 participants