RFR: JDK-8001931 The new build system whitespace cleanup
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Tue Oct 8 07:42:20 UTC 2013
Based on the code conventions posted earlier, Erik and I have made a
complete overhaul on the use of whitespace and indentation in the new
build system.
We set up an important limitation on this change: *only* whitespace
should be changed. This self-imposed limitation helped us verify that
nothing else was accidentally changed.
I have verified (using basically "cat * | tr -d ' \t\n' >
all-files-without-whitespace.txt") that this change does indeed only
change whitespace.
It should be noted, that while this is a comfort, it is not a 100%
guarantee on the correctness of this change, since whitespace in
makefiles sometimes have semantic meaning.
This also means that we were not able to add line breaks, since most
line breaks would require us to add a '\' at the end, thus breaking the
whitespace-only properly. We are considering complementing this change
with an additional change without this strict limitation, that can fix
some of the remaining issues we ran into.
Unfortunately, apart from the whitespace-only guarantee, this is a hard
change to review. Webrev does not do a very good job at presenting
whitespace-only changes (a behavior shared with most diff utilities, we
learnt). Also, a lot of files are changed, even though most of the files
have only minimal changes in them.
When preparing this change, we combined manual editing with automatic
tools. All automatic changes were manually reviewed, and any bad changes
reverted.
The automatic changes performed include:
* Remove trailing whitespace
* No tabs except at start of line
* When breaking lines, always end with ' \'
* Condense multiple space in lines into one
Purely manual editing included:
* Fix indentation levels
* Add space around assignment operators, where possible
* Add space after comma in function calls, where possible
We have tested our changes on JRPT, the internal test system, and it
works well.
WebRevs:
Note: The webrevs are created with -b, to show anything meaningful at all.
root:
http://cr.openjdk.java.net/~ihse/JDK-8001931-build-infra-whitespace-cleanup/root/webrev.01
jdk:
http://cr.openjdk.java.net/~ihse/JDK-8001931-build-infra-whitespace-cleanup/jdk/webrev.01
langtools:
http://cr.openjdk.java.net/~ihse/JDK-8001931-build-infra-whitespace-cleanup/langtools/webrev.01
corba:
http://cr.openjdk.java.net/~ihse/JDK-8001931-build-infra-whitespace-cleanup/corba/webrev.01
jaxp:
http://cr.openjdk.java.net/~ihse/JDK-8001931-build-infra-whitespace-cleanup/jaxp/webrev.01
jaxws:
http://cr.openjdk.java.net/~ihse/JDK-8001931-build-infra-whitespace-cleanup/jaxws/webrev.01
Bug: https://bugs.openjdk.java.net/browse/JDK-8001931
/Magnus
More information about the build-dev
mailing list