Skip to content

print zip file hash during start#141

Merged
barraguda merged 2 commits intomasterfrom
bp/hashprint
May 8, 2024
Merged

print zip file hash during start#141
barraguda merged 2 commits intomasterfrom
bp/hashprint

Conversation

@barraguda
Copy link
Copy Markdown
Contributor

Problem

for app store publishing, it's a slight hassle to find the package zip hash from inside of the vfs of the node you're trying to serve it from.

Solution

print it upon start (perhaps write it into metadata.json automatically too? (this seems like too much to me, but perhaps gate behind a flag)

Docs Update

Corresponding docs PR

Notes

kit s
simtester:template.os
package zip hash: be8a31d6fc1fcea9395c10ecf3486fdde3c86ee61a8ab7f15211717d555fb474
Successfully installed package simtester:template.os on node at http://localhost:8080

@barraguda barraguda requested a review from nick1udwig April 30, 2024 17:34
Copy link
Copy Markdown
Member

@nick1udwig nick1udwig left a comment

Choose a reason for hiding this comment

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

A couple changes. Aside from that, could you give me some more context for the problem we're trying to solve here? Is this for app store publishing? If so, why would we not write the hash to the metadata.json? That seems like a strictly better devex to me, especially considering we should be able to grab the current version number from Cargo.toml and then either overwrite or add to the metadata.json code_hashes field.

Comment thread src/start_package/mod.rs Outdated
Comment thread src/start_package/mod.rs Outdated
@barraguda
Copy link
Copy Markdown
Contributor Author

A couple changes. Aside from that, could you give me some more context for the problem we're trying to solve here? Is this for app store publishing? If so, why would we not write the hash to the metadata.json? That seems like a strictly better devex to me, especially considering we should be able to grab the current version number from Cargo.toml and then either overwrite or add to the metadata.json code_hashes field.

Yes for app_store publishing ergonomics. Grabbing the current version number from Cargo.toml assumes that such a file exists in the package root (it might not) + that its version number is kept updated by the developer. I think it makes more sense as a light print for correctness/a helper for developers, rather than explicitly modifying files within the same package

Copy link
Copy Markdown
Member

@nick1udwig nick1udwig left a comment

Choose a reason for hiding this comment

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

I agree with the first point: I wasn't thinking about non-Rust langs.

Re: the second point (I guess the first as well), it becomes a question of what the source of truth is for version number. The dev must keep the version number correct SOMEWHERE, and if we can programatically find it we should take advantage of it. The correct strategy is not "dev types it in to metadata.json by hand and then pastes in hash".

Am I understanding right that the flow with this PR is:

  1. kit build
  2. Copy hash out of terminal
  3. Paste hash into metadata.json
  4. Do whatever FE publishing

?

If so, there is no reason kit should not be able to do the copy-pasting.

I'm fine to put this off for another PR, though. Assuming I am correct about the above steps, please open an issue and then merge when ready. Otherwise lets continue the discussion in discord/tg.

Thanks for the PR!

@dr-frmr
Copy link
Copy Markdown
Contributor

dr-frmr commented May 6, 2024

@bitful-pannul please MAKE SURE that the printed hash is CORRECT, meaning, usable for a successful download!!!!

@barraguda
Copy link
Copy Markdown
Contributor Author

@bitful-pannul please MAKE SURE that the printed hash is CORRECT, meaning, usable for a successful download!!!!

check!

@barraguda barraguda merged commit 9d916dc into master May 8, 2024
@barraguda barraguda deleted the bp/hashprint branch May 8, 2024 20:37
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.

3 participants