Only remove sibling paths from cargo manifests - #781
Conversation
We can leave direct references to crates in subdirectories. Assisted-by: Gemini 3.5 Flash <gemini@google.com>
Blast545
left a comment
Blast545
left a comment
There was a problem hiding this comment.
I mainly checked the tests and I'm confused, can you leave us a msg about why this is needed? and how it's failing.
I thought I was aware of the bug as per a previous discussion, but I don't get completely get it from test case covered.
| 'dependencies': { | ||
| 'absolute_dep': {'path': '/abs/path/to/dep'}, | ||
| 'parent_dep': {'path': '../parent/dep'}, | ||
| 'sibling_dep': {'path': '../sibling'}, |
There was a problem hiding this comment.
In my mental model, ../sibling it's a sibling of my current crate, correct, but I don't see why ../parent/dep it's a parent. Looks like it's a child of another sibling, a nephew?
Maybe I'm getting something wrong
There was a problem hiding this comment.
This is maybe just a mismatch in nomenclature between what I asked Gemini to do and how I write the commit description.
All of these paths are from the perspective of a package at it's root directory. We archive the packages from that directory, so anything above it (starting with ../) is removed by Bloom's package-level processing and will NOT be present when the package is built on the buildfarm.
Maybe we just call these sibling_dep and nested_sibling_dep. Doesn't really matter, either way they should be removed (and possibly converted to a version = '*' constraint).
There was a problem hiding this comment.
Maybe we just call these
sibling_depandnested_sibling_dep
That change sounds good to me then, that renaming would makes sense from the description and matches the test behavior
| assert 'path' not in deps['parent_dep'] | ||
| assert deps['parent_dep']['version'] == '*' | ||
|
|
||
| assert 'path' not in deps['sibling_dep'] | ||
| assert deps['sibling_dep']['version'] == '*' |
There was a problem hiding this comment.
As per the title description, I thought parent_dep and sibling_dep were going to have a different behavior
There was a problem hiding this comment.
Also, if this is the expected behavior, maybe it should be better aligned with the title
3 participants
We can leave direct references to crates in subdirectories.
This code hasn't been released yet so it's still safe to make critical changes to it.
Assisted-by: Gemini 3.5 Flash