jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8328074: Add jcheck whitespace checking for assembly files

Open magicus opened this issue 1 year ago • 11 comments
trafficstars

As part of the ongoing effort to enable jcheck whitespace checking to all text files, it is now time to address assembly (.S) files. The hotspot assembly files were fixed as part of the hotspot mapfile removal, so only a single incorrect jdk library remains.

The main issue with libjsvml was inconsistent line starts, sometimes using tabs. I used the expand unix command line tool to replace these with spaces.


Progress

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

Issue

  • JDK-8328074: Add jcheck whitespace checking for assembly files (Enhancement - P4)

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 18268

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

Using diff file

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

Webrev

Link to Webrev Comment

magicus avatar Mar 13 '24 11:03 magicus

:wave: Welcome back ihse! 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.

bridgekeeper[bot] avatar Mar 13 '24 11:03 bridgekeeper[bot]

@magicus The following label will be automatically applied to this pull request:

  • build

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

openjdk[bot] avatar Mar 13 '24 11:03 openjdk[bot]

Webrevs

  • 03: [Full](Webrev is not available because diff is too large) - Incremental (4e729cb6)
  • 02: [Full](Webrev is not available because diff is too large) - Incremental (ce1d4c9f)
  • 01: [Full](Webrev is not available because diff is too large) - Incremental (49fb1016)
  • 00: [Full](Webrev is not available because diff is too large) (ce1d4c9f)

mlbridge[bot] avatar Mar 13 '24 11:03 mlbridge[bot]

(Apologies for the force push; I mistakenly added commits to this PR that was intended to go on another branch)

magicus avatar Mar 13 '24 11:03 magicus

@magicus Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

openjdk[bot] avatar Mar 13 '24 11:03 openjdk[bot]

/label core-libs

magicus avatar Mar 13 '24 11:03 magicus

@magicus The core-libs label was successfully added.

openjdk[bot] avatar Mar 13 '24 11:03 openjdk[bot]

I tagged this core-libs as that is what @sviswa7 did for changes to libjsvml in https://github.com/openjdk/jdk/pull/8508. I hope this is correct. (We should really add rules to Skara for this library.)

magicus avatar Mar 13 '24 11:03 magicus

@magicus 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:

8328074: Add jcheck whitespace checking for assembly files

Reviewed-by: erikj, sviswanathan

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 77 new commits pushed to the master branch:

  • 0204aacb0305e94a7d6d5299a5ae835f3f71b030: 8328115: Convert java/awt/font/TextLayout/TestJustification.html applet test to main
  • 9bc1b065db238b7c9d0562f9bd55d2f338c6ff3d: 8328242: Add a log area to the PassFailJFrame
  • 65a84c2642822862fa186f290f8d6e83537bea06: 8328006: refactor large anonymous inner class in HtmlDocletWriter
  • 044f4ed55dfce7f1aed9e10accf459b4af9b975e: 8326979: (jdeps) improve the error message for FindException caused by InvalidModuleDescriptorException
  • 71f9c4e7fd5818be6e79dead3caa02d738456ad6: 8320534: fatal error for the NMTBenchmark test run for the mainline build
  • bfba4894e14c9547735a6b4ae73ed423af09f0d4: 8328157: Move C[XX]FLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk
  • 42aec8c5e72837400818d931855bade8acad8e9d: 8328146: Set LIBCXX automatically
  • 90ab52e598f74ed872fe404130d57f6596430727: 8328166: Epsilon: 'EpsilonHeap::allocate_work' misuses the parameter 'size' as size in bytes
  • bf990c8b32f1ce050450d94f4f8706e5a01aa3ec: 8328269: NonFocusablePopupMenuTest.java should be marked as headful
  • 80b98c86b82eb9ffd5aee67a92daa31c8b649cc6: 8320503: Print warning if VM reaches MallocLimit during error reporting
  • ... and 67 more: https://git.openjdk.org/jdk/compare/49d8008947534898e2ba36e2d81d87d6ae4b4c02...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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk[bot] avatar Mar 13 '24 13:03 openjdk[bot]

Maybe someone in Hotspot wants to take a look? /label +hotspot

magicus avatar Mar 15 '24 14:03 magicus

@magicus The hotspot label was successfully added.

openjdk[bot] avatar Mar 15 '24 14:03 openjdk[bot]

Thanks @sviswa7!

/integrate

magicus avatar Mar 18 '24 09:03 magicus

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

  • 3c70f26b2f3fa9bc143e2506af30f9b1daf20022: 8328112: Remove CardTable::_guard_region
  • 48717d63cc58f693f0917e61eafd672cd6af02ed: 8326333: jshell <TAB> completion on arrays is incomplete
  • ece4124f25f676da9bf2d1b7fd8e4394dd7d31af: 8328247: Remove redundant dir for tests converted from applet to main
  • d32ce65781c1d7815a69ceac720cdf3ae39caa9e: 8327651: Rename DictionaryEntry members related to protection domain
  • 07194195cefc568048fa639b6f8534ce3718c8d2: 8328236: module_entry in CDS map file has stale value
  • 0204aacb0305e94a7d6d5299a5ae835f3f71b030: 8328115: Convert java/awt/font/TextLayout/TestJustification.html applet test to main
  • 9bc1b065db238b7c9d0562f9bd55d2f338c6ff3d: 8328242: Add a log area to the PassFailJFrame
  • 65a84c2642822862fa186f290f8d6e83537bea06: 8328006: refactor large anonymous inner class in HtmlDocletWriter
  • 044f4ed55dfce7f1aed9e10accf459b4af9b975e: 8326979: (jdeps) improve the error message for FindException caused by InvalidModuleDescriptorException
  • 71f9c4e7fd5818be6e79dead3caa02d738456ad6: 8320534: fatal error for the NMTBenchmark test run for the mainline build
  • ... and 72 more: https://git.openjdk.org/jdk/compare/49d8008947534898e2ba36e2d81d87d6ae4b4c02...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Mar 18 '24 09:03 openjdk[bot]

@magicus Pushed as commit c342188fd978dd94e7788fb0fb0345fd8c0eaa9a.

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

openjdk[bot] avatar Mar 18 '24 09:03 openjdk[bot]