8281262: Windows builds in different directories are not fully reproducible #7344

mkartashev · 2022-02-04T10:55:40Z

Some dll/exe files end up having absolute path names embedded in them despite the use of --disable-absolute-paths-in-output build option. This option effectively translates into adding -pathmap to compilation lines, but doesn't (always) achieve the desired effects. The reason for that is in the use of Windows-style path for the argument provided to pathmap. The slash characters in the path passed as an argument get removed by the fixpath script that pre-processes all commands on Windows prior to running them and is supposed to convert Unix-style paths to what is understood by Windows.

For example:

$ build/windows-x86_64-server-release/fixpath print -pathmap:C:\cygwin64\home\maxim\work\repr.build.1=s
-pathmap:C:cygwin64homemaximworkrepr.build.1=s

However, if a "normal" Unix-style path is provided, it gets converted correctly:

$ build/windows-x86_64-server-release/fixpath print -pathmap:/home/maxim/work/repr.build.1=s
-pathmap:C:\cygwin64\home\maxim\work\repr.build.1=s

This commit changes the -pathmap compiler option to use "normal", Unix-style paths on Windows. Those will be changed to Windows style by the fixpath script.

Verified by running two builds (details below) in different directories on the same machine; with this commit, all the exe/dll files under .../images/jdk/... are the same.

Builds were produced by running the following commands on Windows 10 x64 with Visual Studio 2019 installed:

 ./configure --with-debug-level=release --with-jvm-features=shenandoahgc --with-version-pre= --with-version-build=1 --with-version-opt=b42 --with-toolchain-version=2019 --disable-ccache --enable-cds=yes --enable-reproducible-build --with-source-date=1643953637 --with-hotspot-build-time=2022-02-04 --with-copyright-year=2022 --disable-absolute-paths-in-output --with-boot-jdk=/home/maxim/work/zulu17.0.81-ea-jdk17.0.0-ea.35-win_x64
make reconfigure clean
make images

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8281262: Windows builds in different directories are not fully reproducible

Reviewers

Erik Joelsson (@erikj79 - Reviewer) ⚠️ Review applies to c66ea18
Magnus Ihse Bursie (@magicus - Reviewer) ⚠️ Review applies to c66ea18

Contributors

Erik Joelsson <erikj@openjdk.org>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7344/head:pull/7344
$ git checkout pull/7344

Update a local copy of the PR:
$ git checkout pull/7344
$ git pull https://git.openjdk.java.net/jdk pull/7344/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 7344

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7344.diff

…ucible Changed -pathmap compiler option to use "normal", Unix-style paths on Windows. Those will be changed to Windows style by the fixpath script. But if Windows-style paths are used from the start, this script will wreck them by removing all the backslash characters.

bridgekeeper · 2022-02-04T10:56:51Z

👋 Welcome back mkartashev! 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 · 2022-02-04T11:00:17Z

@mkartashev 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 · 2022-02-04T11:03:35Z

Webrevs

erikj79 · 2022-02-04T14:57:04Z

Thank you for finding this! We do have a test that is supposed to catch an issue like this, but unfortunately it seems that test is suffering a similar issue. It's not looking for the correct path strings as the backslashes have been truncated by one too many shell expansions on the way. I created a fix for the test in a branch here:

https://github.com/erikj79/jdk/tree/JDK-8281262-abspath-windows

I've verified that your fix makes the test pass together with my fix. Something like this should go in with this fix. Feel free to take my change.

mkartashev · 2022-02-04T15:35:27Z

@erikj79 Thank you! I added another commit (under your name) with your changes to this pull request.

erikj79 · 2022-02-04T17:22:47Z

Thanks for incorporating my change. After thinking about this some more, I figured we could add some form of validation of the root paths that would actually catch the build problem as well. I added another change to the same branch with this. The test now makes sure the root paths can be parsed as paths and are absolute. This validation fails on Windows without my previous patch, causing the test to fail.

erikj79

I think this looks good, but would be good if Magnus could take a look as well.

mkartashev · 2022-02-07T06:05:44Z

would be good if Magnus could take a look as well.

Now that a good part of the PR is authored by you, jcheck wouldn't have it any other way :-)

mkartashev · 2022-02-08T07:53:05Z

@magicus Could you please take a look?

magicus

Looks good. Thanks for catching this!

openjdk · 2022-02-10T09:17:39Z

⚠️ @mkartashev the full name on your profile does not match the author name in this pull requests' HEAD commit. If this pull request gets integrated then the author name from this pull requests' HEAD commit will be used for the resulting commit. If you wish to push a new commit with a different author name, then please run the following commands in a local repository of your personal fork:

$ git checkout JDK-8281262
$ git commit -c user.name='Preferred Full Name' --allow-empty -m 'Update full name'
$ git push

openjdk · 2022-02-10T09:17:41Z

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

8281262: Windows builds in different directories are not fully reproducible

Co-authored-by: Erik Joelsson <erikj@openjdk.org>
Reviewed-by: erikj, ihse

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

fa0a72c: 8252496: C2: Useless code in MergeMemNode::Ideal
7218d84: 8281567: Remove @throws IOException from X509CRLImpl::getExtension docs
fd8a3dc: 8280820: Clean up bug8033699 and bug8075609.java tests: regtesthelpers aren't used
178b962: 8265765: DomainKeyStore may stop enumerating aliases if a constituting KeyStore is empty
c5c8c06: 8279822: CI: Constant pool entries in error state are not supported
f823bed: 8280007: Enable Neoverse N1 optimizations for Arm Neoverse V1 & N2
8b384b9: 8281470: tools/jar/CreateMissingParentDirectories.java fails with "Should have failed creating jar file"
bb2e10c: 8281274: deal with ActiveProcessorCount in os::Linux::print_container_info
69e390a: 8262721: Add Tests to verify single iteration loops are properly optimized
f092bab: 8281195: Mistakenly used logging causes significant overhead in interpreter
... and 47 more: https://git.openjdk.java.net/jdk/compare/e44dc638b8936b1b76ca9ddf9ece0c5c4705a19c...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, @magicus) 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).

