Opened 20 months ago

Closed 11 months ago

Last modified 11 months ago

#54602 closed defect (fixed)

git @2.14.1 generates corrupt SHA1 hashes on Tiger when built with gcc42, but works OK when built with gcc6

Reported by: kencu (Ken) Owned by: ci42
Priority: Normal Milestone:
Component: ports Version:
Keywords: tiger Cc: rickyzhang82 (Ricky Zhang), ryandesign (Ryan Schmidt)
Port: git

Description

This is a somewhat odd one. It's a corner case to be sure, but I thought I'd note it here as others are likely to run into it as well, I think.

Came across this after the recent upgrade of git, which builds without errors with gcc-4.2 by default on Tiger. On Tiger, git was unable to add or commit files after the upgrade to 2.14.1; also unable to clone repositories, with errors such as:

$ sudo git clone https://github.com/kencu/TigerPorts.git
Cloning into 'TigerPorts'...
remote: Counting objects: 475, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 475 (delta 0), reused 9 (delta 0), pack-reused 465
Receiving objects: 100% (475/475), 206.52 KiB | 538.00 KiB/s, done.
fatal: pack is corrupted (SHA1 mismatch)
fatal: index-pack failed

I tried rebuilding it again using the default gcc-4.2, but ran into the same errors.

Rebuilding git 2.14.1 using gcc6 however generates a normally-working git:

$ sudo git clone https://github.com/kencu/TigerPorts.git
Cloning into 'TigerPorts'...
remote: Counting objects: 475, done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 475 (delta 0), reused 9 (delta 0), pack-reused 465
Receiving objects: 100% (475/475), 206.52 KiB | 599.00 KiB/s, done.
Resolving deltas: 100% (112/112), done.

Just to make it nice and confusing, the same version of git works perfectly well on Leopard PPC when built with gcc-4.2.

Attachments (2)

git.build.gcc42.txt (523.3 KB) - added by kencu (Ken) 19 months ago.
git.build.gcc6.txt (519.6 KB) - added by kencu (Ken) 19 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 20 months ago by ryandesign (Ryan Schmidt)

Cc: ci42 removed
Owner: set to ci42
Status: newassigned

Gosh, I have no idea where to begin with this, but it sounds like it's not a MacPorts-specific problem. Please report it to the developers of git.

You should be more specific about the compilers, however. When you say "gcc-4.2 ... on Tiger", I assume you mean the Apple version of gcc 4.2 installed by the apple-gcc42 port? And when you talk about gcc-4.2 on Leopard, I assume you mean the Apple version of gcc 4.2 installed by Xcode 3.1.4?

comment:2 Changed 19 months ago by kencu (Ken)

Will do. Your assumption about the compilers is correct. I'll also try it on a different Tiger system I have and see what happens there.

comment:3 Changed 19 months ago by kencu (Ken)

I haven't dug in on this (because it worked with gcc6), but it is worthy of an upstream report now that others are coming across it as well.

comment:4 Changed 19 months ago by mf2k (Frank Schima)

Cc: rickyzhang82 added

Has duplicate #54786.

comment:5 Changed 19 months ago by kencu (Ken)

I can't see much in the way of configuration differences, building with apple-gcc-4.2 vs macports-gcc-6. I'll attach the full build logs for each.

/opt/local/bin/gcc-apple-4.2 -o common-main.o -c -MF ./.depend/common-main.o.d -MQ common-main.o -MMD -MP  -I. -Wall -O2 -I/opt/local/include -arch ppc -I. -DPRECOMPOSE_UNICODE -DPROTECT_HFS_DEFAULT=1 -DUSE_LIBPCRE2 -I/opt/local/include -I/opt/local/include -DUSE_CURL_FOR_IMAP_SEND -I/opt/local/include -I/opt/local/include -DUSE_ST_TIMESPEC -DOLD_ICONV -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""  -DHAVE_DEV_TTY -DHAVE_BSD_SYSCTL  -DFREAD_READS_DIRECTORIES -DNO_MEMMEM -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"'  common-main.c

/opt/local/bin/gcc-mp-6      -o common-main.o -c -MF ./.depend/common-main.o.d -MQ common-main.o -MMD -MP  -I. -Wall -O2 -I/opt/local/include -m32      -I. -DPRECOMPOSE_UNICODE -DPROTECT_HFS_DEFAULT=1 -DUSE_LIBPCRE2 -I/opt/local/include -I/opt/local/include -DUSE_CURL_FOR_IMAP_SEND -I/opt/local/include -I/opt/local/include -DUSE_ST_TIMESPEC -DOLD_ICONV -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_C="\"sha1dc_git.c\"" -DSHA1DC_CUSTOM_TRAILING_INCLUDE_SHA1_H="\"sha1dc_git.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\""  -DHAVE_DEV_TTY -DHAVE_BSD_SYSCTL  -DFREAD_READS_DIRECTORIES -DNO_MEMMEM -Icompat/regex -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"'  common-main.c

