cargo icon indicating copy to clipboard operation
cargo copied to clipboard

cargo-metadata always resolves features at the workspace level

Open sfackler opened this issue 6 years ago • 12 comments

Problem cargo-metadata appears to always resolve features at the "workspace" level, rather than for root crate. This means that cargo metadata will report features as enabled that aren't actually when cargo build is run in the same directory.

Steps

  1. Clone https://github.com/sfackler/rust-postgres
  2. Run cargo metadata --format-version 1 --manifest-path postgres-derive/Cargo.toml | jq '.resolve.nodes|.[]|select(.id|test("postgres-types"))'.
  3. Note that the entry for postgres-types has the derive and postgres-derive features enabled even though they are off by default for that crate. The postgres-derive-test crate in the same workspace enables that feature, which I assume is where that's coming from:
{
  "id": "postgres-types 0.1.0 (path+file:///home/sfackler/code/rust-postgres/postgres-types)",
  "dependencies": [
    "bytes 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
    "fallible-iterator 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
    "postgres-derive 0.4.0 (path+file:///home/sfackler/code/rust-postgres/postgres-derive)",
    "postgres-protocol 0.5.0 (path+file:///home/sfackler/code/rust-postgres/postgres-protocol)"
  ],
  "deps": [
    {
      "name": "bytes",
      "pkg": "bytes 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)",
      "dep_kinds": [
        {
          "kind": null,
          "target": null
        },
        {
          "kind": null,
          "target": null
        }
      ]
    },
    {
      "name": "fallible_iterator",
      "pkg": "fallible-iterator 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
      "dep_kinds": [
        {
          "kind": null,
          "target": null
        },
        {
          "kind": null,
          "target": null
        }
      ]
    },
    {
      "name": "postgres_derive",
      "pkg": "postgres-derive 0.4.0 (path+file:///home/sfackler/code/rust-postgres/postgres-derive)",
      "dep_kinds": [
        {
          "kind": null,
          "target": null
        }
      ]
    },
    {
      "name": "postgres_protocol",
      "pkg": "postgres-protocol 0.5.0 (path+file:///home/sfackler/code/rust-postgres/postgres-protocol)",
      "dep_kinds": [
        {
          "kind": null,
          "target": null
        },
        {
          "kind": null,
          "target": null
        }
      ]
    }
  ],
  "features": [
    "derive",
    "postgres-derive"
  ]
}

Possible Solution(s) cargo metadata should respect the root crate when resolving the crate graph.

Notes

Output of cargo version: cargo 1.41.0-nightly (19a0de242 2019-12-12)

sfackler avatar Dec 29 '19 19:12 sfackler

Are there any workarounds for this? cargo tree seems to work as expected.

emakryo avatar Feb 09 '22 07:02 emakryo

A possible workaround for this issue is to create a dedicated package where you run cargo metadata, and then add that package to the workspace.exclude list in the top-level Cargo.toml file.

sffc avatar Jun 01 '22 00:06 sffc

For the solution, could we instead (or in addition? idk which) make a --package argument that lets you select one or more packages from a workspace to use to root the dependency tree? I'd like to be able to root the dependency tree at any of several packages in my workspace (it doesn't have a root crate), which it looks like the proposed solution here isn't powerful enough to enable.

JarredAllen avatar Sep 25 '23 20:09 JarredAllen

There are two fundamental problems here

  • Changing cargo metadatas behavior in a backwards compatible way
  • Deciding how to handle which set of features to show as multiple unique sets of features can be enabled for one build
    • See also https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/cargo.3A.20Best.20API.20to.20get.20resolved.20dependency.20tree

epage avatar Sep 25 '23 20:09 epage

There are two fundamental problems here

  • Changing cargo metadatas behavior in a backwards compatible way

