-
Notifications
You must be signed in to change notification settings - Fork 639
Description
Summary
We're trying to reduce git clean
time for our developers. We have a C# repo used by hundreds of developers that invokes rush build to build 80+ rush projects. Long story short, git clean is part of their workflow due to a full build with caching being faster than incremental rebuilds when switching branches.
Currently, git clean can take 5 to 10 minutes to run. From what I can tell, git spends most of the time enumerating files in common/temp.
if (EnvironmentConfiguration.rushTempFolderOverride !== undefined) { |
Repro steps
The symlinks created point to the virtual store in common/temp, which is indistinguishable from regular files to git. Just enumerating the files takes a while.
$ time find . | wc -l
263979
real 0m19.698s
user 0m1.531s
sys 0m11.717s
Expected result:
I'd like to specify an environment variable when running rush commands or installing rush that basically has the effect of RUSH_TEMP_FOLDER. Then I would set this environment variable to a location outside of the git repository.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status
Activity
iclanton commentedon Oct 21, 2024
Do you want to just move the virtual store folder? We could probably include something similar to
RUSH_PNPM_STORE_PATH
for the virtual store. @D4N14L had some thoughts about this.It'd require a fair bit of testing, though.
timmydo commentedon Oct 21, 2024
I'm trying to get all written files by the build out of the git source directory. I'm using RUSH_PNPM_STORE_PATH successfully, so something similar to that would work.
SquGus commentedon Oct 28, 2024
@D4N14L would you be able to share your thoughts here?
D4N14L commentedon Oct 31, 2024
@SquGus I mean I think this is a quick fix from my perspective. Should just be allowing us to modify the default virtual store directory that we use. We don't manually touch anything in the virtual store I don't believe, so it should just be specifying the override via env var and passing it through. We could possibly do a pre-release of the change to see if it works for what you need? @iclanton thoughts?
SquGus commentedon Nov 6, 2024
@D4N14L why is it that
RUSH_TEMP_FOLDER
is not compatible with workspace installs (as mentioned here)?What @timmydo and I were hoping to do is to set an alternative location for the files placed in
common/temp/
, while still using pnpm workspaces.While I wait for your response, I will experiment by removing the error thrown at
WorkspaceInstallManager.prepareCommonTempAsync()
to figure out if we can actually do this.D4N14L commentedon Nov 6, 2024
@SquGus The reason for this limitation is because the
pnpm-lock.yaml
file uses relative paths from its location on-disk in order to perform the install. Changing the location of thecommon/temp/
folder modifies the path it exists in during the install, and therefore would make the produced install fail. We can move the other paths (the store and virtual store directories) because these are just paths that get linked where they need to be linked by PNPM. But thepnpm-lock.yaml
file would need to remain in-place.