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

ci: Finch Windows MSI tool #624

Merged
merged 1 commit into from
Oct 12, 2023
Merged

ci: Finch Windows MSI tool #624

merged 1 commit into from
Oct 12, 2023

Conversation

KevinLiAWS
Copy link
Contributor

Issue #, if available:

Description of changes:
MSI tool to generate Windows installer
Testing done:
Tested with github action and locally

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

SET FilePath=%InstallDir%\os\finch.yaml

if exist "%FilePath%" (
powershell -Command "$installPath = '%InstallDir%'.Replace('\', '/'); (Get-Content '%FilePath%') -replace '__INSTALLFOLDER__', $installPath | Set-Content '%FilePath%'"
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where __INSTALLFOLDER__ is set. Should there be a manual step to add it to os\finch.yaml before starting to build the installer?

Copy link
Contributor Author

@KevinLiAWS KevinLiAWS Oct 12, 2023

Choose a reason for hiding this comment

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

It is set when build Finch: make FINCH_ROOTFS_LOCATION_ROOT=/INSTALLFOLDER, similar to mac OS: https://github.com/runfinch/finch/blob/e21ae836d4f73ea07852e3cd1fc6cc16d56e4f32/.github/workflows/build-and-test-pkg.yaml#L62C12-L62C12
I have the github action for auto build, so we don't need to do it manually, unless we want to build it at local env

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. We should probably still add that to the README just in case we ever need to do a manual build

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I will update the ReadMe with the auto build PR.

Copy link
Member

@pendo324 pendo324 left a comment

Choose a reason for hiding this comment

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

LGTM pending one addition to the readme

SET FilePath=%InstallDir%\os\finch.yaml

if exist "%FilePath%" (
powershell -Command "$installPath = '%InstallDir%'.Replace('\', '/'); (Get-Content '%FilePath%') -replace '__INSTALLFOLDER__', $installPath | Set-Content '%FilePath%'"
Copy link
Member

Choose a reason for hiding this comment

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

Thanks. We should probably still add that to the README just in case we ever need to do a manual build

@KevinLiAWS KevinLiAWS merged commit be98367 into windev Oct 12, 2023
@KevinLiAWS KevinLiAWS deleted the msi-tool branch October 12, 2023 15:58
vsiravar pushed a commit that referenced this pull request Oct 17, 2023
Issue #, if available:

*Description of changes:*
MSI tool to generate Windows installer
*Testing done:*
Tested with github action and locally


- [X] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: [email protected] <[email protected]>
Co-authored-by: [email protected] <[email protected]>
vsiravar pushed a commit that referenced this pull request Oct 17, 2023
Issue #, if available:

*Description of changes:*
MSI tool to generate Windows installer
*Testing done:*
Tested with github action and locally

- [X] I've reviewed the guidance in CONTRIBUTING.md

#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: [email protected] <[email protected]>
Co-authored-by: [email protected] <[email protected]>
Signed-off-by: Vishwas Siravara <[email protected]>
pendo324 added a commit that referenced this pull request Jan 10, 2024
Issue #, if available:

*Description of changes:*
- translation logic to wsl paths
- persistent disk for windows
- CI/CD (workflows to run CI on every PR on windows runners, MSI
builder, Windows release automation)

This PR combines 4 distinct PRs to a separate windev branch. 
- additional disk for windows #594
- translation logic for wsl paths
#581
- CI #581
- Installer #624

This PR also contains bug fixes and modifications to e2e tests.  
*Testing done:*
Yes


- [X] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Signed-off-by: Vishwas Siravara <[email protected]>
Signed-off-by: Vishwas Siravara <[email protected]>
Signed-off-by: Gavin Inglis <[email protected]>
Signed-off-by: Justin Alvarez <[email protected]>
Signed-off-by: chaoningusc <[email protected]>
Signed-off-by: [email protected] <[email protected]>
Signed-off-by: Kevin Li <[email protected]>
Co-authored-by: Vishwas Siravara <[email protected]>
Co-authored-by: Gavin Inglis <[email protected]>
Co-authored-by: Justin <[email protected]>
Co-authored-by: Kevin Li <[email protected]>
Co-authored-by: chaoningusc <[email protected]>
Co-authored-by: Justin Alvarez <[email protected]>
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.

3 participants