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

Use toml_edit for pretty tool receipt formatting #4637

Merged
merged 2 commits into from
Jun 29, 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: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 11 additions & 9 deletions crates/uv-tool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,21 @@ license = { workspace = true }
workspace = true

[dependencies]
uv-fs = { workspace = true }
uv-state = { workspace = true }
pep508_rs = { workspace = true }
pypi-types = { workspace = true }
uv-virtualenv = { workspace = true }
uv-toolchain = { workspace = true }
install-wheel-rs = { workspace = true }
pep440_rs = { workspace = true }
pep508_rs = { workspace = true }
pypi-types = { workspace = true }
uv-cache = { workspace = true }
uv-fs = { workspace = true }
uv-state = { workspace = true }
uv-toolchain = { workspace = true }
uv-virtualenv = { workspace = true }

thiserror = { workspace = true }
tracing = { workspace = true }
dirs-sys = { workspace = true }
fs-err = { workspace = true }
path-slash = { workspace = true }
serde = { workspace = true }
thiserror = { workspace = true }
toml = { workspace = true }
dirs-sys = { workspace = true }
toml_edit = { workspace = true }
tracing = { workspace = true }
5 changes: 2 additions & 3 deletions crates/uv-tool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,9 @@ impl InstalledTools {
path.user_display()
);

let doc = toml::to_string(&tool_receipt)
.map_err(|err| Error::ReceiptWrite(path.clone(), Box::new(err)))?;
let doc = tool_receipt.to_toml();

// Save the modified `tools.toml`.
// Save the modified `uv-receipt.toml`.
fs_err::write(&path, doc)?;

Ok(())
Expand Down
14 changes: 12 additions & 2 deletions crates/uv-tool/src/receipt.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use std::path::Path;

use serde::{Deserialize, Serialize};
use serde::Deserialize;

use crate::Tool;

/// A `uv-receipt.toml` file tracking the installation of a tool.
#[allow(dead_code)]
#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Debug, Clone, Deserialize)]
pub struct ToolReceipt {
pub(crate) tool: Tool,

Expand All @@ -30,6 +30,16 @@ impl ToolReceipt {
Err(err) => Err(err.into()),
}
}

/// Returns the TOML representation of this receipt.
pub(crate) fn to_toml(&self) -> String {
// We construct a TOML document manually instead of going through Serde to enable
// the use of inline tables.
let mut doc = toml_edit::DocumentMut::new();
doc.insert("tool", toml_edit::Item::Table(self.tool.to_toml()));

doc.to_string()
}
}

// Ignore raw document in comparison.
Expand Down
85 changes: 82 additions & 3 deletions crates/uv-tool/src/tool.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
use std::path::PathBuf;

use path_slash::PathBufExt;
use pypi_types::VerbatimParsedUrl;
use serde::{Deserialize, Serialize};
use serde::Deserialize;
use toml_edit::value;
use toml_edit::Array;
use toml_edit::Table;
use toml_edit::Value;

/// A tool entry.
#[allow(dead_code)]
#[derive(Debug, Clone, Serialize, PartialEq, Eq, Deserialize)]
#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
#[serde(rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct Tool {
Expand All @@ -17,12 +22,40 @@ pub struct Tool {
entrypoints: Vec<ToolEntrypoint>,
}

#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize)]
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub struct ToolEntrypoint {
name: String,
install_path: PathBuf,
}

/// Format an array so that each element is on its own line and has a trailing comma.
///
/// Example:
///
/// ```toml
/// requirements = [
/// "foo",
/// "bar",
/// ]
/// ```
fn each_element_on_its_line_array(elements: impl Iterator<Item = impl Into<Value>>) -> Array {
let mut array = elements
.map(Into::into)
.map(|mut value| {
// Each dependency is on its own line and indented.
value.decor_mut().set_prefix("\n ");
value
})
.collect::<Array>();
// With a trailing comma, inserting another entry doesn't change the preceding line,
// reducing the diff noise.
array.set_trailing_comma(true);
// The line break between the last element's comma and the closing square bracket.
array.set_trailing("\n");
array
}

impl Tool {
/// Create a new `Tool`.
pub fn new(
Expand All @@ -38,11 +71,57 @@ impl Tool {
entrypoints,
}
}

/// Returns the TOML table for this tool.
pub(crate) fn to_toml(&self) -> Table {
let mut table = Table::new();

table.insert("requirements", {
let requirements = match self.requirements.as_slice() {
[] => Array::new(),
[requirement] => Array::from_iter([Value::from(requirement.to_string())]),
requirements => each_element_on_its_line_array(
requirements
.iter()
.map(|requirement| Value::from(requirement.to_string())),
),
};
value(requirements)
});

if let Some(ref python) = self.python {
table.insert("python", value(python));
}

table.insert("entrypoints", {
let entrypoints = each_element_on_its_line_array(
self.entrypoints
.iter()
.map(ToolEntrypoint::to_toml)
.map(toml_edit::Table::into_inline_table),
);
value(entrypoints)
});

table
}
}

impl ToolEntrypoint {
/// Create a new [`ToolEntrypoint`].
pub fn new(name: String, install_path: PathBuf) -> Self {
Self { name, install_path }
}

/// Returns the TOML table for this entrypoint.
pub(crate) fn to_toml(&self) -> Table {
let mut table = Table::new();
table.insert("name", value(&self.name));
table.insert(
"install-path",
// Use cross-platform slashes so the toml string type does not change
value(self.install_path.to_slash_lossy().to_string()),
);
table
}
}
87 changes: 33 additions & 54 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ mod common;
/// Test installing a tool with `uv tool install`
#[test]
fn tool_install() {
let context = TestContext::new("3.12").with_filtered_counts();
let context = TestContext::new("3.12")
.with_filtered_counts()
.with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

Expand Down Expand Up @@ -77,14 +79,10 @@ fn tool_install() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black"]

[[tool.entrypoints]]
name = "black"
install_path = "[TEMP_DIR]/bin/black"

[[tool.entrypoints]]
name = "blackd"
install_path = "[TEMP_DIR]/bin/blackd"
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});

Expand Down Expand Up @@ -162,10 +160,9 @@ fn tool_install() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("flask").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["flask"]

[[tool.entrypoints]]
name = "flask"
install_path = "[TEMP_DIR]/bin/flask"
entrypoints = [
{ name = "flask", install-path = "[TEMP_DIR]/bin/flask" },
]
"###);
});
}
Expand Down Expand Up @@ -235,14 +232,10 @@ fn tool_install_version() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black==24.2.0"]

[[tool.entrypoints]]
name = "black"
install_path = "[TEMP_DIR]/bin/black"

[[tool.entrypoints]]
name = "blackd"
install_path = "[TEMP_DIR]/bin/blackd"
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});

Expand Down Expand Up @@ -325,7 +318,9 @@ fn tool_install_from() {
/// Test installing and reinstalling an already installed tool
#[test]
fn tool_install_already_installed() {
let context = TestContext::new("3.12").with_filtered_counts();
let context = TestContext::new("3.12")
.with_filtered_counts()
.with_filtered_exe_suffix();
let tool_dir = context.temp_dir.child("tools");
let bin_dir = context.temp_dir.child("bin");

Expand Down Expand Up @@ -387,14 +382,10 @@ fn tool_install_already_installed() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black"]

[[tool.entrypoints]]
name = "black"
install_path = "[TEMP_DIR]/bin/black"

[[tool.entrypoints]]
name = "blackd"
install_path = "[TEMP_DIR]/bin/blackd"
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});

Expand Down Expand Up @@ -424,14 +415,10 @@ fn tool_install_already_installed() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black"]

[[tool.entrypoints]]
name = "black"
install_path = "[TEMP_DIR]/bin/black"

[[tool.entrypoints]]
name = "blackd"
install_path = "[TEMP_DIR]/bin/blackd"
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});

Expand Down Expand Up @@ -649,14 +636,10 @@ fn tool_install_entry_point_exists() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black"]

[[tool.entrypoints]]
name = "black"
install_path = "[TEMP_DIR]/bin/black"

[[tool.entrypoints]]
name = "blackd"
install_path = "[TEMP_DIR]/bin/blackd"
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});

Expand Down Expand Up @@ -686,14 +669,10 @@ fn tool_install_entry_point_exists() {
assert_snapshot!(fs_err::read_to_string(tool_dir.join("black").join("uv-receipt.toml")).unwrap(), @r###"
[tool]
requirements = ["black"]

[[tool.entrypoints]]
name = "black"
install_path = "[TEMP_DIR]/bin/black"

[[tool.entrypoints]]
name = "blackd"
install_path = "[TEMP_DIR]/bin/blackd"
entrypoints = [
{ name = "black", install-path = "[TEMP_DIR]/bin/black" },
{ name = "blackd", install-path = "[TEMP_DIR]/bin/blackd" },
]
"###);
});

Expand Down
Loading