RFR: 8239026: Support non-maven artifacts by JibArtifactManager

[Leonid Mesnik]

Hi

Could you please review following fix which extend JibArtifactManager to 
support download and installation of artifacts with layout different from maven 
style. 
It is planned that user provide name and properties to resolve layout using 
default or jib manager. There are no tests which use this customization  
currently added in the testbase. So there is no good interface like Artifact 
annotation is added.

webrev: http://cr.openjdk.java.net/~lmesnik/8239026/webrev.00/ 

bug: https://bugs.openjdk.java.net/browse/JDK-8239026 


Leonid

Re: RFR: 8239001: Hotspot build broken on linux-sparc after 8238281

[Kim Barrett]

> On Feb 13, 2020, at 5:21 PM, Magnus Ihse Bursie 
>  wrote:
> 
> I searched the code base for mentions of old gcc workarounds, and tried to 
> remedy them now that they were not needed. 
> 
> If it turns out that they still were needed, by all means, revert that part 
> of the patch. 

I think the offset_of definition in question was not an "old gcc
workaround".

offsetof with a non-POD type is undefined behavior in C++98.  C++11
relaxed the restriction to allow standard-layout types.  C++17 changed
offsetof with a non-standard-layout type to be "conditionally
supported", which is more or less effectively UB for portable code.

Contrary to what was said in the RFR for JDK-8238281, we should not be
considering eliminating offset_of and just using offsetof directly.
offset_of is the HotSpot portability shim to let us knowingly do
something that is potentially problematic but we claim we know what
we're doing.  The fact that gcc warns about problematic offsetof uses
(unless -Wno-invalid-offsetof) is a good thing, because it helps keep
us honest about using that portability shim.  (This is similar in
concept to JDK-8214976.)  (There are a couple of direct uses of
offsetof in os_perf_solaris.cpp.)

Then there's the separate issue that the failing code in the
RegistersForDebugging class is using offset_of / offsetof in an
invalid way, since the result is supposed to be a constant expression
and that's impossible because these uses involve the non-constant "j"
argument.

I think that can be fixed by something like

/* Add this in RegistersForDebugging */
private:
  static const RegistersForDebugging& _dummy; // not ODR-used so not defined

/* Change offset functions based on this model. */
static int i_offset(int j) {
  return offset_of(RegistersForDebugging, i) + j * (sizeof _dummy.i[0]);
}

Of course, reverting to the gcc implementation of offset_of also
"fixes" this problem, because that version of offset_of happens to
"accidentally" be more lenient.

RFR: JDK-8239019: testmake fails with FATAL: VCS_TYPE is empty

[Erik Joelsson]

Hello,

The fix to make idea.sh support both git and mercurial had the 
unfortunate side effect of bailing out completely if neither was found. 
This happens to be the case when we run our build system tests (which 
includes running idea.sh) in our distributed test environment. I think 
idea.sh should still work even if run on a source tree outside of any 
source control system.


This patch just changes the lack of a detected VCS_TYPE to a warning 
instead of being fatal. The vcs.xml is still generated, but from what I 
can tell, Intellij doesn't seem to care much.


Webrev: http://cr.openjdk.java.net/~erikj/8239019/webrev.01

Bug: https://bugs.openjdk.java.net/browse/JDK-8239019

/Erik

Re: RFR: 8239001: Hotspot build broken on linux-sparc after 8238281

[Magnus Ihse Bursie]

I searched the code base for mentions of old gcc workarounds, and tried to 
remedy them now that they were not needed. 

If it turns out that they still were needed, by all means, revert that part of 
the patch. 

/Magnus

13 feb. 2020 kl. 20:35 skrev Kim Barrett :

>> On Feb 13, 2020, at 7:26 AM, John Paul Adrian Glaubitz 
>>  wrote:
>> 
>> Hello!
>> 
>> Hotspot fails to build on linux-sparc after 8238281 due to the redefinition
>> of offset_of() in src/hotspot/share/prims/jvm.cpp [1]:
>> 
>> /home/glaubitz/jdk/src/hotspot/cpu/sparc/macroAssembler_sparc.hpp: In static 
>> member function 'static int RegistersForDebugging::i_offset(int)':
>> /home/glaubitz/jdk/src/hotspot/cpu/sparc/macroAssembler_sparc.hpp:488:74: 
>> error: 'j' cannot appear in a constant-expression
>> 488 | static int i_offset(int j) { return offset_of(RegistersForDebugging, 
>> i[j]); }
>> | ^ 
>> 
>> Since offsetof() can only be used with constant expressions, I have reused
>> the old definition of offset_of() in 
>> src/hotspot/cpu/sparc/macroAssembler_sparc.hpp
>> to fix the issue.
>> 
>> I couldn't come up with a more elegant solution, but I'm open for 
>> suggestions.
>> 
>> Please review my change in [2].
>> 
>> Thanks,
>> Adrian
>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8239001
>>> [2] http://cr.openjdk.java.net/~glaubitz/8239001/webrev.00/
> 
> I don't think this is the right way to address this problem.
> 
> I think the JDK-8238281 change to offset_of and the associated
> addition of -Wno-invalid-offsetof to the build configuration was a
> mistake, and should be reverted.  Those changes seem unrelated to the
> purpose of JDK-8238281, which was to raise the minimum acceptable gcc
> version.
> 

