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

Improve Winget workflow #147

Merged
merged 2 commits into from
Aug 16, 2022
Merged

Improve Winget workflow #147

merged 2 commits into from
Aug 16, 2022

Conversation

sitiom
Copy link
Contributor

@sitiom sitiom commented Jun 16, 2022

Use the Winget Releaser action instead. This significantly simplifies the workflow. You also just only need the public_repo scope on your PAT.
image

If you would like to read about this action further, the documentation is here, and the source code is here.

If you want to see an example of a PR created using this action, see microsoft/winget-pkgs/pulls (Pull request has been automatically created using 🛫 WinGet Releaser).

@rcmaehl
Copy link
Owner

rcmaehl commented Jun 16, 2022

Tagging my existing winget CI contributors for their feedback.
@gnpaone and @Macleykun

@gnpaone
Copy link
Collaborator

gnpaone commented Jun 17, 2022

Um, there's no necessity to move from existing solution to new one as the existing one is already pretty straightforward. You mean "simplified" in the sense just reducing the words, but it increases the loop - instead of existing mser is released, GitHub action runs and package updates, now it's mser released, GitHub action runs, the package is ran by the bot and submits to winget, the same bot works in main repo doing the job - unnecessary increase of loop + first time contributor stuff for that new repo (The workflow stuff idea initiated by me [here] (microsoft/winget-pkgs#56758) few months ago.). In my opinion one simple working automation is good and existing stuff is already working hassle-free.

@sitiom
Copy link
Contributor Author

sitiom commented Jun 17, 2022

Can you clarify what you are trying to say? The new action does the same exact thing (find the new URLs, update the manifest correspondingly, and PR to Winget) as before, so there really no added "complexity" in the end. You can check the other repos using it so you could see it for yourself.

@gnpaone
Copy link
Collaborator

gnpaone commented Jun 17, 2022

Same thing, there's no complexity in existing solution too for making it simplified 😉. For more clarity, u can take some package that's using this method as an example, let's say "X" package, go to X package developer original repository, there GitHub action is ran with vedantmgoyal2009/vedantmgoyal2009/winget-pkgs-automation/[email protected], then it submits to vedantmgoyal2009 repo where the bot (it's using firebase over there)(u can check actions page in that repository) to verify the package and submits to winget-pkg repository where checks are ran again (this time azure is used) and moderator approves (the package to main repo is submitted by wingetbot that they created). Now mser submission is done via the GitHub action (first step as previous) then checks are ran in main winget-pkgs repo and moderator approves, direct step. U can definitely use above clues to link everything and their code is over there u can verify that.

@sitiom
Copy link
Contributor Author

sitiom commented Jun 17, 2022

GitHub action is ran with vedantmgoyal2009/vedantmgoyal2009/winget-pkgs-automation/[email protected], then it submits to vedantmgoyal2009 repo

Oh, no it doesn't. You directly PR to https://github.com/microsoft/winget-pkgs like before. Another prerequisite is that you need to have https://github.com/microsoft/winget-pkgs forked (which @rcmaehl already does, so I didn't mention). Example: microsoft/winget-pkgs#60863

@gnpaone
Copy link
Collaborator

gnpaone commented Jun 19, 2022

Example: microsoft/winget-pkgs#60863

No, u still didn't get it, why do we need Winget releaser to update stuff when @rcmaehl can do it. It's in the same example of what u shown, the package from developer goes using "github-actions" to "winget releaser" then "winget releaser" to "winget-pkgs" repo as a PR instead of the package from developer submits using "github-actions" to "winget-pkgs" repo as a PR, what's the point of extra step, not needed, and what's the "necessity" of changing from current working stuff to alternate stuff when it's doing the same thing - update the package, is the current way obsolete?

@sitiom
Copy link
Contributor Author

sitiom commented Jun 19, 2022

"github-actions" to "winget releaser" then "winget releaser" to "winget-pkgs" repo as a PR instead of the package from developer submits using "github-actions" to "winget-pkgs" repo as a PR

Winget Releaser is the GitHub Action, what extra step are you talking about?

