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

Add support for pip-compile's --unsafe-package flag #1889

Merged
merged 2 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions crates/uv-resolver/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,13 @@ impl ResolutionGraph {
self.petgraph.node_count() == 0
}

/// Returns `true` if the graph contains the given package.
pub fn contains(&self, name: &PackageName) -> bool {
self.petgraph
.node_indices()
.any(|index| self.petgraph[index].name() == name)
}

/// Return the [`Diagnostic`]s that were encountered while building the graph.
pub fn diagnostics(&self) -> &[Diagnostic] {
&self.diagnostics
Expand All @@ -273,6 +280,8 @@ impl ResolutionGraph {
pub struct DisplayResolutionGraph<'a> {
/// The underlying graph.
resolution: &'a ResolutionGraph,
/// The packages to exclude from the output.
no_emit_packages: &'a [PackageName],
/// Whether to include hashes in the output.
show_hashes: bool,
/// Whether to include annotations in the output, to indicate which dependency or dependencies
Expand All @@ -285,20 +294,22 @@ pub struct DisplayResolutionGraph<'a> {

impl<'a> From<&'a ResolutionGraph> for DisplayResolutionGraph<'a> {
fn from(resolution: &'a ResolutionGraph) -> Self {
Self::new(resolution, false, true, AnnotationStyle::default())
Self::new(resolution, &[], false, true, AnnotationStyle::default())
}
}

impl<'a> DisplayResolutionGraph<'a> {
/// Create a new [`DisplayResolutionGraph`] for the given graph.
pub fn new(
underlying: &'a ResolutionGraph,
no_emit_packages: &'a [PackageName],
show_hashes: bool,
include_annotations: bool,
annotation_style: AnnotationStyle,
) -> DisplayResolutionGraph<'a> {
Self {
resolution: underlying,
no_emit_packages,
show_hashes,
include_annotations,
annotation_style,
Expand Down Expand Up @@ -348,15 +359,19 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
.resolution
.petgraph
.node_indices()
.map(|index| {
.filter_map(|index| {
let dist = &self.resolution.petgraph[index];
let name = dist.name();
if self.no_emit_packages.contains(name) {
return None;
}

let node = if let Some((editable, _)) = self.resolution.editables.get(name) {
Node::Editable(name, editable)
} else {
Node::Distribution(name, dist)
};
(index, node)
Some((index, node))
})
.collect::<Vec<_>>();

Expand Down
19 changes: 19 additions & 0 deletions crates/uv/src/commands/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub(crate) async fn pip_compile(
dependency_mode: DependencyMode,
upgrade: Upgrade,
generate_hashes: bool,
no_emit_packages: Vec<PackageName>,
include_annotations: bool,
include_header: bool,
include_index_url: bool,
Expand Down Expand Up @@ -383,12 +384,30 @@ pub(crate) async fn pip_compile(
"{}",
DisplayResolutionGraph::new(
&resolution,
&no_emit_packages,
generate_hashes,
include_annotations,
annotation_style,
)
)?;

// If any "unsafe" packages were excluded, notify the user.
let excluded = no_emit_packages
.into_iter()
.filter(|name| resolution.contains(name))
.collect::<Vec<_>>();
if !excluded.is_empty() {
writeln!(writer)?;
writeln!(
writer,
"{}",
"# The following packages were included while generating the resolution:".green()
Copy link
Contributor

@hauntsaninja hauntsaninja Feb 23, 2024

Choose a reason for hiding this comment

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

Thank you for implementing the feature!

nit: I was confused by "include" here, maybe The following packages were omitted from the result: (although maybe omit vs no-emit confuses folks too, so maybe just "excluded")

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea -- changed in #1935.

)?;
for package in excluded {
writeln!(writer, "# {package}")?;
}
}

Ok(ExitStatus::Success)
}

Expand Down
9 changes: 0 additions & 9 deletions crates/uv/src/compat/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ pub(crate) struct PipCompileCompatArgs {
#[clap(long, hide = true)]
no_emit_trusted_host: bool,

#[clap(long, hide = true)]
unsafe_package: Vec<String>,

#[clap(long, hide = true)]
config: Option<String>,

Expand Down Expand Up @@ -171,12 +168,6 @@ impl CompatArgs for PipCompileCompatArgs {
);
}

if !self.unsafe_package.is_empty() {
return Err(anyhow!(
"pip-compile's `--unsafe-package` is not supported."
));
}

if self.config.is_some() {
return Err(anyhow!(
"pip-compile's `--config` is unsupported (uv does not use a configuration file)."
Expand Down
6 changes: 6 additions & 0 deletions crates/uv/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,11 @@ struct PipCompileArgs {
#[arg(long, value_parser = date_or_datetime, hide = true)]
exclude_newer: Option<DateTime<Utc>>,

/// Specify a package to omit from the output resolution. Its dependencies will still be
/// included in the resolution. Equivalent to pip-compile's `--unsafe-package` option.
#[clap(long, alias = "unsafe-package")]
no_emit_package: Vec<PackageName>,

/// Include `--index-url` and `--extra-index-url` entries in the generated output file.
#[clap(long, hide = true)]
emit_index_url: bool,
Expand Down Expand Up @@ -904,6 +909,7 @@ async fn run() -> Result<ExitStatus> {
dependency_mode,
upgrade,
args.generate_hashes,
args.no_emit_package,
!args.no_annotate,
!args.no_header,
args.emit_index_url,
Expand Down
43 changes: 43 additions & 0 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4225,3 +4225,46 @@ fn override_with_incompatible_constraint() -> Result<()> {

Ok(())
}

/// Resolve a package, marking a dependency as unsafe.
#[test]
fn unsafe_package() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str("flask")?;

uv_snapshot!(context.compile()
.arg("requirements.in")
.arg("--unsafe-package")
.arg("jinja2")
.arg("--unsafe-package")
.arg("pydantic"), @r###"
success: true
exit_code: 0
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z requirements.in --unsafe-package jinja2 --unsafe-package pydantic
blinker==1.7.0
# via flask
click==8.1.7
# via flask
flask==3.0.0
itsdangerous==2.1.2
# via flask
markupsafe==2.1.3
# via
# jinja2
# werkzeug
werkzeug==3.0.1
# via flask

# The following packages were included while generating the resolution:
# jinja2

----- stderr -----
Resolved 7 packages in [TIME]
"###
);

Ok(())
}