-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
don't constrain cyclic deps to build version #411
Conversation
|
should also allow us to build specific llvm versions. |
@@ -61,6 +61,10 @@ export default async function hydrate( | |||
|
|||
for (const dep of await get_deps(node.pkg, initial_set.has(node.project))) { | |||
if (children.has(dep.project)) { | |||
const self = graph[dep.project] | |||
if (self.pkg.constraint === target.pkg.constraint) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like you need to implement Range.eq
, surely ===
will only work here if the object itself is the same object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to do exactly what I want, in testing. Replaces the build pkg with the dep so we don't require the version we want to build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's a coincidence that ===
works. It would be trivial for some part of the code to create a new copy of the object at some prior step.
k, so changing this code is dangerous. We better get it right. Can you highlight what part of the code is failing per your failure spec? |
d49e1a8
to
6923330
Compare
No patch: $ ./libexec/deps.ts go.dev~1.20 --build
go.dev must be bootstrapped to build go.dev
curl.se/ca-certs
gnu.org/m4^1
tea.xyz/gx/cc^0.1
openssl.org^1
go.dev~1.20 Patched: $ ./libexec/deps.ts go.dev~1.20 --build
go.dev must be bootstrapped to build go.dev
curl.se/ca-certs
gnu.org/m4^1
tea.xyz/gx/cc^0.1
openssl.org^1
go.dev
|
I think the actual solution for the this is to hydrate the deps of go rather than go itself. So this is a regression in brewkit. |
That sounds like the right answer. However, if you pass two packages and one depends on the other, it won't be "self-hydrating" via the other build. But that might be ok. We generally don't do multi-package builds (and source matching is currently broken due to re-ordering). |
requires #406 to pass checks
fixes #412
Basically if a dep requests the target (
go.dev
requiresgo.dev: '*'
), this will prevent locked loop (buildgo.dev=1.20.1
trying to installgo.dev=1.20.1
)