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

Fix dependency conflicts when used with git or path locations #3777

Closed

Conversation

yangby-cryptape
Copy link

Issue

When specifying crates that in lighthouse as dependencies with git or path locations, it always throws errors when builds.

Because the latest versions of some crates haven't published to crates.io.

An example:

  • In Cargo.toml:

    eth2_ssz_types = { git = "https://github.com/sigp/lighthouse" }
  • The error:

        Updating git repository `https://github.com/sigp/lighthouse`
        Updating crates.io index
    error: failed to select a version for the requirement `eth2_serde_utils = "^0.1.1"`
    candidate versions found which didn't match: 0.1.0
    location searched: crates.io index
    required by package `eth2_ssz_types v0.2.2 (https://github.com/sigp/lighthouse#bf533c8e)`
        ... which satisfies git dependency `eth2_ssz_types` of package `lighthouse-demo v0.1.0 (/tmp/lighthouse-demo)`
    

Proposed Changes

Replace the patch section with multiple locations to specify dependencies.

@CLAassistant
Copy link

CLAassistant commented Dec 6, 2022

CLA assistant check
All committers have signed the CLA.

@michaelsproul
Copy link
Member

What does the Cargo.toml for an external project end up looking with this change? I'm worried that external projects may not compile if they try to use the crates.io versions of crates with the Lighthouse versions, because of features added, etc that haven't been published.

The workaround that I've been using for external projects is just to replicate the [patch] section: https://github.com/michaelsproul/blockdreamer/blob/ee26a5ce4fbc4c411c4f35ca5797c4bda11ce71b/Cargo.toml#L29-L36. This isn't particularly nice though, so we are definitely interested in longer-term solutions. Recently I've been wondering if we should extract the independent packages from Lighthouse to separate repos/crates (or auto-publish them on every Lighthouse release instead of our current system that nobody uses).

@michaelsproul michaelsproul changed the base branch from stable to unstable December 6, 2022 05:46
@michaelsproul michaelsproul added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Dec 12, 2022
@yangby-cryptape
Copy link
Author

yangby-cryptape commented Dec 16, 2022

What does the Cargo.toml for an external project end up looking with this change?

For external projects:

  • Used the version in their Cargo.toml: the projects will download the crates from "crates.io" with the specify version.

  • Used the git in their Cargo.toml: the projects will use the crates that checkout from the git repository.

    Would NOT download these crates from "crates.io".

  • Used the path in their Cargo.toml: the projects will use the crates following the path on local disk.

    Would NOT download these crates from "crates.io".

N.B. If both version and git (or path) are used, that if the real version doesn't match, Cargo will fail to compile!

p.s. When the crates are published into crates.io, the git and path fields will be removed.

Ref: The Cargo Book / Specifying Dependencies / Multiple locations

@michaelsproul
Copy link
Member

We've actually just made a resolution this week to extract these common crates into their own repos, which will force us to keep them up to date on crates.io. The goal is that Lighthouse becomes a downstream consumer of these crates just like any other project.

We'll likely do the crate extraction and cleanup in January

@yangby-cryptape
Copy link
Author

We've actually just made a resolution this week to extract these common crates into their own repos,

That sounds awesome!

I was working on a downstream cross-chain project, we couldn't publish our project until all upstream dependencies are published.

And, I still suggest that avoiding [the patch section] to specify dependencies, even after extraction.


Another Irrelevant Suggestion: Is there any plan for no-std?

Some crates in the bottom layer could has the no-std feature.

In our forked version, I implemented no-std for ssz-related, hashing-related and merkle-related crates, since we want to verify transactions and headers in a virtual RISC-V machine.

If you would accept that, I could submit PRs after your extraction and cleanup.

@michaelsproul
Copy link
Member

michaelsproul commented Dec 16, 2022

And, I still suggest that avoiding [the patch section] to specify dependencies, even after extraction.

Yeah that's the plan!

If you would accept that, I could submit PRs after your extraction and cleanup.

We could definitely support no-std compatibility, particularly if you're happy to build it and we have a way of testing it on CI.

I'll ping you once we've extracted the repos

@michaelsproul
Copy link
Member

Hey @yangby-cryptape, we've finished extracting the repos but haven't merged the PR that uses them in Lighthouse yet. I'll close this PR in favour of that one: #3890.

Thanks!

@yangby-cryptape yangby-cryptape deleted the pr/fix-dep-conflicts branch June 6, 2023 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants