From: Taylor Blau <me@ttaylorr.com>
To: Jeff King <peff@peff.net>
Cc: Son Luong Ngoc via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Son Luong Ngoc <sluongng@gmail.com>
Subject: Re: [PATCH] commit-graph: add verify changed paths option
Date: Fri, 31 Jul 2020 14:09:56 -0400 [thread overview]
Message-ID: <20200731180956.GA60133@syl.lan> (raw)
In-Reply-To: <20200731180235.GA846620@coredump.intra.peff.net>
On Fri, Jul 31, 2020 at 02:02:35PM -0400, Jeff King wrote:
> On Fri, Jul 31, 2020 at 07:49:25AM +0000, Son Luong Ngoc via GitGitGadget wrote:
>
> > From: Son Luong Ngoc <sluongng@gmail.com>
> >
> > Add '--has-changed-paths' option to 'git commit-graph verify' subcommand
> > to validate whether the commit-graph was written with '--changed-paths'
> > option.
>
> Is a single boolean flag sufficient? If you have incrementals, you might
> have some slices with this chunk and some without. What should the
> boolean be in that case?
I think you'd really want to know which layers do and don't have
filters. It might be even more interesting to have a tool like what 'git
show-index' is to '*.idx' files, maybe something like 'git show-graph'
or 'git show-commit-graph'. Its output would be one line per commit that
shows:
- what layer in the chain it's located at
- its graph_pos
- its generation number
- whether or not it has a Bloom filter
- ???
That would be a useful tool for debugging anyway, even outside of the
test suite. It would be even better if we could replace the test-tool
with it.
On an unrelated note; this patch is broken as-is, since it will only
report that Bloom filters exist if the top-most graph has them. I have a
patch to fix this that I have been meaning to send out for most of this
week. I'll try to get to it shortly.
> I thought we had some way of reporting the number of commits covered by
> filters, but I can't seem to find it.
I don't recall having anything like that.
> Our "test-tool read-graph" can report on whether there's a bloom filter
> chunk, but I think it also doesn't distinguish between different slices
> (and anyway, it wouldn't be suitable for tools that don't rely on an
> actual built git.git directory).
>
> -Peff
Thanks,
Taylor
next prev parent reply other threads:[~2020-07-31 18:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-31 7:49 [PATCH] commit-graph: add verify changed paths option Son Luong Ngoc via GitGitGadget
2020-07-31 16:21 ` Christian Couder
2020-07-31 17:14 ` Junio C Hamano
2020-07-31 18:06 ` Taylor Blau
2020-07-31 18:02 ` Jeff King
2020-07-31 18:09 ` Taylor Blau [this message]
2020-07-31 19:14 ` Jeff King
2020-07-31 19:31 ` Son Luong Ngoc
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200731180956.GA60133@syl.lan \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=peff@peff.net \
--cc=sluongng@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.