* Git v2.13.1 SHA1 very broken @ 2017-06-05 20:34 Adam Dinwoodie 2017-06-05 21:05 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 29+ messages in thread From: Adam Dinwoodie @ 2017-06-05 20:34 UTC (permalink / raw) To: git; +Cc: Ævar Arnfjörð Bjarmason I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very significant number of test cases in both 32-bit and 64-bit Cygwin builds. The first failure is t0000.46 "validate object ID of a known tree"; output with -x and -v is below, although it's not very interesting: expecting success: test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a + test_eval_ret_=1 + want_trace + test t = t + test t = t + set +x error: last command exited with $?=1 not ok 46 - validate object ID of a known tree # # test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a # I have no idea where to even begin debugging this, but I'm happy to take pointers / try things out on my box. Cheers, Adam ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-05 20:34 Git v2.13.1 SHA1 very broken Adam Dinwoodie @ 2017-06-05 21:05 ` Ævar Arnfjörð Bjarmason 2017-06-05 23:20 ` Ramsay Jones 2017-06-06 1:20 ` Junio C Hamano 0 siblings, 2 replies; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-05 21:05 UTC (permalink / raw) To: Adam Dinwoodie; +Cc: Git Mailing List On Mon, Jun 5, 2017 at 10:34 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote: > I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears > a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very > significant number of test cases in both 32-bit and 64-bit Cygwin > builds. > > The first failure is t0000.46 "validate object ID of a known tree"; output with > -x and -v is below, although it's not very interesting: > > expecting success: > test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a > > ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a > + test_eval_ret_=1 > + want_trace > + test t = t > + test t = t > + set +x > error: last command exited with $?=1 > not ok 46 - validate object ID of a known tree > # > # test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a > # > > I have no idea where to even begin debugging this, but I'm happy to take > pointers / try things out on my box. That looks scary, can you please comment out this: #define SHA1DC_ALLOW_UNALIGNED_ACCESS In sha1dc/sha1.c and see if that helps, alternatively comment out the ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c The functional differences between 2.13.0 and 2.13.1 on that platform should be none aside from possibly those changes, unless I've missed something. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-05 21:05 ` Ævar Arnfjörð Bjarmason @ 2017-06-05 23:20 ` Ramsay Jones 2017-06-06 0:11 ` Ramsay Jones 2017-06-06 1:20 ` Junio C Hamano 1 sibling, 1 reply; 29+ messages in thread From: Ramsay Jones @ 2017-06-05 23:20 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Adam Dinwoodie; +Cc: Git Mailing List On 05/06/17 22:05, Ævar Arnfjörð Bjarmason wrote: > On Mon, Jun 5, 2017 at 10:34 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote: >> I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears >> a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very >> significant number of test cases in both 32-bit and 64-bit Cygwin >> builds. >> >> The first failure is t0000.46 "validate object ID of a known tree"; output with >> -x and -v is below, although it's not very interesting: >> >> expecting success: >> test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a >> >> ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a >> + test_eval_ret_=1 >> + want_trace >> + test t = t >> + test t = t >> + set +x >> error: last command exited with $?=1 >> not ok 46 - validate object ID of a known tree >> # >> # test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a >> # >> >> I have no idea where to even begin debugging this, but I'm happy to take >> pointers / try things out on my box. > > That looks scary, can you please comment out this: > > #define SHA1DC_ALLOW_UNALIGNED_ACCESS No, that doesn't fix it. > > In sha1dc/sha1.c and see if that helps, alternatively comment out the > ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c This can't possibly make a difference! ;-) However, rebuilding with: $ make OPENSSL_SHA1=YesPlease >out2 2>&1 ... make t0000-basic.sh pass just fine, so ... ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-05 23:20 ` Ramsay Jones @ 2017-06-06 0:11 ` Ramsay Jones 0 siblings, 0 replies; 29+ messages in thread From: Ramsay Jones @ 2017-06-06 0:11 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason, Adam Dinwoodie; +Cc: Git Mailing List On 06/06/17 00:20, Ramsay Jones wrote: > > > On 05/06/17 22:05, Ævar Arnfjörð Bjarmason wrote: >> On Mon, Jun 5, 2017 at 10:34 PM, Adam Dinwoodie <adam@dinwoodie.org> wrote: >>> I'm trying to compile Git v2.13.1 to release for Cygwin, but it appears >>> a010391 ("sha1dc: update from upstream", 2017-05-20) is breaking a very >>> significant number of test cases in both 32-bit and 64-bit Cygwin >>> builds. >>> >>> The first failure is t0000.46 "validate object ID of a known tree"; output with >>> -x and -v is below, although it's not very interesting: >>> >>> expecting success: >>> test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a >>> >>> ++ test ceb282701536fe61bea01075664405caa7d6343f = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a >>> + test_eval_ret_=1 >>> + want_trace >>> + test t = t >>> + test t = t >>> + set +x >>> error: last command exited with $?=1 >>> not ok 46 - validate object ID of a known tree >>> # >>> # test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a >>> # >>> >>> I have no idea where to even begin debugging this, but I'm happy to take >>> pointers / try things out on my box. >> >> That looks scary, can you please comment out this: >> >> #define SHA1DC_ALLOW_UNALIGNED_ACCESS > > No, that doesn't fix it. > >> >> In sha1dc/sha1.c and see if that helps, alternatively comment out the >> ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c > > This can't possibly make a difference! ;-) > > However, rebuilding with: > > $ make OPENSSL_SHA1=YesPlease >out2 2>&1 > > ... make t0000-basic.sh pass just fine, so ... commit 7e71542e8b ("sha1dc: avoid CPP macro collisions", 25-03-2017) runs t0000-basic.sh just fine. commit a0103914c2 ("sha1dc: update from upstream", 20-05-2017) fails when running t0000-basic.sh. I will look into this more tomorrow (it is late, I need sleep), unless someone finds the solution overnight, of course. Adam, you can build using the OPENSSL_SHA1 build variable for now (if you want to release v2.13.1), or wait for another maint release I suppose, ... I'll leave that to you! ;-) ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-05 21:05 ` Ævar Arnfjörð Bjarmason 2017-06-05 23:20 ` Ramsay Jones @ 2017-06-06 1:20 ` Junio C Hamano 2017-06-06 10:03 ` Adam Dinwoodie 1 sibling, 1 reply; 29+ messages in thread From: Junio C Hamano @ 2017-06-06 1:20 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Adam Dinwoodie, Git Mailing List Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > That looks scary, can you please comment out this: > > #define SHA1DC_ALLOW_UNALIGNED_ACCESS > > In sha1dc/sha1.c and see if that helps, alternatively comment out the > ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c That is merely a performance (and theoretical correctness) thing, no? > The functional differences between 2.13.0 and 2.13.1 on that platform > should be none aside from possibly those changes, unless I've missed > something. If it does not hash correctly, the cause is more likely that the endianness detection is going haywire. make CFLAGS="-DSHA1DC_FORCE_LITTLEENDIAN -g -O2 -Wall" or something like that, perhaps? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-06 1:20 ` Junio C Hamano @ 2017-06-06 10:03 ` Adam Dinwoodie 2017-06-06 11:55 ` Junio C Hamano 0 siblings, 1 reply; 29+ messages in thread From: Adam Dinwoodie @ 2017-06-06 10:03 UTC (permalink / raw) To: Junio C Hamano, git; +Cc: Ævar Arnfjörð Bjarmason, Ramsay Jones On Tue, Jun 06, 2017 at 10:20:45AM +0900, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > That looks scary, can you please comment out this: > > > > #define SHA1DC_ALLOW_UNALIGNED_ACCESS > > > > In sha1dc/sha1.c and see if that helps, alternatively comment out the > > ifdefs guarded by "#ifdef _MSC_VER" calls in sha1dc/sha1.c > > That is merely a performance (and theoretical correctness) thing, > no? Confirmed rebuilding with either of these suggested changes has t0000.46 still failing. > > The functional differences between 2.13.0 and 2.13.1 on that platform > > should be none aside from possibly those changes, unless I've missed > > something. > > If it does not hash correctly, the cause is more likely that the > endianness detection is going haywire. > > make CFLAGS="-DSHA1DC_FORCE_LITTLEENDIAN -g -O2 -Wall" Confirmed rebuilding with this option has t0000 passing. I can also get the test to pass with Ramsay Jones' suggestion of using OpenSSL's SHA1. Digging briefly into the endianness detection, it appears Cygwin has both _LITTLE_ENDIAN and _BIG_ENDIAN defined. Git's detection works by assuming it's in a little endian environment and switching to big endian if it detects any of the defines that indicate such, and a010391 adds _BIG_ENDIAN to the set of defines that indicate big endianness. The obvious quick fix would be one of the two below patches. I'll also take the discussion to the Cygwin mailing list in the hope that someone can explain why Cygwin defines both _LITTLE_ENDIAN and _BIG_ENDIAN (and indeed _PDP_ENDIAN). Patch 1 (probably safer?): diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 3dff80ac7..e47d004b1 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -36,6 +36,7 @@ #undef SHA1DC_BIGENDIAN #endif #if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \ + (!defined _LITTLE_ENDIAN) && \ ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \ defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ Patch 2: diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 3dff80ac7..8d7b1ee7d 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -38,7 +38,7 @@ #if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \ ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \ - defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ + defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN)) #define SHA1DC_BIGENDIAN ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-06 10:03 ` Adam Dinwoodie @ 2017-06-06 11:55 ` Junio C Hamano 2017-06-06 12:43 ` Adam Dinwoodie 2017-06-06 12:49 ` Git v2.13.1 SHA1 very broken Morten Welinder 0 siblings, 2 replies; 29+ messages in thread From: Junio C Hamano @ 2017-06-06 11:55 UTC (permalink / raw) To: Adam Dinwoodie; +Cc: git, Ævar Arnfjörð Bjarmason, Ramsay Jones Adam Dinwoodie <adam@dinwoodie.org> writes: > Digging briefly into the endianness detection, it appears Cygwin has > both _LITTLE_ENDIAN and _BIG_ENDIAN defined. Git's detection works by > assuming it's in a little endian environment and switching to big endian > if it detects any of the defines that indicate such, and a010391 adds > _BIG_ENDIAN to the set of defines that indicate big endianness. I suspect that the upstream has already fixed this one to cope with FreeBSD. My preference is that we do another import on top of the ab/sha1dc-maint topic, below the commit on ab/sha1dc that adds the upstream as a submodule. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-06 11:55 ` Junio C Hamano @ 2017-06-06 12:43 ` Adam Dinwoodie 2017-06-06 14:47 ` Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) Jason Pyeron 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason 2017-06-06 12:49 ` Git v2.13.1 SHA1 very broken Morten Welinder 1 sibling, 2 replies; 29+ messages in thread From: Adam Dinwoodie @ 2017-06-06 12:43 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason, Ramsay Jones On Tue, Jun 06, 2017 at 08:55:04PM +0900, Junio C Hamano wrote: > Adam Dinwoodie <adam@dinwoodie.org> writes: > > > Digging briefly into the endianness detection, it appears Cygwin has > > both _LITTLE_ENDIAN and _BIG_ENDIAN defined. Git's detection works by > > assuming it's in a little endian environment and switching to big endian > > if it detects any of the defines that indicate such, and a010391 adds > > _BIG_ENDIAN to the set of defines that indicate big endianness. > > I suspect that the upstream has already fixed this one to cope with > FreeBSD. My preference is that we do another import on top of the > ab/sha1dc-maint topic, below the commit on ab/sha1dc that adds the > upstream as a submodule. Apparently so! a010391 brings Git up to the upstream's cc46554 ("Skip temporary variable for SHA1DC_ALLOW_UNSIGNED_ACCESS", 2017-05-18); the problem has been fixed in upstream's a24eef5 ("rewrote Endianness selection", 2017-05-29). In the interim, I'll use the CFLAGS route to try to get a v2.13.1 build ready to release for Cygwin. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) 2017-06-06 12:43 ` Adam Dinwoodie @ 2017-06-06 14:47 ` Jason Pyeron 2017-06-06 15:04 ` Lars Schneider 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 29+ messages in thread From: Jason Pyeron @ 2017-06-06 14:47 UTC (permalink / raw) To: git Cc: 'Adam Dinwoodie', 'Junio C Hamano', 'Ævar Arnfjörð Bjarmason', 'Ramsay Jones' Do we have Jenkins (or something else) setup for Git? We would be happy to donate (slave) VMs for cygwin builds og Git. -Jason Pyeron ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) 2017-06-06 14:47 ` Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) Jason Pyeron @ 2017-06-06 15:04 ` Lars Schneider 2017-07-02 20:35 ` Adam Dinwoodie 0 siblings, 1 reply; 29+ messages in thread From: Lars Schneider @ 2017-06-06 15:04 UTC (permalink / raw) To: Jason Pyeron Cc: git, Adam Dinwoodie, Junio C Hamano, Ævar Arnfjörð Bjarmason, Ramsay Jones > On 06 Jun 2017, at 16:47, Jason Pyeron <jpyeron@pdinc.us> wrote: > > Do we have Jenkins (or something else) setup for Git? > > We would be happy to donate (slave) VMs for cygwin builds og Git. > > -Jason Pyeron > We use TravisCI for Linux, Mac, and (in a special way) Windows: https://travis-ci.org/git/git Windows is not supported by TravisCI. We just use TravisCI to trigger a build on MS Azure and read the results: https://github.com/git/git/commit/029aeeed55f6bfe8014e8ffe5fc7a6f2e5b110fc Maybe we could trigger a Cgywin build on your slaves in the same way? - Lars ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) 2017-06-06 15:04 ` Lars Schneider @ 2017-07-02 20:35 ` Adam Dinwoodie 2017-07-03 12:34 ` Johannes Schindelin 0 siblings, 1 reply; 29+ messages in thread From: Adam Dinwoodie @ 2017-07-02 20:35 UTC (permalink / raw) To: Lars Schneider Cc: Jason Pyeron, git, Junio C Hamano, Ævar Arnfjörð Bjarmason, Ramsay Jones On Tue, Jun 06, 2017 at 05:04:32PM +0200, Lars Schneider wrote: > > On 06 Jun 2017, at 16:47, Jason Pyeron <jpyeron@pdinc.us> wrote: > > > > Do we have Jenkins (or something else) setup for Git? > > > > We would be happy to donate (slave) VMs for cygwin builds og Git. > > > > -Jason Pyeron > > > > We use TravisCI for Linux, Mac, and (in a special way) Windows: > https://travis-ci.org/git/git > > Windows is not supported by TravisCI. We just use TravisCI to > trigger a build on MS Azure and read the results: > https://github.com/git/git/commit/029aeeed55f6bfe8014e8ffe5fc7a6f2e5b110fc > > Maybe we could trigger a Cgywin build on your slaves in the same way? I tried setting up a Cygwin Git build on AppVeyor a little while ago, but the problem I found is that Cygwin builds are slow, particularly if you want to also run the test suite. I do the builds for the Cygwin distribution on my normal PC (so reasonably powerful but definitely not devoted to the purpose), and doing the build and running the default tests takes in the region of 8 hours for the 64-bit build and 12 hours for the 32-bit build. At the moment, I'm trying to set up automated regular builds on my PC using BuildBot. I think that, short of someone donating some fairly significant resources for Cygwin builds, that's going to be the closest I'll be able to find for spotting problems early. It's a project in my currently limited spare time, though, and not something I've done before, so it's taking a little while to get going. Adam ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) 2017-07-02 20:35 ` Adam Dinwoodie @ 2017-07-03 12:34 ` Johannes Schindelin 0 siblings, 0 replies; 29+ messages in thread From: Johannes Schindelin @ 2017-07-03 12:34 UTC (permalink / raw) To: Adam Dinwoodie Cc: Lars Schneider, Jason Pyeron, git, Junio C Hamano, Ævar Arnfjörð Bjarmason, Ramsay Jones Hi Adam, On Sun, 2 Jul 2017, Adam Dinwoodie wrote: > I do the builds for the Cygwin distribution on my normal PC (so > reasonably powerful but definitely not devoted to the purpose), and > doing the build and running the default tests takes in the region of 8 > hours for the 64-bit build and 12 hours for the 32-bit build. Wow. 8 hours. I take it that you are bitten by the same issue as Git for Windows, where Git's test suite uses Unix shell scripting rather heavily, and Unix shell simply requiring tons of expensive emulation to run decently on Windows. > At the moment, I'm trying to set up automated regular builds on my PC > using BuildBot. I think that, short of someone donating some fairly > significant resources for Cygwin builds, that's going to be the closest > I'll be able to find for spotting problems early. It's a project in my > currently limited spare time, though, and not something I've done > before, so it's taking a little while to get going. How automated is your process? (Including, most importantly, updating the Cygwin build setup...) I ask because I could possibly set up something using the existing infrastructure for Git for Windows' testing, as long as 1) everything is scripted, and 2) you do not need interactive access to any box (I mainly use Docker containers to run the builds) If you are interested, feel free to ping me via private mail. Ciao, Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/3] update sha1dc 2017-06-06 12:43 ` Adam Dinwoodie 2017-06-06 14:47 ` Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) Jason Pyeron @ 2017-06-06 15:12 ` Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 1/3] sha1dc: update from upstream Ævar Arnfjörð Bjarmason ` (4 more replies) 1 sibling, 5 replies; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 UTC (permalink / raw) To: git Cc: Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe, Ævar Arnfjörð Bjarmason This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1, and hopefully not regressing elsewhere. Liam, it would be much appreciated if you could test this on SPARC. As before the "sha1dc: update from upstream" patch is what should fast-track to master/maint and be in 2.13.2, the other two are the cooking submodule use, that's all unchanged aside from of course the submodule pointing to the same upstream commit as the code import itself does. Junio: There's a whitespace change to sha1.h that am warns about, but which it applies anyway that you didn't apply from my previous patch. I think it probably makes sense to just take upstream's whitespace shenanigans as-is instead of seeing that diff every time we update. I guess we could also send them a pull request... Junio C Hamano (1): sha1collisiondetection: automatically enable when submodule is populated Ævar Arnfjörð Bjarmason (2): sha1dc: update from upstream sha1dc: optionally use sha1collisiondetection as a submodule .gitmodules | 4 ++++ Makefile | 16 ++++++++++++++++ hash.h | 4 ++++ sha1collisiondetection | 1 + sha1dc/sha1.c | 30 ++++++++++++++++++++++++------ sha1dc/sha1.h | 6 +++--- 6 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 .gitmodules create mode 160000 sha1collisiondetection -- 2.13.0.506.g27d5fe0cd ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/3] sha1dc: update from upstream 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 ` Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 4 siblings, 0 replies; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 UTC (permalink / raw) To: git Cc: Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe, Ævar Arnfjörð Bjarmason Update sha1dc from the latest version by the upstream maintainer[1]. See commit a0103914c2 ("sha1dc: update from upstream", 2017-05-20) for the latest update. That update was done sans some whitespace changes by upstream, which is why the diff here isn't the same as the upstream cc46554..e139984. It also brings in a change[2] upstream made which should hopefully address the breakage in 2.13.1 on Cygwin, see [3]. Cygwin defines both _BIG_ENDIAN and _LITTLE_ENDIAN. Adam Dinwoodie reports on the mailing list that that upstream commit fixes the issue on Cygwin[4]. 1. https://github.com/cr-marcstevens/sha1collisiondetection/commit/e1399840b501a68ac6c8d7ed9a5cb1455480200e 2. https://github.com/cr-marcstevens/sha1collisiondetection/commit/a24eef58c0684078405f8c7a89f9b78271432005 3. <20170606100355.GC25777@dinwoodie.org> (https://public-inbox.org/git/20170606100355.GC25777@dinwoodie.org/) 4. <20170606124323.GD25777@dinwoodie.org> (https://public-inbox.org/git/20170606124323.GD25777@dinwoodie.org/) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- sha1dc/sha1.c | 30 ++++++++++++++++++++++++------ sha1dc/sha1.h | 6 +++--- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c index 3dff80ac72..facea1bb56 100644 --- a/sha1dc/sha1.c +++ b/sha1dc/sha1.c @@ -35,15 +35,33 @@ #ifdef SHA1DC_BIGENDIAN #undef SHA1DC_BIGENDIAN #endif -#if (!defined SHA1DC_FORCE_LITTLEENDIAN) && \ - ((defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ - (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) || \ - defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN__) || defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ - defined(_MIPSEB) || defined(__MIPSEB) || defined(__MIPSEB__) || defined(SHA1DC_FORCE_BIGENDIAN)) +#if (defined(_BYTE_ORDER) || defined(__BYTE_ORDER) || defined(__BYTE_ORDER__)) + +#if ((defined(_BYTE_ORDER) && (_BYTE_ORDER == _BIG_ENDIAN)) || \ + (defined(__BYTE_ORDER) && (__BYTE_ORDER == __BIG_ENDIAN)) || \ + (defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __BIG_ENDIAN__)) ) #define SHA1DC_BIGENDIAN +#endif + +#else + +#if (defined(_BIG_ENDIAN) || defined(__BIG_ENDIAN) || defined(__BIG_ENDIAN__) || \ + defined(__ARMEB__) || defined(__THUMBEB__) || defined(__AARCH64EB__) || \ + defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \ + defined(__sparc)) +#define SHA1DC_BIGENDIAN +#endif -#endif /*ENDIANNESS SELECTION*/ +#endif + +#if (defined(SHA1DC_FORCE_LITTLEENDIAN) && defined(SHA1DC_BIGENDIAN)) +#undef SHA1DC_BIGENDIAN +#endif +#if (defined(SHA1DC_FORCE_BIGENDIAN) && !defined(SHA1DC_BIGENDIAN)) +#define SHA1DC_BIGENDIAN +#endif +/*ENDIANNESS SELECTION*/ #if (defined SHA1DC_FORCE_UNALIGNED_ACCESS || \ defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || defined(__x86_64) || \ diff --git a/sha1dc/sha1.h b/sha1dc/sha1.h index a0ff5d1305..1e4e94be54 100644 --- a/sha1dc/sha1.h +++ b/sha1dc/sha1.h @@ -61,9 +61,9 @@ void SHA1DCInit(SHA1_CTX*); Function to enable safe SHA-1 hashing: Collision attacks are thwarted by hashing a detected near-collision block 3 times. Think of it as extending SHA-1 from 80-steps to 240-steps for such blocks: - The best collision attacks against SHA-1 have complexity about 2^60, - thus for 240-steps an immediate lower-bound for the best cryptanalytic attacks would be 2^180. - An attacker would be better off using a generic birthday search of complexity 2^80. + The best collision attacks against SHA-1 have complexity about 2^60, + thus for 240-steps an immediate lower-bound for the best cryptanalytic attacks would be 2^180. + An attacker would be better off using a generic birthday search of complexity 2^80. Enabling safe SHA-1 hashing will result in the correct SHA-1 hash for messages where no collision attack was detected, but it will result in a different SHA-1 hash for messages where a collision attack was detected. -- 2.13.0.506.g27d5fe0cd ^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 1/3] sha1dc: update from upstream Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 ` Ævar Arnfjörð Bjarmason 2017-06-06 18:48 ` Stefan Beller 2017-06-06 15:12 ` [PATCH 3/3] sha1collisiondetection: automatically enable when submodule is populated Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 4 siblings, 1 reply; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 UTC (permalink / raw) To: git Cc: Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe, Ævar Arnfjörð Bjarmason Add an option to use the sha1collisiondetection library from the submodule in sha1collisiondetection/ instead of in the copy in the sha1dc/ directory. This allows us to try out the submodule in sha1collisiondetection without breaking the build for anyone who's not expecting them as we work out any kinks. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- .gitmodules | 4 ++++ Makefile | 12 ++++++++++++ hash.h | 4 ++++ sha1collisiondetection | 1 + 4 files changed, 21 insertions(+) create mode 100644 .gitmodules create mode 160000 sha1collisiondetection diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000000..cbeebdab7a --- /dev/null +++ b/.gitmodules @@ -0,0 +1,4 @@ +[submodule "sha1collisiondetection"] + path = sha1collisiondetection + url = https://github.com/cr-marcstevens/sha1collisiondetection.git + branch = master diff --git a/Makefile b/Makefile index 7c621f7f76..adeff26d57 100644 --- a/Makefile +++ b/Makefile @@ -146,6 +146,12 @@ all:: # algorithm. This is slower, but may detect attempted collision attacks. # Takes priority over other *_SHA1 knobs. # +# Define DC_SHA1_SUBMODULE in addition to DC_SHA1 to use the +# sha1collisiondetection shipped as a submodule instead of the +# non-submodule copy in sha1dc/. This is an experimental option used +# by the git project to migrate to using sha1collisiondetection as a +# submodule. +# # Define OPENSSL_SHA1 environment variable when running make to link # with the SHA1 routine from openssl library. # @@ -1416,8 +1422,14 @@ ifdef APPLE_COMMON_CRYPTO BASIC_CFLAGS += -DSHA1_APPLE else DC_SHA1 := YesPlease +ifdef DC_SHA1_SUBMODULE + LIB_OBJS += sha1collisiondetection/lib/sha1.o + LIB_OBJS += sha1collisiondetection/lib/ubc_check.o + BASIC_CFLAGS += -DDC_SHA1_SUBMODULE +else LIB_OBJS += sha1dc/sha1.o LIB_OBJS += sha1dc/ubc_check.o +endif BASIC_CFLAGS += \ -DSHA1_DC \ -DSHA1DC_NO_STANDARD_INCLUDES \ diff --git a/hash.h b/hash.h index a11fc9233f..bef3e630a0 100644 --- a/hash.h +++ b/hash.h @@ -8,7 +8,11 @@ #elif defined(SHA1_OPENSSL) #include <openssl/sha.h> #elif defined(SHA1_DC) +#ifdef DC_SHA1_SUBMODULE +#include "sha1collisiondetection/lib/sha1.h" +#else #include "sha1dc/sha1.h" +#endif #else /* SHA1_BLK */ #include "block-sha1/sha1.h" #endif diff --git a/sha1collisiondetection b/sha1collisiondetection new file mode 160000 index 0000000000..e1399840b5 --- /dev/null +++ b/sha1collisiondetection @@ -0,0 +1 @@ +Subproject commit e1399840b501a68ac6c8d7ed9a5cb1455480200e -- 2.13.0.506.g27d5fe0cd ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-06 15:12 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason @ 2017-06-06 18:48 ` Stefan Beller 2017-06-06 19:03 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 29+ messages in thread From: Stefan Beller @ 2017-06-06 18:48 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > Add an option to use the sha1collisiondetection library from the > submodule in sha1collisiondetection/ instead of in the copy in the > sha1dc/ directory. > > This allows us to try out the submodule in sha1collisiondetection > without breaking the build for anyone who's not expecting them as we > work out any kinks. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Other projects using submodules sometimes have a .gitattributes entry to have .gitmodules not exported via git-archive. Do we want a similar thing? Speaking of attributes, I wonder if we want to specify the .gitmodules file to be text with unixy file endings: Having an entry .gitattributes eol=crlf to simulate a Windows environment doesn't harm submodule operation, which is good. I'll check if we have a test for that. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-06 18:48 ` Stefan Beller @ 2017-06-06 19:03 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:09 ` Stefan Beller 0 siblings, 1 reply; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 19:03 UTC (permalink / raw) To: Stefan Beller Cc: git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 6, 2017 at 8:48 PM, Stefan Beller <sbeller@google.com> wrote: > On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> Add an option to use the sha1collisiondetection library from the >> submodule in sha1collisiondetection/ instead of in the copy in the >> sha1dc/ directory. >> >> This allows us to try out the submodule in sha1collisiondetection >> without breaking the build for anyone who's not expecting them as we >> work out any kinks. >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > > Other projects using submodules sometimes have > a .gitattributes entry to have .gitmodules not exported > via git-archive. Do we want a similar thing? Right now we end up with an empty directory due to the issue you noted in https://public-inbox.org/git/CAGZ79kZC98CxA69QjmX2s_SU6z1CSgKgwZeqvwiMRAQc6+S3xg@mail.gmail.com/ It's probably best to have the .gitmodules file as some hint that something should be there. We also ship the other .git* files. > Speaking of attributes, I wonder if we want to specify > the .gitmodules file to be text with unixy file endings: > Having an entry > .gitattributes eol=crlf > to simulate a Windows environment doesn't harm > submodule operation, which is good. I'll check if we > have a test for that. I have no idea what that would do or why we'd have it, but I'm going to understand this as you looking into it :) ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-06 19:03 ` Ævar Arnfjörð Bjarmason @ 2017-06-06 19:09 ` Stefan Beller 0 siblings, 0 replies; 29+ messages in thread From: Stefan Beller @ 2017-06-06 19:09 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 6, 2017 at 12:03 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Tue, Jun 6, 2017 at 8:48 PM, Stefan Beller <sbeller@google.com> wrote: >> On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason >> <avarab@gmail.com> wrote: >>> Add an option to use the sha1collisiondetection library from the >>> submodule in sha1collisiondetection/ instead of in the copy in the >>> sha1dc/ directory. >>> >>> This allows us to try out the submodule in sha1collisiondetection >>> without breaking the build for anyone who's not expecting them as we >>> work out any kinks. >>> >>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> >> Other projects using submodules sometimes have >> a .gitattributes entry to have .gitmodules not exported >> via git-archive. Do we want a similar thing? > > Right now we end up with an empty directory due to the issue you noted > in https://public-inbox.org/git/CAGZ79kZC98CxA69QjmX2s_SU6z1CSgKgwZeqvwiMRAQc6+S3xg@mail.gmail.com/ > > It's probably best to have the .gitmodules file as some hint that > something should be there. We also ship the other .git* files. Ok, but then let's talk about the other .git* files, would we want to distribute these via tarballs? (I guess it is a minor thing if at all and nobody downloading a git tarball would be surprised by these metadata files or annoyed by them, so all is good?) > >> Speaking of attributes, I wonder if we want to specify >> the .gitmodules file to be text with unixy file endings: >> Having an entry >> .gitattributes eol=crlf >> to simulate a Windows environment doesn't harm >> submodule operation, which is good. I'll check if we >> have a test for that. > > I have no idea what that would do or why we'd have it, but I'm going > to understand this as you looking into it :) I looked briefly into it and it seems to be no problem just as config files on Windows are no problem. I just spoke up too quickly. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] sha1collisiondetection: automatically enable when submodule is populated 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 1/3] sha1dc: update from upstream Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 ` Ævar Arnfjörð Bjarmason 2017-06-06 18:23 ` [PATCH 0/3] update sha1dc Stefan Beller 2017-06-13 2:09 ` [PATCH 0/3] update sha1dc Liam R. Howlett 4 siblings, 0 replies; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 15:12 UTC (permalink / raw) To: git Cc: Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe, Ævar Arnfjörð Bjarmason From: Junio C Hamano <gitster@pobox.com> If a user wants to experiment with the version of collision detecting sha1 from the submodule, the user needed to not just populate the submodule but also needed to turn the knob. A Makefile trick is easy enough to do so, so let's do this. When somebody with a copy of the submodule populated wants not to use it, that can be done by overriding it in config.mak or from the command line. Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Makefile b/Makefile index adeff26d57..eeccbff1cd 100644 --- a/Makefile +++ b/Makefile @@ -993,6 +993,10 @@ EXTLIBS = GIT_USER_AGENT = git/$(GIT_VERSION) +ifeq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h) +DC_SHA1_SUBMODULE = auto +endif + include config.mak.uname -include config.mak.autogen -include config.mak -- 2.13.0.506.g27d5fe0cd ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] update sha1dc 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 2017-06-06 15:12 ` [PATCH 3/3] sha1collisiondetection: automatically enable when submodule is populated Ævar Arnfjörð Bjarmason @ 2017-06-06 18:23 ` Stefan Beller 2017-06-06 18:51 ` Ævar Arnfjörð Bjarmason 2017-06-13 2:09 ` [PATCH 0/3] update sha1dc Liam R. Howlett 4 siblings, 1 reply; 29+ messages in thread From: Stefan Beller @ 2017-06-06 18:23 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1, > and hopefully not regressing elsewhere. Liam, it would be much > appreciated if you could test this on SPARC. > > As before the "sha1dc: update from upstream" patch is what should > fast-track to master/maint and be in 2.13.2, the other two are the > cooking submodule use, that's all unchanged aside from of course the > submodule pointing to the same upstream commit as the code import > itself does. > > Junio: There's a whitespace change to sha1.h that am warns about, but > which it applies anyway that you didn't apply from my previous > patch. I think it probably makes sense to just take upstream's > whitespace shenanigans as-is instead of seeing that diff every time we > update. I guess we could also send them a pull request... I would suggest the pull request. Also as to not make the mistake from before that I jump on the submodule bandwagon here: Patch 1 ought to go in its on series/patch, so with that out the way we have more time to consider the pros and cons of the rest of the series? Thanks, Stefan ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] update sha1dc 2017-06-06 18:23 ` [PATCH 0/3] update sha1dc Stefan Beller @ 2017-06-06 18:51 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:01 ` [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations Jeff King 0 siblings, 1 reply; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 18:51 UTC (permalink / raw) To: Stefan Beller Cc: git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 6, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote: > On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1, >> and hopefully not regressing elsewhere. Liam, it would be much >> appreciated if you could test this on SPARC. >> >> As before the "sha1dc: update from upstream" patch is what should >> fast-track to master/maint and be in 2.13.2, the other two are the >> cooking submodule use, that's all unchanged aside from of course the >> submodule pointing to the same upstream commit as the code import >> itself does. >> >> Junio: There's a whitespace change to sha1.h that am warns about, but >> which it applies anyway that you didn't apply from my previous >> patch. I think it probably makes sense to just take upstream's >> whitespace shenanigans as-is instead of seeing that diff every time we >> update. I guess we could also send them a pull request... > > I would suggest the pull request. Looking at this again it's not a bug, just upstream choosing to indent a comment with spaces, not a bug. So it makes sense to just apply as-is so we don't have that diff with them / different sha1s on the files etc. > Also as to not make the mistake from before that I jump on the > submodule bandwagon here: > Patch 1 ought to go in its on series/patch, so with that out the way > we have more time to consider the pros and cons of the rest of > the series? Yes it makes perfect sense to just take the 1st patch here and make the submodule changes cook. This is just how I submitted it the last time and Junio took the 1st patch into a maint topic, so I figured I'd send it like this again. ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations 2017-06-06 18:51 ` Ævar Arnfjörð Bjarmason @ 2017-06-06 19:01 ` Jeff King 2017-06-06 19:04 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:05 ` Stefan Beller 0 siblings, 2 replies; 29+ messages in thread From: Jeff King @ 2017-06-06 19:01 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Stefan Beller, git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 06, 2017 at 08:51:35PM +0200, Ævar Arnfjörð Bjarmason wrote: > On Tue, Jun 6, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote: > > On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason > > <avarab@gmail.com> wrote: > >> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1, > >> and hopefully not regressing elsewhere. Liam, it would be much > >> appreciated if you could test this on SPARC. > >> > >> As before the "sha1dc: update from upstream" patch is what should > >> fast-track to master/maint and be in 2.13.2, the other two are the > >> cooking submodule use, that's all unchanged aside from of course the > >> submodule pointing to the same upstream commit as the code import > >> itself does. > >> > >> Junio: There's a whitespace change to sha1.h that am warns about, but > >> which it applies anyway that you didn't apply from my previous > >> patch. I think it probably makes sense to just take upstream's > >> whitespace shenanigans as-is instead of seeing that diff every time we > >> update. I guess we could also send them a pull request... > > > > I would suggest the pull request. > > Looking at this again it's not a bug, just upstream choosing to indent > a comment with spaces, not a bug. > > So it makes sense to just apply as-is so we don't have that diff with > them / different sha1s on the files etc. Agreed. Maybe we'd also want this patch: -- >8 -- Subject: sha1dc: ignore indent-with-non-tab whitespace violations The upstream sha1dc code indents some lines with spaces. While this doesn't match Git's coding guidelines, it's better to leave this imported code untouched than to try to make it match our style. However, we can use .gitattributes to tell "diff --check" and "git am" not to bother us about it. Signed-off-by: Jeff King <peff@peff.net> --- sha1dc/.gitattributes | 1 + 1 file changed, 1 insertion(+) create mode 100644 sha1dc/.gitattributes diff --git a/sha1dc/.gitattributes b/sha1dc/.gitattributes new file mode 100644 index 000000000..da53f4054 --- /dev/null +++ b/sha1dc/.gitattributes @@ -0,0 +1 @@ +* whitespace=-indent-with-non-tab -- 2.13.1.664.g1b5a21ec3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations 2017-06-06 19:01 ` [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations Jeff King @ 2017-06-06 19:04 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:05 ` Stefan Beller 1 sibling, 0 replies; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-06 19:04 UTC (permalink / raw) To: Jeff King Cc: Stefan Beller, git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe On Tue, Jun 6, 2017 at 9:01 PM, Jeff King <peff@peff.net> wrote: > On Tue, Jun 06, 2017 at 08:51:35PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> On Tue, Jun 6, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote: >> > On Tue, Jun 6, 2017 at 8:12 AM, Ævar Arnfjörð Bjarmason >> > <avarab@gmail.com> wrote: >> >> This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1, >> >> and hopefully not regressing elsewhere. Liam, it would be much >> >> appreciated if you could test this on SPARC. >> >> >> >> As before the "sha1dc: update from upstream" patch is what should >> >> fast-track to master/maint and be in 2.13.2, the other two are the >> >> cooking submodule use, that's all unchanged aside from of course the >> >> submodule pointing to the same upstream commit as the code import >> >> itself does. >> >> >> >> Junio: There's a whitespace change to sha1.h that am warns about, but >> >> which it applies anyway that you didn't apply from my previous >> >> patch. I think it probably makes sense to just take upstream's >> >> whitespace shenanigans as-is instead of seeing that diff every time we >> >> update. I guess we could also send them a pull request... >> > >> > I would suggest the pull request. >> >> Looking at this again it's not a bug, just upstream choosing to indent >> a comment with spaces, not a bug. >> >> So it makes sense to just apply as-is so we don't have that diff with >> them / different sha1s on the files etc. > > Agreed. Maybe we'd also want this patch: Great, that makes perfect sense for prepending to the series. > -- >8 -- > Subject: sha1dc: ignore indent-with-non-tab whitespace violations > > The upstream sha1dc code indents some lines with spaces. > While this doesn't match Git's coding guidelines, it's better > to leave this imported code untouched than to try to make it > match our style. However, we can use .gitattributes to tell > "diff --check" and "git am" not to bother us about it. > > Signed-off-by: Jeff King <peff@peff.net> > --- > sha1dc/.gitattributes | 1 + > 1 file changed, 1 insertion(+) > create mode 100644 sha1dc/.gitattributes > > diff --git a/sha1dc/.gitattributes b/sha1dc/.gitattributes > new file mode 100644 > index 000000000..da53f4054 > --- /dev/null > +++ b/sha1dc/.gitattributes > @@ -0,0 +1 @@ > +* whitespace=-indent-with-non-tab > -- > 2.13.1.664.g1b5a21ec3 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations 2017-06-06 19:01 ` [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations Jeff King 2017-06-06 19:04 ` Ævar Arnfjörð Bjarmason @ 2017-06-06 19:05 ` Stefan Beller 1 sibling, 0 replies; 29+ messages in thread From: Stefan Beller @ 2017-06-06 19:05 UTC (permalink / raw) To: Jeff King Cc: Ævar Arnfjörð Bjarmason, git@vger.kernel.org, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Liam R . Howlett, Michael Kebe > Subject: sha1dc: ignore indent-with-non-tab whitespace violations > > The upstream sha1dc code indents some lines with spaces. > While this doesn't match Git's coding guidelines, it's better > to leave this imported code untouched than to try to make it > match our style. However, we can use .gitattributes to tell > "diff --check" and "git am" not to bother us about it. > > Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Stefan Beller <sbeller@google.com> > --- > sha1dc/.gitattributes | 1 + > 1 file changed, 1 insertion(+) > create mode 100644 sha1dc/.gitattributes > > diff --git a/sha1dc/.gitattributes b/sha1dc/.gitattributes > new file mode 100644 > index 000000000..da53f4054 > --- /dev/null > +++ b/sha1dc/.gitattributes > @@ -0,0 +1 @@ > +* whitespace=-indent-with-non-tab > -- > 2.13.1.664.g1b5a21ec3 > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] update sha1dc 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason ` (3 preceding siblings ...) 2017-06-06 18:23 ` [PATCH 0/3] update sha1dc Stefan Beller @ 2017-06-13 2:09 ` Liam R. Howlett 4 siblings, 0 replies; 29+ messages in thread From: Liam R. Howlett @ 2017-06-13 2:09 UTC (permalink / raw) To: ?var Arnfj?r? Bjarmason Cc: git, Junio C Hamano, Adam Dinwoodie, Ramsay Jones, Michael Kebe * ?var Arnfj?r? Bjarmason <avarab@gmail.com> [170606 11:12]: > This updates sha1dc fixing the issue on Cygwin introduced in 2.13.1, > and hopefully not regressing elsewhere. Liam, it would be much > appreciated if you could test this on SPARC. Tested and confirmed this works for SPARC with no build flags. Sorry for the delay, I was away last week. Thanks, Liam ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Git v2.13.1 SHA1 very broken 2017-06-06 11:55 ` Junio C Hamano 2017-06-06 12:43 ` Adam Dinwoodie @ 2017-06-06 12:49 ` Morten Welinder 1 sibling, 0 replies; 29+ messages in thread From: Morten Welinder @ 2017-06-06 12:49 UTC (permalink / raw) To: Junio C Hamano Cc: Adam Dinwoodie, GIT Mailing List, Ævar Arnfjörð Bjarmason, Ramsay Jones One could have configure ask some existing dependency that has already determined the byte order. For example: # perl -e 'use Config; $o=$Config{byteorder}; print(($o=~/^1234/ ? "little" : ($o=~/4321$/ ? "big" : "weird")), "\n");' little Good: less #ifdef soup; bad: not so great for cross-compiling. (That's the integer byte order. The floating-point byte order can be different; hopefully git doesn't care.) Morten On Tue, Jun 6, 2017 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote: > Adam Dinwoodie <adam@dinwoodie.org> writes: > >> Digging briefly into the endianness detection, it appears Cygwin has >> both _LITTLE_ENDIAN and _BIG_ENDIAN defined. Git's detection works by >> assuming it's in a little endian environment and switching to big endian >> if it detects any of the defines that indicate such, and a010391 adds >> _BIG_ENDIAN to the set of defines that indicate big endianness. > > I suspect that the upstream has already fixed this one to cope with > FreeBSD. My preference is that we do another import on top of the > ab/sha1dc-maint topic, below the commit on ab/sha1dc that adds the > upstream as a submodule. > ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 0/3] update sha1dc from PR #36 2017-06-26 7:32 ` Compile Error v2.13.2 on Solaris SPARC Michael Kebe @ 2017-06-27 12:17 Ævar Arnfjörð Bjarmason 2017-06-26 7:32 ` Compile Error v2.13.2 on Solaris SPARC Michael Kebe 0 siblings, 1 reply; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-27 12:17 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Michael Kebe, Liam R . Howlett, Adam Dinwoodie, Stefan Beller, Ævar Arnfjörð Bjarmason This hopefully fixes the Solaris SPARC issue & doesn't cause regressions elsewhere, e.g. on Cygwin. Adam, it would be great if you could test that platform. I've already confirmed with Michael Kebe + another SPARC user (CosmicDJ on Freenode #Solaris) that it works on Solaris SPARC. The question is whether it breaks anything else. Per the upstream pull request: https://github.com/cr-marcstevens/sha1collisiondetection/pull/36 Marc would (understandably) like some wider testing of this before merging it into the upstream project. WRT the submodule URL & branch changing: I have no idea how git-submodule handles this in the general case, but it Just Works with GitHub because it allows fetching arbitrary SHA1s that any ref (including pull req refs) point to. So on top of pu just doing "git submodule update" works to update to the new copy. Junio C Hamano (1): sha1collisiondetection: automatically enable when submodule is populated Ævar Arnfjörð Bjarmason (2): sha1dc: update from my PR #36 sha1dc: optionally use sha1collisiondetection as a submodule .gitmodules | 4 +++ Makefile | 16 ++++++++++ hash.h | 4 +++ sha1collisiondetection | 1 + sha1dc/sha1.c | 80 ++++++++++++++++++++++++++++++++++++++------------ 5 files changed, 86 insertions(+), 19 deletions(-) create mode 100644 .gitmodules create mode 160000 sha1collisiondetection -- 2.13.1.611.g7e3b11ae1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Compile Error v2.13.2 on Solaris SPARC @ 2017-06-26 7:32 ` Michael Kebe 2017-06-27 12:17 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 29+ messages in thread From: Michael Kebe @ 2017-06-26 7:32 UTC (permalink / raw) To: Git Mailing List When compiling 2.13.2 on Solaris SPARC I get this error: CC sha1dc/sha1.o sha1dc/sha1.c:41:58: error: operator '==' has no right operand #if ((defined(_BYTE_ORDER) && (_BYTE_ORDER == _BIG_ENDIAN)) || \ ^ gmake: *** [sha1dc/sha1.o] Error 1 The define _BIG_ENDIAN is set by Solaris on SPARC systems. So the check in line 41 gives this error. The _BIG_ENDIAN define is used few line below for defining SHA1DC_BIGENDIAN. This is needed for Solaris SPARC systems. See https://github.com/cr-marcstevens/sha1collisiondetection/commit/33a694a9ee1b79c24be45f9eab5ac0e1aeeaf271 Greetings Michael ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-26 7:32 ` Compile Error v2.13.2 on Solaris SPARC Michael Kebe @ 2017-06-27 12:17 ` Ævar Arnfjörð Bjarmason 2017-06-27 18:46 ` Stefan Beller 0 siblings, 1 reply; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-27 12:17 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Michael Kebe, Liam R . Howlett, Adam Dinwoodie, Stefan Beller, Ævar Arnfjörð Bjarmason Add an option to use the sha1collisiondetection library from the submodule in sha1collisiondetection/ instead of in the copy in the sha1dc/ directory. This allows us to try out the submodule in sha1collisiondetection without breaking the build for anyone who's not expecting them as we work out any kinks. This uses my own fork which integrates PR #36. See the preceding commit ("sha1dc: update from my PR #36", 2017-06-27) for details. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- .gitmodules | 4 ++++ Makefile | 12 ++++++++++++ hash.h | 4 ++++ sha1collisiondetection | 1 + 4 files changed, 21 insertions(+) create mode 100644 .gitmodules create mode 160000 sha1collisiondetection diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000000..2fea9996e9 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,4 @@ +[submodule "sha1collisiondetection"] + path = sha1collisiondetection + url = https://github.com/avar/sha1collisiondetection.git + branch = bigend-detect-solaris-again diff --git a/Makefile b/Makefile index b94cd5633c..f0cac1f246 100644 --- a/Makefile +++ b/Makefile @@ -162,6 +162,12 @@ all:: # algorithm. This is slower, but may detect attempted collision attacks. # Takes priority over other *_SHA1 knobs. # +# Define DC_SHA1_SUBMODULE in addition to DC_SHA1 to use the +# sha1collisiondetection shipped as a submodule instead of the +# non-submodule copy in sha1dc/. This is an experimental option used +# by the git project to migrate to using sha1collisiondetection as a +# submodule. +# # Define OPENSSL_SHA1 environment variable when running make to link # with the SHA1 routine from openssl library. # @@ -1448,8 +1454,14 @@ ifdef APPLE_COMMON_CRYPTO BASIC_CFLAGS += -DSHA1_APPLE else DC_SHA1 := YesPlease +ifdef DC_SHA1_SUBMODULE + LIB_OBJS += sha1collisiondetection/lib/sha1.o + LIB_OBJS += sha1collisiondetection/lib/ubc_check.o + BASIC_CFLAGS += -DDC_SHA1_SUBMODULE +else LIB_OBJS += sha1dc/sha1.o LIB_OBJS += sha1dc/ubc_check.o +endif BASIC_CFLAGS += \ -DSHA1_DC \ -DSHA1DC_NO_STANDARD_INCLUDES \ diff --git a/hash.h b/hash.h index a11fc9233f..bef3e630a0 100644 --- a/hash.h +++ b/hash.h @@ -8,7 +8,11 @@ #elif defined(SHA1_OPENSSL) #include <openssl/sha.h> #elif defined(SHA1_DC) +#ifdef DC_SHA1_SUBMODULE +#include "sha1collisiondetection/lib/sha1.h" +#else #include "sha1dc/sha1.h" +#endif #else /* SHA1_BLK */ #include "block-sha1/sha1.h" #endif diff --git a/sha1collisiondetection b/sha1collisiondetection new file mode 160000 index 0000000000..56ab30c4c9 --- /dev/null +++ b/sha1collisiondetection @@ -0,0 +1 @@ +Subproject commit 56ab30c4c998e1e7f3075705087a2f0c4c4202d7 -- 2.13.1.611.g7e3b11ae1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-27 12:17 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason @ 2017-06-27 18:46 ` Stefan Beller 2017-06-27 18:56 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 29+ messages in thread From: Stefan Beller @ 2017-06-27 18:46 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git@vger.kernel.org, Junio C Hamano, Jeff King, Michael Kebe, Liam R . Howlett, Adam Dinwoodie On Tue, Jun 27, 2017 at 5:17 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > Add an option to use the sha1collisiondetection library from the > submodule in sha1collisiondetection/ instead of in the copy in the > sha1dc/ directory. > > This allows us to try out the submodule in sha1collisiondetection > without breaking the build for anyone who's not expecting them as we > work out any kinks. > > This uses my own fork which integrates PR #36. See the preceding > commit ("sha1dc: update from my PR #36", 2017-06-27) for details. > > +++ b/.gitmodules > @@ -0,0 +1,4 @@ > +[submodule "sha1collisiondetection"] > + path = sha1collisiondetection > + url = https://github.com/avar/sha1collisiondetection.git > + branch = bigend-detect-solaris-again What is the motivation for the branch field here? While this series fixes a hot issue, in the long run we'd rather want to plug in the original upstream with the master branch? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule 2017-06-27 18:46 ` Stefan Beller @ 2017-06-27 18:56 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 29+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2017-06-27 18:56 UTC (permalink / raw) To: Stefan Beller Cc: git@vger.kernel.org, Junio C Hamano, Jeff King, Michael Kebe, Liam R . Howlett, Adam Dinwoodie On Tue, Jun 27, 2017 at 8:46 PM, Stefan Beller <sbeller@google.com> wrote: > On Tue, Jun 27, 2017 at 5:17 AM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> Add an option to use the sha1collisiondetection library from the >> submodule in sha1collisiondetection/ instead of in the copy in the >> sha1dc/ directory. >> >> This allows us to try out the submodule in sha1collisiondetection >> without breaking the build for anyone who's not expecting them as we >> work out any kinks. >> >> This uses my own fork which integrates PR #36. See the preceding >> commit ("sha1dc: update from my PR #36", 2017-06-27) for details. >> > >> +++ b/.gitmodules >> @@ -0,0 +1,4 @@ >> +[submodule "sha1collisiondetection"] >> + path = sha1collisiondetection >> + url = https://github.com/avar/sha1collisiondetection.git >> + branch = bigend-detect-solaris-again > > What is the motivation for the branch field here? > While this series fixes a hot issue, in the long run we'd rather > want to plug in the original upstream with the master branch? Just so we can cook this in pu (including for those cloning with --recursive) until upstream merges my patches. We're now in a chicken & egg scenario where the patch I have (minus nits mentioned on list, will fix) should work, but needs testing, and once it has testing upstream is willing to merge it, and getting it into pu (with my clone/branch) will give it more testing). ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2017-07-03 12:34 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-05 20:34 Git v2.13.1 SHA1 very broken Adam Dinwoodie 2017-06-05 21:05 ` Ævar Arnfjörð Bjarmason 2017-06-05 23:20 ` Ramsay Jones 2017-06-06 0:11 ` Ramsay Jones 2017-06-06 1:20 ` Junio C Hamano 2017-06-06 10:03 ` Adam Dinwoodie 2017-06-06 11:55 ` Junio C Hamano 2017-06-06 12:43 ` Adam Dinwoodie 2017-06-06 14:47 ` Continous Integration (was: RE: Git v2.13.1 SHA1 very broken) Jason Pyeron 2017-06-06 15:04 ` Lars Schneider 2017-07-02 20:35 ` Adam Dinwoodie 2017-07-03 12:34 ` Johannes Schindelin 2017-06-06 15:12 ` [PATCH 0/3] update sha1dc Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 1/3] sha1dc: update from upstream Ævar Arnfjörð Bjarmason 2017-06-06 15:12 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason 2017-06-06 18:48 ` Stefan Beller 2017-06-06 19:03 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:09 ` Stefan Beller 2017-06-06 15:12 ` [PATCH 3/3] sha1collisiondetection: automatically enable when submodule is populated Ævar Arnfjörð Bjarmason 2017-06-06 18:23 ` [PATCH 0/3] update sha1dc Stefan Beller 2017-06-06 18:51 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:01 ` [PATCH] sha1dc: ignore indent-with-non-tab whitespace violations Jeff King 2017-06-06 19:04 ` Ævar Arnfjörð Bjarmason 2017-06-06 19:05 ` Stefan Beller 2017-06-13 2:09 ` [PATCH 0/3] update sha1dc Liam R. Howlett 2017-06-06 12:49 ` Git v2.13.1 SHA1 very broken Morten Welinder -- strict thread matches above, loose matches on Subject: below -- 2017-06-27 12:17 [PATCH 0/3] update sha1dc from PR #36 Ævar Arnfjörð Bjarmason 2017-06-26 7:32 ` Compile Error v2.13.2 on Solaris SPARC Michael Kebe 2017-06-27 12:17 ` [PATCH 2/3] sha1dc: optionally use sha1collisiondetection as a submodule Ævar Arnfjörð Bjarmason 2017-06-27 18:46 ` Stefan Beller 2017-06-27 18:56 ` Ævar Arnfjörð Bjarmason
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).