Skip to content

Add support for systems running only .NET 9.0 (without .NET 8.0) in Refitter.MSBuild #746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christianhelle merged 3 commits into from
Sep 12, 2025
Merged

Add support for systems running only .NET 9.0 (without .NET 8.0) in Refitter.MSBuild #746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
229e31c
Convert build.ps1 to a bash script
christianhelle Sep 11, 2025
bb5cd4e
Update package paths for net8.0 in MSBuild project and RefitterGenera…
christianhelle Sep 12, 2025
2d68dd3
Add support for systems that only have .NET 9.0 installed
christianhelle Sep 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Refitter.MSBuild/Refitter.MSBuild.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
<Content Include="$(AssemblyName).targets" PackagePath="build" />
<Content Include="$(AssemblyName).props" PackagePath="tasks" />
<Content Include="$(AssemblyName).targets" PackagePath="tasks" />
<Content Include="../Refitter/bin/$(Configuration)/net8.0/**/*" PackagePath="tasks" />
<Content Include="../Refitter/bin/$(Configuration)/net8.0/**/*" PackagePath="tasks/net8.0" />
<Content Include="../Refitter/bin/$(Configuration)/net9.0/**/*" PackagePath="tasks/net9.0" />
</ItemGroup>

</Project>
47 changes: 37 additions & 10 deletions src/Refitter.MSBuild/RefitterGenerateTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,28 @@ public override bool Execute()

