Skip to content

Commit eb27133

Browse files
committed
support clang-format wheels' odd version scheme
Looks like subsequent releases for the same clang version are suffixed with a build number (`11.1.0.2`), but this is not semver compliant. It seems PEP440 is not as strict as semver rules. As a workaround, we use the build number as a prerelease version and append a prerelease version of `0` to those with no build number suffixed. I also updated the logs to be less confusing about which build number (for identical clang versions) is being downloaded.
1 parent d0ece80 commit eb27133

1 file changed

Lines changed: 48 additions & 24 deletions

File tree

  • clang-installer/src/downloader

clang-installer/src/downloader/pypi.rs

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -353,11 +353,6 @@ impl FromStr for PlatformTag {
353353
/// ```
354354
#[derive(Debug)]
355355
struct WheelTags {
356-
/// The version of the package for which the wheel is built.
357-
///
358-
/// For clang-format and clang-tidy wheels, this corresponds to the version of the clang tool.
359-
version: Version,
360-
361356
/// The platform tag indicates the wheel's target platform.
362357
platform: PlatformTag,
363358
}
@@ -377,20 +372,14 @@ impl FromStr for WheelTags {
377372
}
378373
iter.next(); // already know the package name
379374

380-
// The exact version (PEP 440 compatible) should comply with semver parsing.
381-
let version = Version::parse(
382-
iter.next()
383-
.ok_or(PyPiDownloadError::InvalidWheelName(s.to_string()))?,
384-
)
385-
.map_err(|_| PyPiDownloadError::InvalidVersion)?;
386375
let platform = PlatformTag::from_str(iter.next_back().unwrap())?;
387376

388377
// The remaining tags are not used for compatibility checks.
389378
// These binary wheels come with the executable within, so
390379
// we don't need to validate python version nor abi tags here.
391380
// optional build tag is not used in clang_format or clang_tidy wheel deployments
392381

393-
Ok(Self { version, platform })
382+
Ok(Self { platform })
394383
}
395384
}
396385

@@ -414,35 +403,67 @@ impl PyPiDownloader {
414403
clang_tool: &ClangTool,
415404
pypi_info: &PyPiProjectInfo,
416405
version: &VersionReq,
417-
) -> Result<(Version, PyPiReleaseInfo), PyPiDownloadError> {
406+
) -> Result<(Version, PyPiReleaseInfo, String), PyPiDownloadError> {
418407
let mut result = None;
419408

420409
for (ver_str, releases) in &pypi_info.releases {
421410
let ver = match Version::parse(ver_str) {
422-
Ok(v) => v,
423-
Err(_) => continue,
411+
Ok(v) => Version {
412+
// append a pre-release number to weight it properly against a build number (translated to prerelease number below)
413+
pre: semver::Prerelease::from_str("0").unwrap_or_default(),
414+
..v
415+
},
416+
Err(_) => {
417+
let mut components = ver_str.split('.');
418+
let count = components.clone().count();
419+
if count <= 3 {
420+
// should've parsed this normally, log warning and skip this version
421+
log::warn!("Skipping malformed version {ver_str} for {clang_tool} on PyPI");
422+
continue;
423+
} else {
424+
// take the first four components and treat them like a `major.minor.patch.build-number`
425+
let major: u64 = components.next().unwrap_or("0").parse().unwrap_or(0);
426+
let minor: u64 = components.next().unwrap_or("0").parse().unwrap_or(0);
427+
let patch: u64 = components.next().unwrap_or("0").parse().unwrap_or(0);
428+
let build: &str = components.next().unwrap_or("0");
429+
Version {
430+
major,
431+
minor,
432+
patch,
433+
pre: semver::Prerelease::from_str(build).unwrap_or_default(),
434+
build: semver::BuildMetadata::EMPTY,
435+
}
436+
}
437+
}
424438
};
425-
if version.matches(&ver) {
439+
// do not compare pre-release numbers since these are actually build numbers
440+
if version.matches(&Version {
441+
major: ver.major,
442+
minor: ver.minor,
443+
patch: ver.patch,
444+
pre: semver::Prerelease::default(),
445+
build: semver::BuildMetadata::EMPTY,
446+
}) {
426447
for release in releases {
427448
if !release.filename.ends_with(".whl") {
428449
continue;
429450
}
430451
let wheel_tags = WheelTags::from_str(&release.filename)?;
431452
if !release.yanked && wheel_tags.is_compatible_with_system() {
432453
log::debug!(
433-
"Found {clang_tool} (size: {}, digest: {:?}); {wheel_tags:?}",
454+
"Found {clang_tool} v{ver_str} (size: {}, digest: {:?}); {wheel_tags:?}",
434455
release.size,
435456
release.digests
436457
);
437-
if result.as_ref().is_none_or(|(v, _)| *v < ver) {
438-
result = Some((wheel_tags.version.clone(), release));
458+
if result.as_ref().is_none_or(|(v, _, _)| *v < ver) {
459+
result = Some((ver.clone(), release, ver_str.to_owned()));
439460
}
440461
}
441462
}
442463
}
443464
}
444465
result
445-
.map(|(a, b)| (a, b.to_owned()))
466+
.map(|(a, b, c)| (Version::new(a.major, a.minor, a.patch), b.to_owned(), c))
446467
.ok_or(PyPiDownloadError::NoVersionFound)
447468
}
448469

@@ -484,18 +505,21 @@ impl PyPiDownloader {
484505
directory: Option<&PathBuf>,
485506
) -> Result<PathBuf, PyPiDownloadError> {
486507
let info = Self::get_pypi_release_info(clang_tool).await?;
487-
let (ver, info) = Self::get_best_pypi_release(clang_tool, &info, version)?;
488-
let cached_filename = format!("{clang_tool}_{ver}.whl");
508+
let (ver, info, ver_str) = Self::get_best_pypi_release(clang_tool, &info, version)?;
509+
let cached_filename = format!("{clang_tool}_{ver_str}.whl",);
489510
let cached_dir = Self::get_cache_dir();
490511
let cached_wheel = cached_dir.join("pypi").join(&cached_filename);
491512
let file_lock = lock_path(&cached_wheel)?;
492513
if Self::is_cache_valid(&cached_wheel, None) {
493514
log::info!(
494-
"Using cached wheel for {clang_tool} version {ver} from {}",
515+
"Using cached wheel for {clang_tool} version {ver_str} from {}",
495516
cached_wheel.to_string_lossy()
496517
);
497518
} else {
498-
log::info!("Downloading {clang_tool} version {ver} from {}", info.url);
519+
log::info!(
520+
"Downloading {clang_tool} version {ver_str} from {}",
521+
info.url
522+
);
499523
download(&Url::parse(&info.url)?, &cached_wheel, 60).await?;
500524
}
501525
if let Some(digest) = info.digests.first() {

0 commit comments

Comments
 (0)