RFR: 8339570: Add Tidy build support for JDK tests

[Erik Joelsson]

On Fri, 4 Oct 2024 00:17:14 GMT, Nizar Benalla <nbenalla at openjdk.org> wrote:

> Can I get a review for this patch that adds the necessary changes for local support of the `tidy` library.
> 
> The dependency can be retrieved by running `make/devkit/createTidyBundle.sh` on Linux and MacOs systems.
> 
> This dependency is primarily going to be used to test the generated documentation.
> 
> This patch is meant to be integrated before #21272.
> 
> Note: we need to be a very specific revision of `tidy` and cannot use any of the available artifacts, as older versions do not recognize some HTML 5 elements. 
> 
> TIA

make/conf/jib-profiles.js line 456:

> 454:             target_os: "macosx",
> 455:             target_cpu: "aarch64",
> 456:             dependencies: ["devkit", "gtest", "graphviz", "pandoc", "tidy"],

Is there a reason for not providing Tidy on macosx-x64? It looks like the binary built by the script below would be a multi-arch variant. If so, you should deploy it as just "tidy-html-macosx" and add some code in the dependencies section below that defines "module" as just `"tidy-html" + input.target_os` on macosx.

make/conf/jib-profiles.js line 1280:

> 1278:         tidy: {
> 1279:             organization: common.organization,
> 1280:             environment_name: "TIDY",

Not sure this is a good idea. This will set an environment variable pointing to the "home_path" of the installation and will conflict with the configure arg below. The configure arg will win, so it will still work, but it could be confusing. A better environment variable name would be something like `TIDY_HOME`, but unless this is needed for something, I would just skip defining one.

make/conf/jib-profiles.js line 1284:

> 1282:             revision: "5.9.20+1",
> 1283:             environment_path: input.get("tidy", "home_path") + "/tidy/bin/tidy",
> 1284:             configure_args: "TIDY=" + input.get("tidy", "home_path") +"/bin/tidy",

These paths are different. I'm guessing the latter one is corret?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21341#discussion_r1790300323
PR Review Comment: https://git.openjdk.org/jdk/pull/21341#discussion_r1790306484
PR Review Comment: https://git.openjdk.org/jdk/pull/21341#discussion_r1790301745