Sort builds if dependency relation between manifest components #3284
Conversation
| return Ok(()); | ||
| } | ||
|
|
||
| let (components_to_build, has_cycle) = sort(components_to_build); |
There was a problem hiding this comment.
We should comment why we're sorting. This code is pretty opaque when you don't have the context of #3283.
crates/build/src/lib.rs
Outdated
| let (components_to_build, has_cycle) = sort(components_to_build); | ||
|
|
||
| if has_cycle { | ||
| terminal::warn!("Cyclic dependency detected"); |
There was a problem hiding this comment.
While this just be more confusing than actionable? What do we expect users to do with this information?
There was a problem hiding this comment.
@rylev Good question. It's less "actionable" than it is "if you see errors where a consumer is not 'seeing' changes to the dependency interface then this could be why." But maybe it's so pathological a case that it's not worth flagging...
There was a problem hiding this comment.
Seems like something only someone with deep knowledge would be able to take action on which would indicate debug being a more appropriate logging level.
There was a problem hiding this comment.
I think this will become a thing we'll have to show more prominently before too long. Or rather, I think it should :)
Specifically, if and when we add features that require build outputs of dependencies as inputs to the dependee's build process.
It might make sense to look at the error message cargo reports for cyclic dependencies between crates as inspiration.
There was a problem hiding this comment.
@tschneidereit In the interests of getting this PR over the line, I propose we cross that bridge when we come to it...
There was a problem hiding this comment.
Oh yes, my apologies: I hadn't meant to say that this needs to happen now or otherwise nothing can possibly land.
itowlson
force-pushed
the
build-deps-before-components
branch
from
07c7281 to
1ff8a8f
Compare
Signed-off-by: itowlson <[email protected]>
itowlson
force-pushed
the
build-deps-before-components
branch
from
1ff8a8f to
fdf1345
Compare
Fixes #3283.