* [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2018-12-20 16:24 ` [PATCH 0/5] travis-ci: build with the right compiler SZEDER Gábor
@ 2018-12-20 16:24 ` SZEDER Gábor
2018-12-20 23:12 ` Ævar Arnfjörð Bjarmason
2018-12-20 16:24 ` [PATCH 2/5] .gitignore: ignore external debug symbols from GCC on macOS SZEDER Gábor
` (4 subsequent siblings)
5 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2018-12-20 16:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin, git, SZEDER Gábor
When building Git with GCC 8.2.0 (at least from Homebrew on macOS,
DEVELOPER flags enabled) one is greeted with a screenful of compiler
errors:
compat/obstack.c: In function '_obstack_begin':
compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type]
h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
^
compat/obstack.c:163:16: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(void *, struct _obstack_chunk *)' [-Werror=cast-function-type]
h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
^
compat/obstack.c:116:8: error: cast between incompatible function types from 'struct _obstack_chunk * (*)(void *, long int)' to 'struct _obstack_chunk * (*)(long int)' [-Werror=cast-function-type]
: (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
^
compat/obstack.c:168:22: note: in expansion of macro 'CALL_CHUNKFUN'
chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
^~~~~~~~~~~~~
<snip>
'struct obstack' stores pointers to two functions to allocate and free
"chunks", and depending on how obstack is used, these functions take
either one parameter (like standard malloc() and free() do; this is
how we use it) or two parameters. Presumably to reduce memory
footprint, a single field is used to store the function pointer for
both signatures, and then it's casted to the appropriate signature
when the function pointer is accessed. These casts between function
pointers with different number of parameters are what trigger those
compiler errors.
Modify 'struct obstack' to use unions to store function pointers with
different signatures, and then use the union member with the
appropriate signature when accessing these function pointers. This
eliminates the need for those casts, and thus avoids this compiler
error.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
compat/obstack.c | 17 +++++++++--------
compat/obstack.h | 18 +++++++++++-------
2 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/compat/obstack.c b/compat/obstack.c
index 4d1d95beeb..dd9504c684 100644
--- a/compat/obstack.c
+++ b/compat/obstack.c
@@ -112,15 +112,15 @@ compat_symbol (libc, _obstack_compat, _obstack, GLIBC_2_0);
# define CALL_CHUNKFUN(h, size) \
(((h) -> use_extra_arg) \
- ? (*(h)->chunkfun) ((h)->extra_arg, (size)) \
- : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
+ ? (*(h)->chunkfun.fn_extra_arg) ((h)->extra_arg, (size)) \
+ : (*(h)->chunkfun.fn) ((size)))
# define CALL_FREEFUN(h, old_chunk) \
do { \
if ((h) -> use_extra_arg) \
- (*(h)->freefun) ((h)->extra_arg, (old_chunk)); \
+ (*(h)->freefun.fn_extra_arg) ((h)->extra_arg, (old_chunk)); \
else \
- (*(void (*) (void *)) (h)->freefun) ((old_chunk)); \
+ (*(h)->freefun.fn) ((old_chunk)); \
} while (0)
\f
@@ -159,8 +159,8 @@ _obstack_begin (struct obstack *h,
size = 4096 - extra;
}
- h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
- h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
+ h->chunkfun.fn = chunkfun;
+ h->freefun.fn = freefun;
h->chunk_size = size;
h->alignment_mask = alignment - 1;
h->use_extra_arg = 0;
@@ -206,8 +206,9 @@ _obstack_begin_1 (struct obstack *h, int size, int alignment,
size = 4096 - extra;
}
- h->chunkfun = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
- h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
+ h->chunkfun.fn_extra_arg = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
+ h->freefun.fn_extra_arg = (void (*) (void *, struct _obstack_chunk *)) freefun;
+
h->chunk_size = size;
h->alignment_mask = alignment - 1;
h->extra_arg = arg;
diff --git a/compat/obstack.h b/compat/obstack.h
index 6bc24b7644..0f9b2232a9 100644
--- a/compat/obstack.h
+++ b/compat/obstack.h
@@ -160,11 +160,15 @@ struct obstack /* control current object in current chunk */
void *tempptr;
} temp; /* Temporary for some macros. */
int alignment_mask; /* Mask of alignment for each object. */
- /* These prototypes vary based on `use_extra_arg', and we use
- casts to the prototypeless function type in all assignments,
- but having prototypes here quiets -Wstrict-prototypes. */
- struct _obstack_chunk *(*chunkfun) (void *, long);
- void (*freefun) (void *, struct _obstack_chunk *);
+ /* These prototypes vary based on `use_extra_arg'. */
+ union {
+ struct _obstack_chunk *(*fn_extra_arg) (void *, long);
+ void *(*fn) (long);
+ } chunkfun;
+ union {
+ void (*fn_extra_arg) (void *, struct _obstack_chunk *);
+ void (*fn) (void *);
+ } freefun;
void *extra_arg; /* first arg for chunk alloc/dealloc funcs */
unsigned use_extra_arg:1; /* chunk alloc/dealloc funcs take extra arg */
unsigned maybe_empty_object:1;/* There is a possibility that the current
@@ -235,10 +239,10 @@ extern void (*obstack_alloc_failed_handler) (void);
(void (*) (void *, void *)) (freefun), (arg))
#define obstack_chunkfun(h, newchunkfun) \
- ((h) -> chunkfun = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
+ ((h)->chunkfun.fn_extra_arg = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
#define obstack_freefun(h, newfreefun) \
- ((h) -> freefun = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
+ ((h)->freefun.fn_extra_arg = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
#define obstack_1grow_fast(h,achar) (*((h)->next_free)++ = (achar))
--
2.20.1.151.gec613c4b75
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2018-12-20 16:24 ` [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings SZEDER Gábor
@ 2018-12-20 23:12 ` Ævar Arnfjörð Bjarmason
2019-01-10 21:22 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2018-12-20 23:12 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Junio C Hamano, Jonathan Nieder, Johannes Schindelin, git
On Thu, Dec 20 2018, SZEDER Gábor wrote:
> When building Git with GCC 8.2.0 (at least from Homebrew on macOS,
> DEVELOPER flags enabled) one is greeted with a screenful of compiler
> errors:
>
> compat/obstack.c: In function '_obstack_begin':
> compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type]
> h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
> ^
> compat/obstack.c:163:16: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(void *, struct _obstack_chunk *)' [-Werror=cast-function-type]
> h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
> ^
> compat/obstack.c:116:8: error: cast between incompatible function types from 'struct _obstack_chunk * (*)(void *, long int)' to 'struct _obstack_chunk * (*)(long int)' [-Werror=cast-function-type]
> : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
> ^
> compat/obstack.c:168:22: note: in expansion of macro 'CALL_CHUNKFUN'
> chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
> ^~~~~~~~~~~~~
> <snip>
We originally got this from now-discontinued eglibc, but I notice that
glibc.git's malloc/obstack.[ch]'s diff also changes these lines. If you
backport those do does that fix this warning?
I.e. is this another case where we're blindly fixing bugs but should
just re-import upstream's code instead?
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2018-12-20 23:12 ` Ævar Arnfjörð Bjarmason
@ 2019-01-10 21:22 ` Junio C Hamano
2019-01-11 0:37 ` SZEDER Gábor
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2019-01-10 21:22 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: SZEDER Gábor, Jonathan Nieder, Johannes Schindelin, git
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On Thu, Dec 20 2018, SZEDER Gábor wrote:
>
>> When building Git with GCC 8.2.0 (at least from Homebrew on macOS,
>> DEVELOPER flags enabled) one is greeted with a screenful of compiler
>> errors:
>>
>> compat/obstack.c: In function '_obstack_begin':
>> compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type]
>> h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
>> ^
>> compat/obstack.c:163:16: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(void *, struct _obstack_chunk *)' [-Werror=cast-function-type]
>> h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
>> ^
>> compat/obstack.c:116:8: error: cast between incompatible function types from 'struct _obstack_chunk * (*)(void *, long int)' to 'struct _obstack_chunk * (*)(long int)' [-Werror=cast-function-type]
>> : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
>> ^
>> compat/obstack.c:168:22: note: in expansion of macro 'CALL_CHUNKFUN'
>> chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
>> ^~~~~~~~~~~~~
>> <snip>
>
> We originally got this from now-discontinued eglibc, but I notice that
> glibc.git's malloc/obstack.[ch]'s diff also changes these lines. If you
> backport those do does that fix this warning?
>
> I.e. is this another case where we're blindly fixing bugs but should
> just re-import upstream's code instead?
Good point. I am inclined to queue the remainder of the series
without this one for now.
Thanks.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-10 21:22 ` Junio C Hamano
@ 2019-01-11 0:37 ` SZEDER Gábor
2019-01-11 18:03 ` Junio C Hamano
0 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-11 0:37 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Johannes Schindelin, git
On Thu, Jan 10, 2019 at 01:22:00PM -0800, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
> > On Thu, Dec 20 2018, SZEDER Gábor wrote:
> >
> >> When building Git with GCC 8.2.0 (at least from Homebrew on macOS,
> >> DEVELOPER flags enabled) one is greeted with a screenful of compiler
> >> errors:
> >>
> >> compat/obstack.c: In function '_obstack_begin':
> >> compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type]
> >> h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
> >> ^
> >> compat/obstack.c:163:16: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(void *, struct _obstack_chunk *)' [-Werror=cast-function-type]
> >> h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
> >> ^
> >> compat/obstack.c:116:8: error: cast between incompatible function types from 'struct _obstack_chunk * (*)(void *, long int)' to 'struct _obstack_chunk * (*)(long int)' [-Werror=cast-function-type]
> >> : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
> >> ^
> >> compat/obstack.c:168:22: note: in expansion of macro 'CALL_CHUNKFUN'
> >> chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
> >> ^~~~~~~~~~~~~
> >> <snip>
> >
> > We originally got this from now-discontinued eglibc, but I notice that
> > glibc.git's malloc/obstack.[ch]'s diff also changes these lines. If you
> > backport those do does that fix this warning?
> >
> > I.e. is this another case where we're blindly fixing bugs but should
> > just re-import upstream's code instead?
>
> Good point. I am inclined to queue the remainder of the series
> without this one for now.
Note that without this first patch the linux-gcc build job will fail
with the above compiler error, and that's the only build job that runs
the test suite with all the misc test knobs (split-index,
commit-graph, etc.) enabled.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-11 0:37 ` SZEDER Gábor
@ 2019-01-11 18:03 ` Junio C Hamano
2019-01-11 18:51 ` SZEDER Gábor
0 siblings, 1 reply; 33+ messages in thread
From: Junio C Hamano @ 2019-01-11 18:03 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Johannes Schindelin, git
SZEDER Gábor <szeder.dev@gmail.com> writes:
> On Thu, Jan 10, 2019 at 01:22:00PM -0800, Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> ...
>> > I.e. is this another case where we're blindly fixing bugs but should
>> > just re-import upstream's code instead?
>>
>> Good point. I am inclined to queue the remainder of the series
>> without this one for now.
>
> Note that without this first patch the linux-gcc build job will fail
> with the above compiler error, and that's the only build job that runs
> the test suite with all the misc test knobs (split-index,
> commit-graph, etc.) enabled.
I know.
The point is to give more incentive to try what was suggested
earlier by Ævar (in short, "try to do the right thing, before
hacking around locally in this project" ;-)
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-11 18:03 ` Junio C Hamano
@ 2019-01-11 18:51 ` SZEDER Gábor
2019-01-15 23:55 ` SZEDER Gábor
0 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-11 18:51 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Johannes Schindelin, git
On Fri, Jan 11, 2019 at 10:03:01AM -0800, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
> > On Thu, Jan 10, 2019 at 01:22:00PM -0800, Junio C Hamano wrote:
> >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> >> ...
> >> > I.e. is this another case where we're blindly fixing bugs but should
> >> > just re-import upstream's code instead?
> >>
> >> Good point. I am inclined to queue the remainder of the series
> >> without this one for now.
> >
> > Note that without this first patch the linux-gcc build job will fail
> > with the above compiler error, and that's the only build job that runs
> > the test suite with all the misc test knobs (split-index,
> > commit-graph, etc.) enabled.
>
> I know.
>
> The point is to give more incentive to try what was suggested
> earlier by Ævar (in short, "try to do the right thing, before
> hacking around locally in this project" ;-)
Well, first I'm not sure what changes Ævar meant to be backported.
Back then I briefly glanced at glibc's gitweb [1], but didn't see
anything remotely relevant to these compiler errors.
As to re-importing obstack.{c,h} from upstream, we've made some
portability fixes to these files, and neither of the commit messages
of those fixes mention that they are backports from upstream. OTOH,
one of those commits mentions platforms like
"i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1, SunOS 5.10", which makes
me suspect that the re-import will be susceptible to those portability
issues again. Therefore, I think re-importing these files from
upstream is beyond the scope of this patch series (and might not be
the right thing at all).
[1] https://sourceware.org/git/?p=glibc.git;a=history;f=malloc/obstack.c;h=1669641983512d64f66c1ad659562f77ef48adfd;hb=refs/heads/master
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-11 18:51 ` SZEDER Gábor
@ 2019-01-15 23:55 ` SZEDER Gábor
2019-01-16 1:13 ` Jonathan Nieder
2019-01-16 4:16 ` Junio C Hamano
0 siblings, 2 replies; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-15 23:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Johannes Schindelin, git
On Fri, Jan 11, 2019 at 07:51:18PM +0100, SZEDER Gábor wrote:
> On Fri, Jan 11, 2019 at 10:03:01AM -0800, Junio C Hamano wrote:
> > SZEDER Gábor <szeder.dev@gmail.com> writes:
> >
> > > On Thu, Jan 10, 2019 at 01:22:00PM -0800, Junio C Hamano wrote:
> > >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> > >> ...
> > >> > I.e. is this another case where we're blindly fixing bugs but should
> > >> > just re-import upstream's code instead?
> > >>
> > >> Good point. I am inclined to queue the remainder of the series
> > >> without this one for now.
> > >
> > > Note that without this first patch the linux-gcc build job will fail
> > > with the above compiler error, and that's the only build job that runs
> > > the test suite with all the misc test knobs (split-index,
> > > commit-graph, etc.) enabled.
> >
> > I know.
> >
> > The point is to give more incentive to try what was suggested
> > earlier by Ævar (in short, "try to do the right thing, before
> > hacking around locally in this project" ;-)
>
> Well, first I'm not sure what changes Ævar meant to be backported.
> Back then I briefly glanced at glibc's gitweb [1], but didn't see
> anything remotely relevant to these compiler errors.
So, I looked at the gnulib repository, where glibc got it's
obstack.{c,h} from, and it does have a fix for this issue in commit
127ed6a3e (obstack: avoid potentially-nonportable function casts,
2014-11-04):
http://git.savannah.gnu.org/cgit/gnulib.git/commit?id=127ed6a3ea9c46452f079dee50382dc1f70ea796
It chose basically the same approach as my fix, i.e. storing pointers
to functions with different signatures in an union. However,, the
differences between our and their obstack.{c,h} are way too big to
backport their patch.
> As to re-importing obstack.{c,h} from upstream, we've made some
> portability fixes to these files, and neither of the commit messages
> of those fixes mention that they are backports from upstream. OTOH,
> one of those commits mentions platforms like
> "i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1, SunOS 5.10", which makes
> me suspect that the re-import will be susceptible to those portability
> issues again. Therefore, I think re-importing these files from
> upstream is beyond the scope of this patch series (and might not be
> the right thing at all).
gnulib's obstack.{c,h} doesn't fix the issues that we've fixed in
3254310863 (obstack.c: Fix some sparse warnings, 2011-09-11) and
d190a0875f (obstack: Fix portability issues, 2011-08-28). So if we
were to re-import from gnulib, then these two patches would have to be
applied on top yet again.
> [1] https://sourceware.org/git/?p=glibc.git;a=history;f=malloc/obstack.c;h=1669641983512d64f66c1ad659562f77ef48adfd;hb=refs/heads/master
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-15 23:55 ` SZEDER Gábor
@ 2019-01-16 1:13 ` Jonathan Nieder
2019-01-17 1:36 ` SZEDER Gábor
2019-01-16 4:16 ` Junio C Hamano
1 sibling, 1 reply; 33+ messages in thread
From: Jonathan Nieder @ 2019-01-16 1:13 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
Johannes Schindelin, git
Hi,
SZEDER Gábor wrote:
> On Fri, Jan 11, 2019 at 07:51:18PM +0100, SZEDER Gábor wrote:
>> As to re-importing obstack.{c,h} from upstream, we've made some
>> portability fixes to these files, and neither of the commit messages
>> of those fixes mention that they are backports from upstream. OTOH,
>> one of those commits mentions platforms like
>> "i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1, SunOS 5.10", which makes
>> me suspect that the re-import will be susceptible to those portability
>> issues again. Therefore, I think re-importing these files from
>> upstream is beyond the scope of this patch series (and might not be
>> the right thing at all).
>
> gnulib's obstack.{c,h} doesn't fix the issues that we've fixed in
> 3254310863 (obstack.c: Fix some sparse warnings, 2011-09-11) and
> d190a0875f (obstack: Fix portability issues, 2011-08-28). So if we
> were to re-import from gnulib, then these two patches would have to be
> applied on top yet again.
Thanks for looking into it. The former looks applicable to upstream,
while the latter appears to do some Git-specific things (e.g. relying
on git-compat-util.h).
Mind if I send the former upstream? I believe gnulib upstream relies
on copyright assignment, so it would help if you have a copyright
assignment for the project on file, but if not, they may consider it a
small enough change to take without.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-16 1:13 ` Jonathan Nieder
@ 2019-01-17 1:36 ` SZEDER Gábor
0 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:36 UTC (permalink / raw)
To: Jonathan Nieder, Ramsay Jones
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason,
Johannes Schindelin, git
On Tue, Jan 15, 2019 at 05:13:11PM -0800, Jonathan Nieder wrote:
> Hi,
>
> SZEDER Gábor wrote:
> > gnulib's obstack.{c,h} doesn't fix the issues that we've fixed in
> > 3254310863 (obstack.c: Fix some sparse warnings, 2011-09-11) and
> > d190a0875f (obstack: Fix portability issues, 2011-08-28). So if we
> > were to re-import from gnulib, then these two patches would have to be
> > applied on top yet again.
>
> Thanks for looking into it. The former looks applicable to upstream,
> while the latter appears to do some Git-specific things (e.g. relying
> on git-compat-util.h).
>
> Mind if I send the former upstream?
That's not my patch, so it doesn't matter whether I mind :) Cc-ing
Ramsay, to make sure.
> I believe gnulib upstream relies
> on copyright assignment, so it would help if you have a copyright
> assignment for the project on file, but if not, they may consider it a
> small enough change to take without.
>
> Thanks,
> Jonathan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-15 23:55 ` SZEDER Gábor
2019-01-16 1:13 ` Jonathan Nieder
@ 2019-01-16 4:16 ` Junio C Hamano
1 sibling, 0 replies; 33+ messages in thread
From: Junio C Hamano @ 2019-01-16 4:16 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder,
Johannes Schindelin, git
SZEDER Gábor <szeder.dev@gmail.com> writes:
>> Well, first I'm not sure what changes Ævar meant to be backported.
>> Back then I briefly glanced at glibc's gitweb [1], but didn't see
>> anything remotely relevant to these compiler errors.
>
> So, I looked at the gnulib repository, where glibc got it's
> obstack.{c,h} from, and it does have a fix for this issue in commit
> 127ed6a3e (obstack: avoid potentially-nonportable function casts,
> 2014-11-04):
>
> http://git.savannah.gnu.org/cgit/gnulib.git/commit?id=127ed6a3ea9c46452f079dee50382dc1f70ea796
>
> It chose basically the same approach as my fix, i.e. storing pointers
> to functions with different signatures in an union. However,, the
> differences between our and their obstack.{c,h} are way too big to
> backport their patch.
Thanks. In the meantime, I've queued your fix in my tree, and with
your digging, we can say we did due diligence and apply a local fix
with conscience ;-).
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/5] .gitignore: ignore external debug symbols from GCC on macOS
2018-12-20 16:24 ` [PATCH 0/5] travis-ci: build with the right compiler SZEDER Gábor
2018-12-20 16:24 ` [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings SZEDER Gábor
@ 2018-12-20 16:24 ` SZEDER Gábor
2018-12-20 16:24 ` [PATCH 3/5] travis-ci: don't be '--quiet' when running the tests SZEDER Gábor
` (3 subsequent siblings)
5 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2018-12-20 16:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin, git, SZEDER Gábor
When Git is build with a "real" GCC on macOS [1], or at least with GCC
installed via Homebrew, and CFLAGS includes the '-g' option (and our
default CFLAGS does), then by default GCC writes the debug symbols
into external files under '<binary>.dSYM/' directories (e.g.
'git-daemon.dSYM/', 'git.dSYM/', etc.).
Update '.gitignore' to ignore these directories, so they don't clutter
the output of 'git status'. Furthermore, these build artifacts then
won't trigger build failures on Travis CI via b92cb86ea1 (travis-ci:
check that all build artifacts are .gitignore-d, 2017-12-31) once one
of the following patches updates our CI build scripts to use a real
GCC in the 'osx-gcc' build job.
[1] On macOS the default '/usr/bin/gcc' executable is not a real GCC,
but merely a compatibility wrapper around Clang:
$ gcc --version
Configured with: --prefix=<...>
Apple LLVM version 9.0.0 (clang-900.0.39.2)
<...>
So even though 'make CC=gcc' does indeed execute a command called
'gcc', in the end Git will be built with Clang all the same.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index 0d77ea5894..a9db568712 100644
--- a/.gitignore
+++ b/.gitignore
@@ -229,3 +229,4 @@
*.pdb
/Debug/
/Release/
+*.dSYM
--
2.20.1.151.gec613c4b75
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 3/5] travis-ci: don't be '--quiet' when running the tests
2018-12-20 16:24 ` [PATCH 0/5] travis-ci: build with the right compiler SZEDER Gábor
2018-12-20 16:24 ` [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings SZEDER Gábor
2018-12-20 16:24 ` [PATCH 2/5] .gitignore: ignore external debug symbols from GCC on macOS SZEDER Gábor
@ 2018-12-20 16:24 ` SZEDER Gábor
2018-12-20 16:24 ` [PATCH 4/5] travis-ci: switch to Xcode 10.1 macOS image SZEDER Gábor
` (2 subsequent siblings)
5 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2018-12-20 16:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin, git, SZEDER Gábor
All Travis CI build jobs run the test suite with 'make --quiet test'.
On one hand, being quiet doesn't save us from much clutter in the
output:
$ make test |wc -l
861
$ make --quiet test |wc -l
848
It only spares 13 lines, mostly the output of entering the 't/'
directory and the pre- and post-cleanup commands, which is negligible
compared to the ~700 lines printed while building Git and the ~850
lines of 'prove' output.
On the other hand, it's asking for trouble. In our CI build scripts
we build Git and run the test suite in two separate 'make'
invocations. In a prelimiary version of one of the later patches in
this series, to explicitly specify which compiler to use, I changed
them to basically run:
make CC=$CC
make --quiet test
naively thinking that it should Just Work... but then that 'make
--quiet test' got all clever on me, noticed the changed build flags,
and then proceeded to rebuild everything with the default 'cc'. And
because of that '--quiet' option, it did so, well, quietly, only
saying "* new build flags", and it was by mere luck that I happened to
notice that something is amiss.
Let's just drop that '--quiet' option when running the test suite in
all build scripts.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
ci/run-build-and-tests.sh | 4 ++--
ci/run-linux32-build.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index cda170d5c2..84431c097e 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -8,7 +8,7 @@
ln -s "$cache_dir/.prove" t/.prove
make --jobs=2
-make --quiet test
+make test
if test "$jobname" = "linux-gcc"
then
export GIT_TEST_SPLIT_INDEX=yes
@@ -17,7 +17,7 @@ then
export GIT_TEST_OE_DELTA_SIZE=5
export GIT_TEST_COMMIT_GRAPH=1
export GIT_TEST_MULTI_PACK_INDEX=1
- make --quiet test
+ make test
fi
check_unignored_build_artifacts
diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh
index 2c60d2e70a..26c168a016 100755
--- a/ci/run-linux32-build.sh
+++ b/ci/run-linux32-build.sh
@@ -56,5 +56,5 @@ linux32 --32bit i386 su -m -l $CI_USER -c '
cd /usr/src/git
test -n "$cache_dir" && ln -s "$cache_dir/.prove" t/.prove
make --jobs=2
- make --quiet test
+ make test
'
--
2.20.1.151.gec613c4b75
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 4/5] travis-ci: switch to Xcode 10.1 macOS image
2018-12-20 16:24 ` [PATCH 0/5] travis-ci: build with the right compiler SZEDER Gábor
` (2 preceding siblings ...)
2018-12-20 16:24 ` [PATCH 3/5] travis-ci: don't be '--quiet' when running the tests SZEDER Gábor
@ 2018-12-20 16:24 ` SZEDER Gábor
2018-12-20 16:24 ` [PATCH 5/5] travis-ci: build with the right compiler SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
5 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2018-12-20 16:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin, git, SZEDER Gábor
When building something with GCC installed from Homebrew in the
default macOS (with Xcode 9.4) image on Travis CI, it errors out with
something like this:
/usr/local/Cellar/gcc/8.1.0/lib/gcc/8/gcc/x86_64-apple-darwin17.5.0/8.1.0/include-fixed/stdio.h:78:10: fatal error: _stdio.h: No such file or directory
#include <_stdio.h>
^~~~~~~~~~
This seems to be a common problem affecting several projects, and the
common solution is to use a Travis CI macOS image with more recent
Xcode version, e.g. 10 or 10.1.
While we don't use such a GCC yet, in the very next patch we will, so
switch our OSX build jobs to use the Xcode 10.1 image. Compared to
the Xcode 10 image, this has the benefit that it comes with GCC (v8.2)
preinstalled from Homebrew.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
.travis.yml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.travis.yml b/.travis.yml
index 03c8e4c613..36cbdea7f4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -8,6 +8,8 @@ os:
- linux
- osx
+osx_image: xcode10.1
+
compiler:
- clang
- gcc
--
2.20.1.151.gec613c4b75
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 5/5] travis-ci: build with the right compiler
2018-12-20 16:24 ` [PATCH 0/5] travis-ci: build with the right compiler SZEDER Gábor
` (3 preceding siblings ...)
2018-12-20 16:24 ` [PATCH 4/5] travis-ci: switch to Xcode 10.1 macOS image SZEDER Gábor
@ 2018-12-20 16:24 ` SZEDER Gábor
2019-01-03 16:01 ` Johannes Schindelin
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
5 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2018-12-20 16:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin, git, SZEDER Gábor
Our 'Makefile' hardcodes the compiler to build Git as 'CC = cc'. This
can be overridden from the command line, i.e. 'make CC=gcc-X.Y' will
build with that particular GCC version, but not from the environment,
i.e. 'CC=gcc-X.Y make' will still build with whatever 'cc' happens to
be on the platform.
Our build jobs on Travis CI are badly affected by this. In the build
matrix we have dedicated build jobs to build Git with GCC and Clang
both on Linux and macOS from the very beginning (522354d70f (Add
Travis CI support, 2015-11-27)). Alas, this never really worked as
supposed to, because Travis CI specifies the compiler for those build
jobs as 'export CC=gcc' and 'export CC=clang' (which works fine for
projects built with './configure && make'). Consequently, our
'linux-clang' build job has always used GCC, because that's where 'cc'
points at in Travis CI's Linux images, while the 'osx-gcc' build job
has always used Clang. Furthermore, 37fa4b3c78 (travis-ci: run gcc-8
on linux-gcc jobs, 2018-05-19) added an 'export CC=gcc-8' in an
attempt to build with a more modern compiler, but to no avail.
Set MAKEFLAGS with CC based on the $CC environment variable, so 'make'
will run the "right" compiler. The Xcode 10.1 macOS image on Travis
CI already contains the gcc@8 package from Homebrew, but we have to
'brew link' it first to be able to use it.
So with this patch our build jobs will build Git with the following
compiler versions:
linux-clang: clang version 5.0.0 (tags/RELEASE_500/final)
linux-gcc: gcc-8 (Ubuntu 8.1.0-5ubuntu1~14.04) 8.1.0
osx-clang: Apple LLVM version 10.0.0 (clang-1000.11.45.5)
osx-gcc: gcc-8 (Homebrew GCC 8.2.0) 8.2.0
GETTEXT_POISON: gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
ci/install-dependencies.sh | 5 +++++
ci/lib-travisci.sh | 15 ++++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 06c3546e1e..dc719876bb 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -40,6 +40,11 @@ osx-clang|osx-gcc)
brew install git-lfs gettext
brew link --force gettext
brew install caskroom/cask/perforce
+ case "$jobname" in
+ osx-gcc)
+ brew link gcc@8
+ ;;
+ esac
;;
StaticAnalysis)
sudo apt-get -q update
diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 69dff4d1ec..a479613a57 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -99,12 +99,14 @@ export DEFAULT_TEST_TARGET=prove
export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
export GIT_TEST_OPTS="--verbose-log -x --immediate"
export GIT_TEST_CLONE_2GB=YesPlease
-if [ "$jobname" = linux-gcc ]; then
- export CC=gcc-8
-fi
case "$jobname" in
linux-clang|linux-gcc)
+ if [ "$jobname" = linux-gcc ]
+ then
+ export CC=gcc-8
+ fi
+
export GIT_TEST_HTTPD=YesPlease
# The Linux build installs the defined dependency versions below.
@@ -118,6 +120,11 @@ linux-clang|linux-gcc)
export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
;;
osx-clang|osx-gcc)
+ if [ "$jobname" = osx-gcc ]
+ then
+ export CC=gcc-8
+ fi
+
# t9810 occasionally fails on Travis CI OS X
# t9816 occasionally fails with "TAP out of sequence errors" on
# Travis CI OS X
@@ -127,3 +134,5 @@ GIT_TEST_GETTEXT_POISON)
export GIT_TEST_GETTEXT_POISON=YesPlease
;;
esac
+
+export MAKEFLAGS="CC=${CC:-cc}"
--
2.20.1.151.gec613c4b75
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 5/5] travis-ci: build with the right compiler
2018-12-20 16:24 ` [PATCH 5/5] travis-ci: build with the right compiler SZEDER Gábor
@ 2019-01-03 16:01 ` Johannes Schindelin
0 siblings, 0 replies; 33+ messages in thread
From: Johannes Schindelin @ 2019-01-03 16:01 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Junio C Hamano, Jonathan Nieder, git
[-- Attachment #1: Type: text/plain, Size: 4158 bytes --]
Hi Gábor,
On Thu, 20 Dec 2018, SZEDER Gábor wrote:
> Our 'Makefile' hardcodes the compiler to build Git as 'CC = cc'. This
... This CC variable ...
> can be overridden from the command line, i.e. 'make CC=gcc-X.Y' will
> build with that particular GCC version, but not from the environment,
> i.e. 'CC=gcc-X.Y make' will still build with whatever 'cc' happens to
> be on the platform.
Without this edit, it read to me as if the commit message claimed that
CC cannot be overridden via the environment *at all*, even with MAKEFLAGS.
The rest of the entire patch series looks good to me, I did not dig as
deeply as Ævar about that obstack patch, but if there is *some* sort of
upstream from where we can get a fix, I think we should try to go for that
(rather than risking to diverge even further).
Thanks,
Dscho
>
> Our build jobs on Travis CI are badly affected by this. In the build
> matrix we have dedicated build jobs to build Git with GCC and Clang
> both on Linux and macOS from the very beginning (522354d70f (Add
> Travis CI support, 2015-11-27)). Alas, this never really worked as
> supposed to, because Travis CI specifies the compiler for those build
> jobs as 'export CC=gcc' and 'export CC=clang' (which works fine for
> projects built with './configure && make'). Consequently, our
> 'linux-clang' build job has always used GCC, because that's where 'cc'
> points at in Travis CI's Linux images, while the 'osx-gcc' build job
> has always used Clang. Furthermore, 37fa4b3c78 (travis-ci: run gcc-8
> on linux-gcc jobs, 2018-05-19) added an 'export CC=gcc-8' in an
> attempt to build with a more modern compiler, but to no avail.
>
> Set MAKEFLAGS with CC based on the $CC environment variable, so 'make'
> will run the "right" compiler. The Xcode 10.1 macOS image on Travis
> CI already contains the gcc@8 package from Homebrew, but we have to
> 'brew link' it first to be able to use it.
>
> So with this patch our build jobs will build Git with the following
> compiler versions:
>
> linux-clang: clang version 5.0.0 (tags/RELEASE_500/final)
> linux-gcc: gcc-8 (Ubuntu 8.1.0-5ubuntu1~14.04) 8.1.0
>
> osx-clang: Apple LLVM version 10.0.0 (clang-1000.11.45.5)
> osx-gcc: gcc-8 (Homebrew GCC 8.2.0) 8.2.0
>
> GETTEXT_POISON: gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
> ci/install-dependencies.sh | 5 +++++
> ci/lib-travisci.sh | 15 ++++++++++++---
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 06c3546e1e..dc719876bb 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -40,6 +40,11 @@ osx-clang|osx-gcc)
> brew install git-lfs gettext
> brew link --force gettext
> brew install caskroom/cask/perforce
> + case "$jobname" in
> + osx-gcc)
> + brew link gcc@8
> + ;;
> + esac
> ;;
> StaticAnalysis)
> sudo apt-get -q update
> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
> index 69dff4d1ec..a479613a57 100755
> --- a/ci/lib-travisci.sh
> +++ b/ci/lib-travisci.sh
> @@ -99,12 +99,14 @@ export DEFAULT_TEST_TARGET=prove
> export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
> export GIT_TEST_OPTS="--verbose-log -x --immediate"
> export GIT_TEST_CLONE_2GB=YesPlease
> -if [ "$jobname" = linux-gcc ]; then
> - export CC=gcc-8
> -fi
>
> case "$jobname" in
> linux-clang|linux-gcc)
> + if [ "$jobname" = linux-gcc ]
> + then
> + export CC=gcc-8
> + fi
> +
> export GIT_TEST_HTTPD=YesPlease
>
> # The Linux build installs the defined dependency versions below.
> @@ -118,6 +120,11 @@ linux-clang|linux-gcc)
> export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
> ;;
> osx-clang|osx-gcc)
> + if [ "$jobname" = osx-gcc ]
> + then
> + export CC=gcc-8
> + fi
> +
> # t9810 occasionally fails on Travis CI OS X
> # t9816 occasionally fails with "TAP out of sequence errors" on
> # Travis CI OS X
> @@ -127,3 +134,5 @@ GIT_TEST_GETTEXT_POISON)
> export GIT_TEST_GETTEXT_POISON=YesPlease
> ;;
> esac
> +
> +export MAKEFLAGS="CC=${CC:-cc}"
> --
> 2.20.1.151.gec613c4b75
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 0/5] travis-ci: build with the right compiler
2018-12-20 16:24 ` [PATCH 0/5] travis-ci: build with the right compiler SZEDER Gábor
` (4 preceding siblings ...)
2018-12-20 16:24 ` [PATCH 5/5] travis-ci: build with the right compiler SZEDER Gábor
@ 2019-01-17 1:29 ` SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 1/5] compat/obstack: fix -Wcast-function-type warnings SZEDER Gábor
` (4 more replies)
5 siblings, 5 replies; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, git, SZEDER Gábor
Ever since we started using Travis CI, some of our Travis CI build
jobs don't use the compiler they are supposed to. The last patch
explains what's going on and fixes this issue. The first four patches
are necessary cleanups/fixes to make the fix work.
Changes since v1:
- In patch 1, renamed the fields in the unions holding those
function pointers to match how gnulib's obstack calls them
http://git.savannah.gnu.org/cgit/gnulib.git/commit?id=127ed6a3ea9c46452f079dee50382dc1f70ea796
- Clarified the commit messages of patches 1 and 5.
v1: https://public-inbox.org/git/20181220162452.17732-1-szeder.dev@gmail.com/T/#u
SZEDER Gábor (5):
compat/obstack: fix -Wcast-function-type warnings
.gitignore: ignore external debug symbols from GCC on macOS
travis-ci: don't be '--quiet' when running the tests
travis-ci: switch to Xcode 10.1 macOS image
travis-ci: build with the right compiler
.gitignore | 1 +
.travis.yml | 2 ++
ci/install-dependencies.sh | 5 +++++
ci/lib-travisci.sh | 15 ++++++++++++---
ci/run-build-and-tests.sh | 4 ++--
ci/run-linux32-build.sh | 2 +-
compat/obstack.c | 17 +++++++++--------
compat/obstack.h | 18 +++++++++++-------
8 files changed, 43 insertions(+), 21 deletions(-)
Range-diff:
1: da8f709650 ! 1: be132420c0 compat/obstack: fix -Wcast-function-type warnings
@@ -2,9 +2,11 @@
compat/obstack: fix -Wcast-function-type warnings
- When building Git with GCC 8.2.0 (at least from Homebrew on macOS,
- DEVELOPER flags enabled) one is greeted with a screenful of compiler
- errors:
+ GCC 8 introduced the new -Wcast-function-type warning, which is
+ implied by -Wextra (which, in turn is enabled in our DEVELOPER flags).
+ When building Git with GCC 8 and this warning enabled on a non-glibc
+ platform [1], one is greeted with a screenful of compiler
+ warnings/errors:
compat/obstack.c: In function '_obstack_begin':
compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type]
@@ -24,9 +26,9 @@
'struct obstack' stores pointers to two functions to allocate and free
"chunks", and depending on how obstack is used, these functions take
either one parameter (like standard malloc() and free() do; this is
- how we use it) or two parameters. Presumably to reduce memory
- footprint, a single field is used to store the function pointer for
- both signatures, and then it's casted to the appropriate signature
+ how we use it in 'kwset.c') or two parameters. Presumably to reduce
+ memory footprint, a single field is used to store the function pointer
+ for both signatures, and then it's casted to the appropriate signature
when the function pointer is accessed. These casts between function
pointers with different number of parameters are what trigger those
compiler errors.
@@ -37,6 +39,10 @@
eliminates the need for those casts, and thus avoids this compiler
error.
+ [1] Compiling 'compat/obstack.c' on a platform with glibc is sort of
+ a noop, see the comment before '# define ELIDE_CODE', so this is
+ not an issue on common Linux distros.
+
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
diff --git a/compat/obstack.c b/compat/obstack.c
@@ -48,17 +54,17 @@
(((h) -> use_extra_arg) \
- ? (*(h)->chunkfun) ((h)->extra_arg, (size)) \
- : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
-+ ? (*(h)->chunkfun.fn_extra_arg) ((h)->extra_arg, (size)) \
-+ : (*(h)->chunkfun.fn) ((size)))
++ ? (*(h)->chunkfun.extra) ((h)->extra_arg, (size)) \
++ : (*(h)->chunkfun.plain) ((size)))
# define CALL_FREEFUN(h, old_chunk) \
do { \
if ((h) -> use_extra_arg) \
- (*(h)->freefun) ((h)->extra_arg, (old_chunk)); \
-+ (*(h)->freefun.fn_extra_arg) ((h)->extra_arg, (old_chunk)); \
++ (*(h)->freefun.extra) ((h)->extra_arg, (old_chunk)); \
else \
- (*(void (*) (void *)) (h)->freefun) ((old_chunk)); \
-+ (*(h)->freefun.fn) ((old_chunk)); \
++ (*(h)->freefun.plain) ((old_chunk)); \
} while (0)
\f
@@ -68,8 +74,8 @@
- h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
- h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
-+ h->chunkfun.fn = chunkfun;
-+ h->freefun.fn = freefun;
++ h->chunkfun.plain = chunkfun;
++ h->freefun.plain = freefun;
h->chunk_size = size;
h->alignment_mask = alignment - 1;
h->use_extra_arg = 0;
@@ -79,8 +85,8 @@
- h->chunkfun = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
- h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
-+ h->chunkfun.fn_extra_arg = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
-+ h->freefun.fn_extra_arg = (void (*) (void *, struct _obstack_chunk *)) freefun;
++ h->chunkfun.extra = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
++ h->freefun.extra = (void (*) (void *, struct _obstack_chunk *)) freefun;
+
h->chunk_size = size;
h->alignment_mask = alignment - 1;
@@ -100,12 +106,12 @@
- void (*freefun) (void *, struct _obstack_chunk *);
+ /* These prototypes vary based on `use_extra_arg'. */
+ union {
-+ struct _obstack_chunk *(*fn_extra_arg) (void *, long);
-+ void *(*fn) (long);
++ void *(*plain) (long);
++ struct _obstack_chunk *(*extra) (void *, long);
+ } chunkfun;
+ union {
-+ void (*fn_extra_arg) (void *, struct _obstack_chunk *);
-+ void (*fn) (void *);
++ void (*plain) (void *);
++ void (*extra) (void *, struct _obstack_chunk *);
+ } freefun;
void *extra_arg; /* first arg for chunk alloc/dealloc funcs */
unsigned use_extra_arg:1; /* chunk alloc/dealloc funcs take extra arg */
@@ -115,11 +121,11 @@
#define obstack_chunkfun(h, newchunkfun) \
- ((h) -> chunkfun = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
-+ ((h)->chunkfun.fn_extra_arg = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
++ ((h)->chunkfun.extra = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
#define obstack_freefun(h, newfreefun) \
- ((h) -> freefun = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
-+ ((h)->freefun.fn_extra_arg = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
++ ((h)->freefun.extra = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
#define obstack_1grow_fast(h,achar) (*((h)->next_free)++ = (achar))
2: 1f6be06841 = 2: 983e5a4107 .gitignore: ignore external debug symbols from GCC on macOS
3: 6f6ece8872 = 3: cc3eb12441 travis-ci: don't be '--quiet' when running the tests
4: a4179caeba = 4: 2155876a85 travis-ci: switch to Xcode 10.1 macOS image
5: 36323a7c8a ! 5: c91a5bc7ee travis-ci: build with the right compiler
@@ -3,10 +3,10 @@
travis-ci: build with the right compiler
Our 'Makefile' hardcodes the compiler to build Git as 'CC = cc'. This
- can be overridden from the command line, i.e. 'make CC=gcc-X.Y' will
- build with that particular GCC version, but not from the environment,
- i.e. 'CC=gcc-X.Y make' will still build with whatever 'cc' happens to
- be on the platform.
+ CC variable can be overridden from the command line, i.e. 'make
+ CC=gcc-X.Y' will build with that particular GCC version, but not from
+ the environment, i.e. 'CC=gcc-X.Y make' will still build with whatever
+ 'cc' happens to be on the platform.
Our build jobs on Travis CI are badly affected by this. In the build
matrix we have dedicated build jobs to build Git with GCC and Clang
--
2.20.1.499.gf60de1223c
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 1/5] compat/obstack: fix -Wcast-function-type warnings
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
@ 2019-01-17 1:29 ` SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 2/5] .gitignore: ignore external debug symbols from GCC on macOS SZEDER Gábor
` (3 subsequent siblings)
4 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, git, SZEDER Gábor
GCC 8 introduced the new -Wcast-function-type warning, which is
implied by -Wextra (which, in turn is enabled in our DEVELOPER flags).
When building Git with GCC 8 and this warning enabled on a non-glibc
platform [1], one is greeted with a screenful of compiler
warnings/errors:
compat/obstack.c: In function '_obstack_begin':
compat/obstack.c:162:17: error: cast between incompatible function types from 'void * (*)(long int)' to 'struct _obstack_chunk * (*)(void *, long int)' [-Werror=cast-function-type]
h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
^
compat/obstack.c:163:16: error: cast between incompatible function types from 'void (*)(void *)' to 'void (*)(void *, struct _obstack_chunk *)' [-Werror=cast-function-type]
h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
^
compat/obstack.c:116:8: error: cast between incompatible function types from 'struct _obstack_chunk * (*)(void *, long int)' to 'struct _obstack_chunk * (*)(long int)' [-Werror=cast-function-type]
: (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
^
compat/obstack.c:168:22: note: in expansion of macro 'CALL_CHUNKFUN'
chunk = h->chunk = CALL_CHUNKFUN (h, h -> chunk_size);
^~~~~~~~~~~~~
<snip>
'struct obstack' stores pointers to two functions to allocate and free
"chunks", and depending on how obstack is used, these functions take
either one parameter (like standard malloc() and free() do; this is
how we use it in 'kwset.c') or two parameters. Presumably to reduce
memory footprint, a single field is used to store the function pointer
for both signatures, and then it's casted to the appropriate signature
when the function pointer is accessed. These casts between function
pointers with different number of parameters are what trigger those
compiler errors.
Modify 'struct obstack' to use unions to store function pointers with
different signatures, and then use the union member with the
appropriate signature when accessing these function pointers. This
eliminates the need for those casts, and thus avoids this compiler
error.
[1] Compiling 'compat/obstack.c' on a platform with glibc is sort of
a noop, see the comment before '# define ELIDE_CODE', so this is
not an issue on common Linux distros.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
compat/obstack.c | 17 +++++++++--------
compat/obstack.h | 18 +++++++++++-------
2 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/compat/obstack.c b/compat/obstack.c
index 4d1d95beeb..27cd5c1ea1 100644
--- a/compat/obstack.c
+++ b/compat/obstack.c
@@ -112,15 +112,15 @@ compat_symbol (libc, _obstack_compat, _obstack, GLIBC_2_0);
# define CALL_CHUNKFUN(h, size) \
(((h) -> use_extra_arg) \
- ? (*(h)->chunkfun) ((h)->extra_arg, (size)) \
- : (*(struct _obstack_chunk *(*) (long)) (h)->chunkfun) ((size)))
+ ? (*(h)->chunkfun.extra) ((h)->extra_arg, (size)) \
+ : (*(h)->chunkfun.plain) ((size)))
# define CALL_FREEFUN(h, old_chunk) \
do { \
if ((h) -> use_extra_arg) \
- (*(h)->freefun) ((h)->extra_arg, (old_chunk)); \
+ (*(h)->freefun.extra) ((h)->extra_arg, (old_chunk)); \
else \
- (*(void (*) (void *)) (h)->freefun) ((old_chunk)); \
+ (*(h)->freefun.plain) ((old_chunk)); \
} while (0)
\f
@@ -159,8 +159,8 @@ _obstack_begin (struct obstack *h,
size = 4096 - extra;
}
- h->chunkfun = (struct _obstack_chunk * (*)(void *, long)) chunkfun;
- h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
+ h->chunkfun.plain = chunkfun;
+ h->freefun.plain = freefun;
h->chunk_size = size;
h->alignment_mask = alignment - 1;
h->use_extra_arg = 0;
@@ -206,8 +206,9 @@ _obstack_begin_1 (struct obstack *h, int size, int alignment,
size = 4096 - extra;
}
- h->chunkfun = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
- h->freefun = (void (*) (void *, struct _obstack_chunk *)) freefun;
+ h->chunkfun.extra = (struct _obstack_chunk * (*)(void *,long)) chunkfun;
+ h->freefun.extra = (void (*) (void *, struct _obstack_chunk *)) freefun;
+
h->chunk_size = size;
h->alignment_mask = alignment - 1;
h->extra_arg = arg;
diff --git a/compat/obstack.h b/compat/obstack.h
index 6bc24b7644..ced94d0118 100644
--- a/compat/obstack.h
+++ b/compat/obstack.h
@@ -160,11 +160,15 @@ struct obstack /* control current object in current chunk */
void *tempptr;
} temp; /* Temporary for some macros. */
int alignment_mask; /* Mask of alignment for each object. */
- /* These prototypes vary based on `use_extra_arg', and we use
- casts to the prototypeless function type in all assignments,
- but having prototypes here quiets -Wstrict-prototypes. */
- struct _obstack_chunk *(*chunkfun) (void *, long);
- void (*freefun) (void *, struct _obstack_chunk *);
+ /* These prototypes vary based on `use_extra_arg'. */
+ union {
+ void *(*plain) (long);
+ struct _obstack_chunk *(*extra) (void *, long);
+ } chunkfun;
+ union {
+ void (*plain) (void *);
+ void (*extra) (void *, struct _obstack_chunk *);
+ } freefun;
void *extra_arg; /* first arg for chunk alloc/dealloc funcs */
unsigned use_extra_arg:1; /* chunk alloc/dealloc funcs take extra arg */
unsigned maybe_empty_object:1;/* There is a possibility that the current
@@ -235,10 +239,10 @@ extern void (*obstack_alloc_failed_handler) (void);
(void (*) (void *, void *)) (freefun), (arg))
#define obstack_chunkfun(h, newchunkfun) \
- ((h) -> chunkfun = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
+ ((h)->chunkfun.extra = (struct _obstack_chunk *(*)(void *, long)) (newchunkfun))
#define obstack_freefun(h, newfreefun) \
- ((h) -> freefun = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
+ ((h)->freefun.extra = (void (*)(void *, struct _obstack_chunk *)) (newfreefun))
#define obstack_1grow_fast(h,achar) (*((h)->next_free)++ = (achar))
--
2.20.1.499.gf60de1223c
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v2 2/5] .gitignore: ignore external debug symbols from GCC on macOS
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 1/5] compat/obstack: fix -Wcast-function-type warnings SZEDER Gábor
@ 2019-01-17 1:29 ` SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 3/5] travis-ci: don't be '--quiet' when running the tests SZEDER Gábor
` (2 subsequent siblings)
4 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, git, SZEDER Gábor
When Git is build with a "real" GCC on macOS [1], or at least with GCC
installed via Homebrew, and CFLAGS includes the '-g' option (and our
default CFLAGS does), then by default GCC writes the debug symbols
into external files under '<binary>.dSYM/' directories (e.g.
'git-daemon.dSYM/', 'git.dSYM/', etc.).
Update '.gitignore' to ignore these directories, so they don't clutter
the output of 'git status'. Furthermore, these build artifacts then
won't trigger build failures on Travis CI via b92cb86ea1 (travis-ci:
check that all build artifacts are .gitignore-d, 2017-12-31) once one
of the following patches updates our CI build scripts to use a real
GCC in the 'osx-gcc' build job.
[1] On macOS the default '/usr/bin/gcc' executable is not a real GCC,
but merely a compatibility wrapper around Clang:
$ gcc --version
Configured with: --prefix=<...>
Apple LLVM version 9.0.0 (clang-900.0.39.2)
<...>
So even though 'make CC=gcc' does indeed execute a command called
'gcc', in the end Git will be built with Clang all the same.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
.gitignore | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitignore b/.gitignore
index 0d77ea5894..a9db568712 100644
--- a/.gitignore
+++ b/.gitignore
@@ -229,3 +229,4 @@
*.pdb
/Debug/
/Release/
+*.dSYM
--
2.20.1.499.gf60de1223c
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v2 3/5] travis-ci: don't be '--quiet' when running the tests
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 1/5] compat/obstack: fix -Wcast-function-type warnings SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 2/5] .gitignore: ignore external debug symbols from GCC on macOS SZEDER Gábor
@ 2019-01-17 1:29 ` SZEDER Gábor
2019-01-17 13:28 ` Johannes Schindelin
2019-01-17 1:29 ` [PATCH v2 4/5] travis-ci: switch to Xcode 10.1 macOS image SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 5/5] travis-ci: build with the right compiler SZEDER Gábor
4 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, git, SZEDER Gábor
All Travis CI build jobs run the test suite with 'make --quiet test'.
On one hand, being quiet doesn't save us from much clutter in the
output:
$ make test |wc -l
861
$ make --quiet test |wc -l
848
It only spares 13 lines, mostly the output of entering the 't/'
directory and the pre- and post-cleanup commands, which is negligible
compared to the ~700 lines printed while building Git and the ~850
lines of 'prove' output.
On the other hand, it's asking for trouble. In our CI build scripts
we build Git and run the test suite in two separate 'make'
invocations. In a prelimiary version of one of the later patches in
this series, to explicitly specify which compiler to use, I changed
them to basically run:
make CC=$CC
make --quiet test
naively thinking that it should Just Work... but then that 'make
--quiet test' got all clever on me, noticed the changed build flags,
and then proceeded to rebuild everything with the default 'cc'. And
because of that '--quiet' option, it did so, well, quietly, only
saying "* new build flags", and it was by mere luck that I happened to
notice that something is amiss.
Let's just drop that '--quiet' option when running the test suite in
all build scripts.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
ci/run-build-and-tests.sh | 4 ++--
ci/run-linux32-build.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
index cda170d5c2..84431c097e 100755
--- a/ci/run-build-and-tests.sh
+++ b/ci/run-build-and-tests.sh
@@ -8,7 +8,7 @@
ln -s "$cache_dir/.prove" t/.prove
make --jobs=2
-make --quiet test
+make test
if test "$jobname" = "linux-gcc"
then
export GIT_TEST_SPLIT_INDEX=yes
@@ -17,7 +17,7 @@ then
export GIT_TEST_OE_DELTA_SIZE=5
export GIT_TEST_COMMIT_GRAPH=1
export GIT_TEST_MULTI_PACK_INDEX=1
- make --quiet test
+ make test
fi
check_unignored_build_artifacts
diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh
index 2c60d2e70a..26c168a016 100755
--- a/ci/run-linux32-build.sh
+++ b/ci/run-linux32-build.sh
@@ -56,5 +56,5 @@ linux32 --32bit i386 su -m -l $CI_USER -c '
cd /usr/src/git
test -n "$cache_dir" && ln -s "$cache_dir/.prove" t/.prove
make --jobs=2
- make --quiet test
+ make test
'
--
2.20.1.499.gf60de1223c
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v2 3/5] travis-ci: don't be '--quiet' when running the tests
2019-01-17 1:29 ` [PATCH v2 3/5] travis-ci: don't be '--quiet' when running the tests SZEDER Gábor
@ 2019-01-17 13:28 ` Johannes Schindelin
0 siblings, 0 replies; 33+ messages in thread
From: Johannes Schindelin @ 2019-01-17 13:28 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Junio C Hamano, Jonathan Nieder,
Ævar Arnfjörð Bjarmason, git
[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]
Hi Gábor,
On Thu, 17 Jan 2019, SZEDER Gábor wrote:
> All Travis CI build jobs run the test suite with 'make --quiet test'.
>
> On one hand, being quiet doesn't save us from much clutter in the
> output:
>
> $ make test |wc -l
> 861
> $ make --quiet test |wc -l
> 848
>
> It only spares 13 lines, mostly the output of entering the 't/'
> directory and the pre- and post-cleanup commands, which is negligible
> compared to the ~700 lines printed while building Git and the ~850
> lines of 'prove' output.
>
> On the other hand, it's asking for trouble. In our CI build scripts
> we build Git and run the test suite in two separate 'make'
> invocations. In a prelimiary version of one of the later patches in
s/prelimiary/preliminary/
> this series, to explicitly specify which compiler to use, I changed
> them to basically run:
>
> make CC=$CC
> make --quiet test
>
> naively thinking that it should Just Work... but then that 'make
> --quiet test' got all clever on me, noticed the changed build flags,
> and then proceeded to rebuild everything with the default 'cc'. And
> because of that '--quiet' option, it did so, well, quietly, only
> saying "* new build flags", and it was by mere luck that I happened to
> notice that something is amiss.
>
> Let's just drop that '--quiet' option when running the test suite in
> all build scripts.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Just like the first two patches, this makes tons of sense to me.
I thank you and will read on,
Dscho
> ---
> ci/run-build-and-tests.sh | 4 ++--
> ci/run-linux32-build.sh | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh
> index cda170d5c2..84431c097e 100755
> --- a/ci/run-build-and-tests.sh
> +++ b/ci/run-build-and-tests.sh
> @@ -8,7 +8,7 @@
> ln -s "$cache_dir/.prove" t/.prove
>
> make --jobs=2
> -make --quiet test
> +make test
> if test "$jobname" = "linux-gcc"
> then
> export GIT_TEST_SPLIT_INDEX=yes
> @@ -17,7 +17,7 @@ then
> export GIT_TEST_OE_DELTA_SIZE=5
> export GIT_TEST_COMMIT_GRAPH=1
> export GIT_TEST_MULTI_PACK_INDEX=1
> - make --quiet test
> + make test
> fi
>
> check_unignored_build_artifacts
> diff --git a/ci/run-linux32-build.sh b/ci/run-linux32-build.sh
> index 2c60d2e70a..26c168a016 100755
> --- a/ci/run-linux32-build.sh
> +++ b/ci/run-linux32-build.sh
> @@ -56,5 +56,5 @@ linux32 --32bit i386 su -m -l $CI_USER -c '
> cd /usr/src/git
> test -n "$cache_dir" && ln -s "$cache_dir/.prove" t/.prove
> make --jobs=2
> - make --quiet test
> + make test
> '
> --
> 2.20.1.499.gf60de1223c
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 4/5] travis-ci: switch to Xcode 10.1 macOS image
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
` (2 preceding siblings ...)
2019-01-17 1:29 ` [PATCH v2 3/5] travis-ci: don't be '--quiet' when running the tests SZEDER Gábor
@ 2019-01-17 1:29 ` SZEDER Gábor
2019-01-17 1:29 ` [PATCH v2 5/5] travis-ci: build with the right compiler SZEDER Gábor
4 siblings, 0 replies; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, git, SZEDER Gábor
When building something with GCC installed from Homebrew in the
default macOS (with Xcode 9.4) image on Travis CI, it errors out with
something like this:
/usr/local/Cellar/gcc/8.1.0/lib/gcc/8/gcc/x86_64-apple-darwin17.5.0/8.1.0/include-fixed/stdio.h:78:10: fatal error: _stdio.h: No such file or directory
#include <_stdio.h>
^~~~~~~~~~
This seems to be a common problem affecting several projects, and the
common solution is to use a Travis CI macOS image with more recent
Xcode version, e.g. 10 or 10.1.
While we don't use such a GCC yet, in the very next patch we will, so
switch our OSX build jobs to use the Xcode 10.1 image. Compared to
the Xcode 10 image, this has the benefit that it comes with GCC (v8.2)
preinstalled from Homebrew.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
.travis.yml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.travis.yml b/.travis.yml
index 03c8e4c613..36cbdea7f4 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -8,6 +8,8 @@ os:
- linux
- osx
+osx_image: xcode10.1
+
compiler:
- clang
- gcc
--
2.20.1.499.gf60de1223c
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v2 5/5] travis-ci: build with the right compiler
2019-01-17 1:29 ` [PATCH v2 0/5] " SZEDER Gábor
` (3 preceding siblings ...)
2019-01-17 1:29 ` [PATCH v2 4/5] travis-ci: switch to Xcode 10.1 macOS image SZEDER Gábor
@ 2019-01-17 1:29 ` SZEDER Gábor
2019-01-17 13:44 ` Johannes Schindelin
4 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 1:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Johannes Schindelin,
Ævar Arnfjörð Bjarmason, git, SZEDER Gábor
Our 'Makefile' hardcodes the compiler to build Git as 'CC = cc'. This
CC variable can be overridden from the command line, i.e. 'make
CC=gcc-X.Y' will build with that particular GCC version, but not from
the environment, i.e. 'CC=gcc-X.Y make' will still build with whatever
'cc' happens to be on the platform.
Our build jobs on Travis CI are badly affected by this. In the build
matrix we have dedicated build jobs to build Git with GCC and Clang
both on Linux and macOS from the very beginning (522354d70f (Add
Travis CI support, 2015-11-27)). Alas, this never really worked as
supposed to, because Travis CI specifies the compiler for those build
jobs as 'export CC=gcc' and 'export CC=clang' (which works fine for
projects built with './configure && make'). Consequently, our
'linux-clang' build job has always used GCC, because that's where 'cc'
points at in Travis CI's Linux images, while the 'osx-gcc' build job
has always used Clang. Furthermore, 37fa4b3c78 (travis-ci: run gcc-8
on linux-gcc jobs, 2018-05-19) added an 'export CC=gcc-8' in an
attempt to build with a more modern compiler, but to no avail.
Set MAKEFLAGS with CC based on the $CC environment variable, so 'make'
will run the "right" compiler. The Xcode 10.1 macOS image on Travis
CI already contains the gcc@8 package from Homebrew, but we have to
'brew link' it first to be able to use it.
So with this patch our build jobs will build Git with the following
compiler versions:
linux-clang: clang version 5.0.0 (tags/RELEASE_500/final)
linux-gcc: gcc-8 (Ubuntu 8.1.0-5ubuntu1~14.04) 8.1.0
osx-clang: Apple LLVM version 10.0.0 (clang-1000.11.45.5)
osx-gcc: gcc-8 (Homebrew GCC 8.2.0) 8.2.0
GETTEXT_POISON: gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
ci/install-dependencies.sh | 5 +++++
ci/lib-travisci.sh | 15 ++++++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 06c3546e1e..dc719876bb 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -40,6 +40,11 @@ osx-clang|osx-gcc)
brew install git-lfs gettext
brew link --force gettext
brew install caskroom/cask/perforce
+ case "$jobname" in
+ osx-gcc)
+ brew link gcc@8
+ ;;
+ esac
;;
StaticAnalysis)
sudo apt-get -q update
diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
index 69dff4d1ec..a479613a57 100755
--- a/ci/lib-travisci.sh
+++ b/ci/lib-travisci.sh
@@ -99,12 +99,14 @@ export DEFAULT_TEST_TARGET=prove
export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
export GIT_TEST_OPTS="--verbose-log -x --immediate"
export GIT_TEST_CLONE_2GB=YesPlease
-if [ "$jobname" = linux-gcc ]; then
- export CC=gcc-8
-fi
case "$jobname" in
linux-clang|linux-gcc)
+ if [ "$jobname" = linux-gcc ]
+ then
+ export CC=gcc-8
+ fi
+
export GIT_TEST_HTTPD=YesPlease
# The Linux build installs the defined dependency versions below.
@@ -118,6 +120,11 @@ linux-clang|linux-gcc)
export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
;;
osx-clang|osx-gcc)
+ if [ "$jobname" = osx-gcc ]
+ then
+ export CC=gcc-8
+ fi
+
# t9810 occasionally fails on Travis CI OS X
# t9816 occasionally fails with "TAP out of sequence errors" on
# Travis CI OS X
@@ -127,3 +134,5 @@ GIT_TEST_GETTEXT_POISON)
export GIT_TEST_GETTEXT_POISON=YesPlease
;;
esac
+
+export MAKEFLAGS="CC=${CC:-cc}"
--
2.20.1.499.gf60de1223c
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v2 5/5] travis-ci: build with the right compiler
2019-01-17 1:29 ` [PATCH v2 5/5] travis-ci: build with the right compiler SZEDER Gábor
@ 2019-01-17 13:44 ` Johannes Schindelin
2019-01-17 14:56 ` SZEDER Gábor
0 siblings, 1 reply; 33+ messages in thread
From: Johannes Schindelin @ 2019-01-17 13:44 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Junio C Hamano, Jonathan Nieder,
Ævar Arnfjörð Bjarmason, git
[-- Attachment #1: Type: text/plain, Size: 4488 bytes --]
Hi Gábor,
On Thu, 17 Jan 2019, SZEDER Gábor wrote:
> Our 'Makefile' hardcodes the compiler to build Git as 'CC = cc'. This
> CC variable can be overridden from the command line, i.e. 'make
> CC=gcc-X.Y' will build with that particular GCC version, but not from
> the environment, i.e. 'CC=gcc-X.Y make' will still build with whatever
> 'cc' happens to be on the platform.
>
> Our build jobs on Travis CI are badly affected by this. In the build
> matrix we have dedicated build jobs to build Git with GCC and Clang
> both on Linux and macOS from the very beginning (522354d70f (Add
> Travis CI support, 2015-11-27)). Alas, this never really worked as
> supposed to, because Travis CI specifies the compiler for those build
> jobs as 'export CC=gcc' and 'export CC=clang' (which works fine for
> projects built with './configure && make'). Consequently, our
> 'linux-clang' build job has always used GCC, because that's where 'cc'
> points at in Travis CI's Linux images, while the 'osx-gcc' build job
> has always used Clang. Furthermore, 37fa4b3c78 (travis-ci: run gcc-8
> on linux-gcc jobs, 2018-05-19) added an 'export CC=gcc-8' in an
> attempt to build with a more modern compiler, but to no avail.
>
> Set MAKEFLAGS with CC based on the $CC environment variable, so 'make'
> will run the "right" compiler. The Xcode 10.1 macOS image on Travis
> CI already contains the gcc@8 package from Homebrew, but we have to
> 'brew link' it first to be able to use it.
>
> So with this patch our build jobs will build Git with the following
> compiler versions:
>
> linux-clang: clang version 5.0.0 (tags/RELEASE_500/final)
> linux-gcc: gcc-8 (Ubuntu 8.1.0-5ubuntu1~14.04) 8.1.0
>
> osx-clang: Apple LLVM version 10.0.0 (clang-1000.11.45.5)
> osx-gcc: gcc-8 (Homebrew GCC 8.2.0) 8.2.0
>
> GETTEXT_POISON: gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Nicely explained and implemented. Thank you!
Apart from the single typo I stumbled over (quite honestly, I do not
really care about typos, I just point them out when I see them to beat
Eric to it), I have just one more wish: I would *love* to see a Travis
run.
Surely you opened a PR at https://github.com/git/git. Oh wait, I can look
for it! But no, there does not seem to be one. So probably in your fork,
https://github.com/szeder/git. No PRs. Ah, but there are branches! 60 of
them. And yes, I guess I found a Travis build:
https://travis-ci.org/szeder/git/builds/480654256
But no, the associated branch does not look like it reflects this patch
series...
In any case, this series is:
Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Ciao,
Dscho
> ---
> ci/install-dependencies.sh | 5 +++++
> ci/lib-travisci.sh | 15 ++++++++++++---
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 06c3546e1e..dc719876bb 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -40,6 +40,11 @@ osx-clang|osx-gcc)
> brew install git-lfs gettext
> brew link --force gettext
> brew install caskroom/cask/perforce
> + case "$jobname" in
> + osx-gcc)
> + brew link gcc@8
> + ;;
> + esac
> ;;
> StaticAnalysis)
> sudo apt-get -q update
> diff --git a/ci/lib-travisci.sh b/ci/lib-travisci.sh
> index 69dff4d1ec..a479613a57 100755
> --- a/ci/lib-travisci.sh
> +++ b/ci/lib-travisci.sh
> @@ -99,12 +99,14 @@ export DEFAULT_TEST_TARGET=prove
> export GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
> export GIT_TEST_OPTS="--verbose-log -x --immediate"
> export GIT_TEST_CLONE_2GB=YesPlease
> -if [ "$jobname" = linux-gcc ]; then
> - export CC=gcc-8
> -fi
>
> case "$jobname" in
> linux-clang|linux-gcc)
> + if [ "$jobname" = linux-gcc ]
> + then
> + export CC=gcc-8
> + fi
> +
> export GIT_TEST_HTTPD=YesPlease
>
> # The Linux build installs the defined dependency versions below.
> @@ -118,6 +120,11 @@ linux-clang|linux-gcc)
> export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
> ;;
> osx-clang|osx-gcc)
> + if [ "$jobname" = osx-gcc ]
> + then
> + export CC=gcc-8
> + fi
> +
> # t9810 occasionally fails on Travis CI OS X
> # t9816 occasionally fails with "TAP out of sequence errors" on
> # Travis CI OS X
> @@ -127,3 +134,5 @@ GIT_TEST_GETTEXT_POISON)
> export GIT_TEST_GETTEXT_POISON=YesPlease
> ;;
> esac
> +
> +export MAKEFLAGS="CC=${CC:-cc}"
> --
> 2.20.1.499.gf60de1223c
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 5/5] travis-ci: build with the right compiler
2019-01-17 13:44 ` Johannes Schindelin
@ 2019-01-17 14:56 ` SZEDER Gábor
2019-01-18 8:40 ` Johannes Schindelin
0 siblings, 1 reply; 33+ messages in thread
From: SZEDER Gábor @ 2019-01-17 14:56 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Jonathan Nieder,
Ævar Arnfjörð Bjarmason, git
On Thu, Jan 17, 2019 at 02:44:59PM +0100, Johannes Schindelin wrote:
> Hi Gábor,
>
> On Thu, 17 Jan 2019, SZEDER Gábor wrote:
>
> > Our 'Makefile' hardcodes the compiler to build Git as 'CC = cc'. This
> > CC variable can be overridden from the command line, i.e. 'make
> > CC=gcc-X.Y' will build with that particular GCC version, but not from
> > the environment, i.e. 'CC=gcc-X.Y make' will still build with whatever
> > 'cc' happens to be on the platform.
> >
> > Our build jobs on Travis CI are badly affected by this. In the build
> > matrix we have dedicated build jobs to build Git with GCC and Clang
> > both on Linux and macOS from the very beginning (522354d70f (Add
> > Travis CI support, 2015-11-27)). Alas, this never really worked as
> > supposed to, because Travis CI specifies the compiler for those build
> > jobs as 'export CC=gcc' and 'export CC=clang' (which works fine for
> > projects built with './configure && make'). Consequently, our
> > 'linux-clang' build job has always used GCC, because that's where 'cc'
> > points at in Travis CI's Linux images, while the 'osx-gcc' build job
> > has always used Clang. Furthermore, 37fa4b3c78 (travis-ci: run gcc-8
> > on linux-gcc jobs, 2018-05-19) added an 'export CC=gcc-8' in an
> > attempt to build with a more modern compiler, but to no avail.
> >
> > Set MAKEFLAGS with CC based on the $CC environment variable, so 'make'
> > will run the "right" compiler. The Xcode 10.1 macOS image on Travis
> > CI already contains the gcc@8 package from Homebrew, but we have to
> > 'brew link' it first to be able to use it.
> >
> > So with this patch our build jobs will build Git with the following
> > compiler versions:
> >
> > linux-clang: clang version 5.0.0 (tags/RELEASE_500/final)
> > linux-gcc: gcc-8 (Ubuntu 8.1.0-5ubuntu1~14.04) 8.1.0
> >
> > osx-clang: Apple LLVM version 10.0.0 (clang-1000.11.45.5)
> > osx-gcc: gcc-8 (Homebrew GCC 8.2.0) 8.2.0
> >
> > GETTEXT_POISON: gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
> >
> > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>
> Nicely explained and implemented. Thank you!
>
> Apart from the single typo I stumbled over (quite honestly, I do not
> really care about typos, I just point them out when I see them to beat
> Eric to it)
Heh ;)
> I have just one more wish: I would *love* to see a Travis
> run.
>
> Surely you opened a PR at https://github.com/git/git. Oh wait, I can look
> for it! But no, there does not seem to be one.
Nope, "all pull requests are ignored", so I don't bother.
> So probably in your fork,
> https://github.com/szeder/git. No PRs. Ah, but there are branches! 60 of
> them.
Oh, wow, 60, that really needs to be cleaned up and all the already
upstreamed, obsolete, or test branches removed. And, more
importantly, my WIP branches finished and finally upstreamed...
Sigh, maybe someday...
> And yes, I guess I found a Travis build:
>
> https://travis-ci.org/szeder/git/builds/480654256
That would be the right build, but it was the result of a rebase
squashing a fixup commit and editing commit messages while leaving the
resulting tree intact, and then 9cc2c76f5e (travis-ci: record and skip
successfully built trees, 2017-12-31) kicked in and optimized away the
build.
Anyway, I deleted the branch's cache and restarted the build, it will
most likely be finished by the time I send this email.
> But no, the associated branch does not look like it reflects this patch
> series...
That's the right branch, though, except that it contains a debug patch
on top (that I didn't submitted) that shows the used compiler and its
version from within the Makefile, to make sure that the build jobs do
indeed use the compilers that I think they should be using.
The other patches on that branch are the same that were submitted in
this patch series.
Here are the most interesting bits of the problematic build jobs, i.e.
the compiler versions from that debug patch:
linux-clang: https://travis-ci.org/szeder/git/jobs/480654257#L824
osx-gcc: https://travis-ci.org/szeder/git/jobs/480654264#L824
> In any case, this series is:
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Thanks.
^ permalink raw reply [flat|nested] 33+ messages in thread