on iko/ghcjs-patch-master help sign in
Fix cleanGit submodule support performance issue (#1072)

Currently in order to support submodules all the directories are included (but not all files) when building the whitelist. This is done because we need the `.git` files (or directories) the contain to be present when we run `git ls-files --recurse-submodules`. Including every directory causes two performance issues: * Creating a directory in your repo (even empty one not added to git at all) will require the whitelist to be recalculated. * Recalculation of the whiltelist may be slowed down if there are a large number of directories. In particular `lstat` is called on every file (probably to get the right `type` to pass to the filter function). The fix is to use `git submodule status --recursive` to get a list of all the submodule directories. Then the `.git` files (or directories) in them can be included. This has to be repeated until all the submodules are known (as `--recursive` will not see the submodules until the parent module is included).

commit9085d9f44e59550f3c8b6a10d69cac1b6cc17599 authorHamish Mackenzie <Hamish.Mackenzie@iohk.io> dateThu 18 Mar 2021, 19:49 (6 years ago) parentdff62ec
1 file changed+57−22
lib/clean-git.nix+57−22
lib/clean-git.nix+57−22
@@ -87,32 +87,37 @@ then
8787 gitModulesStr = origSrcSubDir + "/.gitmodules";
8888 gitModules = builtins.path { name = "gitmodules"; path = gitModulesStr; };
8989
90− gitSubmoduleFiles = cleanSourceWith {
91− caller = "cleanGit";
92− name = (if name == null then "" else name + "-") + "gitSubmoduleFiles";
93− inherit src;
94− filter = path: type:
95− type == "directory" # TODO get sudmodule directories from `.gitmodules`
96− # and use that to filter directory tree here
97− ||
98− lib.any (i: (lib.hasSuffix i path)) [
99− "/.git" ".gitmodules" ".git/config" ".git/index" ".git/HEAD" ".git/objects" ".git/refs" ] ||
100− (lib.strings.hasInfix ".git/modules/" path &&
90+ # Files from the git repository related knownSubmoduleDirs.
91+ gitSubmoduleFiles = knownSubmoduleDirs:
92+ let pathsNeeded = map (p: toString (src.origSrcSubDir or src) + "/${p}") (
93+ lib.concatMap (x: all_paren_dirs (x + "/.git")) knownSubmoduleDirs);
94+ in cleanSourceWith {
95+ caller = "cleanGit";
96+ name = (if name == null then "" else name + "-") + "gitSubmoduleFiles";
97+ inherit src;
98+ filter = path: type:
99+ elem path pathsNeeded
100+ ||
101101 lib.any (i: (lib.hasSuffix i path)) [
102− "config" "index" "HEAD" "objects" "refs" ]);
103− };
102+ ".gitmodules" ".git/config" ".git/index" ".git/HEAD" ".git/objects" ".git/refs" ]
103+ ||
104+ (lib.strings.hasInfix ".git/modules/" path &&
105+ lib.any (i: (lib.hasSuffix i path)) [
106+ "config" "index" "HEAD" "objects" "refs" ]);
107+ };
108+
109+ hasSubmodules = !isWorktree && builtins.pathExists gitModulesStr;
104110
105111 # Make a temporary dir that looks enough like the real thing for
106112 # `git ls-files --recurse-submodules` to give us an accurate list
107113 # of all the files in the index.
108− whitelist_file =
109− runCommand "git-ls-files" {envVariable = true;} ''
114+ whitelist_files = knownSubmoduleDirs:
115+ runCommand "git-ls-files" {} ''
110116 tmp=$(mktemp -d)
111117 cd $tmp
112− ${ lib.optionalString (!isWorktree && builtins.pathExists gitModulesStr) ''
113− cp -ra ${gitSubmoduleFiles}/. $tmp
118+ ${ lib.optionalString hasSubmodules ''
119+ cp -ra ${gitSubmoduleFiles knownSubmoduleDirs}/. $tmp
114120 chmod +w -R $tmp
115− rm -rf .git
116121 ''}
117122 cp -r ${gitDir} .git
118123 chmod +w -R .git
@@ -123,10 +128,37 @@ then
123128 ${ lib.optionalString (isWorktree && builtins.pathExists gitModulesStr) ''
124129 cp ${gitModules} ./.gitmodules
125130 ''}
126− ${git}/bin/git ls-files --recurse-submodules > $out
131+ mkdir $out
132+ ${ lib.optionalString hasSubmodules ''
133+ ${git}/bin/git submodule status --recursive | awk '{ print $2 }' > $out/submoduleDirs
134+ ''}
135+ ${git}/bin/git ls-files --recurse-submodules > $out/files
127136 '';
128137
129− whitelist = lines (readFile (whitelist_file.out));
138+ # Find the submodules
139+ whitelist_recursive = knownSubmoduleDirs:
140+ let
141+ # Get the new list of submoduleDirs and files
142+ # (useing the submoduleDirs we already know about)
143+ files = whitelist_files knownSubmoduleDirs;
144+ new = {
145+ submoduleDirs = lines (readFile (files + "/submoduleDirs"));
146+ files = lines (readFile (files + "/files"));
147+ };
148+ in
149+ # If we are not expecting submodules or if the new list does not
150+ # contain any additional submodules we are done.
151+ if !hasSubmodules || lib.all (x: elem x knownSubmoduleDirs) new.submoduleDirs
152+ then new
153+ else
154+ # Look again using what we know now about the submodules
155+ whitelist_recursive new.submoduleDirs;
156+
157+ # Use empty as a starting point for looking for submodules.
158+ # We could allow a list to be passed into cleanGit, but when testing
159+ # on the `ghcjs` repo (one with a lot of submodules) it did not
160+ # make much of a difference to the speed of `cleanGit`.
161+ whitelist = whitelist_recursive [];
130162
131163 all_paren_dirs = p:
132164 if p == "." || p == "/"
@@ -138,9 +170,9 @@ then
138170 concatMap (p:
139171 # Using `origSrcSubDir` (if present) makes it possible to cleanGit src that
140172 # has already been cleaned with cleanSrcWith.
141− let full_path = src.origSrcSubDir or (toString src) + "/${p}"; in
173+ let full_path = toString (src.origSrcSubDir or src) + "/${p}"; in
142174 map (p': { name = p'; value = true; }) (all_paren_dirs full_path)
143− ) whitelist
175+ ) whitelist.files
144176 );
145177
146178 # Identify files in the `.git` dir
@@ -155,6 +187,9 @@ then
155187 cleanSourceWith {
156188 caller = "cleanGit";
157189 inherit name src subDir includeSiblings filter;
190+ } // {
191+ # For testing
192+ # inherit whitelist whitelist_set;
158193 }
159194
160195else