private List<string> StartProcess(string file)
{
var expectedFiles = GetExpectedGeneratedFiles(file);
var expectedFiles = GetExpectedGeneratedFiles(file);
var assembly = Assembly.GetExecutingAssembly();
var packageFolder = Path.GetDirectoryName(assembly.Location);
var seperator = Path.DirectorySeparatorChar;
var refitterDll = $"{packageFolder}{seperator}..{seperator}refitter.dll";
var refitterDll = $"{packageFolder}{seperator}..{seperator}net8.0{seperator}refitter.dll";

var args = $"{refitterDll} --settings-file {file}";
if (DisableLogging)
List<string> installedRuntimes = GetInstalledDotnetRuntimes();
if (installedRuntimes.Any(r => r.StartsWith("Microsoft.NETCore.App 9.")))
{
args += " --no-logging";
// Use .NET 9 version if available
refitterDll = $"{packageFolder}{seperator}..{seperator}net9.0{seperator}refitter.dll";
TryLogCommandLine("Detected .NET 9 runtime. Using .NET 9 version of Refitter.");
}
else
{
TryLogCommandLine("Using .NET 8 version of Refitter.");
}
Comment on lines +63 to 79

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add fallback for when the dotnet command fails to detect runtimes.

The runtime detection could fail if the dotnet command is not available or returns an error. Consider adding error handling.

Apply this diff to add error handling:

         var refitterDll = $"{packageFolder}{seperator}..{seperator}net8.0{seperator}refitter.dll";
 
-        List<string> installedRuntimes = GetInstalledDotnetRuntimes();
-        if (installedRuntimes.Any(r => r.StartsWith("Microsoft.NETCore.App 9.")))
+        try
         {
-            // Use .NET 9 version if available
-            refitterDll = $"{packageFolder}{seperator}..{seperator}net9.0{seperator}refitter.dll";
-            TryLogCommandLine("Detected .NET 9 runtime. Using .NET 9 version of Refitter.");
+            List<string> installedRuntimes = GetInstalledDotnetRuntimes();
+            if (installedRuntimes.Any(r => r.StartsWith("Microsoft.NETCore.App 9.")))
+            {
+                // Use .NET 9 version if available
+                refitterDll = $"{packageFolder}{seperator}..{seperator}net9.0{seperator}refitter.dll";
+                TryLogCommandLine("Detected .NET 9 runtime. Using .NET 9 version of Refitter.");
+            }
+            else
+            {
+                TryLogCommandLine("Using .NET 8 version of Refitter.");
+            }
         }
-        else
+        catch (Exception ex)
         {
-            TryLogCommandLine("Using .NET 8 version of Refitter.");
+            TryLogCommandLine($"Failed to detect .NET runtimes: {ex.Message}. Defaulting to .NET 8 version.");
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var expectedFiles = GetExpectedGeneratedFiles(file);
var assembly = Assembly.GetExecutingAssembly();
var packageFolder = Path.GetDirectoryName(assembly.Location);
var seperator = Path.DirectorySeparatorChar;
var refitterDll = $"{packageFolder}{seperator}..{seperator}refitter.dll";
var refitterDll = $"{packageFolder}{seperator}..{seperator}net8.0{seperator}refitter.dll";
var args = $"{refitterDll} --settings-file {file}";
if (DisableLogging)
List<string> installedRuntimes = GetInstalledDotnetRuntimes();
if (installedRuntimes.Any(r => r.StartsWith("Microsoft.NETCore.App 9.")))
{
args += " --no-logging";
// Use .NET 9 version if available
refitterDll = $"{packageFolder}{seperator}..{seperator}net9.0{seperator}refitter.dll";
TryLogCommandLine("Detected .NET 9 runtime. Using .NET 9 version of Refitter.");
}
else
{
TryLogCommandLine("Using .NET 8 version of Refitter.");
}
var expectedFiles = GetExpectedGeneratedFiles(file);
var assembly = Assembly.GetExecutingAssembly();
var packageFolder = Path.GetDirectoryName(assembly.Location);
var seperator = Path.DirectorySeparatorChar;
var refitterDll = $"{packageFolder}{seperator}..{seperator}net8.0{seperator}refitter.dll";
try
{
List<string> installedRuntimes = GetInstalledDotnetRuntimes();
if (installedRuntimes.Any(r => r.StartsWith("Microsoft.NETCore.App 9.")))
{
// Use .NET 9 version if available
refitterDll = $"{packageFolder}{seperator}..{seperator}net9.0{seperator}refitter.dll";
TryLogCommandLine("Detected .NET 9 runtime. Using .NET 9 version of Refitter.");
}
else
{
TryLogCommandLine("Using .NET 8 version of Refitter.");
}
}
catch (Exception ex)
{
TryLogCommandLine($"Failed to detect .NET runtimes: {ex.Message}. Defaulting to .NET 8 version.");
}
🤖 Prompt for AI Agents 
In src/Refitter.MSBuild/RefitterGenerateTask.cs around lines 63 to 79, the code
assumes GetInstalledDotnetRuntimes() always succeeds and will throw or return
nothing if the dotnet command is missing; wrap the call in a try/catch (or
otherwise detect failures) so exceptions are caught and logged via
TryLogCommandLine/TryLogError, treat failures as “unknown” and fall back to
using the .NET 8 refitter.dll path, and ensure you also handle an empty list
case by falling back to .NET 8 and logging a warning that runtime detection
failed so the build continues with the safe default.


if (!File.Exists(refitterDll))
var args = $"{refitterDll} --settings-file {file}";
if (DisableLogging)
{
TryLogError($"Refitter CLI not found at: {refitterDll}");
TryLogError($"Assembly location: {assembly.Location}");
TryLogError($"Package folder: {packageFolder}");
return new List<string>();
args += " --no-logging";
}
Comment on lines +81 to 85

Copilot AI Sep 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code no longer validates if the refitterDll file exists before attempting to execute it. The original file existence check was removed but not replaced, which could lead to runtime errors when the selected .NET version's DLL is missing.

Copilot uses AI. Check for mistakes.
Comment on lines +81 to 85

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify that the refitter.dll exists before attempting to execute.

The previous version had a check to ensure the refitter.dll file exists. Without this check, the error message might be less clear if the file is missing.

Let me verify if there's any validation elsewhere in the codebase:

🏁 Script executed:

#!/bin/bash
# Check if there's any file existence validation for refitter.dll
rg -n "File\.Exists.*refitter" --type cs
rg -n "refitter\.dll" --type cs -C3

Length of output: 1327

Add a File.Exists guard for refitter.dll before executing.

No existence check was found in src/Refitter.MSBuild/RefitterGenerateTask.cs — refitterDll is built around line ~67 and args are assembled at lines 81–85; add a guard like if (!File.Exists(refitterDll)) { Log.LogError($"Refitter not found at '{refitterDll}'"); return false; } so failures are clear.

🤖 Prompt for AI Agents 
In src/Refitter.MSBuild/RefitterGenerateTask.cs around lines 81–85 (refitterDll
is constructed near line ~67), add a File.Exists check for refitterDll before
assembling or executing the args; if the file does not exist call Log.LogError
with a clear message including the refitterDll path and return false to abort
the task so failures are explicit and surfaced.


TryLogCommandLine($"Starting dotnet {args}");
Expand Down Expand Up @@ -108,6 +112,29 @@ private List<string> StartProcess(string file)
return expectedFiles.Where(File.Exists).ToList();
}

private static List<string> GetInstalledDotnetRuntimes()
{
var installedRuntimes = new List<string>();
using (var process = new Process())
{
process.StartInfo.FileName = "dotnet";
process.StartInfo.Arguments = "--list-runtimes";
process.StartInfo.RedirectStandardOutput = true;
process.StartInfo.UseShellExecute = false;
process.StartInfo.CreateNoWindow = true;

process.Start();
using (var reader = process.StandardOutput)
{
var output = reader.ReadToEnd();
installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));

Copilot AI Sep 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The null-conditional operator on output is unnecessary since Split is called on a potentially null string. If output is null, this will throw a NullReferenceException. Should use output?.Split(...) ?? [] or check for null explicitly before splitting.

Suggested change
installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));
installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>());

Copilot uses AI. Check for mistakes.

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential NullReferenceException when splitting output.

The null-conditional operator ?. is used with Split, but the result is then passed to AddRange which doesn't handle null gracefully.

Apply this diff to handle potential null output:

-                installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));
+                if (!string.IsNullOrEmpty(output))
+                {
+                    installedRuntimes.AddRange(output.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));
+                }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));
if (!string.IsNullOrEmpty(output))
{
installedRuntimes.AddRange(output.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries));
}
🤖 Prompt for AI Agents 
In src/Refitter.MSBuild/RefitterGenerateTask.cs around line 130, the code calls
AddRange(output?.Split([Environment.NewLine],
StringSplitOptions.RemoveEmptyEntries)) which can pass null to AddRange if
output is null; change it so AddRange receives a non-null sequence (for example
use the null-coalescing operator to supply Array.Empty<string>() or an empty
enumerable when output is null) so AddRange is never called with a null
argument.

}
process.WaitForExit();
}
Comment on lines +115 to +133

Copilot AI Sep 12, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method doesn't handle process execution failures or capture stderr output. If the 'dotnet' command fails or isn't found, the method will silently return an empty list, causing the runtime detection to fail without proper error reporting.

Suggested change
private static List<string> GetInstalledDotnetRuntimes()
{
var installedRuntimes = new List<string>();
using (var process = new Process())
{
process.StartInfo.FileName = "dotnet";
process.StartInfo.Arguments = "--list-runtimes";
process.StartInfo.RedirectStandardOutput = true;
process.StartInfo.UseShellExecute = false;
process.StartInfo.CreateNoWindow = true;
process.Start();
using (var reader = process.StandardOutput)
{
var output = reader.ReadToEnd();
installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));
}
process.WaitForExit();
}
private List<string> GetInstalledDotnetRuntimes()
{
var installedRuntimes = new List<string>();
try
{
using (var process = new Process())
{
process.StartInfo.FileName = "dotnet";
process.StartInfo.Arguments = "--list-runtimes";
process.StartInfo.RedirectStandardOutput = true;
process.StartInfo.RedirectStandardError = true;
process.StartInfo.UseShellExecute = false;
process.StartInfo.CreateNoWindow = true;
process.Start();
string output = process.StandardOutput.ReadToEnd();
string error = process.StandardError.ReadToEnd();
process.WaitForExit();
if (process.ExitCode != 0)
{
TryLogError($"Failed to execute 'dotnet --list-runtimes'. Exit code: {process.ExitCode}. Error: {error}");
return new List<string>();
}
if (!string.IsNullOrWhiteSpace(error))
{
TryLogError($"Error output from 'dotnet --list-runtimes': {error}");
}
installedRuntimes.AddRange(output?.Split([Environment.NewLine], StringSplitOptions.RemoveEmptyEntries));
}
}
catch (Exception ex)
{
TryLogError($"Exception while executing 'dotnet --list-runtimes': {ex.Message}");
return new List<string>();
}

Copilot uses AI. Check for mistakes.

return installedRuntimes;
}

