8306408: Fix the format of several tables in building.md #2205

zzambers · 2024-02-12T18:58:12Z

Backport fixing several tables in doc/building.md, so they would display correctly in GitHub. Change set applied almost cleanly to doc/building.md (only last table has one more target in original changeset). File doc/building.html was regenerated.

Progress

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

Issue

JDK-8306408: Fix the format of several tables in building.md (Enhancement - P4 - Approved)

Reviewers

Andrew John Hughes (@gnu-andrew - Reviewer) ⚠️ Review applies to 38336330

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/2205/head:pull/2205
$ git checkout pull/2205

Update a local copy of the PR:
$ git checkout pull/2205
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/2205/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2205

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/2205.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-02-12T18:58:34Z

👋 Welcome back zzambers! 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-02-12T19:00:19Z

This backport pull request has now been updated with issue from the original commit.

mlbridge · 2024-02-12T19:04:18Z

Webrevs

01: Full - Incremental (ed7aaa16)
00: Full (38336330)

gnu-andrew

building.md changes look fine.
How was the HTML re-generated? I'm seeing some unrelated changes which I guess may be down to the tool version used?

+@@ -11,14 +11,14 @@
+       span.underline{text-decoration: underline;}
+       div.column{display: inline-block; vertical-align: top; width: 50%;}
+   </style>
+-  <link rel="stylesheet" href="../make/data/docs-resources/resources/jdk-default.css" />
++  <link rel="stylesheet" href="../make/data/docs-resources/resources/jdk-default.css">
+   <!--[if lt IE 9]>
+     <script src="//cdnjs.cloudflare.com/ajax/libs/html5shiv/3.7.3/html5shiv-printshiv.min.js"></script>
+   <![endif]-->
+   <style type="text/css">pre, code, tt { color: #1d6ae5; }</style>
+ </head>
+ <body>
+-<header id="title-block-header">
++<header>
+ <h1 class="title">Building the JDK</h1>
+ </header>
+ <nav id="TOC">

zzambers · 2024-02-19T11:53:12Z

@gnu-andrew I have used appropriate makefile target on rhel-8 (pandoc 2.0.6)

zzambers · 2024-02-19T11:57:02Z

I can try with different version of pandoc.

zzambers · 2024-02-19T14:16:56Z

@gnu-andrew I have tried on few more systems, but pandoc from rhel-8 is closest to orignal generator I could get.

output of newer pandoc on f38 (2.19.2-21.fc38) or even ubuntu-20.04 (2.5-3build2) were much more different
output of older pandoc on rhel-7 (1.12.3.1-2.el7) was also much more different

Unfortunately generated html does not include information about pandoc used. Rhel-8's pandoc (2.0.6-6.el8_6) seems pretty close.

gnu-andrew · 2024-02-19T16:33:33Z

Thanks for looking into this, @zzambers. Yes, this is the nightmare any time we have checked-in generated files. I'm mulling over backporting the change to remove the generated configure in the next cycle, as that keeps being altered with unrelated changes when people using a different autoconf.

I'm happy for this to go in as is. It would be nice to have known what was used to generate the original, but I imagine it was close to what you have on RHEL 8.

gnu-andrew

Good to go.

openjdk · 2024-02-19T16:34:57Z

⚠️ @zzambers This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

zzambers · 2024-02-19T18:18:06Z

/approval request Fixes format of tables in doc/building.md, so they display correctly on GitHub. (docs only change)

openjdk · 2024-02-19T18:18:39Z

@zzambers
8306408: The approval request has been created successfully.

zzambers · 2024-02-19T18:54:24Z

Thanks for looking into this, @zzambers. Yes, this is the nightmare any time we have checked-in generated files. I'm mulling over backporting the change to remove the generated configure in the next cycle, as that keeps being altered with unrelated changes when people using a different autoconf.

I'm happy for this to go in as is. It would be nice to have known what was used to generate the original, but I imagine it was close to what you have on RHEL 8.

Based on my experiments and release info, pandoc used was >=2.2.1 and <2.5 (unless it was patched), since:

it has:
Make HTML template polyglot ([#4606](https://github.com/jgm/pandoc/issues/4606), OvidiusCicero), by making <link rel="stylesheet" href="$css$"> self-closing.

but does not have:
Text.Pandoc.Writers.HTML: Use plain " instead of " outside of attributes.

zzambers · 2024-02-19T19:35:15Z

I thought maybe debian buster (oldoldstable), but no. This one has <!--[if lt IE 9]> ... stuff missing (probably custom patch/backport, as even newer pandoc on f38 has this).

Output of pandoc on RHEL-8 seems closest (of all current pandoc packages on any version of RHEL, Debian, Ubuntu and supported Fedora).

gnu-andrew · 2024-02-19T23:33:44Z

I thought maybe debian buster (oldoldstable), but no. This one has <!--[if lt IE 9]> ... stuff missing (probably custom patch/backport, as even newer pandoc on f38 has this).

Output of pandoc on RHEL-8 seems closest (of all current pandoc packages on any version of RHEL, Debian, Ubuntu and supported Fedora).

Yes it may not even be a packaged version but a local install. Thanks for investigating so thoroughly.

gnu-andrew · 2024-02-19T23:33:55Z

/approve yes

openjdk · 2024-02-19T23:34:50Z

@gnu-andrew
8306408: The approval request has been approved.

openjdk · 2024-02-19T23:35:41Z

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

8306408: Fix the format of several tables in building.md

Reviewed-by: andrew

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

3285292: 8308336: Test java/net/HttpURLConnection/HttpURLConnectionExpectContinueTest.java failed: java.net.BindException: Address already in use
b27d63f: 8318468: compiler/tiered/LevelTransitionTest.java fails with -XX:CompileThreshold=100 -XX:TieredStopAtLevel=1
f64f07f: 8315602: Open source swing security manager test
d6ad2f1: 8274122: java/io/File/createTempFile/SpecialTempFile.java fails in Windows 11
ed7c6f4: 8324347: Enable "maybe-uninitialized" warning for FreeType 2.13.1
4395668: 8305906: HttpClient may use incorrect key when finding pooled HTTP/2 connection for IPv6 address
200c2a0: 8290203: ProblemList vmTestbase/nsk/jvmti/scenarios/capability/CM03/cm03t001/TestDescription.java on linux-all
d9b412e: 8318603: Parallelize sun/java2d/marlin/ClipShapeTest.java
a234b7c: 8324514: ClassLoaderData::print_on should print address of class loader
77b15bb: 8322098: os::Linux::print_system_memory_info enhance the THP output with /sys/kernel/mm/transparent_hugepage/hpage_pmd_size
... and 1 more: https://git.openjdk.org/jdk17u-dev/compare/e0c7d6298544cbc0bcb5f7f2004b234f41e46693...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 (@gnu-andrew) 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).

zzambers · 2024-02-20T11:21:45Z

@gnu-andrew thanks
/integrate

openjdk · 2024-02-20T11:23:00Z

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

zzambers · 2024-02-20T12:20:33Z

Oh, when moving on to jdk11u-dev, I have found there is a script for creating pandoc bundle in devkit. This seems to be the right pandoc. I'll try to update html as changeset is not yet in (as I am not commiter here). Hopefully bot will not be too angry about that.. :)

zzambers · 2024-02-20T15:16:22Z

/integrate

openjdk · 2024-02-20T15:17:30Z

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

gnu-andrew · 2024-02-21T00:24:00Z

/sponsor

openjdk · 2024-02-21T00:24:59Z

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

3285292: 8308336: Test java/net/HttpURLConnection/HttpURLConnectionExpectContinueTest.java failed: java.net.BindException: Address already in use
b27d63f: 8318468: compiler/tiered/LevelTransitionTest.java fails with -XX:CompileThreshold=100 -XX:TieredStopAtLevel=1
f64f07f: 8315602: Open source swing security manager test
d6ad2f1: 8274122: java/io/File/createTempFile/SpecialTempFile.java fails in Windows 11
ed7c6f4: 8324347: Enable "maybe-uninitialized" warning for FreeType 2.13.1
4395668: 8305906: HttpClient may use incorrect key when finding pooled HTTP/2 connection for IPv6 address
200c2a0: 8290203: ProblemList vmTestbase/nsk/jvmti/scenarios/capability/CM03/cm03t001/TestDescription.java on linux-all
d9b412e: 8318603: Parallelize sun/java2d/marlin/ClipShapeTest.java
a234b7c: 8324514: ClassLoaderData::print_on should print address of class loader
77b15bb: 8322098: os::Linux::print_system_memory_info enhance the THP output with /sys/kernel/mm/transparent_hugepage/hpage_pmd_size
... and 1 more: https://git.openjdk.org/jdk17u-dev/compare/e0c7d6298544cbc0bcb5f7f2004b234f41e46693...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-02-21T00:25:07Z

@gnu-andrew @zzambers Pushed as commit ef598eb.

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

gnu-andrew · 2024-02-21T00:27:08Z

Oh, when moving on to jdk11u-dev, I have found there is a script for creating pandoc bundle in devkit. This seems to be the right pandoc. I'll try to update html as changeset is not yet in (as I am not commiter here). Hopefully bot will not be too angry about that.. :)

Oops, sorry I sponsored it before I saw this. I don't think it's too much of an issue. At least we now know the 'correct' way (though it's hardly obvious!)

Backport 26755a968665545a151adce79a5227c79724bb6b

3833633

openjdk bot changed the title ~~Backport 26755a968665545a151adce79a5227c79724bb6b~~ 8306408: Fix the format of several tables in building.md Feb 12, 2024

openjdk bot added backport rfr Pull request is ready for review labels Feb 12, 2024

gnu-andrew reviewed Feb 16, 2024

View reviewed changes

gnu-andrew approved these changes Feb 19, 2024

View reviewed changes

openjdk bot added the approval label Feb 19, 2024

openjdk bot added ready Pull request is ready to be integrated and removed approval labels Feb 19, 2024

openjdk bot added the sponsor Pull request is ready to be sponsored label Feb 20, 2024

Regenerated doc/building.html using pandoc created by devkit

ed7aaa1

openjdk bot removed the sponsor Pull request is ready to be sponsored label Feb 20, 2024

openjdk bot added the sponsor Pull request is ready to be sponsored label Feb 20, 2024

openjdk bot added the integrated Pull request has been integrated label Feb 21, 2024

openjdk bot closed this Feb 21, 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 Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8306408: Fix the format of several tables in building.md #2205

8306408: Fix the format of several tables in building.md #2205

zzambers commented Feb 12, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Feb 12, 2024

openjdk bot commented Feb 12, 2024

mlbridge bot commented Feb 12, 2024 •

edited

Loading

gnu-andrew left a comment

zzambers commented Feb 19, 2024

zzambers commented Feb 19, 2024

zzambers commented Feb 19, 2024

gnu-andrew commented Feb 19, 2024

gnu-andrew left a comment

openjdk bot commented Feb 19, 2024

zzambers commented Feb 19, 2024

openjdk bot commented Feb 19, 2024

zzambers commented Feb 19, 2024 •

edited

Loading

zzambers commented Feb 19, 2024

gnu-andrew commented Feb 19, 2024

gnu-andrew commented Feb 19, 2024

openjdk bot commented Feb 19, 2024

openjdk bot commented Feb 19, 2024

zzambers commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

zzambers commented Feb 20, 2024 •

edited

Loading

zzambers commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

gnu-andrew commented Feb 21, 2024

openjdk bot commented Feb 21, 2024

openjdk bot commented Feb 21, 2024

gnu-andrew commented Feb 21, 2024

8306408: Fix the format of several tables in building.md #2205

8306408: Fix the format of several tables in building.md #2205

Conversation

zzambers commented Feb 12, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Feb 12, 2024

openjdk bot commented Feb 12, 2024

mlbridge bot commented Feb 12, 2024 • edited Loading

Webrevs

gnu-andrew left a comment

Choose a reason for hiding this comment

zzambers commented Feb 19, 2024

zzambers commented Feb 19, 2024

zzambers commented Feb 19, 2024

gnu-andrew commented Feb 19, 2024

gnu-andrew left a comment

Choose a reason for hiding this comment

openjdk bot commented Feb 19, 2024

zzambers commented Feb 19, 2024

openjdk bot commented Feb 19, 2024

zzambers commented Feb 19, 2024 • edited Loading

zzambers commented Feb 19, 2024

gnu-andrew commented Feb 19, 2024

gnu-andrew commented Feb 19, 2024

openjdk bot commented Feb 19, 2024

openjdk bot commented Feb 19, 2024

zzambers commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

zzambers commented Feb 20, 2024 • edited Loading

zzambers commented Feb 20, 2024

openjdk bot commented Feb 20, 2024

gnu-andrew commented Feb 21, 2024

openjdk bot commented Feb 21, 2024

openjdk bot commented Feb 21, 2024

gnu-andrew commented Feb 21, 2024

zzambers commented Feb 12, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Feb 12, 2024 •

edited

Loading

zzambers commented Feb 19, 2024 •

edited

Loading

zzambers commented Feb 20, 2024 •

edited

Loading