Skip to content

Conversation

@ascjones
Copy link
Collaborator

Closes #927.

I have a vague memory of some reasoning that the package name and lib name were separate. However I cannot remember or imagine what it might be, so LFG.

@ascjones ascjones requested a review from athei January 26, 2023 15:17
@ascjones
Copy link
Collaborator Author

CI errors should be fixed in #930

@athei
Copy link
Contributor

athei commented Jan 26, 2023

I think rlib is the default for libs anyways. Do we even need crate-type in the manifest then?

@HCastano
Copy link
Contributor

I think rlib is the default for libs anyways. Do we even need crate-type in the manifest then?

From here it doesn't necessarily say that it's the default. It just says that the compiler will choose the best option

scale-info = { version = "2.3", default-features = false, features = ["derive"], optional = true }

[lib]
name = "{{name}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this goes back to at least 2019 🤷

@ascjones
Copy link
Collaborator Author

I think rlib is the default for libs anyways. Do we even need crate-type in the manifest then?

I've tested this with the delegator ink! example, removing the crate-type section from the adder manifest, and the it compiles fine with the same resulting contract size.

@athei
Copy link
Contributor

athei commented Jan 27, 2023

I think rlib is the default for libs anyways. Do we even need crate-type in the manifest then?

From here it doesn't necessarily say that it's the default. It just says that the compiler will choose the best option

Yes. But the default should always allow depending on it from another crate. We actively prevented this by setting it to only cdylib before this PR. I just tested without any crate type: I can depend on a contract.

I've tested this with the delegator ink! example, removing the crate-type section from the adder manifest, and the it compiles fine with the same resulting contract size.

This was expected because we set it to cdylib during build.

@ascjones ascjones merged commit da24337 into master Jan 30, 2023
@ascjones ascjones deleted the aj/template-changes branch January 30, 2023 20:04
@HCastano
Copy link
Contributor

@ascjones can you add a CHANGELOG entry since this changes the default behaviour of contract dependencies?

@ascjones ascjones mentioned this pull request Feb 14, 2023
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.

cargo-contract new should not emit name for the [lib] section

4 participants