Re: RFR: 8287366: Improve test failure reporting in GHA

2022-06-06 Thread Jaikiran Pai
On Thu, 26 May 2022 12:04:41 GMT, Magnus Ihse Bursie  wrote:

> With this PR, the overview of failures are presented on the "Summary" page 
> for the action (the top-most line to the left, with the outline house icon).

@magicus, thank you. This is really useful. I didn't even know that this 
"Summary" page existed. I now checked this page on one of my PRs (which 
includes this commit) and it does indeed make it much simpler to analyze these 
failures.

-

PR: https://git.openjdk.java.net/jdk/pull/8901


Integrated: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues

2022-05-16 Thread Jaikiran Pai
On Fri, 13 May 2022 14:17:44 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which proposes to fix the failure 
> handler command `dmesg` on macOS?
> 
> As noted in the JBS issue, the command currently fails with permission error. 
> The commit in this PR uses `sudo` as suggested in the man pages of that 
> command.
> 
> Tested locally on macOS by running:
> 
> 
> cd test/failure_handler
> make clean
> make test
> 
> Then checked the generated environment.html files for the dmesg command 
> output. Looks fine after this change.

This pull request has now been integrated.

Changeset: 125efe6c
Author:Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/125efe6cbaf1e2c263d74a4ada395ac30c479faa
Stats: 4 lines in 1 file changed: 2 ins; 0 del; 2 mod

8286744: failure_handler: dmesg command on macos fails to collect data due to 
permission issues

Reviewed-by: dfuchs, lancea, lmesnik

-

PR: https://git.openjdk.java.net/jdk/pull/8703


Re: RFR: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues [v2]

2022-05-16 Thread Jaikiran Pai
On Mon, 16 May 2022 22:06:25 GMT, Leonid Mesnik  wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   copyright year
>
> Looks reasonable.

Thank you @lmesnik  for the review.

-

PR: https://git.openjdk.java.net/jdk/pull/8703


Re: RFR: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues [v2]

2022-05-16 Thread Jaikiran Pai
On Fri, 13 May 2022 14:39:44 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this change which proposes to fix the failure 
>> handler command `dmesg` on macOS?
>> 
>> As noted in the JBS issue, the command currently fails with permission 
>> error. The commit in this PR uses `sudo` as suggested in the man pages of 
>> that command.
>> 
>> Tested locally on macOS by running:
>> 
>> 
>> cd test/failure_handler
>> make clean
>> make test
>> 
>> Then checked the generated environment.html files for the dmesg command 
>> output. Looks fine after this change.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   copyright year

Thank you Daniel and Lance for the reviews. 

Would anyone from the build team provide any additional reviews?

-

PR: https://git.openjdk.java.net/jdk/pull/8703


Re: RFR: 8286744: failure_handler: dmesg command on macos fails to collect data due to permission issues [v2]

2022-05-13 Thread Jaikiran Pai
> Can I please get a review of this change which proposes to fix the failure 
> handler command `dmesg` on macOS?
> 
> As noted in the JBS issue, the command currently fails with permission error. 
> The commit in this PR uses `sudo` as suggested in the man pages of that 
> command.
> 
> Tested locally on macOS by running:
> 
> 
> cd test/failure_handler
> make clean
> make test
> 
> Then checked the generated environment.html files for the dmesg command 
> output. Looks fine after this change.

Jaikiran Pai has updated the pull request incrementally with one additional 
commit since the last revision:

  copyright year

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8703/files
  - new: https://git.openjdk.java.net/jdk/pull/8703/files/db538083..f85acab5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8703=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8703=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8703.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8703/head:pull/8703

PR: https://git.openjdk.java.net/jdk/pull/8703


Integrated: 8286623: Bundle zlib by default with JDK on macos aarch64

2022-05-12 Thread Jaikiran Pai
On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

This pull request has now been integrated.

Changeset: c3bade2e
Author:Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/c3bade2e08f865bf1e65d48e6d27bff9c022d35f
Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod

8286623: Bundle zlib by default with JDK on macos aarch64

Reviewed-by: lancea, ihse, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/8675


Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

2022-05-12 Thread Jaikiran Pai
On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

Thank you everyone for the reviews.

-

PR: https://git.openjdk.java.net/jdk/pull/8675


Re: RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

2022-05-12 Thread Jaikiran Pai
On Thu, 12 May 2022 08:43:56 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change to the build system which now makes 
> bundled zlib as the default for macos aarch64 systems? 
> 
> We have been running into various intermittent failures on macos aarch64 
> systems for several months now. After investigation we have been able to 
> ascertain that the root cause of the issue lies within the zlib that's 
> shipped on macos aarch64 systems. The complete details about that issue is 
> available at https://bugs.openjdk.java.net/browse/JDK-8282954. 
> We have filed a bug with Apple for their inputs on what we can do to fix it 
> in that shipped library. Given the nature of that issue, we don't have a 
> timeline on when/if the solution for that will be available. Until that time, 
> at least, the proposal is to use bundled zlib (which doesn't reproduce those 
> failures) by default on macos aarch64.

Dummy comment to initiate a mail to core-libs mailing list too.

-

PR: https://git.openjdk.java.net/jdk/pull/8675


RFR: 8286623: Bundle zlib by default with JDK on macos aarch64

2022-05-12 Thread Jaikiran Pai
Can I please get a review of this change to the build system which now makes 
bundled zlib as the default for macos aarch64 systems? 

We have been running into various intermittent failures on macos aarch64 
systems for several months now. After investigation we have been able to 
ascertain that the root cause of the issue lies within the zlib that's shipped 
on macos aarch64 systems. The complete details about that issue is available at 
https://bugs.openjdk.java.net/browse/JDK-8282954. 
We have filed a bug with Apple for their inputs on what we can do to fix it in 
that shipped library. Given the nature of that issue, we don't have a timeline 
on when/if the solution for that will be available. Until that time, at least, 
the proposal is to use bundled zlib (which doesn't reproduce those failures) by 
default on macos aarch64.

-

Commit messages:
 - jib profile - don't explicitly set system zlib for macosx-aarch64
 - default to bundled zlib on macos aarch64

Changes: https://git.openjdk.java.net/jdk/pull/8675/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8675=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286623
  Stats: 4 lines in 2 files changed: 2 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8675.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8675/head:pull/8675

PR: https://git.openjdk.java.net/jdk/pull/8675


Integrated: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled

2022-05-12 Thread Jaikiran Pai
On Wed, 11 May 2022 11:38:31 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which fixes build failures on macos 
> when using `--with-zlib=bundled`?
> 
> With this change the build now passes (tested both with bundled and system 
> zlib variants).
> 
> tier1, tier2 and tier3 testing has been done and no related failures have 
> been noticed.

This pull request has now been integrated.

Changeset: 50d47de8
Author:Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/50d47de8358e2f22bf3a4a165d660c25ef6eacbc
Stats: 9 lines in 3 files changed: 5 ins; 0 del; 4 mod

8286582: Build fails on macos aarch64 when using --with-zlib=bundled

Reviewed-by: ihse, lancea

-

PR: https://git.openjdk.java.net/jdk/pull/8651


Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]

2022-05-11 Thread Jaikiran Pai
On Wed, 11 May 2022 14:24:38 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this change which fixes build failures on macos 
>> when using `--with-zlib=bundled`?
>> 
>> With this change the build now passes (tested both with bundled and system 
>> zlib variants).
>> 
>> tier1, tier2 and tier3 testing has been done and no related failures have 
>> been noticed.
>
> Jaikiran Pai has updated the pull request incrementally with four additional 
> commits since the last revision:
> 
>  - copyright years
>  - disable format-nonliteral warning when building LIBSPLASHSCREEN with 
> bundled zlib
>  - Magnus' suggestion - make the LIBZ_CFLAGS more readable in the build file
>  - Magnus' suggestion - Disable format-nonliteral in build section of zlib 
> instead of source code

Thank you Lance and Magnus for the reviews and inputs. I've triggered various 
build and test runs with this state of the PR. Once those complete 
satisfactorily, I'll go ahead and integrate this.

-

PR: https://git.openjdk.java.net/jdk/pull/8651


Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]

2022-05-11 Thread Jaikiran Pai
On Wed, 11 May 2022 12:50:39 GMT, Alan Bateman  wrote:

>> Thank you for these useful inputs Magnus. I did these changes locally but 
>> for some reason this format-nonliteral is not getting picked up while 
>> building that library. I will investigate and see what's going on. Will 
>> update the PR once I figure it out.
>
> I agree with Magnus and try to avoid changing the imported zlib code.

>  I did these changes locally but for some reason this format-nonliteral is 
> not getting picked up while building that library.

Turns out that was slightly inaccurate. What was actually happening was that, 
that setting you suggested in that build file did indeed work and got picked 
up. But it ran into an error which looked like:


src/java.base/share/native/libzip/zlib/gzwrite.c:452:40: error: format string 
is not a string literal [-Werror,-Wformat-nonliteral]
len = vsnprintf(next, state->size, format, va);
   ^~

