-
Notifications
You must be signed in to change notification settings - Fork 47
Winget manifest uploader #620
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
base: main
Are you sure you want to change the base?
Conversation
2979a2c
to
90a5c5a
Compare
@Gijsreyn thanks for this, but I think the team needs to discuss if we want to separately publish to winget given that winget can install from the Store. Just not sure if we want to support this different way of installing. |
@SteveL-MSFT Got it Steve. Personally with all the issues related to the store, having the ability to choose the |
@denelon do you think there's value/reason to publish to WinGet repo in addition to the Store? |
I think there are folks who would prefer WinGet as it can support making earlier versions available as opposed to the store just offering the latest version. If you add the packaged version here in addition to the other methods, users can choose which type they prefer, but WinGet would still default to the MSIX package for a new installation unless the user specifies via arguments or settings. |
@Gijsreyn can you resolve the merge conflict and I'll review and get this merged. Thanks! |
…n/operation-methods into winget-manifest-uploader
…into winget-manifest-uploader
@SteveL-MSFT Merge conflicts resolved. I had to rewrite a little part, because the PR was quite outdated. There's a new switch introduced that switches between |
@Gijsreyn, I'd also suggest taking a look at how PowerToys publishes to WinGet using wingetcreate. @mdanish-kh has done a bunch of these at GitHub. |
While this script approach can definitely work, it involves a lot more "plumbing" than what could be simplified into a relatively straightforward post-publish GitHub Action. Examples:
Regardless, if it works it works :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gijsreyn it doesn't seem like all of Muhammad's feedback has been addressed? Also, do we only care about winget for msixbundle or do we expect people to want the zip?
build.ps1
Outdated
function Submit-DSCWinGetAssets { | ||
param( | ||
[string]$GitToken, | ||
[switch] $IsPreRelease |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[switch] $IsPreRelease | |
[switch]$IsPreRelease |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we've addressed the underlying issue for us to be able to automate this properly, which is we don't know what release assets are gonna be available when the release is created / published. Is it gonna be both ZIP & MSIX? Or only ZIP then MSIX is published later? This matters because as I mentioned in #620 (comment), wingetcreate requires the number of URLs passed to update command to match the number of installers in the latest winget-pkgs manifest. That means, if winget-pkgs manifest contains ZIP and MSIX installers, then the update command needs to receive ZIP and MSIX URLs for the new release. If winget-pkgs only contains ZIP, then the update flow expects only a ZIP URL to be provided, otherwise the command will fail. We can debate if this is an area of improvement for wingetcreate (which it may be -> microsoft/winget-create#351 (comment)), but the current flow requires same count of URLs.
Some hacky approaches I can think of:
Retry with multiple commands
- Try first with wingetcreate update passing only the ZIP URL -> this will pass if release assets includes ZIP & winget-pkgs manifest for latest DSC version only includes ZIP
- If above fails, it probably means the winget-pkgs manifest contains MSIXBundle installer & update command needs the MSIX URL to succeed.
- If the MSIX is available in release asset, pass that to wingetcreate upgrade command along with ZIP URL
- If MSIX is added at a later time to release assets, retry the workflow again when it's added or have the workflow be triggered automatically at a "release edited" event. (this may complicate things if release has to be manually edited for any other reason than adding the MSIX URL, invoking the workflow again for no reason & creating a PR)
on:
release:
types: [published, edited]
Only include ZIP for winget-pkgs
This may not be viable since I know some winget-cli workflows depend on the MSIX variant being available from winget-pkgs. However, if we only publish ZIP to winget-pkgs, we can reliably automate the update everytime since we know the ZIP asset is always available on release
- name: Publish Microsoft.DSC ${{ github.event.release.prerelease && 'Preview' || 'Stable' }} | ||
run: | | ||
$assets = '${{ toJSON(github.event.release.assets) }}' | ConvertFrom-Json | ||
$wingetRelevantAsset = $assets | Where-Object { $_.name -like '*.zip' -and $_.name -like '*.msixbundle' } | Select-Object -First 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code looks like it's either only gonna get the zip URL OR the MSIX URL, and we don't know which will it be?
Would it make sense to only support MSIX for now? Or do you think users would prefer ZIP since they can just install MSIX via Store (except for Server)? |
I'm not particularly aware of the user preferences, since I haven't seen an issue at winget-pkgs requesting a particular installer for DSC. Personally I would prefer the packaged MSIX version to be available via WinGet for convenience. Users can install it once & future updates can be handled automatically via the MSFT Store (or manually by WinGet as well). But I don't see why we would want to support "only" MSIX since ZIP is always available in the release assets. No reason to exclude that installation medium when it's always available in the release assets. I feel the question should be should we support MSIX or no, and to that I feel we should. WinGet will always default to MSIX installation if user doesn't specify, but having all installation mediums available gives users more choices. |
That makes sense. In that case, we should support both MSIX and ZIP |
PR Summary
This PR adds a function to the build script to upload the assets related to Windows machines to the winget-pkg repository.
PR Context
The function grabs the relevant content types for Windows machines. One switch is introduced to submit the manifest, including a parameter to add the GitHub token.
The only two things remaining is to have it added to the workflow after a release is created and not rebuild the enter project. I was thinking something of this:
Note
The
OTBS
setting formatted some code in the build.ps1 file.