feat: Embed symbols and enable continuous integration builds (deterministic source paths)

[tom-englert]

What does this PR do?

Use embedded symbols instead of separate symbol package

Why is it important?

Works better across all IDEs

Related issues

• Closes [Enhancement]: Use deterministic builds with SourceLink support #1128

[netlify]

✅ Deploy Preview for testcontainers-dotnet ready!

Name	Link
🔨 Latest commit	e6f2fea
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-dotnet/deploys/65dc366f5a1c5f00080aa5e8
😎 Deploy Preview	https://deploy-preview-1129--testcontainers-dotnet.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[HofmeisterAn]

According to this comment and this section, Source Link is now included in the SDK. The added properties, among others, are set in our Cake build file. According to the Source Link docs, they are not necessary anymore, either. Probably, we can even remove them.

Is something not working? I haven't had issues in the past debugging the library. The NuGet Package Explorer looks Ok too (although, we need to move ContinuousIntegrationBuild to Cake's build task).

[tom-englert]

It should look like this:

• no symbol server
• deterministic, including repo ulr & commit
  

I'll have a look whats wrong with this...

[HofmeisterAn]

It should look like this:

I see, thanks! Maybe we just need to move the properties (args) to the Cake build task (the pack task does not rebuild the projects):

diff --git a/build.cake b/build.cake
index 1d2e4ae..0843d4e 100644
--- a/build.cake
+++ b/build.cake
@@ -67,6 +67,8 @@ Task("Build")
     Verbosity = param.Verbosity,
     NoRestore = true,
     ArgumentCustomization = args => args
+      .Append("/p:ContinuousIntegrationBuild=true")
+      .Append("/p:EmbedUntrackedSources=true")
   });
 });
 
@@ -134,8 +136,6 @@ Task("Create-NuGet-Packages")
     SymbolPackageFormat = "snupkg",
     OutputDirectory = param.Paths.Directories.NuGetDirectoryPath,
     ArgumentCustomization = args => args
-      .Append("/p:ContinuousIntegrationBuild=true")
-      .Append("/p:EmbedUntrackedSources=true")
       .Append($"/p:Version={param.Version}")
   });
 });

[tom-englert]

Probably yes, and the symbols should be embedded rather than having a separate symbol package, from my experience the latter never works without friction.

[tom-englert]

Also maybe .Append("/p:ContinuousIntegrationBuild=true") should not really be needed, SourceLink should determine this automatically

[tom-englert]

https://github.com/dotnet/sourcelink/blob/a22426a46745b8fe77b9d306fa4bc1d03eff44ea/src/Microsoft.Build.StandardCI/build/Microsoft.Build.StandardCI.props#L15-L19

[tom-englert]

This is what I got with the current version of TestContainers:

• listed in the "External Sources", but seems not to match the symbols found on the symbol server.
  

[HofmeisterAn]

This is what I got with the current version of TestContainers:

This is strange. Sorry, I do not have access to Visual Studio. Rider seems to be able to download the symbols, etc. I can debug into the sources without any issues. However, we can merge the suggestions. The NuGet Package Explorer looks even better with them. Thanks.

[HofmeisterAn]

I published the pipeline artifacts (NuGets) to ensure that the NuGets are built and configured properly, and everything is set up correctly for the next release. It appears that ContinuousIntegrationBuild is somehow not automatically applied to the build. We need to set it explicitly (before and after).

[tom-englert]

Strange, I never had to set this - however I never used Cake, maybe that's special about this.

[HofmeisterAn]

Probably, we cannot enable ContinuousIntegrationBuild that easily 😐. It obviously breaks our CommonDirectoryPathTest tests. Not sure how (or if) we can solve this, because we are using the CallerFilePath attribute to resolve common directory paths:

testcontainers-dotnet/src/Testcontainers/Builders/CommonDirectoryPath.cs

Lines 61 to 65 in 5c677d9

	[PublicAPI]
	public static CommonDirectoryPath GetGitDirectory([CallerFilePath, NotNull] string filePath = "")
	{
	return new CommonDirectoryPath(GetDirectoryPath(Path.GetDirectoryName(filePath), ".git"));
	}

[tom-englert]

It obviously breaks our CommonDirectoryPathTest tests

That's strange, how can this have such a side effect? And why only on linux?

[HofmeisterAn]

It obviously breaks our CommonDirectoryPathTest tests

That's strange, how can this have such a side effect? And why only on linux?

I think I found the "reason" and have a potential working configuration, which also explains why you never had to set ContinuousIntegrationBuild explicitly. I need a few more minutes to validate my changes.

[HofmeisterAn]

If we set ContinuousIntegrationBuild to true during the build, it will break the tests because CallerFilePath now returns a deterministic root path. We can simply disable ContinuousIntegrationBuild for the test projects.

My assumption was, if we allow Cake to rebuild the packages on Pack (if the packages are already built, we likely need to force a rebuild), the ContinuousIntegrationBuild is applied automatically, and it won't interfere with the tests. Unfortunately, the configuration is not applied automatically...

[HofmeisterAn]

I do not think we need that many changes. The following changes work fine (incl. Sonar). We just need to make sure the pack target rebuilds the NuGets. I am still wondering why we need to set ContinuousIntegrationBuild explicitly, but maybe Source Link behaves differently if the source control provider is added explicitly.

[tom-englert]

Just needed to turn off deterministic for the tests, they don't get deployed anyhow.

[HofmeisterAn]

Thank you for the efforts, and apologies for the back and forth.

[tom-englert]

Happy to contribute to such a great tool! Thanks for all YOUR efforts 😄