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

Migrate to Velopack & Github Action #2616

Open
wants to merge 74 commits into
base: dev
Choose a base branch
from
Open

Migrate to Velopack & Github Action #2616

wants to merge 74 commits into from

Conversation

taooceros
Copy link
Member

@taooceros taooceros commented Mar 24, 2024

Velopack (https://velopack.io/) is the successor of Cloud.Squirrel (https://github.com/clowd/Clowd.Squirrel)

Several change in this pr

  1. Remove shortcut related function https://docs.velopack.io/integrating/shortcuts
  2. Implement a custom restart script which doesn't depend on the updater (which should not be the use case anyway). This allows restart be called at dev environment (breaking change).
  3. Create the portable data folder if the package is not inside %LocalAppData% as velopack will pack the portable version for us, which doesn't allow us to modify (easily) to insert a custom folder.
  4. The post_build script is modified to add information about channel. Prerelease repo will have win-x64-prerelease channel, while the main repo will have win-x64-stable channel. https://docs.velopack.io/packaging/channels
  5. It also contains an experimental github action CI, which has a faster build time and larger concurrency allowance.

TODO:

  • Toggling out of portable via settings doesnt seem to do anything except creating the UserData folder in roaming.
  • Expecting to move portable UserData to roaming, plus start menu and uninstall program entries created.
  • Toggling into portable mode does not remove the uninstall entry in 'Uninstall Program' in control panel, nor does it remove the entry from start menu.
  • Can we please make the parent folder name FlowLauncher not flowlauncher, same goes for the UserData folder name in roaming.
  • Can we please change the publisher in uninstall program to 'Flow Launcher Team' not 'FlowLauncher'.

Tested:

  • The build file size should be similar size
  • The protable file should create UserData folder
  • Things should work

Futher Consideration:

  • Cleanup post_build script. Build Portable is no longer needed as that will be handled by vpk pack
  • Implement the logic of switching channel (maybe can delay to future). With that and once this pr is merged, we can officially test whether the upgrade process work. I will also create one pr stemmed from master to see how the upgrade from squirrel will behave.

Note: Maybe we want to squash so revert is easier?

@taooceros taooceros changed the title Velopack Migrate to Velopack Mar 24, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@VictoriousRaptor
Copy link
Contributor

vpk: D:\a\Flow.Launcher\Flow.Launcher\Scripts\post_build.ps1:126
Line |
126 | vpk pack --packVersion $version --packDir $input --packId FlowLau …
| ~~~
| The term '~/.dotnet/tools/vpk.exe' is not recognized as a name of a cmdlet, function, script file, or executable
| program. Check the spelling of the name, or if a path was included, verify that the path is correct and try
| again.

Looks like something's broken but I don't have any idea.

@taooceros
Copy link
Member Author

vpk: D:\a\Flow.Launcher\Flow.Launcher\Scripts\post_build.ps1:126
Line |
126 | vpk pack --packVersion $version --packDir $input --packId FlowLau …
| ~~~
| The term '~/.dotnet/tools/vpk.exe' is not recognized as a name of a cmdlet, function, script file, or executable
| program. Check the spelling of the name, or if a path was included, verify that the path is correct and try
| again.

Looks like something's broken but I don't have any idea.

Previously the vpk installation takes 30s, so I add a cache, but weird, it should at least run the install🤔

@jjw24 jjw24 marked this pull request as draft May 22, 2024 03:28
@taooceros taooceros marked this pull request as ready for review May 25, 2024 23:39
@taooceros
Copy link
Member Author

@JohnTheGr8 maybe let's get this in first. I think handling the conflict in .net 8 will be easier.

@taooceros taooceros changed the title Migrate to Velopack Migrate to Velopack & Github Action May 25, 2024
@jjw24 jjw24 enabled auto-merge (squash) May 25, 2024 23:57
@jjw24 jjw24 added the review in progress Indicates that a review is in progress for this PR label May 26, 2024
@@ -49,7 +49,8 @@

<ItemGroup>
<PackageReference Include="Moq" Version="4.18.4" />
<PackageReference Include="nunit" Version="3.14.0" />
<PackageReference Include="NUnit" Version="4.1.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
Copy link
Member

Choose a reason for hiding this comment

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

double reference to Microsoft.NET.Test.Sdk here (mistake during conflict resolve, I assume)

Comment on lines +20 to +22
#cache:
# - '%USERPROFILE%\.nuget\packages -> **.sln, **.csproj' # preserve nuget folder (packages) unless the solution or projects change

Copy link
Member

Choose a reason for hiding this comment

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

was this change meant to be temporary? Also, shouldn't we be caching the dotnet tools folder here, as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think cache is not improving the building time....it seems to take very long time to unzip the cache...

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

Hmm, couple of things:

  1. Toggling out of portable via settings doesnt seem to do anything except creating the UserData folder in roaming. Expecting to move portable UserData to roaming, plus start menu and uninstall program entries created.
  2. Toggling into portable mode does not remove the uninstall entry in 'Uninstall Program' in control panel, nor does it remove the entry from start menu.
  3. Can we please make the parent folder name FlowLauncher not flowlauncher, same goes for the UserData folder name in roaming.
  4. Can we please change the publisher in uninstall program to 'Flow Launcher Team' not 'FlowLauncher'.
    image

@taooceros
Copy link
Member Author

Actually, on the second thought, do we still want to preserve the ability to change from non-portable to portable? I think if user want to use portable they would download the portable version on first use, rather than converting the installed version to portable. I think this is kindly a historical reason for us to have these feature...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review in progress Indicates that a review is in progress for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants