| Message ID | 3d92df7a0cf9655dd34895f106cfac26ea44ad94.1614111270.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Sparse Index: Design, Format, Tests | expand |
On Tue, Feb 23, 2021 at 12:14 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote: > > From: Derrick Stolee <dstolee@microsoft.com> > > This table is helpful for discovering data in the index to ensure it is > being written correctly, especially as we build and test the > sparse-index. This table includes an output format similar to 'git > ls-tree', but should not be compared to that directly. The biggest > reasons are that 'git ls-tree' includes a tree entry for every > subdirectory, even those that would not appear as a sparse directory in > a sparse-index. Further, 'git ls-tree' does not use a trailing directory > separator for its tree rows. > > This does not print the stat() information for the blobs. That could be > added in a future change with another option. The tests that are added > in the next few changes care only about the object types and IDs. > > To make the option parsing slightly more robust, wrap the string > comparisons in a loop adapted from test-dir-iterator.c. > > Care must be taken with the final check for the 'cnt' variable. We > continue the expectation that the numerical value is the final argument. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/helper/test-read-cache.c | 50 ++++++++++++++++++++++++++++++-------- > 1 file changed, 40 insertions(+), 10 deletions(-) > > diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c > index 244977a29bdf..e4c3492f7d3e 100644 > --- a/t/helper/test-read-cache.c > +++ b/t/helper/test-read-cache.c > @@ -2,35 +2,65 @@ > #include "cache.h" > #include "config.h" > > +static void print_cache_entry(struct cache_entry *ce) > +{ > + printf("%06o ", ce->ce_mode & 0777777); This constant is curious. I think it's because you want to strip off the special in-memory bits of the ce_mode where git stores extra data, which would be everything beyond the first 16 bits (as noted in a comment near the beginning of cache.h). But here you keep the first 18 bits. Is CE_UPDATE and CE_REMOVE just 0 in the cases you've viewed so this works (but you really should use 0177777 or 0xFFFF), or am I off in my guess of what you're trying to do and you do want to see those two flags? It also seems surprising to me that this constant isn't already defined somewhere in cache.h or as some variant of S_IFMT, though I'm not finding it. > + > + if (S_ISSPARSEDIR(ce->ce_mode)) > + printf("tree "); > + else if (S_ISGITLINK(ce->ce_mode)) > + printf("commit "); > + else > + printf("blob "); Perhaps make use of the tree_type, commit_type, and blob_type global constants? > + > + printf("%s\t%s\n", > + oid_to_hex(&ce->oid), > + ce->name); > +} > + > +static void print_cache(struct index_state *cache) > +{ > + int i; > + for (i = 0; i < the_index.cache_nr; i++) > + print_cache_entry(the_index.cache[i]); Why are you passing cache as a parameter, then ignoring it and using the_index? > +} > + > int cmd__read_cache(int argc, const char **argv) > { > + struct repository *r = the_repository; > int i, cnt = 1; > const char *name = NULL; > + int table = 0; > > - if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) { > - argc--; > - argv++; > + for (++argv, --argc; *argv && starts_with(*argv, "--"); ++argv, --argc) { > + if (skip_prefix(*argv, "--print-and-refresh=", &name)) > + continue; > + if (!strcmp(*argv, "--table")) > + table = 1; > } > > - if (argc == 2) > - cnt = strtol(argv[1], NULL, 0); > + if (argc == 1) > + cnt = strtol(argv[0], NULL, 0); > setup_git_directory(); > git_config(git_default_config, NULL); > + > for (i = 0; i < cnt; i++) { > - read_cache(); > + repo_read_index(r); > if (name) { > int pos; > > - refresh_index(&the_index, REFRESH_QUIET, > + refresh_index(r->index, REFRESH_QUIET, > NULL, NULL, NULL); > - pos = index_name_pos(&the_index, name, strlen(name)); > + pos = index_name_pos(r->index, name, strlen(name)); > if (pos < 0) > die("%s not in index", name); > printf("%s is%s up to date\n", name, > - ce_uptodate(the_index.cache[pos]) ? "" : " not"); > + ce_uptodate(r->index->cache[pos]) ? "" : " not"); > write_file(name, "%d\n", i); > } > - discard_cache(); > + if (table) > + print_cache(r->index); > + discard_index(r->index); > } > return 0; > } > -- > gitgitgadget
On 2/25/2021 2:02 AM, Elijah Newren wrote: > On Tue, Feb 23, 2021 at 12:14 PM Derrick Stolee via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> >> From: Derrick Stolee <dstolee@microsoft.com> >> >> This table is helpful for discovering data in the index to ensure it is >> being written correctly, especially as we build and test the >> sparse-index. This table includes an output format similar to 'git >> ls-tree', but should not be compared to that directly. The biggest >> reasons are that 'git ls-tree' includes a tree entry for every >> subdirectory, even those that would not appear as a sparse directory in >> a sparse-index. Further, 'git ls-tree' does not use a trailing directory >> separator for its tree rows. >> >> This does not print the stat() information for the blobs. That could be >> added in a future change with another option. The tests that are added >> in the next few changes care only about the object types and IDs. >> >> To make the option parsing slightly more robust, wrap the string >> comparisons in a loop adapted from test-dir-iterator.c. >> >> Care must be taken with the final check for the 'cnt' variable. We >> continue the expectation that the numerical value is the final argument. >> >> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> >> --- >> t/helper/test-read-cache.c | 50 ++++++++++++++++++++++++++++++-------- >> 1 file changed, 40 insertions(+), 10 deletions(-) >> >> diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c >> index 244977a29bdf..e4c3492f7d3e 100644 >> --- a/t/helper/test-read-cache.c >> +++ b/t/helper/test-read-cache.c >> @@ -2,35 +2,65 @@ >> #include "cache.h" >> #include "config.h" >> >> +static void print_cache_entry(struct cache_entry *ce) >> +{ >> + printf("%06o ", ce->ce_mode & 0777777); > > This constant is curious. I think it's because you want to strip off > the special in-memory bits of the ce_mode where git stores extra data, > which would be everything beyond the first 16 bits (as noted in a > comment near the beginning of cache.h). But here you keep the first > 18 bits. Is CE_UPDATE and CE_REMOVE just 0 in the cases you've viewed > so this works (but you really should use 0177777 or 0xFFFF), or am I > off in my guess of what you're trying to do and you do want to see > those two flags? You're right, 0177777 is what I want. I'm focusing only on the file permissions bits that are reported by ls-tree. > It also seems surprising to me that this constant isn't already > defined somewhere in cache.h or as some variant of S_IFMT, though I'm > not finding it. I'm not, either. >> + >> + if (S_ISSPARSEDIR(ce->ce_mode)) >> + printf("tree "); >> + else if (S_ISGITLINK(ce->ce_mode)) >> + printf("commit "); >> + else >> + printf("blob "); > > Perhaps make use of the tree_type, commit_type, and blob_type global constants? Today I Learned... >> + >> + printf("%s\t%s\n", >> + oid_to_hex(&ce->oid), >> + ce->name); >> +} >> + >> +static void print_cache(struct index_state *cache) >> +{ >> + int i; >> + for (i = 0; i < the_index.cache_nr; i++) >> + print_cache_entry(the_index.cache[i]); > > Why are you passing cache as a parameter, then ignoring it and using the_index? Good catch! Thanks, -Stolee
diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c index 244977a29bdf..e4c3492f7d3e 100644 --- a/t/helper/test-read-cache.c +++ b/t/helper/test-read-cache.c @@ -2,35 +2,65 @@ #include "cache.h" #include "config.h" +static void print_cache_entry(struct cache_entry *ce) +{ + printf("%06o ", ce->ce_mode & 0777777); + + if (S_ISSPARSEDIR(ce->ce_mode)) + printf("tree "); + else if (S_ISGITLINK(ce->ce_mode)) + printf("commit "); + else + printf("blob "); + + printf("%s\t%s\n", + oid_to_hex(&ce->oid), + ce->name); +} + +static void print_cache(struct index_state *cache) +{ + int i; + for (i = 0; i < the_index.cache_nr; i++) + print_cache_entry(the_index.cache[i]); +} + int cmd__read_cache(int argc, const char **argv) { + struct repository *r = the_repository; int i, cnt = 1; const char *name = NULL; + int table = 0; - if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) { - argc--; - argv++; + for (++argv, --argc; *argv && starts_with(*argv, "--"); ++argv, --argc) { + if (skip_prefix(*argv, "--print-and-refresh=", &name)) + continue; + if (!strcmp(*argv, "--table")) + table = 1; } - if (argc == 2) - cnt = strtol(argv[1], NULL, 0); + if (argc == 1) + cnt = strtol(argv[0], NULL, 0); setup_git_directory(); git_config(git_default_config, NULL); + for (i = 0; i < cnt; i++) { - read_cache(); + repo_read_index(r); if (name) { int pos; - refresh_index(&the_index, REFRESH_QUIET, + refresh_index(r->index, REFRESH_QUIET, NULL, NULL, NULL); - pos = index_name_pos(&the_index, name, strlen(name)); + pos = index_name_pos(r->index, name, strlen(name)); if (pos < 0) die("%s not in index", name); printf("%s is%s up to date\n", name, - ce_uptodate(the_index.cache[pos]) ? "" : " not"); + ce_uptodate(r->index->cache[pos]) ? "" : " not"); write_file(name, "%d\n", i); } - discard_cache(); + if (table) + print_cache(r->index); + discard_index(r->index); } return 0; }