Skip to content

Commit 96ab845

Browse files
Refactor schema.rs using a cargo command builder (pgcentralfoundation#2187)
I find the code in schema.rs to be poorly factored in general. A great deal of code is repeated in `first_build` and `second_build`. In other places, arguments are created long before they are needed. All of this makes it hard to follow basic data flow or understand what is distinct about each invocation. First, split some functions more finely to make it easier to follow where arguments are used. - `compute_symbols` grows `find_and_compute_symbols` as its "front end" - `generate_schema` forks into `_for_cli` and `_implicit` Secondly, absorb repeated code in `first_build` and `second_build` into `cargo_pgrx::cargo::Cargo`, a builder for `process::Command`s. This doesn't _entirely_ "pay off" yet due to the weight of boilerplate, some of the distinctions being still weak, and not all sites in cargo-pgrx using improved abstractions. This will improve over time.
1 parent 025a1f8 commit 96ab845

3 files changed

Lines changed: 258 additions & 163 deletions

File tree

cargo-pgrx/src/cargo.rs

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,178 @@
77
//LICENSE All rights reserved.
88
//LICENSE
99
//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file.
10+
use crate::profile::CargoProfile;
11+
use std::collections::BTreeMap;
12+
use std::path::PathBuf;
13+
use std::{env, process};
14+
15+
/// Configuration for building a cargo execution
16+
#[derive(Default, Clone, Debug)]
17+
pub struct Cargo {
18+
subcmd: String,
19+
features: clap_cargo::Features,
20+
stdio: [Stdio; 3],
21+
manifest: Option<PathBuf>,
22+
package: String,
23+
log_level: Option<String>,
24+
target: Option<String>,
25+
profile: CargoProfile,
26+
// use a BTreeMap for deterministic order of iteration
27+
more_args: BTreeMap<String, Vec<String>>,
28+
}
29+
30+
impl Cargo {
31+
pub fn subcommand(mut self, s: &str) -> Self {
32+
self.subcmd.clear();
33+
self.subcmd.push_str(s);
34+
self
35+
}
36+
37+
pub fn std_streams(mut self, streams: [Stdio; 3]) -> Self {
38+
self.stdio = streams;
39+
self
40+
}
41+
42+
pub fn manifest_path(mut self, path: Option<PathBuf>) -> Self {
43+
self.manifest = path;
44+
self
45+
}
46+
47+
pub fn package(mut self, package: String) -> Self {
48+
self.package = package;
49+
self
50+
}
51+
52+
pub fn target(mut self, target: Option<String>) -> Self {
53+
self.target = target;
54+
self
55+
}
56+
57+
pub fn profile(mut self, profile: CargoProfile) -> Self {
58+
self.profile = profile;
59+
self
60+
}
61+
62+
pub fn log_level(mut self, level: Option<String>) -> Self {
63+
self.log_level = level;
64+
self
65+
}
66+
67+
pub fn flag(mut self, flag: impl Into<String>) -> Self {
68+
self.more_args.insert(flag.into(), Vec::new());
69+
self
70+
}
71+
72+
pub fn flag_args(mut self, flag: impl Into<String>, args: Vec<String>) -> Self {
73+
self.more_args.insert(flag.into(), args);
74+
self
75+
}
76+
77+
pub fn features(mut self, features: clap_cargo::Features) -> Self {
78+
self.features = features;
79+
self
80+
}
81+
82+
#[track_caller]
83+
pub fn into_command(self) -> process::Command {
84+
let mut cmd = cargo();
85+
86+
// subcommand *must* go first
87+
if self.subcmd != "" {
88+
cmd.arg(&self.subcmd);
89+
} else {
90+
panic!("`Cargo::into_command` requires a subcommand to be set, was: {self:?}")
91+
}
92+
93+
let Cargo {
94+
features,
95+
stdio,
96+
manifest,
97+
log_level,
98+
target,
99+
profile,
100+
package,
101+
subcmd: _,
102+
more_args,
103+
} = self;
104+
105+
// set most-interesting flags first, like profile, target, and manifest-path
106+
// so that when we read dumped command lines we can see that info first
107+
cmd.args(profile.cargo_args());
108+
109+
if let Some(target) = target {
110+
cmd.arg("--target").arg(target);
111+
}
112+
if let Some(manifest) = manifest {
113+
cmd.arg("--manifest-path").arg(manifest);
114+
}
115+
if !package.is_empty() {
116+
cmd.arg("--package").arg(package);
117+
}
118+
119+
// set std streams
120+
let [stdin, stdout, stderr] = stdio;
121+
if let Some(stdio) = stdin.into_stdio() {
122+
cmd.stdin(stdio);
123+
}
124+
if let Some(stdio) = stdout.into_stdio() {
125+
cmd.stdout(stdio);
126+
}
127+
if let Some(stdio) = stderr.into_stdio() {
128+
cmd.stderr(stdio);
129+
}
130+
131+
// set features
132+
if features.no_default_features {
133+
cmd.arg("--no-default-features");
134+
}
135+
136+
if features.all_features {
137+
cmd.arg("--all-features");
138+
}
139+
140+
if !features.features.is_empty() {
141+
cmd.arg("--features");
142+
cmd.arg(features.features.join(" "));
143+
}
144+
145+
// And now the miscellaneous build flags!
146+
let flags = env::var("PGRX_BUILD_FLAGS").unwrap_or_default();
147+
for arg in flags.split_ascii_whitespace() {
148+
cmd.arg(arg);
149+
}
150+
151+
// set envs
152+
if let Some(log_level) = log_level {
153+
cmd.env("RUST_LOG", log_level);
154+
}
155+
156+
for (flag, args) in more_args {
157+
cmd.arg(flag).args(args);
158+
}
159+
160+
cmd
161+
}
162+
}
163+
164+
#[derive(Clone, Copy, Debug, Default, PartialEq)]
165+
pub enum Stdio {
166+
Inherit,
167+
Null,
168+
#[default]
169+
Default,
170+
}
171+
172+
impl Stdio {
173+
fn into_stdio(self) -> Option<process::Stdio> {
174+
match self {
175+
Stdio::Inherit => Some(process::Stdio::inherit()),
176+
Stdio::Null => Some(process::Stdio::null()),
177+
Stdio::Default => None,
178+
}
179+
}
180+
}
181+
10182
pub(crate) fn cargo() -> std::process::Command {
11183
let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into());
12184
std::process::Command::new(cargo)

0 commit comments

Comments
 (0)