Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Enable omnibus build cache #20117
Enable omnibus build cache #20117
Changes from 21 commits
c5d3e9e
dfadb8a
a3d09bd
8653336
ac458d2
37abbc5
bae0667
0e50cb4
45c6717
6caa677
7d35a23
74d010a
134d81d
cea95fa
53185cf
5a1e18a
913de28
5fd51a2
ee0a2e6
604b6f2
1e44837
e8d83e2
16d0588
2c8b4c7
93aab43
b5e6c3e
d90f609
e9b9351
3fe2d17
838fd95
edf92fa
978a516
031ca3e
5a8eef1
32d6afe
d505a22
48da5a2
4cfe191
2245627
b6a0c5c
1313603
424e169
86a84d0
3092fc1
af4473b
bdf7369
e9d2dad
3d78dd7
590bd4c
98b6a70
1e414d2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
IIUC,
OMNIBUS_GIT_CACHE_DIR
is not set on the Windows Agent builds, meaning that the build cache is not enabled for Windows.The PR description doesn't mention that this change is Linux-only, so could you confirm that this is expected? If yes, can this be added somewhere in code and in the PR description?
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.
This seems fragile as a path concatenation operation, should that use os.path.join instead?
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.
💬 suggestion: I'd probably lean towards writing this in python directly, rather than shelling out. But, this isn't a blocking comment.