Skip to content
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

Fix requirements parsing with index URL option #1251

Merged
merged 13 commits into from
Oct 11, 2023
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [Unreleased]

### Fixed
- Pip requirements.txt parser failing with third-party registries

## [5.7.2] - 2023-10-10

### Fixed
Expand Down
35 changes: 29 additions & 6 deletions lockfile/src/parsers/pypi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ use nom::Err as NomErr;
use phylum_types::types::package::PackageType;

use crate::parsers::{self, IResult};
use crate::{Package, PackageVersion};
use crate::{Package, PackageVersion, ThirdPartyVersion};

pub fn parse(mut input: &str) -> IResult<&str, Vec<Package>> {
let mut pkgs = Vec::new();

let mut registry = None;
while !input.is_empty() {
// Get the next line.
let (new_input, line) = line(input)?;
let (new_input, line) = line(input, &mut registry)?;
input = new_input;

// Ignore empty lines.
Expand All @@ -30,15 +31,15 @@ pub fn parse(mut input: &str) -> IResult<&str, Vec<Package>> {
let (_, line) = alt((take_until(" #"), rest))(line)?;

// Parse dependency.
let (_, pkg) = package(line)?;
let (_, pkg) = package(line, registry)?;
pkgs.push(pkg);
}

Ok((input, pkgs))
}

/// Parse one line in the lockfile.
fn line(input: &str) -> IResult<&str, &str> {
fn line<'a>(input: &'a str, registry: &mut Option<&'a str>) -> IResult<&'a str, &'a str> {
// Take everything until the next newline.
//
// This takes line continuation characters into account.
Expand All @@ -52,10 +53,26 @@ fn line(input: &str) -> IResult<&str, &str> {
line = "";
}

// Ignore index config options.
//
// Since `ThirdPartyVersion` only allows a single registry, we only record the
// primary one.
if let Some(index_url) = line
.strip_prefix("--index-url")
.and_then(|line| line.strip_prefix(['=', ' ']))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure, I'd assume --index-url = https://whatever.com is not valid?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct...that form is not valid:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other weird edge cases... But I don't want to waste a bunch of time on them...

> cat req.txt
--index-url https://pypi.org/simple AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

pyyaml

> pip install --dry-run -r req.txt
Collecting pyyaml
  Downloading PyYAML-6.0.1-cp310-cp310-macosx_11_0_arm64.whl (169 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 169.3/169.3 kB 2.1 MB/s eta 0:00:00
Would install PyYAML-6.0.1

.or_else(|| line.strip_prefix("-i"))
{
*registry = Some(index_url.trim());
line = "";
}
if line.starts_with("--extra-index-url") {
line = "";
}
cd-work marked this conversation as resolved.
Show resolved Hide resolved

Ok((input, line))
}

fn package(input: &str) -> IResult<&str, Package> {
fn package<'a>(input: &'a str, registry: Option<&str>) -> IResult<&'a str, Package> {
// Ignore everything after `;`.
let (_, input) = alt((take_until(";"), rest))(input)?;

Expand Down Expand Up @@ -85,7 +102,13 @@ fn package(input: &str) -> IResult<&str, Package> {

// Parse first-party dependencies.
let (input, version) = package_version(input)?;
let version = PackageVersion::FirstParty(version.trim().into());
let version = match registry {
Some(registry) => PackageVersion::ThirdParty(ThirdPartyVersion {
version: version.trim().into(),
registry: registry.into(),
}),
kylewillmon marked this conversation as resolved.
Show resolved Hide resolved
None => PackageVersion::FirstParty(version.trim().into()),
};

// Ensure line is empty after the dependency.
line_done(input)?;
Expand Down
18 changes: 17 additions & 1 deletion lockfile/src/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ mod tests {
let pkgs = PyRequirements
.parse(include_str!("../../tests/fixtures/requirements-locked.txt"))
.unwrap();
assert_eq!(pkgs.len(), 12);
assert_eq!(pkgs.len(), 14);

let expected_pkgs = [
Package {
Expand Down Expand Up @@ -301,6 +301,22 @@ mod tests {
version: PackageVersion::Path(Some("/tmp/editable".into())),
package_type: PackageType::PyPi,
},
Package {
name: "other-registry-a".into(),
version: PackageVersion::ThirdParty(ThirdPartyVersion {
registry: "https://mirror1.phylum.io/simple/".into(),
version: "3.2.1".into(),
}),
package_type: PackageType::PyPi,
},
Package {
name: "other-registry".into(),
version: PackageVersion::ThirdParty(ThirdPartyVersion {
registry: "https://mirror2.phylum.io/simple/".into(),
version: "1.2.3".into(),
}),
package_type: PackageType::PyPi,
},
];

for expected_pkg in expected_pkgs {
Expand Down
7 changes: 7 additions & 0 deletions tests/fixtures/requirements-locked.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,10 @@ tomli @ https://files.pythonhosted.org/packages/97/75/10a9ebee3fd790d20926a90a25
-e git+ssh://git@github.com/phylum-dev/phylum-ci.git@7d6d859ad368d1ab0a933f24679e3d3c08a40eac#egg=phylum

-e /tmp/editable ; python_version >= "3.7" and python_version < "3.12"

--index-url https://unused.phylum.io/simple/
--index-url=https://mirror1.phylum.io/simple/
other-registry-a==3.2.1
-ihttps://mirror2.phylum.io/simple/
--extra-index-url=https://mirror3.phylum.io/simple/
other-registry==1.2.3