RFR: JDK-8071329: Stop exporting INCLUDE and LIB when building on windows

[Erik Joelsson]

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 <erik.joelsson at oracle.com 
> <mailto:erik.joelsson at oracle.com>> 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
>
>