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.

Sign up for GitHub

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

Jump to bottom

chore: lots of lints and fixes #1523

Merged
merged 8 commits into from
Feb 7, 2024
Merged

chore: lots of lints and fixes #1523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5ce3f85
chore: lots of lints and fixes
nyurik Feb 4, 2024
3599ded
a few more lints
nyurik Feb 4, 2024
1ece4aa
revert sum_vector_simd - might need benchmarks
nyurik Feb 4, 2024
8e40971
address feedback
nyurik Feb 5, 2024
8e70102
match improvements
nyurik Feb 7, 2024
88f2e2d
use suggested natts parsing
nyurik Feb 7, 2024
e26b8eb
use io::Error::Other
nyurik Feb 7, 2024
4c31d5a
Update pgrx/src/list/flat_list.rs
nyurik Feb 7, 2024
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 Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ cargo-pgrx = { path = "cargo-pgrx" }
pgrx-macros = { path = "./pgrx-macros", version = "=0.12.0-alpha.0" }
pgrx-pg-sys = { path = "./pgrx-pg-sys", version = "=0.12.0-alpha.0" }
pgrx-sql-entity-graph = { path = "./pgrx-sql-entity-graph", version = "=0.12.0-alpha.0" }
pgrx-pg-config = { path = "./pgrx-pg-config/", version = "=0.12.0-alpha.0" }
pgrx-pg-config = { path = "./pgrx-pg-config", version = "=0.12.0-alpha.0" }

cargo_toml = "0.16" # used for building projects
eyre = "0.6.9" # simplifies error-handling
Expand Down
50 changes: 19 additions & 31 deletions cargo-pgrx/src/command/init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use std::collections::HashMap;
use std::fs::File;
use std::io::{Read, Write};
use std::num::NonZeroUsize;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::sync::OnceLock;

Expand Down Expand Up @@ -208,7 +208,7 @@ pub(crate) fn init_pgrx(pgrx: &Pgrx, init: &Init) -> eyre::Result<()> {
output_configs.sort_by(|a, b| {
a.major_version()
.unwrap_or_else(|e| panic!("{e}: could not determine major version for: `{a:?}`"))
.cmp(&b.major_version().ok().expect("could not determine major version"))
.cmp(&b.major_version().expect("could not determine major version"))
});
for pg_config in output_configs.iter() {
validate_pg_config(pg_config)?;
Expand All @@ -231,7 +231,7 @@ pub(crate) fn init_pgrx(pgrx: &Pgrx, init: &Init) -> eyre::Result<()> {
#[tracing::instrument(level = "error", skip_all, fields(pg_version = %pg_config.version()?, pgrx_home))]
fn download_postgres(
pg_config: &PgConfig,
pgrx_home: &PathBuf,
pgrx_home: &Path,
init: &Init,
) -> eyre::Result<PgConfig> {
use crate::command::build_agent_for_url;
Expand Down Expand Up @@ -263,15 +263,10 @@ fn download_postgres(
make_install_postgres(pg_config, &pgdir, init) // returns a new PgConfig object
}

fn untar(
bytes: &[u8],
pgrxdir: &PathBuf,
pg_config: &PgConfig,
init: &Init,
) -> eyre::Result<PathBuf> {
fn untar(bytes: &[u8], pgrxdir: &Path, pg_config: &PgConfig, init: &Init) -> eyre::Result<PathBuf> {
let _token = init.jobserver.get().unwrap().acquire().unwrap();

let mut unpackdir = pgrxdir.clone();
let mut unpackdir = pgrxdir.to_path_buf();
unpackdir.push(&format!("{}_unpack", pg_config.version()?));
if unpackdir.exists() {
// delete everything at this path if it already exists
Expand All @@ -289,7 +284,7 @@ fn untar(
let mut tar_decoder = Archive::new(BzDecoder::new(bytes));
tar_decoder.unpack(&unpackdir)?;

let mut pgdir = pgrxdir.clone();
let mut pgdir = pgrxdir.to_path_buf();
pgdir.push(&pg_config.version()?);
if pgdir.exists() {
// delete everything at this path if it already exists
Expand Down Expand Up @@ -398,11 +393,11 @@ fn fixup_homebrew_for_icu(configure_cmd: &mut Command) {
}
}

fn configure_postgres(pg_config: &PgConfig, pgdir: &PathBuf, init: &Init) -> eyre::Result<()> {
fn configure_postgres(pg_config: &PgConfig, pgdir: &Path, init: &Init) -> eyre::Result<()> {
let _token = init.jobserver.get().unwrap().acquire().unwrap();

println!("{} Postgres v{}", " Configuring".bold().green(), pg_config.version()?);
let mut configure_path = pgdir.clone();
let mut configure_path = pgdir.to_path_buf();
configure_path.push("configure");
let mut command = std::process::Command::new(configure_path);
// Some of these are redundant with `--enable-debug`.
Expand Down Expand Up @@ -454,19 +449,16 @@ fn configure_postgres(pg_config: &PgConfig, pgdir: &PathBuf, init: &Init) -> eyr
if output.status.success() {
Ok(())
} else {
Err(std::io::Error::new(
std::io::ErrorKind::Other,
format!(
"{}\n{}{}",
command_str,
String::from_utf8(output.stdout).unwrap(),
String::from_utf8(output.stderr).unwrap()
),
))?
Err(std::io::Error::other(format!(
"{}\n{}{}",
command_str,
String::from_utf8(output.stdout).unwrap(),
String::from_utf8(output.stderr).unwrap()
)))?
}
}

fn make_postgres(pg_config: &PgConfig, pgdir: &PathBuf, init: &Init) -> eyre::Result<()> {
fn make_postgres(pg_config: &PgConfig, pgdir: &Path, init: &Init) -> eyre::Result<()> {
println!("{} Postgres v{}", " Compiling".bold().green(), pg_config.version()?);
let mut command = std::process::Command::new("make");

Expand Down Expand Up @@ -499,11 +491,7 @@ fn make_postgres(pg_config: &PgConfig, pgdir: &PathBuf, init: &Init) -> eyre::Re
}
}

fn make_install_postgres(
version: &PgConfig,
pgdir: &PathBuf,
init: &Init,
) -> eyre::Result<PgConfig> {
fn make_install_postgres(version: &PgConfig, pgdir: &Path, init: &Init) -> eyre::Result<PgConfig> {
println!(
"{} Postgres v{} to {}",
" Installing".bold().green(),
Expand Down Expand Up @@ -582,8 +570,8 @@ fn write_config(pg_configs: &Vec<PgConfig>, init: &Init) -> eyre::Result<()> {
Ok(())
}

fn get_pg_installdir(pgdir: &PathBuf) -> PathBuf {
let mut dir = PathBuf::from(pgdir);
fn get_pg_installdir(pgdir: &Path) -> PathBuf {
let mut dir = pgdir.to_path_buf();
dir.push("pgrx-install");
dir
}
Expand All @@ -601,7 +589,7 @@ fn is_root_user() -> bool {
false
}

pub(crate) fn initdb(bindir: &PathBuf, datadir: &PathBuf) -> eyre::Result<()> {
pub(crate) fn initdb(bindir: &Path, datadir: &Path) -> eyre::Result<()> {
println!(" {} data directory at {}", "Initializing".bold().green(), datadir.display());
let mut command = std::process::Command::new(format!("{}/initdb", bindir.display()));
command
Expand Down
72 changes: 35 additions & 37 deletions cargo-pgrx/src/command/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub(crate) fn install_extension(
);
copy_file(
&control_file,
&dest,
dest,
"control file",
true,
&package_manifest_path,
Expand Down Expand Up @@ -207,13 +207,13 @@ pub(crate) fn install_extension(
// process which will mash up all pointers in the .TEXT segment.
// this simulate linux's install(1) behavior
if dest.exists() {
std::fs::remove_file(&dest)
fs::remove_file(&dest)
.wrap_err_with(|| format!("unable to remove existing file {}", dest.display()))?;
}

copy_file(
&shlibpath,
&dest,
dest,
"shared library",
false,
&package_manifest_path,
Expand Down Expand Up @@ -241,8 +241,8 @@ pub(crate) fn install_extension(
}

fn copy_file(
src: &PathBuf,
dest: &PathBuf,
src: &Path,
dest: PathBuf,
msg: &str,
do_filter: bool,
package_manifest_path: impl AsRef<Path>,
Expand All @@ -263,28 +263,28 @@ fn copy_file(
})?,
};

println!("{} {} to {}", " Copying".bold().green(), msg, format_display_path(dest)?.cyan());
println!("{} {} to {}", " Copying".bold().green(), msg, format_display_path(&dest)?.cyan());

if do_filter {
// we want to filter the contents of the file we're to copy
let input = std::fs::read_to_string(src)
let input = fs::read_to_string(src)
.wrap_err_with(|| format!("failed to read `{}`", src.display()))?;
let mut input = filter_contents(package_manifest_path, input)?;

if src.display().to_string().ends_with(".control") {
input = filter_out_fields_in_control(pg_config, input)?;
}

std::fs::write(dest, input).wrap_err_with(|| {
fs::write(&dest, input).wrap_err_with(|| {
format!("failed writing `{}` to `{}`", src.display(), dest.display())
})?;
} else {
std::fs::copy(src, dest).wrap_err_with(|| {
fs::copy(src, &dest).wrap_err_with(|| {
format!("failed copying `{}` to `{}`", src.display(), dest.display())
})?;
}

output_tracking.push(dest.clone());
output_tracking.push(dest);

Ok(())
}
Expand Down Expand Up @@ -347,10 +347,10 @@ pub(crate) fn build_extension(

fn get_target_sql_file(
manifest_path: impl AsRef<Path>,
extdir: &PathBuf,
base_directory: &PathBuf,
extdir: &Path,
base_directory: PathBuf,
) -> eyre::Result<PathBuf> {
let mut dest = base_directory.clone();
let mut dest = base_directory;
dest.push(extdir);

let (_, extname) = find_control_file(&manifest_path)?;
Expand All @@ -368,12 +368,12 @@ fn copy_sql_files(
profile: &CargoProfile,
is_test: bool,
features: &clap_cargo::Features,
extdir: &PathBuf,
base_directory: &PathBuf,
extdir: &Path,
base_directory: &Path,
skip_build: bool,
output_tracking: &mut Vec<PathBuf>,
) -> eyre::Result<()> {
let dest = get_target_sql_file(&package_manifest_path, extdir, base_directory)?;
let dest = get_target_sql_file(&package_manifest_path, extdir, base_directory.to_path_buf())?;
let (_, extname) = find_control_file(&package_manifest_path)?;

crate::command::schema::generate_schema(
Expand All @@ -392,26 +392,24 @@ fn copy_sql_files(
)?;

// now copy all the version upgrade files too
if let Ok(dir) = std::fs::read_dir("sql/") {
for sql in dir {
if let Ok(sql) = sql {
let filename = sql.file_name().into_string().unwrap();

if filename.starts_with(&format!("{extname}--")) && filename.ends_with(".sql") {
let mut dest = base_directory.clone();
dest.push(extdir);
dest.push(filename);

copy_file(
&sql.path(),
&dest,
"extension schema upgrade file",
true,
&package_manifest_path,
output_tracking,
pg_config,
)?;
}
if let Ok(dir) = fs::read_dir("sql/") {
for sql in dir.flatten() {
let filename = sql.file_name().into_string().unwrap();

if filename.starts_with(&format!("{extname}--")) && filename.ends_with(".sql") {
let mut dest = base_directory.to_path_buf();
dest.push(extdir);
dest.push(filename);

copy_file(
&sql.path(),
dest,
"extension schema upgrade file",
true,
&package_manifest_path,
output_tracking,
pg_config,
)?;
}
}
}
Expand All @@ -420,7 +418,7 @@ fn copy_sql_files(

#[tracing::instrument(level = "error", skip_all)]
pub(crate) fn find_library_file(
manifest: &cargo_toml::Manifest,
manifest: &Manifest,
build_command_messages: &Vec<cargo_metadata::Message>,
) -> eyre::Result<PathBuf> {
let target_name = manifest.target_name()?;
Expand Down
45 changes: 18 additions & 27 deletions cargo-pgrx/src/command/new.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,18 @@ pub(crate) fn create_crate_template(
name: &str,
is_bgworker: bool,
) -> eyre::Result<()> {
create_directory_structure(&path)?;
create_control_file(&path, name)?;
create_cargo_toml(&path, name)?;
create_dotcargo_config_toml(&path, name)?;
create_lib_rs(&path, name, is_bgworker)?;
create_git_ignore(&path, name)?;
create_pgrx_embed_rs(&path)?;
create_directory_structure(path.clone())?;
create_control_file(path.clone(), name)?;
create_cargo_toml(path.clone(), name)?;
create_dotcargo_config_toml(path.clone(), name)?;
create_lib_rs(path.clone(), name, is_bgworker)?;
create_git_ignore(path.clone(), name)?;
create_pgrx_embed_rs(path)?;

Ok(())
}

fn create_directory_structure(path: &PathBuf) -> Result<(), std::io::Error> {
let mut src_dir = path.clone();

fn create_directory_structure(mut src_dir: PathBuf) -> Result<(), std::io::Error> {
src_dir.push("src");
std::fs::create_dir_all(&src_dir)?;

Expand All @@ -85,9 +83,7 @@ fn create_directory_structure(path: &PathBuf) -> Result<(), std::io::Error> {
Ok(())
}

fn create_control_file(path: &PathBuf, name: &str) -> Result<(), std::io::Error> {
let mut filename = path.clone();

fn create_control_file(mut filename: PathBuf, name: &str) -> Result<(), std::io::Error> {
filename.push(format!("{name}.control"));
let mut file = std::fs::File::create(filename)?;

Expand All @@ -96,9 +92,7 @@ fn create_control_file(path: &PathBuf, name: &str) -> Result<(), std::io::Error>
Ok(())
}

fn create_cargo_toml(path: &PathBuf, name: &str) -> Result<(), std::io::Error> {
let mut filename = path.clone();

fn create_cargo_toml(mut filename: PathBuf, name: &str) -> Result<(), std::io::Error> {
filename.push("Cargo.toml");
let mut file = std::fs::File::create(filename)?;

Expand All @@ -107,9 +101,7 @@ fn create_cargo_toml(path: &PathBuf, name: &str) -> Result<(), std::io::Error> {
Ok(())
}

fn create_dotcargo_config_toml(path: &PathBuf, _name: &str) -> Result<(), std::io::Error> {
let mut filename = path.clone();

fn create_dotcargo_config_toml(mut filename: PathBuf, _name: &str) -> Result<(), std::io::Error> {
filename.push(".cargo");
filename.push("config.toml");
let mut file = std::fs::File::create(filename)?;
Expand All @@ -119,9 +111,11 @@ fn create_dotcargo_config_toml(path: &PathBuf, _name: &str) -> Result<(), std::i
Ok(())
}

fn create_lib_rs(path: &PathBuf, name: &str, is_bgworker: bool) -> Result<(), std::io::Error> {
let mut filename = path.clone();

fn create_lib_rs(
mut filename: PathBuf,
name: &str,
is_bgworker: bool,
) -> Result<(), std::io::Error> {
filename.push("src");
filename.push("lib.rs");
let mut file = std::fs::File::create(filename)?;
Expand All @@ -137,9 +131,7 @@ fn create_lib_rs(path: &PathBuf, name: &str, is_bgworker: bool) -> Result<(), st
Ok(())
}

fn create_git_ignore(path: &PathBuf, _name: &str) -> Result<(), std::io::Error> {
let mut filename = path.clone();

fn create_git_ignore(mut filename: PathBuf, _name: &str) -> Result<(), std::io::Error> {
filename.push(".gitignore");
let mut file = std::fs::File::create(filename)?;

Expand All @@ -148,8 +140,7 @@ fn create_git_ignore(path: &PathBuf, _name: &str) -> Result<(), std::io::Error>
Ok(())
}

fn create_pgrx_embed_rs(path: &PathBuf) -> Result<(), std::io::Error> {
let mut filename = path.clone();
fn create_pgrx_embed_rs(mut filename: PathBuf) -> Result<(), std::io::Error> {
filename.push("src");
filename.push("bin");
filename.push(format!("pgrx_embed.rs"));
Expand Down
Loading
Loading