Re: [patch 00/16] Portability Patches for git-1.7.1 (v4)

Michael J Gruber [off-list ref] writes:

Michael J Gruber venit, vidit, dixit 27.04.2010 17:30:

quoted

Gary V. Vaughan venit, vidit, dixit 27.04.2010 15:57:

quoted

Here are the portability patches we needed at TWW to enable git-1.7.1
to compile and run on all of the wide range of Unix machines we
support.  These patches apply to the git-1.7.1 release,  and address
all of the feedback from the previous three times I posted them to
this list, including fixing the massive testsuite failures I was
experiencing and taking into account that the ss_family fixes and
partial GMT_CMP_TEST fixes that have been pushed since my last post of
this patch queue.

General remark: None of your patches have a s-o-b line. If you want to
have your patches in git you are required to sign-off on them (commit
-s) in order to certify that you can submit them under the license terms
of the project.

Your diff -> test_cmp are certainly something we want to have in any
case. The code changes look ugly, honestly, making the code much less
readable, but it seems to be the only way to make those older platforms
and compilers happy. (Erik pointed out some good ways to reduce the
uglyness somewhat.)

I can't test your target platform, but I would test the impact of the
code and test changes on mine. Do you have your series somewhere to pull
from?

OK, unsurprsingly, tests still pass on Linux (Fedora 12 x86_64).
If anyone wants to try, the series can be found at

git://repo.or.cz/git/mjg.git

in branch

gvv/platform-compatibility

applied cleanly on current master.

Thanks.  

Like everybody else, I like the s/diff/test_cmp/ one in general.  On
platforms without "diff -u", test_cmp may want to use "diff -c" instead
for readability, but that can be fixed/enhanced independently.

I agree with your general remark and also noticed that the titles are way
suboptimal.

* user-cppflags.patch

  Makefile: pass CPPFLAGS through to fllow customization

* const-expr.patch

  Rewrite dynamic structure initializations to runtime assignment

* pthread.patch

  Makefile: -lpthread may still be necessary when libc has only pthread stubs

* Without this patch at least IBM VisualAge C 5.0 (I have 5.0.2) on AIX 5.1 fails to compile git.

  enums: omit trailing comma for portability

* diff-export.patch

  Do not use "diff" found on PATH while building and installing

I think the change to git-merge-one-file.sh in this patch is wrong, by the
way.

* diff-test_cmp.patch

  tests: use "test_cmp", not "diff", when verifying the result

The patch to t9400 has indent-with-spaces, by the way.

* diff-defaults.patch

  test_cmp: do not use "diff -u" on platforms that lack one

It may be better to use "diff -c" on most of them, though.

* host-SunOS56.patch

  Makefile: SunOS 5.6 portability fix

* host-IRIX.patch

  git-compat-util.h: Irix 6.5 defines 'sgi' but not '__sgi'.

* host-HPUX10.patch

This would be better as two patches

  Makefile: HP-UX 10.20 lacks pread()
  git-compat-util.h: some platforms with mmap() lack MAP_FAILED definition

* host-HPUX11.patch

  Makefile: HPUX does not have nanosecond timestamp in struct stat

* host-OSF1.patch

  Makefile: Tru64 portability fix

* no-hstrerror.patch

I think this should come before "Makefile: SunOS 5.6 portability fix"
(split the change to Makefile from this one and move it to the other
patch).  Then this patch does not talk about SunOS specific issues.

  Makefile: some platforms do not have hstrerror anywhere

* no-inet_ntop.patch

It might make sense to squash this patch into the previous one (and again
do this before HPUX patches to Makefile) that deals with three functions
that are traditionally related to libresolv (hstrerror, inet_ntop/pton).

  Make NO_{HSTRERROR,INET_NTOP,INET_PTON} configured independently

* no-socklen_t.patch

Do this before the platform dependent bits, i.e. move the hunks that
changes "ifeq ($(uname_S,XXX)" block from this patch to host-XXX patch,
and do the remainder of this patch before any of the platform ones.

  Some platforms lack socklen_t type

* no-inline.patch

Do this before ... (ditto) ...

  Allow disabling "inline"

Hi Michael,

Thanks for putting up a public repo of those patches; much
appreciated.

On Sat, May 01, 2010 at 04:25:12PM -0700, Junio C Hamano wrote:

Michael J Gruber [off-list ref] writes:

quoted

Gary V. Vaughan venit, vidit, dixit 27.04.2010 15:57:

quoted

Here are the portability patches we needed at TWW to enable git-1.7.1
to compile and run on all of the wide range of Unix machines we
support. [[...]]

OK, unsurprsingly, tests still pass on Linux (Fedora 12 x86_64).
If anyone wants to try, the series can be found at

git://repo.or.cz/git/mjg.git

in branch

gvv/platform-compatibility

applied cleanly on current master.

Thanks.  

Hi Junio,

Like everybody else, I like the s/diff/test_cmp/ one in general.  On
platforms without "diff -u", test_cmp may want to use "diff -c" instead
for readability, but that can be fixed/enhanced independently.

I agree with your general remark and also noticed that the titles are way
suboptimal.

I think this is my first patch series submission to the git list, but
in any case I just used used the patch file-names as is my habit for
locally maintained patchsets.

Thanks for suggesting better title, which I've already adopted into my
local tree.  If upstream integration stalls, or if it will help smooth
integration, I'll make the merging and reordering adjustments you
request and resubmit the whole queue.

Cheers,
-- 
Gary V. Vaughan (gary@thewrittenword.com)