Skip to content

Commit 5b62608

Browse files
Copilotmax-charlamb
andcommitted
Remove --baseline-name command line option per feedback
The baseline should be read from the scraped object file, not passed as a command line argument. Reverted changes that added the --baseline-name option and removed associated tests. Co-authored-by: max-charlamb <[email protected]>
1 parent 5ebc0f5 commit 5b62608

File tree

6 files changed

+8
-321
lines changed

6 files changed

+8
-321
lines changed

src/coreclr/clrdatadescriptors.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ function(generate_data_descriptors)
5959
add_custom_command(
6060
OUTPUT "${CONTRACT_DESCRIPTOR_OUTPUT}"
6161
VERBATIM
62-
COMMAND ${CLR_DOTNET_HOST_PATH} ${CDAC_BUILD_TOOL_BINARY_PATH} compose -i "${CONTRACT_DESCRIPTOR_INPUT}" -o "${CONTRACT_DESCRIPTOR_OUTPUT}" -b "${CONTRACT_BASELINE_DIR}" --baseline-name empty $<TARGET_OBJECTS:${INTERMEDIARY_LIBRARY}>
62+
COMMAND ${CLR_DOTNET_HOST_PATH} ${CDAC_BUILD_TOOL_BINARY_PATH} compose -i "${CONTRACT_DESCRIPTOR_INPUT}" -o "${CONTRACT_DESCRIPTOR_OUTPUT}" -b "${CONTRACT_BASELINE_DIR}" $<TARGET_OBJECTS:${INTERMEDIARY_LIBRARY}>
6363
DEPENDS ${INTERMEDIARY_LIBRARY} ${DATA_DESCRIPTORS_DEPENDENCIES} $<TARGET_OBJECTS:${INTERMEDIARY_LIBRARY}> "${CONTRACT_DESCRIPTOR_INPUT}"
6464
USES_TERMINAL
6565
)

