Remove the pg_sql_graph_magic!() macro.

articles/forging-sql-from-rust.md

@@ -338,20 +338,19 @@ for object in archive.objects() {
 }
 ```
 
-This list gets passed into the binary, which dynamically loads the code using something like:
+This list gets passed into the binary, which builds the entity graph structure using something like:
 
 ```rust
 let mut entities = Vec::default();
 // We *must* use this or the extension might not link in.
-let control_file = __pgrx_marker()?;
+let control_file = pgrx_sql_entity_graph::ControlFile:try_from("/the/known/path/to/extname.control")?;
 entities.push(SqlGraphEntity::ExtensionRoot(control_file));
 unsafe {
-    let lib = libloading::os::unix::Library::this();
     for symbol_to_call in symbols_to_call {
-        let symbol: libloading::os::unix::Symbol<
-            unsafe extern fn() -> SqlGraphEntity
-        > = lib.get(symbol_to_call.as_bytes()).unwrap();
-        let entity = symbol();
+        extern "Rust" {
+            fn $symbol_name() -> pgrx_sql_entity_graph::SqlGraphEntity;
+        } 
+        let entity = unsafe { $symbol_name() };
         entities.push(entity);
     }
 };

cargo-pgrx/src/command/schema.rs

@@ -183,7 +183,14 @@ pub(crate) fn generate_schema(
         out_dot = Some(x.to_string());
     };
 
-    let codegen = compute_codegen(package_manifest_path, &symbols, &lib_name, out_path, out_dot)?;
+    let codegen = compute_codegen(
+        control_file,
+        package_manifest_path,
+        &symbols,
+        &lib_name,
+        out_path,
+        out_dot,
+    )?;
 
     let embed = {
         let mut embed = tempfile::NamedTempFile::new()?;
@@ -214,7 +221,7 @@ pub(crate) fn generate_schema(
         &package_name,
     )?;
 
-    compute_sql(profile, &package_name)?;
+    compute_sql(profile, &package_name, &manifest)?;
 
     Ok(())
 }