Changed 19 months ago by kencu (Ken)

Attachment: git.build.gcc42.txt added

Changed 19 months ago by kencu (Ken)

Attachment: git.build.gcc6.txt added

comment:6 Changed 11 months ago by ryandesign (Ryan Schmidt)

Cc: ryandesign added

Did you file an upstream bug report? If so can you share the URL?

For some additional datapoints, on Tiger PPC, I rebuilt git 2.17.0 with Xcode gcc-4.0, which has the same problem. And on Leopard i386, I rebuilt git with MacPorts apple-gcc-4.2, which did not exhibit the problem.

comment:7 Changed 11 months ago by kencu (Ken)

You know, I admit I never did file a report. Such an old system, odd error with ancient compiler, easily fixed by using a newer compiler -- I thought they'd laugh me out of the git world...

comment:8 Changed 11 months ago by ryandesign (Ryan Schmidt)

Requiring a newer compiler for git makes bootstrapping an installation of git that much more difficult, or at least time-consuming. And the problem isn't just the old compiler, since it seems like the same old compiler works fine on Leopard. If upstream doesn't want to fix it, fine; we can accept the compiler requirement. But I think it's only fair for us to give the developers an opportunity to fix it.

comment:9 Changed 11 months ago by kencu (Ken)

I had time to look at this today, in the "what else do I have to do?" category.

Git introduced a new SHA1 calculating scheme with this commit and this made it into git 13.1. I noticed git 12.x worked properly and git 14.x did not, so that fits with the timing. This only applies to systems that do not use Apple Common Crypto (i.e. < 10.5) so not noted on Leopard PPC. There were issues with this commit noted on several systems.

This was reworked, and then reworked again. Looking this over, an error in the endianess of this seemed to be the most likely thing that has gone wrong on PowerPC Tiger. Sure enough, forcing the endianess by adding the following to the CFLAGS:

-DSHA1DC_FORCE_BIGENDIAN=1

fixes git on Tiger PPC when built with stock compilers. I suppose gcc6 must define one of the tests that git is using to decide on BIGENDIAN, whereas gcc-4.x does not define it.

It's not so easy to add a cppflag to the git build, as it doesn't use configure, and it hard codes in all the flags directly. One way to do it is to put this block below the current CFLAGS definintion

platform darwin 8 powerpc {
    set CFLAGS "${CFLAGS} -DSHA1DC_FORCE_BIGENDIAN=1"
}

better, of course, would be to have the git-ters fix it properly, if they want to.

comment:10 Changed 11 months ago by kencu (Ken)

It looks like there is another environment variable that could be used when building git, PPC_SHA1, that might turn out to be simpler to implement.

Edit: It is simpler to implement, but pulls in a source file written in assembly that gcc-4.2 barfs on, probably because it's 64bit assembly.

Last edited 11 months ago by kencu (Ken) (previous) (diff)

comment:11 Changed 11 months ago by ryandesign (Ryan Schmidt)

Thanks for looking into it.

Forcing endianness would mess up the universal variant. Probably simplest to disable the universal variant on darwin 8 then.

Did you file an upstream bug report?

comment:12 Changed 11 months ago by kencu (Ken)

There is one more possibility to consider -- perhaps the SHA1 routine in OPENSSL might work:

# Define OPENSSL_SHA1 environment variable when running make to link
# with the SHA1 routine from openssl library.

I'll do a bug report as soon as I figure it out fully. Still not sure why gcc6 works. Like to get that nailed.

comment:13 Changed 11 months ago by kencu (Ken)

comment:14 Changed 11 months ago by ken-cunningham-webuse

Resolution: fixed
Status: assignedclosed

In 4dbe3e0a8deed4fc0dbc54013702c54bda3c69f9/macports-ports (master):

git: fix SHA1 calculation for older compilers

fixes stock Apple compilers on older PPC machines
by correcting endianness checks
closes: #54602

comment:15 Changed 11 months ago by ryandesign (Ryan Schmidt)

Are you certain that fix is correct for Intel Macs?

comment:16 Changed 11 months ago by ryandesign (Ryan Schmidt)

After looking at it further, it looks fine to me. I was just misled by your comment:

/* older gcc compilers which are the default on Apple PPC do not define __BYTE_ORDER__ */

Those older compilers are the default on older systems regardless of architecture, but I understand now that the code defaults to little-endian, so the problem only occurs on big-endian machines.

Note: See TracTickets for help on using tickets.