Skip to content

Commit 56dac46

Browse files
encounterencounter
authored
Support overriding diff options in project config (& for individual units) (#263)
* Support loading diff options from project config * Support per-unit option overrides
1 parent 1866158 commit 56dac46

File tree

11 files changed

+414
-105
lines changed

11 files changed

+414
-105
lines changed

‎Cargo.lock‎

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config.schema.json‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,25 @@
111111
"items": {
112112
"$ref": "#/$defs/progress_category"
113113
}
114+
},
115+
"options": {
116+
"type": "object",
117+
"description": "Diff configuration options that should be applied automatically when the project is loaded.",
118+
"additionalProperties": {
119+
"oneOf": [
120+
{
121+
"type": "boolean"
122+
},
123+
{
124+
"type": "string"
125+
}
126+
]
127+
},
128+
"examples": [
129+
{
130+
"demangler": "gnu_legacy"
131+
}
132+
]
114133
}
115134
},
116135
"$defs": {
@@ -156,6 +175,20 @@
156175
"additionalProperties": {
157176
"type": "string"
158177
}
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+
}
159192
}
160193
}
161194
},

‎objdiff-cli/src/cmd/diff.rs‎

Lines changed: 39 additions & 21 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, 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,18 +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

167-
fn build_config_from_args(args: &Args) -> Result<(DiffObjConfig, MappingConfig)> {
173+
fn build_config_from_args(
174+
args: &Args,
175+
project_config: Option<&ProjectConfig>,
176+
unit_options: Option<&ProjectOptions>,
177+
) -> Result<(DiffObjConfig, MappingConfig)> {
168178
let mut diff_config = DiffObjConfig::default();
179+
if let Some(options) = project_config.and_then(|config| config.options.as_ref()) {
180+
apply_project_options(&mut diff_config, options)?;
181+
}
182+
if let Some(options) = unit_options {
183+
apply_project_options(&mut diff_config, options)?;
184+
}
169185
apply_config_args(&mut diff_config, &args.config)?;
170186
let mut mapping_config = MappingConfig {
171187
mappings: Default::default(),
@@ -316,11 +332,13 @@ fn run_interactive(
316332
target_path: Option<Utf8PlatformPathBuf>,
317333
base_path: Option<Utf8PlatformPathBuf>,
318334
project_config: Option<ProjectConfig>,
335+
unit_options: Option<ProjectOptions>,
319336
) -> Result<()> {
320337
let Some(symbol_name) = &args.symbol else { bail!("Interactive mode requires a symbol name") };
321338
let time_format = time::format_description::parse_borrowed::<2>("[hour]:[minute]:[second]")
322339
.context("Failed to parse time format")?;
323-
let (diff_obj_config, mapping_config) = build_config_from_args(&args)?;
340+
let (diff_obj_config, mapping_config) =
341+
build_config_from_args(&args, project_config.as_ref(), unit_options.as_ref())?;
324342
let mut state = AppState {
325343
jobs: Default::default(),
326344
waker: Default::default(),

‎objdiff-cli/src/cmd/report.rs‎

Lines changed: 54 additions & 16 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::path::platform_path,
10+
config::{ProjectObject, ProjectOptions, apply_project_options, path::platform_path},
1111
diff,
1212
obj::{self, SectionKind, SymbolFlag, SymbolKind},
1313
};
@@ -83,14 +83,13 @@ 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,
9090
ppc_calculate_pool_relocations: false,
9191
..Default::default()
9292
};
93-
apply_config_args(&mut diff_config, &args.config)?;
9493

9594
let output_format = OutputFormat::from_option(args.format.as_deref())?;
9695
let project_dir = args.project.as_deref().unwrap_or_else(|| Utf8PlatformPath::new("."));
@@ -101,31 +100,44 @@ fn generate(args: GenerateArgs) -> Result<()> {
101100
Some((Err(err), _)) => bail!("Failed to load project configuration: {}", err),
102101
None => bail!("No project configuration found"),
103102
};
104-
info!(
105-
"Generating report for {} units (using {} threads)",
106-
project.units().len(),
107-
if args.deduplicate { 1 } else { rayon::current_num_threads() }
108-
);
109-
110103
let target_obj_dir =
111104
project.target_dir.as_ref().map(|p| project_dir.join(p.with_platform_encoding()));
112105
let base_obj_dir =
113106
project.base_dir.as_ref().map(|p| project_dir.join(p.with_platform_encoding()));
114-
let objects = project
115-
.units
107+
let project_units = project.units.as_deref().unwrap_or_default();
108+
let objects = project_units
116109
.iter()
117-
.flatten()
118-
.map(|o| {
119-
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+
)
120121
})
121122
.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+
);
122128

123129
let start = Instant::now();
124130
let mut units = vec![];
125131
let mut existing_functions: HashSet<String> = HashSet::new();
126132
if args.deduplicate {
127133
// If deduplicating, we need to run single-threaded
128-
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+
)?;
129141
if let Some(unit) = report_object(object, &diff_config, Some(&mut existing_functions))?
130142
{
131143
units.push(unit);
@@ -134,7 +146,15 @@ fn generate(args: GenerateArgs) -> Result<()> {
134146
} else {
135147
let vec = objects
136148
.par_iter()
137-
.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+
})
138158
.collect::<Result<Vec<Option<ReportUnit>>>>()?;
139159
units = vec.into_iter().flatten().collect();
140160
}
@@ -156,6 +176,24 @@ fn generate(args: GenerateArgs) -> Result<()> {
156176
Ok(())
157177
}
158178

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+
159197
fn report_object(
160198
object: &ObjectConfig,
161199
diff_config: &diff::DiffObjConfig,

0 commit comments

Comments
 (0)