@@ -381,6 +388,7 @@ fn first_build(
 }
 
 fn compute_codegen(
+    control_file_path: impl AsRef<Path>,
     package_manifest_path: impl AsRef<Path>,
     symbols: &[String],
     lib_name: &str,
@@ -391,9 +399,20 @@ fn compute_codegen(
     let lib_name_ident = Ident::new(&lib_name, Span::call_site());
 
     let inputs = {
+        let control_file_path = control_file_path
+            .as_ref()
+            .to_str()
+            .expect(".control file filename should be valid UTF8");
         let mut out = quote::quote! {
+            // call the marker.  Primarily this ensures that rustc will actually link to the library
+            // during the "pgrx_embed" build initiated by `cargo-pgrx schema` generation
+            #lib_name_ident::__pgrx_marker();
+
             let mut entities = Vec::new();
-            let control_file_entity = ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity::ExtensionRoot(::#lib_name_ident::__pgrx_marker());
+            let control_file_path = std::path::PathBuf::from(#control_file_path);
+            let control_file = ::pgrx::pgrx_sql_entity_graph::ControlFile::try_from(control_file_path).expect(".control file should properly formatted");
+            let control_file_entity = ::pgrx::pgrx_sql_entity_graph::SqlGraphEntity::ExtensionRoot(control_file);
+
             entities.push(control_file_entity);
         };
         for name in symbols.iter() {
@@ -535,7 +554,11 @@ fn second_build(
     Ok(())
 }
 
-fn compute_sql(profile: &CargoProfile, package_name: &str) -> eyre::Result<()> {
+fn compute_sql(
+    profile: &CargoProfile,
+    package_name: &str,
+    manifest: &Manifest,
+) -> eyre::Result<()> {
     let mut bin = get_target_dir()?;
     bin.push(profile.target_subdir());
     bin.push(format!("pgrx_embed_{package_name}"));
@@ -545,6 +568,12 @@ fn compute_sql(profile: &CargoProfile, package_name: &str) -> eyre::Result<()> {
     command.stdout(Stdio::inherit());
     command.stderr(Stdio::inherit());
 
+    // pass the package version through as an environment variable
+    let cargo_pkg_version = std::env::var("CARGO_PKG_VERSION").unwrap_or_else(|_| {
+        manifest.package_version().expect("`CARGO_PKG_VERSION` should be set, and barring that, `Cargo.toml` should have a package version property")
+    });
+    command.env("CARGO_PKG_VERSION", cargo_pkg_version);
+
     let command_str = format!("{:?}", command);
     tracing::debug!(command = %command_str, "Running");
     let embed_output = command

pgrx-sql-entity-graph/src/control_file.rs

@@ -18,6 +18,7 @@ to the `pgrx` framework and very subject to change between versions. While you m
 use super::{SqlGraphEntity, SqlGraphIdentifier, ToSql};
 use core::convert::TryFrom;
 use std::collections::HashMap;
+use std::path::PathBuf;
 use thiserror::Error;
 
 /// The parsed contents of a `.control` file.
@@ -26,7 +27,8 @@ use thiserror::Error;
 /// use pgrx_sql_entity_graph::ControlFile;
 /// use std::convert::TryFrom;
 /// # fn main() -> eyre::Result<()> {
-/// let context = include_str!("../../pgrx-examples/custom_types/custom_types.control");
+/// # // arrays.control chosen because it does **NOT** use the @CARGO_VERSION@ variable
+/// let context = include_str!("../../pgrx-examples/arrays/arrays.control");
 /// let _control_file = ControlFile::try_from(context)?;
 /// # Ok(())
 /// # }
@@ -43,18 +45,46 @@ pub struct ControlFile {
 }
 
 impl ControlFile {
-    /// Parse a `.control` file.
+    /// Parse a `.control` file, performing all known pgrx dynamic variable substitutions.
+    ///
+    /// # Supported Dynamic Variable Substitutions
+    ///
+    /// `@CARGO_VERSION@`:  Replaced with the value of the environment variable `CARGO_PKG_VERSION`,
+    ///                     which is set by cargo, or failing that, `cargo-pgrx` using the package
+    ///                     version from the extension's `Cargo.toml` file
+    ///
+    /// # Errors
+    ///
+    /// Returns a `ControlFileError` if any of the required fields are missing from the input string
+    /// or if any required environment variables (for dynamic variable substitution) are missing
     ///
     /// ```rust
     /// use pgrx_sql_entity_graph::ControlFile;
     /// # fn main() -> eyre::Result<()> {
-    /// let context = include_str!("../../pgrx-examples/custom_types/custom_types.control");
+    /// # // arrays.control chosen because it does **NOT** use the @CARGO_VERSION@ variable
+    /// let context = include_str!("../../pgrx-examples/arrays/arrays.control");
     /// let _control_file = ControlFile::from_str(context)?;
     /// # Ok(())
     /// # }
     /// ```
     #[allow(clippy::should_implement_trait)]
     pub fn from_str(input: &str) -> Result<Self, ControlFileError> {
+        fn do_var_replacements(mut input: String) -> Result<String, ControlFileError> {
+            const CARGO_VERSION: &str = "@CARGO_VERSION@";
+
+            // endeavor to not require external values if they're not used by the input
+            if input.contains(CARGO_VERSION) {
+                input = input.replace(
+                    CARGO_VERSION,
+                    &std::env::var("CARGO_PKG_VERSION").map_err(|_| {
+                        ControlFileError::MissingEnvvar("CARGO_PKG_VERSION".to_string())
+                    })?,
+                );
+            }
+
+            Ok(input)
+        }
+
         let mut temp = HashMap::new();
         for line in input.lines() {
             let parts: Vec<&str> = line.split('=').collect();
@@ -68,7 +98,7 @@ impl ControlFile {
             let v = v.trim_start_matches('\'');
             let v = v.trim_end_matches('\'');
 
-            temp.insert(k, v);
+            temp.insert(k, do_var_replacements(v.to_string())?);
         }
         let control_file = ControlFile {
             comment: temp
@@ -83,13 +113,13 @@ impl ControlFile {
             relocatable: temp
                 .get("relocatable")
                 .ok_or(ControlFileError::MissingField { field: "relocatable" })?
-                == &"true",
+                == "true",
             superuser: temp
                 .get("superuser")
                 .ok_or(ControlFileError::MissingField { field: "superuser" })?
-                == &"true",
+                == "true",
             schema: temp.get("schema").map(|v| v.to_string()),
-            trusted: if let Some(v) = temp.get("trusted") { v == &"true" } else { false },
+            trusted: if let Some(v) = temp.get("trusted") { v == "true" } else { false },
         };
 
         if !control_file.superuser && control_file.trusted {
@@ -108,12 +138,28 @@ impl From<ControlFile> for SqlGraphEntity {
 }
 
 /// An error met while parsing a `.control` file.
-#[derive(Debug, Clone, Error)]
+#[derive(Debug, Error)]
 pub enum ControlFileError {
+    #[error("Filesystem error reading control file")]
+    IOError {
+        #[from]
+        error: std::io::Error,
+    },
     #[error("Missing field in control file! Please add `{field}`.")]
     MissingField { field: &'static str },
     #[error("Redundant field in control file! Please remove `{field}`.")]
     RedundantField { field: &'static str },
+    #[error("Missing environment variable: {0}")]
+    MissingEnvvar(String),
+}
+
+impl TryFrom<PathBuf> for ControlFile {
+    type Error = ControlFileError;
+
+    fn try_from(value: PathBuf) -> Result<Self, Self::Error> {
+        let contents = std::fs::read_to_string(value)?;
+        ControlFile::try_from(contents.as_str())
+    }
 }
 
 impl TryFrom<&str> for ControlFile {

pgrx-tests/src/framework.rs

@@ -312,7 +312,7 @@ fn install_extension() -> eyre::Result<()> {
         .arg("--pg-config")
         .arg(pg_config.path().ok_or(eyre!("No pg_config found"))?)
         .stdout(Stdio::inherit())
-        .stderr(Stdio::piped())
+        .stderr(Stdio::inherit())
         .env("CARGO_TARGET_DIR", get_target_dir()?);
 
     if let Ok(manifest_path) = std::env::var("PGRX_MANIFEST_PATH") {

pgrx-tests/src/lib.rs

@@ -16,7 +16,7 @@ pub use framework::*;
 pub mod proptest;
 
 #[cfg(any(test, feature = "pg_test"))]
-pgrx::pg_sql_graph_magic!();
+pgrx::pg_module_magic!();
 
 #[cfg(test)]
 pub mod pg_test {

pgrx-tests/src/tests/mod.rs

@@ -61,5 +61,3 @@ mod variadic_tests;
 mod xact_callback_tests;
 mod xid64_tests;
 mod zero_datum_edge_cases;
-
-pgrx::pg_magic_func!();

pgrx/src/lib.rs

@@ -177,12 +177,19 @@ const _: () = {
 ///
 /// </pre></div>
 ///
-/// This calls both [`pg_magic_func!()`](pg_magic_func) and [`pg_sql_graph_magic!()`](pg_sql_graph_magic).
+/// This calls [`pg_magic_func!()`](pg_magic_func).
 #[macro_export]
 macro_rules! pg_module_magic {
     () => {
         $crate::pg_magic_func!();
-        $crate::pg_sql_graph_magic!();
+
+        // A marker function which must exist in the root of the extension for proper linking by the
+        // "pgrx_embed" binary during `cargo-pgrx schema` generation.
+        #[inline(never)] /* we don't want DCE to remove this as it *could* cause the compiler to decide to not link to us */
+        #[doc(hidden)]
+        pub fn __pgrx_marker() {
+            // noop
+        }
     };
 }
 
@@ -259,39 +266,6 @@ macro_rules! pg_magic_func {
     };
 }
 
-/// Create necessary extension-local internal marker for use with SQL generation.
-///
-/// <div class="example-wrap" style="display:inline-block">
-/// <pre class="ignore" style="white-space:normal;font:inherit;">
-///
-/// **Note**: Generally [`pg_module_magic`] is preferred, and results in this macro being called.
-/// This macro should only be directly called in advanced use cases.
-///
-/// </pre></div>
-#[macro_export]
-macro_rules! pg_sql_graph_magic {
-    () => {
-        // A marker which must exist in the root of the extension.
-        #[doc(hidden)]
-        pub fn __pgrx_marker() -> $crate::pgrx_sql_entity_graph::ControlFile {
-            use ::core::convert::TryFrom;
-            let package_version = env!("CARGO_PKG_VERSION");
-            let context = include_str!(concat!(
-                env!("CARGO_MANIFEST_DIR"),
-                "/",
-                env!("CARGO_CRATE_NAME"),
-                ".control"
-            ))
-            .replace("@CARGO_VERSION@", package_version);
-
-            let control_file =
-                $crate::pgrx_sql_entity_graph::ControlFile::try_from(context.as_str())
-                    .expect("Could not parse control file, is it valid?");
-            control_file
-        }
-    };
-}
-
 pub(crate) static UTF8DATABASE: Lazy<Utf8Compat> = Lazy::new(|| {
     let encoding_int = unsafe { pgrx_pg_sys::GetDatabaseEncoding() };
     match encoding_int as core::ffi::c_uint {