src/coreclr/tools/cdac-build-tool/ComposeCommand.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ internal sealed class ComposeCommand : Command
1313
private readonly Argument<string[]> inputFiles = new("INPUT [INPUTS...]") { Arity = ArgumentArity.OneOrMore, Description = "One or more input files" };
1414
private readonly Option<string> outputFile = new("-o") { Arity = ArgumentArity.ExactlyOne, HelpName = "OUTPUT", Required = true, Description = "Output file" };
1515
private readonly Option<string> baselinePath = new("-b", "--baseline") { Arity = ArgumentArity.ExactlyOne, HelpName = "BASELINEPATH", Description = "Directory containing the baseline contracts"};
16-
private readonly Option<string?> baselineName = new("--baseline-name") { Arity = ArgumentArity.ZeroOrOne, HelpName = "BASELINENAME", Description = "Name of the baseline to use (optional, overrides baseline from input files)" };
1716
private readonly Option<string> templateFile = new("-i", "--input-template") { Arity = ArgumentArity.ExactlyOne, HelpName = "TEMPLATE", Description = "Contract descriptor template to be filled in" };
1817
private readonly Option<bool> _verboseOption;
1918
public ComposeCommand(Option<bool> verboseOption) : base("compose")
@@ -22,7 +21,6 @@ public ComposeCommand(Option<bool> verboseOption) : base("compose")
2221
Add(inputFiles);
2322
Add(outputFile);
2423
Add(baselinePath);
25-
Add(baselineName);
2624
Add(templateFile);
2725
SetAction(Run);
2826
}
@@ -66,8 +64,7 @@ private async Task<int> Run(ParseResult parse, CancellationToken token = default
6664
return 1;
6765
}
6866
var verbose = parse.GetValue(_verboseOption);
69-
var baselineNameValue = parse.GetValue(baselineName);
70-
var builder = new DataDescriptorModel.Builder(baselinesDir, baselineNameValue);
67+
var builder = new DataDescriptorModel.Builder(baselinesDir);
7168
var scraper = new ObjectFileScraper(verbose, builder);
7269
foreach (var input in inputs)
7370
{

src/coreclr/tools/cdac-build-tool/DataDescriptorModel.cs

Lines changed: 6 additions & 155 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ internal void DumpModel()
8484
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
8585
DictionaryKeyPolicy = null, // leave unchanged
8686
};
87-
8887
public string ToJson()
8988
{
9089
// always writes the "compact" format, see data_descriptor.md
@@ -96,19 +95,15 @@ public class Builder
9695
private string _baseline;
9796
private readonly string _baselinesDir;
9897
private bool _baselineParsed;
99-
private readonly string? _overrideBaselineName;
10098
private readonly Dictionary<string, TypeModelBuilder> _types = new();
10199
private readonly Dictionary<string, GlobalBuilder> _globals = new();
102100
private readonly Dictionary<string, GlobalBuilder> _subDescriptors = new();
103101
private readonly Dictionary<string, ContractBuilder> _contracts = new();
104-
private DataDescriptorModel? _baselineModel;
105-
106-
public Builder(string baselinesDir, string? overrideBaselineName = null)
102+
public Builder(string baselinesDir)
107103
{
108104
_baseline = string.Empty;
109105
_baselineParsed = false;
110106
_baselinesDir = baselinesDir;
111-
_overrideBaselineName = overrideBaselineName;
112107
}
113108

114109
public uint PlatformFlags {get; set;}
@@ -172,12 +167,6 @@ public void AddOrUpdateContract(string name, int version)
172167

173168
public void SetBaseline(string baseline)
174169
{
175-
// If an override baseline name was provided via command line, use it instead
176-
if (_overrideBaselineName is not null)
177-
{
178-
baseline = _overrideBaselineName;
179-
}
180-
181170
if (_baseline != string.Empty && _baseline != baseline)
182171
{
183172
throw new InvalidOperationException($"Baseline already set to {_baseline} cannot set to {baseline}");
@@ -201,54 +190,20 @@ public void SetBaseline(string baseline)
201190

202191
private void ParseBaseline()
203192
{
204-
// Load the baseline file to check if it's empty
205-
var baselinePath = Path.Combine(_baselinesDir, _baseline + ".jsonc");
206-
if (!File.Exists(baselinePath))
207-
{
208-
baselinePath = Path.Combine(_baselinesDir, _baseline + ".json");
209-
if (!File.Exists(baselinePath))
210-
{
211-
throw new InvalidOperationException($"Baseline file not found: {_baseline}.json or {_baseline}.jsonc in {_baselinesDir}");
212-
}
213-
}
214-
215-
var json = File.ReadAllText(baselinePath);
216-
217-
// Check if this is an empty baseline (version 0 with no data)
218-
using var doc = JsonDocument.Parse(json, new JsonDocumentOptions
193+
if (_baseline != "empty")
219194
{
220-
CommentHandling = JsonCommentHandling.Skip,
221-
AllowTrailingCommas = true
222-
});
223-
224-
if (doc.RootElement.TryGetProperty("version", out var versionProp) &&
225-
versionProp.GetInt32() == 0)
226-
{
227-
// Empty baseline - no types, globals, or contracts to load
228-
_baselineModel = null;
229-
return;
195+
throw new InvalidOperationException("TODO: [cdac] - implement baseline parsing");
230196
}
231-
232-
// TODO: [cdac] - implement non-empty baseline parsing
233-
// For now, we only support empty baselines (version 0) which contain no data
234-
// Future work: Add proper JSON deserialization for non-empty baselines
235-
// This would require custom JsonConverters for the compact array format used
236-
// in baseline files (e.g., "Field1": [0, "uint32"] instead of expanded objects)
237-
throw new InvalidOperationException($"Non-empty baseline parsing is not yet implemented for baseline '{_baseline}'. Only empty baselines (version 0) are currently supported.");
238197
}
239198

240199
public DataDescriptorModel Build()
241200
{
242201
var types = new Dictionary<string, TypeModel>();
243-
var globals = new Dictionary<string, GlobalModel>();
244-
var subDescriptors = new Dictionary<string, GlobalModel>();
245-
var contracts = new Dictionary<string, int>();
246-
247-
// Build current model
248202
foreach (var (typeName, typeBuilder) in _types)
249203
{
250204
types[typeName] = typeBuilder.Build(typeName);
251205
}
206+
var globals = new Dictionary<string, GlobalModel>();
252207
foreach (var (globalName, globalBuilder) in _globals)
253208
{
254209
GlobalValue? v = globalBuilder.Value;
@@ -258,6 +213,7 @@ public DataDescriptorModel Build()
258213
}
259214
globals[globalName] = new GlobalModel { Type = globalBuilder.Type, Value = v.Value };
260215
}
216+
var subDescriptors = new Dictionary<string, GlobalModel>();
261217
foreach (var (subDescriptorName, subDescriptorBuilder) in _subDescriptors)
262218
{
263219
GlobalValue? v = subDescriptorBuilder.Value;
@@ -267,118 +223,13 @@ public DataDescriptorModel Build()
267223
}
268224
subDescriptors[subDescriptorName] = new GlobalModel { Type = subDescriptorBuilder.Type, Value = v.Value };
269225
}
226+
var contracts = new Dictionary<string, int>();
270227
foreach (var (contractName, contractBuilder) in _contracts)
271228
{
272229
contracts[contractName] = contractBuilder.Build();
273230
}
274-
275-
// If we have a baseline model loaded, only include differences
276-
// Note: Empty baselines (version 0) set _baselineModel to null, so they result in full model output
277-
if (_baselineModel is not null)
278-
{
279-
types = ComputeTypeDifferences(types, _baselineModel.Types);
280-
globals = ComputeGlobalDifferences(globals, _baselineModel.Globals);
281-
subDescriptors = ComputeGlobalDifferences(subDescriptors, _baselineModel.SubDescriptors);
282-
contracts = ComputeContractDifferences(contracts, _baselineModel.Contracts);
283-
}
284-
285231
return new DataDescriptorModel(_baseline, types, globals, subDescriptors, contracts, PlatformFlags);
286232
}
287-
288-
private static Dictionary<string, TypeModel> ComputeTypeDifferences(
289-
IReadOnlyDictionary<string, TypeModel> current,
290-
IReadOnlyDictionary<string, TypeModel> baseline)
291-
{
292-
var differences = new Dictionary<string, TypeModel>();
293-
294-
foreach (var (typeName, currentType) in current)
295-
{
296-
if (!baseline.TryGetValue(typeName, out var baselineType))
297-
{
298-
// New type not in baseline
299-
differences[typeName] = currentType;
300-
continue;
301-
}
302-
303-
// Check if type has differences
304-
if (!TypesEqual(currentType, baselineType))
305-
{
306-
differences[typeName] = currentType;
307-
}
308-
}
309-
310-
return differences;
311-
}
312-
313-
private static bool TypesEqual(TypeModel a, TypeModel b)
314-
{
315-
if (a.Size != b.Size)
316-
return false;
317-
318-
if (a.Fields.Count != b.Fields.Count)
319-
return false;
320-
321-
foreach (var (fieldName, fieldA) in a.Fields)
322-
{
323-
if (!b.Fields.TryGetValue(fieldName, out var fieldB))
324-
return false;
325-
326-
if (fieldA.Type != fieldB.Type || fieldA.Offset != fieldB.Offset)
327-
return false;
328-
}
329-
330-
return true;
331-
}
332-
333-
private static Dictionary<string, GlobalModel> ComputeGlobalDifferences(
334-
IReadOnlyDictionary<string, GlobalModel> current,
335-
IReadOnlyDictionary<string, GlobalModel> baseline)
336-
{
337-
var differences = new Dictionary<string, GlobalModel>();
338-
339-
foreach (var (globalName, currentGlobal) in current)
340-
{
341-
if (!baseline.TryGetValue(globalName, out var baselineGlobal))
342-
{
343-
// New global not in baseline
344-
differences[globalName] = currentGlobal;
345-
continue;
346-
}
347-
348-
// Check if global has differences
349-
if (currentGlobal.Type != baselineGlobal.Type || currentGlobal.Value != baselineGlobal.Value)
350-
{
351-
differences[globalName] = currentGlobal;
352-
}
353-
}
354-
355-
return differences;
356-
}
357-
358-
private static Dictionary<string, int> ComputeContractDifferences(
359-
IReadOnlyDictionary<string, int> current,
360-
IReadOnlyDictionary<string, int> baseline)
361-
{
362-
var differences = new Dictionary<string, int>();
363-
364-
foreach (var (contractName, currentVersion) in current)
365-
{
366-
if (!baseline.TryGetValue(contractName, out var baselineVersion))
367-
{
368-
// New contract not in baseline
369-
differences[contractName] = currentVersion;
370-
continue;
371-
}
372-
373-
// Check if version has changed
374-
if (currentVersion != baselineVersion)
375-
{
376-
differences[contractName] = currentVersion;
377-
}
378-
}
379-
380-
return differences;
381-
}
382233
}
383234

384235
public class TypeModelBuilder

src/coreclr/tools/cdac-build-tool/cdac-build-tool.csproj

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,6 @@
1616
<RootNamespace>Microsoft.DotNet.Diagnostics.DataContract</RootNamespace>
1717
</PropertyGroup>
1818

19-
<ItemGroup>
20-
<Compile Remove="tests/**/*.cs" />
21-
</ItemGroup>
22-
2319
<ItemGroup>
2420
<PackageReference Include="System.CommandLine" Version="$(SystemCommandLineVersion)" />
2521
</ItemGroup>

0 commit comments

Comments
 (0)