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) > 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 > > <[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
