Re: [RFC/PATCH 0/5] Re: [PATCH v3 07/20] test-read-cache: print cache entries with --table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 17, 2021 at 6:28 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
> > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxx>
> > ---
> >  t/helper/test-read-cache.c | 55 +++++++++++++++++++++++++++++++-------
> >  1 file changed, 45 insertions(+), 10 deletions(-)
> >
> > diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
> > index 244977a29bdf..6cfd8f2de71c 100644
> > --- a/t/helper/test-read-cache.c
> > +++ b/t/helper/test-read-cache.c
> > @@ -1,36 +1,71 @@
> >  #include "test-tool.h"
> >  #include "cache.h"
> >  #include "config.h"
> > +#include "blob.h"
> > +#include "commit.h"
> > +#include "tree.h"
> > +
> > +static void print_cache_entry(struct cache_entry *ce)
> > +{
> > +     const char *type;
> > +     printf("%06o ", ce->ce_mode & 0177777);
> > +
> > +     if (S_ISSPARSEDIR(ce->ce_mode))
> > +             type = tree_type;
> > +     else if (S_ISGITLINK(ce->ce_mode))
> > +             type = commit_type;
> > +     else
> > +             type = blob_type;
> > +
> > +     printf("%s %s\t%s\n",
> > +            type,
> > +            oid_to_hex(&ce->oid),
> > +            ce->name);
> > +}
> > +
>
> So we have a test tool that's mostly ls-files but mocks the output
> ls-tree would emit, won't these tests eventually care about what stage
> things are in?
>
> What follows is an RFC series on top that's the result of me wondering
> why if we're adding new index constructs we aren't updating our
> plumbing to emit that data, can we just add this to ls-files and drop
> this test helper?
>
> Turns out: Yes we can.

I like the idea of having ls-files be usable to show the entries that
are in the index; that seems great to me.  I very much dislike the
--sparse flag to ls-files, as noted on that commit.

Also, as a minor point, the first two patches seemed a bit confusing
to me.  The first commit said that it was there solely to make "the
next commit" easier, and the second was worded as just making the next
patch easier, which made me wonder if the wording in the first commit
message was referring to 3/5 when it said "the next commit".  Both of
the first two commits were so tiny that if they are both prep for 3/5,
maybe it makes sense to combine them (together or both to 3/5)?  If
not, maybe the commit messages could be cleaned up or clarified a bit?

> Ævar Arnfjörð Bjarmason (5):
>   ls-files: defer read_index() after parse_options() etc.
>   ls-files: make "mode" in show_ce() loop a variable
>   ls-files: add and use a new --sparse option
>   test-tool read-cache: --table is redundant to ls-files
>   test-tool: split up test-tool read-cache
>
>  Documentation/git-ls-files.txt           |  4 ++
>  Makefile                                 |  3 +-
>  builtin/ls-files.c                       | 29 +++++++--
>  t/helper/test-read-cache-again.c         | 31 +++++++++
>  t/helper/test-read-cache-perf.c          | 21 ++++++
>  t/helper/test-read-cache.c               | 82 ------------------------
>  t/helper/test-tool.c                     |  3 +-
>  t/helper/test-tool.h                     |  3 +-
>  t/perf/p0002-read-cache.sh               |  2 +-
>  t/t1091-sparse-checkout-builtin.sh       |  9 +--
>  t/t1092-sparse-checkout-compatibility.sh | 57 ++++++++++------
>  t/t7519-status-fsmonitor.sh              |  2 +-
>  12 files changed, 131 insertions(+), 115 deletions(-)
>  create mode 100644 t/helper/test-read-cache-again.c
>  create mode 100644 t/helper/test-read-cache-perf.c
>  delete mode 100644 t/helper/test-read-cache.c
>
> --
> 2.31.0.260.g719c683c1d




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux