Skip to content

Commit 841c430

Browse files
charliermarshzanieb
authored andcommitted
Reject non-PEP 751 TOML files in install commands (#13120)
If you pass a TOML file to `uv pip install` that isn't recognized, we should just reject it instead of assuming `requirements.txt`. I just don't see a real case where it's better to let the command proceed.
1 parent 7fc9e33 commit 841c430

File tree

3 files changed

+136
-73
lines changed

3 files changed

+136
-73
lines changed

crates/uv-requirements/src/sources.rs

Lines changed: 86 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1+
use std::ffi::OsStr;
12
use std::path::{Path, PathBuf};
23

34
use anyhow::{Context, Result};
45
use console::Term;
56

67
use uv_fs::{Simplified, CWD};
78
use uv_requirements_txt::RequirementsTxtRequirement;
8-
use uv_warnings::warn_user;
99

1010
#[derive(Debug, Clone)]
1111
pub enum RequirementsSource {
@@ -32,89 +32,127 @@ pub enum RequirementsSource {
3232
impl RequirementsSource {
3333
/// Parse a [`RequirementsSource`] from a [`PathBuf`]. The file type is determined by the file
3434
/// extension.
35-
pub fn from_requirements_file(path: PathBuf) -> Self {
35+
pub fn from_requirements_file(path: PathBuf) -> Result<Self> {
3636
if path.ends_with("pyproject.toml") {
37-
Self::PyprojectToml(path)
37+
Ok(Self::PyprojectToml(path))
3838
} else if path.ends_with("setup.py") {
39-
Self::SetupPy(path)
39+
Ok(Self::SetupPy(path))
4040
} else if path.ends_with("setup.cfg") {
41-
Self::SetupCfg(path)
41+
Ok(Self::SetupCfg(path))
4242
} else if path.ends_with("environment.yml") {
43-
Self::EnvironmentYml(path)
43+
Ok(Self::EnvironmentYml(path))
4444
} else if path
4545
.file_name()
4646
.is_some_and(|file_name| file_name.to_str().is_some_and(is_pylock_toml))
4747
{
48-
Self::PylockToml(path)
48+
Ok(Self::PylockToml(path))
49+
} else if path
50+
.extension()
51+
.is_some_and(|ext| ext.eq_ignore_ascii_case("toml"))
52+
{
53+
Err(anyhow::anyhow!(
54+
"`{}` is not a valid PEP 751 filename: expected TOML file to start with `pylock.` and end with `.toml` (e.g., `pylock.toml`, `pylock.dev.toml`)",
55+
path.user_display(),
56+
))
4957
} else {
50-
Self::RequirementsTxt(path)
58+
Ok(Self::RequirementsTxt(path))
5159
}
5260
}
5361

5462
/// Parse a [`RequirementsSource`] from a `requirements.txt` file.
55-
pub fn from_requirements_txt(path: PathBuf) -> Self {
63+
pub fn from_requirements_txt(path: PathBuf) -> Result<Self> {
5664
for file_name in ["pyproject.toml", "setup.py", "setup.cfg"] {
5765
if path.ends_with(file_name) {
58-
warn_user!(
59-
"The file `{}` appears to be a `{}` file, but requirements must be specified in `requirements.txt` format.",
66+
return Err(anyhow::anyhow!(
67+
"The file `{}` appears to be a `{}` file, but requirements must be specified in `requirements.txt` format",
6068
path.user_display(),
6169
file_name
62-
);
70+
));
6371
}
6472
}
65-
if let Some(file_name) = path.file_name() {
66-
if file_name.to_str().is_some_and(is_pylock_toml) {
67-
warn_user!(
68-
"The file `{}` appears to be a `pylock.toml` file, but requirements must be specified in `requirements.txt` format.",
69-
path.user_display(),
70-
);
71-
}
73+
if path
74+
.file_name()
75+
.and_then(OsStr::to_str)
76+
.is_some_and(is_pylock_toml)
77+
{
78+
return Err(anyhow::anyhow!(
79+
"The file `{}` appears to be a `pylock.toml` file, but requirements must be specified in `requirements.txt` format",
80+
path.user_display(),
81+
));
82+
} else if path
83+
.extension()
84+
.is_some_and(|ext| ext.eq_ignore_ascii_case("toml"))
85+
{
86+
return Err(anyhow::anyhow!(
87+
"The file `{}` appears to be a TOML file, but requirements must be specified in `requirements.txt` format",
88+
path.user_display(),
89+
));
7290
}
73-
Self::RequirementsTxt(path)
91+
Ok(Self::RequirementsTxt(path))
7492
}
7593

7694
/// Parse a [`RequirementsSource`] from a `constraints.txt` file.
77-
pub fn from_constraints_txt(path: PathBuf) -> Self {
95+
pub fn from_constraints_txt(path: PathBuf) -> Result<Self> {
7896
for file_name in ["pyproject.toml", "setup.py", "setup.cfg"] {
7997
if path.ends_with(file_name) {
80-
warn_user!(
81-
"The file `{}` appears to be a `{}` file, but constraints must be specified in `requirements.txt` format.",
98+
return Err(anyhow::anyhow!(
99+
"The file `{}` appears to be a `{}` file, but constraints must be specified in `requirements.txt` format",
82100
path.user_display(),
83101
file_name
84-
);
102+
));
85103
}
86104
}
87-
if let Some(file_name) = path.file_name() {
88-
if file_name.to_str().is_some_and(is_pylock_toml) {
89-
warn_user!(
90-
"The file `{}` appears to be a `pylock.toml` file, but constraints must be specified in `requirements.txt` format.",
91-
path.user_display(),
92-
);
93-
}
105+
if path
106+
.file_name()
107+
.and_then(OsStr::to_str)
108+
.is_some_and(is_pylock_toml)
109+
{
110+
return Err(anyhow::anyhow!(
111+
"The file `{}` appears to be a `pylock.toml` file, but constraints must be specified in `requirements.txt` format",
112+
path.user_display(),
113+
));
114+
} else if path
115+
.extension()
116+
.is_some_and(|ext| ext.eq_ignore_ascii_case("toml"))
117+
{
118+
return Err(anyhow::anyhow!(
119+
"The file `{}` appears to be a TOML file, but constraints must be specified in `requirements.txt` format",
120+
path.user_display(),
121+
));
94122
}
95-
Self::RequirementsTxt(path)
123+
Ok(Self::RequirementsTxt(path))
96124
}
97125

98126
/// Parse a [`RequirementsSource`] from an `overrides.txt` file.
99-
pub fn from_overrides_txt(path: PathBuf) -> Self {
127+
pub fn from_overrides_txt(path: PathBuf) -> Result<Self> {
100128
for file_name in ["pyproject.toml", "setup.py", "setup.cfg"] {
101129
if path.ends_with(file_name) {
102-
warn_user!(
103-
"The file `{}` appears to be a `{}` file, but overrides must be specified in `requirements.txt` format.",
130+
return Err(anyhow::anyhow!(
131+
"The file `{}` appears to be a `{}` file, but overrides must be specified in `requirements.txt` format",
104132
path.user_display(),
105133
file_name
106-
);
134+
));
107135
}
108136
}
109-
if let Some(file_name) = path.file_name() {
110-
if file_name.to_str().is_some_and(is_pylock_toml) {
111-
warn_user!(
112-
"The file `{}` appears to be a `pylock.toml` file, but overrides must be specified in `requirements.txt` format.",
113-
path.user_display(),
114-
);
115-
}
137+
if path
138+
.file_name()
139+
.and_then(OsStr::to_str)
140+
.is_some_and(is_pylock_toml)
141+
{
142+
return Err(anyhow::anyhow!(
143+
"The file `{}` appears to be a `pylock.toml` file, but overrides must be specified in `requirements.txt` format",
144+
path.user_display(),
145+
));
146+
} else if path
147+
.extension()
148+
.is_some_and(|ext| ext.eq_ignore_ascii_case("toml"))
149+
{
150+
return Err(anyhow::anyhow!(
151+
"The file `{}` appears to be a TOML file, but overrides must be specified in `requirements.txt` format",
152+
path.user_display(),
153+
));
116154
}
117-
Self::RequirementsTxt(path)
155+
Ok(Self::RequirementsTxt(path))
118156
}
119157

120158
/// Parse a [`RequirementsSource`] from a user-provided string, assumed to be a positional
@@ -134,7 +172,7 @@ impl RequirementsSource {
134172
);
135173
let confirmation = uv_console::confirm(&prompt, &term, true)?;
136174
if confirmation {
137-
return Ok(Self::from_requirements_file(name.into()));
175+
return Self::from_requirements_file(name.into());
138176
}
139177
}
140178
}
@@ -154,7 +192,7 @@ impl RequirementsSource {
154192
);
155193
let confirmation = uv_console::confirm(&prompt, &term, true)?;
156194
if confirmation {
157-
return Ok(Self::from_requirements_file(name.into()));
195+
return Self::from_requirements_file(name.into());
158196
}
159197
}
160198
}
@@ -182,7 +220,7 @@ impl RequirementsSource {
182220
);
183221
let confirmation = uv_console::confirm(&prompt, &term, true)?;
184222
if confirmation {
185-
return Ok(Self::from_requirements_file(name.into()));
223+
return Self::from_requirements_file(name.into());
186224
}
187225
}
188226
}
@@ -202,7 +240,7 @@ impl RequirementsSource {
202240
);
203241
let confirmation = uv_console::confirm(&prompt, &term, true)?;
204242
if confirmation {
205-
return Ok(Self::from_requirements_file(name.into()));
243+
return Self::from_requirements_file(name.into());
206244
}
207245
}
208246
}

0 commit comments

Comments
 (0)