8298459: Fix msys2 linking and handling out of tree build directory for source zip creation

[RealCLanger]

This PR fixes two issues.

In ZipSource.gmk we don't account for the possibility of the build directory or $(SUPPORT_OUTPUTDIR) not being in the source directory tree ($(TOPDIR)).
It doesn't manifest in a build error though since link-file-relative will then fall back to absolute linking. But it results in awkward paths.

Furthermore, there's a lurking issue in msys2 builds on Windows. There, ln would create deep copies of a file system tree instead of linking. If the file system to be linked contains long paths, it could result in 'File name too long' errors, like can be seen here. We can avoid this by calling Windows mklink to create a file system junction. Presumably that would also help build performance (though not verified).

---

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-8298459: Fix msys2 linking and handling out of tree build directory for source zip creation

Reviewers

• Erik Joelsson (@erikj79 - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk20/pull/9.diff

[bridgekeeper]

👋 Welcome back clanger! 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]

@RealCLanger 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.

[mlbridge]

Webrevs

• 03: Full - Incremental (3a2caf38)
• 02: Full - Incremental (b90cd261)
• 01: Full - Incremental (11b6dce8)
• 00: Full (cff1a823)

[RealCLanger]

@erikj79 I would like to use these make conditionals but somehow it doesn't work out. What am I doing wrong, any idea/hint?

[erikj79]

@erikj79 I would like to use these make conditionals but somehow it doesn't work out. What am I doing wrong, any idea/hint?

Sorry, I have been away from makefiles for too long. What I suggested will not work inside a macro body. You will need to define the whole macro differently based on conditionals. Not as neat, but certainly more performant, and easier to debug when reading command lines from make.

Something like this (quick pseudo code):

ifeq (OS, msys)
  define link-file-relative
     ...
  endef
  define link-file-absolute
    ...
  endef
else
  define link-file-relative
  ...
endif

[erikj79]

Looks good, thanks!

[openjdk]

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

8298459: Fix msys2 linking and handling out of tree build directory for source zip creation

Reviewed-by: erikj

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

• a894649: 8298225: [AIX] Disable PPC64LE continuations on AIX
• 52fffdd: 8298463: tools/javac/modules/EdgeCases.java fails on Windows after JDK-8297988
• b7b996c: 8298353: C2 fails with assert(opaq->outcnt() == 1 && opaq->in(1) == limit) failed
• 05d67f6: 8298340: java/net/httpclient/CancelRequestTest.java fails with AssertionError: Found some subscribers for testPostInterrupt
• 5a92bee: 8298455: JFR: Add logging to TestClose.java
• 133ad8e: 8297988: NPE in JavacTypes.getOverriddenMethods from doclint

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[RealCLanger]

Looks good, thanks!

Thanks for the review. In this GHA run one can see some traces that it works and effectively calls mklink for MSYS but ln in the other cases.

/integrate

[openjdk]

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

• a894649: 8298225: [AIX] Disable PPC64LE continuations on AIX
• 52fffdd: 8298463: tools/javac/modules/EdgeCases.java fails on Windows after JDK-8297988
• b7b996c: 8298353: C2 fails with assert(opaq->outcnt() == 1 && opaq->in(1) == limit) failed
• 05d67f6: 8298340: java/net/httpclient/CancelRequestTest.java fails with AssertionError: Found some subscribers for testPostInterrupt
• 5a92bee: 8298455: JFR: Add logging to TestClose.java
• 133ad8e: 8297988: NPE in JavacTypes.getOverriddenMethods from doclint

Your commit was automatically rebased without conflicts.

[openjdk]

@RealCLanger Pushed as commit d624deb.

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