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