Hello Dave,
While readability is improved with aligned variable assignments, we have
opted not to encourage that style of coding. See our build system code
conventions [1], specifically:
18. Avoid padding internally in a line with spaces to try to align some
feature into columns with surrounding lines.
With the following rationale:
17-18. A very typical use case of makefile changes is to add things
(files, compiler directives, etc) to a list. These rules help making
such changes easy and context free. Otherwise the developer must modify
several lines that are unrelated to the actual change. Chances are that
a nicely padded grid will not be updated and start deteriorating from
the very first change.
So it would be unlikely for us to accept such a change.
[1] http://openjdk.java.net/groups/build/doc/code-conventions.html
/Erik
On 2015-01-22 17:37, Dave Pointon wrote:
Hiya Erik ,
FWIW, this looks not unreasonable to this non-reviewer.
By way of an aside, I have nearly completed a set of changes to the
make files (in our repos) that, IMO, considerably improves the
readability of makefiles in which there are calls to macros using
extended argument lists - by judicious use of alignment of the ':='
together with continuation lines. Presumably, I'd have to raise a bug
to cover such changes ?
TIA & best rgds ,
--
Dave Pointon FIAP MBCS
Now I saw, tho' too late, the folly of beginning a work before we
count the cost and before we we judge rightly of our strength to go
thro' with it - Robinson Crusoe
On 22 January 2015 at 14:40, Erik Joelsson <[email protected]
<mailto:[email protected]>> wrote:
Hello,
Please review this patch, which makes it possible to take a
compile command line from the make debug log on Windows, and rerun
it in a normal cygwin environment, without the need for running
vsvars*.bat first.
When building native code on windows, using Visual Studio,
configure extracts the build environment from the setup .bat file
provided by VS and sets 3 variables in spec.gmk: PATH, INCLUDE and
LIB. These 3 variables are also exported into the environment in
spec.gmk, so that every tool run by the build will see them. While
this is convenient, it makes the command lines used by the build
unusable outside of the build, unless you also export these
variables with the correct values.
I have removed the need for INCLUDE and LIB to be exported, by
converting their contents into compiler and linker flags. These
flags conceptually fit well in the recent SYSROOT_CFLAGS and
SYSROOT_LDFLAGS variables.
The PATH variable would be nice to not have to set, and while not
setting it seems to work most of the time, I suspect that there
are cases when it won't work. More specifically, in certain
environments, some dll needed by the compiler program might not be
on the path without it. So I left it being set for now.
The new LDFLAGS requires unpack200.exe to stop being linked
differently to all other executables. There is no reason for this
discrepancy that I can find, it just seems like someone did a bit
of a quick hack getting it to build long ago in the old build, and
we wanted to keep it equivalent in build-infra.
The hotspot build still requires the variables to be exported, so
they are still being defined in hotspot-spec.gmk.
While working on this, I stumbled on a problem when running "make
reconfigure". The PATH variable value, since exported in make,
would get longer and longer for each time you run reconfigure. I
fixed this by saving the original path and resetting it before
running configure from make.
Bug: https://bugs.openjdk.java.net/browse/JDK-8071329
Webrevs:
http://cr.openjdk.java.net/~erikj/8071329/webrev.root.01/
<http://cr.openjdk.java.net/%7Eerikj/8071329/webrev.root.01/>
http://cr.openjdk.java.net/~erikj/8071329/webrev.jdk.01/
<http://cr.openjdk.java.net/%7Eerikj/8071329/webrev.jdk.01/>
/Erik
