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.

Sign up for GitHub

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

Jump to bottom

Duplicated deps when multiple targets share dependencies #451

Open
PiotrSikora opened this issue Nov 12, 2021 · 3 comments · Fixed by #478 · May be fixed by #512
Open

Duplicated deps when multiple targets share dependencies #451

PiotrSikora opened this issue Nov 12, 2021 · 3 comments · Fixed by #478 · May be fixed by #512

Comments

@PiotrSikora
Copy link
Contributor

errno crate lists multiple targets that depend on libc crate (Cargo.toml):

[target.'cfg(unix)'.dependencies]
libc = "0.2"

[target.'cfg(windows)'.dependencies]
winapi = { version = "0.3", features = ["errhandlingapi", "minwindef"] }

[target.'cfg(target_os="dragonfly")'.dependencies]
errno-dragonfly = "0.1.1"

[target.'cfg(target_os="wasi")'.dependencies]
libc = "0.2"

[target.'cfg(target_os="hermit")'.dependencies]
libc = "0.2"

which gets translated by cargo raze into this (BUILD.errno-0.2.8.bazel):

    deps = [
    ] + selects.with_or({
        # cfg(target_os = "wasi")
        (
            "@rules_rust//rust/platform:wasm32-wasi",
        ): [
            "@proxy_wasm_cpp_host__libc__0_2_106//:libc",
        ],
        "//conditions:default": [],
    }) + selects.with_or({
        # cfg(unix)
        (
            "@rules_rust//rust/platform:aarch64-apple-darwin",
            "@rules_rust//rust/platform:aarch64-apple-ios",
            "@rules_rust//rust/platform:aarch64-linux-android",
            "@rules_rust//rust/platform:aarch64-unknown-linux-gnu",
            "@rules_rust//rust/platform:arm-unknown-linux-gnueabi",
            "@rules_rust//rust/platform:i686-apple-darwin",
            "@rules_rust//rust/platform:i686-linux-android",
            "@rules_rust//rust/platform:i686-unknown-freebsd",
            "@rules_rust//rust/platform:i686-unknown-linux-gnu",
            "@rules_rust//rust/platform:powerpc-unknown-linux-gnu",
            "@rules_rust//rust/platform:s390x-unknown-linux-gnu",
            "@rules_rust//rust/platform:x86_64-apple-darwin",
            "@rules_rust//rust/platform:x86_64-apple-ios",
            "@rules_rust//rust/platform:x86_64-linux-android",
            "@rules_rust//rust/platform:x86_64-unknown-freebsd",
            "@rules_rust//rust/platform:x86_64-unknown-linux-gnu",
        ): [
            "@proxy_wasm_cpp_host__libc__0_2_106//:libc",
        ],
        "//conditions:default": [],
    }) + selects.with_or({
        # cfg(windows)
        (
            "@rules_rust//rust/platform:i686-pc-windows-msvc",
            "@rules_rust//rust/platform:x86_64-pc-windows-msvc",
        ): [
            "@proxy_wasm_cpp_host__winapi__0_3_9//:winapi",
        ],
        "//conditions:default": [],
    }),

However, such duplicates are disallowed in Bazel and bazel query treats it as an error (see: bazelbuild/bazel#13785):

$ bazel query 'deps(set(@proxy_wasm_cpp_host__errno__0_2_8//:errno))'

ERROR: .../external/proxy_wasm_cpp_host__errno__0_2_8/BUILD.bazel:34:13: Label '@proxy_wasm_cpp_host__libc__0_2_106//:libc' is duplicated in the 'deps' attribute of rule 'errno'
ERROR: .../external/proxy_wasm_cpp_host__errno__0_2_8/BUILD.bazel:34:13: Target '@proxy_wasm_cpp_host__errno__0_2_8//:src/lib.rs' contains an error and its package is in error and referenced by '@proxy_wasm_cpp_host__errno__0_2_8//:errno'
ERROR: .../external/proxy_wasm_cpp_host__errno__0_2_8/BUILD.bazel:34:13: Target '@proxy_wasm_cpp_host__errno__0_2_8//:src/hermit.rs' contains an error and its package is in error and referenced by '@proxy_wasm_cpp_host__errno__0_2_8//:errno'
ERROR: .../external/proxy_wasm_cpp_host__errno__0_2_8/BUILD.bazel:34:13: Target '@proxy_wasm_cpp_host__errno__0_2_8//:src/unix.rs' contains an error and its package is in error and referenced by '@proxy_wasm_cpp_host__errno__0_2_8//:errno'
ERROR: .../external/proxy_wasm_cpp_host__errno__0_2_8/BUILD.bazel:34:13: Target '@proxy_wasm_cpp_host__errno__0_2_8//:src/wasi.rs' contains an error and its package is in error and referenced by '@proxy_wasm_cpp_host__errno__0_2_8//:errno'
ERROR: .../external/proxy_wasm_cpp_host__errno__0_2_8/BUILD.bazel:34:13: Target '@proxy_wasm_cpp_host__errno__0_2_8//:src/windows.rs' contains an error and its package is in error and referenced by '@proxy_wasm_cpp_host__errno__0_2_8//:errno'
ERROR: Evaluation of query "deps(set(@proxy_wasm_cpp_host__errno__0_2_8//:errno))" failed: errors were encountered while computing transitive closure

This could be solved by either generating a single select (as suggested here: #437 (comment)) or by combining all select conditions that evaluate into the same value into one (although, that would work only when dependencies are exactly the same and not simply overlapping). cc @illicitonion
@PiotrSikora PiotrSikora mentioned this issue Nov 12, 2021
Merged
@supercmmetry
Copy link

Is there any fix for this?

@sayrer sayrer mentioned this issue Mar 28, 2022
Merged
dfreese pushed a commit that referenced this issue Jun 20, 2022
@sayrer
Improve per-platform feature selection. (#478)
743bd26 
This patch should fix #451.

The patch runs cargo-tree to determine which features are enabled on each platform, and then groups them by using vectors of platforms as map keys. It's a lot of code, and there are no tests yet. But I wanted to post this initial effort and get some feedback from the code owners. @illicitonion has already seen this code, but I believe it will be new to @dfreese and @PiotrSikora (who filed the linked issue).

The new file is copyright AgileBits, as this was work for hire for them. They are fine with contributing, and my repo has been public for months.
@sayrer
Copy link
Contributor

sayrer commented Jun 22, 2022

Hmm, this shouldn't have been closed. I mistakenly mentioned it in #478.

@sayrer sayrer linked a pull request Jun 23, 2022 that will close this issue
Open
@dfreese dfreese reopened this Jun 23, 2022
PiotrSikora added a commit to PiotrSikora/proxy-wasm-cpp-host that referenced this issue Jul 31, 2022
@PiotrSikora
wasmtime: update to v0.39.1.
31b3661 
While there, add a format check to verify presence of manual fixes
to workaround google/cargo-raze#451.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora mentioned this issue Jul 31, 2022
Merged
PiotrSikora added a commit to proxy-wasm/proxy-wasm-cpp-host that referenced this issue Aug 1, 2022
@PiotrSikora
wasmtime: update to v0.39.1. (#296)
9387a5b 
While there, add a format check to verify presence of manual fixes
to workaround google/cargo-raze#451.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
knm3000 pushed a commit to knm3000/proxy-wasm-cpp-host that referenced this issue Aug 17, 2022
@PiotrSikora @knm3000
wasmtime: update to v0.39.1. (proxy-wasm#296)
7d483d9 
While there, add a format check to verify presence of manual fixes
to workaround google/cargo-raze#451.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora PiotrSikora mentioned this issue Sep 28, 2022
Merged
@RiverPhillips RiverPhillips mentioned this issue Nov 21, 2022
Open
@dmitrii-ubskii dmitrii-ubskii mentioned this issue Nov 23, 2022
Merged
dmitrii-ubskii added a commit to typedb/dependencies that referenced this issue Nov 23, 2022
@dmitrii-ubskii
Add cucumber crate to cargo raze (#394)
57d73f9 
## What is the goal of this PR?

We add the cucumber testing framework crate to our pinned rust
dependencies.

## What are the changes implemented in this PR?

While making this change, we encountered an issue in bazel
(bazelbuild/bazel#13785) that made it throw an
error because, in essence, libc was included in two out of three
conditions, which was regarded as a dependency duplication.

Cargo-raze is aware of this issue
(google/cargo-raze#451) and has their own
workaround in the works (google/cargo-raze#512)
which is not being reviewed or merged due to reviewer bandwidth issues.

As our own workaround, we separate the inclusion of libc into its own
condition, overriding raze and pacifying bazel. This change is stored
separately from the razed crates and reapplied when
`library/crates/update.sh` is run.
@PiotrSikora
Copy link
Contributor Author

It looks that rules_rust's cargo_universe decided to group deps by a platform triple rather than by cfg string: bazelbuild/rules_rust#1647.

jamesreprise pushed a commit to jamesreprise/vaticle-dependencies that referenced this issue Dec 5, 2022
@dmitrii-ubskii
Add cucumber crate to cargo raze (typedb#394)
29ddd4c 
## What is the goal of this PR?

We add the cucumber testing framework crate to our pinned rust
dependencies.

## What are the changes implemented in this PR?

While making this change, we encountered an issue in bazel
(bazelbuild/bazel#13785) that made it throw an
error because, in essence, libc was included in two out of three
conditions, which was regarded as a dependency duplication.

Cargo-raze is aware of this issue
(google/cargo-raze#451) and has their own
workaround in the works (google/cargo-raze#512)
which is not being reviewed or merged due to reviewer bandwidth issues.

As our own workaround, we separate the inclusion of libc into its own
condition, overriding raze and pacifying bazel. This change is stored
separately from the razed crates and reapplied when
`library/crates/update.sh` is run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@PiotrSikora @sayrer @dfreese @supercmmetry