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

Fixes MSI Parameters Port and InstallDir #287

Merged
merged 4 commits into from
Mar 3, 2022

Conversation

JPRuskin
Copy link
Contributor

@JPRuskin JPRuskin commented Feb 25, 2022

Description

This PR aims to fix the parameters that are currently ignored in the MSI installer, PORT and INSTALLDIR.

As it was, it seems that they were being stored and then ignored in favour of a different property (PORTNUMBER and JENKINSDIR respectively).

Searching through the file for references to INSTALLDIR and PORT, you find there are no references where these are used - including where they are stored in the registry for later retrieval (:214).

An alternative would be to adjust the property names to match the internal use, but that could in theory surprise someone (though it's unlikely anyone has been successfully using these). Additionally, I was unsure where (if anywhere) these were documented.

Fixes #286

Testing

  1. Rebuilt MSI with latest changes (though I will note that I used WiX 3.14, this shouldn't affect the changes I made)
  2. Installed using parameters, e.g. msiexec /qn /norestart /l*v "~\Desktop\latestAttempt.log" /i "~\Desktop\jenkins-2.331.msi" INSTALLDIR=C:\ Jenkins PORT=9080 JAVA_HOME=``"$JavaHome``" JENKINS_ROOT=$env:ProgramData\Jenkins\
  3. Verified that the registry keys in HKLM:\SOFTWARE\Jenkins\InstalledProducts\Jenkins matched the expected values
  4. Verified that Jenkins was installed in the right directory, and was available on the expected port
  5. Verified that the contents of Jenkins.xml matched the expected values

Checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

InstallDir was being taken, processed, restored (in some cases) but never actually used.
JenkinsDir was the property that was actually being used.

This was confusing, so this commit changes the flow of the InstallDir input to end up in the JenkinsDir property.
The PORT property was able to be passed in, held in a separate variable ready to overwrite registry-searched values, and... was completely ignored in favour of the PORTNUMBER property.

This commit changes the flow of this variable such that it now ends up overwriting PORTNUMBER instead of PORT.
This comment appears to be outdated, or otherwise completely unfollowed.

Also removes a few instances of untidy whitespace.
@JPRuskin JPRuskin requested a review from a team as a code owner February 25, 2022 11:00
@JPRuskin JPRuskin changed the title Improve msi Fixes MSI Parameters Port and InstallDir Feb 25, 2022
@JPRuskin
Copy link
Contributor Author

JPRuskin commented Feb 25, 2022

It seems like this build error in centos-stream-9 is unrelated to my PR, as other PRs are having the same issue (and, well, I've not touched anything that would effect that).

Is there anything I should do to attempt to solve that?

@basil basil requested a review from slide March 3, 2022 15:38
@timja timja merged commit 762c08c into jenkinsci:master Mar 3, 2022
imonteroperez pushed a commit to imonteroperez/packaging that referenced this pull request Mar 23, 2022
Co-authored-by: Basil Crow <me@basilcrow.com>
(cherry picked from commit 762c08c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSI Parameters PORT and INSTALLDIR do not work
4 participants