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

ci: add sha256 checksums #103

Merged
merged 7 commits into from
Oct 6, 2023
Merged

Conversation

simonsan
Copy link
Contributor

I want to create a scoop install manifest, but the version inside the zipped folder is pretty uncommon, the usual way is $app_name-$target_platform with the zip, can we please rename it?

It would be also lovely, if we could add SHA256 hashes to the release, do you want me to add that as well in this PR?

I want to create a scoop install manifest, but the version inside the zipped folder is pretty uncommon, the usual way is `$app_name-$target_platform` with the zip, can we please rename it?
@simonsan
Copy link
Contributor Author

simonsan commented Sep 12, 2023

Current scoop manifest

{
    "version": "0.11.3",
    "description": "A CLI task runner defined by a simple markdown file.",
    "homepage": "https://github.com/jacobdeichert/mask",
    "license": "MIT",
    "architecture": {
        "64bit": {
            "url": "https://github.com/jacobdeichert/mask/releases/download/v0.11.3/mask-v0.11.3-x86_64-pc-windows-msvc.zip",
            "hash": "0d849c344fdcb64b0047ce1faf95f8c6cb9b4d282f1a6c78e9f30132d9bace55"
        }
    },
    "extract_dir": "mask-x86_64-pc-windows-msvc",
    "bin": "mask.exe",
    "checkver": {
        "github": "https://github.com/jacobdeichert/mask"
    },
    "autoupdate": {
        "architecture": {
            "64bit": {
                "url": "https://github.com/jacobdeichert/mask/releases/download/v$version/mask-v$version-x86_64-pc-windows-msvc.zip"
            }
        },
        "hash": {
            "url": "$url.sha256"
        }
    }
}

Comment on lines 159 to 163
- name: Attach the binary to the release
shell: bash
run: ./.github/actions/attach-release-assets/run.sh ./*.sha256
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if it is worth to have the hassle to change the whole bash script, so I just called it twice to upload the sha checksums as well? Is that ok?

@simonsan
Copy link
Contributor Author

Afaic assets in the upload-artifact isn't really taking more than one asset glob, because I didn't want to change the whole thing (I'm in the online editor), I was doing it this way.

@simonsan simonsan mentioned this pull request Sep 12, 2023
7 tasks
@simonsan simonsan changed the title ci: remove version tag from contained folder ci: remove version tag from contained folder, add sha256 checksums Sep 13, 2023
@jacobdeichert
Copy link
Owner

Hey, thanks for the PR. I don't have time to review right now, but will aim to catch up with this in a few weeks.

Regarding removing the version, I'm not sure yet, I'll have to consider it. Many other rust tools follow the same convention, including the version in the zip name. Here's another example https://github.com/sharkdp/fd/releases

@simonsan simonsan changed the title ci: remove version tag from contained folder, add sha256 checksums ci: add sha256 checksums Sep 16, 2023
@simonsan
Copy link
Contributor Author

Hey, thanks for the PR. I don't have time to review right now, but will aim to catch up with this in a few weeks.

Regarding removing the version, I'm not sure yet, I'll have to consider it. Many other rust tools follow the same convention, including the version in the zip name. Here's another example https://github.com/sharkdp/fd/releases

Found a way to circumvent that with the version name in the zip folder so I removed the code and just let the sha checksum stuff inside. 👍🏽

@simonsan simonsan mentioned this pull request Sep 16, 2023
1 task
@jacobdeichert
Copy link
Owner

Hey @simonsan, thanks for keeping this PR open with the checksums - this looks pretty good!

Before merging as is, I'm going to take a look at some other rust projects and see if it's common to upload checksums as a release asset or if it's more common to just add the checksums to the release notes/description area. If you happen to know of any other rust projects that do one or the other, let me know 👍

I'll get back to this within the next week hopefully. I plan on automating the mask release process this month and will be cutting a new release after that is done.

@simonsan
Copy link
Contributor Author

simonsan commented Oct 5, 2023

add the checksums to the release notes/description area.

That would be pretty bad, as that would actively work against automating checking checksums when downloading. Where you could just download both, checksums and the zipped binaries and use the checksums from the file. You would rather need to parse the checksum from the text description area.

Unfortunately, fewer projects upload their checksums at all, so I can't really show examples in this very moment. I would recommend uploading them as an extra asset, though, as having them in the description would be a pretty bad idea going against automation.

@jacobdeichert
Copy link
Owner

That would be pretty bad, as that would actively work against automating checking checksums when downloading. Where you could just download both, checksums and the zipped binaries and use the checksums from the file. You would rather need to parse the checksum from the text description area.

Oh, okay this makes sense for automation purposes 👍

@jacobdeichert jacobdeichert merged commit 72f578d into jacobdeichert:master Oct 6, 2023
7 checks passed
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.

2 participants