Re: RFR [jdk11]: 8234525: enable link-time section-gc for linux s390x to remove unused code

[Andrew John Hughes]

On 13/02/2020 11:48, Baesken, Matthias wrote:
> Ping - any reviews ?
> 
> Thanks, Matthias
> 
> From: Baesken, Matthias
> Sent: Dienstag, 11. Februar 2020 10:24
> To: jdk-updates-...@openjdk.java.net; 'build-dev@openjdk.java.net' 
> 
> Subject: RFR [jdk11]: 8234525: enable link-time section-gc for linux s390x to 
> remove unused code
> 
> Hello , please review  the downport of   "8234525: enable link-time 
> section-gc for linux s390x to remove unused code"   to jdk11 .
> 
> My change from jdk/jdk  did not apply directly and I had to adjust it 
> slightly .
> 
> 
> 
> 
> Bug  and  jdk/jdk change :
> 
> https://bugs.openjdk.java.net/browse/JDK-8234525
> https://hg.openjdk.java.net/jdk/jdk/rev/c04fa10636fd
> 
> 
> Adjusted change for jdk11u-dev :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8234525.0_jdk11/
> 
> 
> Original review thread :
> 
> https://mail.openjdk.java.net/pipermail/build-dev/2019-November/026326.html
> 
> 
> Thanks, Matthias
> 

In the original patch, the lines are added after
TOOLCHAIN_CFLAGS_JDK_CONLY. In the 11u version, they seem to have
needlessly moved above it.

Otherwise, looks good.

Regarding the patch itself, are these flags only useful when the s390x
port is present or would they be advantageous on older versions where
s390x is still using the Zero assembler port?

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew

RE: RFR: JDK-8238534

[Langer, Christoph]

Great. I'll push it tomorrow.

/Christoph

