Skip to content

Commit bb1e9a2

Browse files
authored
Update preview installation of Python executables to be non-fatal (#14612)
Previously, if installation of executables into the bin directory failed we'd with a non-zero code. However, if we make this behavior the default we don't want it to be fatal. There's a `--bin` opt-in to _require_ successful executable installation and a `--no-bin` opt-out to silence the warning / opt-out of installation entirely. Part of #14296 — we need this before we can stabilize the behavior. In #14614 we do the same for writing entries to the Windows registry.
1 parent cd0d5d4 commit bb1e9a2

File tree

8 files changed

+212
-40
lines changed

8 files changed

+212
-40
lines changed

crates/uv-cli/src/lib.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4941,6 +4941,19 @@ pub struct PythonInstallArgs {
49414941
#[arg(long, short, env = EnvVars::UV_PYTHON_INSTALL_DIR)]
49424942
pub install_dir: Option<PathBuf>,
49434943

4944+
/// Install a Python executable into the `bin` directory.
4945+
///
4946+
/// This is the default behavior. If this flag is provided explicitly, uv will error if the
4947+
/// executable cannot be installed.
4948+
///
4949+
/// See `UV_PYTHON_BIN_DIR` to customize the target directory.
4950+
#[arg(long, overrides_with("no_bin"), hide = true)]
4951+
pub bin: bool,
4952+
4953+
/// Do not install a Python executable into the `bin` directory.
4954+
#[arg(long, overrides_with("bin"), conflicts_with("default"))]
4955+
pub no_bin: bool,
4956+
49444957
/// The Python version(s) to install.
49454958
///
49464959
/// If not provided, the requested Python version(s) will be read from the `UV_PYTHON`
@@ -5003,7 +5016,7 @@ pub struct PythonInstallArgs {
50035016
/// and `python`.
50045017
///
50055018
/// If multiple Python versions are requested, uv will exit with an error.
5006-
#[arg(long)]
5019+
#[arg(long, conflicts_with("no_bin"))]
50075020
pub default: bool,
50085021
}
50095022

crates/uv-python/src/windows_registry.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,13 @@ fn read_registry_entry(company: &str, tag: &str, tag_key: &Key) -> Option<Window
129129
pub enum ManagedPep514Error {
130130
#[error("Windows has an unknown pointer width for arch: `{_0}`")]
131131
InvalidPointerSize(Arch),
132+
#[error("Failed to write registry entry: {0}")]
133+
WriteError(#[from] windows_result::Error),
132134
}
133135

134136
/// Register a managed Python installation in the Windows registry following PEP 514.
135137
pub fn create_registry_entry(
136138
installation: &ManagedPythonInstallation,
137-
errors: &mut Vec<(PythonInstallationKey, anyhow::Error)>,
138139
) -> Result<(), ManagedPep514Error> {
139140
let pointer_width = match installation.key().arch().family().pointer_width() {
140141
Ok(PointerWidth::U32) => 32,
@@ -146,9 +147,7 @@ pub fn create_registry_entry(
146147
}
147148
};
148149

149-
if let Err(err) = write_registry_entry(installation, pointer_width) {
150-
errors.push((installation.key().clone(), err.into()));
151-
}
150+
write_registry_entry(installation, pointer_width)?;
152151

153152
Ok(())
154153
}

crates/uv/src/commands/python/install.rs

Lines changed: 112 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ impl Changelog {
135135
}
136136
}
137137

138+
#[derive(Debug, Clone, Copy)]
139+
enum InstallErrorKind {
140+
DownloadUnpack,
141+
Bin,
142+
#[cfg(windows)]
143+
Registry,
144+
}
145+
138146
/// Download and install Python versions.
139147
#[allow(clippy::fn_params_excessive_bools)]
140148
pub(crate) async fn install(
@@ -143,6 +151,7 @@ pub(crate) async fn install(
143151
targets: Vec<String>,
144152
reinstall: bool,
145153
upgrade: bool,
154+
bin: Option<bool>,
146155
force: bool,
147156
python_install_mirror: Option<String>,
148157
pypy_install_mirror: Option<String>,
@@ -432,12 +441,16 @@ pub(crate) async fn install(
432441
downloaded.push(installation.clone());
433442
}
434443
Err(err) => {
435-
errors.push((download.key().clone(), anyhow::Error::new(err)));
444+
errors.push((
445+
InstallErrorKind::DownloadUnpack,
446+
download.key().clone(),
447+
anyhow::Error::new(err),
448+
));
436449
}
437450
}
438451
}
439452

440-
let bin = if preview.is_enabled() {
453+
let bin_dir = if matches!(bin, Some(true)) || preview.is_enabled() {
441454
Some(python_executable_dir()?)
442455
} else {
443456
None
@@ -460,35 +473,46 @@ pub(crate) async fn install(
460473
continue;
461474
}
462475

463-
let bin = bin
476+
let bin_dir = bin_dir
464477
.as_ref()
465478
.expect("We should have a bin directory with preview enabled")
466479
.as_path();
467480

468481
let upgradeable = (default || is_default_install)
469482
|| requested_minor_versions.contains(&installation.key().version().python_version());
470483

471-
create_bin_links(
472-
installation,
473-
bin,
474-
reinstall,
475-
force,
476-
default,
477-
upgradeable,
478-
upgrade,
479-
is_default_install,
480-
first_request,
481-
&existing_installations,
482-
&installations,
483-
&mut changelog,
484-
&mut errors,
485-
preview,
486-
)?;
484+
if !matches!(bin, Some(false)) {
485+
create_bin_links(
486+
installation,
487+
bin_dir,
488+
reinstall,
489+
force,
490+
default,
491+
upgradeable,
492+
upgrade,
493+
is_default_install,
494+
first_request,
495+
&existing_installations,
496+
&installations,
497+
&mut changelog,
498+
&mut errors,
499+
preview,
500+
);
501+
}
487502

488503
if preview.is_enabled() {
489504
#[cfg(windows)]
490505
{
491-
uv_python::windows_registry::create_registry_entry(installation, &mut errors)?;
506+
match uv_python::windows_registry::create_registry_entry(installation) {
507+
Ok(()) => {}
508+
Err(err) => {
509+
errors.push((
510+
InstallErrorKind::Registry,
511+
installation.key().clone(),
512+
err.into(),
513+
));
514+
}
515+
}
492516
}
493517
}
494518
}
@@ -636,24 +660,47 @@ pub(crate) async fn install(
636660
}
637661
}
638662

639-
if preview.is_enabled() {
640-
let bin = bin
663+
if preview.is_enabled() && !matches!(bin, Some(false)) {
664+
let bin_dir = bin_dir
641665
.as_ref()
642666
.expect("We should have a bin directory with preview enabled")
643667
.as_path();
644-
warn_if_not_on_path(bin);
668+
warn_if_not_on_path(bin_dir);
645669
}
646670
}
647671

648672
if !errors.is_empty() {
649-
for (key, err) in errors
673+
// If there are only bin install errors and the user didn't opt-in, we're only going to warn
674+
let fatal = errors
675+
.iter()
676+
.all(|(kind, _, _)| matches!(kind, InstallErrorKind::Bin))
677+
&& bin.is_none();
678+
679+
for (kind, key, err) in errors
650680
.into_iter()
651-
.sorted_unstable_by(|(key_a, _), (key_b, _)| key_a.cmp(key_b))
681+
.sorted_unstable_by(|(_, key_a, _), (_, key_b, _)| key_a.cmp(key_b))
652682
{
683+
let (level, verb) = match kind {
684+
InstallErrorKind::DownloadUnpack => ("error".red().bold().to_string(), "install"),
685+
InstallErrorKind::Bin => {
686+
let level = match bin {
687+
None => "warning".yellow().bold().to_string(),
688+
Some(false) => continue,
689+
Some(true) => "error".red().bold().to_string(),
690+
};
691+
(level, "install executable for")
692+
}
693+
#[cfg(windows)]
694+
InstallErrorKind::Registry => (
695+
"error".red().bold().to_string(),
696+
"install registry entry for",
697+
),
698+
};
699+
653700
writeln!(
654701
printer.stderr(),
655-
"{}: Failed to install {}",
656-
"error".red().bold(),
702+
"{level}{} Failed to {verb} {}",
703+
":".bold(),
657704
key.green()
658705
)?;
659706
for err in err.chain() {
@@ -665,13 +712,20 @@ pub(crate) async fn install(
665712
)?;
666713
}
667714
}
715+
716+
if fatal {
717+
return Ok(ExitStatus::Success);
718+
}
719+
668720
return Ok(ExitStatus::Failure);
669721
}
670722

671723
Ok(ExitStatus::Success)
672724
}
673725

674726
/// Link the binaries of a managed Python installation to the bin directory.
727+
///
728+
/// This function is fallible, but errors are pushed to `errors` instead of being thrown.
675729
#[allow(clippy::fn_params_excessive_bools)]
676730
fn create_bin_links(
677731
installation: &ManagedPythonInstallation,
@@ -686,9 +740,9 @@ fn create_bin_links(
686740
existing_installations: &[ManagedPythonInstallation],
687741
installations: &[&ManagedPythonInstallation],
688742
changelog: &mut Changelog,
689-
errors: &mut Vec<(PythonInstallationKey, Error)>,
743+
errors: &mut Vec<(InstallErrorKind, PythonInstallationKey, Error)>,
690744
preview: PreviewMode,
691-
) -> Result<(), Error> {
745+
) {
692746
let targets =
693747
if (default || is_default_install) && first_request.matches_installation(installation) {
694748
vec![
@@ -773,6 +827,7 @@ fn create_bin_links(
773827
);
774828
} else {
775829
errors.push((
830+
InstallErrorKind::Bin,
776831
installation.key().clone(),
777832
anyhow::anyhow!(
778833
"Executable already exists at `{}` but is not managed by uv; use `--force` to replace it",
@@ -848,7 +903,17 @@ fn create_bin_links(
848903
}
849904

850905
// Replace the existing link
851-
fs_err::remove_file(&to)?;
906+
if let Err(err) = fs_err::remove_file(&to) {
907+
errors.push((
908+
InstallErrorKind::Bin,
909+
installation.key().clone(),
910+
anyhow::anyhow!(
911+
"Executable already exists at `{}` but could not be removed: {err}",
912+
to.simplified_display()
913+
),
914+
));
915+
continue;
916+
}
852917

853918
if let Some(existing) = existing {
854919
// Ensure we do not report installation of this executable for an existing
@@ -860,7 +925,18 @@ fn create_bin_links(
860925
.remove(&target);
861926
}
862927

863-
create_link_to_executable(&target, executable)?;
928+
if let Err(err) = create_link_to_executable(&target, executable) {
929+
errors.push((
930+
InstallErrorKind::Bin,
931+
installation.key().clone(),
932+
anyhow::anyhow!(
933+
"Failed to create link at `{}`: {err}",
934+
target.simplified_display()
935+
),
936+
));
937+
continue;
938+
}
939+
864940
debug!(
865941
"Updated executable at `{}` to {}",
866942
target.simplified_display(),
@@ -874,11 +950,14 @@ fn create_bin_links(
874950
.insert(target.clone());
875951
}
876952
Err(err) => {
877-
errors.push((installation.key().clone(), anyhow::Error::new(err)));
953+
errors.push((
954+
InstallErrorKind::Bin,
955+
installation.key().clone(),
956+
anyhow::Error::new(err),
957+
));
878958
}
879959
}
880960
}
881-
Ok(())
882961
}
883962

884963
pub(crate) fn format_executables(

crates/uv/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,7 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
14021402
args.targets,
14031403
args.reinstall,
14041404
upgrade,
1405+
args.bin,
14051406
args.force,
14061407
args.python_install_mirror,
14071408
args.pypy_install_mirror,
@@ -1430,6 +1431,7 @@ async fn run(mut cli: Cli) -> Result<ExitStatus> {
14301431
args.targets,
14311432
reinstall,
14321433
upgrade,
1434+
args.bin,
14331435
args.force,
14341436
args.python_install_mirror,
14351437
args.pypy_install_mirror,

crates/uv/src/settings.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,7 @@ pub(crate) struct PythonInstallSettings {
933933
pub(crate) targets: Vec<String>,
934934
pub(crate) reinstall: bool,
935935
pub(crate) force: bool,
936+
pub(crate) bin: Option<bool>,
936937
pub(crate) python_install_mirror: Option<String>,
937938
pub(crate) pypy_install_mirror: Option<String>,
938939
pub(crate) python_downloads_json_url: Option<String>,
@@ -961,6 +962,8 @@ impl PythonInstallSettings {
961962
install_dir,
962963
targets,
963964
reinstall,
965+
bin,
966+
no_bin,
964967
force,
965968
mirror: _,
966969
pypy_mirror: _,
@@ -973,6 +976,7 @@ impl PythonInstallSettings {
973976
targets,
974977
reinstall,
975978
force,
979+
bin: flag(bin, no_bin, "bin"),
976980
python_install_mirror: python_mirror,
977981
pypy_install_mirror: pypy_mirror,
978982
python_downloads_json_url,
@@ -992,6 +996,7 @@ pub(crate) struct PythonUpgradeSettings {
992996
pub(crate) pypy_install_mirror: Option<String>,
993997
pub(crate) python_downloads_json_url: Option<String>,
994998
pub(crate) default: bool,
999+
pub(crate) bin: Option<bool>,
9951000
}
9961001

9971002
impl PythonUpgradeSettings {
@@ -1013,6 +1018,7 @@ impl PythonUpgradeSettings {
10131018
args.python_downloads_json_url.or(python_downloads_json_url);
10141019
let force = false;
10151020
let default = false;
1021+
let bin = None;
10161022

10171023
let PythonUpgradeArgs {
10181024
install_dir,
@@ -1030,6 +1036,7 @@ impl PythonUpgradeSettings {
10301036
pypy_install_mirror: pypy_mirror,
10311037
python_downloads_json_url,
10321038
default,
1039+
bin,
10331040
}
10341041
}
10351042
}

crates/uv/tests/it/help.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,9 @@ fn help_subsubcommand() {
504504
505505
[env: UV_PYTHON_INSTALL_DIR=]
506506
507+
--no-bin
508+
Do not install a Python executable into the `bin` directory
509+
507510
--mirror <MIRROR>
508511
Set the URL to use as the source for downloading Python installations.
509512
@@ -790,6 +793,8 @@ fn help_flag_subsubcommand() {
790793
Options:
791794
-i, --install-dir <INSTALL_DIR>
792795
The directory to store the Python installation in [env: UV_PYTHON_INSTALL_DIR=]
796+
--no-bin
797+
Do not install a Python executable into the `bin` directory
793798
--mirror <MIRROR>
794799
Set the URL to use as the source for downloading Python installations [env:
795800
UV_PYTHON_INSTALL_MIRROR=]

0 commit comments

Comments
 (0)