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

feat: Support --target input in args #301

Merged
merged 16 commits into from
Oct 31, 2022
Merged

feat: Support --target input in args #301

merged 16 commits into from
Oct 31, 2022

Conversation

FabianLars
Copy link
Member

@FabianLars FabianLars commented Sep 19, 2022

This is really not a nice solution, but a better one would need a bigger rewrite which i don't have time for tbh. We should have a cargo metadata follow-up PR either way in case people commit their config.toml file.

Another problem is how it deletes existing release assets. The way it is now it will remove them per OS, not per arch, so you can't upload a .dmg file for example for both arches. The .app file obviously has a naming problem, so maybe we don't want to support this at all right now?

Successful run: https://github.com/FabianLars/mw-toolbox/actions/runs/3078695681
Now with macos asset renaming per arch and better asset overwrite handling: https://github.com/FabianLars/mw-toolbox/actions/runs/3333153297 - i wonder how happy github was with me uploading over 30 release assets 🤔

Release: https://user-images.githubusercontent.com/30730186/191006883-9a3e9887-e240-4067-8bf8-6733dbaf3c6a.png

New release:
grafik

p.s. i didn't plan to PR from this branch so the commit messages are weird, but squash merge to the rescue 🥳

resolves #252
resolves #243
resolves #176

@FabianLars FabianLars marked this pull request as draft October 18, 2022 10:49
@FabianLars FabianLars marked this pull request as ready for review October 27, 2022 22:22
@lucasfernog lucasfernog merged commit a99d0ba into tauri-apps:dev Oct 31, 2022
@robertkern
Copy link

Thanks so much for this change. I know it hasn't been tagged so shouldn't be being used at the moment, but i was testing it out with the commit hash as the action version and get the error assetPath is not defined which looks like it's coming from

data: fs.readFileSync(assetPath),
(other references were changed to asset.path) just a heads up @FabianLars

@FabianLars
Copy link
Member Author

FabianLars commented Nov 1, 2022

thanks, fixed it.
If it still doesn't work you can try the commit hash of the merge of this PR before the other pr got merged: a99d0ba (but please report any other issues before that :D )

Edit: it's not fixed because i forgot to rebuild the action. That has to wait for tomorrow unfortunately

@FabianLars FabianLars deleted the artifacts-path branch November 1, 2022 00:32
@Layendan
Copy link

Layendan commented Nov 3, 2022

I am still receiving the assetPath is not defined error
image
Summary of run: https://github.com/Layendan/NineAnimator-Tauri/actions/runs/3381798110

The only reference to assetPath that I can find that would break the action is this line

a.rest.repos.uploadReleaseAsset({headers:r,name:p,data:s().readFileSync(assetPath),owner:o.context.repo.owner,repo:o.context.repo.repo,release_id:t})

in /packages/action/dist/index.js which I do not know where that comes from.

@FabianLars
Copy link
Member Author

The last part of my last message is still the case. I didn't have access to my dev system today so I couldn't rebuild the action yet. Gonna do that first thing in the morning tho.

@lucasfernog
Copy link
Member

Pushed it @Layendan , thanks for the report and sorry for this issue, it was probably a wrong merge.

@Layendan
Copy link

Layendan commented Nov 3, 2022

No problem and thank you for the help!

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.

ability to specify a target Handle universal and aarch64 targets aarch64 support?
4 participants