git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Paul Tan <pyokagan@gmail.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] reset cached ident date before creating objects
Date: Mon, 1 Aug 2016 14:00:47 -0400	[thread overview]
Message-ID: <20160801180047.sgzqgh7pvp2qzwug@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqbn1cl6qv.fsf@gitster.mtv.corp.google.com>

On Mon, Aug 01, 2016 at 10:49:12AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >> So maybe we would have to put reset_ident_date() at the end of the
> >> function instead, at least after git_committer_info() is called.
> >
> > Yes, although "reset and end" still feels a bit weird to me.
> >
> > I'd almost prefer to just have long-running programs insert resets at
> > strategic points.
> 
> Certainly "reset at the end" feels weird but it can be explained as
> "for a one-shot thing we use the first time of the default date and
> it gives a consistent timestamp; conceptually, things that make
> multiple commits are like doing that one-shot thing multiple times
> in a row."
> 
> When viewed that way, it is not _too_ bad, I would guess.

I think what bothers me is that you could use similar logic for "reset
at the beginning". The problem is that we don't know when "the
beginning" is. I thought it was inside commit_tree_extended(), but for
some operations, it's not, as Paul showed.

So when is "the end"? Is it at the end of commit_tree_extended()? I'm
not sure. Already we know it depends on whether you consider the ref
update part of the same operation. Whether that matters is debatable.
Are there other cases (an obvious one would be the human-readable bits
printed by git-commit, but it looks like those are done beforehand)?
Even if we do check every case, though, it seems like a fragile
assumption to be making.

So at this point I think I'd lean towards programs which have multiple
logical commit operations explicitly saying "I am starting a new
operation". It may be that we end up attaching more to that in the long
run than just timestamp resets, too (i.e., what other cached data might
there be that used to happen in separate sub-processes?).

-Peff

  parent reply	other threads:[~2016-08-01 19:02 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-28 23:35 Small trivial annoyance with the nice new builtin "git am" Linus Torvalds
2016-07-28 23:47 ` Junio C Hamano
2016-07-29  0:29   ` Jeff King
2016-07-29  0:37     ` Linus Torvalds
2016-07-29 15:50       ` Jeff King
2016-07-29 17:15         ` Junio C Hamano
2016-07-29 18:05           ` [PATCH] reset cached ident date before creating objects Jeff King
2016-07-29 18:12             ` Linus Torvalds
2016-07-29 18:15             ` Junio C Hamano
2016-07-29 18:25               ` Jeff King
2016-07-30  2:11             ` Paul Tan
2016-07-30  2:41               ` Jeff King
2016-08-01 17:49                 ` Junio C Hamano
2016-08-01 17:58                   ` Junio C Hamano
2016-08-01 18:12                     ` Jeff King
2016-08-01 18:17                       ` Jeff King
2016-08-01 18:00                   ` Jeff King [this message]
2016-08-01 19:37                     ` Jeff King
2016-08-01 21:54                       ` Junio C Hamano
2016-08-01 22:00                         ` Jeff King
2016-07-29 18:10       ` Small trivial annoyance with the nice new builtin "git am" Junio C Hamano
2016-07-29 17:06     ` Junio C Hamano
2016-07-29  0:21 ` Jeff King
2016-07-29  0:32   ` Linus Torvalds
2016-07-29  8:19     ` Christian Couder
2016-07-29 17:18     ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160801180047.sgzqgh7pvp2qzwug@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pyokagan@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).