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

MSI Installer for CLI #1065

Merged
merged 9 commits into from
Sep 22, 2022
Merged

MSI Installer for CLI #1065

merged 9 commits into from
Sep 22, 2022

Conversation

shivamkm07
Copy link
Contributor

Description

This adds Support for MSI Installer in CLI.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1064 _

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: shivamkm07 <shivamkm07@gmail.com>
Signed-off-by: shivamkm07 <shivamkm07@gmail.com>
@shivamkm07 shivamkm07 changed the title Msi installer CLI MSI Installer Sep 13, 2022
@shivamkm07 shivamkm07 changed the title CLI MSI Installer MSI Installer for CLI Sep 13, 2022
@shivamkm07
Copy link
Contributor Author

NOTE: MSI ProductVersion is incompatible with semver. It only supports major.minor.build format, i.e. no pre-releases or build metadata. Also it has additional constraints like maximum value of major and minor fields is 255 and build field is 65535.

So for msi installer, we can't have versions like "1.9.0-rc.1". Instead msi does allow a fourth numeric field in version, which is although ignored, but still legally allowed, like 1.9.0.1. So for rc releases, msi would be versioned the same.

In summary:
Stable Releases: REL_VERSION: 1.9.0, MSI_VERSION: 1.9.0
Pre-Releases: REL_VERSION: 1.9.0-rc.1, MSI_VERSION: 1.9.0.1

For edge builds, it would be tagged as '0.0.0.0'

@shivamkm07 shivamkm07 marked this pull request as ready for review September 13, 2022 23:43
@shivamkm07 shivamkm07 requested review from a team as code owners September 13, 2022 23:43
Signed-off-by: shivam <shivamkm07@gmail.com>
@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #1065 (ffb4c37) into master (9e07fcd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1065   +/-   ##
=======================================
  Coverage   29.20%   29.20%           
=======================================
  Files          35       35           
  Lines        2359     2359           
=======================================
  Hits          689      689           
  Misses       1594     1594           
  Partials       76       76           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pravinpushkar
Copy link
Contributor

@shivamkm07 Also update the README.md section for this ?

@shivamkm07 shivamkm07 mentioned this pull request Sep 16, 2022
3 tasks
Signed-off-by: shivam <shivamkm07@gmail.com>
Signed-off-by: shivam <shivamkm07@gmail.com>
Copy link
Collaborator

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

@shivamkm07 have the workflow changes been tested? If so please link it here

@shivamkm07
Copy link
Contributor Author

shivamkm07 commented Sep 16, 2022

@shivamkm07 have the workflow changes been tested? If so please link it here

Yes: https://github.com/shivamkm07/cli/actions/runs/3051046246

The published msi file: https://github.com/shivamkm07/cli/releases/download/vedge/dapr.msi

Signed-off-by: shivam <shivamkm07@gmail.com>
Copy link
Collaborator

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Contributor

@pravinpushkar pravinpushkar left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks @shivamkm07 for the clarifications.

@@ -29,3 +29,8 @@ test_output.json

# Go Workspaces (introduced in Go 1.18+)
go.work

#Wix files
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#Wix files
# Wix files

@@ -0,0 +1,416 @@
{\rtf1\adeflang1025\ansi\ansicpg1252\uc1\adeff31507\deff0\stshfdbch31506\stshfloch31506\stshfhich31506\stshfbi31507\deflang16393\deflangfe16393\themelang16393\themelangfe0\themelangcs0{\fonttbl{\f2\fbidi \fmodern\fcharset0\fprq1{\*\panose 02070309020205020404}Courier New;}{\f34\fbidi \froman\fcharset0\fprq2{\*\panose 02040503050406030204}Cambria Math;}
Copy link
Member

Choose a reason for hiding this comment

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

Curious to know, how did you generate this RTF file? Is the themedata at the bottom required?

@mukundansundar mukundansundar merged commit 80f7bee into dapr:master Sep 22, 2022
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 Installer for CLI
4 participants