RFR: 8356047: [macos] jpackage produces confusing post- and pre- installation PKG scripts [v8]

Alexey Semenyuk asemenyuk at openjdk.org
Wed Oct 8 01:15:08 UTC 2025


On Tue, 7 Oct 2025 23:50:55 GMT, Alexander Matveev <almatvee at openjdk.org> wrote:

>> - Removed pre- and post- installation PKG scripts.
>> - This code is not needed and PKG should create destination folder and set correct permissions.
>> - If for some reason it is not happens, permissions issues should be fixed when jpackage prepares application bundle. PKG should keep all permissions unchanged when packaging and installing bundle.
>> - Users will have ability to provide pre- and post- installation PKG scripts if needed.
>> - `INSTALL_LOCATION` and `APP_LOCATION` substitution is removed, since `$2` argument in scripts is same as `INSTALL_LOCATION`.
>> - I think code in these scripts are some legacy leftovers.
>> - Added test to test that pre- and post-scripts are no longer exist and can be added via `--resource-dir`.
>
> Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8356047: [macos] jpackage produces confusing post- and pre- installation PKG scripts [v6]

src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacPkgPackager.java line 291:

> 289:             } catch (IOException ex) {
> 290:                 throw new UncheckedIOException(ex);
> 291:             }

This test is wrong. It contradicts the semantics of the `scriptsRoot()` function. If `scriptsRoot()` returns a non-empty Optional, something should be in the script directory.

It would be better to alter `scriptsRoot()` to return an empty Optional instance if there are no scripts for the main pkg package. Something like:

    Optional<Path> scriptsRoot() {
        if (pkg.app().appStore() || pkg.isRuntimeInstaller() || MacPkgInstallerScripts.createAppScripts().setResourceDir(env).isEmpty()) {
            return Optional.empty();
        } else {
            return Optional.of(env.configDir().resolve("scripts"));
        }
    }


where `setResourceDir(BuildEnv)` and `isEmpty()` are two new functions in PackageScripts class:

    PackageScripts<T> setResourceDir(BuildEnv env) {
        env.resourceDir().ifPresent(this::setResourceDir);
        return this;
    }

    boolean isEmpty() {
        return scripts.values().stream().map(ShellScriptResource::getResource).allMatch(overridableResource -> {
            try {
                return overridableResource.saveToStream(null) == null;
            } catch (IOException ex) {
                throw new UncheckedIOException(ex);
            }
        });
    }


As a supplementary change in MacPkgPackager, it would be good to replace `.setResourceDir(env.resourceDir().orElse(null))` with the new `.setResourceDir(env)`.

src/jdk.jpackage/share/classes/jdk/jpackage/internal/OverridableResource.java line 237:

> 235:         Log.verbose(I18N.format("message.no-default-resource",
> 236:                 publicName, getPrintableCategory(), publicName));
> 237: 

I still believe this log message is misleading.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25510#discussion_r2412275171
PR Review Comment: https://git.openjdk.org/jdk/pull/25510#discussion_r2412276864


More information about the core-libs-dev mailing list