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

runfiles_manifest cached incorrectly with output_user_root and output_base flags set #17267

Closed
JesseTatasciore opened this issue Jan 19, 2023 · 7 comments
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Local-Exec Issues and PRs for the Execution (Local) team type: bug

Comments

@JesseTatasciore
Copy link

Description of the bug:

This bug can be encountered using a target that outputs a runfiles_manifest and contains a source file as an input. In this case, an absolute path to the source file will be stamped into the runfiles_manifest. However, if we run 2 builds from 2 different clones of the same repo, on the same commit, with output_user_root and output_base set to the same paths on both builds, then the runfiles_manifest will be incorrectly cached for the second clone / build. The runfiles_manifest on the second clone / build will contain a reference to the absolute path of the inputted source file located in the first clone.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Create 2 identical bazel workspaces containing a simple sh_binary that references a source file in 2 different folders. Build both targets with --output_user_root and --output_base set to the same folders on both. Check the 2 different runfiles_manifest files and they will contain the same absolute path. That of the source file from the first one built.

A minimal example can be found here: https://github.com/JesseTatasciore/bazel-manifest-caching-repro

I added a simple script named bazel_repro.sh which will perform the instructions I mentioned above (create the clones, run the builds and cat the different manifest_runfiles files)

Which operating system are you running Bazel on?

Confirmed on macOS Monterey (Apple M1) and Amazon Linux 2 (centos). Please note that I have not tried on any other OS's

What is the output of bazel info release?

release 6.0.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No

Any other information, logs, or outputs that you want to share?

This is some output of the script I mentioned in the minimal repro I provided:

-------------------------------------
First Build PWD
-------------------------------------
/Users/jesse/Development/bazel-manifest-caching-repro/test/test1/bazel-manifest-caching-repro
-------------------------------------
First Build runfiles_manifest
-------------------------------------
repro/hello_world /Users/jesse/Development/bazel-manifest-caching-repro/test/outputs/__main__/execroot/repro/bazel-out/darwin_arm64-fastbuild/bin/hello_world
repro/hello_world.sh /Users/jesse/Development/bazel-manifest-caching-repro/test/test1/bazel-manifest-caching-repro/hello_world.sh


-------------------------------------
Second Build PWD
-------------------------------------
/Users/jesse/Development/bazel-manifest-caching-repro/test/test2/bazel-manifest-caching-repro
-------------------------------------
Second Build runfiles_manifest
-------------------------------------
repro/hello_world /Users/jesse/Development/bazel-manifest-caching-repro/test/outputs/__main__/execroot/repro/bazel-out/darwin_arm64-fastbuild/bin/hello_world
repro/hello_world.sh /Users/jesse/Development/bazel-manifest-caching-repro/test/test1/bazel-manifest-caching-repro/hello_world.sh

From which, you can see that the absolute path under Second Build runfiles_manifest does not match the output from Second Build PWD as it should. It has test1 from First Build PWD instead of the expected test2

@sgowroji sgowroji added type: bug team-Rules-Server Issues for serverside rules included with Bazel untriaged labels Jan 20, 2023
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 30 days. It will be closed in the next 7 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler".

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 20, 2023
@fmeum
Copy link
Collaborator

fmeum commented Feb 20, 2023

@sgowroji Not stale as it's untriaged.

@sgowroji sgowroji removed the stale Issues or PRs that are stale (no activity for 30 days) label Feb 20, 2023
@comius comius added team-Local-Exec Issues and PRs for the Execution (Local) team and removed team-Rules-Server Issues for serverside rules included with Bazel labels Aug 4, 2023
@comius
Copy link
Contributor

comius commented Aug 4, 2023

Local-exec is probably not the best label, but it's probably more related that team-rules-Server.

@fmeum @Wyverald I believe you last worked on runfiles, could you help investigating/triaging this one?

@fmeum
Copy link
Collaborator

fmeum commented Aug 4, 2023

I can't really say which team this belongs to, but I think it's pretty clear what the root cause is: SourceManifestAction emits absolute paths for artifacts, but does not include the paths of the roots in its action key. I will try the "obvious" fix and see whether it breaks anything.

@fmeum
Copy link
Collaborator

fmeum commented Aug 4, 2023

@JesseTatasciore Can you also reproduce this if you change --output_user_root within the same project or is this specific to two different projects sharing an --output_user_root?

@lberki
Copy link
Contributor

lberki commented Aug 7, 2023

Wow, that's a very nice repro! Minimal, well-documented, straightforward. Kudos. (/me now starts grokking @fmeum 's fix)

@tjgq tjgq added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Aug 8, 2023
@JesseTatasciore
Copy link
Author

Can you also reproduce this if you change --output_user_root within the same project or is this specific to two different projects sharing an --output_user_root?

Sorry for the delay. You may have already tried this given there is an open PR. I just tried and I believe it needs to be "two different projects sharing an --output_user_root" in order to reproduce. I added a new script to my repro repo (bazel_repro_same_directory.sh) so you can see what I tried, where I only use a single repo but do 2 different builds using different --output_user_roots. Both times I see the same absolute path to the file in the runfiles manifest. I think this makes sense though? Even if we are reproducing the issue I have outlined here, the absolute path to the file would not have changed, so I would expect the path to be the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Local-Exec Issues and PRs for the Execution (Local) team type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants