Skip to content

Commit 79dbfe3

Browse files
committed
Support per-unit option overrides
1 parent 4a64b5d commit 79dbfe3

File tree

7 files changed

+159
-57
lines changed

7 files changed

+159
-57
lines changed

config.schema.json

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,20 @@
175175
"additionalProperties": {
176176
"type": "string"
177177
}
178+
},
179+
"options": {
180+
"type": "object",
181+
"description": "Diff configuration options that should be applied when this unit is active.",
182+
"additionalProperties": {
183+
"oneOf": [
184+
{
185+
"type": "boolean"
186+
},
187+
{
188+
"type": "string"
189+
}
190+
]
191+
}
178192
}
179193
}
180194
},

objdiff-cli/src/cmd/diff.rs

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ use objdiff_core::{
2424
watcher::{Watcher, create_watcher},
2525
},
2626
config::{
27-
ProjectConfig, ProjectObject, ProjectObjectMetadata, apply_project_options, build_globset,
27+
ProjectConfig, ProjectObject, ProjectObjectMetadata, ProjectOptions, apply_project_options,
28+
build_globset,
2829
path::{check_path_buf, platform_path, platform_path_serde_option},
2930
},
3031
diff::{DiffObjConfig, MappingConfig, ObjectDiff},
@@ -77,11 +78,11 @@ pub struct Args {
7778
}
7879

7980
pub fn run(args: Args) -> Result<()> {
80-
let (target_path, base_path, project_config) =
81+
let (target_path, base_path, project_config, unit_options) =
8182
match (&args.target, &args.base, &args.project, &args.unit) {
8283
(Some(_), Some(_), None, None)
8384
| (Some(_), None, None, None)
84-
| (None, Some(_), None, None) => (args.target.clone(), args.base.clone(), None),
85+
| (None, Some(_), None, None) => (args.target.clone(), args.base.clone(), None, None),
8586
(None, None, p, u) => {
8687
let project = match p {
8788
Some(project) => project.clone(),
@@ -106,36 +107,40 @@ pub fn run(args: Args) -> Result<()> {
106107
.base_dir
107108
.as_ref()
108109
.map(|p| project.join(p.with_platform_encoding()));
109-
let objects = project_config
110-
.units
110+
let units = project_config.units.as_deref().unwrap_or_default();
111+
let objects = units
111112
.iter()
112-
.flatten()
113-
.map(|o| {
114-
ObjectConfig::new(
115-
o,
116-
&project,
117-
target_obj_dir.as_deref(),
118-
base_obj_dir.as_deref(),
113+
.enumerate()
114+
.map(|(idx, o)| {
115+
(
116+
ObjectConfig::new(
117+
o,
118+
&project,
119+
target_obj_dir.as_deref(),
120+
base_obj_dir.as_deref(),
121+
),
122+
idx,
119123
)
120124
})
121125
.collect::<Vec<_>>();
122-
let object = if let Some(u) = u {
126+
let (object, unit_idx) = if let Some(u) = u {
123127
objects
124128
.iter()
125-
.find(|obj| obj.name == *u)
129+
.find(|(obj, _)| obj.name == *u)
130+
.map(|(obj, idx)| (obj, *idx))
126131
.ok_or_else(|| anyhow!("Unit not found: {}", u))?
127132
} else if let Some(symbol_name) = &args.symbol {
128133
let mut idx = None;
129134
let mut count = 0usize;
130-
for (i, obj) in objects.iter().enumerate() {
135+
for (i, (obj, unit_idx)) in objects.iter().enumerate() {
131136
if obj
132137
.target_path
133138
.as_deref()
134139
.map(|o| obj::read::has_function(o.as_ref(), symbol_name))
135140
.transpose()?
136141
.unwrap_or(false)
137142
{
138-
idx = Some(i);
143+
idx = Some((i, *unit_idx));
139144
count += 1;
140145
if count > 1 {
141146
break;
@@ -144,7 +149,7 @@ pub fn run(args: Args) -> Result<()> {
144149
}
145150
match (count, idx) {
146151
(0, None) => bail!("Symbol not found: {}", symbol_name),
147-
(1, Some(i)) => &objects[i],
152+
(1, Some((i, unit_idx))) => (&objects[i].0, unit_idx),
148153
(2.., Some(_)) => bail!(
149154
"Multiple instances of {} were found, try specifying a unit",
150155
symbol_name
@@ -154,24 +159,29 @@ pub fn run(args: Args) -> Result<()> {
154159
} else {
155160
bail!("Must specify one of: symbol, project and unit, target and base objects")
156161
};
162+
let unit_options = units.get(unit_idx).and_then(|u| u.options().cloned());
157163
let target_path = object.target_path.clone();
158164
let base_path = object.base_path.clone();
159-
(target_path, base_path, Some(project_config))
165+
(target_path, base_path, Some(project_config), unit_options)
160166
}
161167
_ => bail!("Either target and base or project and unit must be specified"),
162168
};
163169

164-
run_interactive(args, target_path, base_path, project_config)
170+
run_interactive(args, target_path, base_path, project_config, unit_options)
165171
}
166172

167173
fn build_config_from_args(
168174
args: &Args,
169175
project_config: Option<&ProjectConfig>,
176+
unit_options: Option<&ProjectOptions>,
170177
) -> Result<(DiffObjConfig, MappingConfig)> {
171178
let mut diff_config = DiffObjConfig::default();
172179
if let Some(options) = project_config.and_then(|config| config.options.as_ref()) {
173180
apply_project_options(&mut diff_config, options)?;
174181
}
182+
if let Some(options) = unit_options {
183+
apply_project_options(&mut diff_config, options)?;
184+
}
175185
apply_config_args(&mut diff_config, &args.config)?;
176186
let mut mapping_config = MappingConfig {
177187
mappings: Default::default(),
@@ -322,11 +332,13 @@ fn run_interactive(
322332
target_path: Option<Utf8PlatformPathBuf>,
323333
base_path: Option<Utf8PlatformPathBuf>,
324334
project_config: Option<ProjectConfig>,
335+
unit_options: Option<ProjectOptions>,
325336
) -> Result<()> {
326337
let Some(symbol_name) = &args.symbol else { bail!("Interactive mode requires a symbol name") };
327338
let time_format = time::format_description::parse_borrowed::<2>("[hour]:[minute]:[second]")
328339
.context("Failed to parse time format")?;
329-
let (diff_obj_config, mapping_config) = build_config_from_args(&args, project_config.as_ref())?;
340+
let (diff_obj_config, mapping_config) =
341+
build_config_from_args(&args, project_config.as_ref(), unit_options.as_ref())?;
330342
let mut state = AppState {
331343
jobs: Default::default(),
332344
waker: Default::default(),

objdiff-cli/src/cmd/report.rs

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use objdiff_core::{
77
ChangeItem, ChangeItemInfo, ChangeUnit, Changes, ChangesInput, Measures, REPORT_VERSION,
88
Report, ReportCategory, ReportItem, ReportItemMetadata, ReportUnit, ReportUnitMetadata,
99
},
10-
config::{apply_project_options, path::platform_path},
10+
config::{ProjectObject, ProjectOptions, apply_project_options, path::platform_path},
1111
diff,
1212
obj::{self, SectionKind, SymbolFlag, SymbolKind},
1313
};
@@ -83,7 +83,7 @@ pub fn run(args: Args) -> Result<()> {
8383
}
8484

8585
fn generate(args: GenerateArgs) -> Result<()> {
86-
let mut diff_config = diff::DiffObjConfig {
86+
let base_diff_config = diff::DiffObjConfig {
8787
function_reloc_diffs: diff::FunctionRelocDiffs::None,
8888
combine_data_sections: true,
8989
combine_text_sections: true,
@@ -100,35 +100,44 @@ fn generate(args: GenerateArgs) -> Result<()> {
100100
Some((Err(err), _)) => bail!("Failed to load project configuration: {}", err),
101101
None => bail!("No project configuration found"),
102102
};
103-
if let Some(options) = project.options.as_ref() {
104-
apply_project_options(&mut diff_config, options)?;
105-
}
106-
apply_config_args(&mut diff_config, &args.config)?;
107-
info!(
108-
"Generating report for {} units (using {} threads)",
109-
project.units().len(),
110-
if args.deduplicate { 1 } else { rayon::current_num_threads() }
111-
);
112-
113103
let target_obj_dir =
114104
project.target_dir.as_ref().map(|p| project_dir.join(p.with_platform_encoding()));
115105
let base_obj_dir =
116106
project.base_dir.as_ref().map(|p| project_dir.join(p.with_platform_encoding()));
117-
let objects = project
118-
.units
107+
let project_units = project.units.as_deref().unwrap_or_default();
108+
let objects = project_units
119109
.iter()
120-
.flatten()
121-
.map(|o| {
122-
ObjectConfig::new(o, project_dir, target_obj_dir.as_deref(), base_obj_dir.as_deref())
110+
.enumerate()
111+
.map(|(idx, o)| {
112+
(
113+
ObjectConfig::new(
114+
o,
115+
project_dir,
116+
target_obj_dir.as_deref(),
117+
base_obj_dir.as_deref(),
118+
),
119+
idx,
120+
)
123121
})
124122
.collect::<Vec<_>>();
123+
info!(
124+
"Generating report for {} units (using {} threads)",
125+
objects.len(),
126+
if args.deduplicate { 1 } else { rayon::current_num_threads() }
127+
);
125128

126129
let start = Instant::now();
127130
let mut units = vec![];
128131
let mut existing_functions: HashSet<String> = HashSet::new();
129132
if args.deduplicate {
130133
// If deduplicating, we need to run single-threaded
131-
for object in &objects {
134+
for (object, unit_idx) in &objects {
135+
let diff_config = build_unit_diff_config(
136+
&base_diff_config,
137+
project.options.as_ref(),
138+
project_units.get(*unit_idx).and_then(ProjectObject::options),
139+
&args.config,
140+
)?;
132141
if let Some(unit) = report_object(object, &diff_config, Some(&mut existing_functions))?
133142
{
134143
units.push(unit);
@@ -137,7 +146,15 @@ fn generate(args: GenerateArgs) -> Result<()> {
137146
} else {
138147
let vec = objects
139148
.par_iter()
140-
.map(|object| report_object(object, &diff_config, None))
149+
.map(|(object, unit_idx)| {
150+
let diff_config = build_unit_diff_config(
151+
&base_diff_config,
152+
project.options.as_ref(),
153+
project_units.get(*unit_idx).and_then(ProjectObject::options),
154+
&args.config,
155+
)?;
156+
report_object(object, &diff_config, None)
157+
})
141158
.collect::<Result<Vec<Option<ReportUnit>>>>()?;
142159
units = vec.into_iter().flatten().collect();
143160
}
@@ -159,6 +176,24 @@ fn generate(args: GenerateArgs) -> Result<()> {
159176
Ok(())
160177
}
161178

179+
fn build_unit_diff_config(
180+
base: &diff::DiffObjConfig,
181+
project_options: Option<&ProjectOptions>,
182+
unit_options: Option<&ProjectOptions>,
183+
cli_args: &[String],
184+
) -> Result<diff::DiffObjConfig> {
185+
let mut diff_config = base.clone();
186+
if let Some(options) = project_options {
187+
apply_project_options(&mut diff_config, options)?;
188+
}
189+
if let Some(options) = unit_options {
190+
apply_project_options(&mut diff_config, options)?;
191+
}
192+
// CLI args override project and unit options
193+
apply_config_args(&mut diff_config, cli_args)?;
194+
Ok(diff_config)
195+
}
196+
162197
fn report_object(
163198
object: &ObjectConfig,
164199
diff_config: &diff::DiffObjConfig,

objdiff-core/src/config/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ pub struct ProjectObject {
119119
pub metadata: Option<ProjectObjectMetadata>,
120120
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
121121
pub symbol_mappings: Option<BTreeMap<String, String>>,
122+
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
123+
pub options: Option<ProjectOptions>,
122124
}
123125

124126
#[derive(Default, Clone)]
@@ -191,6 +193,8 @@ impl ProjectObject {
191193
pub fn auto_generated(&self) -> Option<bool> {
192194
self.metadata.as_ref().and_then(|m| m.auto_generated)
193195
}
196+
197+
pub fn options(&self) -> Option<&ProjectOptions> { self.options.as_ref() }
194198
}
195199

196200
#[derive(Default, Clone, Eq, PartialEq)]

objdiff-gui/src/app.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,18 @@ impl AppState {
336336

337337
pub fn effective_diff_config(&self) -> DiffObjConfig {
338338
let mut config = self.config.diff_obj_config.clone();
339-
if let Some(options) =
340-
self.current_project_config.as_ref().and_then(|project| project.options.as_ref())
341-
{
342-
// Ignore errors here, we display them when loading the project config
343-
let _ = apply_project_options(&mut config, options);
339+
if let Some(project) = self.current_project_config.as_ref() {
340+
if let Some(options) = project.options.as_ref() {
341+
// Ignore errors here, we display them when loading the project config
342+
let _ = apply_project_options(&mut config, options);
343+
}
344+
if let Some(selected) = self.config.selected_obj.as_ref()
345+
&& let Some(units) = project.units.as_deref()
346+
&& let Some(unit) = units.iter().find(|unit| unit.name() == selected.name)
347+
&& let Some(options) = unit.options()
348+
{
349+
let _ = apply_project_options(&mut config, options);
350+
}
344351
}
345352
config
346353
}

objdiff-gui/src/config.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,29 @@ pub fn load_project_config(state: &mut AppState) -> Result<()> {
138138
state.show_error_toast("Failed to apply project config options", &e);
139139
}
140140
}
141+
if let Some(project) = state.current_project_config.as_ref()
142+
&& let Some(units) = project.units.as_deref()
143+
{
144+
let mut unit_option_errors = Vec::new();
145+
for unit in units {
146+
if let Some(options) = unit.options() {
147+
let mut diff_config = DiffObjConfig::default();
148+
if let Err(e) = apply_project_options(&mut diff_config, options) {
149+
unit_option_errors.push((unit.name().to_string(), e));
150+
}
151+
}
152+
}
153+
for (unit_name, error) in unit_option_errors {
154+
log::error!(
155+
"Failed to apply project config options for unit {}: {error:#}",
156+
unit_name
157+
);
158+
state.show_error_toast(
159+
&format!("Failed to apply project config options for unit {unit_name}"),
160+
&error,
161+
);
162+
}
163+
}
141164

142165
// Reload selected object
143166
if let Some(selected_obj) = &state.config.selected_obj {

objdiff-gui/src/views/config.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#[cfg(all(windows, feature = "wsl"))]
22
use std::string::FromUtf16Error;
3-
use std::{mem::take, path::MAIN_SEPARATOR, str::FromStr};
3+
use std::{mem::take, path::MAIN_SEPARATOR};
44

55
#[cfg(all(windows, feature = "wsl"))]
66
use anyhow::{Context, Result};
@@ -894,20 +894,27 @@ fn config_property_ui(
894894
property_id: ConfigPropertyId,
895895
) -> bool {
896896
let mut changed = false;
897-
let project_override = state
898-
.current_project_config
899-
.as_ref()
900-
.and_then(|config| config.options.as_ref())
901-
.and_then(|options| {
902-
options.iter().find_map(|(key, value)| {
903-
ConfigPropertyId::from_str(key).ok().filter(|id| *id == property_id).map(|_| value)
904-
})
905-
});
906-
let is_overridden = project_override.is_some();
907-
let effective_value =
897+
let is_overridden = state.current_project_config.as_ref().is_some_and(|config| {
898+
let key = property_id.name();
899+
if let Some(selected) = state.config.selected_obj.as_ref()
900+
&& let Some(units) = config.units.as_deref()
901+
&& let Some(unit) = units.iter().find(|unit| unit.name() == selected.name)
902+
&& let Some(options) = unit.options()
903+
&& options.contains_key(key)
904+
{
905+
return true;
906+
}
907+
if let Some(options) = config.options.as_ref()
908+
&& options.contains_key(key)
909+
{
910+
return true;
911+
}
912+
false
913+
});
914+
let override_value =
908915
is_overridden.then(|| state.effective_diff_config().get_property_value(property_id));
909916
let base_value = state.config.diff_obj_config.get_property_value(property_id);
910-
match (property_id.kind(), base_value, effective_value) {
917+
match (property_id.kind(), base_value, override_value) {
911918
(
912919
ConfigPropertyKind::Boolean,
913920
ConfigPropertyValue::Boolean(base_checked),

0 commit comments

Comments
 (0)