mkartashev · 2022-02-10T10:25:03Z

/contributor @erikj79

openjdk · 2022-02-10T10:25:38Z

@mkartashev Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

/contributor add @openjdk-bot
/contributor add duke
/contributor add J. Duke <duke@openjdk.org>

mkartashev · 2022-02-10T10:27:00Z

/contributor add @erikj79

mkartashev · 2022-02-10T10:27:32Z

/integrate

openjdk · 2022-02-10T10:29:14Z

@mkartashev
Contributor Erik Joelsson <erikj@openjdk.org> successfully added.

openjdk · 2022-02-10T10:30:54Z

@mkartashev
Your change (at version 7c6dc56) is now ready to be sponsored by a Committer.

magicus · 2022-02-10T10:44:57Z

/sponsor

openjdk · 2022-02-10T10:46:40Z

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

c820d1a: 8281379: Assign package declarations to all jtreg test cases under gc
fa0a72c: 8252496: C2: Useless code in MergeMemNode::Ideal
7218d84: 8281567: Remove @throws IOException from X509CRLImpl::getExtension docs
fd8a3dc: 8280820: Clean up bug8033699 and bug8075609.java tests: regtesthelpers aren't used
178b962: 8265765: DomainKeyStore may stop enumerating aliases if a constituting KeyStore is empty
c5c8c06: 8279822: CI: Constant pool entries in error state are not supported
f823bed: 8280007: Enable Neoverse N1 optimizations for Arm Neoverse V1 & N2
8b384b9: 8281470: tools/jar/CreateMissingParentDirectories.java fails with "Should have failed creating jar file"
bb2e10c: 8281274: deal with ActiveProcessorCount in os::Linux::print_container_info
69e390a: 8262721: Add Tests to verify single iteration loops are properly optimized
... and 48 more: https://git.openjdk.java.net/jdk/compare/e44dc638b8936b1b76ca9ddf9ece0c5c4705a19c...master

Your commit was automatically rebased without conflicts.

openjdk · 2022-02-10T10:46:59Z

@magicus @mkartashev Pushed as commit d442328.

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

openjdk bot added the rfr Pull request is ready for review label Feb 4, 2022

openjdk bot added the build build-dev@openjdk.org label Feb 4, 2022

Fixed the test that was supposed to catch this kind of problem.

2f99ca5

Added validation of root paths.

c66ea18

erikj79 approved these changes Feb 4, 2022

View reviewed changes

openjdk bot removed the rfr Pull request is ready for review label Feb 4, 2022

magicus approved these changes Feb 10, 2022

View reviewed changes

openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Feb 10, 2022

Update full name

7c6dc56

openjdk bot added the sponsor Pull request is ready to be sponsored label Feb 10, 2022

openjdk bot added the integrated Pull request has been integrated label Feb 10, 2022

openjdk bot closed this Feb 10, 2022

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 10, 2022

mkartashev deleted the JDK-8281262 branch February 10, 2022 12:12

8281262: Windows builds in different directories are not fully reproducible #7344

8281262: Windows builds in different directories are not fully reproducible #7344

mkartashev commented Feb 4, 2022 •

edited by openjdk bot

bridgekeeper bot commented Feb 4, 2022

openjdk bot commented Feb 4, 2022

mlbridge bot commented Feb 4, 2022 •

edited

erikj79 commented Feb 4, 2022 •

edited

mkartashev commented Feb 4, 2022

erikj79 commented Feb 4, 2022

erikj79 left a comment

mkartashev commented Feb 7, 2022

mkartashev commented Feb 8, 2022

magicus left a comment

openjdk bot commented Feb 10, 2022

openjdk bot commented Feb 10, 2022 •

edited

mkartashev commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

mkartashev commented Feb 10, 2022

mkartashev commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

magicus commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

8281262: Windows builds in different directories are not fully reproducible #7344

8281262: Windows builds in different directories are not fully reproducible #7344

Conversation

mkartashev commented Feb 4, 2022 • edited by openjdk bot

Progress

Issue

Reviewers

Contributors

Reviewing

bridgekeeper bot commented Feb 4, 2022

openjdk bot commented Feb 4, 2022

mlbridge bot commented Feb 4, 2022 • edited

Webrevs

erikj79 commented Feb 4, 2022 • edited

mkartashev commented Feb 4, 2022

erikj79 commented Feb 4, 2022

erikj79 left a comment

Choose a reason for hiding this comment

mkartashev commented Feb 7, 2022

mkartashev commented Feb 8, 2022

magicus left a comment

Choose a reason for hiding this comment

openjdk bot commented Feb 10, 2022

openjdk bot commented Feb 10, 2022 • edited

mkartashev commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

mkartashev commented Feb 10, 2022

mkartashev commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

magicus commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

openjdk bot commented Feb 10, 2022

mkartashev commented Feb 4, 2022 •

edited by openjdk bot

mlbridge bot commented Feb 4, 2022 •

edited

erikj79 commented Feb 4, 2022 •

edited

openjdk bot commented Feb 10, 2022 •

edited