Ralf Wildenhues <ralf.wildenh...@gmx.de> writes:
> I casually looked at libgo/{Makefile.am,configure.ac}.
Thanks for the review.
> You don't need to list .c files in the BUILT_SOURCES variable if they
> are also listed in some <prog>_SOURCES variable (and are not
> undocumented prerequisites for other targets). make will know that it
> needs to create these files. (Omitting BUILT_SOURCES saves a recursive
> make invocation in practice.)
Fixed.
>> if LIBGO_IS_386
>> GOARCH = 386
>> else
>> if LIBGO_IS_X86_64
>> GOARCH = amd64
>> else
>> if LIBGO_IS_ARM
>> GOARCH = arm
>> else
>> GOARCH = unknown
>> endif
>> endif
>> endif
>
> I wouldn't write Automake conditionals deeply nested, it makes for long
> lines and quadratic number of characters in Makefile.in when the number
> of conditionals increases. But who am I telling that. ;-)
> You could just AC_SUBST([GOARCH]) and GOOS from configure.ac; but this
> is a really minor stylistic issue.
It's ugly but it puts the value where it is used, rather than splitting
it between configure.ac and Makefile.am. This makes it easier to
understand what is happening.
If only the person who implemented automake conditionals had added elif.
What a maroon.
> Several %/check targets use 'mkdir -p'. While that is quite portable
> nowadays, there are still mkdir implementations that are racy when run
> in parallel. You can use $(MKDIR_P) instead.
Fixed.
> I suppose portability to systems where OBJEXT is .obj is not a question
> at this point.
Not really, but I changed .o to .$(OBJEXT) anyhow.
All patches committed to gccgo branch. Thanks again for the review.
Ian
