fix: filegroup circular deps #164
Conversation
|
I don't think this solves the problem. By not depending on the |
|
Agree. But currently package From another side gathering all needed _PACKAGES is pointless as if packages should handle transitive dependencies then this list should contain only those explicitly specified in Indeed current change is not a fix but rather a quick workaround. What I see now, the real package (e.g. At least this is mine understanding of current state 🤷♂️ |
Simplifying to only one arch `amd64`, we have
main package (e.g. `libgcc-1s`) defined as:
```bzl
alias(
name = "data",
actual = "//libgcc-s1/amd64:data",
visibility = ["//visibility:public"],
)
filegroup(
name = "libgcc-1s",
srcs = [
"//libgcc-s1/amd64:amd64",
] + [":data"],
)
```
The appropriate arch target is of:
```bzl
alias(
name = "data",
actual = "@@rules_distroless++apt+bookworm_libgcc-s1_12.2.0-14-p-deb12u1_amd64//:data",
visibility = ["//visibility:public"],
)
filegroup(
name = "amd64",
srcs = [
"//libc6/amd64",
"//gcc-12-base/amd64",
] + [":data"],
visibility = ["//visibility:public"],
)
```
In the end main `libgcc-s1` contains redundant `//libgcc-s1/amd64:data` target,
thus remove redundant `:data` target from main package definition.
Eliminate circular dependencies within package definitions.
Bazel dependency graph rely on DAG (Directed Acyclic Graph).
Provided sources (`srcs`) to the target are already all transitive
package dependencies taken from the lockfile, thus its `:data`
is sufficient to fully cover package and all its dependencies.
Taking above and current implementation, all package deps should be
listed with `:data`. E.g. for `libgcc-s1/amd64:amd64` package, it is:
```bzl
filegroup(
name = "amd64",
srcs = [
"//libc6/amd64:data",
"//gcc-12-base/amd64:data",
] + [":data"],
visibility = ["//visibility:public"],
)
```
is equal
birunts
force-pushed
the
filegroup_circular_deps
branch
from
fdd5757 to
a9e67d1
Compare
|
One more look at the issue 😉 I have checked generated package defs and first commit removes redundant When it comes to the second commit all the package's data dependencies should be listed with Bazel dependency graph is of Directed Acyclic Graph, thus current filegroup's definition is wrong, making cycle in dependency graph. @thesayyn & @alexeagle plase have a look once again for the fix. |
2 participants
Previously generated
filegrouptargets can contain dependency cycle.The case here is that defined filegroup of a package should not have deps of other packages but rather to its
:datapackage.This change removes potential dependency cycles and makes references to package's
:data.Resolves #163 & Closes #163.