git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Marc Stevens <marc@marc-stevens.nl>,
	Michael Kebe <michael.kebe@gmail.com>, Jeff King <peff@peff.net>,
	Stefan Beller <sbeller@google.com>,
	Brandon Williams <bmwill@google.com>
Subject: Re: [PATCH 2/3] sha1dc: use sha1collisiondetection as a submodule
Date: Sat, 20 May 2017 20:13:15 +0900	[thread overview]
Message-ID: <xmqqpof3srw4.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170518212858.3649-3-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 18 May 2017 21:28:57 +0000")

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Replace the forked sha1dc directory with a copy of the upstream code
> imported as a submodule. This is the exact same code as now exists in
> the sha1dc/ directory.
>
> The initial reason for copy/pasting the code into sha1dc and locally
> modifying it was that it needed to be altered to work with the git
> project.
>
> The upstream project has accepted my code changes to allow us to use
> their code as-is, see the preceding commit for details. So import the
> code as a submodule instead, this will make it easier to keep
> up-to-date with any upstream fixes or improvements.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  .gitmodules            | 4 ++++
>  Makefile               | 4 ++--
>  hash.h                 | 2 +-
>  sha1collisiondetection | 1 +
>  4 files changed, 8 insertions(+), 3 deletions(-)
>  create mode 100644 .gitmodules
>  create mode 160000 sha1collisiondetection

I am not sure how prepared our .travis.yml is to deal with a
submodule, I'd prefer to have this step broken down to two step
process.

That is, [PATCH 2.1/3] first adds an otherwise unused submodule, so
that people can optionally do "git submodule init && git submodule
update" so that they can compare the contents of sha1dc/ that has
been updated by [PATCH 1/3] with the up-to-date upstream.  Then
[PATCH 2.2/3] would update the Makefile and hash.h to use the code
in the submodule.

I actually would want to see us proceed even more cautiously---if
the latter-half, i.e. [PATCH 2.2/3], is arranged so that it uses the
new sha1collisiondetection/ only when the submodule is initialized
and populated, and otherwise it uses sha1dc/ as before, I would feel
a lot safer.  I wouldn't be this paranoid if this "let's start using
submodule ourselves" were done to some optional corner (like compat/
or contrib/ somewhere), but this is the default hash function.  I do
want to have something like this to force us (and submodule folks)
to get any kinks out, but I do not want to see many people not even
be able to build while this new arrangement is eased in.  Once
people are comfortable with the new arrangement to use code from
submodule, we can then take [PATCH 3/3] to remove the old sha1dc/
directory and the migration will be complete.

I also am not very happy with .gitmodules pointing at a single point
of failure.  It would be nice if you can arrange a couple of mirrors
and have a comment in .gitmodules file to tell folks that they can
use these alternates by insteadOf or some other mechanism.

Thanks.

  reply	other threads:[~2017-05-20 11:13 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-18 21:28 [PATCH 0/3] Update sha1dc from upstream & optionally make it a submodule Ævar Arnfjörð Bjarmason
2017-05-18 21:28 ` [PATCH 1/3] sha1dc: update from upstream Ævar Arnfjörð Bjarmason
2017-05-18 21:28 ` [PATCH 2/3] sha1dc: use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason
2017-05-20 11:13   ` Junio C Hamano [this message]
2017-05-20 11:54     ` [PATCH v2 0/2] Update sha1dc from upstream & optionally make it " Ævar Arnfjörð Bjarmason
2017-05-22 22:27       ` Junio C Hamano
2017-05-22 22:48         ` Stefan Beller
2017-05-23  3:22           ` Junio C Hamano
2017-05-23 10:55         ` Ævar Arnfjörð Bjarmason
2017-05-23 13:06           ` Junio C Hamano
2017-05-25 10:44             ` Ævar Arnfjörð Bjarmason
2017-05-25 23:31               ` Junio C Hamano
2017-05-20 11:54     ` [PATCH v2 1/2] sha1dc: update from upstream Ævar Arnfjörð Bjarmason
2017-05-20 11:54     ` [PATCH v2 2/2] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason
2017-05-22  1:33       ` Junio C Hamano
2017-05-22  2:48         ` Junio C Hamano
2017-05-22  8:27           ` Ævar Arnfjörð Bjarmason
2017-05-22  8:48             ` Junio C Hamano
2017-05-25 10:47     ` [PATCH 2/3] sha1dc: " Ævar Arnfjörð Bjarmason
2017-05-18 21:28 ` [PATCH 3/3] sha1dc: remove the unused sha1dc/ directory Ævar Arnfjörð Bjarmason

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=xmqqpof3srw4.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=marc@marc-stevens.nl \
    --cc=michael.kebe@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.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.
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).