> -Original Message-
> From: Erik Joelsson 
> Sent: Donnerstag, 13. Februar 2020 18:18
> To: René Schünemann ; Langer, Christoph
> 
> Cc: build-dev@openjdk.java.net
> Subject: Re: RFR: JDK-8238534
> 
> Looks good.
> 
> /Erik
> 
> On 2020-02-13 01:17, René Schünemann wrote:
> > Hi Erik and Christoph,
> >
> > thank you for your reviews.
> > The added touch works for me too. I have adapted the suggested
> whitespace fixes.
> >
> > Here is the updated WebRev:
> > http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-
> macos_sign_bundles/04/
> >
> > Rene
> >
> > On Wed, Feb 12, 2020 at 8:09 PM Erik Joelsson 
> wrote:
> >> Hello Rene,
> >>
> >> On 2020-02-12 02:54, René Schünemann wrote:
> >>> Hello Erik,
> >>>
> >>> thank you for your guidance.
> >>> I have implemented your requested changes.
> >>>
> >>> Updated WebRev:
> >>> http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-
> macos_sign_bundles/03/
> >> Much better. I still have some style issues [1]. The recipe lines for
> >> the signing rule all have extra spaces, which is not something we do.
> >> Also there is no continuation indent for the long command line.
> >>
> >> I would also recommend against redirecting stderr as that will hide any
> >> error message from codesign.
> >>
> >>> There is one thin I notice though. On my machine the bundles are
> >>> getting resigned and rebuilt every time I call make, even if they
> >>> already exist.
> >>> Do you notice the same behavior?
> >> Yes, it seems the CodeResources file does not get timestamped with a
> >> newer date than the source files. Adding a touch fixes it for me. (We
> >> also use that in our closed implementation)
> >>
> >> Here is a webrev with my suggested whitespace fixes and the touch.
> >>
> >> http://cr.openjdk.java.net/~erikj/8238534/webrev.02/
> >>
> >> [1] http://openjdk.java.net/groups/build/doc/code-conventions.html
> >>
> >> /Erik
> >>
> >>> Rene
> >>>
> >>> On Tue, Feb 11, 2020 at 6:57 PM Erik Joelsson
>  wrote:
>  On 2020-02-11 01:08, René Schünemann wrote:
> > Hello Erik,
> >
> > thank you for your review. Please see my answers below.
> >
> > Rene
> >
> > On Mon, Feb 10, 2020 at 9:34 PM Erik Joelsson
>  wrote:
> >> Hello René,
> >>
> >> That looks better. I still have some issues though.
> >>
> >> I don't understand line 273 and 305. There is no reason to declare
> those
> >> rules.
> >>
> >> Line 311, the CodeResources file needs prerequisites. Those should
> be
> >> $(CREATE_JDK_BUNDLE_DIR_SIGNED) (which is the list of all files
> copied
> >> into the signed image).
> >>
> > This doesn't work for me. Without declaring this rule I get:
> >
> > Building target 'product-bundles' in configuration
> > 'macosx-x86_64-server-release'
> > gmake[3]: *** No rule to make target
> > '[...]/hg/jdk/build/macosx-x86_64-server-release/images/jdk-
> bundle-signed/jdk-15.jdk',
> > needed by [...]/hg/jdk/build/macosx-x86_64-server-
> release/bundles/jdk-15-internal+0_osx-x64_bin.tar.gz'.
> > Stop.
> > gmake[2]: *** [make/Main.gmk:627: product-bundles] Error 2
> >
> > ERROR: Build failed for target 'product-bundles' in configuration
> > 'macosx-x86_64-server-release' (exit code 2)
>  That's because you declared the directory as part of FILES in the call
>  to SetupBundleFile.
> >> Instead of piping to /dev/null, I recommend using the macro
> >> $(LOG_DEBUG). It will resolve to >/dev/null for log levels less
> detailed
> >> than debug. Does codesign output to stderr on successful signing? If
> >> not, leave the stderr alone. If something goes wrong we want to see
> it
> >> in the build log.
> >>
> > I will do that.
> >
> >> The FILES list for BUILD_JDK_BUNDLE should be
> >> $(CREATE_JDK_BUNDLE_DIR_SIGNED) and
> >>
> $(JDK_MACOSX_BUNDLE_DIR_SIGNED)/$(JDK_MACOSX_BUNDLE_TOP_DIR
> ). Those are
> >> the exact files that should be included and by specifying them we
> get
> >> correct dependency declarations.
> >>
> > I tried this, it didn't work for me. I tried:
> >
> > $(JDK_SIGNED_CODE_RESOURCES):
> $(CREATE_JDK_BUNDLE_DIR_SIGNED)
> >
> > and/or
> >
> > $(BUILD_JDK_BUNDLE): $(CREATE_JDK_BUNDLE_DIR_SIGNED)
> >
> > and/or
> >
> > $(CREATE_JDK_BUNDLE_DIR_SIGNED) in the file list of
> BUILD_JDK_BUNDLE
> >
> > Without the line 273 and 305 rules make bails out with the said error.
>  I did my suggested changes for the JDK bundle and it built fine. You
>  still need to fix the JRE part.
> 
>  http://cr.openjdk.java.net/~erikj/8238534/webrev.01/
> 
> >> When signing the bundle, you should not need to specify
> entitlements.
> >> Those should only be supplied when signing executables that
> actually
> >> need them. Not sure if --force is a good idea here either.
> >>
> > The --fo

Re: RFR: 8239001: Hotspot build broken on linux-sparc after 8238281

[Kim Barrett]

> On Feb 13, 2020, at 7:26 AM, John Paul Adrian Glaubitz 
>  wrote:
> 
> Hello!
> 
> Hotspot fails to build on linux-sparc after 8238281 due to the redefinition
> of offset_of() in src/hotspot/share/prims/jvm.cpp [1]:
> 
> /home/glaubitz/jdk/src/hotspot/cpu/sparc/macroAssembler_sparc.hpp: In static 
> member function 'static int RegistersForDebugging::i_offset(int)':
> /home/glaubitz/jdk/src/hotspot/cpu/sparc/macroAssembler_sparc.hpp:488:74: 
> error: 'j' cannot appear in a constant-expression
>  488 | static int i_offset(int j) { return offset_of(RegistersForDebugging, 
> i[j]); }
>  | ^ 
> 
> Since offsetof() can only be used with constant expressions, I have reused
> the old definition of offset_of() in 
> src/hotspot/cpu/sparc/macroAssembler_sparc.hpp
> to fix the issue.
> 
> I couldn't come up with a more elegant solution, but I'm open for suggestions.
> 
> Please review my change in [2].
> 
> Thanks,
> Adrian
> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8239001
>> [2] http://cr.openjdk.java.net/~glaubitz/8239001/webrev.00/

I don't think this is the right way to address this problem.

I think the JDK-8238281 change to offset_of and the associated
addition of -Wno-invalid-offsetof to the build configuration was a
mistake, and should be reverted.  Those changes seem unrelated to the
purpose of JDK-8238281, which was to raise the minimum acceptable gcc
version.

Re: RFR(S) : 8238943: switch to jtreg 5.0

[Erik Joelsson]

Looks good.

/Erik

On 2020-02-13 10:08, Igor Ignatev wrote:

Oh, I’m sorry I actually changed it to 5.0 when were (re)doing testing, and 
apparently forgot to replace the webrev, the right is 
http://cr.openjdk.java.net/~iignatyev//8238943/webrev.01 ; with version field 
value being the only difference b/w .00 and .01

Thanks,
— Igor


On Feb 13, 2020, at 9:23 AM, Erik Joelsson  wrote:

Looks good, but could you change the "version" field to "5.0", it should work 
now.

/Erik


On 2020-02-13 08:50, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8238943/webrev.00

10 lines changed: 1 ins; 0 del; 9 mod;

Hi all,
could you please review the patch which changes jtreg version used in jdk/jdk 
to the latest and greatest -- jtreg 5.0? and as (recently became) usually, this 
patch also bumps requiredVersion in all the jtreg test suites in order to 
reduce possible discrepancy in results.
JBS: https://bugs.openjdk.java.net/browse/JDK-8238943
webrev: http://cr.openjdk.java.net/~iignatyev/8238943/webrev.00
testing: tier1-4
Thanks,
-- Igor

Re: RFR(S) : 8238943: switch to jtreg 5.0

[Joe Wang]

+1 for the change to test/jaxp/TEST.ROOT.

Best,
Joe


On 2/13/20 10:08 AM, Igor Ignatev wrote:

Oh, I’m sorry I actually changed it to 5.0 when were (re)doing testing, and 
apparently forgot to replace the webrev, the right is 
http://cr.openjdk.java.net/~iignatyev//8238943/webrev.01 ; with version field 
value being the only difference b/w .00 and .01

Thanks,
— Igor


On Feb 13, 2020, at 9:23 AM, Erik Joelsson  wrote:

Looks good, but could you change the "version" field to "5.0", it should work 
now.

/Erik


On 2020-02-13 08:50, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8238943/webrev.00

10 lines changed: 1 ins; 0 del; 9 mod;

Hi all,
could you please review the patch which changes jtreg version used in jdk/jdk 
to the latest and greatest -- jtreg 5.0? and as (recently became) usually, this 
patch also bumps requiredVersion in all the jtreg test suites in order to 
reduce possible discrepancy in results.
JBS: https://bugs.openjdk.java.net/browse/JDK-8238943
webrev: http://cr.openjdk.java.net/~iignatyev/8238943/webrev.00
testing: tier1-4
Thanks,
-- Igor

Re: RFR(S) : 8238943: switch to jtreg 5.0

[Igor Ignatev]

Oh, I’m sorry I actually changed it to 5.0 when were (re)doing testing, and 
apparently forgot to replace the webrev, the right is 
http://cr.openjdk.java.net/~iignatyev//8238943/webrev.01 ; with version field 
value being the only difference b/w .00 and .01

Thanks,
— Igor

> On Feb 13, 2020, at 9:23 AM, Erik Joelsson  wrote:
> 
> Looks good, but could you change the "version" field to "5.0", it should 
> work now.
> 
> /Erik
> 
>>> On 2020-02-13 08:50, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev//8238943/webrev.00
>>> 10 lines changed: 1 ins; 0 del; 9 mod;
>> Hi all,
>> could you please review the patch which changes jtreg version used in 
>> jdk/jdk to the latest and greatest -- jtreg 5.0? and as (recently became) 
>> usually, this patch also bumps requiredVersion in all the jtreg test suites 
>> in order to reduce possible discrepancy in results.
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8238943
>> webrev: http://cr.openjdk.java.net/~iignatyev/8238943/webrev.00
>> testing: tier1-4
>> Thanks,
>> -- Igor

Re: RFR: 8237566: FindTests.gmk should only include existing TEST.ROOT files

[Erik Joelsson]

Hello Erik,

Looks good. Style wise I try to put closing braces for logical blocks on 
a new line, aligned with the opening brace. I think that helps making 
the already quite convoluted makefile logic a tad more readable.


/Erik

On 2020-02-13 04:44, Erik Helin wrote:

Hi all,

this small patch changes FindTest.gmk to only include TEST.ROOT files 
that actually are present. The motivation for this change is that both 
Git and Mercurial supports so called "sparse" checkouts [0][1] (still 
somewhat experimental for both, but actively worked on). While 
experimenting with these features I noticed that FindTests.gmk demands 
that test/{jtreg,jdk,langtools,nashorn,jaxp}/TEST.ROOT are present 
even if you are only trying to run e.g. `make hotspot`. This small 
patch ensures that we only include TEST.ROOT files that actually exist 
on disk.


Webrev:
https://cr.openjdk.java.net/~ehelin/8237566/00/

Issue:
https://bugs.openjdk.java.net/browse/JDK-8237566

Testing:
- Tier 1,2,3 on Linux, macOS, Windows (all x64)

Thanks,
Erik

[0]: https://www.mercurial-scm.org/repo/hg/file/tip/hgext/sparse.py
[1]: 
https://github.blog/2020-01-17-bring-your-monorepo-down-to-size-with-sparse-checkout/


PS. For the curious, I did manage to create a minimal working 
directory for hotspot using git version 2.25 (with this patch applied):


$ git clone https://github.com/openjdk/jdk --no-checkout
$ cd jdk
$ git sparse-checkout init --cone
$ git sparse-checkout set make src/java.base src/hotspot
$ curl 
https://cr.openjdk.java.net/~ehelin/8237566/00/JDK-8237566.patch | git 
apply

$ bash configure
$ make hotspot

Re: RFR(S) : 8238943: switch to jtreg 5.0

[Erik Joelsson]

Looks good, but could you change the "version" field to "5.0", it should 
work now.


/Erik

On 2020-02-13 08:50, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8238943/webrev.00

10 lines changed: 1 ins; 0 del; 9 mod;

Hi all,

could you please review the patch which changes jtreg version used in jdk/jdk 
to the latest and greatest -- jtreg 5.0? and as (recently became) usually, this 
patch also bumps requiredVersion in all the jtreg test suites in order to 
reduce possible discrepancy in results.

JBS: https://bugs.openjdk.java.net/browse/JDK-8238943
webrev: http://cr.openjdk.java.net/~iignatyev/8238943/webrev.00
testing: tier1-4

Thanks,
-- Igor
  

Re: RFR: JDK-8238534

[Erik Joelsson]

Looks good.

/Erik

On 2020-02-13 01:17, René Schünemann wrote:

Hi Erik and Christoph,

thank you for your reviews.
The added touch works for me too. I have adapted the suggested whitespace fixes.

Here is the updated WebRev:
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/04/

Rene

On Wed, Feb 12, 2020 at 8:09 PM Erik Joelsson  wrote:

Hello Rene,

On 2020-02-12 02:54, René Schünemann wrote:

Hello Erik,

thank you for your guidance.
I have implemented your requested changes.

Updated WebRev:
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/03/

Much better. I still have some style issues [1]. The recipe lines for
the signing rule all have extra spaces, which is not something we do.
Also there is no continuation indent for the long command line.

I would also recommend against redirecting stderr as that will hide any
error message from codesign.


There is one thin I notice though. On my machine the bundles are
getting resigned and rebuilt every time I call make, even if they
already exist.
Do you notice the same behavior?

Yes, it seems the CodeResources file does not get timestamped with a
newer date than the source files. Adding a touch fixes it for me. (We
also use that in our closed implementation)

Here is a webrev with my suggested whitespace fixes and the touch.

http://cr.openjdk.java.net/~erikj/8238534/webrev.02/

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

/Erik


Rene

On Tue, Feb 11, 2020 at 6:57 PM Erik Joelsson  wrote:

On 2020-02-11 01:08, René Schünemann wrote:

Hello Erik,

thank you for your review. Please see my answers below.

Rene

On Mon, Feb 10, 2020 at 9:34 PM Erik Joelsson  wrote:

Hello René,

That looks better. I still have some issues though.

I don't understand line 273 and 305. There is no reason to declare those
rules.

Line 311, the CodeResources file needs prerequisites. Those should be
$(CREATE_JDK_BUNDLE_DIR_SIGNED) (which is the list of all files copied
into the signed image).


This doesn't work for me. Without declaring this rule I get:

Building target 'product-bundles' in configuration
'macosx-x86_64-server-release'
gmake[3]: *** No rule to make target
'[...]/hg/jdk/build/macosx-x86_64-server-release/images/jdk-bundle-signed/jdk-15.jdk',
needed by 
[...]/hg/jdk/build/macosx-x86_64-server-release/bundles/jdk-15-internal+0_osx-x64_bin.tar.gz'.
Stop.
gmake[2]: *** [make/Main.gmk:627: product-bundles] Error 2

ERROR: Build failed for target 'product-bundles' in configuration
'macosx-x86_64-server-release' (exit code 2)

That's because you declared the directory as part of FILES in the call
to SetupBundleFile.

Instead of piping to /dev/null, I recommend using the macro
$(LOG_DEBUG). It will resolve to >/dev/null for log levels less detailed
than debug. Does codesign output to stderr on successful signing? If
not, leave the stderr alone. If something goes wrong we want to see it
in the build log.


I will do that.


The FILES list for BUILD_JDK_BUNDLE should be
$(CREATE_JDK_BUNDLE_DIR_SIGNED) and
$(JDK_MACOSX_BUNDLE_DIR_SIGNED)/$(JDK_MACOSX_BUNDLE_TOP_DIR). Those are
the exact files that should be included and by specifying them we get
correct dependency declarations.


I tried this, it didn't work for me. I tried:

$(JDK_SIGNED_CODE_RESOURCES): $(CREATE_JDK_BUNDLE_DIR_SIGNED)

and/or

$(BUILD_JDK_BUNDLE): $(CREATE_JDK_BUNDLE_DIR_SIGNED)

and/or

$(CREATE_JDK_BUNDLE_DIR_SIGNED) in the file list of BUILD_JDK_BUNDLE

Without the line 273 and 305 rules make bails out with the said error.

I did my suggested changes for the JDK bundle and it built fine. You
still need to fix the JRE part.

http://cr.openjdk.java.net/~erikj/8238534/webrev.01/


When signing the bundle, you should not need to specify entitlements.
Those should only be supplied when signing executables that actually
need them. Not sure if --force is a good idea here either.


The --force flag is needed, because libjli.dylib is getting re-signed
in the process. Without this flag codesign fails with:

[...]/hg/jdk/build/macosx-x86_64-server-release/images/jdk-bundle-signed/jdk-15.jdk:
is already signed

Ah right, ok never mind that comment then.

Can you confirm, that libjli.dylib doesn't need the entitlements?

Absolutely. Apple states clearly that entitlements are only for
executables, not for libraries. The executable loading the library is
responsible for acquiring the necessary entitlements. Setting them on a
library has no effect as I understand it.

/Erik


/Erik

On 2020-02-10 10:12, René Schünemann wrote:

Hi Erik,

I have implemented your requested changes. I think it is a lot cleaner
now and the bundling as well as the
signing parts are now only executed when necessary.

New WebRev: 
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/02/

Rene

On Mon, Feb 10, 2020 at 9:23 AM René Schünemann
 wrote:

Hi Erik,

thank you for your review and I totally agree with you. It would
definitely be better a

Re: RFR(S) : 8238943: switch to jtreg 5.0

[Jonathan Gibbons]

Igor,

The change to langtools/TEST.ROOT is OK.

That being said, there are some old entries there which could also be 
cleaned up (separately?)  These lines probably date from the development 
of JDK 9 and the evolution of Project Jigsaw.



   20 # Use new module options
   21 useNewOptions=true
   22
   23 # Use --patch-module instead of -Xmodule:
   24 useNewPatchModule=true

-- Jon


On 2/13/20 8:50 AM, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8238943/webrev.00

10 lines changed: 1 ins; 0 del; 9 mod;

Hi all,

could you please review the patch which changes jtreg version used in jdk/jdk 
to the latest and greatest -- jtreg 5.0? and as (recently became) usually, this 
patch also bumps requiredVersion in all the jtreg test suites in order to 
reduce possible discrepancy in results.

JBS: https://bugs.openjdk.java.net/browse/JDK-8238943
webrev: http://cr.openjdk.java.net/~iignatyev/8238943/webrev.00
testing: tier1-4

Thanks,
-- Igor
  

RFR(S) : 8238943: switch to jtreg 5.0

[Igor Ignatyev]

http://cr.openjdk.java.net/~iignatyev//8238943/webrev.00
> 10 lines changed: 1 ins; 0 del; 9 mod;

Hi all,

could you please review the patch which changes jtreg version used in jdk/jdk 
to the latest and greatest -- jtreg 5.0? and as (recently became) usually, this 
patch also bumps requiredVersion in all the jtreg test suites in order to 
reduce possible discrepancy in results.

JBS: https://bugs.openjdk.java.net/browse/JDK-8238943
webrev: http://cr.openjdk.java.net/~iignatyev/8238943/webrev.00
testing: tier1-4 

Thanks,
-- Igor
 

RFR: 8237566: FindTests.gmk should only include existing TEST.ROOT files

[Erik Helin]

Hi all,

this small patch changes FindTest.gmk to only include TEST.ROOT files 
that actually are present. The motivation for this change is that both 
Git and Mercurial supports so called "sparse" checkouts [0][1] (still 
somewhat experimental for both, but actively worked on). While 
experimenting with these features I noticed that FindTests.gmk demands 
that test/{jtreg,jdk,langtools,nashorn,jaxp}/TEST.ROOT are present even 
if you are only trying to run e.g. `make hotspot`. This small patch 
ensures that we only include TEST.ROOT files that actually exist on disk.


Webrev:
https://cr.openjdk.java.net/~ehelin/8237566/00/

Issue:
https://bugs.openjdk.java.net/browse/JDK-8237566

Testing:
- Tier 1,2,3 on Linux, macOS, Windows (all x64)

Thanks,
Erik

[0]: https://www.mercurial-scm.org/repo/hg/file/tip/hgext/sparse.py
[1]: 
https://github.blog/2020-01-17-bring-your-monorepo-down-to-size-with-sparse-checkout/


PS. For the curious, I did manage to create a minimal working directory 
for hotspot using git version 2.25 (with this patch applied):


$ git clone https://github.com/openjdk/jdk --no-checkout
$ cd jdk
$ git sparse-checkout init --cone
$ git sparse-checkout set make src/java.base src/hotspot
$ curl https://cr.openjdk.java.net/~ehelin/8237566/00/JDK-8237566.patch 
| git apply

$ bash configure
$ make hotspot

Re: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images

[Alan Bateman]

On 12/02/2020 22:16, Erik Joelsson wrote:

Hello Christoph,

This patch certainly looks better to me, though I agree it's a bit 
hackish to have to filter and rename the stripped.pdb files twice, 
once for jmods and again for bundles. I think I'm ok with it for now 
though. The future improvement I would like to make would be to create 
two sets of jdk images, one that contains debug symbols and demos, 
which we continue to use for testing, and another which only contains 
exactly what we bundle up, including the correctly named top dir. The 
latter would be created first and used as input for the former. I 
think lots of things would be cleaner then, especially Bundles.gmk.
There may be a role for a jlink plugin here. On Linux, jlink 
--strip-native-debug-symbols runs the objdump tool and sub-options can 
be used to configure the debuginfo files to keep or exclude. In time I'm 
sure there will be other plugins like this.


-Alan.

RE: RFR [jdk11]: 8234525: enable link-time section-gc for linux s390x to remove unused code

[Baesken, Matthias]

Ping - any reviews ?

Thanks, Matthias

From: Baesken, Matthias
Sent: Dienstag, 11. Februar 2020 10:24
To: jdk-updates-...@openjdk.java.net; 'build-dev@openjdk.java.net' 

Subject: RFR [jdk11]: 8234525: enable link-time section-gc for linux s390x to 
remove unused code

Hello , please review  the downport of   "8234525: enable link-time section-gc 
for linux s390x to remove unused code"   to jdk11 .

My change from jdk/jdk  did not apply directly and I had to adjust it slightly .




Bug  and  jdk/jdk change :

https://bugs.openjdk.java.net/browse/JDK-8234525
https://hg.openjdk.java.net/jdk/jdk/rev/c04fa10636fd


Adjusted change for jdk11u-dev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8234525.0_jdk11/


Original review thread :

https://mail.openjdk.java.net/pipermail/build-dev/2019-November/026326.html


Thanks, Matthias

Re: RFR: JDK-8238534

[René Schünemann]

Hi Erik and Christoph,

thank you for your reviews.
The added touch works for me too. I have adapted the suggested whitespace fixes.

Here is the updated WebRev:
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/04/

Rene

On Wed, Feb 12, 2020 at 8:09 PM Erik Joelsson  wrote:
>
> Hello Rene,
>
> On 2020-02-12 02:54, René Schünemann wrote:
> > Hello Erik,
> >
> > thank you for your guidance.
> > I have implemented your requested changes.
> >
> > Updated WebRev:
> > http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/03/
>
> Much better. I still have some style issues [1]. The recipe lines for
> the signing rule all have extra spaces, which is not something we do.
> Also there is no continuation indent for the long command line.
>
> I would also recommend against redirecting stderr as that will hide any
> error message from codesign.
>
> > There is one thin I notice though. On my machine the bundles are
> > getting resigned and rebuilt every time I call make, even if they
> > already exist.
> > Do you notice the same behavior?
>
> Yes, it seems the CodeResources file does not get timestamped with a
> newer date than the source files. Adding a touch fixes it for me. (We
> also use that in our closed implementation)
>
> Here is a webrev with my suggested whitespace fixes and the touch.
>
> http://cr.openjdk.java.net/~erikj/8238534/webrev.02/
>
> [1] http://openjdk.java.net/groups/build/doc/code-conventions.html
>
> /Erik
>
> > Rene
> >
> > On Tue, Feb 11, 2020 at 6:57 PM Erik Joelsson  
> > wrote:
> >> On 2020-02-11 01:08, René Schünemann wrote:
> >>> Hello Erik,
> >>>
> >>> thank you for your review. Please see my answers below.
> >>>
> >>> Rene
> >>>
> >>> On Mon, Feb 10, 2020 at 9:34 PM Erik Joelsson  
> >>> wrote:
>  Hello René,
> 
>  That looks better. I still have some issues though.
> 
>  I don't understand line 273 and 305. There is no reason to declare those
>  rules.
> 
>  Line 311, the CodeResources file needs prerequisites. Those should be
>  $(CREATE_JDK_BUNDLE_DIR_SIGNED) (which is the list of all files copied
>  into the signed image).
> 
> >>> This doesn't work for me. Without declaring this rule I get:
> >>>
> >>> Building target 'product-bundles' in configuration
> >>> 'macosx-x86_64-server-release'
> >>> gmake[3]: *** No rule to make target
> >>> '[...]/hg/jdk/build/macosx-x86_64-server-release/images/jdk-bundle-signed/jdk-15.jdk',
> >>> needed by 
> >>> [...]/hg/jdk/build/macosx-x86_64-server-release/bundles/jdk-15-internal+0_osx-x64_bin.tar.gz'.
> >>> Stop.
> >>> gmake[2]: *** [make/Main.gmk:627: product-bundles] Error 2
> >>>
> >>> ERROR: Build failed for target 'product-bundles' in configuration
> >>> 'macosx-x86_64-server-release' (exit code 2)
> >> That's because you declared the directory as part of FILES in the call
> >> to SetupBundleFile.
>  Instead of piping to /dev/null, I recommend using the macro
>  $(LOG_DEBUG). It will resolve to >/dev/null for log levels less detailed
>  than debug. Does codesign output to stderr on successful signing? If
>  not, leave the stderr alone. If something goes wrong we want to see it
>  in the build log.
> 
> >>> I will do that.
> >>>
>  The FILES list for BUILD_JDK_BUNDLE should be
>  $(CREATE_JDK_BUNDLE_DIR_SIGNED) and
>  $(JDK_MACOSX_BUNDLE_DIR_SIGNED)/$(JDK_MACOSX_BUNDLE_TOP_DIR). Those are
>  the exact files that should be included and by specifying them we get
>  correct dependency declarations.
> 
> >>> I tried this, it didn't work for me. I tried:
> >>>
> >>> $(JDK_SIGNED_CODE_RESOURCES): $(CREATE_JDK_BUNDLE_DIR_SIGNED)
> >>>
> >>> and/or
> >>>
> >>> $(BUILD_JDK_BUNDLE): $(CREATE_JDK_BUNDLE_DIR_SIGNED)
> >>>
> >>> and/or
> >>>
> >>> $(CREATE_JDK_BUNDLE_DIR_SIGNED) in the file list of BUILD_JDK_BUNDLE
> >>>
> >>> Without the line 273 and 305 rules make bails out with the said error.
> >> I did my suggested changes for the JDK bundle and it built fine. You
> >> still need to fix the JRE part.
> >>
> >> http://cr.openjdk.java.net/~erikj/8238534/webrev.01/
> >>
>  When signing the bundle, you should not need to specify entitlements.
>  Those should only be supplied when signing executables that actually
>  need them. Not sure if --force is a good idea here either.
> 
> >>> The --force flag is needed, because libjli.dylib is getting re-signed
> >>> in the process. Without this flag codesign fails with:
> >>>
> >>> [...]/hg/jdk/build/macosx-x86_64-server-release/images/jdk-bundle-signed/jdk-15.jdk:
> >>> is already signed
> >> Ah right, ok never mind that comment then.
> >>> Can you confirm, that libjli.dylib doesn't need the entitlements?
> >> Absolutely. Apple states clearly that entitlements are only for
> >> executables, not for libraries. The executable loading the library is
> >> responsible for acquiring the necessary entitlements. Setting them on a
> >> library has no effect as I un