All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] t1500: ensure current --since= behavior remains
Date: Thu, 11 Feb 2021 17:06:25 +0000	[thread overview]
Message-ID: <20210211170625.GA8280@dcvr> (raw)
In-Reply-To: <YCUSaXCg8Abg+vGs@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> On Wed, Feb 10, 2021 at 09:55:43PM +0000, Eric Wong wrote:
> 
> > This behavior of git-rev-parse is observed since git 1.8.3.1
> > at least(*), and likely earlier versions.
> > 
> > At least one git-reliant project in-the-wild relies on this
> > current behavior of git-rev-parse being able to handle multiple
> > --since= arguments without squeezing identical results together.
> > So add a test to prevent the potential for regression in
> > downstream projects.
> 
> I had to read this a few times to understand what "this behavior" meant.
> It is just: when given multiple --since options, output a --max-age for
> each of them, even though internally, Git's revision traversal will only
> use one (in the usual last-one-wins fashion).
> 
> I'm not sure if I was just being dense, or if this could be spelled out
> more clearly. :)

*shrug* :>  My brain struggles with coherent thought so I'm
surprised anybody is able to understand me at all :x

> Out of curiosity, why does the other project want that? From your
> mention of libgit2's git__date_parse(), I assume it's something that
> wants to parse approxidates into timestamps in a script. Maybe we ought
> to provide a more direct and robust way of doing that. We have a similar
> need in t0006, but we use a test-helper program for it.

It takes about 5ms for my system to run git-rev-parse once.  I
may be getting multiple approxidates at once, 1-2 in a typical
input (start..end); but a strange or malicious input could have
hundreds/thousands of approxidates to parse.

Thus, I'm batching up all the approxidates into one rev-parse
invocation (up to system argv limits right now).  With the
output lines split into an array, walking the output/input
arrays in parallel will match them up.  Hypothetically, if
rev-parse were to get clever and deduplicate or reject identical
inputs; then the parallel walk would be broken.

> (I have no problem in the meantime with this patch, though; any new
> method for accomplishing this would want to give other projects time to
> adapt to its use).

Yes.  I think I've mentioned some years/decade ago having
general functionality along the lines of "git cat-file --batch"
or fast-import would be nice (even for some existing scripts and
tests shipped with git).  (v)fork+execve is painful even on
Linux.

      reply	other threads:[~2021-02-11 17:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10 21:55 [PATCH] t1500: ensure current --since= behavior remains Eric Wong
2021-02-11 11:18 ` Jeff King
2021-02-11 17:06   ` Eric Wong [this message]

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=20210211170625.GA8280@dcvr \
    --to=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.