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

Rollback pyproject.toml changes on all errors #7022

Merged
merged 2 commits into from
Sep 4, 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
2 changes: 1 addition & 1 deletion crates/uv-workspace/src/pyproject_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ impl PyProjectTomlMut {
.doc()?
.entry("optional-dependencies")
.or_insert(Item::Table(Table::new()))
.as_table_mut()
.as_table_like_mut()
.ok_or(Error::MalformedDependencies)?;

let group = optional_dependencies
Expand Down
141 changes: 80 additions & 61 deletions crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use crate::commands::project::ProjectError;
use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter};
use crate::commands::{pip, project, ExitStatus, SharedState};
use crate::printer::Printer;
use crate::settings::ResolverInstallerSettings;
use crate::settings::{ResolverInstallerSettings, ResolverInstallerSettingsRef};

/// Add one or more packages to the project requirements.
#[allow(clippy::fn_params_excessive_bools)]
Expand Down Expand Up @@ -479,22 +479,27 @@ pub(crate) async fn add(
return Ok(ExitStatus::Success);
}

let existing = project.pyproject_toml();
// Store the content prior to any modifications.
let existing = project.pyproject_toml().as_ref().to_vec();
let root = project.root().to_path_buf();

// Update the `pypackage.toml` in-memory.
let mut project = project
.clone()
.with_pyproject_toml(toml::from_str(&content)?)
.context("Failed to update `pyproject.toml`")?;

// Lock and sync the environment, if necessary.
let mut lock = match project::lock::do_safe_lock(
let project = project
.with_pyproject_toml(toml::from_str(&content).map_err(ProjectError::TomlParse)?)
.ok_or(ProjectError::TomlUpdate)?;

match lock_and_sync(
project,
&mut toml,
&edits,
&venv,
state,
locked,
frozen,
project.workspace(),
venv.interpreter(),
settings.as_ref().into(),
Box::new(DefaultResolveLogger),
no_sync,
&dependency_type,
raw_sources,
settings.as_ref(),
connectivity,
concurrency,
native_tls,
Expand All @@ -503,7 +508,7 @@ pub(crate) async fn add(
)
.await
{
Ok(result) => result.into_lock(),
Ok(()) => Ok(ExitStatus::Success),
Err(ProjectError::Operation(pip::operations::Error::Resolve(
uv_resolver::ResolveError::NoSolution(err),
))) => {
Expand All @@ -513,13 +518,56 @@ pub(crate) async fn add(

// Revert the changes to the `pyproject.toml`, if necessary.
if modified {
fs_err::write(project.root().join("pyproject.toml"), existing)?;
fs_err::write(root.join("pyproject.toml"), existing)?;
}

return Ok(ExitStatus::Failure);
Ok(ExitStatus::Failure)
}
Err(err) => return Err(err.into()),
};
Err(err) => {
// Revert the changes to the `pyproject.toml`, if necessary.
if modified {
fs_err::write(root.join("pyproject.toml"), existing)?;
}
Err(err.into())
}
}
}

/// Re-lock and re-sync the project after a series of edits.
#[allow(clippy::fn_params_excessive_bools)]
async fn lock_and_sync(
mut project: VirtualProject,
toml: &mut PyProjectTomlMut,
edits: &[DependencyEdit<'_>],
venv: &PythonEnvironment,
state: SharedState,
locked: bool,
frozen: bool,
no_sync: bool,
dependency_type: &DependencyType,
raw_sources: bool,
settings: ResolverInstallerSettingsRef<'_>,
connectivity: Connectivity,
concurrency: Concurrency,
native_tls: bool,
cache: &Cache,
printer: Printer,
) -> Result<(), ProjectError> {
let mut lock = project::lock::do_safe_lock(
locked,
frozen,
project.workspace(),
venv.interpreter(),
settings.into(),
Box::new(DefaultResolveLogger),
connectivity,
concurrency,
native_tls,
cache,
printer,
)
.await?
.into_lock();

// Avoid modifying the user request further if `--raw-sources` is set.
if !raw_sources {
Expand All @@ -543,7 +591,7 @@ pub(crate) async fn add(

// If any of the requirements were added without version specifiers, add a lower bound.
let mut modified = false;
for edit in &edits {
for edit in edits {
// Only set a minimum version for newly-added dependencies (as opposed to updates).
let ArrayEdit::Add(index) = &edit.edit else {
continue;
Expand Down Expand Up @@ -599,49 +647,31 @@ pub(crate) async fn add(

// Update the `pypackage.toml` in-memory.
project = project
.clone()
.with_pyproject_toml(toml::from_str(&content)?)
.context("Failed to update `pyproject.toml`")?;
.with_pyproject_toml(toml::from_str(&content).map_err(ProjectError::TomlParse)?)
.ok_or(ProjectError::TomlUpdate)?;

// If the file was modified, we have to lock again, though the only expected change is
// the addition of the minimum version specifiers.
lock = match project::lock::do_safe_lock(
lock = project::lock::do_safe_lock(
locked,
frozen,
project.workspace(),
venv.interpreter(),
settings.as_ref().into(),
settings.into(),
Box::new(SummaryResolveLogger),
connectivity,
concurrency,
native_tls,
cache,
printer,
)
.await
{
Ok(result) => result.into_lock(),
Err(ProjectError::Operation(pip::operations::Error::Resolve(
uv_resolver::ResolveError::NoSolution(err),
))) => {
let header = err.header();
let report = miette::Report::new(WithHelp { header, cause: err, help: Some("If this is intentional, run `uv add --frozen` to skip the lock and sync steps.") });
anstream::eprint!("{report:?}");

// Revert the changes to the `pyproject.toml`, if necessary.
if modified {
fs_err::write(project.root().join("pyproject.toml"), existing)?;
}

return Ok(ExitStatus::Failure);
}
Err(err) => return Err(err.into()),
};
.await?
.into_lock();
}
}

if no_sync {
return Ok(ExitStatus::Success);
return Ok(());
}

// Sync the environment.
Expand All @@ -663,19 +693,15 @@ pub(crate) async fn add(
}
};

// Initialize any shared state.
let state = SharedState::default();
let install_options = InstallOptions::default();

if let Err(err) = project::sync::do_sync(
project::sync::do_sync(
InstallTarget::from(&project),
&venv,
venv,
&lock,
&extras,
dev,
install_options,
InstallOptions::default(),
Modifications::Sufficient,
settings.as_ref().into(),
settings.into(),
&state,
Box::new(DefaultInstallLogger),
connectivity,
Expand All @@ -684,16 +710,9 @@ pub(crate) async fn add(
cache,
printer,
)
.await
{
// Revert the changes to the `pyproject.toml`, if necessary.
if modified {
fs_err::write(project.root().join("pyproject.toml"), existing)?;
}
return Err(err.into());
}
.await?;

Ok(ExitStatus::Success)
Ok(())
}

/// Resolves the source for a requirement and processes it into a PEP 508 compliant format.
Expand Down
9 changes: 9 additions & 0 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ pub(crate) enum ProjectError {
#[error("Environment marker is empty")]
EmptyEnvironment,

#[error("Failed to parse `pyproject.toml`")]
TomlParse(#[source] toml::de::Error),

#[error("Failed to update `pyproject.toml`")]
TomlUpdate,

#[error(transparent)]
Python(#[from] uv_python::Error),

Expand Down Expand Up @@ -120,6 +126,9 @@ pub(crate) enum ProjectError {
#[error(transparent)]
NamedRequirements(#[from] uv_requirements::NamedRequirementsError),

#[error(transparent)]
PyprojectMut(#[from] uv_workspace::pyproject_mut::Error),

#[error(transparent)]
Fmt(#[from] std::fmt::Error),

Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,7 @@ impl From<ResolverOptions> for ResolverSettings {
}
}

#[derive(Debug, Clone)]
#[derive(Debug, Clone, Copy)]
pub(crate) struct ResolverInstallerSettingsRef<'a> {
pub(crate) index_locations: &'a IndexLocations,
pub(crate) index_strategy: IndexStrategy,
Expand Down
15 changes: 10 additions & 5 deletions crates/uv/tests/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1394,13 +1394,16 @@ fn add_remove_inline_optional() -> Result<()> {
"#})?;

uv_snapshot!(context.filters(), context.add().arg("typing-extensions").arg("--optional=types"), @r###"
success: false
exit_code: 2
success: true
exit_code: 0
----- stdout -----

----- stderr -----
Resolved 5 packages in [TIME]
error: Dependencies in `pyproject.toml` are malformed
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
+ project==0.1.0 (from file://[TEMP_DIR]/)
+ typing-extensions==4.10.0
"###);

let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;
Expand All @@ -1418,7 +1421,7 @@ fn add_remove_inline_optional() -> Result<()> {
optional-dependencies = { io = [
"anyio==3.7.0",
], types = [
"typing-extensions",
"typing-extensions>=4.10.0",
] }

[build-system]
Expand All @@ -1436,11 +1439,13 @@ fn add_remove_inline_optional() -> Result<()> {
----- stderr -----
Resolved 4 packages in [TIME]
Prepared 4 packages in [TIME]
Uninstalled 2 packages in [TIME]
Installed 4 packages in [TIME]
+ anyio==3.7.0
+ idna==3.6
+ project==0.1.0 (from file://[TEMP_DIR]/)
~ project==0.1.0 (from file://[TEMP_DIR]/)
+ sniffio==1.3.1
- typing-extensions==4.10.0
"###);

let pyproject_toml = fs_err::read_to_string(context.temp_dir.join("pyproject.toml"))?;
Expand Down
Loading