search

Sigil-Ebook / Sigil

Sigil is a multi-platform EPUB ebook editor
GNU General Public License v3.0
5.96k stars 578 forks source link

Publish to WinGet #684

Closed vedantmgoyal9 closed 2 years ago

vedantmgoyal9 commented 2 years ago

This action automatically generates manifests for WinGet Community Repository (microsoft/winget-pkgs) and submits them.

Before merging this:

  1. Please add a GitHub token with public_repo scope as a repository secret and rename the secret name in the workflow.
  2. Fork microsoft/winget-pkgs under @Sigil-Ebook.
dougmassay commented 2 years ago

Hmmm... adding the workflow seems innocuous enough, but will we need to keep the forked winget-pkgs repo in our Sigil-Ebook org in sync with upstream, or will the automation take care of everything in that regard?

vedantmgoyal9 commented 2 years ago

the automation take care of everything in that regard

eli-schwartz commented 2 years ago

Does WinGet not have some method of scraping for new versions and creating updates on its own? It seems interesting that this PR apparently requires giving OAuth write access to the entire Sigil-Ebook org namespace to a third-party individual acting as a package maintainer for WinGet.

dougmassay commented 2 years ago

Why does the standard GITHUB_TOKEN secret not suffice? Does the secret/token need to be created for the Sigil repo or the winget-pkgs fork? If the latter, fine. If the former why?

Eli scooped me a bit while was typing this, but his concerns are mine.

eli-schwartz commented 2 years ago

GITHUB_TOKEN would grant write access to this repository to the externally-controlled uses action. A new token with the public_repo scope grants write access to all public repos for this org, including this one and the proposed Sigil-Ebook/winget-pkgs fork.

The objective is apparently to grant write access to the proposed Sigil-Ebook/winget-pkgs fork, so that this workflow can run as a Sigil-Ebook org member and push updates to Sigil-Ebook/winget-pkgs, then create a pull request to microsoft/winget-pkgs asking to merge those commits. I'm not sure whether Github allows you to create OAuth tokens which are limited to a different repo? Having access to this repo would just be a side effect, albeit a side effect that grants org access to whoever controls the source code of that workflow (vedantmgoyal2009/vedantmgoyal2009).

I also wonder why it's not feasible to:

dougmassay commented 2 years ago

have an external party regularly run a workflow that scrapes the releases API to see if this repo has published a new Github Release.

This is sort of what I thought was already going on. But there was a failure of some sort with that automation with the latest Sigil release.

I was hoping to just get that repaired rather than needing to become actively involved in the winget process.

vedantmgoyal9 commented 2 years ago

It seems interesting that this PR apparently requires giving OAuth write access to the entire Sigil-Ebook org namespace to a third-party individual acting as a package maintainer for WinGet

The workflow is triggered on the release event and gets the following information:

and updates the fields in the manifests. Then it is pushed to a new branch on the fork of winget-pkgs and a pull requests is created using the token you've supplied. I think one of the main benefits of open-source is that you can view the code that is being executed when you're running it. Source code of the action: https://github.com/vedantmgoyal2009/vedantmgoyal2009/blob/main/winget-pkgs-automation/releaser-action/action.yml.

Since the default GITHUB_TOKEN cannot create pull requests since it doesn't have required permissions, a pull request is created using a token which same as you would do on GitHub (compare across forks -> select branch -> create pull request -> fill pr title and metadata -> create pull request or create draft pr).

@dougmassay there are couple of developers who say that they want to manage releases of their application themselves, a action was created which can be used by the original vendor and not a thrid-party person manages the releases. the above is true for at least for the packages which are open-source on platforms like github or gitlab.

the non-github packages can be updated by automation which can periodically check the release APIs or something from where we can get new version info and update the manifests at winget.

eli-schwartz commented 2 years ago

The concern is not that no one knows what code is being run.

The concern is that the code being run could change, because by the very nature of GitHub workflows it fetches arbitrary contents at the time the workflow is run.

Anything could happen between now and then, unvetted people on the internet could go rogue, that repository could be hacked...

Open source is great, and being able to verify the code you run is great, but in order to verify the code you're running, you have to make sure you're always running the code you've vetted, e.g. by using securely checksummed or code-signed versions of the code. This isn't naturally something GitHub actions are set up for, AFAIK.

This is usually only a concern once you give that code access to sensitive security tokens. In such circumstances, it's generally advisable to stick to only actions that are:

