8333477: Delete extra empty spaces in Makefiles #19537

sendaoYan · 2024-06-04T07:47:46Z

Hi all,
This PR several extra empty spaces and extra empty lines in several Makefiles. It's trivial fix, no risk.

Thanks.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8333477: Delete extra empty spaces in Makefiles (Bug - P5)

Reviewers

Erik Joelsson (@erikj79 - Reviewer)
Christian Hagedorn (@chhagedorn - Reviewer) ⚠️ Review applies to 0d2be363
Chen Liang (@liach - Author)
Julian Waters (@TheShermanTanker - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19537/head:pull/19537
$ git checkout pull/19537

Update a local copy of the PR:
$ git checkout pull/19537
$ git pull https://git.openjdk.org/jdk.git pull/19537/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19537

View PR using the GUI difftool:
$ git pr show -t 19537

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19537.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-06-04T07:48:08Z

👋 Welcome back syan! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-06-04T07:48:21Z

@sendaoYan This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8333477: Delete extra empty spaces in Makefiles

Reviewed-by: erikj, chagedorn, liach, jwaters

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 76 new commits pushed to the master branch:

486dee2: 8333653: Remove MallocHeader::get_stack
40b2fbd: 8331733: [PPC64] saving and restoring CR is not needed at most places
6968770: 8331935: Add support for primitive array C1 clone intrinsic in PPC
a2030ff: 8332516: Serial: Always sample promoted bytes to avoid getting stuck in Full GCs
bf7f1c4: 8333211: NMT Reports: replace manual indentation handling with auto indent
8ffc35d: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
f7862bd: 8331311: C2: Big Endian Port of 8318446: optimize stores into primitive arrays by combining values into larger store
b4beda2: 8332537: C2: High memory usage reported for compiler/loopopts/superword/TestAlignVectorFuzzer.java
e5383d7: 8333713: C2 SuperWord: cleanup in vectornode.cpp/hpp
944aeb8: 8325155: C2 SuperWord: remove alignment boundaries
... and 66 more: https://git.openjdk.org/jdk/compare/454660d361e39f362ff0e10a5c2389af910cca23...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@erikj79, @chhagedorn, @TheShermanTanker) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk · 2024-06-04T07:48:44Z

@sendaoYan The following labels will be automatically applied to this pull request:

core-libs
hotspot-compiler
security
serviceability

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

mlbridge · 2024-06-04T07:51:35Z

Webrevs

01: Full - Incremental (e80b98da)
00: Full (0d2be363)

erikj79 · 2024-06-04T12:54:32Z

/label build

sendaoYan · 2024-06-04T12:55:42Z

/label build

Thanks.

openjdk · 2024-06-04T12:55:59Z

@erikj79
The build label was successfully added.

sendaoYan · 2024-06-04T13:21:04Z

Thanks for the review.
/integrate

chhagedorn

Looks good! Somehow the integrate command did not work.

sendaoYan · 2024-06-04T14:40:04Z

Thanks all for the review.
/integrate

openjdk · 2024-06-04T16:45:10Z

@sendaoYan
Your change (at version 0d2be36) is now ready to be sponsored by a Committer.

openjdk · 2024-06-04T16:50:57Z

@sendaoYan
Your change (at version 0d2be36) is now ready to be sponsored by a Committer.

liach · 2024-06-05T00:52:00Z

@altrisi As trivial and low-effort as this seems, this is actually fixing some technical debt for legacy Makefiles last changed before 8e7a855 when trailing whitespace checks applied to Makefiles.

test/jdk/java/rmi/reliability/benchmark/bench/rmi/Makefile

…ty/benchmark/bench/Makefile

liach

Thank you for the fix.

sendaoYan · 2024-06-07T12:28:23Z

Thanks all for the review.
/integrate

openjdk · 2024-06-07T12:29:45Z

@sendaoYan
Your change (at version e80b98d) is now ready to be sponsored by a Committer.

TheShermanTanker · 2024-06-07T12:37:48Z

test/jdk/java/rmi/reliability/benchmark/bench/Makefile

@@ -47,4 +47,3 @@ all: .classes

 clean:
 	rm -f *.class .classes
-


Hmm, shouldn't this newline at EOF be kept? Asking @ all the people who've reviewed this so far, no need to change it just yet

No, it's an extra newline. A file should end with a newline but one is enough.

As confusing as they are, unfortunately GitHub UI does not render extra trailing newlines. This is the only one I could find with grepWin.

I find the extra trailing newlines through below shell command:

for i in `find . -iname "Makefile*" | sed "/\.\/build/d"` ; do tail -n 2 $i | grep -c "^$" | grep -q "^1$" ; if [[ 0 -eq $? ]] ; then echo $i ; fi ; done

There are only two files has been found:

./test/jdk/java/rmi/reliability/benchmark/bench/rmi/Makefile ./test/jdk/java/rmi/reliability/benchmark/bench/Makefile

Ah, I had not realized that there was more than 1 newline. GitHub's UI confused me here, so we're good to go

GitHub's UI assumes the final line has an line break. If it is missing, it displays a red 🚫 at the end of the last line. If there is an empty line showing up in the UI, then it is an additional empty line.

In this case, GitHub is actually not showing up extra empty lines, that they appear the same as the single line break in the end of the file.

TheShermanTanker · 2024-06-07T13:34:53Z

/sponsor

openjdk · 2024-06-07T13:36:07Z

Going to push as commit d130d2f.
Since your change was applied there have been 76 commits pushed to the master branch:

486dee2: 8333653: Remove MallocHeader::get_stack
40b2fbd: 8331733: [PPC64] saving and restoring CR is not needed at most places
6968770: 8331935: Add support for primitive array C1 clone intrinsic in PPC
a2030ff: 8332516: Serial: Always sample promoted bytes to avoid getting stuck in Full GCs
bf7f1c4: 8333211: NMT Reports: replace manual indentation handling with auto indent
8ffc35d: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
f7862bd: 8331311: C2: Big Endian Port of 8318446: optimize stores into primitive arrays by combining values into larger store
b4beda2: 8332537: C2: High memory usage reported for compiler/loopopts/superword/TestAlignVectorFuzzer.java
e5383d7: 8333713: C2 SuperWord: cleanup in vectornode.cpp/hpp
944aeb8: 8325155: C2 SuperWord: remove alignment boundaries
... and 66 more: https://git.openjdk.org/jdk/compare/454660d361e39f362ff0e10a5c2389af910cca23...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-06-07T13:36:14Z

@TheShermanTanker @sendaoYan Pushed as commit d130d2f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

sendaoYan · 2024-06-07T14:14:32Z

Thanks all for the review and sponsor.

8333477: Delete extra empty spaces in Makefiles

0d2be36

openjdk bot added the rfr Pull request is ready for review label Jun 4, 2024

openjdk bot added serviceability serviceability-dev@openjdk.org security security-dev@openjdk.org hotspot-compiler hotspot-compiler-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Jun 4, 2024

openjdk bot added the build build-dev@openjdk.org label Jun 4, 2024

erikj79 approved these changes Jun 4, 2024

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jun 4, 2024

chhagedorn approved these changes Jun 4, 2024

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Jun 4, 2024

liach suggested changes Jun 6, 2024

View reviewed changes

test/jdk/java/rmi/reliability/benchmark/bench/rmi/Makefile Show resolved Hide resolved

delete extra empty trailing blank line in test/jdk/java/rmi/reliabili…

e80b98d

…ty/benchmark/bench/Makefile

openjdk bot removed the sponsor Pull request is ready to be sponsored label Jun 7, 2024

liach approved these changes Jun 7, 2024

View reviewed changes

openjdk bot added the sponsor Pull request is ready to be sponsored label Jun 7, 2024

TheShermanTanker reviewed Jun 7, 2024

View reviewed changes

TheShermanTanker approved these changes Jun 7, 2024

View reviewed changes

erikj79 approved these changes Jun 7, 2024

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jun 7, 2024

openjdk bot closed this Jun 7, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jun 7, 2024

sendaoYan deleted the jbs8333477 branch June 7, 2024 14:13

8333477: Delete extra empty spaces in Makefiles #19537

8333477: Delete extra empty spaces in Makefiles #19537

Uh oh!

Conversation

sendaoYan commented Jun 4, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Jun 4, 2024

Uh oh!

openjdk bot commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Jun 4, 2024

Uh oh!

mlbridge bot commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

erikj79 commented Jun 4, 2024

Uh oh!

sendaoYan commented Jun 4, 2024

Uh oh!

openjdk bot commented Jun 4, 2024

Uh oh!

sendaoYan commented Jun 4, 2024

Uh oh!

chhagedorn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sendaoYan commented Jun 4, 2024

Uh oh!

openjdk bot commented Jun 4, 2024

Uh oh!

openjdk bot commented Jun 4, 2024

Uh oh!

liach commented Jun 5, 2024

Uh oh!

Uh oh!

liach left a comment

Choose a reason for hiding this comment

Uh oh!

sendaoYan commented Jun 7, 2024

Uh oh!

openjdk bot commented Jun 7, 2024

Uh oh!

TheShermanTanker Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

erikj79 Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

liach Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

sendaoYan Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

TheShermanTanker Jun 7, 2024

Choose a reason for hiding this comment

Uh oh!

magicus Jun 14, 2024

Choose a reason for hiding this comment

Uh oh!

liach Jun 14, 2024

Choose a reason for hiding this comment

Uh oh!

TheShermanTanker commented Jun 7, 2024

Uh oh!

openjdk bot commented Jun 7, 2024

Uh oh!

openjdk bot commented Jun 7, 2024

Uh oh!

sendaoYan commented Jun 7, 2024

Uh oh!

sendaoYan commented Jun 4, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Jun 4, 2024 •

edited

Loading

mlbridge bot commented Jun 4, 2024 •

edited

Loading

chhagedorn left a comment •

edited

Loading