8257679: Improved unix compatibility layer in Windows build (winenv) #1597

magicus · 2020-12-03T14:19:13Z

For the build to work on Windows, we need a unix compatibility layer (known as the "winenv" in the build system). This can be e.g. Cygwin or Msys. The build system then needs to adapt various aspect to get the build to work in this winenv. Over time, our current solutions (which were never that well-designed) has collapsed into an unmaintainable mess.

This rewrite takes on a fresh approach, by giving up on the native "fixpath.exe" converter, and instead relying on a platform-independent shell script "fixpath.sh", which can dynamically adapt to the current winenv. It also provides a proper framework on how to categorize, and support, different winenvs. As a result, we now easily support Cygwin, Msys2, WSL1 and WSL2 (the latter is unfortunately not mature enough to be able to compile the JDK).

Furthermore, this rewrite removes all the kludges and hacks that were put in place all over the code base, and consolidates all tricky part of handling the winenv to basically two places: setting up in configure, and run-time handling by fixpath.sh.

This patch also cleans up our handling of how we detect tools in configure, and makes a proper framework for cross-compilation on Windows.

Progress

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

Issue

JDK-8257679: Improved unix compatibility layer in Windows build (winenv)

Reviewers

Erik Joelsson (@erikj79 - Reviewer)
Jorn Vernee (@JornVernee - Committer) ⚠️ Review applies to 41cee7d
Bernhard Urban-Forster (@lewurm - Author)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1597/head:pull/1597
$ git checkout pull/1597

bridgekeeper · 2020-12-03T14:20:11Z

👋 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.

openjdk · 2020-12-03T14:21:27Z

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

build
core-libs
hotspot-compiler
i18n

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

magicus · 2020-12-03T14:24:22Z

/label remove i18n
/label remove core-libs

openjdk · 2020-12-03T14:24:33Z

@magicus
The i18n label was successfully removed.

openjdk · 2020-12-03T14:25:01Z

@magicus
The core-libs label was successfully removed.

mlbridge · 2020-12-03T14:28:10Z

Webrevs

erikj79

Overall this is very nice. Some comments and questions inline.

erikj79 · 2020-12-03T15:20:07Z

make/CreateJmods.gmk

@@ -169,7 +169,7 @@ ifeq ($(MODULE), java.base)
    endif
  endif
 else # not java.base
-  ifeq ($(call isTargetOs, windows), true)
+  ifeq ($(call isTargetOs, windows)+$(CREATING_BUILDJDK), true+false)


This looks strange. Why would CREATING_BUILDJDK be relevant here?

If we are creating a buildjdk (which really means we are cross-compiling), we must not meddle with the Windows runtime dll:s, since they are for the target CPU architecture. To be fully correct, we should really turn the Windows runtime dll:s into a "normal"/BUILD_ split version, like with SYSROOT etc.

But since this is just used locally for building, I'm assuming the build system has these runtime DLLs installed already. There seems to be no point in wasting time preparing a buildjdk with files that are just bundled for installation.

Maybe there's a better way to express this, but the only thing I could get a hold of for the fact that we are creating a build-JDK (that is, not targeting the normal OPENJDK_TARGET_CPU) was the CREATING_BUILDJDK variable.

I reverted this change, and added a comment when we copy the libs.

make/TestImage.gmk

make/autoconf/toolchain.m4

erikj79 · 2020-12-03T20:04:44Z

make/autoconf/util_paths.m4

+
+            if test "x$4" != xNOFIXPATH; then
+              [ if [[ $FIXPATH != "" && $result =~ ^"$FIXPATH " ]]; then ]
+                result="\$FIXPATH ${result#"$FIXPATH "}"


Maybe I'm missing something, but is this unconditionally adding fixpath to any executable based just on if FIXPATH is set? Shouldn't there be a conditional on if the executable is Windows or Unix type?

I agree it is a bit hairy. Perhaps not ideal; even though this is a rewrite it has suffered some legacy "lava flow" itself, being in development for so long.

The idea here is. that NOFIXPATH (or not) as $4 is passed to UTIL_FIXUP_EXECUTABLE. If it is set, then UTIL_FIXUP_EXECUTABLE will never prepend FIXPATH. Otherwise, UTIL_FIXUP_EXECUTABLE will do the check you are requesting, and see if we are on Windows and it is a non-unix-aware executable, and if so, prepend FIXPATH.

