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

[jvmfinder] add registry HKLM/JavaSoft/JRE to searched places #370

Merged
merged 6 commits into from Nov 11, 2018

Conversation

Projects
None yet
2 participants
@marscher
Collaborator

marscher commented Nov 4, 2018

Fixes #345

@marscher marscher requested a review from Thrameos Nov 4, 2018

@Thrameos

This comment has been minimized.

Contributor

Thrameos commented Nov 4, 2018

Hmm. Did a problematic patch go in? Looks like something needs to be resolved with the CI.

@Thrameos

Everything seems quite reasonable in this pull request. However, the appveyor CI is not currently working so it will be difficult to tell if this is successful. I will try it manually as soon as I get a chance.

@marscher

This comment has been minimized.

Collaborator

marscher commented Nov 4, 2018

the usage of the preinstalled jdk speeds up Appveyor significantly. @Thrameos was there any reason to switch to the chocolatey version in the first place?

@marscher

This comment has been minimized.

Collaborator

marscher commented Nov 4, 2018

I just switched to pip to install dependencies because of this issue conda/conda/issues/7239

@Thrameos

This comment has been minimized.

Contributor

Thrameos commented Nov 4, 2018

I do not recall why I made a switch to using chocolaty. I think it was a matter of convenience as I believe we were manually installing pieces we needed at some point.

Any system the works to keep th CI going is fine with me.

@Thrameos

I like the change to hide jdk versions wit install dir.

# - cinst jdk7 -params 'installdir=C:\\jdk7'
# - cinst jdk9 -version 9.0.4.11 -params 'installdir=C:\\jdk9'
- cinst ant %CINST_OPTS%
- cinst ant %CINST_OPTS% --ignore-dependencies

This comment has been minimized.

@Thrameos

Thrameos Nov 4, 2018

Contributor

Can we add an install dir to the cinst of ant? That way we don't need to track versions.

This comment has been minimized.

@marscher

marscher Nov 5, 2018

Collaborator

I tried it, but the passed value was ignored. So it might follow another install location parameter...

marscher added some commits Nov 5, 2018

@marscher

This comment has been minimized.

Collaborator

marscher commented Nov 11, 2018

Test failures are unrelated. Since this improves CI, I'm going to merge it now.

@marscher marscher merged commit 3ce953a into jpype-project:master Nov 11, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@marscher marscher deleted the marscher:add_jvm_path branch Nov 11, 2018

@Thrameos

This comment has been minimized.

Contributor

Thrameos commented Nov 12, 2018

I think this whole patch should get pulled into devel as well. Everything looks good so it is just a matter of cleaning up any conflicts. I can try and give a shot in a day or two.

@marscher

This comment has been minimized.

Collaborator

marscher commented Nov 12, 2018

@Thrameos

This comment has been minimized.

Contributor

Thrameos commented Nov 12, 2018

That would be great. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment