Re: RFR(S) : 8245610 : remove in-tree copy on gtest

[René Schünemann]

Hi all,

I agree with Thomas. I think it would be beneficial to have a fallback
when there is no gtest directory explicitly set.

Also, running make target "run-test-gtest" now without "--with-gtest"
results in a not so descriptive error message.

make run-test-gtest
Building target 'run-test-gtest' in configuration 'linux-x86_64-server-release'
Unknown test selection: 'gtest'
See doc/testing.[md|html] for help
RunTests.gmk:539: *** Cannot continue.  Stop.
make[2]: *** [test-gtest] Error 1

I think it would be good to have the error message say that one tries
to run the gtest suite without having the gtest sources set.
The documentation under doc/testing.[md|html] doesn't describe the new
behavior, so hotspot developers that missed this change might be a bit
confused with this error message.

Thanks, Rene


On Fri, May 29, 2020 at 7:05 AM Thomas Stüfe  wrote:
>
> Hi Igor,
>
> thank you for taking the time to answer my grumblings.
>
> Yes, comparison with jtreg is a bit crooked - it is not needed to get a
> valid build. But jtreg is also maintained in the official OpenJDK
> repositories; I can clone codetools/jtreg and have the correct version.
> With gtests, I have to know which version OpenJDK needs, which is not the
> latest version, and have to download that from outside our repositories.
> Getting it wrong version may yield me difficult to analyze build errors
> (plattform zoo, handicapped C++ compilers etc). Also, updating to a new
> gtest version will now involve a lot more communicatio (a version check in
> configure would help with that).
>
> But this is a minor complaint, I could live with that. I most dislike the
> fact that I have to specify this option *every single time*, and that
> omitting it is objectively wrong and may give me a false sense of security.
> Omitting it gives no warning, but if my changes break gtests I will only
> notice it hours later when jdk/submit results are back.
>
> Yes, I can hide this behind an alias or script, but I think this is the
> wrong way. You guys did such a good job in making the build dead easy:
> first time, it tells me exactly which Debian packets to install, I always
> loved that special touch. I specify boot jdk and maybe debug level and I am
> done. In fact, the build is so easy that until recently I did not even know
> we had a build documentation :) The new --with-gtest option is a jarring
> break from that.
>
> In my mind, gtest is in the same ballpark as the freetype library on
> Windows. That has always been a bit annoying but that was only Windows.
> Something you cannot rely the OS library mechanism to deliver but have to
> download and place yourself and tell the build about it.
>
> I wonder whether we can find a compromise somehow. Maybe something like
> an agreed on structure and place for a "companion directory", containing
> build dependencies like these. Its location could be by convention in a
> clear relation to the OpenJDK sources (e.g. just beside it) and configure
> would look for the libraries in there by default. Like this:
>
> - openjdk-source
>- configure
>- Makefile
>- ..
> - openjdk-builddeps
>   - google-test
>   - freetype
>   - ..
>
> This would also lend itself very well to a maintained repository of build
> dependencies somewhere (not necessarily maintained by Oracle). We would
> have similar ease of use as with platform libraries, which by default are
> also expected in a standard place in the file system.
>
> Thanks, Thomas
>
>
> On Fri, May 29, 2020 at 5:20 AM Igor Ignatyev 
> wrote:
>
> > Hi Thomas,
> >
> > I regret that this patch made you unhappier. I fully agree that all
> > hotspot developers are to always build *and* run gtest tests, yet not all
> > people who work on OpenJDK project are hotspot developers.
> >
> > I also believe that all OpenJDK developers are to run tests related to the
> > area they are changing. The vast majority of such tests are jtreg tests.
> > And jtreg, for better or worse, is a counterexample to your last paragraph
> > -- it's an essential part of OpenJDK, but it doesn't come in a form of well
> > established library, and none of known to me platforms have jtreg in their
> > package manager, so you do have to manually download jtreg and specify its
> > location one way or another. I understand that there is a difference b/w
> > gtest and jtreg, as jtreg isn't really need at build time. However the
> > proper way to run any of OpenJDK tests is by `make test` and for it to work
> > you need to either executed configure w/ --with-jtreg or specify JT_HOME on
> > each invocation of `make test`, where the latter is a preferred way. From
> > that point of view, gtest and jtreg aren't different, you need to download
> > both manually and specify their location at configure-time.
> >
> > with that being said, I truly understand the inconvenience it causes to
> > you, yet given you need to download gtest only once and it's fairly easy to
> > hide `with-gtest` behind a script or an 

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

Re: RFR: JDK-8238534

[René Schünemann]

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/

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?

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,
> >>>>
> >>>> t

Re: RFR: JDK-8238534

[René Schünemann]

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)

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

> 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

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

> /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 avoid temp dirs.
> >> I will try to move the creation of the signed image to MacBundles.gmk
> >> and then re-use the SetubBundleFile in Bundles.gmk.
> >>
> >> Rene
> >>
> >> On Fri, Feb 7, 2020 at 5:19 PM Erik Joelsson  
> >> wrote:
> >>> Hello René,
> >>>
> >>> It's good to see an open solution to this, but I have some opinions on
> >>> the patch.
> >>>
> >>> The concept of building into "temp dirs" that are then removed is a
> >>> practice we try to avoid in the build. Whenever possible, each rule
> >>> should be a well defined transformation from a set of source files to a
> >>> target file. There is just no reason to remove the jdk-signed dir here.
> >>> If something goes wrong, you would want the dir around to investigate.
> >>> This also keeps incremental builds working as expected. Your current
> >>> patch will always rebuild the bundles, which is not ok.
> >>>
> >>> I would recommend putting the jdk-signed dir in
> >>> $(IMAGES_OUTPUTDIR)/jdk-signed and just leave it there. I would create a
> >>> separate rule for the signing part, where the target file is the
> >>> CodeResources file that codesign actually creates, and the prerequisite
> >>> files simply $(COPY_SIGNED_JDK_BUNDLE).
> >>>
> >>> Separate rules for creating a top level directory are not needed. The
> >>> rules generated from SetupCopyFiles will create all directories needed.
> >>>
> >>> I would also keep using the existing SetupBundleFile for the actual
&g

Re: RFR: JDK-8238534

[René Schünemann]

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 avoid temp dirs.
> I will try to move the creation of the signed image to MacBundles.gmk
> and then re-use the SetubBundleFile in Bundles.gmk.
>
> Rene
>
> On Fri, Feb 7, 2020 at 5:19 PM Erik Joelsson  wrote:
> >
> > Hello René,
> >
> > It's good to see an open solution to this, but I have some opinions on
> > the patch.
> >
> > The concept of building into "temp dirs" that are then removed is a
> > practice we try to avoid in the build. Whenever possible, each rule
> > should be a well defined transformation from a set of source files to a
> > target file. There is just no reason to remove the jdk-signed dir here.
> > If something goes wrong, you would want the dir around to investigate.
> > This also keeps incremental builds working as expected. Your current
> > patch will always rebuild the bundles, which is not ok.
> >
> > I would recommend putting the jdk-signed dir in
> > $(IMAGES_OUTPUTDIR)/jdk-signed and just leave it there. I would create a
> > separate rule for the signing part, where the target file is the
> > CodeResources file that codesign actually creates, and the prerequisite
> > files simply $(COPY_SIGNED_JDK_BUNDLE).
> >
> > Separate rules for creating a top level directory are not needed. The
> > rules generated from SetupCopyFiles will create all directories needed.
> >
> > I would also keep using the existing SetupBundleFile for the actual
> > bundling, even if most of the functionality in it is not used, just to
> > avoid more separate code paths than necessary.
> >
> > /Erik
> >
> > On 2020-02-07 02:05, René Schünemann wrote:
> > > For the Apple notarization process, the whole bundle in its final form
> > > has to be signed with the codesign tool.
> > > See the discussion here: https://bugs.openjdk.java.net/browse/JDK-8238225
> > >
> > > This change copies all JDK/JRE files to a temporary directory, which
> > > is then passed to the codesign tool. The temporary directory is then
> > > used as the base directory for the bundle archive and is getting
> > > removed after the archive has been created. This only applies when a
> > > valid code signing identity is set and the build type is release.
> > >
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8238534
> > > WebRev: 
> > > http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/01/
> > >
> > > Rene

