Re: [RFC PATCH v2 2/2] hook: remote-suggested hooks

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 27, 2021, 22:40 UTC
Message-ID: <xmqqmtq7crdz.fsf@gitster.g>
In-Reply-To: <20210727213942.2574308-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> I think both "I want to vet" and "good enough for project X is good
> enough for me" are both reasonable points of view, and this
> remote-suggested hook scheme supports both.

Sure.

I was just pointing out that the design is opinionated and not giving two points of view fair chance to compete. It will strongly encourage users to the latter choice by prodding them when they want to do a hook-invoking operation (like "git commit").

Not that opinionated design is necessarily a bad thing.

Show 9 quoted lines

> I don't think we should compare the installed .git/hooks/pre-commit with
> remotes/origin/suggested-hooks (since the user may have locally modified
> that hook), so a solution involving storing the OID of the installed
> hook somewhere (I haven't figured out where, though) and comparing that
> OID against remotes/origin/suggested-hooks would be reasonable and would
> be compatible with the current approach (as opposed to the one which
> Ævar describes which, if I understand it correctly, would require
> "commit" to access the network to figure out if the hook the client has
> is the latest one).

Coping with local modification would not be rocket science.

If I were to do this, when the end-user approves installation of and/or updates from remotes/origin/suggested-hooks/, the following would happen:

 (1) If .git/hooks/* does not have the hook installed, copy it from
     the suggested hooks, and append two line trailer:

	# do not edit below
	# hook taken from <suggested hooks blob object name>

 (2) If .git/hooks/* does hold the hook, look for the "hook taken
     from" trailer

   (a) if "hook taken from" trailer is missing (i.e. it came from
       somewhere other than "remote suggested" workflow) or it does
       not point at a valid blob object, jump to "conflict exists"
       below.

   (b) using that (old) blob object name, perform (1) to recreate
       the hook the user would have seen when on-disk version of
       hook was created.  Difference between that and what is
       on-disk is the end-user customization.

       extract the current blob object from the suggested hooks tree
       object, do the same as (1), and then replay the end-user
       customization we figured out above.

       If the replaying succeeds cleanly, we are done.  Otherwise we
       have conflicts that cannot be resolved automatically.

   (c) "conflict exists".  The usual three-way merge resolution is
       needed.  I'd suggest to give users two (or three) files:

       - Rename the current version the user has to *.bak;
       - The new version from the project in the final file;
       - The patch obtained in (b) above, if exists in a separate file.

       and ask them to carry their customization forward to the
       second one (this is in line with the "we encourage them to
       adopt the project preferences" philosophy this proposal is
       taking us, I think).

I think configuration files under /etc/ on Debian-based distros have been managed in a similar way for at least the past 10 years if not longer, and since we are version control software ourselves, the conflict resolution users are asked to perform in (2)-(c) shouldn't be too much of a burden to our users anyway.