SDK style CSProj fixes by tru · Pull Request #468 · ubisoft/Sharpmake

tru · 2026-05-27T11:37:29Z

When integrating our hardcoded SDK-style csproj into the sharpmake project we ran into a crash that was

related to the fact that those csproj solutions don't have a generated GUID. So we will generate a stable

one instead.

Second fix is that there was no way to put a reference into a folder. That was fairly small and easy to fix.

I used Claude to write the tests since nothing was written for this yet.

tru · 2026-05-27T12:01:25Z

The CI fails with public api warnings because I marked a few functions as public to be able to unittest them. Is there another pattern to use here that doesn't trip this warning?

jspelletier · 2026-05-27T13:41:53Z

there is a new .bat to update the reference file. See UpdateVerifiedPublicApi.bat

tru · 2026-05-27T13:58:27Z

there is a new .bat to update the reference file. See UpdateVerifiedPublicApi.bat

This didn't do anything - or does it write a file that's hidden from git output?

jspelletier · 2026-05-27T14:05:24Z

@sylvainfortin I think the question from tobias is for you!

sylvainfortin · 2026-05-27T14:58:46Z

Make sure you run:

dotnet test Sharpmake.PublicApiTests/Sharpmake.PublicApiTests.csproj

prior to use UpdateVerifiedPublicApi.bat. It will generate the necessary .received.txt it needs to accept as the verified file. Sorry for the inconvenience. This step should be added to UpdateVerifiedPublicApi.bat so it work even if these tests haven't ran.

tru · 2026-05-27T18:32:46Z

Still seems to fail?

sylvainfortin · 2026-05-27T21:00:15Z

I think the file Sharpmake.PublicApiTests/Snapshots/Sharpmake.Generators.PublicApi.verified.txt was wrongly updated. It's like the PublicApiTests were ran from a Posix environment where line ending is '\n'. Unfortunately, there are some problematic constant string in the public API that need to be removed. Because line endings in them are not consistent depending the runtime environment.

To fix your build, just remove Sharpmake.Generators.PublicApi.verified.txt changes from the commit.

tru · 2026-05-28T06:33:32Z

I think the file Sharpmake.PublicApiTests/Snapshots/Sharpmake.Generators.PublicApi.verified.txt was wrongly updated. It's like the PublicApiTests were ran from a Posix environment where line ending is '\n'. Unfortunately, there are some problematic constant string in the public API that need to be removed. Because line endings in them are not consistent depending the runtime environment.

To fix your build, just remove Sharpmake.Generators.PublicApi.verified.txt changes from the commit.

It was done on Windows - but I have a git setting to normalize the line-endings, so that's probably it.

tru · 2026-05-28T06:35:41Z

But it still doesn't work. Should I just remove the unittest using the public symbol and make it private again?

jspelletier · 2026-05-28T13:01:22Z

@tru You added public static string ReadOrGenerateGuidFromProjectFile(string projectFile) but I don't see it in the verifiedapi txt file

tru · 2026-05-28T13:46:19Z

@tru You added public static string ReadOrGenerateGuidFromProjectFile(string projectFile) but I don't see it in the verifiedapi txt file

I just committed what the script generated.

I am fine with just reverting the visibility changes and remove the tests if that's simpler.

sylvainfortin · 2026-05-28T13:48:32Z

Then my bad. I asked @tru to revert Sharpmake.Generators.PublicApi.verified.txt entirely but didn't notice the added method in that file.

tru · 2026-05-29T06:46:48Z

I asked claude to fix the issue and it did it with a entry in .gitattributes - it seems to work now.

jspelletier · 2026-05-29T13:40:24Z


+        // SDK-style csproj files omit <ProjectGuid>. Generate a stable GUID from the
+        // normalised file path so that solution references remain consistent across runs.
+        internal static string GenerateDeterministicGuid(string projectFile)


There is already a Sharpmake.Util.BuildGuid method that is pretty much similar to this.

jspelletier · 2026-05-29T13:43:18Z

+            return new Guid(hash).ToString("D").ToUpperInvariant();
+        }
+
+        public static string ReadOrGenerateGuidFromProjectFile(string projectFile)


this method should be internal. it is only used in this file and in nunit tests.

jspelletier · 2026-05-29T13:46:26Z

 *.reg text eof=crlf
 *.csproj text eof=crlf

-# Declare files that must keep lf 


I don't like this fix. We will try something else.

jspelletier · 2026-05-29T13:48:29Z

