First of all, this is the relevant cargo syntax.
To do so on the sev crate directly, the first example is correct.

However, the second example would not look like

cargo build --no-default-features --features snp,openssl?vendored

but
cargo build --no-default-features --features snp,openssl/vendored
This is because the syntax with the question mark only applies if openssl is included through other means, the syntax without the question mark includes the dependency as well.

Finally, in the event of including sev as a dependency of another crate, simply disabling openssl vendoring would look like

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["openssl"] }

However, enabling only snp and vendored openssl would look like

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl"] }
openssl = { version = "*", features = ["vendored"] }

This example relies upon Rust's feature unification, to add the vendored feature back into openssl in a transitive manner.

First of all, this is the relevant cargo syntax. To do so on the sev crate directly, the first example is correct.

However, the second example would not look like

cargo build --no-default-features --features snp,openssl?vendored

but cargo build --no-default-features --features snp,openssl/vendored This is because the syntax with the question mark only applies if openssl is included through other means, the syntax without the question mark includes the dependency as well.

Finally, in the event of including sev as a dependency of another crate, simply disabling openssl vendoring would look like

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["openssl"] }

However, enabling only snp and vendored openssl would look like

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl"] }
openssl = { version = "*", features = ["vendored"] }

This example relies upon Rust's feature unification, to add the vendored feature back into openssl in a transitive manner.

Perhaps I'm misunderstanding, but the text states:

You can add a ? as in "package-name?/feature-name" which will only enable the given feature if something else enables the optional dependency.

Otherwise it takes a union of the features of each crate. Since we didn't explicitly specify the openssl dependency in sev to be vendored, it won't be. The openssl build in sev would be both vendored and would include an OpenSSL build, no? So you're example to enable snp and vendored openssl would instead look like:

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl?/vendored"] }
openssl = { version = "*", features = ["vendored"] }

Right? You specify openssl?/vendored in the default features for the crate, but we explicitly turn off the default features in this example, so that's not what is used. You have to still explicitly specify the openssl dependency (and vendored feature of it).

You can add a ? as in "package-name?/feature-name" which will only enable the given feature if something else enables the optional dependency.

That is correct, but my understanding is that the syntax is only applicable within the "[features]" section of the Cargo.toml, as it pertains to (conditionally) activate a feature of a direct dependency

Thus you would not be able to use it within the dependencies section

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl?/vendored"] }
openssl = { version = "*", features = ["vendored"] }

as the crate "sev" does not declare a "openssl?/vendored" feature, and you would not be activating the "openssl" feature of the "sev" crate either.

The field "features" of a dependency may only reference features declared within the "[features]" section of the dependency itself. In our case, we may only use sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl"] }, which will enable the "openssl" feature of sev, thus include the optional "openssl" dependency of sev.

And to be able to enable the "vendored" feature of the "openssl" dependency of the "sev" dependency, we must rely upon the feature unification, by ourselves declaring an openssl dependency with the vendored feature, thus openssl = { version = "*", features = ["vendored"] }.

Hopefully this makes sense

The field "features" of a dependency may only reference features declared within the "[features]" section of the dependency itself. In our case, we may only use sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl"] }, which will enable the "openssl" feature of sev, thus include the optional "openssl" dependency of sev.

And to be able to enable the "vendored" feature of the "openssl" dependency of the "sev" dependency, we must rely upon the feature unification, by ourselves declaring an openssl dependency with the vendored feature, thus openssl = { version = "*", features = ["vendored"] }.

The feature unification doc specifies:

When a dependency is used by multiple packages, Cargo will use the union of all features enabled on that dependency when building it.

So to the previous example:

[dependencies]
sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl"] }
openssl = { version = "*", features = ["vendored"] }

Since the sev/openssl dependency is default (i.e. NOT vendored) and the openssl dependency is vendored, then the union of the two would be [default, vendored]. It's not as if because the openssl dependency is vendored, then the sev/openssl dependency is also vendored as well.

This is how I am understanding it.

Since the sev/openssl dependency is default

Am I misunderstanding something ?
Because I don't see the openssl dependency as default, as it's marked as optional, and the feature that enables it is not default either :

Anyway, to put those discussions to rest regarding the proper syntax on the dependency declaration, here are the cargo outputs using the sev = { version = "6.2.1", default-features = false, features = ["snp", "openssl?/vendored"] } syntax

error: failed to load manifest for workspace member `<snip>`
referenced by workspace at `/<snip>/Cargo.toml`

Caused by:
  failed to parse manifest at `/<snip>/Cargo.toml`

Caused by:
  feature `openssl?/vendored` in dependency `sev` is not allowed to contain slashes
  If you want to enable features of a transitive dependency, the direct dependency needs to re-export those features from the `[features]` table.
 

Since the sev/openssl dependency is default (i.e. NOT vendored) and the openssl dependency is vendored, then the union of the two would be [default, vendored]. It's not as if because the openssl dependency is vendored, then the sev/openssl dependency is also vendored as well.

This is how I am understanding it.

that is correct, the features of the openssl crate in use would be the union of the two, thus include the vendored as well

Since the sev/openssl dependency is default (i.e. NOT vendored) and the openssl dependency is vendored, then the union of the two would be [default, vendored]. It's not as if because the openssl dependency is vendored, then the sev/openssl dependency is also vendored as well.
This is how I am understanding it.

that is correct, the features of the openssl crate in use would be the union of the two, thus include the vendored as well

Right, that's not what we want. If we specify vendored then we explicitly do not want OpenSSL built for the rust crate, we want to instead rely on the OpenSSL already installed on the system.

Edit: I think I've misunderstood. You're saying that as long as vendored is specified, then the OpenSSL will not be built for the crate, correct?

@DGonzalezVillal PTAL.

Since the sev/openssl dependency is default (i.e. NOT vendored) and the openssl dependency is vendored, then the union of the two would be [default, vendored]. It's not as if because the openssl dependency is vendored, then the sev/openssl dependency is also vendored as well.
This is how I am understanding it.

that is correct, the features of the openssl crate in use would be the union of the two, thus include the vendored as well

Right, that's not what we want. If we specify vendored then we explicitly do not want OpenSSL built for the rust crate, we want to instead rely on the OpenSSL already installed on the system.

huh ?
That's the opposite behavior of the "vendored" attribute of the "openssl" crate https://github.com/sfackler/rust-openssl/blob/master/openssl-sys/Cargo.toml

The "vendored" feature of the openssl crate includes the openssl source code and builds it locally to link to the rust code; not using the "vendored" feature of openssl is what leads to using the dynamic shared object openssl library.

Was the intent behind this commit 7a80d99 to not build the openssl dependency for SEV and use the shared object instead ?

If so, it had the opposite effect, as can be seen in the git blame of the cargo.lock file, which includes the openssl-src crate starting from that commit : https://github.com/virtee/sev/blame/7a80d9982495ae941b36430fecaa7b7ad680e55f/Cargo.lock#L1077-L1085

Edit: I think I've misunderstood. You're saying that as long as vendored is specified, then the OpenSSL will not be built for the crate, correct?

the other way around, as long as "vendored" is specified, OpenSSL will be built for the crate, which I understood was the original intent behind vendoring OpenSSL in the first place with 7a80d99

That's where my confusion was, thanks for clearing it up. LGTM.

I've rebased the PR, could it be reviewed ?

@tylerfanelli we need your approval again to merge