private List<string> GetExpectedGeneratedFiles(string refitterFilePath)
{
try
Expand Down
22 changes: 22 additions & 0 deletions test/MSBuild/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash

rm -rf bin
rm -rf obj
dotnet build-server shutdown
dotnet nuget locals global-packages --clear
rm -f Refitter.MSBuild.*.nupkg
rm -f Petstore.cs
dotnet restore ../../src/Refitter.sln
dotnet clean -c release ../../src/Refitter.sln
dotnet build -c release ../../src/Refitter/Refitter.csproj
dotnet build -c release ../../src/Refitter.MSBuild/Refitter.MSBuild.csproj
dotnet pack -c release ../../src/Refitter.MSBuild/Refitter.MSBuild.csproj -o .

# Find the nupkg file and add it as a package
find . -name "Refitter.MSBuild.*.nupkg" -exec dotnet add package {} --source . \;

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

The find command may fail if no .nupkg files exist.

The find command with -exec will fail if no matching files are found, which could happen on first run or after cleanup.

Apply this diff to handle the case when no packages are found:

 # Find the nupkg file and add it as a package
-find . -name "Refitter.MSBuild.*.nupkg" -exec dotnet add package {} --source . \;
+NUPKG_FILES=$(find . -name "Refitter.MSBuild.*.nupkg" 2>/dev/null || true)
+if [ -n "$NUPKG_FILES" ]; then
+    echo "$NUPKG_FILES" | xargs -I {} dotnet add package {} --source .
+else
+    echo "Error: No .nupkg files found after packing"
+    exit 1
+fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
find . -name "Refitter.MSBuild.*.nupkg" -exec dotnet add package {} --source . \;
# Find the nupkg file and add it as a package
NUPKG_FILES=$(find . -name "Refitter.MSBuild.*.nupkg" 2>/dev/null || true)
if [ -n "$NUPKG_FILES" ]; then
echo "$NUPKG_FILES" | xargs -I {} dotnet add package {} --source .
else
echo "Error: No .nupkg files found after packing"
exit 1
fi
🤖 Prompt for AI Agents 
In test/MSBuild/build.sh around line 16 the find command uses -exec which causes
the script to exit/fail when no Refitter.MSBuild.*.nupkg files exist; replace
that single-line command with a safe variant that does nothing when there are no
matches (for example use find ... -print0 piped to xargs -0 with the GNU xargs
-r/--no-run-if-empty option, or use find -exec sh -c 'dotnet add package "$1"
--source .' _ {} \; wrapped to ignore missing results) so the script continues
without error when no .nupkg files are found.


dotnet restore
dotnet add package Refitter.MSBuild --source .
dotnet run -v d -filelogger
dotnet remove package Refitter.MSBuild
rm -f Refitter.MSBuild.*.nupkg
Comment on lines +1 to +22

@coderabbitai coderabbitai Bot Sep 12, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and make the script more robust.

The script lacks error handling which could lead to silent failures or incomplete cleanup. Consider adding set -e to exit on errors and trap cleanup operations.

Apply this diff to add error handling:

 #!/bin/bash
+set -e  # Exit on error
+
+# Trap to ensure cleanup even on failure
+trap 'echo "Build failed. Cleaning up..."; rm -f Refitter.MSBuild.*.nupkg' ERR
 
 rm -rf bin
 rm -rf obj

Also consider checking if required tools are available:

# Check required tools
for cmd in dotnet find; do
    if ! command -v $cmd &> /dev/null; then
        echo "Error: $cmd is not installed"
        exit 1
    fi
done
🤖 Prompt for AI Agents 
In test/MSBuild/build.sh around lines 1 to 22, the script lacks error handling
and checks for required commands; update it to fail fast and perform guaranteed
cleanup by adding "set -euo pipefail" (or at least "set -e"), add a trap to run
cleanup on EXIT (removing temporary files and shutting down build server), and
explicitly verify required tools (dotnet and find) are available before
proceeding, printing a clear error and exiting if missing; also ensure each
critical command is allowed to fail the script (no silent ignores) and log
failures with descriptive messages so failures are visible.

Loading