list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Jeff King <>
To: "Ævar Arnfjörð Bjarmason" <>
Cc: Thomas Braun <>,
	Duy Nguyen <>,
	Junio C Hamano <>,
	Jeffrey Walton <>,
	Todd Zullinger <>, Git List <>,
	Marc Stevens <>
Subject: Re: disabling sha1dc unaligned access, was Re: One failed self test on Fedora 29
Date: Tue, 12 Mar 2019 07:05:49 -0400
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Mar 12, 2019 at 09:53:41AM +0100, Ævar Arnfjörð Bjarmason wrote:

> There's a at least a couple of aspects to this.
> One is whether we should have the submodule in
> sha1collisiondetection/. I agree that's probably a bad idea now
> per-se. Honestly I wasn't expecting the answer when I submitted the
> final patch to switch to it fully to be to the effect of submodules
> being too immature for the git project itself to use. So now we're
> effectively mid-series, and should maybe just back out.

I think it's especially funky because we have three different ways of
getting sha1dc (in-tree, submodule, or against an external library). And
I almost blindly submitted a patch making the in-tree version work
(since that's what's used by default, and what I use) which could have
totally broken things for the other use cases without anybody realizing
until the change trickled down to somebody who uses those flags.

(Technically in this case it wouldn't actually have _broken_ them, but
just not helped them, so they'd be no worse off. But hopefully you get
the point).

Speaking of external libraries, in some ways the issue I raised is no
different than it is for any external library, where we're at the mercy
of whatever version is on the system. The big dependency for us is
usually libcurl, and we do have to sometimes work around old versions

But I do think there's one thing that make the sha1dc submodule approach
more painful is that we don't control the content of the code, but we
_do_ build it ourselves with our usual compiler flags. So we're weirdly
intimate with it (and in fact, an external library would not have the
problem being discussed here, since it would have been built separately
without UBSan).

> I fully agree with what you've said in theory, but if we look at what's
> happened in practice we as a project are demonstrably not disciplined
> enough to manage upstream code like this without overtly perma-forking
> it.

I'm not sure I agree completely. Most of the things we've imported are
small enough that we're reasonably happy to accept them as a snapshot in
time and take ownership. I.e., I do not recall a lot of instances of
fixing bugs in compat/regex or compat/poll that we could have gotten
more easily by merging from upstream. But I admit I don't actually pay
much attention to those areas, so I might be completely off-base.

The one place I really _would_ have liked to remain compatible with
upstream is xdiff. And we were traditionally pretty hesitant to clean
things up there for fear of diverging. But in practice, upstream there
has been stagnant, and we've done most of the bug fixes and improvements
to it (in-tree).

> As far as I can tell none of the people changing that code went through
> the process of submitting a parallel upstream fix or seeing if the issue
> was fixed upstream and we could just update the code we were carrying,
> and of course that gets progressively harder for any one contributor as
> our divergence grows.

To be clear, I do sympathize with the notion that not pulling things
in-tree keeps our relationship with upstream more disciplined, and that
has value. I'm just not altogether clear how much it's really hurt us
overall to be undisciplined.


  reply	other threads:[~2019-03-12 11:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-08 10:48 Jeffrey Walton
2019-03-08 17:43 ` Todd Zullinger
2019-03-09 12:34   ` Jeffrey Walton
2019-03-09 13:12     ` Jeffrey Walton
2019-03-11  2:00       ` Junio C Hamano
2019-03-11  2:16         ` Jeffrey Walton
2019-03-11  3:37         ` disabling sha1dc unaligned access, was " Jeff King
2019-03-11 10:40           ` Jeffrey Walton
2019-03-11 18:19             ` Jeff King
2019-03-11 11:58           ` Duy Nguyen
2019-03-11 18:15             ` Thomas Braun
2019-03-11 18:23               ` Jeff King
2019-03-12  7:27                 ` Junio C Hamano
2019-03-12 10:51                   ` Jeff King
2019-03-13 11:47                     ` Thomas Braun
2019-03-13 15:39                       ` Jeff King
2019-03-13 16:00                         ` Ævar Arnfjörð Bjarmason
2019-03-12  8:53                 ` Ævar Arnfjörð Bjarmason
2019-03-12 11:05                   ` Jeff King [this message]
2019-03-12 12:09                     ` Ævar Arnfjörð Bjarmason
2019-03-12 21:01                       ` Jeff King
2019-03-12 21:06           ` [PATCH] Makefile: fix unaligned loads in sha1dc with UBSan Jeff King
2019-03-12 21:17             ` Ævar Arnfjörð Bjarmason
2019-03-12 21:19               ` Jeff King
2019-03-11  3:29     ` One failed self test on Fedora 29 Jeff King

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:

  List information:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ \
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
 note: .onion URLs require Tor:

code repositories for the project(s) associated with this inbox:

AGPL code for this site: git clone