This PR is essentially requesting that you be considered part of group 3, and that's not a decision to be made unaware, or lightly.

Now, maybe if GitHub had some way to verify the checksum of an action before you use it...

vedantmgoyal9 commented 2 years ago

Well, if that is your concern, I can modify the pull request to run a PowerShell script which will do the same thing which the action does. The goal here is to automate the publishing of manifests to winget and not that use a particular github action to do the same. I hope that will solve your concern and will come under group 1 (actions that are under your control).

Now, maybe if GitHub had some way to verify the checksum of an action before you use it...

Also, I think it is something that we can report at github/feedback so there is something we can know about it and there's not, we can ask to have something like verify the fetched action code in GitHub Actions.

dougmassay commented 2 years ago

The bottom line for me is that while I like the very latest version Sigil being available from winget, I'm just not very interested in being the one to make that happen.

I don't mind making a tweak here or there to help third-party packagers (be that for Linux or Windows) make Sigil available to their users, but this is the first time I've been asked to fork a repository and give special (and let's face it--a bit scary) permissions to modify that forked repository (and the potential to alter others) just to update the latest Sigil version number and asset urls in a manifest file. Sigil shows up in Chocolatey, Npackd and Arch Linux (and several others) without me needing to copy their repository and hand out keys to the organization.

I'm not raising hell, and I'm not mad, but this is starting to feel like a very long, and very winding lane that ends at a fairly unimpressive house. ;)

If a volunteer doesn't want to update Sigil in winget, perhaps I'll just want until winget offers a safer (and less convoluted) method for software devs to maintain/update manifests in the winget community packages repository themselves.

Thanks for your interest, your offer, and for whatever part you played in helping @OfficialEsco keep Sigil updated in winget-pkgs, but I think I'll pass on this PR for now.

vedantmgoyal9 commented 2 years ago

I think I'll pass on this PR for now

I'm fine both the ways.

vedantmgoyal9 commented 2 years ago

Should I convert it to raw code, instead of using action?

dougmassay commented 2 years ago

If you can set it up so that all code that needs public_repo permissions is running under my control instead of a third-party action's, I'd be happy to take a look.

We're talking about a github action that runs whenever I release, that runs a powershell script (in my repository) which creates a new branch in my winget-pkgs fork, creates a PR in that branch and then pushes the PR upstream, right?

My only concern is inheriting a script that I don't fully understand when something inevitably breaks in the future. But then the worst thing that happens in that situation is that Sigil doesn't automatically get updated on winget.

vedantmgoyal9 commented 2 years ago

We're talking about a github action that runs whenever I release, that runs a powershell script (in my repository) which creates a new branch in my winget-pkgs fork, creates a PR in that branch and then pushes the PR upstream, right?

Yes, I'll convert it into the powershell script once I get free from other stuff :)

dougmassay commented 2 years ago

Sounds good. Don't hurry on my account. ;)

vedantmgoyal9 commented 2 years ago

@dougmassay @eli-schwartz I have updated the pull request to run a PowerShell script.

However, you'll still need to fork microsoft/winget-pkgs under the account of which the GitHub PAT will be used. The token will still require public_repo permission but there shouldn't be any concerns now since all the code is running from your repo and not from any external source.

dougmassay commented 2 years ago

Whether or not we still had to fork the winget-pkgs repo was one of my questions. I was under the impression that wingetcreate might handle that. But that's no big deal either way.

I think I have my head around it all, though. The manifest templates are hosted here and are updated with a version number and asset download urls by the powershell script called by the winget.yml workflow. That workflow appears to need to be manually triggered (workflow_dispatch) by me (providing the latest version as input). Then the script downloads Microsoft's wingetcreate (from the official repo) which handles submitting the pull request to winget-pkgs.

That about it?

vedantmgoyal9 commented 2 years ago

That about it?

Yes 👍🏻

Since wingetcreate isn't mature enough and Microsoft's team currently has more focus over WinGet (the original Windows Package Manager), we have to host manifests' templates and update them using a script. In future, when wingetcreate will be more advanced, it would be as simple as one or two commands in GitHub Workflow.

In case you have a fully automated release pipeline in the future, it could be triggered automatically when the release is fully published (including all the binaries). What I've been able to guess by seeing the repository is that, currently, you manually create a draft release, upload binaries and then publish the release (most of the process is manually done), hence it would be best to have it run on workflow_dispatch event.

dougmassay commented 2 years ago

That is correct. With signed MacOS packages and tweaked Python and Qt builds, we just feel safer building/uploading/publishing manually for now. Our CI builds are mainly for testing commits, PRs and for occasionally deploying betas.

Sigil does have its own internal update notifications that are based on checking the version.xml file in the Sigil repo. We also have automation that updates that file (and publishes release posts to our blog) whenever releases are published, so we could conceivably launch the winget.yml workflow from that other workflow and grab the version from the version.xml file (rather than manual input) after it's updated.

But I'm fine with manually triggering the winget-pkgs update for now. In fact, the opportunity to wait for a day or two after a new release might come in handy.

Thanks for your time and effort! Give me a little more time to get some things ready and I'll merge this.

dougmassay commented 2 years ago

Quick question: does the winget-pkgs repo need to be forked to my personal account or to the Sigil-Ebook org. I know it was initially the Sigil-Ebook org, but didn't know if the refactoring changed any of that.

vedantmgoyal9 commented 2 years ago

Personal account.

dougmassay commented 2 years ago

I'm no PowerShell expert (by any stretch of the imagination), but I'm a little confused by the seemingly two different uses of $Version. One appears to be the sigil version passed in as a parameter (and used to replace the placeholder in the manifests), the other appears to be the path to PWD. $Version is also passed as a parameter to the wingetcreate submit command (path to manifests). Am I missing something?

dougmassay commented 2 years ago

@vedantmgoyal2009 This failed the first time I tried to run it after a new Sigil release. Any ideas?

Submitting pull request for manifest...

Octokit.NotFoundException: Not Found at Octokit.Connection.HandleErrors(IResponse response) at Octokit.Connection.RunRequest(IRequest request, CancellationToken cancellationToken) at Octokit.Connection.Run[T](IRequest request, CancellationToken cancellationToken) at Octokit.ApiConnection.Patch[T](Uri uri, Object data) at Microsoft.WingetCreateCore.Common.GitHub.UpdateForkedRepoWithUpstreamCommits(Repository forkedRepo) at Microsoft.WingetCreateCore.Common.GitHub.<>cDisplayClass19_0.<b1>d.MoveNext() --- End of stack trace from previous location --- at Polly.AsyncPolicy.<>cDisplayClass40_0.<b0>d.MoveNext() --- End of stack trace from previous location --- at Polly.Retry.AsyncRetryEngine.ImplementationAsync[TResult](Func3 action, Context context, CancellationToken cancellationToken, ExceptionPredicates shouldRetryExceptionPredicates, ResultPredicates1 shouldRetryResultPredicates, Func5 onRetryAsync, Int[32](https://github.com/Sigil-Ebook/Sigil/runs/8171583766?check_suite_focus=true#step:3:33) permittedRetryCount, IEnumerable1 sleepDurationsEnumerable, Func4 sleepDurationProvider, Boolean continueOnCapturedContext) at Polly.AsyncPolicy.ExecuteAsync(Func3 action, Context context, CancellationToken cancellationToken, Boolean continueOnCapturedContext) at Microsoft.WingetCreateCore.Common.GitHub.SubmitPRAsync(String packageId, String version, Dictionary2 contents, Boolean submitToFork) at Microsoft.WingetCreateCore.Common.GitHub.SubmitPRAsync(String packageId, String version, Dictionary2 contents, Boolean submitToFork) at Microsoft.WingetCreateCLI.Commands.BaseCommand.GitHubSubmitManifests(Manifests manifests) at Microsoft.WingetCreateCLI.Commands.SubmitCommand.SubmitManifest() at Microsoft.WingetCreateCLI.Commands.SubmitCommand.Execute() at Microsoft.WingetCreateCLI.Program.Main(String[] args) {"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/git#update-a-reference"}

Error: Process completed with exit code 1.

vedantmgoyal9 commented 2 years ago

An issue for this is already there on microsoft/winget-create repo: https://github.com/microsoft/winget-create/issues/287

Since we don't have access to GitHub Runners, the only thing you can do is try re-running the workflow.

dougmassay commented 2 years ago

Thanks for the tip. For all the more I'll be doing this ... deleting and recreating my fork of winget-pkgs was much quicker than cloning locally, pulling in upstream changes, and then pushing them back to my fork.

It seems to have gone OK. It's just waiting for review, apparently.

Sometime in the future (when we officially move to Qt6) we will be dropping support for 32-bit architectures. So we'll only be releasing one package (64-bit) for winget after that. I might need a nudge accomplishing that change.