+                var guid1 = Sln.ReadOrGenerateGuidFromProjectFile(path);
+                var guid2 = Sln.ReadOrGenerateGuidFromProjectFile(path);
+
+                Assert.That(guid2, Is.EqualTo(guid1), "deterministic GUID must be stable across calls");


I think it should also check that it doesn't return null

jspelletier · 2026-05-29T13:48:45Z

+                var guid1 = Sln.ReadOrGenerateGuidFromProjectFile(path1);
+                var guid2 = Sln.ReadOrGenerateGuidFromProjectFile(path2);
+
+                Assert.That(guid2, Is.Not.EqualTo(guid1), "different files must get different GUIDs");


same comment than above

Modern SDK-style .csproj files (net5+) omit <ProjectGuid>. ResolveReferencesByPath stored the null result of ReadGuidFromProjectFile directly into UserData["Guid"], causing a NullReferenceException when the .sln generator tried to write the project entry template. The project-level path (ProjectReferencesByPathContainer) would throw ArgumentNullException via new Guid(null). Add ReadOrGenerateGuidFromProjectFile that falls back to a deterministic MD5-based GUID derived from the normalised file path when no <ProjectGuid> is present. The generated GUID is stable across runs, so .sln files remain reproducible without requiring every project to declare one explicitly. Add unit tests covering old-style GUID extraction, null return for SDK-style files, and the deterministic fallback properties.

…in solution folders Adds a new case-insensitive dictionary on Solution.Configuration that maps absolute project paths to solution folder strings. ResolveReferencesByPath applies the mapping so projects added via ProjectReferencesByPath can be placed in nested solution folders instead of always appearing at the root. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

…thod Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

… API snapshot The verbatim string constants in Bff.Template.cs contain actual newlines, so their public API snapshot representation changes between LF and CRLF checkouts. Pin this file to LF so the verified snapshot is consistent on all platforms. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

…ert snapshot/gitattributes - Make ReadOrGenerateGuidFromProjectFile internal (only used in Sln.cs and tests) - GenerateDeterministicGuid now reuses Util.BuildGuid instead of duplicating MD5 code - Revert .gitattributes line-ending changes (rejected approach) - Revert Sharpmake.Generators.PublicApi.verified.txt to base now that the helper is internal and no longer part of the public API - Assert generated GUIDs are non-null in SlnTest Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tru · 2026-06-01T05:25:20Z

Review addressed and retested.

The problem with the LF stuff in the string file is that I have my machine configured to do autocrlf=input - which is the recommendation these days on Windows, but that means that the file get's checked out as LF on my local machine and that made the test break for me.

I think .gitattributes in some way would fix this issue - or the test could account for the LF/CRLF change.

tru force-pushed the csproj_fixes branch from 78d88ce to 3aab672 Compare May 27, 2026 11:57

tru force-pushed the csproj_fixes branch from 3aab672 to f998a6b Compare May 27, 2026 13:52

tru force-pushed the csproj_fixes branch from d9688cd to 855e1cc Compare May 28, 2026 06:32

jspelletier reviewed May 29, 2026

View reviewed changes

tru force-pushed the csproj_fixes branch from eb0b436 to a604360 Compare June 1, 2026 05:12

tru and others added 6 commits June 1, 2026 07:13

Update the publicAPi.verified.txt

4793914

Update Generators public API snapshot with LF line endings and new me…

551f647

…thod Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

tru force-pushed the csproj_fixes branch from a604360 to 658318a Compare June 1, 2026 05:13

Conversation

tru commented May 27, 2026

Uh oh!

tru commented May 27, 2026

Uh oh!

jspelletier commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tru commented May 27, 2026

Uh oh!

jspelletier commented May 27, 2026

Uh oh!

sylvainfortin commented May 27, 2026

Uh oh!

tru commented May 27, 2026

Uh oh!

sylvainfortin commented May 27, 2026

Uh oh!

tru commented May 28, 2026

Uh oh!

tru commented May 28, 2026

Uh oh!

jspelletier commented May 28, 2026

Uh oh!

tru commented May 28, 2026

Uh oh!

sylvainfortin commented May 28, 2026

Uh oh!

tru commented May 29, 2026

Uh oh!

jspelletier May 29, 2026

Choose a reason for hiding this comment

Uh oh!

jspelletier May 29, 2026

Choose a reason for hiding this comment

Uh oh!

jspelletier May 29, 2026

Choose a reason for hiding this comment

Uh oh!

jspelletier May 29, 2026

Choose a reason for hiding this comment

Uh oh!

jspelletier May 29, 2026

Choose a reason for hiding this comment

Uh oh!

tru commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jspelletier commented May 27, 2026 •

edited

Loading