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

Convention: Negate "library" feature? #1627

Closed
webmaster128 opened this issue Mar 9, 2023 · 4 comments · Fixed by #2246
Closed

Convention: Negate "library" feature? #1627

webmaster128 opened this issue Mar 9, 2023 · 4 comments · Fixed by #2246
Labels
dev-experience improvements to DevX

Comments

@webmaster128
Copy link
Member

This is not really a topic of this repository but a larger CosmWasm ecosystem convention. cw-plus introduced a "library" feature convention to be able to use one contract package as a dependency of another contract. As brought up by multiple community members1 2 3, the library feature used to disable entry points is not additive.

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.

https://doc.rust-lang.org/cargo/reference/features.html

While I think in general this is the right™ way, there are a few things that are no clear to me yet:

  • Would this make adding a contract as a library much harder because you cannot unselect a single feature in Rust?
  • Would a negation achieve the additive property? Adding those global C exports probably still conflicts with it (imagine packages libraries with the entry_points feature enabled).
  • Shouldn't a contract and a library be in separate packages or build targets? A native executable (bin or example) is also built separately from the lib code. The idea of depending on a contract feels non-idiomatic.

Footnotes

  1. https://github.com/CosmWasm/cw-template/issues/140

  2. Who can help me find that earlier Twitter conversation that also included Shame and others?

  3. https://twitter.com/TheCyberHoward/status/1633833986182254594

@hashedone
Copy link
Contributor

I was thinking about it a long time ago, but I remember there were some problems with that - I don't remember exactly, but I think something with the dependency resolver. I would vote strongly "for" this feature. I would start with picking up some repo with more than one, but not too many contracts, try to apply the change there and then see if there are any issues (I can easily be wrong about that, I remember I tried to do something around that over a year ago so maybe I did something stupid, and that was causing problems).

@aeryz
Copy link

aeryz commented Mar 11, 2023

Would this make adding a contract as a library much harder because you cannot unselect a single feature in Rust?

Assuming there will be an entry_points feature for exporting the entry points, if a contract would want to use a cw-plus contract as a dependency, it needs to do:

cw20-base = { version = "*", default-features = false, features = [ /* remaining default features */ ] }

instead of:

cw20-base = { version = "*", features = [ "library"] }

I don't think it makes it much harder because currently there is no additional default features that is needed to be included, the only default feature will be entry_points. Even if you add default features, it can still be solved with a single line of documentation I guess.

Would a negation achieve the additive property? Adding those global C exports probably still conflicts with it (imagine packages libraries with the entry_points feature enabled).

Do you mean whether this can cause any problem if a dependency depends on another package with entry_points feature enabled?
eg.

A depends on:
-------> B (default-features = false) depends on:
        ----------> C (entry_points is enabled, hence A's exports conflict with C)

Shouldn't a contract and a library be in separate packages or build targets? A native executable (bin or example) is also built separately from the lib code. The idea of depending on a contract feels non-idiomatic.

I am also interested in seeing the use case here (how the failure happened).

@ethanfrey
Copy link
Member

Shouldn't a contract and a library be in separate packages or build targets? A native executable (bin or example) is also built separately from the lib code. The idea of depending on a contract feels non-idiomatic.

Actually I think this may be an interesting approach. All importable code in src and some minor shim in bin/wasm.rs or whatever to provide the exports. Not sure how doable this is or if it makes sense, but it would make sure that no one importing the source code could ever activate the external "C" exports (which is what we want to achieve with all this)

@aumetra
Copy link
Member

aumetra commented Sep 4, 2024

We can detect whether the current compilation unit (i.e. crate) is a dependency or not. Cargo sets the environment variable CARGO_PRIMARY_PACKAGE not for dependencies, so this code should detect it:

option_env!("CARGO_PRIMARY_PACKAGE").is_none()

@chipshort chipshort added the dev-experience improvements to DevX label Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-experience improvements to DevX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants