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

cli: Add extension_dependencies metadata #468

Merged
merged 24 commits into from
Oct 20, 2023
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 cli/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pg-trunk"
version = "0.11.4"
version = "0.11.5"
edition = "2021"
authors = ["Steven Miller", "Ian Stanton", "Vinícius Miguel"]
description = "A package manager for PostgreSQL extensions"
Expand Down
12 changes: 12 additions & 0 deletions cli/src/commands/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ pub struct BuildCommand {
name: Option<String>,
#[arg(short = 'e', long = "extension_name")]
extension_name: Option<String>,
#[arg(short = 'x', long = "extension_dependencies")]
extension_dependencies: Option<Vec<String>>,
#[arg(short = 's', long = "preload-libraries")]
preload_libraries: Option<Vec<String>>,
#[arg(short = 'P', long = "platform")]
Expand All @@ -43,6 +45,7 @@ pub struct BuildSettings {
pub version: Option<String>,
pub name: Option<String>,
pub extension_name: Option<String>,
pub extension_dependencies: Option<Vec<String>>,
pub preload_libraries: Option<Vec<String>>,
pub system_dependencies: Option<SystemDependencies>,
pub glob_patterns_to_include: Vec<glob::Pattern>,
Expand Down Expand Up @@ -89,6 +92,12 @@ impl BuildCommand {
&trunk_toml,
);

let extension_dependencies = cli_or_trunk_opt(
&self.extension_dependencies,
|toml| &toml.extension.extension_dependencies,
&trunk_toml,
);

let preload_libraries = cli_or_trunk_opt(
&self.preload_libraries,
|toml| &toml.extension.preload_libraries,
Expand Down Expand Up @@ -144,6 +153,7 @@ impl BuildCommand {
version,
name,
extension_name,
extension_dependencies,
preload_libraries,
system_dependencies,
glob_patterns_to_include,
Expand Down Expand Up @@ -217,6 +227,7 @@ impl SubCommand for BuildCommand {
path,
&build_settings.output_path,
build_settings.extension_name,
build_settings.extension_dependencies,
build_settings.preload_libraries,
cargo_toml,
build_settings.system_dependencies,
Expand Down Expand Up @@ -260,6 +271,7 @@ impl SubCommand for BuildCommand {
&build_settings.output_path,
build_settings.name.clone().unwrap().as_str(),
build_settings.extension_name,
build_settings.extension_dependencies,
build_settings.preload_libraries,
build_settings.system_dependencies,
build_settings.version.clone().unwrap().as_str(),
Expand Down
5 changes: 5 additions & 0 deletions cli/src/commands/containers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ pub async fn package_installed_extension_files(
name: &str,
mut extension_name: Option<String>,
extension_version: &str,
extension_dependencies: Option<Vec<String>>,
inclusion_patterns: Vec<glob::Pattern>,
) -> Result<(), anyhow::Error> {
let name = name.to_owned();
Expand Down Expand Up @@ -442,6 +443,9 @@ pub async fn package_installed_extension_files(
let options_usrdir = Some(DownloadFromContainerOptions { path: "/usr" });
let file_stream = docker.download_from_container(container_id, options_usrdir);

// TODO: If extension_dependencies is none, check for control file and fetch 'requires' field (similar to below)
// example: https://github.com/paradedb/paradedb/blob/9a0b1601a9c7026e5c89eef51a422b9d284b3058/pg_search/pg_search.control#L6C1-L6C9

Comment on lines +446 to +448
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EvanHStanton as a follow-up, we'll want to pull the dependency information from the control file if it exists. The information will then be pulled automatically and we won't need to set extension_dependencies in our toml unless absolutely necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to add a test for this as well

if let Some(control) = sharedir_list.iter().find(|path| path.contains(".control")) {
// If extension_name parameter is none, check for control file and fetch extension_name
if extension_name.is_none() {
Expand Down Expand Up @@ -480,6 +484,7 @@ pub async fn package_installed_extension_files(
name,
extension_name,
extension_version,
extension_dependencies,
manifest_version: 2,
preload_libraries,
architecture: target_arch,
Expand Down
2 changes: 2 additions & 0 deletions cli/src/commands/generic_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub async fn build_generic(
output_path: &str,
name: &str,
extension_name: Option<String>,
extension_dependencies: Option<Vec<String>>,
preload_libraries: Option<Vec<String>>,
system_dependencies: Option<SystemDependencies>,
extension_version: &str,
Expand Down Expand Up @@ -144,6 +145,7 @@ pub async fn build_generic(
name,
extension_name,
extension_version,
extension_dependencies,
inclusion_patterns,
)
.await?;
Expand Down
23 changes: 23 additions & 0 deletions cli/src/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use reqwest::Url;
use std::ffi::OsStr;
use std::fs::File;
use std::io::{Read, Seek, Write};
use std::ops::Not;
use std::path::{Path, PathBuf};
use tar::{Archive, EntryType};
use tokio_task_manager::Task;
Expand Down Expand Up @@ -255,6 +256,7 @@ async fn install_file(
extensions_to_install.push(ext);

let deps = parsed_control_file.dependencies();

// For each dependency, check if it's not in depenedent_extensions_to_install and not in extensions_to_install.
// If not, add to depenedent_extensions_to_install.
// We don't want to install dependencies that are already present in the tar.gz
Expand All @@ -271,6 +273,20 @@ async fn install_file(
}
}

let maybe_manifest_deps = manifest
.as_ref()
.map(|manifest| manifest.extension_dependencies.as_ref())
.flatten();
if let Some(manifest_deps) = maybe_manifest_deps {
for dep in manifest_deps {
// If the extension is not in dependent_extensions_to_install,
// it wasn't specified in the control file
if dependent_extensions_to_install.contains(&dep).not() {
dependent_extensions_to_install.push(dep.to_string());
}
}
}

info!("Dependent extensions to be installed: {dependent_extensions_to_install:?}");
for dependency in dependent_extensions_to_install {
// check a control file is present in sharedir for each dependency
Expand Down Expand Up @@ -415,6 +431,13 @@ fn print_post_installation_guide(manifest: &Manifest) {
}
}
}
// If the manifest has extension_dependencies, then we need to install and enable the
// appropriate extension
if let Some(extension_dependencies) = &manifest.extension_dependencies {
let extension_dependencies = extension_dependencies.join(",");
println!("\nInstall and enable the following extensions:");
println!("\textension_dependencies = '{}'", extension_dependencies)
}
// If the manifest has preload_libraries, then we need to add the extension to preload_libraries
// Output will look like preload_libraries = 'spl1,spl2,spl3'
if let Some(preload_libraries) = &manifest.preload_libraries {
Expand Down
2 changes: 2 additions & 0 deletions cli/src/commands/pgrx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ pub async fn build_pgrx(
path: &Path,
output_path: &str,
extension_name: Option<String>,
extension_dependencies: Option<Vec<String>>,
preload_libraries: Option<Vec<String>>,
cargo_toml: toml::Table,
system_dependencies: Option<SystemDependencies>,
Expand Down Expand Up @@ -234,6 +235,7 @@ pub async fn build_pgrx(
name,
extension_name,
extension_version,
extension_dependencies,
inclusion_patterns,
)
.await?;
Expand Down
11 changes: 11 additions & 0 deletions cli/src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub struct PublishCommand {
name: Option<String>,
#[arg(short = 'e', long = "extension_name")]
extension_name: Option<String>,
#[arg(short = 'x', long = "extension_dependencies")]
extension_dependencies: Option<Vec<String>>,
#[arg(short = 's', long = "preload_libraries")]
preload_libraries: Option<Vec<String>>,
#[arg(long = "version", short = 'v')]
Expand Down Expand Up @@ -64,6 +66,7 @@ pub struct Category {
pub struct PublishSettings {
name: String,
extension_name: Option<String>,
extension_dependencies: Option<Vec<String>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to follow up with the registry side of this PR, so the registry can accept this new value

preload_libraries: Option<Vec<String>>,
version: String,
file: Option<PathBuf>,
Expand Down Expand Up @@ -103,6 +106,12 @@ impl PublishCommand {
&trunk_toml,
);

let extension_dependencies = cli_or_trunk_opt(
&self.extension_dependencies,
|toml| &toml.extension.extension_dependencies,
&trunk_toml,
);

let preload_libraries = cli_or_trunk_opt(
&self.preload_libraries,
|toml| &toml.extension.preload_libraries,
Expand Down Expand Up @@ -178,6 +187,7 @@ impl PublishCommand {
repository,
name,
extension_name,
extension_dependencies,
system_dependencies,
categories,
preload_libraries,
Expand Down Expand Up @@ -339,6 +349,7 @@ impl SubCommand for PublishCommand {
let m = json!({
"name": publish_settings.name,
"extension_name": publish_settings.extension_name,
"extension_dependencies": publish_settings.extension_dependencies,
"vers": publish_settings.version,
"description": publish_settings.description,
"documentation": publish_settings.documentation,
Expand Down
1 change: 1 addition & 0 deletions cli/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub struct Manifest {
/// 'apt': ['libgroonga-dev'],
/// }
/// ```
pub extension_dependencies: Option<Vec<String>>,
pub dependencies: Option<HashMap<String, Vec<String>>>,
#[serde(rename = "version")]
pub extension_version: String,
Expand Down
1 change: 1 addition & 0 deletions cli/src/trunk_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub struct TomlExtensionData {
pub name: String,
pub file: Option<PathBuf>,
pub extension_name: Option<String>,
pub extension_dependencies: Option<Vec<String>>,
pub version: String,
pub license: String,
pub repository: Option<String>,
Expand Down
9 changes: 9 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,15 @@ fn build_pg_cron_trunk_toml() -> Result<(), Box<dyn std::error::Error>> {
assert!(stdout.contains("On systems using dnf:"));
assert!(stdout.contains("libpq-devel"));

// assert that the dependencies were written to manifest
let manifest = Command::new("cat")
.arg(format!("{output_dir}/manifest.json").as_str())
.output()
.expect("failed to run cat command");

let stdout = String::from_utf8(manifest.stdout).unwrap();
assert!(stdout.contains("\"extension_dependencies\": [\n \"btree_gin\"\n ],"));

// delete the temporary file
std::fs::remove_dir_all(output_dir)?;

Expand Down
3 changes: 2 additions & 1 deletion cli/tests/test_trunk_toml_dirs/pg_cron/Trunk.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[extension]
name = "pg_cron"
extension_name = "extension_name_from_toml"
extension_dependencies = ["btree_gin"]
preload_libraries = ["shared_preload_libraries_from_toml"]
version = "1.5.2"
repository = "https://github.com/tembo-io/trunk"
Expand All @@ -14,4 +15,4 @@ platform = "linux/amd64"

[dependencies]
apt = ["libpq5"]
dnf = ["libpq-devel"]
dnf = ["libpq-devel"]