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

bump proc-macro2 #1350

Closed
wants to merge 0 commits into from
Closed

Conversation

salaheldinsoliman
Copy link

@salaheldinsoliman salaheldinsoliman commented Feb 18, 2024

What

1- Bump proc-macro2 crate in soroban-builtin-sdk-macros and soroban-env-macros
2- add Cargo.lock to .gitignore

Why

A version conflict is happening while building solang, after adding soroban-env-host as a dependancy:
error: failed to select a version for proc-macro2.
... required by package soroban-builtin-sdk-macros v20.2.2
... which satisfies dependency soroban-builtin-sdk-macros = "=20.2.2" of package soroban-env-host v20.2.2
... which satisfies dependency soroban-env-host = "^20.2.2" of package solang v0.3.3
versions that meet the requirements =1.0.69 are: 1.0.69

all possible versions conflict with previously selected packages.

previously selected package proc-macro2 v1.0.78
... which satisfies dependency proc-macro2 = "^1.0.78" of package parse-display-derive v0.9.0
... which satisfies dependency parse-display-derive = "=0.9.0" of package parse-display v0.9.0
... which satisfies dependency parse-display = "^0.9" of package solang v0.3.3

failed to select a version for proc-macro2 which could resolve this conflict

@salaheldinsoliman salaheldinsoliman requested a review from a team as a code owner February 18, 2024 21:29
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

One request inline.

@graydon I think this is a good example of how pinning deps is going to play out. This won't be the last time that or pinning creates a conflict for embedders.

soroban-builtin-sdk-macros/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

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

One more ask.

.gitignore Outdated
lcov.info
Cargo.lock
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this shouldn't have been in here in the first place. 👍🏻

Copy link
Member

Choose a reason for hiding this comment

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

I misread this diff, I thought it was removing Cargo.lock, but on relook I see it is adding it. We don't want the Cargo.lock file to be excluded from git, for the reasons described here: #1351 (comment)

Cargo.lock Outdated
Comment on lines 1025 to 1027
version = "1.0.69"
version = "1.0.78"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da"
checksum = "e2422ad645d89c99f8f3e6b88a9fdeca7fabeac836b1002371c4367c8f984aae"
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this back to .69 since it isn't necessary for this to select the newer version inside this repo?

Copy link
Author

Choose a reason for hiding this comment

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

Shouldn't I remove the whole file instead?

Copy link
Member

Choose a reason for hiding this comment

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

There shouldn't be any changes to the lock file, yup.

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.

2 participants