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

Implicitly specify whether contractimpl should be exported using CARGO_PRIMARY_PACKAGE #407

Closed
wants to merge 3 commits into from

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Aug 9, 2022

What

Base whether contractimpl functions are exported as callable wasm functions and in the contract spec on whether CARGO_PRIMARY_PACKAGE is set.

Why

The environment variable CARGO_PRIMARY_PACKAGE is set when a crate is being built as the primary crate, and it isn't set when a crate is being built as a dependency.

It is not particularly intuitive that if you import a crate that contains a contract that the contractimpls of that crate will be exported as callable functions of your crate. In fact, I would go as far to say that this is so surprising that it could result in contract developers accidentally exposing functions from other contracts that they wish to wrap or import for shared types.

We support this use case today with the export_if attribute on contractimpl that allows a contract developer to specify a build feature that should be used to gate the exporting on. i.e. If the feature is enabled, the functions are exported. If the feature is disabled, the functions are not exported. This solution of using a feature flag provides flexibility, but at a cost of verbosity, and still relies on developers importing the package to remember to do the right thing by configuring the features appropriately. The solution of feature flags also requires developers to opt-in, which means a developer who doesn't choose to use export_if will break other contracts who import it.

It could be argued that using the CARGO_PRIMARY_PACKAGE is both magical and limiting since it is outside the developers control, but actually it is the same level of capability, it just flips what the default behavior is. With this change the default behavior is no contractimpl is reexported by the importing crate. A developer who wishes to reexport a dependencies contractimpl must do so explicitly by creating a contract type, writing an impl block, and calling through to the dependencies contract type. This is mildly inconvenient, and if there is shown to be significant demand for this type of code we can add a macro that simplifies the whole process.

Note that this change doesn't change whether contracttypes are exposed, since importing a crate that has contracttypes is likely to use those contracttypes. There's an assumption here that we can revisit at a later point.

@leighmcculloch leighmcculloch requested a review from a team August 9, 2022 02:53
@leighmcculloch leighmcculloch changed the title Implicitly specify with contractimpl should be exported using CARGO_PRIMARY_PACKAGE Implicitly specify whether contractimpl should be exported using CARGO_PRIMARY_PACKAGE Aug 9, 2022
@jonjove
Copy link
Contributor

jonjove commented Aug 9, 2022

This is a cool find. I support this change.

@leighmcculloch
Copy link
Member Author

Unfortunately there's an edge case that this does not behave as expected with.

CARGO_PRIMARY_PACKAGE is set to true for dependencies when the dependency is a default member of a workspace (rust-lang/cargo#10956), so this change in this PR as it exists right now is not sufficient.

I looked into alternative ways to conditionally compile code based on if the package is the primary package or a dependency, and the next best option might be conditionally compiling on rlib vs cdylib, but that is not possible yet (rust-lang/rust#45182, rust-lang/rust#20267).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit pattern for defining if contractimpl is exported
2 participants