Add ProjectGuid support for SQL projects and CLI - #970
Conversation
Added --projectguid/-pg CLI option and MSBuild property to set ProjectGuid for SQL projects. Updated templates and .csproj files to generate and substitute ProjectGuid. Modified Sdk.targets to pass ProjectGuid to DacpacTool. Introduced DacOriginModifier to set ProjectGuid in Origin.xml using reflection, integrated into build flow with validation and error handling. Added unit tests for DacOriginModifier covering correct writing, error handling, and default behavior.
There was a problem hiding this comment.
Pull request overview
Adds end-to-end ProjectGuid support for SQL projects built with MSBuild.Sdk.SqlProj by plumbing a new GUID value from templates/MSBuild into the bundled DacpacTool, and writing it into the built .dacpac’s Origin.xml.
Changes:
- Added
--projectguid/-pgCLI option and MSBuild wiring to pass$(ProjectGuid)into theDacpacTool buildinvocation. - Introduced
DacOriginModifierto updateOrigin.xmlinside the.dacpac(via reflection against DacFx internals) and integrated it into the build flow. - Updated templates to generate/substitute a
ProjectGuid, and added unit tests forDacOriginModifier.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DacpacTool.Tests/DacOriginModifierTests.cs | Adds unit tests validating Origin.xml ProjectGuid writing and basic error/default behavior. |
| src/MSBuild.Sdk.SqlProj/Sdk/Sdk.targets | Passes $(ProjectGuid) to the CLI via a new -pg argument. |
| src/MSBuild.Sdk.SqlProj.Templates/templates/sqlproj/sqlproj.csproj | Emits <ProjectGuid> in the generated SQL project template. |
| src/MSBuild.Sdk.SqlProj.Templates/templates/sqlproj/.template.config/template.json | Generates a GUID symbol and substitutes it into the template. |
| src/DacpacTool/Program.cs | Parses --projectguid and applies it post-build by modifying Origin.xml. |
| src/DacpacTool/DacOriginModifier.cs | New helper that edits Origin.xml in the .dacpac to set ProjectGuid. |
| src/DacpacTool/BuildOptions.cs | Adds --projectguid/-pg option to build command options. |
Suppressed comments (1)
test/DacpacTool.Tests/DacOriginModifierTests.cs:77
- Temp dacpac files are only deleted on the happy path; if the assertion fails, the temp file will be left behind. Wrap the test body in a try/finally to ensure cleanup always runs.
originXml.ShouldNotContain("<ProjectGuid>");
// Cleanup
tempFile.Delete();
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
test/DacpacTool.Tests/DacOriginModifierTests.cs:73
- The test for the default behavior is too specific: Origin.xml might contain a self-closing or empty element like
<ProjectGuid />, which would bypass the current"<ProjectGuid>"substring check. Use a prefix match (or XML parsing) so the test fails if any ProjectGuid element exists.
// Assert
originXml.ShouldNotContain("<ProjectGuid>");
src/DacpacTool/DacOriginModifier.cs:107
- ResolveDacOriginType scans all loaded assemblies, which is less deterministic than the existing reflection pattern in this codebase (e.g., Extensions.cs uses Type.GetType with an assembly-qualified name). Prefer trying Type.GetType with the expected assembly first, with an optional fallback scan for resilience.
private static Type ResolveDacOriginType()
{
// Ensure the assembly containing DacOrigin is loaded by touching a public type from it.
_ = typeof(TSqlModel);
foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies())
{
src/DacpacTool/Program.cs:170
- If setting the ProjectGuid fails (e.g., due to DacFx version incompatibility or Origin.xml shape changes), the exception will currently bubble up and crash the build command after the dacpac has been written. Catch and convert this into a normal CLI error + exit code (similar to the deploy path).
// Set the ProjectGuid in Origin.xml (must happen after SaveToDisk)
if (!string.IsNullOrWhiteSpace(options.ProjectGuid))
{
if (!Guid.TryParse(options.ProjectGuid, out var projectGuid))
{
Console.WriteLine($"ERROR: Invalid ProjectGuid '{options.ProjectGuid}', must be a valid GUID");
return 1;
}
var originModifier = new DacOriginModifier(new ActualConsole());
originModifier.SetProjectGuid(options.Output, projectGuid);
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/DacpacTool/PackageBuilder.cs:354
- The exception message in AddScript is now misleading: this method is public and can be used to add any script part (and is called by the new AddOutputArtifacts flow), but the message still says "pre and post scripts". Update it to a more generic message so failures are clearer when AddScript is reused for other artifacts.
if (_modelValid != true)
{
throw new InvalidOperationException("Cannot add pre and post scripts before model has been validated.");
}
|
@jeffrosenberg could you have a look at this? |
Added --projectguid/-pg CLI option and MSBuild property to set ProjectGuid for SQL projects.
Updated templates and .csproj files to generate and substitute ProjectGuid.
Modified Sdk.targets to pass ProjectGuid to DacpacTool.
Introduced DacOriginModifier to set ProjectGuid in Origin.xml using reflection, integrated into build flow with validation and error handling.
Added unit tests for DacOriginModifier covering correct writing, error handling, and default behavior.
fixes #965