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

Manifest reform #848

Merged
merged 25 commits into from
Mar 25, 2024
Merged

Manifest reform #848

merged 25 commits into from
Mar 25, 2024

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Mar 11, 2024

Works towards #843 by introducing top-level "systems" and "assets" maps, similar to artifacts, which are indexed by other items.

The biggest challenge of this is to rework the linkage subsystem to properly refer to binaries by id. It also adds inline checksum values to artifacts, with sha256 checksums being guaranteed on archives.

  • add new fields to manifest (dummied)
  • refactor build code for managing binaries
  • compute linkage bottom up
  • flow data to manifest
    • compute system ids ("{subcommand}:{artifacts_mode}:{targets.join(',')}"?)
    • compute asset ids (guid generated on the system that builds it?) (annoying for snapshot stability, might need safe mode...)
    • bubble linkage up so it can be stored in dist-manifest (or pass manifest down...?)
    • implement manifest merging for new fields
    • make artifacts[] reference assets[]
    • implement artifacts[] merging
    • add new artifact_uploads[] so it's okay for artifacts[] to contain old info
    • change ci.yml to use artifact_uploads[] instead of artifacts[]
  • stop using old linkage format
    • make homebrew dep code read linkage from new manifest location
    • make linkage checker read linkage from new manifest location
      • honestly just remove cargo_dist's Linkage types, only use cargo_dist_schema's?
  • make --artifacts=lies (build_fake) add fake linkage?

@Gankra Gankra marked this pull request as draft March 11, 2024 19:17
@Gankra Gankra force-pushed the manifest-reform branch 2 times, most recently from d7c2e00 to 0dadb5b Compare March 12, 2024 19:13
@Gankra
Copy link
Contributor Author

Gankra commented Mar 13, 2024

https://github.com/Gankra/cargodisttest/releases/tag/v0.6.4

has a dist-manifest.json with the new system (see: assets[] and systems[])

@Gankra
Copy link
Contributor Author

Gankra commented Mar 13, 2024

i've now added inline checksums to the manifest, and changed the homebrew code to use both that and New Linkage.

@Gankra
Copy link
Contributor Author

Gankra commented Mar 13, 2024

lmfao, real fake checksums are uhhhh a problem for RUIN_ME_WITH_INSTALLERS, might need to make a tweak here...

(these RUIN_ME installer tests fetch the tarballs using live github data, on the assumption that we can reproduce the URLs exactly, but, this breaks if we generate installers which also insist they know the checksum...)

image

@Gankra
Copy link
Contributor Author

Gankra commented Mar 14, 2024

nice, new checksum/brew stuff appears to have worked: https://github.com/Gankra/cargodisttest/releases/tag/v0.6.5

Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

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

In general I like the way this is going! I'm going to need to kick the tires a bit to get a feel for what the data looks like in practice, but I think the approach is valid! I'll do some testing on this soon and look at the resulting manifests.

cargo-dist/src/build/mod.rs Show resolved Hide resolved
cargo-dist/src/build/mod.rs Show resolved Hide resolved
cargo-dist/src/tasks.rs Show resolved Hide resolved
cargo-dist/src/build/fake.rs Outdated Show resolved Hide resolved
@Gankra Gankra marked this pull request as ready for review March 18, 2024 21:50
Gankra added 4 commits March 21, 2024 13:40
note that the linkage field remains on dist-manifest to avoid breaking old users
of cargo-dist-schema who do not think the linkage field is ever optional. this can
be migrated away over time.
@Gankra Gankra merged commit fadafe2 into main Mar 25, 2024
15 checks passed
@Gankra Gankra deleted the manifest-reform branch March 25, 2024 22:20
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