Why do we need Winget releaser to update stuff when @rcmaehl can do it.

It works exactly like that, just like the old action: @rcmaehl's PAT is used to directly PR to winget-pkgs and it will be under @rcmaehl's name.

"necessity" of changing from current working stuff to alternate stuff when it's doing the same thing - update the package, is the current way obsolete?

Like I said, it simplifies the workflow. The extra steps of cloning the repo and getting the latest tag are removed, as Winget Releaser already does that.

@sitiom
Copy link
Contributor Author

sitiom commented Jun 19, 2022

Maybe @vedantmgoyal2009 can help clear things up; I'm sure he'd be happy to help.

@vedantmgoyal9
Copy link
Contributor

vedantmgoyal9 commented Jun 19, 2022

@sitiom I will suggest to update the uses: to vedantmgoyal2009/winget-releaser@latest. I've created a separate repository for the action.

Edit: thanks, you've already done when I was writing the comment.

I think @gnpaone hasn't been able to understand the things clearly. I use firebase for hosting the documentation website and it isn't used by action. You can see the new separate repo to get what the action actually uses.

@sitiom
Copy link
Contributor Author

sitiom commented Jun 19, 2022

@sitiom I will suggest to update the uses: to vedantmgoyal2009/winget-releaser@latest. I've created a separate repository for the action.

Already did ahead of time 🙂

@vedantmgoyal9
Copy link
Contributor

Azure checks will be ran everytime whether you, me, or anyone submits a pull request at winget-pkgs repo. They are ran by Microsoft to ensure the Installer URLs in the manifests are safe and it doesn't install any viruses/malware on user's computer.

@vedantmgoyal9
Copy link
Contributor

I also manage a automation to update packages which aren't on github by getting the version and Installer URL from the software's API. Sometimes a maintainer doesn't want to manage the winget releases by themselves but they want their software to be available on winget so they request to put their repository in my automation. Here, I use a github bot ID to make "commits" so that it can be distinguished if there's some issue, etc. You may still notice that "pull requests" are created using my github id since Microsoft doesn't want to add another bot when their own @msftbot and @wingetbot are fighting over different tasks.

Hence the bot is also not used by the action anyways.

@vedantmgoyal9
Copy link
Contributor

Again, the separate repository structure may help you understand things better. I know since I was keeping everything in same repository (monorepo, kind of), you have get confused between the things: automation, it's bot and the ACTION.

@gnpaone
Copy link
Collaborator

gnpaone commented Jun 19, 2022

I think @gnpaone hasn't been able to understand the things clearly. I use firebase for hosting the documentation website and it isn't used by action. You can see the new separate repo to get what the action actually uses.

Yah, I've seen that, buy why is it compulsory to change to this? It's kinda forcing to use this. Let people use the method they already been using na. That's why I asked in another discussion that is the current method obsolete. It's better for new winget users adopting winget releaser as they just started exploring, probably.

@vedantmgoyal9
Copy link
Contributor

Now that's @rcmaehl's decision since he's maintaining the project. I can't say anything about that. If he's fine with the current setup, he can stick with it. My action basically combines the work of getting the release tag, removing "v" from it and then the process of updating values in the manifests into one (3 -> 1 😉).

@gnpaone
Copy link
Collaborator

gnpaone commented Jun 19, 2022

I also manage a automation to update packages which aren't on github by getting the version and Installer URL from the software's API. Sometimes a maintainer doesn't want to manage the winget releases by themselves but they want their software to be available on winget so they request to put their repository in my automation. Here, I use a github bot ID to make "commits" so that it can be distinguished if there's some issue, etc. You may still notice that "pull requests" are created using my github id since Microsoft doesn't want to add another bot when their own @msftbot and @wingetbot are fighting over different tasks.

Hence the bot is also not used by the action anyways.

That's what, we want to manage it ourselves including package stuff like updating the package deleting the package etc etc to checking the logs of what's happening and knowing it to troubleshooting stuff here etc. This is the same reason we took chocolatey package managing from a random maintainer and using GitHub action. Chocolatey has similar stuff where a seperate repo can be used as chocolatey package repo and various packages can be pushed via that using id as you did. That's what we avoided.

@rcmaehl
Copy link
Owner

rcmaehl commented Jun 20, 2022

Geez. Give me a bit to read through all this

@vedantmgoyal9
Copy link
Contributor

Take your time.

@vedantmgoyal9
Copy link
Contributor

@rcmaehl any plans to merge?

@micwoj92

This comment was marked as off-topic.

@sitiom
Copy link
Contributor Author

sitiom commented Jul 24, 2022

@rcmaehl any plans to merge?

Bump

@vedantmgoyal9
Copy link
Contributor

@rcmaehl I guess you should try the improved workflow, and if you have any concerns, you can mention me or sitiom ;)

@rcmaehl rcmaehl self-assigned this Jul 26, 2022
@Macleykun
Copy link
Contributor

Sorry, somehow i wasn't notified about being mentioned here!
I suggest to switch over :) if it simplifies with the same ease then i believe it's worth doing so.

@vedantmgoyal9
Copy link
Contributor

Tagging my existing winget CI contributors for their feedback. @gnpaone and @Macleykun

@rcmaehl would you like to consider this now? One of the existing winget CI contributors has approved the PR.

@rcmaehl
Copy link
Owner

rcmaehl commented Aug 15, 2022

Tagging my existing winget CI contributors for their feedback. @gnpaone and @Macleykun

@rcmaehl would you like to consider this now? One of the existing winget CI contributors has approved the PR.

More than likely, working on trying to get #160 resolved at the moment.

Please let me know what I need to do to distribute only the x64 build with this new CI.

@vedantmgoyal9
Copy link
Contributor

@rcmaehl Well, there's nothing that needs to be modified distribute only x64 build.

@sitiom I would suggest incorporating the WinGet publishing step into the main CI. You can see main...vedantmgoyal2009:MSEdgeRedirect:patch-1.

@sitiom
Copy link
Contributor Author

sitiom commented Aug 16, 2022

@sitiom I would suggest incorporating the WinGet publishing step into the main CI. You can see main...vedantmgoyal2009:MSEdgeRedirect:patch-1.

Is that the recommended way now? The current method works and looks cleaner as it is in a separate workflow.

@gnpaone
Copy link
Collaborator

gnpaone commented Aug 16, 2022

Yah, it's good to try new stuff as long as it works similar to current workflow 🙂. I suggest maintaining seperate workflow as main workflow contains development versions and main workflow will be running many times as per requirement

@vedantmgoyal9
Copy link
Contributor

@rcmaehl please merge 👍🏻

@rcmaehl rcmaehl merged commit d72b307 into rcmaehl:main Aug 16, 2022
@sitiom sitiom deleted the patch-1 branch August 17, 2022 00:57
@rcmaehl
Copy link
Owner

rcmaehl commented Sep 27, 2022

@vedantmgoyal2009

Latest action failed with an error:

  D:\a\_actions\vedantmgoyal2009\winget-releaser\latest\dist\index.js:9765
        version || new RegExp(/(?<=v).*/g).exec(releaseInfo.tag_name)[0],
                                                                     ^
  
  TypeError: Cannot read properties of null (reading '0')
      at D:\a\_actions\vedantmgoyal2009\winget-releaser\latest\dist\index.js:9765:68
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

Repository owner deleted a comment from micwoj92 Sep 27, 2022
@sitiom
Copy link
Contributor Author

sitiom commented Sep 27, 2022

Possibly the regex (?<=v).* does not account for tags without the v prefix:
image

@vedantmgoyal2009

At the time this was merged, the default regex was [0-9.]+ though 🤔

@rcmaehl
Copy link
Owner

rcmaehl commented Sep 27, 2022

Possibly the regex (?<=v).* does not account for tags without the v prefix: image

@vedantmgoyal2009

At the time this was merged, the default regex was [0-9.]+ though 🤔

It's not using version pinning in the PR

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.

6 participants