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

p4 update checksum - disable sha256 #170483

Merged
merged 2 commits into from
Apr 7, 2024
Merged

p4 update checksum - disable sha256 #170483

merged 2 commits into from
Apr 7, 2024

Conversation

akwan
Copy link
Contributor

@akwan akwan commented Apr 3, 2024

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:

Additionally, if adding a new cask:

  • Named the cask according to the token reference.
  • Checked the cask was not already refused.
  • Checked the cask is submitted to the correct repo.
  • brew audit --cask --new <cask> worked successfully.
  • HOMEBREW_NO_INSTALL_FROM_API=1 brew install --cask <cask> worked successfully.
  • brew uninstall --cask <cask> worked successfully.

Perforce has a tendency to update minor versions in place, with the patch version only discoverable by running the binary and/or checking the documented SHA256SUM URL for changes/mod dates.

In #168330 referencing #163174 , it was mentioned removing the checksum. Does that mean just changing to:

sha256 :no_check

If we don't want to remove sha256 sums in the meantime, this change'll suffice. But it will break again when they update.

@akwan
Copy link
Contributor Author

akwan commented Apr 3, 2024

Noticing that:

https://www.perforce.com/perforce/doc.current/user/relnotes.txt

is out of date, causing CI to fail.

It should be reflecting this: https://filehost.perforce.com/perforce/r23.2/doc/user/relnotes.txt

  • Perforce updated the contents of the first link.

@krehel krehel added upstream Issue which needs to be resolved by the upstream project. awaiting user reply Issue needs response from a user. labels Apr 3, 2024
@krehel
Copy link
Member

krehel commented Apr 3, 2024

Thanks @akwan for undertaking this. If there's no good solution we'll probably have to go to sha256 :no_check as this breaks much too often. Bad experience not just for us as maintainers, but everyone else as end users - which is not wanted.

@akwan
Copy link
Contributor Author

akwan commented Apr 3, 2024

Thanks @akwan for undertaking this. If there's no good solution we'll probably have to go to sha256 :no_check as this breaks much too often. Bad experience not just for us as maintainers, but everyone else as end users - which is not wanted.

Acknowledged. I'll update the PR as this matches the pomello example cask and remove the extraneous comments.

@akwan akwan changed the title p4 update checksum 2023.2,2563409 p4 update checksum - disable sha256 Apr 3, 2024
@krehel
Copy link
Member

krehel commented Apr 3, 2024

@akwan - I'm happy to wait for Perforce to come back as the best solution would be to keep the checksums and find a workable way to check for version information.

@akwan
Copy link
Contributor Author

akwan commented Apr 3, 2024

Is there a pattern where the checksums can be derived dynamically instead of being codified?

The checksums are published and updated in files like: https://filehost.perforce.com/perforce/r23.2/bin.macosx12arm64/SHA256SUMS

which looks like:

7cd5db10ac9fe43f2a392abcd216490d1c7f6fb58228642e3ac68d6f4609f687 *p4python-3.9-arm64-whl.zip
498facfc3a927041dc5fb7c0a8917a844fb2d5e4c1e74fc6b94c28b4e2b1a9b4 *p4python-3.10-arm64-whl.zip
7783dc17303baadc262762cd8d3caca8a893a039f44be68f97189b9ca795e578 *p4python-3.11-arm64-whl.zip
18ceb3931785f698ea8c36567e69baf4605a10d79241a5c65cf2ef06f488c89c *helix-core-server.tgz
a5cebdc904e0f1d38839f70007367a5d335b682e1930a1c154f0c9807e0dded7 *p4python-3.12-arm64-whl.zip
bc9729c5a838a270859fffc3d0fcd2f9da663c2fa04e77e0b52cfce5aba7bc07 *p4python-3.8-arm64-whl.zip
bd68a4c6510e3da463385161e547c52c2e52bc7f5c2924ca0449fe83c1244460 *p4api-openssl3.tgz
3c9fb98baa5498a5fda2ee4825c12feba3dd8fb7470ff3ad4629b2f4aa470a6d *p4p
c30f612f3f6d3ec98b23401710a7b8320fbf53e3b910a35c1e0e3e14f180c248 *p4api-openssl1.1.1.tgz
120fcd7e2756a5ef31e3ae0eb69f88ac5e451f73e36429376466cf3e86d1b33c *p4
97a9390759dc725a79c7ebbac53412b79a902d84770e5509a03950142601c7c3 *p4d
6085a35ce86aa6e2d38b5e3f8d2631a17fbf1152dca5250756797940efa3e2c8 *p4broker

That file and its contents can update at any time, just like the binary file in place. Would it make sense to reference the values in that, or would it make more sense for some sort of CI to periodically check that file, and auto-file a PR with the new checksums for merging in? Or is neither approach really common/valid?

Reached out to them to chime on the PR.

@SMillerDev
Copy link
Member

Is there a pattern where the checksums can be derived dynamically instead of being codified?

No, that would invalidate the trust in the checksum since you can just MITM the website then.

@krehel krehel merged commit 26d3fe6 into Homebrew:master Apr 7, 2024
10 checks passed
@krehel
Copy link
Member

krehel commented Apr 7, 2024

@akwan - I merged this to get the software unblocked. It would be great if we can get this sorted out with Perforce. Appreciate if you can keep us updated. Thank you!

github-actions bot pushed a commit to MPLew-is/homebrew-vscodium that referenced this pull request Apr 7, 2024
@JeffAnton
Copy link

JeffAnton commented Apr 9, 2024

There is an issue of trust here. Perforce does not want to distribute code with known problems once those problems are fixed. That's why the version is replaced. But how does brew know this change is something to trust.
Maybe keeping the old version for a while is ok along side the new version.
What should the trust chain be here? What constitutes a notification from Perforce that a new version is legit? Crypto signed?
I don't know what Brew wants to be satisfied. Again, Perforce does not want to distribute old buggy software but brew does not trust when changed files are actually really from Perforce. Point me at a document. How can both parties be happy?
BTW: It's on my profile, but I work for Perforce on the p4 system. I can make changes happen. I do want clarity on what is needed.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting user reply Issue needs response from a user. outdated upstream Issue which needs to be resolved by the upstream project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants