search

stellar / rs-soroban-env

Rust environment for Soroban contracts.
Apache License 2.0
59 stars 40 forks source link

Loosen proc-macro2 version selection #1351

Open salaheldinsoliman opened 5 months ago

salaheldinsoliman commented 5 months ago

What

1-Bump and loosen crates proc-macro2 and quote

Why

To solve a dependancy error while building Solang, when adding soroban-env-host to its Cargo.toml: 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

leighmcculloch commented 5 months ago

Sorry, I think I made a mistake in the previous PR where I commented about the lock file. We need to keep Cargo.lock, and not have it in the gitignore.

While there is a historical recommendation to not commit lock files in Rust libraries, we don't follow it because it is safer for developers working on the library if a lock file is present, otherwise fresh clones of the repo download new versions of crates outside the control of the developer, and for good security, developers should always be in control of what is installed/built/run on their systems.

Could we keep the Cargo.lock file? It shouldn't need modifying as part of this change.

leighmcculloch commented 3 months ago

Related discussion in Discord: https://discord.com/channels/897514728459468821/1067534037310255225/1230929191751647314

leighmcculloch commented 3 months ago

Related internal discussion: https://stellarfoundation.slack.com/archives/C030Z9EHVQE/p1713745643062029

salaheldinsoliman commented 3 months ago

Related internal discussion: https://stellarfoundation.slack.com/archives/C030Z9EHVQE/p1713745643062029

@leighmcculloch Is there a way to have a peek on this discussion, as I don't have access to it?

leighmcculloch commented 3 months ago

We have some things to nut internally about how different projects depend on dependency consistency.

But I see @dmkozh approved already, so possibly we could merge this now.

@dmkozh Is this mergeable?

salaheldinsoliman commented 3 months ago

We have some things to nut internally about how different projects depend on dependency consistency.

But I see @dmkozh approved already, so possibly we could merge this now.

@dmkozh Is this mergeable?

I think I should close this, and instead merge #1415 as it bumps both proc-macro2 and quote

graydon commented 3 months ago

I think we're going to do this (in preference to #1415) but before we do, we'll write a tool that checks that the lockfile on the stellar-core repo is the same (at least on any package in common) with the lockfile in this repo. That ought to address the concern that the two might be testing / releasing different versions. I'll update here when I have such a thing ready.

leighmcculloch commented 3 months ago

@salaheldinsoliman Since we're going to relax the version requirements rather than pin, did you want to also relax the version requirement of quote in this PR?

graydon commented 3 months ago

here's an initial sketch of such a tool: https://github.com/graydon/check-lockfile-intersection output looks like so: https://gist.github.com/graydon/3856f26fe34bb695316f795cf04462c9 I'll work on this more tomorrow, out of time for tonight.

leighmcculloch commented 2 weeks ago

👋🏻 What's the status of the check-lockfile-intersection tool?