-

PR: https://git.openjdk.java.net/jdk/pull/8651


Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]

2022-05-11 Thread Jaikiran Pai
On Wed, 11 May 2022 11:52:55 GMT, Magnus Ihse Bursie  wrote:

>> Jaikiran Pai has updated the pull request incrementally with four additional 
>> commits since the last revision:
>> 
>>  - copyright years
>>  - disable format-nonliteral warning when building LIBSPLASHSCREEN with 
>> bundled zlib
>>  - Magnus' suggestion - make the LIBZ_CFLAGS more readable in the build file
>>  - Magnus' suggestion - Disable format-nonliteral in build section of zlib 
>> instead of source code
>
> make/autoconf/lib-bundled.m4 line 224:
> 
>> 222:   LIBZ_LIBS=""
>> 223:   if test "x$USE_EXTERNAL_LIBZ" = "xfalse"; then
>> 224: LIBZ_CFLAGS="$LIBZ_CFLAGS $APPLE_LIBZ_CFLAGS 
>> -I$TOPDIR/src/java.base/share/native/libzip/zlib"
> 
> Declaring APPLE_LIBZ_CFLAGS far away (and only conditionally) and then using 
> it once here just makes for hard-to-read code. 
> 
> Suggestion:
> 
> LIBZ_CFLAGS="$LIBZ_CFLAGS 
> -I$TOPDIR/src/java.base/share/native/libzip/zlib"
> if test "x$OPENJDK_TARGET_OS" = xmacosx; then
>   LIBZ_CFLAGS="$LIBZ_CFLAGS -DHAVE_UNISTD_H"
> fi
> 
> ... and remove the assignment above.

Updated the PR to implement this suggestion

-

PR: https://git.openjdk.java.net/jdk/pull/8651


Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled [v2]

2022-05-11 Thread Jaikiran Pai
> Can I please get a review of this change which fixes build failures on macos 
> when using `--with-zlib=bundled`?
> 
> With this change the build now passes (tested both with bundled and system 
> zlib variants).
> 
> tier1, tier2 and tier3 testing has been done and no related failures have 
> been noticed.

Jaikiran Pai has updated the pull request incrementally with four additional 
commits since the last revision:

 - copyright years
 - disable format-nonliteral warning when building LIBSPLASHSCREEN with bundled 
zlib
 - Magnus' suggestion - make the LIBZ_CFLAGS more readable in the build file
 - Magnus' suggestion - Disable format-nonliteral in build section of zlib 
instead of source code

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8651/files
  - new: https://git.openjdk.java.net/jdk/pull/8651/files/eee12c25..081a7d34

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8651=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8651=00-01

  Stats: 41 lines in 5 files changed: 5 ins; 30 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8651.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8651/head:pull/8651

PR: https://git.openjdk.java.net/jdk/pull/8651


Re: RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled

2022-05-11 Thread Jaikiran Pai
On Wed, 11 May 2022 11:56:30 GMT, Magnus Ihse Bursie  wrote:

>> Can I please get a review of this change which fixes build failures on macos 
>> when using `--with-zlib=bundled`?
>> 
>> With this change the build now passes (tested both with bundled and system 
>> zlib variants).
>> 
>> tier1, tier2 and tier3 testing has been done and no related failures have 
>> been noticed.
>
> src/java.base/share/native/libzip/zlib/gzwrite.c line 452:
> 
>> 450: len = strlen(next);
>> 451: #  else
>> 452: #   ifdef __APPLE__ // ignore format-nonliteral warning on macOS
> 
> Instead of patching 3rd party code to fix a compilation warning, you should 
> disable that warning instead.
> 
> In `make/modules/java.base/lib/CoreLibraries.gmk`, add 
> 
> DISABLED_WARNINGS_clang := format-nonliteral, \
> 
> as line 138.

Thank you for these useful inputs Magnus. I did these changes locally but for 
some reason this format-nonliteral is not getting picked up while building that 
library. I will investigate and see what's going on. Will update the PR once I 
figure it out.

-

PR: https://git.openjdk.java.net/jdk/pull/8651


RFR: 8286582: Build fails on macos aarch64 when using --with-zlib=bundled

2022-05-11 Thread Jaikiran Pai
Can I please get a review of this change which fixes build failures on macos 
when using `--with-zlib=bundled`?

With this change the build now passes (tested both with bundled and system zlib 
variants).

tier1, tier2 and tier3 testing has been done and no related failures have been 
noticed.

-

Commit messages:
 - fix build issues with bundled zlib on macosx

Changes: https://git.openjdk.java.net/jdk/pull/8651/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8651=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286582
  Stats: 34 lines in 3 files changed: 30 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8651.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8651/head:pull/8651

PR: https://git.openjdk.java.net/jdk/pull/8651


Integrated: 8285915: failure_handler: gather the contents of /etc/hosts file

2022-05-02 Thread Jaikiran Pai
On Fri, 29 Apr 2022 11:28:32 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which addresses 
> https://bugs.openjdk.java.net/browse/JDK-8285915?
> 
> With this change, the environment details collected by the failure handler 
> will now include the contents of the `/etc/hosts/` file, which can be useful 
> in certain cases when debugging network tests that fail.
> 
> Testing done (on macOS):
> 
> 
> cd test/failure_handler 
> make test
> 
> Then verified that the generated environment.html had the `/etc/hosts` file 
> content

This pull request has now been integrated.

Changeset: 45ca81ff
Author:Jaikiran Pai 
URL:   
https://git.openjdk.java.net/jdk/commit/45ca81ff5f25ce7927c5debc2f89b41246b91b92
Stats: 13 lines in 3 files changed: 10 ins; 0 del; 3 mod

8285915: failure_handler: gather the contents of /etc/hosts file

Reviewed-by: dfuchs, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/8466


Re: RFR: 8285915: failure_handler: gather the contents of /etc/hosts file [v2]

2022-05-02 Thread Jaikiran Pai
On Mon, 2 May 2022 06:25:28 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this change which addresses 
>> https://bugs.openjdk.java.net/browse/JDK-8285915?
>> 
>> With this change, the environment details collected by the failure handler 
>> will now include the contents of the `/etc/hosts/` file, which can be useful 
>> in certain cases when debugging network tests that fail.
>> 
>> Testing done (on macOS):
>> 
>> 
>> cd test/failure_handler 
>> make test
>> 
>> Then verified that the generated environment.html had the `/etc/hosts` file 
>> content
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   print contents of hosts file in windows failure handler

Thank you Erik and Daniel for the reviews.

-

PR: https://git.openjdk.java.net/jdk/pull/8466


Re: RFR: 8285915: failure_handler: gather the contents of /etc/hosts file [v2]

2022-05-02 Thread Jaikiran Pai
> Can I please get a review of this change which addresses 
> https://bugs.openjdk.java.net/browse/JDK-8285915?
> 
> With this change, the environment details collected by the failure handler 
> will now include the contents of the `/etc/hosts/` file, which can be useful 
> in certain cases when debugging network tests that fail.
> 
> Testing done (on macOS):
> 
> 
> cd test/failure_handler 
> make test
> 
> Then verified that the generated environment.html had the `/etc/hosts` file 
> content

Jaikiran Pai has updated the pull request incrementally with one additional 
commit since the last revision:

  print contents of hosts file in windows failure handler

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8466/files
  - new: https://git.openjdk.java.net/jdk/pull/8466/files/673fd0bd..8591d0c0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8466=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8466=00-01

  Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8466.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8466/head:pull/8466

PR: https://git.openjdk.java.net/jdk/pull/8466


Re: RFR: 8285915: failure_handler: gather the contents of /etc/hosts file

2022-05-02 Thread Jaikiran Pai
On Fri, 29 Apr 2022 11:28:32 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which addresses 
> https://bugs.openjdk.java.net/browse/JDK-8285915?
> 
> With this change, the environment details collected by the failure handler 
> will now include the contents of the `/etc/hosts/` file, which can be useful 
> in certain cases when debugging network tests that fail.
> 
> Testing done (on macOS):
> 
> 
> cd test/failure_handler 
> make test
> 
> Then verified that the generated environment.html had the `/etc/hosts` file 
> content

Hello Erik, I've updated the PR to include capturing the contents of hosts file 
even on Windows. I've tested this against an internal Windows system where it 
correct shows the content:



[2022-05-02 06:08:52] [C:\cygwin\bin\bash.exe, -c, cat 
$WINDIR/System32/drivers/etc/hosts] timeout=2

# 

# localhost name resolution is handled within DNS itself.
#   127.0.0.1   localhost
#   ::1 localhost

[2022-05-02 06:08:52] exit code: 0 time: 57 ms


-

PR: https://git.openjdk.java.net/jdk/pull/8466


Re: RFR: 8285915: failure_handler: gather the contents of /etc/hosts file

2022-04-29 Thread Jaikiran Pai
On Fri, 29 Apr 2022 11:28:32 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this change which addresses 
> https://bugs.openjdk.java.net/browse/JDK-8285915?
> 
> With this change, the environment details collected by the failure handler 
> will now include the contents of the `/etc/hosts/` file, which can be useful 
> in certain cases when debugging network tests that fail.
> 
> Testing done (on macOS):
> 
> 
> cd test/failure_handler 
> make test
> 
> Then verified that the generated environment.html had the `/etc/hosts` file 
> content

Hello Erik,

> Not sure if it's relevant, but did you consider doing this for Windows as 
> well? The file is located at `"$WINDIR/System32/drivers/etc/hosts"`.

I hadn't investigated what the corresponding command would be for Windows, so 
had left it out. Quick question - the path you note, is that even applicable 
for x64? I see that it has a "System32" so just curious. I'll experiment a bit 
shortly against some CI setups to see how this goes on Windows.

-

PR: https://git.openjdk.java.net/jdk/pull/8466


Re: Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?

2022-03-29 Thread Jaikiran Pai



On 29/03/22 6:12 pm, David Holmes wrote:

On 29/03/2022 7:20 pm, Magnus Ihse Bursie wrote:

On 2022-03-29 03:42, Jaikiran Pai wrote:

Hello Magnus,

On 28/03/22 5:21 pm, Magnus Ihse Bursie wrote:

On 2022-03-28 09:03, David Holmes wrote:

On 28/03/2022 4:56 pm, Alan Bateman wrote:

On 28/03/2022 07:46, David Holmes wrote:

Hi Jai,

It isn't obvious to me that the bundled sources are actually 
intended to build on macOS. There's no include of unistd.h to 
get the lseek definition.
I think the context here is that Jai is chasing an issue that may 
be bug in the libz on macOS. Building the bundled version and 
comparing results would lead to useful information. AFAIK, the 
system zlib has always been used since 7u4 when the macOS was 
added. I think the Apple JDK did the same. So there may be a 
small bit of "porting" to do, adds include files, etc.


Okay, I suggest adding the include of unistd.h as a starter then. 
But Jai should note that this is not a build issue per-se - making 
those sources buildable on all desired platforms is the job of the 
owners of that src in the OpenJDK.
I agree fully, but I'd also like to add that it is not clear that 
this is not "supposed" to work. If bundled zlib is not buildable on 
macOS (that was news to me), then the correct build system behavior 
should have been to block it in configure. So, Jaikiran, if you 
intend to get this to work on macOS, great! If you're not taking 
that route, please let me know so I can open a bug on prohibiting 
bundled zlib on maccOS from configure.



I used David's suggestion to add that include header:

diff --git a/src/java.base/share/native/libzip/zlib/gzlib.c 
b/src/java.base/share/native/libzip/zlib/gzlib.c

index a814dd8c7b6..9df629d4205 100644
--- a/src/java.base/share/native/libzip/zlib/gzlib.c
+++ b/src/java.base/share/native/libzip/zlib/gzlib.c
@@ -35,6 +35,7 @@
 #if defined(_LARGEFILE64_SOURCE) && _LFS64_LARGEFILE-0
 #  define LSEEK lseek64
 #else
+#  include 
 #  define LSEEK lseek
 #endif
 #endif

That did help me get past the lseek error, but there are some more 
errors (as noted in that log) which need to be addressed too. I 
don't have the necessary knowledge of C ecosystem to decide what set 
of includes and whether those includes need to be conditional for 
macOS, are required to get this building. The other thing to 
consider with this code is that it is bundled code and the real code 
lies in a separate upstream zlib project[1]. So I think whatever 
changes we do here might have to be co-ordinated back to that project.
Maybe. But when we bundle code we also kind of rip it out of context. 
It might very well be that the upstream code has an automake system 
which generates a config.h that has proper includes and defines to 
get this to build. I mean, obviously there is an existing system zlib 
on macOS, so *someone* has gotten it to build.


Our sources are for a very old version:

1.2.11 (15 Jan 2017)

Until 2 days back, that was the latest released version of that library. 
A couple of days back 1.2.12 has been released (or at least tagged) 
https://github.com/madler/zlib/tags



-Jaikiran



Re: Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?

2022-03-29 Thread Jaikiran Pai

Hello Magnus,

On 29/03/22 2:50 pm, Magnus Ihse Bursie wrote:

On 2022-03-29 03:42, Jaikiran Pai wrote:

Hello Magnus,

On 28/03/22 5:21 pm, Magnus Ihse Bursie wrote:

On 2022-03-28 09:03, David Holmes wrote:

On 28/03/2022 4:56 pm, Alan Bateman wrote:

On 28/03/2022 07:46, David Holmes wrote:

Hi Jai,

It isn't obvious to me that the bundled sources are actually 
intended to build on macOS. There's no include of unistd.h to get 
the lseek definition.
I think the context here is that Jai is chasing an issue that may 
be bug in the libz on macOS. Building the bundled version and 
comparing results would lead to useful information. AFAIK, the 
system zlib has always been used since 7u4 when the macOS was 
added. I think the Apple JDK did the same. So there may be a small 
bit of "porting" to do, adds include files, etc.


Okay, I suggest adding the include of unistd.h as a starter then. 
But Jai should note that this is not a build issue per-se - making 
those sources buildable on all desired platforms is the job of the 
owners of that src in the OpenJDK.
I agree fully, but I'd also like to add that it is not clear that 
this is not "supposed" to work. If bundled zlib is not buildable on 
macOS (that was news to me), then the correct build system behavior 
should have been to block it in configure. So, Jaikiran, if you 
intend to get this to work on macOS, great! If you're not taking 
that route, please let me know so I can open a bug on prohibiting 
bundled zlib on maccOS from configure.



I used David's suggestion to add that include header:

diff --git a/src/java.base/share/native/libzip/zlib/gzlib.c 
b/src/java.base/share/native/libzip/zlib/gzlib.c

index a814dd8c7b6..9df629d4205 100644
--- a/src/java.base/share/native/libzip/zlib/gzlib.c
+++ b/src/java.base/share/native/libzip/zlib/gzlib.c
@@ -35,6 +35,7 @@
 #if defined(_LARGEFILE64_SOURCE) && _LFS64_LARGEFILE-0
 #  define LSEEK lseek64
 #else
+#  include 
 #  define LSEEK lseek
 #endif
 #endif

That did help me get past the lseek error, but there are some more 
errors (as noted in that log) which need to be addressed too. I don't 
have the necessary knowledge of C ecosystem to decide what set of 
includes and whether those includes need to be conditional for macOS, 
are required to get this building. The other thing to consider with 
this code is that it is bundled code and the real code lies in a 
separate upstream zlib project[1]. So I think whatever changes we do 
here might have to be co-ordinated back to that project.
Maybe. But when we bundle code we also kind of rip it out of context. 
It might very well be that the upstream code has an automake system 
which generates a config.h that has proper includes and defines to get 
this to build. I mean, obviously there is an existing system zlib on 
macOS, so *someone* has gotten it to build.


I think you are right about this. In my experiments, I decided to skip 
the "bundled" approach and instead just build their upstream zlib 
(afresh) locally and that just worked out of the box on this same setup.


-Jaikiran



Is --with-zlib=bundled broken on MacOS aarch64 12.2.1?

2022-03-27 Thread Jaikiran Pai
I'm using the following set of commands to build the JDK on my Mac M1 
12.2.1 version. Specifically, I use the --with-zlib=bundled option 
during configure:


bash configure --with-boot-jdk= --with-zlib=bundled

make clean

make images

This runs into build errors during make images. The configure summary is 
as follows and the error logs (snippet) is pasted at the end of this 
mail. Is this some issue with the versions of compiler/tools on my setup 
or is this genuinely broken?


configure log summary:

The existing configuration has been successfully updated in
/home/me/jdk/build/macosx-aarch64-server-release
using configure arguments 
'--with-boot-jdk=/home/me/java/openjdk/jdk-17.0.2.jdk/Contents/Home 
--with-zlib=bundled'.


Configuration summary:
* Name:   macosx-aarch64-server-release
* Debug level:    release
* HS debug level: product
* JVM variants:   server
* JVM features:   server: 'cds compiler1 compiler2 dtrace epsilongc g1gc 
jfr jni-check jvmci jvmti management parallelgc serialgc services 
shenandoahgc vm-structs zgc'

* OpenJDK target: OS: macosx, CPU architecture: aarch64, address length: 64
* Version string: 19-internal-adhoc.jaikiran.open (19-internal)
* Source date:    Determined at build time

Tools summary:
* Boot JDK:   openjdk version "17.0.2" 2022-01-18 OpenJDK Runtime 
Environment (build 17.0.2+8-86) OpenJDK 64-Bit Server VM (build 
17.0.2+8-86, mixed mode, sharing) (at 
/home/me/java/openjdk/jdk-17.0.2.jdk/Contents/Home)

* Toolchain:  clang (clang/LLVM from Xcode 13.2.1)
* C Compiler: Version 13.0.0 (at /usr/bin/clang)
* C++ Compiler:   Version 13.0.0 (at /usr/bin/clang++)

Build performance summary:
* Build jobs: 8
* Memory limit:   16384 MB


build errors:

/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:276:9: 
error: implicit declaration of function 'lseek' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    LSEEK(state->fd, 0, SEEK_END);  /* so gzoffset() is correct */
    ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: 
expanded from macro 'LSEEK'

#  define LSEEK lseek
    ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:276:9: note: 
did you mean 'fseek'?
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: 
expanded from macro 'LSEEK'

#  define LSEEK lseek
    ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/stdio.h:162:6: 
note: 'fseek' declared here

int  fseek(FILE *, long, int);
 ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:282:24: 
error: implicit declaration of function 'lseek' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    state->start = LSEEK(state->fd, 0, SEEK_CUR);
   ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: 
expanded from macro 'LSEEK'

#  define LSEEK lseek
    ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:383:9: 
error: implicit declaration of function 'lseek' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    if (LSEEK(state->fd, state->start, SEEK_SET) == -1)
    ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: 
expanded from macro 'LSEEK'

#  define LSEEK lseek
    ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:424:15: 
error: implicit declaration of function 'lseek' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    ret = LSEEK(state->fd, offset - state->x.have, SEEK_CUR);
  ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: 
expanded from macro 'LSEEK'

#  define LSEEK lseek
    ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:520:14: 
error: implicit declaration of function 'lseek' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    offset = LSEEK(state->fd, 0, SEEK_CUR);
 ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzlib.c:38:17: note: 
expanded from macro 'LSEEK'

#  define LSEEK lseek
    ^
5 errors generated.
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzread.c:59:15: 
error: implicit declaration of function 'read' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    ret = read(state->fd, buf + *have, get);
  ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzread.c:59:15: 
note: did you mean 'fread'?
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk/usr/include/stdio.h:158:9: 
note: 'fread' declared here
size_t   fread(void * __restrict __ptr, size_t __size, size_t __nitems, 
FILE * __restrict __stream);

 ^
/home/me/jdk/src/java.base/share/native/libzip/zlib/gzread.c:675:11: 
error: implicit declaration of function 'close' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    ret = close(state->fd);
  

Re: RFR: JDK-8276422 Add command-line option to disable finalization

2021-11-17 Thread Jaikiran Pai
On Thu, 18 Nov 2021 01:34:36 GMT, Stuart Marks  wrote:

> Pretty much what it says. The new option controls a static member in 
> InstanceKlass that's consulted to determine whether the finalization 
> machinery is activated for instances when a class is loaded. A new native 
> method is added so that this state can be queried from Java. This is used to 
> control whether a finalizer thread is created and to disable the `System` and 
> `Runtime::runFinalization` methods. Includes tests for the above.

src/java.base/share/classes/java/lang/ref/Finalizer.java line 195:

> 193: 
> 194: static {
> 195: if (Holder.ENABLED) {

Hello Stuart,
My understanding of the the lazy `Holder` is that it's there to delay the 
static initialization of the code that's part of the `Holder`. In this case 
here, the `Holder` is being used right within the `static` block of the 
`Finalizer` class, that too as the first thing. In this case, is that `Holder` 
class necessary?

-

PR: https://git.openjdk.java.net/jdk/pull/6442


Re: RFR: 8276400: openjdk image Jars, Zips and Jmods built from the same source are not reproducible

2021-11-05 Thread Jaikiran Pai
On Thu, 4 Nov 2021 20:56:45 GMT, Andrew Leonard  wrote:

> This PR enables reproducible Jars, Jmods and openjdk image zip files 
> (eg.src.zip).
> It provides support for SOURCE_DATE_EPOCH for Jar, Jmod and underlying 
> ZipOutputStream's.
> It fixes the following keys issues relating to reproducibility:
> - Jar and ZipOutputStream are not SOURCE_DATE_EPOCH aware
>   - Jar and ZipOutputStream now detect SOURCE_DATE_EPOCH environment setting
> - Jar and Jmod file content ordering was non-determinsitic
>   - Fixes to Jar and Jmod main's to ensure sorted classes content ordering
> - openjdk image zip file generation used "zip" which is non-determinsitic
>   - New openjdk build tool "GenerateZip" which produces the final 
> determinsitic zip files as part of the build and also detects 
> SOURCE_DATE_EPOCH
> 
> Signed-off-by: Andrew Leonard 

make/jdk/src/classes/build/tools/generatezip/GenerateZip.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights 
> reserved.

Hello @andrew-m-leonard, for a new file, the year should just be stated once. 
So just `2021,`

-

PR: https://git.openjdk.java.net/jdk/pull/6268


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v3]

2021-09-22 Thread Jaikiran Pai
On Wed, 22 Sep 2021 15:20:21 GMT, Julia Boes  wrote:

> > Thanks for sharing your experience on this, it's appreciated. 0.0.0.0 is 
> > common default for Apache httpd [1], Ngnix [2], the Python web server [3]. 
> > This being said, I want to make sure we're taking the right decision here 
> > so let me seek some further advice on this.
> > [1] http://httpd.apache.org/docs/2.4/bind.html
> > [2] https://docs.nginx.com/nginx/admin-guide/web-server/web-server/
> > [3] https://github.com/python/cpython/blob/3.4/Lib/http/server.py
> 
> Further review concluded that a default binding to 0.0.0.0 creates too a high 
> level of exposure, particularly for a low-threshold utility like this server. 
> Binding to an unrestricted address is a known way for attackers to launch a 
> Denial-of-Service attack, classified by MITRE as CWE-1327 [1]. We therefore 
> update the default binding to the loopback address and amend the help output 
> with information on how to bind to 0.0.0.0

Thank you Julia for considering this input and coordinating the change.

-

PR: https://git.openjdk.java.net/jdk/pull/5505


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v3]

2021-09-17 Thread Jaikiran Pai

Hello Julia,

On 17/09/21 3:14 pm, Julia Boes wrote:

On Thu, 16 Sep 2021 14:05:52 GMT, Jaikiran Pai  wrote:


Julia Boes has updated the pull request incrementally with one additional 
commit since the last revision:

   correct path handling

src/jdk.httpserver/share/classes/module-info.java line 55:


53:  *  [-o none|info|verbose] [-h to show 
options]
54:  *Options:
55:  *-b, --bind-address- Address to bind to. Default: 0.0.0.0 (all 
interfaces).

I understand that the purpose of this simple server is for development and 
testing only. But even then, for security considerations, would it be more 
appropriate to default the bind address to a loopback address instead of making 
it accessible potentially to entire public? In the past, application servers 
which used to bind to all interfaces by default have now moved to using the 
loopback address as a default to avoid such accidental exposing of the server.

We did consider defaulting to the loopback address, but this would limit the 
usefulness of the server too much in the default configuration as it can only 
be accessed from localhost. The goal of this JEP is an out-of-the-box web 
server with easy setup, so in this case we favour usability. The purpose of a 
web server is to make things accessible on the web so it is assumed that the 
developer is familiar with the terms this comes with.

The concern of accidental exposure is alleviated by the informative output 
printed at start up, e.g.
```~ $ java-sb -m jdk.httpserver
Serving /current/directory and subdirectories on 0.0.0.0:8000
http://123.456.7.891:8000/ ...


I think this is still a really big risk. I say this based on some of my 
past experience with application servers (JBoss AS) where in older 
releases it used to do this same thing of binding to 0.0.0.0 by default 
and how that had lead to numerous (production) instances ending up being 
vulnerable. In the case there, the management console ended up being 
exposed and almost anyone over the internet could just access it to 
shutdown the server (through a JMX MBean).


In the case of this simple server being proposed, I think it's a lot 
more riskier because unlike in the case of the application servers where 
the server would have preventive measures that wouldn't allow local 
filesystem access, the current server being proposed will end up 
exposing the user's local filesystem to the internet. It's my opinion 
and experience that log messages no matter how much they scream out, 
won't prevent this default out of the box usage.


I'm not saying 0.0.0.0 should be disabled, but instead, IMO it should be 
the user who should explicitly use -b 0.0.0.0 to do that, so that they 
are at least responsible and aware of what they are doing.


-Jaikiran




Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server [v3]

2021-09-16 Thread Jaikiran Pai
On Thu, 16 Sep 2021 10:14:47 GMT, Julia Boes  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> Julia Boes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   correct path handling

src/jdk.httpserver/share/classes/module-info.java line 55:

> 53:  *  [-o none|info|verbose] [-h to show 
> options]
> 54:  *Options:
> 55:  *-b, --bind-address- Address to bind to. Default: 0.0.0.0 (all 
> interfaces).

I understand that the purpose of this simple server is for development and 
testing only. But even then, for security considerations, would it be more 
appropriate to default the bind address to a loopback address instead of making 
it accessible potentially to entire public? In the past, application servers 
which used to bind to all interfaces by default have now moved to using the 
loopback address as a default to avoid such accidental exposing of the server.

-

PR: https://git.openjdk.java.net/jdk/pull/5505


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-14 Thread Jaikiran Pai
On Wed, 15 Sep 2021 01:54:40 GMT, Florent Guillaume 
 wrote:

> FWIW `.z` is the extension of the old Unix `compress` program.

Thank you Florent, I wasn't aware of that.

-

PR: https://git.openjdk.java.net/jdk/pull/5505


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-14 Thread Jaikiran Pai
On Tue, 14 Sep 2021 15:30:31 GMT, Jaikiran Pai  wrote:

>> This change implements a simple web server that can be run on the 
>> command-line with `java -m jdk.httpserver`.
>> 
>> This is facilitated by adding an entry point for the `jdk.httpserver` 
>> module, an implementation class whose main method is run when the above 
>> command is executed. This is the first such module entry point in the JDK.
>> 
>> The server is a minimal HTTP server that serves the static files of a given 
>> directory, similar to existing alternatives on other platforms and 
>> convenient for testing, development, and debugging.
>> 
>> Additionally, a small API is introduced for programmatic creation and 
>> customization.
>> 
>> Testing: tier1-3.
>
> src/java.base/windows/classes/sun/net/www/content-types.properties line 30:
> 
>> 28: application/octet-stream: \
>> 29:  description=Generic Binary Stream;\
>> 30:  file_extensions=.saveme,.dump,.hqx,.arc,.obj,.lib,.bin,.exe,.gz
> 
> Hello Julia,
> Is this an intentional change, to remove the mapping of `.zip` to 
> `application/octet-stream`? In a later part of this patch there's a commented 
> out test `testCommonExtensions` which deals with these extension types and 
> that has a link to 
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
>  which states that `.zip` should be mapped to `application/zip` instead of 
> the current `application/octet-stream`, so I'm guessing this changed line is 
> intentional.
> 
> On an unrelated note, the unix variant of this file 
> `src/java.base/unix/classes/sun/net/www/content-types.properties` 
> interestingly uses `.z` for `.zip`? Commit history on that file doesn't 
> provide any hint on whether that is intentional either.

I think you can ignore my comment above. I went and checked the 
`content-types.properties` in their current state for both unix and windows and 
they already have a separate `application/zip` which is mapped to `.zip`. So I 
think this above change shouldn't impact anything.

-

PR: https://git.openjdk.java.net/jdk/pull/5505


Re: RFR: 8245095: Implementation of JEP 408: Simple Web Server

2021-09-14 Thread Jaikiran Pai
On Tue, 14 Sep 2021 08:52:37 GMT, Julia Boes  wrote:

> This change implements a simple web server that can be run on the 
> command-line with `java -m jdk.httpserver`.
> 
> This is facilitated by adding an entry point for the `jdk.httpserver` module, 
> an implementation class whose main method is run when the above command is 
> executed. This is the first such module entry point in the JDK.
> 
> The server is a minimal HTTP server that serves the static files of a given 
> directory, similar to existing alternatives on other platforms and convenient 
> for testing, development, and debugging.
> 
> Additionally, a small API is introduced for programmatic creation and 
> customization.
> 
> Testing: tier1-3.

src/java.base/windows/classes/sun/net/www/content-types.properties line 30:

> 28: application/octet-stream: \
> 29:   description=Generic Binary Stream;\
> 30:   file_extensions=.saveme,.dump,.hqx,.arc,.obj,.lib,.bin,.exe,.gz

Hello Julia,
Is this an intentional change, to remove the mapping of `.zip` to 
`application/octet-stream`? In a later part of this patch there's a commented 
out test `testCommonExtensions` which deals with these extension types and that 
has a link to 
https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types
 which states that `.zip` should be mapped to `application/zip` instead of the 
current `application/octet-stream`, so I'm guessing this changed line is 
intentional.

On an unrelated note, the unix variant of this file 
`src/java.base/unix/classes/sun/net/www/content-types.properties` interestingly 
uses `.z` for `.zip`? Commit history on that file doesn't provide any hint on 
whether that is intentional either.

-

PR: https://git.openjdk.java.net/jdk/pull/5505


Re: Proposal: JDK-8231640 - (prop) Canonical property storage

2021-08-26 Thread Jaikiran Pai

Hello Magnus,


On 25/08/21 5:33 pm, Magnus Ihse Bursie wrote:

...
One thing I do remember is the JDK build (through the make files) 
would have certain Java code it would call to do some build steps. Is 
there a easy way to find all such build related Java files within the 
JDK? I would like to see if there are any references/calls to this 
method from those build files. I am doing some searches myself, but 
knowing where to search will give me more confidence that I haven't 
missed out any.


The java buildtool sources are located in the "make" directory, more 
specifically in "make/jdk/src", "make/langtools/src" and "make/src" 
(yeah, I know -- a cleanup is way overdue). I did a quick search now 
but could not find any references to Properties.store().


Thank you for that. I went ahead and verified it again and like you 
note, I don't see any usages in this code.



I'm trying to remember if we create properties during the build... We 
have several instances where .properties files in the Java source code 
are converted to hard-coded classes (for performance), and other where 
.properties files are copied verbatim. Ah, right, they are "cleaned" 
beforehand. We used to do this by a Java program, but nowadays they 
are mangled by sed. I think replacing that sed script with a trivial 
Java program doing storeCanonical() would be on the list of things to 
do. I can assist with that, when you are starting to get your 
implementation done.


Thank you. Once the initial draft version is ready, I will contact you.




We might also write something as part of the jlink process that gets 
embedded in the resulting jimage; not quite sure about that. You 
should find that code in the normal src/ codebase though.


I had a look at the the jlink and image building code and I haven't 
found any references/usages in this area.


-Jaikiran



Re: Proposal: JDK-8231640 - (prop) Canonical property storage

2021-08-25 Thread Jaikiran Pai



On 25/08/21 5:33 pm, Magnus Ihse Bursie wrote:

...

The problem is with the time stamp, which the spec states should be 
present. I understand why changing this might need a new method. But I 
think we should try to steer users to this new method. Otherwise it is 
likely not to be used by those who should use it (the programmers) to 
the detrimental effect of users who get properties file which change 
for no good reason. Having an "attractive" name is definitely part of 
that. The name should scream "use me as a first hand choice for 
storing properties to a file". I don't really get that feeling from 
storeCanonical().


I'm not saying we should do this, but one other way to scream, is to 
deprecate (but not for removal) the old "store" APIs and point the users 
to the new ones (if we introduce them). Similar to what "save" API in 
that same class currently does.


-Jaikiran



Re: RFR: 8268860: Windows-Aarch64 build is failing in GitHub actions

2021-07-13 Thread Jaikiran Pai
On Mon, 5 Jul 2021 21:46:26 GMT, David Holmes  wrote:

> It is still unsettling that we don't know what changed

More as an FYI and something that might help investigate similar failures in 
future. Each run of the GitHub action job generates a log which is available 
for download publicly (and/or can be viewed raw in the browser). For example, I 
just picked a random PR in the current queue 
https://github.com/openjdk/jdk/pull/4759 and clicked the "Details" link beside 
the "Pre-submit tests - Windows aarch64 - Build" under GitHub actions on that 
PR. Then click on the "Windows aarch64 (build debug)" link on that job run 
page. On the page that shows up, there's a visual log viewer of that job run. 
There's a "..." top right of that widget, which has an option which allows you 
to view the raw logs in the browser. Doing so will show the entire log of that 
run. One part of the log contains details about the virtual environment used 
for that run and it has some interesting/useful details like:


2021-07-12T20:41:25.6215958Z ##[group]Virtual Environment
2021-07-12T20:41:25.6216502Z Environment: windows-2019
2021-07-12T20:41:25.6216982Z Version: 20210628.1
2021-07-12T20:41:25.6217813Z Included Software: 
https://github.com/actions/virtual-environments/blob/win19/20210628.1/images/win/Windows2019-Readme.md
2021-07-12T20:41:25.6219152Z Image Release: 
https://github.com/actions/virtual-environments/releases/tag/win19%2F20210628.1
2021-07-12T20:41:25.6219863Z ##[endgroup]

So the details about the virtual environment used for that run are listed in 
the log and additional a (version controlled) link is provided which has the 
full list of softwares that were installed on that VM:


> 2021-07-12T20:41:25.6217813Z Included Software: 
> https://github.com/actions/virtual-environments/blob/win19/20210628.1/images/win/Windows2019-Readme.md


This is a version controlled file and available in their github repo for public 
access as well as review. The commit history shows the exact version of which 
software changed over time. I don't know when the Windows-Aarch64 build started 
failing first, otherwise we could probably check the history on that file 
around that time to know what exactly caused this issue.

-

PR: https://git.openjdk.java.net/jdk/pull/4681


Re: JDK building from source - Prevent user name leaking into the generated binary?

2021-01-10 Thread Jaikiran Pai

Hello David,

On 11/01/21 12:02 pm, David Holmes wrote:

Hi Jaikiran,

On 11/01/2021 2:43 pm, Jaikiran Pai wrote:
I build the JDK from source using "make images". When I use that JDK 
binary, the user name of the user who built the binary "leaks" into 
some of the runtime system properties and (of course as a result) 
into the output of java -version. Here's the output of java -version 
of such a built JDK:



openjdk version "16-internal" 2021-03-16
OpenJDK Runtime Environment (build 16-internal+0-adhoc.jaikiran.jdk16)
OpenJDK 64-Bit Server VM (build 16-internal+0-adhoc.jaikiran.jdk16, 
mixed mode, sharing)



Notice the "jaikiran" in that output - that's the user name who built 
the JDK. Furthermore, this also ends up in runtime properties like:



java.runtime.version = 16-internal+0-adhoc.jaikiran.jdk16
...
java.vm.version = 16-internal+0-adhoc.jaikiran.jdk16


Is there a way/option that I can use while building the JDK that will 
prevent this user name leaking into the built binary?


Yes. You have full control over the version string at configure time. 
See:


make/autoconf/jdk-version.m4

As the format of the version string is controlled by a JEP and has a 
lot of rules there are actually quite a number of configure flags that 
can affect the version string. Your simplest option may just be to use 
--with-version-string="your version string".


Thank you for pointing to me that file. Yes, this option is good enough 
for me to override the default one.






Better still can that option be the default?


Not in my opinion. :) We find it useful to see what binary was used in 
detail.


Fair enough :)


-Jaikiran



JDK building from source - Prevent user name leaking into the generated binary?

2021-01-10 Thread Jaikiran Pai
I build the JDK from source using "make images". When I use that JDK 
binary, the user name of the user who built the binary "leaks" into some 
of the runtime system properties and (of course as a result) into the 
output of java -version. Here's the output of java -version of such a 
built JDK:



openjdk version "16-internal" 2021-03-16
OpenJDK Runtime Environment (build 16-internal+0-adhoc.jaikiran.jdk16)
OpenJDK 64-Bit Server VM (build 16-internal+0-adhoc.jaikiran.jdk16, 
mixed mode, sharing)



Notice the "jaikiran" in that output - that's the user name who built 
the JDK. Furthermore, this also ends up in runtime properties like:



java.runtime.version = 16-internal+0-adhoc.jaikiran.jdk16
...
java.vm.version = 16-internal+0-adhoc.jaikiran.jdk16


Is there a way/option that I can use while building the JDK that will 
prevent this user name leaking into the built binary? Better still can 
that option be the default?



-Jaikiran




Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance) [v3]

2020-09-15 Thread Jaikiran Pai
> Can I please get a review and a sponsor for this patch which fixes the issue 
> reported in
> https://bugs.openjdk.java.net/browse/JDK-8244706?
> The commit here sets the `OS` header flag to `255` (which represents 
> `unknown`) as noted in [1]. A new test has been
> included in this commit to verify the change. Furthermore, this doesn't 
> impact the `java.util.zip.GZIPInputStream`
> since it ignores [2] this header value while reading the headers from the 
> stream.  [1]
> https://tools.ietf.org/html/rfc1952#page-7 [2]
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173

Jaikiran Pai has updated the pull request incrementally with one additional 
commit since the last revision:

  Improve the comment for the OS_UNKNOWN field

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/130/files
  - new: https://git.openjdk.java.net/jdk/pull/130/files/159684de..7679b119

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=130=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=130=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/130.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/130/head:pull/130

PR: https://git.openjdk.java.net/jdk/pull/130


Re: RFR: 8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 non-compliance) [v2]

2020-09-15 Thread Jaikiran Pai
> Can I please get a review and a sponsor for this patch which fixes the issue 
> reported in
> https://bugs.openjdk.java.net/browse/JDK-8244706?
> The commit here sets the `OS` header flag to `255` (which represents 
> `unknown`) as noted in [1]. A new test has been
> included in this commit to verify the change. Furthermore, this doesn't 
> impact the `java.util.zip.GZIPInputStream`
> since it ignores [2] this header value while reading the headers from the 
> stream.  [1]
> https://tools.ietf.org/html/rfc1952#page-7 [2]
> https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#L173

Jaikiran Pai has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental
views will show differences compared to the previous content of the PR. The 
pull request contains one new commit since
the last revision:

  8244706: GZIP "OS" header flag hard-coded to 0 instead of 255 (RFC 1952 
non-compliance)
  Reviewed-By:

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/130/files
  - new: https://git.openjdk.java.net/jdk/pull/130/files/afad1261..159684de

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=130=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=130=00-01

  Stats: 5 lines in 2 files changed: 1 ins; 2 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/130.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/130/head:pull/130

PR: https://git.openjdk.java.net/jdk/pull/130


Too many "no symbols" log messages during build

2020-02-11 Thread Jaikiran Pai
I just updated my local jdk repo to latest upstream and triggered a
"make images" after a "make clean". All went fine, except, I saw these
"no symbols" log messages while the build was in progress:

...

Compiling 1003 files for jdk.hotspot.agent
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
no symbols
Creating support/modules_libs/java.base/libverify.dylib from 1 file(s)



I don't remember seeing those messages before, whenever I have built the
jdk repo. In the current form, those messages don't appear to be useful
either. Are those logs intentional?

-Jaikiran



Re: RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

2019-07-29 Thread Jaikiran Pai
Thank you Erik.

-Jaikiran

On Monday, July 29, 2019, Erik Joelsson  wrote:

> This looks good to me. I can sponsor the change.
>
> /Erik
>
> On 2019-07-29 04:12, Jaikiran Pai wrote:
>
>> I got some more inputs from Stuart Marks on the linked JBS issue[1] and
>> I've incorporated those in the latest updated webrev which is at [2].
>>
>> I've tested this latest change locally on mercurial version 5.0.1 and it
>> has worked as expected (now ignores JTreport and JTwork from the root as
>> well as sub directories). Plus, with this change I haven't seen any
>> files/directories that are now being unexpectedly not ignored.
>>
>> Anyone willing to review and sponsor this please?
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8227170
>>
>> [2] http://cr.openjdk.java.net/~jpai/webrev/8227170/3/webrev/
>>
>> -Jaikiran
>>
>> On 12/07/19 4:04 PM, Jaikiran Pai wrote:
>>
>>> Hello Dean,
>>>
>>> Thank you for testing this. I have now updated the webrev to use the
>>> JTreport/* syntax and have also verified that it works for me too. It
>>> correctly ignores both root level JTreport and JTwork directories and
>>> any such sub-directories. I am on a macOS with:
>>>
>>> hg --version
>>> Mercurial Distributed SCM (version 5.0.1)
>>> (see https://mercurial-scm.org for more information)
>>>
>>> The updated webrev is at
>>> http://cr.openjdk.java.net/~jpai/webrev/8227170/2/webrev/
>>>
>>> -Jaikiran
>>>
>>> On 11/07/19 1:16 AM, dean.l...@oracle.com wrote:
>>>
>>>> The ** syntax isn't working for me in Mercurial 2.9, but the following:
>>>>
>>>> syntax: glob
>>>> JTreport/*
>>>> JTwork/*
>>>>
>>>> seems to work in both 2.9 and 4.6.1.  This also seems to work:
>>>>
>>>> syntax: glob
>>>> JTreport
>>>> JTwork
>>>>
>>>> but it matches files and not just directories.
>>>>
>>>> dl
>>>>
>>>> On 7/10/19 4:48 AM, Jaikiran Pai wrote:
>>>>
>>>>> Ping. Anyone willing to review and sponsor this change?
>>>>>
>>>>> -Jaikiran
>>>>>
>>>>> On 03/07/19 5:43 PM, Jaikiran Pai wrote:
>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> Can I please get a review and a sponsor for the patch for the
>>>>>> enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8227170
>>>>>> ?
>>>>>>
>>>>>> The webrev containing the patch is here
>>>>>> http://cr.openjdk.java.net/~jpai/webrev/8227170/00/webrev/
>>>>>>
>>>>>> For some context about this change - there's been some discussion on
>>>>>> this in the jdk-dev list here
>>>>>> https://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003076.html
>>>>>> .
>>>>>>
>>>>>> The patch in the webrev, does the following things:
>>>>>>
>>>>>> - In addition to ignoring the JTwork and JTreport sub-directories,
>>>>>> this
>>>>>> now also ignores these directories at the root of the repo. This patch
>>>>>> uses "syntax: glob" to easily ignore such directories, instead of the
>>>>>> (default) "regex" syntax. In the jdk-dev mailing list discussion,
>>>>>> Gustavo suggested to continue using the regex syntax and listing the
>>>>>> following in the .hgignore:
>>>>>>
>>>>>> .*JTreport/.*
>>>>>>
>>>>>> .*JTwork/.*
>>>>>>
>>>>>> But that would then end up ignoring directories of the form
>>>>>> "foobarJTreport" and files under it like "foobarJTreport/blah.txt". I
>>>>>> guess we can still stick with a regex syntax and come up with a regex
>>>>>> which ignores these directories both at the root as well as
>>>>>> sub-directories, maybe something like:
>>>>>>
>>>>>> (.*/JTreport/.*|^JTreport/.*)
>>>>>>
>>>>>> (.*/JTwork/.*|^JTwork/.*)
>>>>>>
>>>>>> but IMO, that appears a bit too complex when compared to the glob
>>>>>> syntax
>>>>>> that's used in the webrev.
>>>>>>
>>>>>> - The other change in this webrev is to ignore the
>>>>>> src/utils/hsdis/build/ directory which gets generated when
>>>>>> src/utils/hsdis is built. I included this in this patch based on
>>>>>> Gustavo's suggestion in that linked thread. I verified that this is
>>>>>> indeed the "build" location used by hsdis tool, by checking the
>>>>>> src/utils/hsdis/Makefile as well src/utils/hsdis/README.
>>>>>>
>>>>>> Gustavo also had a suggestion that we ignore the "oprofile_data"
>>>>>> directory/files. But I don't have knowledge of what those files are
>>>>>> and
>>>>>> didn't have a way to ascertain the right location/rule to add to the
>>>>>> .hgignore. So I have not included that in the current version of this
>>>>>> webrev. I'm however open to including this in the webrev if Gustavo
>>>>>> and
>>>>>> others feel that's OK/needed.
>>>>>>
>>>>>> -Jaikiran
>>>>>>
>>>>>>
>>>>>>


Re: RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

2019-07-29 Thread Jaikiran Pai
I got some more inputs from Stuart Marks on the linked JBS issue[1] and
I've incorporated those in the latest updated webrev which is at [2].

I've tested this latest change locally on mercurial version 5.0.1 and it
has worked as expected (now ignores JTreport and JTwork from the root as
well as sub directories). Plus, with this change I haven't seen any
files/directories that are now being unexpectedly not ignored.

Anyone willing to review and sponsor this please?

[1] https://bugs.openjdk.java.net/browse/JDK-8227170

[2] http://cr.openjdk.java.net/~jpai/webrev/8227170/3/webrev/

-Jaikiran

On 12/07/19 4:04 PM, Jaikiran Pai wrote:
> Hello Dean,
>
> Thank you for testing this. I have now updated the webrev to use the
> JTreport/* syntax and have also verified that it works for me too. It
> correctly ignores both root level JTreport and JTwork directories and
> any such sub-directories. I am on a macOS with:
>
> hg --version
> Mercurial Distributed SCM (version 5.0.1)
> (see https://mercurial-scm.org for more information)
>
> The updated webrev is at
> http://cr.openjdk.java.net/~jpai/webrev/8227170/2/webrev/
>
> -Jaikiran
>
> On 11/07/19 1:16 AM, dean.l...@oracle.com wrote:
>> The ** syntax isn't working for me in Mercurial 2.9, but the following:
>>
>> syntax: glob
>> JTreport/*
>> JTwork/*
>>
>> seems to work in both 2.9 and 4.6.1.  This also seems to work:
>>
>> syntax: glob
>> JTreport
>> JTwork
>>
>> but it matches files and not just directories.
>>
>> dl
>>
>> On 7/10/19 4:48 AM, Jaikiran Pai wrote:
>>> Ping. Anyone willing to review and sponsor this change?
>>>
>>> -Jaikiran
>>>
>>> On 03/07/19 5:43 PM, Jaikiran Pai wrote:
>>>> Hello,
>>>>
>>>> Can I please get a review and a sponsor for the patch for the
>>>> enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8227170?
>>>>
>>>> The webrev containing the patch is here
>>>> http://cr.openjdk.java.net/~jpai/webrev/8227170/00/webrev/
>>>>
>>>> For some context about this change - there's been some discussion on
>>>> this in the jdk-dev list here
>>>> https://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003076.html.
>>>>
>>>> The patch in the webrev, does the following things:
>>>>
>>>> - In addition to ignoring the JTwork and JTreport sub-directories, this
>>>> now also ignores these directories at the root of the repo. This patch
>>>> uses "syntax: glob" to easily ignore such directories, instead of the
>>>> (default) "regex" syntax. In the jdk-dev mailing list discussion,
>>>> Gustavo suggested to continue using the regex syntax and listing the
>>>> following in the .hgignore:
>>>>
>>>> .*JTreport/.*
>>>>
>>>> .*JTwork/.*
>>>>
>>>> But that would then end up ignoring directories of the form
>>>> "foobarJTreport" and files under it like "foobarJTreport/blah.txt". I
>>>> guess we can still stick with a regex syntax and come up with a regex
>>>> which ignores these directories both at the root as well as
>>>> sub-directories, maybe something like:
>>>>
>>>> (.*/JTreport/.*|^JTreport/.*)
>>>>
>>>> (.*/JTwork/.*|^JTwork/.*)
>>>>
>>>> but IMO, that appears a bit too complex when compared to the glob
>>>> syntax
>>>> that's used in the webrev.
>>>>
>>>> - The other change in this webrev is to ignore the
>>>> src/utils/hsdis/build/ directory which gets generated when
>>>> src/utils/hsdis is built. I included this in this patch based on
>>>> Gustavo's suggestion in that linked thread. I verified that this is
>>>> indeed the "build" location used by hsdis tool, by checking the
>>>> src/utils/hsdis/Makefile as well src/utils/hsdis/README.
>>>>
>>>> Gustavo also had a suggestion that we ignore the "oprofile_data"
>>>> directory/files. But I don't have knowledge of what those files are and
>>>> didn't have a way to ascertain the right location/rule to add to the
>>>> .hgignore. So I have not included that in the current version of this
>>>> webrev. I'm however open to including this in the webrev if Gustavo and
>>>> others feel that's OK/needed.
>>>>
>>>> -Jaikiran
>>>>
>>>>


Re: Build failing on macOS in hotspot module

2019-07-19 Thread Jaikiran Pai
Thank you Alan.

Sorry, my last update was early this morning and I should have fetched
latest before sending out this mail. Fetched the latest now.

-Jaikiran

On 19/07/19 5:45 PM, Alan Bateman wrote:
> This seems to have been fixed a few hours ago:
>    http://hg.openjdk.java.net/jdk/jdk/rev/4d421888ad63
>
> On 19/07/2019 13:12, Jaikiran Pai wrote:
>> I just updated my local jdk workspace to the latest default branch
>> and did:
>>
>> bash configure
>>
>> make clean
>>
>> make images
>>
>> The build keeps failing with:
>>
>> ERROR: Build failed for target 'images' in configuration
>> 'macosx-x86_64-server-release' (exit code 2)
>> Stopping sjavac server
>>
>> === Output from failing command(s) repeated here ===
>> * For target hotspot_variant-server_libjvm_objs_shenandoahSupport.o:
>> openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
>>
>> error: operator '?:' has lower precedence than '+'; '+' will be
>> evaluated first [-Werror,-Wparentheses]
>>    return Node::hash() + _native ? 1 : 0;
>>   ~~ ^
>> openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
>>
>> note: place parentheses around the '+' expression to silence this
>> warning
>>    return Node::hash() + _native ? 1 : 0;
>>  ^
>>   ( )
>> openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
>>
>> note: place parentheses around the '?:' expression to evaluate it first
>>    return Node::hash() + _native ? 1 : 0;
>>  ^
>>  (  )
>> 1 error generated.
>>
>> * All command lines available in
>> openjdk/jdk/build/macosx-x86_64-server-release/make-support/failure-logs.
>>
>> === End of repeated output ===
>>
>>
>> I'm on macOS 10.14.1. The last working state was a few days back, I
>> think last week, when my local builds were working fine. Is this an
>> expected error, requiring some tool upgrade? Or does this need a fix in
>> the cpp code?
>>
>> -Jaikiran
>


Build failing on macOS in hotspot module

2019-07-19 Thread Jaikiran Pai
I just updated my local jdk workspace to the latest default branch and did:

bash configure

make clean

make images

The build keeps failing with:

ERROR: Build failed for target 'images' in configuration
'macosx-x86_64-server-release' (exit code 2)
Stopping sjavac server

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_shenandoahSupport.o:
openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
error: operator '?:' has lower precedence than '+'; '+' will be
evaluated first [-Werror,-Wparentheses]
  return Node::hash() + _native ? 1 : 0;
 ~~ ^
openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
note: place parentheses around the '+' expression to silence this warning
  return Node::hash() + _native ? 1 : 0;
    ^
 ( )
openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
note: place parentheses around the '?:' expression to evaluate it first
  return Node::hash() + _native ? 1 : 0;
    ^
    (  )
1 error generated.

* All command lines available in
openjdk/jdk/build/macosx-x86_64-server-release/make-support/failure-logs.
=== End of repeated output ===


I'm on macOS 10.14.1. The last working state was a few days back, I
think last week, when my local builds were working fine. Is this an
expected error, requiring some tool upgrade? Or does this need a fix in
the cpp code?

-Jaikiran


Build failing on macOS in hotspot module

2019-07-19 Thread Jaikiran Pai
I just updated my local jdk workspace to the latest default branch and did:

bash configure

make clean

make images

The build keeps failing with:

ERROR: Build failed for target 'images' in configuration
'macosx-x86_64-server-release' (exit code 2)
Stopping sjavac server

=== Output from failing command(s) repeated here ===
* For target hotspot_variant-server_libjvm_objs_shenandoahSupport.o:
openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
error: operator '?:' has lower precedence than '+'; '+' will be
evaluated first [-Werror,-Wparentheses]
  return Node::hash() + _native ? 1 : 0;
 ~~ ^
openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
note: place parentheses around the '+' expression to silence this warning
  return Node::hash() + _native ? 1 : 0;
    ^
 ( )
openjdk/jdk/src/hotspot/share/gc/shenandoah/c2/shenandoahSupport.cpp:3019:33:
note: place parentheses around the '?:' expression to evaluate it first
  return Node::hash() + _native ? 1 : 0;
    ^
    (  )
1 error generated.

* All command lines available in
openjdk/jdk/build/macosx-x86_64-server-release/make-support/failure-logs.
=== End of repeated output ===


I'm on macOS 10.14.1. The last working state was a few days back, I
think last week, when my local builds were working fine. Is this an
expected error, requiring some tool upgrade? Or does this need a fix in
the cpp code?

-Jaikiran


Re: RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

2019-07-12 Thread Jaikiran Pai
Hello Dean,

Thank you for testing this. I have now updated the webrev to use the
JTreport/* syntax and have also verified that it works for me too. It
correctly ignores both root level JTreport and JTwork directories and
any such sub-directories. I am on a macOS with:

hg --version
Mercurial Distributed SCM (version 5.0.1)
(see https://mercurial-scm.org for more information)

The updated webrev is at
http://cr.openjdk.java.net/~jpai/webrev/8227170/2/webrev/

-Jaikiran

On 11/07/19 1:16 AM, dean.l...@oracle.com wrote:
> The ** syntax isn't working for me in Mercurial 2.9, but the following:
>
> syntax: glob
> JTreport/*
> JTwork/*
>
> seems to work in both 2.9 and 4.6.1.  This also seems to work:
>
> syntax: glob
> JTreport
> JTwork
>
> but it matches files and not just directories.
>
> dl
>
> On 7/10/19 4:48 AM, Jaikiran Pai wrote:
>> Ping. Anyone willing to review and sponsor this change?
>>
>> -Jaikiran
>>
>> On 03/07/19 5:43 PM, Jaikiran Pai wrote:
>>> Hello,
>>>
>>> Can I please get a review and a sponsor for the patch for the
>>> enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8227170?
>>>
>>> The webrev containing the patch is here
>>> http://cr.openjdk.java.net/~jpai/webrev/8227170/00/webrev/
>>>
>>> For some context about this change - there's been some discussion on
>>> this in the jdk-dev list here
>>> https://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003076.html.
>>>
>>> The patch in the webrev, does the following things:
>>>
>>> - In addition to ignoring the JTwork and JTreport sub-directories, this
>>> now also ignores these directories at the root of the repo. This patch
>>> uses "syntax: glob" to easily ignore such directories, instead of the
>>> (default) "regex" syntax. In the jdk-dev mailing list discussion,
>>> Gustavo suggested to continue using the regex syntax and listing the
>>> following in the .hgignore:
>>>
>>> .*JTreport/.*
>>>
>>> .*JTwork/.*
>>>
>>> But that would then end up ignoring directories of the form
>>> "foobarJTreport" and files under it like "foobarJTreport/blah.txt". I
>>> guess we can still stick with a regex syntax and come up with a regex
>>> which ignores these directories both at the root as well as
>>> sub-directories, maybe something like:
>>>
>>> (.*/JTreport/.*|^JTreport/.*)
>>>
>>> (.*/JTwork/.*|^JTwork/.*)
>>>
>>> but IMO, that appears a bit too complex when compared to the glob
>>> syntax
>>> that's used in the webrev.
>>>
>>> - The other change in this webrev is to ignore the
>>> src/utils/hsdis/build/ directory which gets generated when
>>> src/utils/hsdis is built. I included this in this patch based on
>>> Gustavo's suggestion in that linked thread. I verified that this is
>>> indeed the "build" location used by hsdis tool, by checking the
>>> src/utils/hsdis/Makefile as well src/utils/hsdis/README.
>>>
>>> Gustavo also had a suggestion that we ignore the "oprofile_data"
>>> directory/files. But I don't have knowledge of what those files are and
>>> didn't have a way to ascertain the right location/rule to add to the
>>> .hgignore. So I have not included that in the current version of this
>>> webrev. I'm however open to including this in the webrev if Gustavo and
>>> others feel that's OK/needed.
>>>
>>> -Jaikiran
>>>
>>>
>


Re: RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

2019-07-10 Thread Jaikiran Pai
Ping. Anyone willing to review and sponsor this change?

-Jaikiran

On 03/07/19 5:43 PM, Jaikiran Pai wrote:
> Hello,
>
> Can I please get a review and a sponsor for the patch for the
> enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8227170?
>
> The webrev containing the patch is here
> http://cr.openjdk.java.net/~jpai/webrev/8227170/00/webrev/
>
> For some context about this change - there's been some discussion on
> this in the jdk-dev list here
> https://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003076.html.
>
> The patch in the webrev, does the following things:
>
> - In addition to ignoring the JTwork and JTreport sub-directories, this
> now also ignores these directories at the root of the repo. This patch
> uses "syntax: glob" to easily ignore such directories, instead of the
> (default) "regex" syntax. In the jdk-dev mailing list discussion,
> Gustavo suggested to continue using the regex syntax and listing the
> following in the .hgignore:
>
> .*JTreport/.*
>
> .*JTwork/.*
>
> But that would then end up ignoring directories of the form
> "foobarJTreport" and files under it like "foobarJTreport/blah.txt". I
> guess we can still stick with a regex syntax and come up with a regex
> which ignores these directories both at the root as well as
> sub-directories, maybe something like:
>
> (.*/JTreport/.*|^JTreport/.*)
>
> (.*/JTwork/.*|^JTwork/.*)
>
> but IMO, that appears a bit too complex when compared to the glob syntax
> that's used in the webrev.
>
> - The other change in this webrev is to ignore the
> src/utils/hsdis/build/ directory which gets generated when
> src/utils/hsdis is built. I included this in this patch based on
> Gustavo's suggestion in that linked thread. I verified that this is
> indeed the "build" location used by hsdis tool, by checking the
> src/utils/hsdis/Makefile as well src/utils/hsdis/README.
>
> Gustavo also had a suggestion that we ignore the "oprofile_data"
> directory/files. But I don't have knowledge of what those files are and
> didn't have a way to ascertain the right location/rule to add to the
> .hgignore. So I have not included that in the current version of this
> webrev. I'm however open to including this in the webrev if Gustavo and
> others feel that's OK/needed.
>
> -Jaikiran
>
>


RFR(xxs): 8227170: (.hg)Ignore the JTwork and JTreport directories generated at the root of the repo

2019-07-03 Thread Jaikiran Pai
Hello,

Can I please get a review and a sponsor for the patch for the
enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8227170?

The webrev containing the patch is here
http://cr.openjdk.java.net/~jpai/webrev/8227170/00/webrev/

For some context about this change - there's been some discussion on
this in the jdk-dev list here
https://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003076.html.

The patch in the webrev, does the following things:

- In addition to ignoring the JTwork and JTreport sub-directories, this
now also ignores these directories at the root of the repo. This patch
uses "syntax: glob" to easily ignore such directories, instead of the
(default) "regex" syntax. In the jdk-dev mailing list discussion,
Gustavo suggested to continue using the regex syntax and listing the
following in the .hgignore:

.*JTreport/.*

.*JTwork/.*

But that would then end up ignoring directories of the form
"foobarJTreport" and files under it like "foobarJTreport/blah.txt". I
guess we can still stick with a regex syntax and come up with a regex
which ignores these directories both at the root as well as
sub-directories, maybe something like:

(.*/JTreport/.*|^JTreport/.*)

(.*/JTwork/.*|^JTwork/.*)

but IMO, that appears a bit too complex when compared to the glob syntax
that's used in the webrev.

- The other change in this webrev is to ignore the
src/utils/hsdis/build/ directory which gets generated when
src/utils/hsdis is built. I included this in this patch based on
Gustavo's suggestion in that linked thread. I verified that this is
indeed the "build" location used by hsdis tool, by checking the
src/utils/hsdis/Makefile as well src/utils/hsdis/README.

Gustavo also had a suggestion that we ignore the "oprofile_data"
directory/files. But I don't have knowledge of what those files are and
didn't have a way to ascertain the right location/rule to add to the
.hgignore. So I have not included that in the current version of this
webrev. I'm however open to including this in the webrev if Gustavo and
others feel that's OK/needed.

-Jaikiran