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

Create nsis installer script #5233

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

IsaacPD
Copy link
Contributor

@IsaacPD IsaacPD commented Jan 15, 2021

Related: #5089

Description
Adds a basic nsis installer script that can be run by:

  1. Install makensis
    1. linux apt-get install -y nsis nsis-doc nsis-pluginapi
    2. mac brew install makensis
    3. windows https://nsis.sourceforge.io/Download
  2. Download https://nsis.sourceforge.io/EnVar_plug-in and unzip in the nsis install directory on whatever platform you are on
    unzip EnVar_plugin.zip -d /usr/local/share/nsis/ 
  3. compile the windows binary make ./out/skaffold-windows-amd64.exe
  4. Copy the binary and whatever other files to include in the installation in the installer out directory
    mkdir installers/windows/out && cp ./out/skaffold-windows-amd64.exe installers/windows/out
  5. Set skaffold version and run makensis SKAFFOLD_VERSION=1.18.0.0 makensis -V4 skaffold.nsi
    1. skaffold version must be of the form X.X.X.X in order to be accepted by makensis

The installer can:

  • Install to a user specified directory
  • Add that directory to the user's PATH
  • Create an uninstaller for that uninstalls skaffold and removes the install directory from the user's PATH

Follow-up Work
Add a step in the release process to publish the installer alongside all the other binaries. Phase out chocolatey from use

@IsaacPD IsaacPD requested a review from a team as a code owner January 15, 2021 00:52
@google-cla google-cla bot added the cla: yes label Jan 15, 2021
@codecov
Copy link

codecov bot commented Jan 15, 2021

Codecov Report

Merging #5233 (6499fcb) into master (35214eb) will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5233      +/-   ##
==========================================
+ Coverage   71.82%   71.85%   +0.02%     
==========================================
  Files         387      388       +1     
  Lines       13928    14036     +108     
==========================================
+ Hits        10004    10085      +81     
- Misses       3190     3206      +16     
- Partials      734      745      +11     
Impacted Files Coverage Δ
pkg/skaffold/yaml/yaml.go 54.83% <0.00%> (-8.13%) ⬇️
pkg/skaffold/runner/build_deploy.go 68.88% <0.00%> (-1.01%) ⬇️
pkg/skaffold/util/util.go 83.19% <0.00%> (-0.15%) ⬇️
pkg/skaffold/docker/docker_init.go 100.00% <0.00%> (ø)
pkg/skaffold/initializer/build/util.go 100.00% <0.00%> (ø)
pkg/skaffold/trigger/fsnotify/trigger.go 71.05% <0.00%> (ø)
pkg/skaffold/initializer/build/builders.go 70.00% <0.00%> (ø)
pkg/skaffold/initializer/analyze/builder.go 100.00% <0.00%> (ø)
pkg/skaffold/trigger/fsnotify/workdir.go
pkg/skaffold/tags/paths.go 65.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35214eb...6499fcb. Read the comment docs.

@IsaacPD IsaacPD force-pushed the nsis-windows-installer branch from 82a227c to 6499fcb Compare January 19, 2021 17:37
@MarlonGamez
Copy link
Contributor

QQ: Should it be installer/windows/ or installers/windows/ ? The PR description uses the the former but the added file path is the latter

@IsaacPD
Copy link
Contributor Author

IsaacPD commented Jan 20, 2021

QQ: Should it be installer/windows/ or installers/windows/ ? The PR description uses the the former but the added file path is the latter

Oops it should be installers/windows/ as installers is what currently exists in the repo

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@IsaacPD IsaacPD merged commit 5719537 into GoogleContainerTools:master Jan 20, 2021
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.

2 participants