Re: RFR: JDK-8238534

[René Schünemann]

Hi Erik,

thank you for your review and I totally agree with you. It would
definitely be better avoid temp dirs.
I will try to move the creation of the signed image to MacBundles.gmk
and then re-use the SetubBundleFile in Bundles.gmk.

Rene

On Fri, Feb 7, 2020 at 5:19 PM Erik Joelsson  wrote:
>
> Hello René,
>
> It's good to see an open solution to this, but I have some opinions on
> the patch.
>
> The concept of building into "temp dirs" that are then removed is a
> practice we try to avoid in the build. Whenever possible, each rule
> should be a well defined transformation from a set of source files to a
> target file. There is just no reason to remove the jdk-signed dir here.
> If something goes wrong, you would want the dir around to investigate.
> This also keeps incremental builds working as expected. Your current
> patch will always rebuild the bundles, which is not ok.
>
> I would recommend putting the jdk-signed dir in
> $(IMAGES_OUTPUTDIR)/jdk-signed and just leave it there. I would create a
> separate rule for the signing part, where the target file is the
> CodeResources file that codesign actually creates, and the prerequisite
> files simply $(COPY_SIGNED_JDK_BUNDLE).
>
> Separate rules for creating a top level directory are not needed. The
> rules generated from SetupCopyFiles will create all directories needed.
>
> I would also keep using the existing SetupBundleFile for the actual
> bundling, even if most of the functionality in it is not used, just to
> avoid more separate code paths than necessary.
>
> /Erik
>
> On 2020-02-07 02:05, René Schünemann wrote:
> > For the Apple notarization process, the whole bundle in its final form
> > has to be signed with the codesign tool.
> > See the discussion here: https://bugs.openjdk.java.net/browse/JDK-8238225
> >
> > This change copies all JDK/JRE files to a temporary directory, which
> > is then passed to the codesign tool. The temporary directory is then
> > used as the base directory for the bundle archive and is getting
> > removed after the archive has been created. This only applies when a
> > valid code signing identity is set and the build type is release.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8238534
> > WebRev: 
> > http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/01/
> >
> > Rene

RFR: JDK-8238534

[René Schünemann]

For the Apple notarization process, the whole bundle in its final form
has to be signed with the codesign tool.
See the discussion here: https://bugs.openjdk.java.net/browse/JDK-8238225

This change copies all JDK/JRE files to a temporary directory, which
is then passed to the codesign tool. The temporary directory is then
used as the base directory for the bundle archive and is getting
removed after the archive has been created. This only applies when a
valid code signing identity is set and the build type is release.

Bug: https://bugs.openjdk.java.net/browse/JDK-8238534
WebRev: 
http://cr.openjdk.java.net/~rschuenemann/wr/2020/8238534-macos_sign_bundles/01/

Rene

Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables

[René Schünemann]

Hi Erik, Christoph,

thank you!

Rene

On Tue, Dec 10, 2019 at 4:27 PM Langer, Christoph
 wrote:
>
> Hi René,
>
> LGTM, too.
>
> I'll sponsor it for you.
>
> Cheers
> Christoph
>
> > -Original Message-
> > From: Erik Joelsson 
> > Sent: Dienstag, 10. Dezember 2019 15:35
> > To: René Schünemann ; Langer, Christoph
> > 
> > Cc: build-dev@openjdk.java.net
> > Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries 
> > and
> > executables
> >
> > Looks good.
> >
> > /Erik
> >
> > On 2019-12-10 03:44, René Schünemann wrote:
> > > Thank you Christoph.
> > >
> > > I have fixed the indentation in NativeCompilation.gmk and removed the
> > > "com.apple.security.cs.disable-executable-page-protection"
> > > entitlement.
> > >
> > > Updated webrev:
> > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-
> > mac_notarization/02/
> > >
> > > Rene
> > >
> > > On Tue, Dec 10, 2019 at 11:25 AM Langer, Christoph
> > >  wrote:
> > >> Hi René,
> > >>
> > >> thanks for doing this.
> > >>
> > >> I agree to Erik's findings, these should be addressed. Other than that, I
> > have no further points.
> > >>
> > >> It would be good, if this little enhancement can be pushed before
> > Thursday to make it into JDK14 without special approval.
> > >>
> > >> Best regards
> > >> Christoph
> > >>
> > >>
> > >>> -Original Message-
> > >>> From: build-dev  On Behalf Of
> > René
> > >>> Schünemann
> > >>> Sent: Dienstag, 10. Dezember 2019 09:27
> > >>> To: Erik Joelsson 
> > >>> Cc: build-dev@openjdk.java.net
> > >>> Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all 
> > >>> libraries
> > and
> > >>> executables
> > >>>
> > >>> Hello Erik,
> > >>>
> > >>> thank you for your review.
> > >>>
> > >>> On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson
> > 
> > >>> wrote:
> > >>>> Hello René,
> > >>>>
> > >>>> Nice to see an OpenJDK solution to this. (Our Oracle solution requires
> > >>>> too much corp specific customization to really benefit from code
> > sharing
> > >>>> with a simple codesign based implementation)
> > >>>>
> > >>>> On 2019-12-09 08:06, René Schünemann wrote:
> > >>>>> Here is the webrev:
> > >>>>> http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-
> > >>> mac_notarization/01/
> > >>>> Generally looks good.
> > >>>>
> > >>>> NativeCompilation.gmk, line 1132 looks weirdly indented. The line
> > could
> > >>>> also benefit from being broken up. See [1] for guidance.
> > >>>>
> > >>> I agree. I will break it into two lines.
> > >>>
> > >>>>> On Mon, Dec 9, 2019 at 5:05 PM René Schünemann
> > >>>>>  wrote:
> > >>>>>> Hi,
> > >>>>>>
> > >>>>>> for the macOS notarization process, all executables and libraries
> > need
> > >>>>>> to be codesigned with hardened runtime (--options runtime) and
> > >>> secure
> > >>>>>> timestamp (--timestamp) enabled. Additionally for the OpenJDK
> > certain
> > >>>>>> entitlements have to be set during codesigning:
> > >>>>>>
> > >>>>>> * com.apple.security.cs.allow-jit
> > >>>>>> * com.apple.security.cs.allow-unsigned-executable-memory
> > >>>>>> * com.apple.security.cs.disable-executable-page-protection
> > >>>> In our testing, we saw no need for disable-executable-page-
> > protection.
> > >>>> Did you actually see missing this trigger any problems?
> > >>> I'm actually not quite sure. We have used this set internally for
> > notarization.
> > >>> I will go back an do some additional testing with this specific
> > >>> entitlement removed.
> > >>>
> > >>>>>> * com.apple.security.cs.allow-dyld-environment-variables
> > >>>>>> * com.apple.security.cs.debugger
> > >>>>>>
> > >>>>>> With this change the macOS codesign tool is being run for all native
> > >>>>>> executables and libraries.
> > >>>>>>
> > >>>>>> Additionally this change introduces a new configure option:
> > >>>>>> --with-macosx-codesign-identity
> > >>>>>>
> > >>>>>> This options allows to specify a codesigning identity stored in the
> > >>>>>> macOS keychain.
> > >>>>>> When this option is not set it falls back to "openjdk_codesign".
> > >>>>>>
> > >>>>>> Thanks,
> > >>>>>> Rene
> > >>>> /Erik
> > >>>>
> > >>>> [1] http://openjdk.java.net/groups/build/doc/code-conventions.html
> > >>>>
> > >>> Rene

Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables

[René Schünemann]

Thank you Christoph.

I have fixed the indentation in NativeCompilation.gmk and removed the
"com.apple.security.cs.disable-executable-page-protection"
entitlement.

Updated webrev:
http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-mac_notarization/02/

Rene

On Tue, Dec 10, 2019 at 11:25 AM Langer, Christoph
 wrote:
>
> Hi René,
>
> thanks for doing this.
>
> I agree to Erik's findings, these should be addressed. Other than that, I 
> have no further points.
>
> It would be good, if this little enhancement can be pushed before Thursday to 
> make it into JDK14 without special approval.
>
> Best regards
> Christoph
>
>
> > -Original Message-
> > From: build-dev  On Behalf Of René
> > Schünemann
> > Sent: Dienstag, 10. Dezember 2019 09:27
> > To: Erik Joelsson 
> > Cc: build-dev@openjdk.java.net
> > Subject: Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries 
> > and
> > executables
> >
> > Hello Erik,
> >
> > thank you for your review.
> >
> > On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson 
> > wrote:
> > >
> > > Hello René,
> > >
> > > Nice to see an OpenJDK solution to this. (Our Oracle solution requires
> > > too much corp specific customization to really benefit from code sharing
> > > with a simple codesign based implementation)
> > >
> > > On 2019-12-09 08:06, René Schünemann wrote:
> > > > Here is the webrev:
> > > > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-
> > mac_notarization/01/
> > >
> > > Generally looks good.
> > >
> > > NativeCompilation.gmk, line 1132 looks weirdly indented. The line could
> > > also benefit from being broken up. See [1] for guidance.
> > >
> >
> > I agree. I will break it into two lines.
> >
> > > >
> > > > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann
> > > >  wrote:
> > > >> Hi,
> > > >>
> > > >> for the macOS notarization process, all executables and libraries need
> > > >> to be codesigned with hardened runtime (--options runtime) and
> > secure
> > > >> timestamp (--timestamp) enabled. Additionally for the OpenJDK certain
> > > >> entitlements have to be set during codesigning:
> > > >>
> > > >> * com.apple.security.cs.allow-jit
> > > >> * com.apple.security.cs.allow-unsigned-executable-memory
> > > >> * com.apple.security.cs.disable-executable-page-protection
> > > In our testing, we saw no need for disable-executable-page-protection.
> > > Did you actually see missing this trigger any problems?
> >
> > I'm actually not quite sure. We have used this set internally for 
> > notarization.
> > I will go back an do some additional testing with this specific
> > entitlement removed.
> >
> > > >> * com.apple.security.cs.allow-dyld-environment-variables
> > > >> * com.apple.security.cs.debugger
> > > >>
> > > >> With this change the macOS codesign tool is being run for all native
> > > >> executables and libraries.
> > > >>
> > > >> Additionally this change introduces a new configure option:
> > > >> --with-macosx-codesign-identity
> > > >>
> > > >> This options allows to specify a codesigning identity stored in the
> > > >> macOS keychain.
> > > >> When this option is not set it falls back to "openjdk_codesign".
> > > >>
> > > >> Thanks,
> > > >> Rene
> > > /Erik
> > >
> > > [1] http://openjdk.java.net/groups/build/doc/code-conventions.html
> > >
> >
> > Rene

Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables

[René Schünemann]

Hello Erik,

thank you for your review.

On Mon, Dec 9, 2019 at 5:48 PM Erik Joelsson  wrote:
>
> Hello René,
>
> Nice to see an OpenJDK solution to this. (Our Oracle solution requires
> too much corp specific customization to really benefit from code sharing
> with a simple codesign based implementation)
>
> On 2019-12-09 08:06, René Schünemann wrote:
> > Here is the webrev:
> > http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-mac_notarization/01/
>
> Generally looks good.
>
> NativeCompilation.gmk, line 1132 looks weirdly indented. The line could
> also benefit from being broken up. See [1] for guidance.
>

I agree. I will break it into two lines.

> >
> > On Mon, Dec 9, 2019 at 5:05 PM René Schünemann
> >  wrote:
> >> Hi,
> >>
> >> for the macOS notarization process, all executables and libraries need
> >> to be codesigned with hardened runtime (--options runtime) and secure
> >> timestamp (--timestamp) enabled. Additionally for the OpenJDK certain
> >> entitlements have to be set during codesigning:
> >>
> >> * com.apple.security.cs.allow-jit
> >> * com.apple.security.cs.allow-unsigned-executable-memory
> >> * com.apple.security.cs.disable-executable-page-protection
> In our testing, we saw no need for disable-executable-page-protection.
> Did you actually see missing this trigger any problems?

I'm actually not quite sure. We have used this set internally for notarization.
I will go back an do some additional testing with this specific
entitlement removed.

> >> * com.apple.security.cs.allow-dyld-environment-variables
> >> * com.apple.security.cs.debugger
> >>
> >> With this change the macOS codesign tool is being run for all native
> >> executables and libraries.
> >>
> >> Additionally this change introduces a new configure option:
> >> --with-macosx-codesign-identity
> >>
> >> This options allows to specify a codesigning identity stored in the
> >> macOS keychain.
> >> When this option is not set it falls back to "openjdk_codesign".
> >>
> >> Thanks,
> >> Rene
> /Erik
>
> [1] http://openjdk.java.net/groups/build/doc/code-conventions.html
>

Rene

Re: RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables

[René Schünemann]

Here is the webrev:
http://cr.openjdk.java.net/~goetz/wr19/rene/8235585-mac_notarization/01/

On Mon, Dec 9, 2019 at 5:05 PM René Schünemann
 wrote:
>
> Hi,
>
> for the macOS notarization process, all executables and libraries need
> to be codesigned with hardened runtime (--options runtime) and secure
> timestamp (--timestamp) enabled. Additionally for the OpenJDK certain
> entitlements have to be set during codesigning:
>
> * com.apple.security.cs.allow-jit
> * com.apple.security.cs.allow-unsigned-executable-memory
> * com.apple.security.cs.disable-executable-page-protection
> * com.apple.security.cs.allow-dyld-environment-variables
> * com.apple.security.cs.debugger
>
> With this change the macOS codesign tool is being run for all native
> executables and libraries.
>
> Additionally this change introduces a new configure option:
> --with-macosx-codesign-identity
>
> This options allows to specify a codesigning identity stored in the
> macOS keychain.
> When this option is not set it falls back to "openjdk_codesign".
>
> Thanks,
> Rene

RFR: JDK-8235585: Enable macOS codesigning for all libraries and executables

[René Schünemann]

Hi,

for the macOS notarization process, all executables and libraries need
to be codesigned with hardened runtime (--options runtime) and secure
timestamp (--timestamp) enabled. Additionally for the OpenJDK certain
entitlements have to be set during codesigning:

* com.apple.security.cs.allow-jit
* com.apple.security.cs.allow-unsigned-executable-memory
* com.apple.security.cs.disable-executable-page-protection
* com.apple.security.cs.allow-dyld-environment-variables
* com.apple.security.cs.debugger

With this change the macOS codesign tool is being run for all native
executables and libraries.

Additionally this change introduces a new configure option:
--with-macosx-codesign-identity

This options allows to specify a codesigning identity stored in the
macOS keychain.
When this option is not set it falls back to "openjdk_codesign".

Thanks,
Rene

Re: RFR: 8205091: AIX: build errors in hotspot after 8203641: Refactor String Deduplication into shared

[René Schünemann]

Hi Matthias,

the name SI seems also quite "common" and may result in other naming
clashes in the future.
Maybe something more readable like STAT_IMPL?

Please also change the name in the comment:

// STAT:  String Dedup Stat implementation

Regards,
Rene

On Fri, Jun 15, 2018 at 9:47 AM, Baesken, Matthias
 wrote:
> Please review  this small  change  that fixes the AIX build after  "8203641: 
> Refactor String Deduplication into shared".
>
> We are getting this  compilation error  :
> /build_ci_jdk_jdk_rs6000_64/src/hotspot/share/gc/shared/stringdedup/stringDedup.hpp",
>  line 107.38: 1540-0063 (S) The text "1" is unexpected.
>
>
> Looks like the name of the second template parameter (STAT)
>
> template 
> static void initialize_impl();
>
> is clashing with defines from the AIX system headers  (where I find  #define 
> STAT  1 ) .
> Renaming  STAT  to something else fixes the build on AIX .
>
> Webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205091/
>
> Bug :
>
> https://bugs.openjdk.java.net/browse/JDK-8205091
>
>
> Thanks, Matthias