Skip to content
This repository has been archived by the owner on Jun 5, 2020. It is now read-only.

Fixes #18 Add option to show whole feature space of dependency tree #59

Closed
wants to merge 1 commit into from

Conversation

psinghal20
Copy link

@psinghal20 psinghal20 commented Mar 5, 2019

Fixes: #18

  • Add enabled_features and feature_space fields to the Node struct
  • Add show-feature-space argument

The output currently looks like this when using the show-feature-space argument

2019-03-05t18 18 36 05 30

I tried converting the feature_space(which is currently a BTreeMap) to Vector collecting only the keys but was unable to do so due to the cargo::core::interning module being private. The workaround I did for this was to iterate over the map and printing the required keys.

@psinghal20 psinghal20 changed the title feat: add option to show whole feature space of dependency tree Fixes #18 Add option to show whole feature space of dependency tree Mar 5, 2019
@brson
Copy link

brson commented Mar 8, 2019

Thanks for coding this up @psinghal20.

@sfackler will have to do a real review, but I see a few things here that might be better.

If I think about what I want out of a tool that analyzes the feature usage in a project, I think I want to know the following:

  • Which of a crate's features are turned on?
  • Why a feature is turned on? - then one has a good clue how to turn it off
  • Which of a crate's features are turned off?

This output makes it easy to answer the first question. It makes it possible to answer the third, but it would be easier probably if the output was divided into enabled features and disabled features.

I think the output in the screenshot has some problems. The featuretest project is here. And at least in the configuration in that link, project 'a' has all of default, foo and bar features turned on because not all of its reverse-dependencies turned off default features. foo is though the only feature in that workspace that is enabled explicitly.

Secondly, it's not clear what 'feature space' represents. It's collected on a per-crate basis, but it contains the same values for every crate listed, so it seems more like the set of all features.

Let's keep in mind a few other things too. First, every crate has a feature named 'default'. Second, crates may declare the features as the same name as other crates. Third, multiple crates in a project may have the name (but very probably different versions).

So to uniquely identify a feature one needs to name the crate it came from, the feature name, and also the crate version. We already have a syntax for crate + feature: "a/bar". There is no syntax for crate + feature + version though.


Ok, so all that said, some off the cuff brainstorming about what I'd want to see to answer the three questions above might be something like:

Enabled features
------------------

mycrate/default
mycrate/foo
dep1/default (mycrate)
dep1/bar (mycrate)
dep2/baz (mycrate)
dep3-1.0.0/qux (mycrate, dep1)
dep3-1.1.0/quazam (dep2)

Disabled features
-------------------

mycrate/z
dep2/default
dep2/y
dep3-1.0.0/default
dep3-1.0.0/x
dep3-1.1.0/default

Yeah this is completely different output from cargo-tree's normal output (maybe this belongs in a different tool), but it conveys a lot of useful info:

  • All the enable features of every crate, including 'default'
  • Which crates enabled each feature, in parens
  • All the disabled features of every crate, including 'default'
  • Distinguishes between multiple versions of the same crate

Some e.g. of what we could learn from this: In a dag with 10 crates depending on crate 'time', and 9 of them disabling default feature, which is the one crate that isn't disabling the default feature?

That's the sum of my thoughts right now. Maybe @sfackler will know why the data doesn't look quite right and where to get the correct data.

@psinghal20
Copy link
Author

@brson Sorry for the confusion! I made some edits to the config files of featuretest while testing on my local and the screenshot attached with the PR was of that edited configuration.

The list of features in case of original featuretest configuration seems to be correct ie. all the features of the crate a were enabled.

For the output, I thought the features coming on the same line as the dependency would make it clear to which crate they belonged to. I was not exactly sure of what type of output we wanted and went with the one which was simplest to implement.

The output you specified looks accurate for what we want to achieve but in my opinion, it shouldn't belong in cargo-tree. Maybe we can add it to a cargo-tree with a special argument if @sfackler is okay with it. If not, we can start working on a separate tool reusing some of the cargo-tree's for our specific scenario.

@dabreegster
Copy link

This has been useful in trying to find transitive deps with different enabled features, which is causing some common crates to be recompiled between two binaries in the same workspace. https://old.reddit.com/r/rust/comments/cqceu4/common_crates_in_cargo_workspace_recompiled/ewzlxmo/ has some commands using this PR.

One request to make the diff a bit easier to read: Print the enabled features in a deterministic order. Could you please change enabled_features to a BTreeSet? Then 'enabled_features: resolve.features(dep_id).into_iter().cloned().collect()'

@psinghal20
Copy link
Author

psinghal20 commented Aug 16, 2019

Hey @dabreegster, as the output for the feature analysis is quite different from what cargo tree provides, I thought it would be better to separate it out in a different crate. In the separate crate, I updated the maps to BTreeMap to provide deterministic order. You can find the crate here.

@sfackler Closing this PR as I have separated the code in different crate. If you feel we can somehow integrate it with cargo-tree, I would happy to work on it.

@psinghal20 psinghal20 closed this Aug 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: print features used to build each crate in tree
3 participants