RFR: 8149594 - Clean up Hotspot makefiles
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Thu Feb 11 00:51:51 UTC 2016
Hi Kim,
Thanks for looking at this!
Den 10/2/16 kl. 23:34, skrev Kim Barrett:
> Den 10/2/16 kl. 21:31, skrev Jesper Wilhelmsson:
>> https://bugs.openjdk.java.net/browse/JDK-8149594
>> http://cr.openjdk.java.net/~jwilhelm/8149594/webrev.00/
>
>
> ------------------------------------------------------------------------------
> I might have preferred two webrevs, one of only whitespace changes and
> one of other changes.
Yes, I'll split it up in the next version if there is a need for it.
> ------------------------------------------------------------------------------
> make/bsd/Makefile
> make/linux/Makefile
> 172 TARGETS = debug fastdebug optimized product
> ...
> 200 BUILDTREE = $(MAKE) -f $(BUILDTREE_MAKE) $(BUILDTREE_VARS)
>
> If there's a non-whitespace change (increasing the separation between
> the variables and the "=" or "+="), I couldn't find it. I'm guessing
> this is being done because the planned later changes introduce
> something in here that leads to that reformatting?
Yes, this is one of those changes that is motivated by future changes. I wanted
to separate this whitespace change from the actual change to make that one
easier to review.
>
> ------------------------------------------------------------------------------
> make/bsd/makefiles/gcc.make
> 278 WARNING_FLAGS += -Wconversion
>
> Oh, cool! So we haven't been using that option after all!
>
> Note: This is a "real change" that wasn't mentioned in the RFR.
>
> I've been meaning to file a bug report against this for a while. The
> pre-gcc4.3 version of -Wconversion probably ought not be used in a
> production context anyway.
>
> https://gcc.gnu.org/wiki/NewWconversion
> The old behavior for -Wconversion was intended to aid translation of
> old C code to modern C standards by identifying places where adding
> function prototypes may result in different behavior. That's just not
> an issue for C++, nor for our code in general.
>
> And we're not prepared to use the new -Wconversion; see JDK-8135181.
>
> So rather than changing our builds to actually use this option with
> old compilers that Oracle doesn't support (so we can't locally test
> this change), I suggest removing the option entirely, since it hasn't
> actually been used anyway.
This typo was only present on bsd. Are you suggesting to remove it only on bsd,
or on linux as well?
>
> ------------------------------------------------------------------------------
> make/bsd/makefiles/jvmti.make
> make/linux/makefiles/trace.make
>
> The only non-copyright change in these files seem to be the addition
> of a blank line to the end of the file.
Yes, this is to make the bsd and linux versions of the files the same. It makes
it easier to apply patches from one platform to the other when porting stuff.
>
> ------------------------------------------------------------------------------
> make/bsd/makefiles/top.make
> 88 vm_build_preliminaries: checks $(Cached_plat) $(AD_Files_If_Required) trace_stuff jvmti_stuff dtrace_stuff
>
> What is the point of re-ordering trace_stuff and jvmti_stuff?
To make the bsd and linux versions the same.
>
> Also, elsewhere the whitespace after the target's ":" is minimized,
> but not here.
Oops. I'll fix that.
/Jesper
>
> ------------------------------------------------------------------------------
>
More information about the build-dev
mailing list