So when we come back to UTIL_LOOKUP_PROGS, if we don't have NOFIXPATH, and we are on Windows (FIXPATH is non-empty) and the path we got from UTIL_FIXUP_EXECUTABLE starts with FIXPATH, then we remove all instances of FIXPATH, and prepend a single FIXPATH instance.

While seems like unneccessary work. I ran into situations where I got a double FIXPATH prefix. It might have been the result of some other bug that is fixed by now, but costs very little to keep this as a safeguard. It should get a comment, though, describing what it does.

Comment is good for me, thanks.

make/autoconf/util_paths.m4

make/common/JavaCompilation.gmk

erikj79 · 2020-12-03T20:51:45Z

make/common/JavaCompilation.gmk

      $$($1_JAVAC_SERVER_CONFIG): $$($1_CONFIG_VARDEPS_FILE)
-	$$($1_ECHO_COMMAND) portfile=$$($1_JAVAC_PORT_FILE) > $$@
-	$$($1_ECHO_COMMAND) servercmd=$$($1_JAVAC_SERVER_CMD) >> $$@
+	$(ECHO) portfile=$$($1_JAVAC_PORT_FILE) > $$@


Did you consider using WriteFile here?

Yes, I tried that first. And pulled my hair a couple of times. Turns out there is no way (that I could find, at least) to get WriteFile to write multiple lines.

Fair enough.

erikj79 · 2020-12-03T21:00:54Z

make/modules/java.base/Copy.gmk

@@ -45,7 +45,7 @@ endif

 ################################################################################
 # Copy the microsoft runtime libraries on windows
-ifeq ($(call isTargetOs, windows), true)
+ifeq ($(call isTargetOs, windows)+$(CREATING_BUILDJDK), true+false)


Is the problem here that we don't have build versions of the runtime libraries so we simply trust that they are present on the build host instead?

Yes. And this applies to the other fix above as well. Hm. Maybe it is enough to avoid copying them here, and then I don't need the patch in CreateJmods.gmk

JornVernee · 2020-12-03T22:21:52Z

Hey,

Since I often work on Windows I'm taking this for a spin, but the current patch fails during configure for me with:

configure: Using default toolchain microsoft (Microsoft Visual Studio)
configure: Found Visual Studio installation at /cygdrive/j/progra~2/micros~1/2019/community/ using --with-tools-dir
configure: Found Microsoft Visual Studio 2019
configure: Trying to extract Visual Studio environment variables for x86_64
configure: using /cygdrive/j/progra~2/micros~1/2019/community//vc/auxiliary/build/vcvarsx86_amd64.bat
configure: Setting extracted environment variables for x86_64
fixpath: failure: Directory containing path 'usr\local\bin' does not exist
checking that Visual Studio variables have been correctly extracted... ok
checking for cl... [not found]
configure: error: Could not find a C compiler.
configure exiting with result code 1

I think this is because I have my VS installation in a non-default directory passed to --with-tools-dir. This is the configure command:

bash configure \
  --with-conf-name=windows-release \
  --with-boot-jdk='/cygdrive/c/Program Files/Java/jdk-15' \
  --with-jtreg=/cygdrive/h/libs/jtreg-5.1-b01/ \
  --with-tools-dir='J:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools' \
  --with-toolchain-version=2019 \
  --with-jmh=/cygdrive/h/libs/jmh

(Note that --with-tools-dir requires a Windows path for some reason, otherwise configure complains that the VS installation is not valid).

Cross-compiling to Linux with WSL works with:

bash configure \
  --build=x86_64-unknown-linux-gnu \
  --host=x86_64-unknown-linux-gnu \
  --with-jtreg=/mnt/h/libs/jtreg-5.1-b01 \
  --with-conf-name=linux-release \
  --with-boot-jdk=/usr/lib/jvm/jdk-15 \
  --with-jmh=/mnt/h/libs/jmh

But trying to configure for Windows on WSL with:

bash configure \
  --with-conf-name=windows-release-wsl \
  --with-boot-jdk='/mnt/c/Program Files/Java/jdk-15' \
  --with-jtreg=/mnt/h/libs/jtreg-5.1-b01/ \
  --with-tools-dir='J:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools' \
  --with-toolchain-version=2019 \
  --with-jmh=/mnt/h/libs/jmh

