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

cargo-metadata always resolves features at the workspace level #7754

Open
sfackler opened this issue Dec 29, 2019 · 12 comments
Open

cargo-metadata always resolves features at the workspace level #7754

sfackler opened this issue Dec 29, 2019 · 12 comments
Labels
A-features Area: features — conditional compilation C-bug Category: bug Command-metadata S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@sfackler
Copy link
Member

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 (19a0de2 2019-12-12)

@emakryo
Copy link

emakryo commented Feb 9, 2022

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

@sffc
Copy link

sffc commented Jun 1, 2022

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.

@JarredAllen
Copy link

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.

@epage
Copy link
Contributor

epage commented Sep 25, 2023

There are two fundamental problems here

@JarredAllen
Copy link

JarredAllen commented Sep 25, 2023

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.

@epage
Copy link
Contributor

epage commented Sep 25, 2023

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.

@JarredAllen
Copy link

...

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).

@epage
Copy link
Contributor

epage commented Sep 26, 2023

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 epage added the S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. label Oct 16, 2023
@wllenyj
Copy link

wllenyj commented Apr 29, 2024

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.

@epage
Copy link
Contributor

epage commented Apr 29, 2024

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.

@wllenyj
Copy link

wllenyj commented Apr 30, 2024

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.

@epage
Copy link
Contributor

epage commented Apr 30, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features Area: features — conditional compilation C-bug Category: bug Command-metadata S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

7 participants