-
Notifications
You must be signed in to change notification settings - Fork 49
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
add cwd to cache prefix #200
base: main
Are you sure you want to change the base?
Conversation
c7aae58
to
a0f6a9b
Compare
2af9bf4
to
ec7e654
Compare
Ready for review |
Not sure why https://github.com/uber/hermetic_cc_toolchain/actions/runs/11867448986/job/33075522540 passed yet https://github.com/uber/hermetic_cc_toolchain/actions/runs/11914055410/job/33201139834?pr=200 fails. The error is: Maybe related to: ziglang/zig#19973 , but I don't have a windows machine to test on. Next things to try:
|
59f75ed
to
288af3c
Compare
19dae46
to
e10f61f
Compare
Rerunning the CI seems to have passed (after I rebased back to a non-upgraded zig 0.13/14 attempt). Moving this back to "Ready for review" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sluongng @laurentlb can you verify if this works for you?
toolchain/defs.bzl
Outdated
elif os == "linux": | ||
cache_prefix = "/tmp/zig-cache" | ||
if os == "windows" or os == "macos" or os == "linux": | ||
cache_prefix = "zig-cache" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put it in either bazel-zig-cache
or bazel-out/zig-cache
? Most bazel project have either /bazel-*
or /bazel-out
in their .gitignore
. They don't need to ignore additional directory if we reuse the existing directories.
Unless mistaken, wouldn't this store the cache inside the action sandbox root, unique to each cc target? |
This is probably right. I think I misinterpreted/understood the prior requirement mistaking the "workspaceRoot" for "action sandbox root". After re-reading https://bazel.build/remote/output-directories , perhaps it should store this cache directory in either the outputRoot (equivalent to bazel-out) or outputBaseRoot, or in the workspaceRoot/bazel-zig-cache as mentioned above . To get the outputRoot, I'm guessing I'd need to get the parent directory of the execRoot or use execRoot/_main/_bin/bazel-out ? |
e10f61f
to
edf5a31
Compare
After doing some more exploration, some findings:
|
Caveat: I don't know what I'm doing with zig. The zig code was mostly code assistant-generated.
I don't think this is a breaking change, but if somebody does rely on the cache location, they can set the
HERMETIC_CC_TOOLCHAIN_CACHE_PREFIX
to one of the prior values (/tmp
,/var/tmp
,C:\Temp
) to maintain same behavior.Test Plan
I added a debug print statement for the
zig_cache_dir
variable to see its value.I tried the following test cases:
HERMETIC_CC_TOOLCHAIN_CACHE_PREFIX
zig_cache_dir
(Printed value)<workspace root>/zig-cache
/tmp/my-test-cache
/tmp/my-test-cache
my-test-cache
<workspace root>/my-test-cache
Where
<workspace root>
for me looked like/private/var/tmp/_bazel_…/…/sandbox/darwin-sandbox/…/execroot/_main
.Closes #83
Closes #183
Closes #187