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: make oci_tarball work with disjoint output root #614

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Conversation

thesayyn
Copy link
Collaborator

@thesayyn thesayyn commented Jun 6, 2024

A while back we changed oci_tarball for performance reasons, and we introduced two different ways to load tarball into the daemon, one where one run bazel run :tarball_target or bazel build :tarball_target --output_groups=tarball && docker load -i tarball.path or directly get the output in DAG.

So in order to support all these cases we introduced some hacks like using --cd option to cd back into the execroot and strip the paths using <ctx.bin_dir.path>.

Unfortunately this did not work for cases where bindir of oci_tarball and oci_image is disjoint, eg oci_image is transitioned but oci_tarball isn't simply because the assumption that everything is in the same bazel-bin/cfg/bin did not hold true in cases above.

This PR fixes it by making eveything relative to execroot, which the case for action version by default, by introducing some execroot detection code copied from rules_js.

@thesayyn thesayyn requested a review from alexeagle June 6, 2024 18:58
@thesayyn thesayyn changed the base branch from main to 2.x June 6, 2024 18:58
@thesayyn
Copy link
Collaborator Author

thesayyn commented Jun 6, 2024

fixes #607

@Strum355
Copy link

Strum355 commented Jun 7, 2024

Confirmed this fixes #607 for us. We also transition oci_image

@alexeagle alexeagle merged commit a36ebe6 into 2.x Jun 7, 2024
18 checks passed
@alexeagle alexeagle deleted the fix_tarball branch June 7, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants