Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openjdk: fix truststore-from-env patch for jdk10 #41776

Merged
merged 1 commit into from Jun 11, 2018

Conversation

pclewis
Copy link
Contributor

@pclewis pclewis commented Jun 10, 2018

Motivation for this change

read-truststore-from-env-jdk10.patch does not work.

storePropName will be jsseDefaultStore if the property isn't present, and jsseDefaultStore is never null, so the branch to use the environment variable will never be taken.

Easy way to test if it is working correctly from shell (should output NONE):

echo 'var c = Class.forName("sun.security.ssl.TrustStoreManager$TrustStoreDescriptor"); var ci = c.getDeclaredMethod("createInstance"); var n = c.getDeclaredField("storeName"); ci.setAccessible(true); n.setAccessible(true); System.out.println(n.get(ci.invoke(null)));' | JAVAX_NET_SSL_TRUSTSTORE=NONE jshell -

This functionality seems necessary to make clojure work after 5a53b98, because the cert for https://repo.clojars.org isn't trusted by the included cacerts.

Things done

The env var is supposed to be preferred to jssecacerts, so we can use it as the default in the call to System.getProperty instead, and use the null check to fall back on jsseDefaultStore.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

storePropName will be jsseDefaultStore if the property isn't present, and
jsseDefaultStore is never null, so the branch to use the environment variable
would never be taken.

The env var is supposed to be preferred to jssecacerts, so we can use it as
the default in the call to System.getProperty, and use the null check to fall
back on jsseDefaultStore instead.
@lukateras
Copy link
Member

@GrahamcOfBorg build jdk10

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: jdk10

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: jdk10

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: jdk10

Partial log (click to expand)

strange: no string table
strange: no string table
strange: no string table
strange: no string table
strange: no string table
strange: no string table
strange: no string table
strange: no string table
strange: no string table
/nix/store/20i73iaqmnjrw1in22fhf004r3mfnm55-openjdk-10.0.1-b10

@Mic92 Mic92 requested a review from NeQuissimus June 11, 2018 09:52
@NeQuissimus NeQuissimus merged commit e0d1c63 into NixOS:master Jun 11, 2018
@pclewis pclewis deleted the openjdk-jdk10-truststore-from-env branch June 14, 2018 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants