Fix overpowered default-features = false specification in manifest mode

[ras0219-msft]

Currently, there is a bug in manifest mode where if the top level manifest marks a package as "default-features": false, all downstream dependencies upon defaults on that package will be ignored. For example,

toplevel -> A, B[core]
A -> B (including defaults)

Expected result: A, B[1,2,3,...]
Current result: A, B[core]

This PR fixes that issue to achieve the expected result.

[autoantwort]

I liked the other default (you don't depend on default features by default) more :)

[ras0219-msft]

This didn't do that -- this was just straight-up a bug. You still depended on default-features if you didn't explicitly disable them, but if you did remove the dependency from top-level it would delete the dependency down your entire transitive tree (even for packages that do depend on them).

I am aware of your preference here that's shared by many users and I intend to propose a solution soon.

[autoantwort]

What I mean:

was the old behavior and now it installs the following:

➜  vcpkg git:(master) ✗ vcpkg install  --triplet=x64-osx --host-triplet=x64-osx --enforce-port-checks 
Detecting compiler hash for triplet x64-osx...
The following packages will be built and installed:
    test-1[core]:x64-osx -> 1.0.0
    test-2[core,test-feature]:x64-osx -> 1.0.0

[BillyONeal]

What I mean: [image] was the old behavior and now it installs the following:

➜  vcpkg git:(master) ✗ vcpkg install  --triplet=x64-osx --host-triplet=x64-osx --enforce-port-checks 
Detecting compiler hash for triplet x64-osx...
The following packages will be built and installed:
    test[core]:x64-osx -> 1.0.0
    test1[core,foo]:x64-osx -> 1.0.0

test-1 depends on test-2[core,test-feature] in your screenshot though. A command line flag changing the interpretation of your vcpkg.json makes sense, but interpretation of dependencies shouldn't care.

[autoantwort]

test-1 depends on test-2[core,test-feature] in your screenshot though.

Previously not. Previously it only depended on core. That was what I meant by I liked the other default (you don't depend on default features by default) more. I know that it was considered a bug/wrong behavior by you.

PS: Sorry for the wrong new example output, I corrected it now

[BillyONeal]

test-1 depends on test-2[core,test-feature] in your screenshot though.

Previously not. Previously it only depended on core.

That's what vcpkg returned before, but that's not what that manifest says. A dependency without default-features: false means it depends on the default features. Returning otherwise would be like saying "if you pass this switch we will make 2+2 == 5".

[autoantwort]

but that's not what that manifest says

That depends how you interpret the manifest file :)

A dependency without default-features: false means it depends on the default features

Imho that's not cleanly specified in the specification. If the vcpkg-tool was the specification it was the other way around and until a month ago I thought it was the intended behavior. But I do not really want to argue against you. I only said that I liked the previous behavior more.

And I don't see why changing the default from "default-features:" true to "default-features:" false would mean 2+2 == 5. It was the default since manifest mode exists and I don't have the feeling that the manifest mode was completely broken because of that...

[BillyONeal]

And I don't see why changing the default from "default-features:" true to "default-features:" false would mean 2+2 == 5. It was the default since manifest mode exists and I don't have the feeling that the manifest mode was completely broken because of that...

The dependencies here don't know anything about manifest mode. If you want to change something about the outermost manifest under the person's control throwing the extra switch, that's one thing, but that's not what's going on here.

[ras0219-msft]

Let me clarify: a simple string in a dependency list has always meant "with defaults" (that's why there's a "default-features": false option). The bug was that we did not correctly handle this. An equivalent bug (setting aside defaults) would be:

test1 -> test2[core,best-feature]
A -> test1[core], test2[core]

And then we actually install test1[core] + test2[core]. This was obviously wrong.

I am working on a feature that adds two things:

1. Sets the default for "default-features" in the current manifest to false, meaning a simple dependency list of strings will not imply dependency upon default features.
2. If used at top level, disables automatic addition of defaults for transitive unnamed dependencies.

With this feature (let's call it "depend-defaults" since that's my wip name), you would need to add "depend-defaults": false to your top level json as well as test1/vcpkg.json. You could optionally simplify the top-level dependency list to just [ "test1", "test2" ]. You would then get the desired behavior on install of not having test-feature.

See #295.