Report project name instead of root when using pyproject.toml files (#295)

Part of https://github.com/astral-sh/puffin/issues/214

Adds a `project: Option<PackageName>` to the `Manifest`, `Resolver`, and
`RequirementsSpecification`.
To populate an optional `name` for `PubGubPackage::Root`.

I'll work on removing the version number next.

Should we consider using the parent directory name when a
`pyproject.toml` file is not present?
This commit is contained in:
Zanie Blue 2023-11-03 10:22:10 -05:00 committed by GitHub
parent e008c43f29
commit e1382cc747
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 123 additions and 9 deletions

View file

@ -44,6 +44,7 @@ pub(crate) async fn pip_compile(
// Read all requirements from the provided sources.
let RequirementsSpecification {
project,
requirements,
constraints,
extras: used_extras,
@ -84,6 +85,7 @@ pub(crate) async fn pip_compile(
preferences,
resolution_mode,
prerelease_mode,
project,
);
// Detect the current Python interpreter.

View file

@ -33,6 +33,7 @@ pub(crate) async fn pip_sync(
) -> Result<ExitStatus> {
// Read all requirements from the provided sources.
let RequirementsSpecification {
project: _,
requirements,
constraints: _,
extras: _,

View file

@ -22,6 +22,7 @@ pub(crate) async fn pip_uninstall(
// Read all requirements from the provided sources.
let RequirementsSpecification {
project: _,
requirements,
constraints: _,
extras: _,

View file

@ -8,7 +8,7 @@ use anyhow::{Context, Result};
use fs_err as fs;
use pep508_rs::Requirement;
use puffin_normalize::ExtraName;
use puffin_normalize::{ExtraName, PackageName};
use puffin_package::requirements_txt::RequirementsTxt;
#[derive(Debug)]
@ -58,6 +58,8 @@ impl ExtrasSpecification<'_> {
#[derive(Debug, Default)]
pub(crate) struct RequirementsSpecification {
/// The name of the project specifying requirements.
pub(crate) project: Option<PackageName>,
/// The requirements for the project.
pub(crate) requirements: Vec<Requirement>,
/// The constraints for the project.
@ -77,6 +79,7 @@ impl RequirementsSpecification {
let requirement = Requirement::from_str(name)
.with_context(|| format!("Failed to parse `{name}`"))?;
Self {
project: None,
requirements: vec![requirement],
constraints: vec![],
extras: HashSet::new(),
@ -85,6 +88,7 @@ impl RequirementsSpecification {
RequirementsSource::RequirementsTxt(path) => {
let requirements_txt = RequirementsTxt::parse(path, std::env::current_dir()?)?;
Self {
project: None,
requirements: requirements_txt
.requirements
.into_iter()
@ -100,6 +104,7 @@ impl RequirementsSpecification {
.with_context(|| format!("Failed to read `{}`", path.display()))?;
let mut used_extras = HashSet::new();
let mut requirements = Vec::new();
let mut project_name = None;
if let Some(project) = pyproject_toml.project {
requirements.extend(project.dependencies.unwrap_or_default());
// Include any optional dependencies specified in `extras`
@ -114,9 +119,12 @@ impl RequirementsSpecification {
}
}
}
// Parse the project name
project_name = Some(PackageName::new(project.name));
}
Self {
project: project_name,
requirements,
constraints: vec![],
extras: used_extras,
@ -141,6 +149,11 @@ impl RequirementsSpecification {
spec.requirements.extend(source.requirements);
spec.constraints.extend(source.constraints);
spec.extras.extend(source.extras);
// Use the first project name discovered
if spec.project.is_none() {
spec.project = source.project;
}
}
// Read all constraints, treating both requirements _and_ constraints as constraints.

View file

@ -1047,3 +1047,51 @@ optional-dependencies.bar = [
Ok(())
}
/// Compile requirements that cannot be solved due to conflict in a `pyproject.toml` fil;e.
#[test]
fn compile_unsolvable_requirements() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = temp_dir.child(".venv");
Command::new(get_cargo_bin(BIN_NAME))
.arg("venv")
.arg(venv.as_os_str())
.arg("--cache-dir")
.arg(cache_dir.path())
.current_dir(&temp_dir)
.assert()
.success();
venv.assert(predicates::path::is_dir());
let pyproject_toml = temp_dir.child("pyproject.toml");
pyproject_toml.touch()?;
pyproject_toml.write_str(
r#"[build-system]
requires = ["setuptools", "wheel"]
[project]
name = "my-project"
dependencies = ["django==5.0b1", "django==5.0a1"]
"#,
)?;
insta::with_settings!({
filters => vec![
(r"\d(ms|s)", "[TIME]"),
(r"# .* pip-compile", "# [BIN_PATH] pip-compile"),
(r"--cache-dir .*", "--cache-dir [CACHE_DIR]"),
]
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-compile")
.arg("pyproject.toml")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.current_dir(&temp_dir));
});
Ok(())
}

View file

@ -0,0 +1,20 @@
---
source: crates/puffin-cli/tests/pip_compile.rs
info:
program: puffin
args:
- pip-compile
- pyproject.toml
- "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpfuaPhl
env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpLxUB5I/.venv
---
success: false
exit_code: 1
----- stdout -----
----- stderr -----
× No solution found when resolving dependencies:
╰─▶ my-project 0a0.dev0 depends on django ∅

View file

@ -75,6 +75,7 @@ impl BuildContext for BuildDispatch {
Vec::default(),
ResolutionMode::default(),
PreReleaseMode::default(),
None, // TODO(zanieb): We may want to provide a project name here
),
self.interpreter_info.markers(),
&tags,

View file

@ -1,4 +1,5 @@
use pep508_rs::Requirement;
use puffin_normalize::PackageName;
use crate::prerelease_mode::PreReleaseMode;
use crate::resolution_mode::ResolutionMode;
@ -11,6 +12,7 @@ pub struct Manifest {
pub(crate) preferences: Vec<Requirement>,
pub(crate) resolution_mode: ResolutionMode,
pub(crate) prerelease_mode: PreReleaseMode,
pub(crate) project: Option<PackageName>,
}
impl Manifest {
@ -20,6 +22,7 @@ impl Manifest {
preferences: Vec<Requirement>,
resolution_mode: ResolutionMode,
prerelease_mode: PreReleaseMode,
project: Option<PackageName>,
) -> Self {
Self {
requirements,
@ -27,6 +30,7 @@ impl Manifest {
preferences,
resolution_mode,
prerelease_mode,
project,
}
}
}

View file

@ -13,7 +13,7 @@ use puffin_normalize::{ExtraName, PackageName};
#[derive(Debug, Clone, Eq, Derivative)]
#[derivative(PartialEq, Hash)]
pub enum PubGrubPackage {
Root,
Root(Option<PackageName>),
Package(
PackageName,
Option<ExtraName>,
@ -27,7 +27,13 @@ pub enum PubGrubPackage {
impl std::fmt::Display for PubGrubPackage {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
PubGrubPackage::Root => write!(f, "root"),
PubGrubPackage::Root(name) => {
if let Some(name) = name {
write!(f, "{}", name.as_ref())
} else {
write!(f, "root")
}
}
PubGrubPackage::Package(name, None, ..) => write!(f, "{name}"),
PubGrubPackage::Package(name, Some(extra), ..) => {
write!(f, "{name}[{extra}]")

View file

@ -19,7 +19,7 @@ impl PubGrubPriorities {
/// Return the priority of the given package, if it exists.
pub(crate) fn get(&self, package: &PubGrubPackage) -> Option<PubGrubPriority> {
match package {
PubGrubPackage::Root => Some(Reverse(0)),
PubGrubPackage::Root(_) => Some(Reverse(0)),
PubGrubPackage::Package(name, _, _) => self
.0
.get(name)

View file

@ -37,6 +37,7 @@ use crate::pubgrub::{PubGrubPackage, PubGrubPriorities, PubGrubVersion, MIN_VERS
use crate::resolution::Graph;
pub struct Resolver<'a, Context: BuildContext + Sync> {
project: Option<PackageName>,
requirements: Vec<Requirement>,
constraints: Vec<Requirement>,
markers: &'a MarkerEnvironment,
@ -60,6 +61,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
Self {
selector: CandidateSelector::from(&manifest),
index: Arc::new(Index::default()),
project: manifest.project,
requirements: manifest.requirements,
constraints: manifest.constraints,
markers,
@ -115,7 +117,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
&self,
request_sink: &futures::channel::mpsc::UnboundedSender<Request>,
) -> Result<Graph, ResolveError> {
let root = PubGrubPackage::Root;
let root = PubGrubPackage::Root(self.project.clone());
// Keep track of the packages for which we've requested metadata.
let mut in_flight = InFlight::default();
@ -284,7 +286,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
request_sink: &futures::channel::mpsc::UnboundedSender<Request>,
) -> Result<(), ResolveError> {
match package {
PubGrubPackage::Root => {}
PubGrubPackage::Root(_) => {}
PubGrubPackage::Package(package_name, _extra, None) => {
// Emit a request to fetch the metadata for this package.
if in_flight.insert_package(package_name) {
@ -371,7 +373,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
request_sink: &futures::channel::mpsc::UnboundedSender<Request>,
) -> Result<Option<PubGrubVersion>, ResolveError> {
return match package {
PubGrubPackage::Root => Ok(Some(MIN_VERSION.clone())),
PubGrubPackage::Root(_) => Ok(Some(MIN_VERSION.clone())),
PubGrubPackage::Package(package_name, _extra, Some(url)) => {
debug!("Searching for a compatible version of {package_name} @ {url} ({range})",);
@ -461,7 +463,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
request_sink: &futures::channel::mpsc::UnboundedSender<Request>,
) -> Result<Dependencies, ResolveError> {
match package {
PubGrubPackage::Root => {
PubGrubPackage::Root(_) => {
let mut constraints =
DependencyConstraints::<PubGrubPackage, Range<PubGrubVersion>>::default();
@ -754,7 +756,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> {
fn on_progress(&self, package: &PubGrubPackage, version: &PubGrubVersion) {
if let Some(reporter) = self.reporter.as_ref() {
match package {
PubGrubPackage::Root => {}
PubGrubPackage::Root(_) => {}
PubGrubPackage::Package(package_name, extra, Some(url)) => {
reporter.on_progress(package_name, extra.as_ref(), VersionOrUrl::Url(url));
}

View file

@ -70,6 +70,7 @@ async fn black() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -92,6 +93,7 @@ async fn black_colorama() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -114,6 +116,7 @@ async fn black_python_310() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_310, &TAGS_310, &client, &DummyContext);
@ -138,6 +141,7 @@ async fn black_mypy_extensions() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -162,6 +166,7 @@ async fn black_mypy_extensions_extra() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -186,6 +191,7 @@ async fn black_flake8() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -208,6 +214,7 @@ async fn black_lowest() -> Result<()> {
vec![],
ResolutionMode::Lowest,
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -230,6 +237,7 @@ async fn black_lowest_direct() -> Result<()> {
vec![],
ResolutionMode::LowestDirect,
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -252,6 +260,7 @@ async fn black_respect_preference() -> Result<()> {
vec![Requirement::from_str("black==23.9.0").unwrap()],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -274,6 +283,7 @@ async fn black_ignore_preference() -> Result<()> {
vec![Requirement::from_str("black==23.9.2").unwrap()],
ResolutionMode::default(),
PreReleaseMode::default(),
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -296,6 +306,7 @@ async fn black_disallow_prerelease() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::Disallow,
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -318,6 +329,7 @@ async fn black_allow_prerelease_if_necessary() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::IfNecessary,
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -340,6 +352,7 @@ async fn pylint_disallow_prerelease() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::Disallow,
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -362,6 +375,7 @@ async fn pylint_allow_prerelease() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::Allow,
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -387,6 +401,7 @@ async fn pylint_allow_explicit_prerelease_without_marker() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::Explicit,
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);
@ -412,6 +427,7 @@ async fn pylint_allow_explicit_prerelease_with_marker() -> Result<()> {
vec![],
ResolutionMode::default(),
PreReleaseMode::Explicit,
None,
);
let resolver = Resolver::new(manifest, &MARKERS_311, &TAGS_311, &client, &DummyContext);