Skip to content

Commit 581e662

Browse files
author
Matt Mackay
authored
fix: split on site-packages path segment when reporting conflicts (#413)
When expanding wheels via rules_pycross' `pycross_wheel_library`, the wheels end up in `bazel-out`, rather than within the runfiles tree like when consuming from rules_python. To account for this, change the delimiter to `site-packages/` given that is the actual common ancestor in the path. This also changes to using the full paths for reporting the conflict to make it easier to see where the incoming conflict is from. Closes #359 --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes Use `site-packages/` as path delimiter when reporting venv package conflicts. ### Test plan - Covered by existing test cases - Manual testing; please provide instructions so we can reproduce: Run against https://github.com/tgeng/pycross_venv_bug_repro
1 parent 781650c commit 581e662

File tree

1 file changed

+22
-7
lines changed

1 file changed

+22
-7
lines changed

py/tools/py/src/pth.rs

+22-7
Original file line numberDiff line numberDiff line change
@@ -138,22 +138,33 @@ fn create_symlink(
138138
let link = dst_dir.join(tgt.strip_prefix(root_dir).unwrap());
139139

140140
fn conflict_report(link: &Path, tgt: &Path, severity: Severity) -> miette::Report {
141-
let path_to_conflict = link
141+
const SITE_PACKAGES: &str = "site-packages/";
142+
143+
let link_str = link.to_str().unwrap();
144+
let tgt_str = tgt.to_str().unwrap();
145+
146+
let link_span_range = link
142147
.to_str()
143-
.and_then(|s| s.split_once("site-packages/"))
148+
.and_then(|s| s.split_once(SITE_PACKAGES))
144149
.map(|s| s.1)
150+
.map(|s| (link_str.len() - s.len() - SITE_PACKAGES.len())..link_str.len())
145151
.unwrap();
146-
let next_conflict = tgt
152+
153+
let conflict_span_range = tgt
147154
.to_str()
148-
.and_then(|s| s.split_once(".runfiles/"))
155+
.and_then(|s| s.split_once(SITE_PACKAGES))
149156
.map(|s| s.1)
157+
.map(|s| {
158+
(link_str.len() + tgt_str.len() - s.len() - SITE_PACKAGES.len() + 1)
159+
..tgt_str.len() + link_str.len() + 1
160+
})
150161
.unwrap();
151162

152163
let mut diag = MietteDiagnostic::new("Conflicting symlinks found when attempting to create venv. More than one package provides the file at these paths".to_string())
153164
.with_severity(severity)
154165
.with_labels([
155-
LabeledSpan::at(0..path_to_conflict.len(), "Existing file in virtual environment"),
156-
LabeledSpan::at((path_to_conflict.len() + 1)..(path_to_conflict.len() + 1 + next_conflict.len()), "Next file to link"),
166+
LabeledSpan::at(link_span_range, "Existing file in virtual environment"),
167+
LabeledSpan::at(conflict_span_range, "Next file to link"),
157168
]);
158169

159170
diag = if severity == Severity::Error {
@@ -162,7 +173,11 @@ fn create_symlink(
162173
diag.with_help("Set `package_collisions = \"ignore\"` on the binary or test rule to ignore this warning")
163174
};
164175

165-
miette!(diag).with_source_code(format!("{}\n{}", path_to_conflict, next_conflict))
176+
miette!(diag).with_source_code(format!(
177+
"{}\n{}",
178+
link.to_str().unwrap(),
179+
tgt.to_str().unwrap()
180+
))
166181
}
167182

168183
if link.exists() {

0 commit comments

Comments
 (0)