From 2cf3dc6f22e25a0f14a73698ce6da5ffc4e0ed05 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 18 Jun 2026 16:21:19 -0700 Subject: [PATCH 1/2] sqlx-macros-core: allow not calling cargo from a proc-macro Problem: We're trying to build https://github.com/MercuryTechnologies/locally-euclidean with buck2, and we're finding that sqlx is trying to call Cargo to find the workspace root and snoop outside of its build directory. This is trouble for us because we want to be able to cache build actions. Thus, we want to be able to run sqlx in an *extra* offline mode which never calls cargo. Solution: check if all the env-vars are set (which requires that you put nonsense in the database url, but that's ok for us as it ensures we cannot possibly regress any existing users). Problem: If a query is missed in .sqlx/, sqlx calls cargo unconditionally to find the workspace root. Solution: accept SQLX_WORKSPACE_DIR to give the fallback directory. You could make it /var/empty if you want to suppress it altogether. ### Does your PR solve an issue? Not one that either I nor an agent can find. Related Bazel usage: https://github.com/transact-rs/sqlx/issues/3555, related env-var shenanigans in the CLI https://github.com/transact-rs/sqlx/issues/3963, but neither are solved by this. --- Cargo.toml | 3 +- sqlx-macros-core/Cargo.toml | 3 + sqlx-macros-core/src/query/metadata.rs | 138 +++++++++++++++++++------ 3 files changed, 112 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3738b814ca..d02b174e33 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -193,6 +193,7 @@ uuid = "1.12.1" base64 = { version = "0.22.1", default-features = false, features = ["alloc"] } cfg-if = "1.0.0" dotenvy = { version = "0.15.7", default-features = false } +tempfile = "3.10.1" thiserror = { version = "2.0.18", default-features = false, features = ["std"] } # Cryptography @@ -247,7 +248,7 @@ serde = { version = "1.0.219", features = ["derive"] } serde_json = "1.0.142" url = "2.2.2" hex = "0.4.3" -tempfile = "3.10.1" +tempfile = { workspace = true } criterion = { version = "0.5.1", features = ["async_tokio"] } libsqlite3-sys = { version = "0.37.0" } diff --git a/sqlx-macros-core/Cargo.toml b/sqlx-macros-core/Cargo.toml index 534b92764d..a5a0617b24 100644 --- a/sqlx-macros-core/Cargo.toml +++ b/sqlx-macros-core/Cargo.toml @@ -80,6 +80,9 @@ syn = { version = "2.0.87", default-features = false, features = ["full", "deriv quote = { version = "1.0.35", default-features = false } url = { version = "2.2.2" } +[dev-dependencies] +tempfile = { workspace = true } + [lints.rust.unexpected_cfgs] level = "warn" check-cfg = ['cfg(sqlx_macros_unstable)', 'cfg(procmacro2_semver_exempt)'] diff --git a/sqlx-macros-core/src/query/metadata.rs b/sqlx-macros-core/src/query/metadata.rs index b3b31c0eb5..ea9fc1463b 100644 --- a/sqlx-macros-core/src/query/metadata.rs +++ b/sqlx-macros-core/src/query/metadata.rs @@ -29,32 +29,44 @@ impl Metadata { pub fn workspace_root(&self) -> PathBuf { let mut root = self.workspace_root.lock().unwrap(); if root.is_none() { - use serde::Deserialize; - use std::process::Command; - - let cargo = crate::env("CARGO").unwrap(); - - let output = Command::new(cargo) - .args(["metadata", "--format-version=1", "--no-deps"]) - .current_dir(&self.manifest_dir) - .env_remove("__CARGO_FIX_PLZ") - .output() - .expect("Could not fetch metadata"); - - #[derive(Deserialize)] - struct CargoMetadata { - workspace_root: PathBuf, - } - - let metadata: CargoMetadata = - serde_json::from_slice(&output.stdout).expect("Invalid `cargo metadata` output"); - - *root = Some(metadata.workspace_root); + *root = Some(resolve_workspace_root( + crate::env("SQLX_WORKSPACE_DIR").ok().map(PathBuf::from), + || { + use serde::Deserialize; + use std::process::Command; + + let cargo = crate::env("CARGO").unwrap(); + + let output = Command::new(cargo) + .args(["metadata", "--format-version=1", "--no-deps"]) + .current_dir(&self.manifest_dir) + .env_remove("__CARGO_FIX_PLZ") + .output() + .expect("Could not fetch metadata"); + + #[derive(Deserialize)] + struct CargoMetadata { + workspace_root: PathBuf, + } + + let metadata: CargoMetadata = serde_json::from_slice(&output.stdout) + .expect("Invalid `cargo metadata` output"); + + metadata.workspace_root + }, + )); } root.clone().unwrap() } } +fn resolve_workspace_root( + override_dir: Option, + cargo_fallback: impl FnOnce() -> PathBuf, +) -> PathBuf { + override_dir.unwrap_or_else(cargo_fallback) +} + pub fn try_for_crate() -> crate::Result> { /// The `MtimeCache` in this type covers the config itself, /// any changes to which will indirectly invalidate the loaded env vars as well. @@ -89,7 +101,7 @@ pub fn try_for_crate() -> crate::Result> { }) } -fn load_env( +fn load_from_dotenv( manifest_dir: &Path, config: &Config, builder: &mut MtimeCacheBuilder, @@ -143,16 +155,32 @@ fn load_env( } } + Ok(Arc::new(from_dotenv)) +} + +fn load_env( + manifest_dir: &Path, + config: &Config, + builder: &mut MtimeCacheBuilder, +) -> crate::Result> { + let database_url_env = crate::env_opt(config.common.database_url_var())?; + let offline_dir_env = crate::env_opt("SQLX_OFFLINE_DIR")?.map(PathBuf::from); + let offline_env = crate::env_opt("SQLX_OFFLINE")?.map(|val| is_truthy_bool(&val)); + + // Don't load .env files if all environment variables are set: we may be in + // a non-Cargo build system like buck2. + let dotenv = if database_url_env.is_none() || offline_dir_env.is_none() || offline_env.is_none() + { + Some(load_from_dotenv(manifest_dir, config, builder)?) + } else { + None + }; + Ok(Arc::new(MacrosEnv { - // Make set variables take precedent - database_url: crate::env_opt(config.common.database_url_var())? - .or(from_dotenv.database_url), - offline_dir: crate::env_opt("SQLX_OFFLINE_DIR")? - .map(PathBuf::from) - .or(from_dotenv.offline_dir), - offline: crate::env_opt("SQLX_OFFLINE")? - .map(|val| is_truthy_bool(&val)) - .or(from_dotenv.offline), + // Make set variables take precedence + database_url: database_url_env.or_else(|| dotenv.as_ref()?.database_url.clone()), + offline_dir: offline_dir_env.or_else(|| dotenv.as_ref()?.offline_dir.clone()), + offline: offline_env.or_else(|| dotenv.as_ref()?.offline), })) } @@ -160,3 +188,51 @@ fn load_env( fn is_truthy_bool(val: &str) -> bool { val.eq_ignore_ascii_case("true") || val == "1" } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn load_from_dotenv_reads_env_file() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write( + dir.path().join(".env"), + "DATABASE_URL=postgres://test\nSQLX_OFFLINE_DIR=/some/dir\nSQLX_OFFLINE=true\n", + ) + .unwrap(); + + let cache: MtimeCache> = MtimeCache::new(); + let env = cache + .get_or_try_init(|builder| load_from_dotenv(dir.path(), &Config::default(), builder)) + .unwrap(); + + assert_eq!(env.database_url.as_deref(), Some("postgres://test")); + assert_eq!(env.offline_dir, Some(PathBuf::from("/some/dir"))); + assert_eq!(env.offline, Some(true)); + } + + #[test] + fn load_from_dotenv_empty_when_no_env_file() { + // The ancestor walk finds nothing as long as no ancestor of the OS temp dir has a .env. + let dir = tempfile::tempdir().unwrap(); + + let cache: MtimeCache> = MtimeCache::new(); + let env = cache + .get_or_try_init(|builder| load_from_dotenv(dir.path(), &Config::default(), builder)) + .unwrap(); + + assert!(env.database_url.is_none()); + assert!(env.offline_dir.is_none()); + assert!(env.offline.is_none()); + } + + #[test] + fn resolve_workspace_root_prefers_override() { + let dir = PathBuf::from("/fake/workspace"); + let result = resolve_workspace_root(Some(dir.clone()), || { + panic!("cargo fallback must not be called when override is set") + }); + assert_eq!(result, dir); + } +} From 0f4f93b2ae9c735da21eb3d370138d93819d8fc6 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Thu, 18 Jun 2026 16:48:53 -0700 Subject: [PATCH 2/2] github pr template: driveby typo fix --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 6d193065c2..7e2cc65057 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -19,7 +19,7 @@ Bug fixes should include a regression test which fails before the fix and passes New features *should* include unit or integration tests in the appropriate folders. Database specific tests should go in `tests/`. -Note that unsolicited pull requests implementing large or complex changes may not be reviwed right away. +Note that unsolicited pull requests implementing large or complex changes may not be reviewed right away. Maintainer time and energy is limited and massive unsolicited pull requests require an outsized effort to review. To make the best use of your time and ours, search for and participate in existing discussion on the issue tracker before opening a pull request.