It would be entirely backwards-compatible to add a new argument (cargo metadata currently doesn't support --package as an argument), so this seems like a complete non-issue with respect to my suggested solution? Unless I'm missing something

It sounds like this is a separate problem from this issue ticket, since we already can run into this problem with just build vs. normal dependencies being different in an existing workspace. Idk what the appropriate resolution is (I can't think of anything backwards-compatible with the current output), but it sounds like that can be worked on separately from my requested feature, as it will happen either way.

JarredAllen avatar Sep 25 '23 20:09 JarredAllen

It would be entirely backwards-compatible to add a new argument (cargo metadata currently doesn't support --package as an argument), so this seems like a complete non-issue with respect to my suggested solution? Unless I'm missing something

By adding a --package flag, cargo metadata is starting to look like the other commands and people will be confused if it doesn't behave like them.

It sounds like this is a separate problem from this issue ticket, since we already can run into this problem with just build vs. normal dependencies being different in an existing workspace. Idk what the appropriate resolution is (I can't think of anything backwards-compatible with the current output), but it sounds like that can be worked on separately from my requested feature, as it will happen either way.

While these both tackle different problems, I see it rare that someone will want one without the other. If we have to rev the message format, we should be doing it at once.

epage avatar Sep 25 '23 20:09 epage

...

By adding a --package flag, cargo metadata is starting to look like the other commands and people will be confused if it doesn't behave like them.

I'm unclear what you mean by "it doesn't behave like them"? All the other tools I can think of that take a --package flag do some processing on the selected package (and maybe its dependencies), to either write something to disk (build puts artifacts, doc builds documentation, fmt reformats the code, add and remove update dependencies in the Cargo.toml) or to provide some output in the terminal (check and clippy with lints, test with test results, run with the output of running the binary). My proposed addition is entirely in line with the latter group, so I don't understand where the confusion would come from.

...

While these both tackle different problems, I see it rare that someone will want one without the other. If we have to rev the message format, we should be doing it at once.

I don't think my proposal requires changing the message format? The output still looks the same, just providing the new flag adjusts the contents presented therein to only show some of the packages in a workspace, so I think it should be compatible with the existing output format (any existing tooling won't supply a --package argument, so its output is unchanged).

JarredAllen avatar Sep 26 '23 16:09 JarredAllen

I'm unclear what you mean by "it doesn't behave like them"? All the other tools I can think of that take a --package flag do some processing on the selected package (and maybe its dependencies),

What happens without --package?

The standard cargo behavior is to use the package for the discovered / specified manifest. When encountering a virtual workspace, the default members are selected (with "all" as a fallback).

There are exceptions

  • cargo update is a workspace-focused command that let's you edit packages anywhere in the dependency tree
  • cargo fmt which does its own thing and can't be brought in conformance for compatibility purposes.

I don't think my proposal requires changing the message format? The output still looks the same, just providing the new flag adjusts the contents presented therein to only show some of the packages in a workspace, so I think it should be compatible with the existing output format (any existing tooling won't supply a --package argument, so its output is unchanged).

One way we can workaround the CLI behavior is to rev the message format (one behavior for v1, another for v2).

epage avatar Sep 26 '23 17:09 epage

Any progress to this ? Currently we can use cargo +nightly build --unit-graph -Z unstable-options instead of. And it can also get the profile related information. But the cargo crate doesn't implement the Deserialize trait for related struct, you need to implement the related struct by yourself.

wllenyj avatar Apr 29 '24 07:04 wllenyj

In my personal opinion, based on the above information, changing cargo metadata to report information equivalent of a cargo check command is a dead end.

However, https://github.com/rust-lang/rfcs/pull/3553 is somewhat like a "unit graph" of what was built. If that works for your use case, then that might be a good alternative. If this is for planning purposes, then we need a different solution and would need more information on how that fits into things. This would likely best be done on a different issue that is more focused on your need than this as this has a fairly specific scope.

epage avatar Apr 29 '24 14:04 epage

In my personal opinion, based on the above information, changing cargo metadata to report information equivalent of a cargo check command is a dead end.

However, rust-lang/rfcs#3553 is somewhat like a "unit graph" of what was built. If that works for your use case, then that might be a good alternative. If this is for planning purposes, then we need a different solution and would need more information on how that fits into things. This would likely best be done on a different issue that is more focused on your need than this as this has a fairly specific scope.

Thanks for your comments. Now I mix the cargo metadata and unit graph, get platform information through cargo metadata, get compile information(features, profile) of platform specific through unit graph.

But the cargo crate doesn't implement the Deserialize trait for related struct, you need to implement the related struct by yourself.

I found the cargo_util_schemas crate that can be used to Serialize/Deserialize.

wllenyj avatar Apr 30 '24 02:04 wllenyj

And the name for cargo_util_schemas was intentionally generic. If there are low level serde types that can easily be moved over into it, we're open to it. Some types can take a bit more work, like the initial manifest work.

epage avatar Apr 30 '24 02:04 epage