Re: [PATCH 11/20] sparse-index: convert from full to sparse

On 2/25/2021 2:33 AM, Elijah Newren wrote:

On Tue, Feb 23, 2021 at 12:14 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:

quoted

+                       /*
+                        * allow terminating directory separators for
+                        * sparse directory enries.

enries -> entries

Thanks.

quoted

+                        */
+                       if (c == '\0')
+                               return S_ISDIR(mode);

Yaay, much simpler (than the RFC version).

quoted

+       /*
+        * Is the current path outside of the sparse cone?
+        * Then check if the region can be replaced by a sparse
+        * directory entry (everything is sparse and merged).
+        */
+       match = path_matches_pattern_list(ct_path, ct_pathlen,
+                                         NULL, &dtype, pl, istate);
+       if (match != NOT_MATCHED)
+               can_convert = 0;

Not sure if you saw my comments on the flow control at
https://lore.kernel.org/git/CABPp-BE9wPwmC0=pA4p1_QSRDHrO8RzqfJQdE2NxYZsYL_Rcig@mail.gmail.com/ (local)
(the typos elsewhere seem to still be present).  If you saw it and
decided against it, that's fine, just wanted the idea to at least be
floated.

Sorry for dropping this one. I _did_ decide against it, and
primarily because the "if (can_convert)" condition contains
a return statement. I like to use 'gotos' for blocks that
will eventually be entered by all paths through the code,
such as "goto cleanup;" but here I find the "can_convert"
check to be clearer.

quoted

+               /*
+                * Detect if this is a normal entry oustide of any subtree

s/oustide/outside/

Got it.

quoted

+test_expect_success 'sparse-index contents' '
+       init_repos &&
+
+       test-tool -C sparse-index read-cache --table >cache &&
+       for dir in folder1 folder2 x
+       do
+               TREE=$(git -C sparse-index rev-parse HEAD:$dir) &&
+               grep "040000 tree $TREE $dir/" cache \
+                       || return 1
+       done &&

Thanks for making the output look more like ls-tree output; it's
easier to parse that way, at least for me.

Excellent.
 

I mostly read over the range-diff since it was much shorter.  You've
addressed a number of questions/comments I had on the RFC version, but
there's still some I didn't see a response to so I reposted them here.

 
Thanks for being diligent!
-Stolee