inkdevhub / drink

De-chained Ready-to-play ink! playground
Apache License 2.0
69 stars 15 forks source link

Build features #104

Closed pmikolajczyk41 closed 7 months ago

pmikolajczyk41 commented 8 months ago

Problem

When specifying a contract dependency with some required features (apart from ink-as-dependency), e.g.:

psp22 = { git = "https://github.com/Cardinal-Cryptography/PSP22.git", default-features = false, features = ["contract", "ink-as-dependency"] }

using either #[drink::contract_bundle_provider] or #[drink::test] used to end up with some linking error, because actually such psp22 would be built without the contract feature.

Solution in this PR

After ~2h of playing with the output from cargo metadata and dependency resolution, I went for the easiest, yet not so accurate approach:

  1. the main contract (the one that is located in the current crate) is still built without any features on
  2. for every contract dependency (i.e. a package with ink-as-dependecy feature present) I take all the features that were turned on on any resolution path. This means that even if some other dependency would depend on such PSP22 with a feature not-related-to-contracts(while our main contract wouldn't), the not-related-to-contracts feature is included; the only exception is the std feature, which is always included on a certain dependency resolution path.

Of course, there are cases where this approach is not correct, but I really don't see any point in wasting more time now, at least somebody actually will have the problem.

Ad 1.: I think that it is now not possible to fetch the --features argument passed to cargo in a procedural macro. One can inspect all present CARGO_FEATURE_<name> env variables (https://doc.rust-lang.org/cargo/reference/environment-variables.html), but then recreating original name (modulo underscores, dashes, capitalization) is hehe.

kroist commented 8 months ago

Code seems good, but this has to be explicit:

It is stated in documentation, that root package features are not being checked. So I think it would be great to state somewhere (better README.md), that if user wants to test the contract which has features, it should be done in external package

pmikolajczyk41 commented 8 months ago

it is stated in the macro docs