Hi René, to me your latest webrev looks quite good. You could pull the PRODUCT_TARGETS += $(BUILD_JDK_BUNDLE) and LEGACY_TARGETS += $(BUILD_JRE_BUNDLE) out of the if/else blocks as they are common to both. But I also see a value in keeping them duplicated where they are, so they immediately follow the respective SetupBundleFile call.
Regarding the constant rebuild, I have no idea. Unfortunately I can't test it on my laptop, as I don't have codesigning set up there. I'll add your patch to our nightly test queue to check it doesn't break anything - but it definitely shouldn't. I can also sponsor the push eventually. Best regards Christoph > -----Original Message----- > From: build-dev <[email protected]> On Behalf Of René > Schünemann > Sent: Mittwoch, 12. Februar 2020 11:55 > To: Erik Joelsson <[email protected]> > Cc: [email protected] > Subject: Re: RFR: JDK-8238534 > > 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 <[email protected]> > 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 > <[email protected]> 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 > > >>> <[email protected]> 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 > <[email protected]> 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
