7

Implement "artifact dependencies" (RFC-3028) by Byron · Pull Request #...

 2 years ago
source link: https://github.com/rust-lang/cargo/pull/9992
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Copy link

Contributor

Byron commented on Oct 21, 2021

edited

Tracking issue: #9096

Tasks

  • -Z unstable option called bindeps
  • white_check_mark(config) allow parsing of newly introduced 'artifact' fields
    • into TomlManifest
    • into Manifest
      • abort warn if artifacts are used on stable
  • white_check_markresolver : assure artifact dependencies are part of the resolution process and unified into the dependency tree
  • microscopecompiler: make it understand 'artifact' dependencies and pass new environment variables to the crate being build
    • lib=false should not be considered a rust library for the dependent, in unit and possibly resolve graph
    • assure profile settings are applied correctly
    • target overrides work
      • target = "target" in build deps
      • other targets on build deps
      • other targets on non-build deps
      • 'no-cross doc tests' seems like a constraint we should apply as well maybe
    • more confidence with resolver = "2"
    • assure artifact placement is correct (bin and various forms of lib)
  • white_check_markserialization: rewriting manifests (i.e. for publishing) does not discard artifact information
    • publishing keeps artifact and lib values
  • Other cargo subcommands
    • cargo metadata
      • leave unchanged
    • artifacts work with cargo check
    • artifacts work with rustdoc, such that it doesn't document them unless lib=true
    • cargo tree maybe?
    • cargo clean should clean artifacts - even though it's more complex, ultimately it deletes the target directory.
    • artifacts work with cargo test (and dev-dependencies)
      • doctests
      • try reproducible repository as well.
  • test_tubetests for more subtle RFC constraints
    • build scripts cannot access artifact environment variables at compile time, only at runtime)
    • assure 'examples' which also support crate-type fields like [[lib]] won't become artifacts themselves.
    • assure --out-dir does not leak artifacts - tested manually, it seemed to niche to add a test.
    • try target="foo" in artifact and assure it sees a decent error message
    • Assure RFC 3176 doesn't work
  • broomcleanup and finalization
    • assure no TODO(ST) markers are left in code
    • assure no tests are ignored
    • use resolver = "1" once to assert everything also works with the previous resolver, but leave it on "2".

Implementation and review notes

  • artifacts and unstable options are only checked when transforming them from TomlManifest to Manifest, discarding artifact information if the unstable flag is not set. Nowhere else in code will the CLI options be checked again.
  • If no binaries are specified, all the binaries in the package will be built and made available. - this should only refer to [[bin]] targets, not examples for instance.
  • artifact binaries won't be uplifted, hence won't be present outside of their output directory
  • exclamationWe don't know how package links will affect artifacts for build dependencies. Should probably be thought through.
  • exclamationThe location of artifacts is only tested roughly to avoid having to deal with different output names on the four platforms that seem to matter (gnu, macos, windows msvc, windows gnu).
  • cargo tree doesn't handle artifacts specifically, and it might be interesting to make clear if an artifact is only an artifact, or both artifact and dependency.
  • Most error and warning messages can probably be more cargo-matic.

Questions

  • Does cargo without the feature enabled have to complain about the "artifact" field in a dependency, like it does right now? It doesn't look like machinery for that exists in do_read_manifest().
    • heavy_check_markIt warns now
  • Should parsing of artifact values, like "bin" be case sensitive?
    • heavy_check_mark It's case sensitive now, which should help with serde roundtripping.

Review Progress

  • address Josh's review notes one by one
    • introduce IsArtifact (see this answer) (76cd48a)
    • prefer uplifting artifact deps that were written into the deps directory, but prefer to do that in this PR instead
    • add extra-tests as described in Josh's comment: " features get unified between a Rust library and a binary, and one that confirms features don't get unified between a Rust library and a binary for a different target?"
    • Make target-based artifact splitting work by porting parts of RFC-3176
      • test-support/cross-compile
      • namespace separation
  • re-create RFC-3176 what's not in RFC-3028, namely multidep support and all related tests
  • Address Eh2406 's review comments
  • Address Eric's comments
    • Unstable features need to be documented in unstable.md.
    • sort out target_data
    • figure out cargo metadata
    • See if target-data can work without an index format update.
    • truncate comments at 80-90 lines and remove unused methods, remove -Z unstable-options, use cfg!(target_env = "msvc")
    • add missing doc comments to newly added methods and funtions
    • simplify method parameters and inline some functions
    • add test and extend test to show additional issues
  • assure current set of tests works consistently, also on windows

Sponsored by Profian


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK