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

build: include bundled actors tag somewhere #10185

Closed
arajasek opened this issue Feb 3, 2023 · 10 comments
Closed

build: include bundled actors tag somewhere #10185

arajasek opened this issue Feb 3, 2023 · 10 comments
Assignees
Labels
P2 P2: Should be resolved
Milestone

Comments

@arajasek
Copy link
Contributor

arajasek commented Feb 3, 2023

We're facing a lot of confusion not knowing what actors bundle release is being run. This information is technically deducable, since the builtin_actors_gen.go file contains the manifest CIDs, which can be cross-compared against the release artifacts in builtin-actors, but that's a lot.

We should modify pack.sh to include this information...somewhere (perhaps builtin_actors_gen.go). Note that this information will not (necessarily) be correct -- no part of the system will be verifying the tag on startup, but that's okay -- this is just for our own sanity.

@jennijuju jennijuju added the P2 P2: Should be resolved label Feb 4, 2023
@jennijuju jennijuju moved this to 🏗 In progress in Network v18 Feb 6, 2023
@snissn
Copy link
Contributor

snissn commented Feb 6, 2023

Hey @arajasek - can you provide some extra guidance here? I found the CID associated with the ManifestCid - and I find when I grep my repo it finds that CID in the lotus binary, but beyond that I'm not sure what next steps to take. I also tried to fetch the CID using ipfs but it wasn't downloading.

@arajasek
Copy link
Contributor Author

arajasek commented Feb 6, 2023

Yeah, so what I think we should do is incorporate the actual git tag of the bundle being used.

Lotus currently updates / adds bundles with the pack.sh script, which takes a git tag as input, and fetches the bundle artifact directly from GitHub. I think it'd be very useful if as part of this process, we recorded the tag somewhere in Lotus itself. This would serve as an easy reference to the Git commit you're running (for example if you wanted to examine the actors source code).

I'm not fussed about where we put this info. The pack.sh script invokes the generator in gen/bundle/bundle.go, which generates builtin_actors_gen.go. It might be possible to include the git tag as input to that generator. Or, if it's easier, just generate a different file with this info.

@jennijuju
Copy link
Member

jennijuju commented Feb 7, 2023

so the way we run the ./pack.sh is ./pack.sh <version> <bundle tag> - as the script is run, if we can just append the version + bundle tag into the builtin_actors_gen.go (first line is also okay).

@snissn
Copy link
Contributor

snissn commented Feb 7, 2023

Does it make sense to add the git tag to BuiltinActorsMetadata or would the git info be better as a comment in builtin_actors_gen.go?

@jennijuju
Copy link
Member

Does it make sense to add the git tag to BuiltinActorsMetadata or would the git info be better as a comment in builtin_actors_gen.go?

Either is fine imho!

@snissn
Copy link
Contributor

snissn commented Feb 7, 2023

Great! This PR adds the git tag to BuiltinActorsMetadata #10200

I'm trying to test the command but am hitting a snag
panic: bundle bundle '"mainnet.tar.zst"' doesn't start with a 'v'

how should the command be run? i'm guessing something other than ./pack.sh versionmikerstest dev/20230206-pre-rc.6

@jennijuju
Copy link
Member

Great! This PR adds the git tag to BuiltinActorsMetadata #10200

I'm trying to test the command but am hitting a snag panic: bundle bundle '"mainnet.tar.zst"' doesn't start with a 'v'

how should the command be run? i'm guessing something other than ./pack.sh versionmikerstest dev/20230206-pre-rc.6

The first param would be the actor version - I.e v9, v10

@snissn
Copy link
Contributor

snissn commented Feb 7, 2023

ok great! The PR looks good to me then, the build appears to succeed when i test it with ./pack.sh v9 dev/20230206-pre-rc.6

@arajasek
Copy link
Contributor Author

arajasek commented Feb 8, 2023

Sooooo, we have an issue with 10200 unfortunately -- the pack.sh script is a bit more flexible than we budgeted for. In particular, it can be used to update:

  • only a specific actors version (the v9 / v10 mentioned by @jennijuju above)
  • only a specific network (eg. mainnet / vs. calibrationnet), by providing optional network overrides (eg. pack.sh v9 dev/20230114-pre-rc.2 mainnet=v9.0.3 would set all v9 actors to dev/20230114-pre-rc.2, EXCEPT mainnet, which would stay at v9.0.3).

As currently implemented in 10200, we just set the git tag for all metadata to the single provided input, which is wrong.

We could:

  • feed more input into the gen/bundle.go method (perhaps a chain of ( ) tuples?)
  • change the output to not be in builtin_actors_gen, but instead somewhere else that we can more easily control directly from pack.sh

@snissn
Copy link
Contributor

snissn commented Feb 16, 2023

updated fix - network name overrides in bundle git tags should be supported now #10294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 P2: Should be resolved
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants