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

Fix JDK install on Windows. #131

Merged
merged 1 commit into from Jan 24, 2014

Conversation

cupcicm
Copy link
Contributor

@cupcicm cupcicm commented Jan 23, 2014

I think that #100 broke the windows install when using a JDK installer.

For example, if I download both a JRE and a JDK here : http://www.oracle.com/technetwork/java/javase/downloads/jdk7-downloads-1880260.html and here http://java.com/en/download/manual.jsp#win
and then run both

.\jdk-7u51-windows-x64.exe /s INSTALLDIR="C:\java"
and
.\jre-7u51-windows-x64.exe /s INSTALLDIR="C:\java"

The second one works, but the first one doesn't. It does when I remove the backward slashes.

The fix seems to be http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4966488

I tested this change with both a JDK and a JRE installer, and both were correctly installed.

Commit b42c246 introduces quotes around the install path, so that
we can safely provide paths with strings. Unfortunately, it seems
that this method works only with JREs, not with JDKs.
It seems that the recommended way to call the installer (described at
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4966488)
is by adding /v"/qn INSTALLDIR=\"path\""
I tested that this change makes it possible to install both a JRE
and a JDK.
@carmstrong
Copy link
Contributor

My first thought is that this isn't necessary a bug - this cookbook aims to install the JDK for all platforms, not the JRE package, which will probably break other recipes as well. That said, I am not necessarily opposed to this change, as long as it doesn't break the installation of the JDK.

Have you tested this change on Windows by setting node['java']['windows']['url'] to the package URL of the JDK? Does the Chef run complete as expected? From your example, I can't tell if you tested this change with Chef yet or just manually running the installers.

Unfortunately I don't think we can run automated tests on Windows like we can on other platforms.. :(

@cupcicm
Copy link
Contributor Author

cupcicm commented Jan 24, 2014

Hi,

Just to be clear : in my case, the JDK install was broken, and the JRE install worked. My change fixes it for the JDK install.

Yes, I have tested by overridding the node['java']['windows']['url'] and then installing both a JDK and a JRE. Both were installed correctly. My path contained no spaces, but I don't think I broke this since I am keeping the backward slashes. Sorry if my message wasn't clear.

carmstrong added a commit that referenced this pull request Jan 24, 2014
@carmstrong carmstrong merged commit 4d75d81 into sous-chefs:master Jan 24, 2014
@carmstrong
Copy link
Contributor

@cupcicm Good enough for me! Thanks for the patch.

@lock
Copy link

lock bot commented May 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants