Skip to content

tkey-ssh-agent: update artifact url and checksum#162731

Merged
BrewTestBot merged 2 commits intoHomebrew:masterfrom
chenrui333:tkey-ssh-agent-checksum
Mar 3, 2024
Merged

tkey-ssh-agent: update artifact url and checksum#162731
BrewTestBot merged 2 commits intoHomebrew:masterfrom
chenrui333:tkey-ssh-agent-checksum

Conversation

@chenrui333
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

found in #162713

@chenrui333 chenrui333 added upstream issue An upstream issue report is needed checksum mismatch SHA-256 doesn't match the download CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. labels Feb 14, 2024
@github-actions github-actions bot added the go Go use is a significant feature of the PR or issue label Feb 14, 2024
@chenrui333 chenrui333 force-pushed the tkey-ssh-agent-checksum branch from a60cc79 to 36528e4 Compare February 14, 2024 22:45
@chenrui333 chenrui333 changed the title tkey-ssh-agent: update checksum tkey-ssh-agent: update artifact url and checksum Feb 14, 2024
@stefanb
Copy link
Member

stefanb commented Feb 15, 2024

This should also change dependancy from "go" to "[email protected]" once

is merged

@dehanj
Copy link

dehanj commented Feb 15, 2024

This should also change dependancy from "go" to "[email protected]" once

* [[email protected] 1.21.7 (new formula) #162713](https://github.com/Homebrew/homebrew-core/pull/162713)

is merged

If I'm not mistaken, I think it should be built with [email protected]. It relates to this issue, which is fixed upstreams but not yet released.
So to close this PR that is the easiest fix. We do intend to make the release fixing the build issue with go above 1.20, but that will happen in about 2 weeks from now.

@dehanj
Copy link

dehanj commented Feb 16, 2024

@chenrui333, can you update the build dependency from "go" to "[email protected]"? Would appreciate it.
Should fix the build issue so this PR can go through.

@bevanjkay
Copy link
Member

@dehanj we are waiting for confirmation from upstream that the changed checksum is legitimate.

@dehanj
Copy link

dehanj commented Feb 16, 2024

@bevanjkay I think it is legitimate. See my comment here.

@bevanjkay
Copy link
Member

Unfortunately what we "think" is generally not enough in these instances. We need official confirmation from upstream

@chenrui333
Copy link
Member Author

chenrui333 commented Feb 17, 2024

@chenrui333, can you update the build dependency from "go" to "[email protected]"? Would appreciate it.

we actually deprecated [email protected] recently, meaning we cannot use it as build dependencies anymore

@dehanj
Copy link

dehanj commented Feb 18, 2024

we actually deprecated [email protected] recently, meaning we cannot use it as build dependencies anymore

Ah, I see. Then this will have to wait until we release in about 2 weeks. It would obviously produce a new checksum for that release, but I will try to dig a bit more until then to see if we can get a better explanation for this change. The source in the tarball has not changed at least.

@chenrui333 chenrui333 added the ready to merge PR can be merged once CI is green label Mar 2, 2024
@chenrui333 chenrui333 requested a review from a team March 2, 2024 16:20
@chenrui333
Copy link
Member Author

@dehanj is the upstream maintainer and confirm the latest checksum, my idea is maybe we can upload the tarball into the github asset rather than use github's source archive tarball for the sake of the tarball stability.

@dehanj
Copy link

dehanj commented Mar 2, 2024

If uploading our own release tarball is the recommendation from Homebrew, that is a solution. Is that the general recommendation from Homebrew? I guess it has to be seen Github does not offer checksum stability on the tarballs (even if they should seldomly change).

Unfortunately, we still have the issue that our latest release only supports Go 1.20 and below, and it has been deprecated. The fix is merged upstream, but we have some other changes the we absolutely need to include in our next release, it is our priority, but It is not finished yet. Sorry for the wait.

@ZhongRuoyu
Copy link
Member

In this case I think the tarball checksum could be a result of the repository rename because the repository name is encoded in the tarball:

$ curl -Ls https://github.com/tillitis/tillitis-key1-apps/archive/refs/tags/v0.0.6.tar.gz | tar zt | head
tkey-ssh-agent-0.0.6/
tkey-ssh-agent-0.0.6/.clang-format
tkey-ssh-agent-0.0.6/.editorconfig
tkey-ssh-agent-0.0.6/.github/
tkey-ssh-agent-0.0.6/.github/workflows/
tkey-ssh-agent-0.0.6/.github/workflows/ci.yaml
tkey-ssh-agent-0.0.6/.gitignore
tkey-ssh-agent-0.0.6/.golangci.yml
tkey-ssh-agent-0.0.6/LICENSE
tkey-ssh-agent-0.0.6/Makefile

I guess it has to be seen Github does not offer checksum stability on the tarballs (even if they should seldomly change).

There was an incident last year changing the checksum of all Git archives. The change was reverted shortly after, but the recommendation from GitHub is that release assets are preferred over Git archives if checksum stability is a requirement.

However, most of the checksum mismatches we see are a result of re-tagging. So, if you don't go against Git's recommendation and re-tag your releases (or frequently rename the repository which I don't think you will :) ), then it should be fine.

Copy link
Member

@ZhongRuoyu ZhongRuoyu left a 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 this is ready to merge. It fails to build on macOS.

@ZhongRuoyu ZhongRuoyu removed the ready to merge PR can be merged once CI is green label Mar 3, 2024
Signed-off-by: Rui Chen <[email protected]>

tkey-ssh-agent: patch `go.bug.st/serial` to build with go1.22

Signed-off-by: Rui Chen <[email protected]>
@chenrui333 chenrui333 force-pushed the tkey-ssh-agent-checksum branch from 36528e4 to 406200e Compare March 3, 2024 02:43
@chenrui333 chenrui333 requested a review from ZhongRuoyu March 3, 2024 02:43
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2024

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Mar 3, 2024
@BrewTestBot BrewTestBot enabled auto-merge March 3, 2024 03:43
@BrewTestBot BrewTestBot added this pull request to the merge queue Mar 3, 2024
Merged via the queue into Homebrew:master with commit 7482229 Mar 3, 2024
@chenrui333 chenrui333 deleted the tkey-ssh-agent-checksum branch March 4, 2024 17:40
@dehanj
Copy link

dehanj commented Mar 11, 2024

In this case I think the tarball checksum could be a result of the repository rename because the repository name is encoded in the tarball:

Thanks, the renaming seems to be the root cause.

However, most of the checksum mismatches we see are a result of re-tagging. So, if you don't go against Git's recommendation and re-tag your releases (or frequently rename the repository which I don't think you will :) ), then it should be fine.

Makes sense! Then I think it should be fine.

@github-actions github-actions bot added the outdated PR was locked due to age label Apr 11, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

checksum mismatch SHA-256 doesn't match the download CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. go Go use is a significant feature of the PR or issue outdated PR was locked due to age upstream issue An upstream issue report is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants