rustPlatform.importCargoLock: add support for git dependencies that use workspace inheritance#217232
Conversation
github-actions
bot
added
the
6.topic: rust
|
I wanted to include bumping Ruff here, but ran into astral-sh/ruff#3039. If it gets fixed before this is merged, I'll add it. |
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
3 times, most recently
from
77aeef1 to
7309269
Compare
ofborg
bot
added
10.rebuild-darwin: 1-10
There was a problem hiding this comment.
instead of testing the behavior of replaceWorskpaceValues, what if we just test cargoLock.lockFile directly?
[dependencies.rustpython-compiler-core]
package = "rustpython-compiler-core"
git = "https://github.com/RustPython/RustPython"There was a problem hiding this comment.
We could do that, but I also kind of prefer the simplistic nature of the current test, as it's tiny + adequately tests what the tool does.
Either works -- let's wait for others to weigh in. (Though if we do decide to go with the current approach, I should add a case for target.*.*, as well as maybe the other dependency types, just to be complete.)
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
from
7309269 to
33488ed
Compare
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
from
33488ed to
ed8fa82
Compare
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
2 times, most recently
from
6b99cde to
413ca39
Compare
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
from
413ca39 to
0cc4830
Compare
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
2 times, most recently
from
021f556 to
ed26384
Compare
figsoda
left a comment
figsoda
left a comment
There was a problem hiding this comment.
Can't speak much about the changes to matrix-conduit, would appreciate a review from one of the maintainers.
The importCargoLock and ruff changes lgtm, would prefer if tests are done using buildRustPackage instead but this is up for discussion
|
Result of 1 package failed to build:
5 packages built:
Result of 3 packages failed to build:
3 packages built:
|
|
All of those failures are unrelated. Darwin: dependency being missed after the libiconv changes (I presume, haven't looked at history to confirm). |
|
Yep I was just posting this so other people don't have to rebuild these, and I think you were right about the dependency version mismatch |
|
Fixing these now... |
…se workspace inheritance Rust 1.64.0 added support for workspace inheritance, which allows for crates to inherit values such as dependency version constraints or package metadata information from their workspaces [0]. This works by having workspace members specify a value as a table, with `workspace` set to true. Thus, supporting this in importCargoLock is as simple as walking the crate's Cargo.toml, replacing inherited values with their workspace counterpart. This is also what a forthcoming Cargo release will do for `cargo vendor` [1], but we can get ahead of it ;) [0]: https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html#cargo-improvements-workspace-inheritance-and-multi-target-builds [1]: rust-lang/cargo#11414
https://github.com/charliermarsh/ruff/releases/tag/v0.0.245 https://github.com/charliermarsh/ruff/releases/tag/v0.0.246 https://github.com/charliermarsh/ruff/releases/tag/v0.0.247 https://github.com/charliermarsh/ruff/releases/tag/v0.0.248
As `cargo vendor` currently doesn't support workspace inherited manifest values, a patch that replaced the values manually was used as a workaround. Now that importCargoLock has support for this, and we're probably going to move towards it anyways, let's switch to it now, as there's a clear benefit either way. This also switches to using our copy of SQLite instead of the vendored one included in `libsqlite3-sys` -- because it's preferred, but also because it causes the build to fail, now that dependencies are read-only.
winterqt
force-pushed
the
import-cargo-lock-git-dep-workspace-inheritance
branch
from
ed26384 to
798e26a
Compare
|
Could you make an estimation when a consumer would be able to make use of this fix inside any nix (pre) release? |
|
This is already in master and the unstable branches, if you are using stable, then 23.05 |
Thanks for the quick reply. Any chance it will be backported to 22.11? |
|
Unfortunately, the error still persists This is the relevant feature where we encounter this: centrifuge/centrifuge-chain#1241 |
|
This fix only applies to - cargoSha256 = "sha256-BkSVd08QksOvyIRO1bVpiOnWf440r+dLR1A3UiND7IM=";
+ cargoLock = {
+ lockFile = ./Cargo.lock;
+ allowBuiltinFetchGit = true;
+ }; |
|
Thanks for sharing this information with us. Looking forward to the release which supports workspace inheritance in |
4 participants
Description of changes
Rust 1.64.0 added support for workspace inheritance, which allows
for crates to inherit values such as dependency version constraints or
package metadata information from their workspaces 0.
This works by having workspace members specify a value as a table, with
workspaceset to true. Thus, supporting this in importCargoLock is assimple as walking the crate's Cargo.toml, replacing inherited values
with their workspace counterpart.
This is also what a forthcoming Cargo release will do for
cargo vendor1,but we can get ahead of it ;)
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)