-
Notifications
You must be signed in to change notification settings - Fork 20
install: install system dependencies defined in Trunk.toml with an additional flag #934
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
Changes from all commits
73e8be9
010cad7
3120a07
1319b35
d0cae7a
5c0aaf7
46bf571
dc142ae
9143e66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,9 +22,12 @@ use std::fs; | |
use std::io::{Cursor, Read}; | ||
use std::ops::Not; | ||
use std::path::{Path, PathBuf}; | ||
use system_dependencies::OperatingSystem; | ||
use tar::EntryType; | ||
use tokio_task_manager::Task; | ||
|
||
mod system_dependencies; | ||
|
||
#[derive(Clone, Copy)] | ||
/// A Trunk project name versus an extension name. | ||
/// Although related, a project's name may differ from its extension name. | ||
|
@@ -67,6 +70,9 @@ pub struct InstallCommand { | |
/// Skip dependency resolution. | ||
#[clap(long, short, action)] | ||
skip_dependencies: bool, | ||
/// Installs required system dependencies for the extension | ||
#[clap(long = "deps", action)] | ||
install_system_dependencies: bool, | ||
} | ||
|
||
impl InstallCommand { | ||
|
@@ -102,9 +108,7 @@ pub struct PgConfig { | |
|
||
impl PgConfig { | ||
fn exec(&self, arg: &str) -> Result<String> { | ||
use std::process::Command; | ||
|
||
let mut bytes = Command::new(&self.pg_config_path) | ||
let mut bytes = std::process::Command::new(&self.pg_config_path) | ||
.arg(arg) | ||
.output() | ||
.with_context(|| format!("Failed to run pgconfig {arg}"))? | ||
|
@@ -176,12 +180,15 @@ impl SubCommand for InstallCommand { | |
install( | ||
Name::TrunkProject(&self.name), | ||
&self.version, | ||
&self.file, | ||
self.file.as_deref(), | ||
&self.registry, | ||
package_lib_dir, | ||
sharedir, | ||
postgres_version, | ||
self.skip_dependencies, | ||
InstallConfig { | ||
package_lib_dir, | ||
sharedir, | ||
postgres_version, | ||
skip_dependency_resolution: self.skip_dependencies, | ||
install_system_dependencies: self.install_system_dependencies, | ||
}, | ||
) | ||
.await?; | ||
|
||
|
@@ -398,16 +405,22 @@ async fn fetch_archive_from_registry<'a>( | |
Ok((data, file_name, maybe_api_extension_name)) | ||
} | ||
|
||
#[derive(Clone)] | ||
struct InstallConfig { | ||
package_lib_dir: PathBuf, | ||
sharedir: PathBuf, | ||
postgres_version: u8, | ||
skip_dependency_resolution: bool, | ||
install_system_dependencies: bool, | ||
} | ||
|
||
vrmiguel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[async_recursion] | ||
async fn install<'name: 'async_recursion>( | ||
name: Name<'name>, | ||
version: &str, | ||
file: &Option<PathBuf>, | ||
file: Option<&Path>, | ||
registry: &str, | ||
package_lib_dir: PathBuf, | ||
sharedir: PathBuf, | ||
postgres_version: u8, | ||
skip_dependency_resolution: bool, | ||
config: InstallConfig, | ||
) -> Result<()> { | ||
let extension_name = match name { | ||
Name::TrunkProject(_) => None, | ||
|
@@ -421,7 +434,7 @@ async fn install<'name: 'async_recursion>( | |
let file_name = file.file_name().and_then(|s| s.to_str()).map(String::from); | ||
(data, file_name, None) | ||
} else { | ||
fetch_archive_from_registry(name, version, registry, postgres_version).await? | ||
fetch_archive_from_registry(name, version, registry, config.postgres_version).await? | ||
}; | ||
let extension_name = extension_name.or(maybe_extension_name); | ||
|
||
|
@@ -443,14 +456,11 @@ async fn install<'name: 'async_recursion>( | |
|
||
install_trunk_archive( | ||
&archive_data, | ||
package_lib_dir, | ||
sharedir, | ||
registry, | ||
postgres_version, | ||
skip_dependency_resolution, | ||
// Send the v1-supplied extension name, in case the manifest.json | ||
// doesn't have extension_name set | ||
extension_name, | ||
config, | ||
) | ||
.await?; | ||
|
||
|
@@ -460,16 +470,13 @@ async fn install<'name: 'async_recursion>( | |
async fn install_trunk_archive( | ||
// Bytes for the project's .tar archive | ||
archive_data: &[u8], | ||
package_lib_dir: PathBuf, | ||
sharedir: PathBuf, | ||
registry: &str, | ||
postgres_version: u8, | ||
skip_dependency_resolution: bool, | ||
extension_name: Option<String>, | ||
config: InstallConfig, | ||
) -> Result<()> { | ||
// Handle symlinks | ||
let sharedir = std::fs::canonicalize(&sharedir)?; | ||
let package_lib_dir = std::fs::canonicalize(&package_lib_dir)?; | ||
let sharedir = std::fs::canonicalize(&config.sharedir)?; | ||
let package_lib_dir = std::fs::canonicalize(&config.package_lib_dir)?; | ||
|
||
// First pass: get to the manifest | ||
// Because we're going over entries with `Seek` enabled, we're not reading everything. | ||
|
@@ -558,7 +565,7 @@ async fn install_trunk_archive( | |
} | ||
} | ||
|
||
if skip_dependency_resolution { | ||
if config.skip_dependency_resolution { | ||
warn!( | ||
"Skipping dependency resolution! {} dependencies are unmet.", | ||
dependent_extensions_to_install.len() | ||
|
@@ -575,12 +582,9 @@ async fn install_trunk_archive( | |
install( | ||
Name::Extension(&dependency), | ||
"latest", | ||
&None, | ||
None, | ||
registry, | ||
package_lib_dir.clone(), | ||
sharedir.clone(), | ||
postgres_version, | ||
skip_dependency_resolution, | ||
config.clone(), | ||
) | ||
.await?; | ||
} | ||
|
@@ -599,7 +603,11 @@ async fn install_trunk_archive( | |
"Installing {} {}", | ||
manifest.name, manifest.extension_version | ||
); | ||
let host_arch = std::env::consts::ARCH; | ||
let host_arch = if cfg!(test) { | ||
"x86_64" | ||
} else { | ||
std::env::consts::ARCH | ||
}; | ||
Comment on lines
+606
to
+610
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why doesn't it work with ARM64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because Trunk is limited to |
||
|
||
if manifest.manifest_version > 1 && host_arch != manifest.architecture { | ||
bail!( | ||
|
@@ -661,6 +669,35 @@ async fn install_trunk_archive( | |
manifest.extension_name = extension_name; | ||
} | ||
|
||
if config.install_system_dependencies { | ||
if let Some(system_deps) = &manifest.dependencies { | ||
let operating_system = OperatingSystem::detect()?; | ||
for package_manager in operating_system.package_managers() { | ||
if let Some(packages_to_install) = system_deps.get(package_manager.as_str()) { | ||
for package in packages_to_install { | ||
let installation_command = package_manager.install(&package); | ||
|
||
let status = mockcmd::Command::new("sh") | ||
.arg("-c") | ||
.arg(&installation_command) | ||
.status() | ||
.with_context(|| { | ||
format!("Failed to execute command: {}", installation_command) | ||
})?; | ||
|
||
if !status.success() { | ||
anyhow::bail!( | ||
"Installation of package '{}' via {} failed", | ||
package, | ||
package_manager.as_str() | ||
); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
post_installation(&manifest); | ||
|
||
Ok(()) | ||
|
@@ -741,3 +778,42 @@ fn assert_sha256_matches(contents: &[u8], maybe_hash: Option<String>) -> Result< | |
|
||
Ok(()) | ||
} | ||
|
||
#[tokio::test] | ||
async fn install_with_system_dependencies() -> Result<()> { | ||
let file = None; | ||
|
||
let pg_config = PgConfig { | ||
pg_config_path: which::which("pg_config")?, | ||
}; | ||
let package_lib_dir = pg_config.pkglibdir()?; | ||
let sharedir = pg_config.sharedir()?; | ||
|
||
install( | ||
Name::Extension("citus"), | ||
"13.0.1", | ||
file, | ||
"https://registry.pgtrunk.io", | ||
InstallConfig { | ||
package_lib_dir, | ||
sharedir, | ||
postgres_version: 17, | ||
skip_dependency_resolution: false, | ||
install_system_dependencies: true, | ||
}, | ||
) | ||
.await?; | ||
|
||
let system_deps = [ | ||
"libpq5", "openssl", "libc6", "liblz4-1", "libzstd1", "libssl3", "libcurl4", | ||
]; | ||
for dep in system_deps { | ||
assert!(mockcmd::was_command_executed(&[ | ||
"sh", | ||
"-c", | ||
&format!("sudo apt-get install -y {}", dep) | ||
])); | ||
} | ||
|
||
Ok(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
use std::{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file basically duplicates what we did for pgxn-deps. If we publish the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pgxn-deps might evolve into something, but not in the immediate future. Seems fine to just copy it here. |
||
fs, | ||
io::{BufRead, BufReader}, | ||
}; | ||
|
||
use anyhow::{Context, Result}; | ||
|
||
#[derive(Debug, Clone, Copy)] | ||
pub enum OperatingSystem { | ||
MacOs, | ||
Debian, | ||
RedHat, | ||
Windows, | ||
} | ||
|
||
impl OperatingSystem { | ||
pub fn package_managers(&self) -> &[PackageManager] { | ||
match self { | ||
OperatingSystem::MacOs => &[PackageManager::Homebrew], | ||
OperatingSystem::Debian => &[PackageManager::Apt], | ||
OperatingSystem::RedHat => &[PackageManager::Dnf, PackageManager::Yum], | ||
OperatingSystem::Windows => &[PackageManager::Chocolatey], | ||
} | ||
} | ||
|
||
/// Detect the current operating system, if it's supported | ||
pub fn detect() -> Result<OperatingSystem> { | ||
let os = if cfg!(test) { | ||
// Always use Debian for testing | ||
Some(OperatingSystem::Debian) | ||
} else if cfg!(target_os = "linux") { | ||
Self::detect_linux_distribution() | ||
} else if cfg!(windows) { | ||
Some(OperatingSystem::Windows) | ||
} else if cfg!(target_os = "macos") { | ||
Some(OperatingSystem::MacOs) | ||
} else { | ||
None | ||
}; | ||
|
||
os.with_context(|| "Current operating system is unsupported") | ||
} | ||
|
||
/// Check `os-release` to detect current Linux distro | ||
fn detect_linux_distribution() -> Option<OperatingSystem> { | ||
let os_release = fs::File::open("/etc/os-release").ok()?; | ||
let reader = BufReader::new(os_release); | ||
|
||
for maybe_line in reader.lines() { | ||
let Ok(line) = maybe_line else { | ||
continue; | ||
}; | ||
|
||
match &*line { | ||
"ID=debian" => return Some(OperatingSystem::Debian), | ||
"ID=fedora" | "ID=centos" | "ID=rhel" => return Some(OperatingSystem::RedHat), | ||
_ => continue, | ||
} | ||
} | ||
|
||
None | ||
} | ||
} | ||
|
||
#[derive(Debug)] | ||
pub enum PackageManager { | ||
Apt, | ||
Dnf, | ||
Yum, | ||
Chocolatey, | ||
Homebrew, | ||
} | ||
|
||
impl PackageManager { | ||
pub fn as_str(&self) -> &str { | ||
match self { | ||
PackageManager::Apt => "apt", | ||
PackageManager::Dnf => "dnf", | ||
PackageManager::Yum => "yum", | ||
PackageManager::Chocolatey => "chocolatey", | ||
PackageManager::Homebrew => "brew", | ||
} | ||
} | ||
|
||
pub fn install(&self, package_name: &str) -> String { | ||
let install_command = match self { | ||
PackageManager::Apt => "apt-get install -y", | ||
PackageManager::Dnf => "dnf install -y", | ||
PackageManager::Yum => "yum install -y", | ||
PackageManager::Homebrew => "brew install", | ||
PackageManager::Chocolatey => "choco install", | ||
}; | ||
|
||
format!( | ||
"{sudo}{install_command} {package_name}", | ||
sudo = if self.requires_sudo() { "sudo " } else { "" } | ||
) | ||
} | ||
|
||
pub fn requires_sudo(&self) -> bool { | ||
match self { | ||
PackageManager::Apt | PackageManager::Dnf | PackageManager::Yum => true, | ||
PackageManager::Homebrew | PackageManager::Chocolatey => false, | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,8 +51,9 @@ pub struct Manifest { | |
/// 'apt': ['libgroonga-dev'], | ||
/// } | ||
/// ``` | ||
pub extension_dependencies: Option<Vec<String>>, | ||
pub dependencies: Option<HashMap<String, Vec<String>>>, | ||
/// A list of extensions the extension being installed depends on | ||
pub extension_dependencies: Option<Vec<String>>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment was tagged on the wrong variable |
||
#[serde(rename = "version")] | ||
pub extension_version: String, | ||
pub manifest_version: i32, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this only be a dev dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's both! We also set it under
dev-dependencies
. In non-dev, however, it's just a rename forstd::process::Command
trunk/cli/Cargo.toml
Line 69 in dc142ae