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

Reply via email to