Fails with:

configure: Found potential Boot JDK using configure arguments
configure: Potential Boot JDK found at /mnt/c/Program Files/Java/jdk-15 is not a working JDK; ignoring
configure: Output from java -version was: /mnt/h/cygwin64/home/Jorn/cygwin-projects-new/git-jdk2/build/.configure-support/generated-configure.sh: line 56895: -version: command not found
configure: error: The path given by --with-boot-jdk does not contain a valid Boot JDK
configure exiting with result code 1

This is not a configuration I normally use though. Since I've had problems with WSL I usually use Cygwin, so the configure command might not be right (am I missing a target platform flag here? According to the build docs Windows is the default).

(I'll continue investigating as well)

magicus · 2020-12-03T22:52:57Z

@JornVernee Thanks for your feedback! I'll have a closer look at your examples, and see if I can reproduce them in my own environment. One question, your first example, was this from a Cygwin environment?

As a general comment, the idea is that paths to configure options should be in "unix" style. If that is not accepted, it is a bug. (Otoh, I try hard to be tolerant and accept Windows style or mixed-style ("c:/windows") paths as well, but this is not guaranteed to work.)

JornVernee · 2020-12-03T22:55:49Z

@JornVernee Thanks for your feedback! I'll have a closer look at your examples, and see if I can reproduce them in my own environment. One question, your first example, was this from a Cygwin environment?

Yes, sorry, the first example is from running in Cygwin. The problem might be with the Windows path then.

As a general comment, the idea is that paths to configure options should be in "unix" style. If that is not accepted, it is a bug. (Otoh, I try hard to be tolerant and accept Windows style or mixed-style ("c:/windows") paths as well, but this is not guaranteed to work.)

Okay, the problem might be with the Windows path then. I'll see if I can make some changes and get it to accept a unix path as well.

JornVernee · 2020-12-03T22:59:23Z

@magicus Ah dang, I tested passing --with-tools-dir a unix path before but messed up the drive letter. So, passing unix path to --with-tools-dir (i.e. --with-tools-dir='/cygdrive/j/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools') works after all (though IIRC this wasn't the case in the past).

magicus · 2020-12-03T23:25:28Z

@JornVernee Yes, this can certainly have been wrong in the past. That's one of the things that I've fixed with this patch, making sure we handle paths correctly :-)

Does this mean the patch works for you on both Cygwin and WSL now?

JornVernee · 2020-12-03T23:31:34Z

@magicus No, the same problems still remain. I seem to have narrowed down the first error to the set-vs-env.sh script; the PATH it extracts for VS_PATH contains usr\local\bin for some reason.

<values that seem to come from vcvars>
C:\windows\system32
H:\cygwin64\bin
usr\local\bin
H:\cygwin64\usr\local\bin
H:\cygwin64\bin
<contents of system PATH>

JornVernee · 2020-12-03T23:53:20Z

Ok, it seems to have been caused by usr/local/bin being added to my Cygwin PATH in .bash_profile. If I remove that I get a little further.

But the same command fails later on a path with a space in it:

fixpath: failure: Path '/cygdrive/c/progra~2/ati technologies/ati.ace/core-static' contains space

Looks like a missing short/8dot3 path for one directory. I'll try to get it working on my end. I'll take further questions offline to keep this thread focused on the review.

magicus · 2020-12-04T12:45:57Z

@JornVernee I have now pushed a fix that will make fixpath import ignore invalid or unknown paths be ignored. Please try again with it.

erikj79 · 2020-12-04T14:17:22Z

This looks good to me now, but seems GHA aren't completely ok with it yet.

JornVernee · 2020-12-04T14:22:45Z

@magicus Thanks! But, now I get the same error as the GH action: https://github.com/magicus/openjdk-sandbox/runs/1498822974#step:11:80

Looks like -a is not supported in my shell. I can make the error go away locally with:

diff --git a/make/scripts/fixpath.sh b/make/scripts/fixpath.sh
index ae5ca1f8a28..c7737a23eb8 100644
--- a/make/scripts/fixpath.sh
+++ b/make/scripts/fixpath.sh
@@ -207,7 +207,7 @@ function import_command_line() {
     if ! [[ $arg =~ ^" "+$ ]]; then
       import_path "$arg"

-      if [[ "$result" != "" -a "$imported" = "" ]]; then
+      if [[ "$result" != "" ]] && [[ "$imported" = "" ]]; then
         imported="$result"
       else
         imported="$imported:$result"

But I'm not sure that does the same thing.

After that, I can confirm that both path issues with Cygwin are resolved (usr\local\bin and paths with spaces don't cause a hard failure anymore).

The issue with WSL still remains though:

configure: Found potential Boot JDK using configure arguments
configure: Potential Boot JDK found at /mnt/c/Program Files/Java/jdk-15 is not a working JDK; ignoring
configure: Output from java -version was: /mnt/h/cygwin64/home/Jorn/cygwin-projects-new/git-jdk2/build/.configure-support/generated-configure.sh: line 57167: -version: command not found
configure: error: The path given by --with-boot-jdk does not contain a valid Boot JDK
configure exiting with result code 1

Line 57167 in generated-configure.sh is this:

    # Now join together the path and the arguments once again
    new_complete="$prefix$new_path$arguments"

JornVernee · 2020-12-04T15:57:44Z

Ok, after looking at set -x output WSL seems to have a problem with the space in the path /mnt/c/Program Files/Java/jdk-15 e.g. if I move the jdk to /mnt/h/jdk-15 it works. Might be a missing fixup somewhere? (FWIW, that Program Files dir does have an 8dot3 name).

With that I can build images on both Cygwin and WSL. Build times are about 29 mins and 18 mins respectively.

erikj79 · 2020-12-04T17:34:33Z

Do you see any significant difference in build times compared to before the patch?

magicus · 2020-12-04T18:01:54Z

The line numbers in the generated configure script seems to be mismatched; that can't have been the actual failing line.

This actual failing line is line 79 in boot-jdk.m4:

  BOOT_JDK_VERSION_OUTPUT=`$java_to_test $USER_BOOT_JDK_OPTIONS -version 2>&1`

If $java_to_test (and $USER_BOOT_JDK_OPTIONS) is empty, then we try to execute -version and get this output. So the question is, why is java_to_test empty? The two preceding lines are:

          java_to_test="$BOOT_JDK/bin/java"
          UTIL_FIXUP_EXECUTABLE(java_to_test)

Sp java_to_test should be /mnt/c/Program Files/Java/jdk-15/bin/java. And obviously UTIL_FIXUP_EXECUTABLE rewrites this to the empty string. Why that is so, I don't know. Perhaps an issue with the space in the path.

magicus · 2020-12-04T18:07:22Z

@JornVernee Ah, sorry, I missed that you had added yet another comment, with the solution! :-) I'll have a look at why the space is problematic.

Do I understand you correctly that the same path to Java worked on Cygwin but failed on WSL?

JornVernee · 2020-12-04T18:11:27Z

@magicus Yes, that is correct. The path worked on Cygwin but not on WSL.

magicus · 2020-12-04T18:20:00Z

@erikj79 The big difference is the shell script fixpath, as opposed to a native fixpath.exe. I've gone to great pains to make sure the "exec" pathway in fixpath.sh contains no external calls, but only native bash constructs, to avoid forks, which are expensive at Windows. With that said, fixpath.sh is still slower than fixpath.exe. A typical invocation with fixpath.sh is about 30 ms slower than fixpath.exe; that is, in average 0.079 s instead of 0.047 s. (This is on Cygwin; on WSL fixpath.sh executes basically instantaneously.) This do translate to longer build times, but I have a hard time getting a proper value. The fluctuation in build times is so large so it's hard to say with certainty, but e.g. on GitHub Actions the build time increased from 53-57 minutes typically, to 58-62 minutes typically. On my personal machine, I could get more reliable and repeatable results. There the time taken to build hotspot increased from 4 min 45 s to 4 min 53 s.

All these values are from running on Cygwin. The build times on WSL noticeably faster. (As a follow-up bug, I intend to migrate the GitHub Action script to running on WSL instead of Cygwin.) On machine, the build time decreased from over. 15 minutes, to under 13, when going from Cygwin to WSL.

JornVernee · 2020-12-04T18:26:21Z

@erikj79 I don't really see a significant difference in the build time of images on my machine when comparing before and after the patch (but, as Magnus says, the build times tend to fluctuate, so it's hard to get an accurate assessment. Maybe the difference is a couple of minutes. Not that big a deal for a 30-ish minute build).

magicus · 2020-12-04T18:52:08Z

@JornVernee I have reproed the issue, identified the problem and pushed a patch. Please try again!

JornVernee

Thanks for the fixes! "Works on my machine" now (Cygwin and WSL).

(FWIW, next week is the fork for RDP1 I believe, so might want to wait with pushing until after that)

lewurm · 2020-12-07T08:49:42Z

make/autoconf/boot-jdk.m4

@@ -63,19 +63,23 @@ AC_DEFUN([BOOTJDK_DO_CHECK],
    # If previous step claimed to have found a JDK, check it to see if it seems to be valid.
    if test "x$BOOT_JDK_FOUND" = xmaybe; then
      # Do we have a bin/java?
-      if test ! -x "$BOOT_JDK/bin/java$EXE_SUFFIX"; then
+      if test ! -x "$BOOT_JDK/bin/java" && test ! -x "$BOOT_JDK/bin/java.exe"; then


BOOTJDK_CHECK_BUILD_JDK also needs those updates I believe

lewurm

Tested the cross compilation bits with a win-aarch64 build from win-x86_64:

Cygwin: --openjdk-target=aarch64-unknown-cygwin --with-boot-jdk=/cygdrive/c/work/jdk-16+22
WSL1: --openjdk-target=aarch64-unknown-cygwin --with-boot-jdk=/mnt/c/work/jdk-16+22

And smoke tested each one on win-aarch64 with jtreg:tier1_compiler_1.

Cosmetic: I get a bunch of warnings for non-existing paths in $PATH during configure on the wsl1 build, e.g.:

configure: Setting extracted environment variables for x86_64
/usr/bin/wslpath: \\wsl$\uwsl1\home\lewurm\riscv\bin: No such file or directory
/usr/bin/wslpath: \\wsl$\uwsl1\home\lewurm\mate: No such file or directory

I'll give the "native compilation" on win-aarch64 a try again when this change has landed. Some bits (e.g. config.guess) required for it have made it into this PR, but some things are still missing (e.g. choose x86 binaries for MSVC, since no native bits are available for MSVC).

Thank you for the hard work on this!

lewurm · 2020-12-07T09:27:39Z

make/autoconf/toolchain_microsoft.m4


  AC_ARG_WITH(ucrt-dll-dir, [AS_HELP_STRING([--with-ucrt-dll-dir],
      [path to Microsoft Windows Kit UCRT DLL dir (Windows only) @<:@probed@:>@])])

-  if test "x$USE_UCRT" = "xtrue"; then
+  if test "x$USE_UCRT" = "xtrue" && test "x$OPENJDK_TARGET_CPU" != xaarch64; then


openjdk · 2020-12-07T14:30:29Z

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

8257679: Improved unix compatibility layer in Windows build (winenv)

Reviewed-by: erikj, jvernee, burban

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

2c04fc0: 8257037: No javac warning when calling deprecated constructor with diamond
46b35ac: 8257798: [PPC64] undefined reference to Klass::vtable_start_offset()
ecd7e47: 8257793: Shenandoah: SATB barrier should only filter out already strongly marked oops
e08b9ed: 8257820: Remove gc/ergonomics/TestMinHeapSize.java as it is too brittle
637b0c6: 8246778: Compiler implementation for Sealed Classes (Second Preview)
09707dd: 8252807: The jdk.jfr.Recording.getStream does not work when toDisk is disabled
04ce8e3: 8257184: Upstream 8252504: Add a method to MemoryLayout which returns a offset-computing method handle
5a03e47: 8255560: Class::isRecord should check that the current class is final and not abstract
8e8e584: 8257588: Make os::_page_sizes a bitmask
566d77a: 8254802: ThrowingPushPromisesAsStringCustom.java fails in "try throwing in GET_BODY"
... and 53 more: https://git.openjdk.java.net/jdk/compare/fa58671f9f8a8230d907c0a6289685f8455bce1f...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.

magicus · 2020-12-07T15:55:15Z

/integrate

openjdk · 2020-12-07T15:56:20Z

@magicus Since your change was applied there have been 64 commits pushed to the master branch:

74be819: 8257517: LogCompilation: Add -z to the help messages
2c04fc0: 8257037: No javac warning when calling deprecated constructor with diamond
46b35ac: 8257798: [PPC64] undefined reference to Klass::vtable_start_offset()
ecd7e47: 8257793: Shenandoah: SATB barrier should only filter out already strongly marked oops
e08b9ed: 8257820: Remove gc/ergonomics/TestMinHeapSize.java as it is too brittle
637b0c6: 8246778: Compiler implementation for Sealed Classes (Second Preview)
09707dd: 8252807: The jdk.jfr.Recording.getStream does not work when toDisk is disabled
04ce8e3: 8257184: Upstream 8252504: Add a method to MemoryLayout which returns a offset-computing method handle
5a03e47: 8255560: Class::isRecord should check that the current class is final and not abstract
8e8e584: 8257588: Make os::_page_sizes a bitmask
... and 54 more: https://git.openjdk.java.net/jdk/compare/fa58671f9f8a8230d907c0a6289685f8455bce1f...master

Your commit was automatically rebased without conflicts.

Pushed as commit d29c78d.

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

8257679: Improved unix compatibility layer in Windows build (winenv)

855751f

openjdk bot added the rfr Pull request is ready for review label Dec 3, 2020

openjdk bot added build [email protected] hotspot-compiler [email protected] core-libs [email protected] i18n [email protected] labels Dec 3, 2020

openjdk bot removed the i18n [email protected] label Dec 3, 2020

openjdk bot removed the core-libs [email protected] label Dec 3, 2020

Missed my last bugfix for fixpath.sh...

c00d5a9

erikj79 reviewed Dec 3, 2020

View reviewed changes

magicus added 2 commits December 4, 2020 09:52

Address remarks in code review

20f2619

Make fixpath import ignore bad paths while importing path lists

5b7a749

Fix syntax error in fixpath.sh

b8a2412

Fix for .exe on WSL in fixpath.sh

41cee7d

JornVernee approved these changes Dec 4, 2020

View reviewed changes

Extract only the actual contents added to the PATH by VS SetEnv.cmd.

ef81e1f

lewurm reviewed Dec 7, 2020

View reviewed changes

lewurm approved these changes Dec 7, 2020

View reviewed changes

erikj79 approved these changes Dec 7, 2020

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Dec 7, 2020

openjdk bot closed this Dec 7, 2020

openjdk bot added integrated Pull request has been integrated and removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 7, 2020

magicus deleted the winenv-rewrite branch December 7, 2020 15:57

8257679: Improved unix compatibility layer in Windows build (winenv) #1597

8257679: Improved unix compatibility layer in Windows build (winenv) #1597

Conversation

magicus commented Dec 3, 2020 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Download

bridgekeeper bot commented Dec 3, 2020

openjdk bot commented Dec 3, 2020

magicus commented Dec 3, 2020

openjdk bot commented Dec 3, 2020

openjdk bot commented Dec 3, 2020

mlbridge bot commented Dec 3, 2020 • edited Loading

Webrevs

erikj79 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JornVernee commented Dec 3, 2020 • edited Loading

magicus commented Dec 3, 2020

JornVernee commented Dec 3, 2020

JornVernee commented Dec 3, 2020 • edited Loading

magicus commented Dec 3, 2020

JornVernee commented Dec 3, 2020 • edited Loading

JornVernee commented Dec 3, 2020

magicus commented Dec 4, 2020

erikj79 commented Dec 4, 2020

JornVernee commented Dec 4, 2020 • edited Loading

JornVernee commented Dec 4, 2020

erikj79 commented Dec 4, 2020

magicus commented Dec 4, 2020

magicus commented Dec 4, 2020

JornVernee commented Dec 4, 2020

magicus commented Dec 4, 2020

JornVernee commented Dec 4, 2020

magicus commented Dec 4, 2020

JornVernee left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lewurm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openjdk bot commented Dec 7, 2020

magicus commented Dec 7, 2020

openjdk bot commented Dec 7, 2020

magicus commented Dec 3, 2020 •

edited by openjdk bot

Loading

mlbridge bot commented Dec 3, 2020 •

edited

Loading

JornVernee commented Dec 3, 2020 •

edited

Loading

JornVernee commented Dec 3, 2020 •

edited

Loading

JornVernee commented Dec 3, 2020 •

edited

Loading

JornVernee commented Dec 4, 2020 •

edited

Loading

JornVernee left a comment •

edited

Loading