Bundle dotnet-script 2.0.1 and roll forward on targets without .NET 8

source/Calamari.Common/Features/Scripting/DotnetScript/DotnetScriptBootstrapper.cs

@@ -95,18 +95,63 @@ public static string FindBundledExecutable()
             throw new CommandException(string.Format("dotnet-script was not found at '{0}'", executable));
         }
 
-        public static string FormatCommandArguments(string bootstrapFile, string? scriptParameters, string? nugetSource = null)
+        // dotnet-script 2.0 makes the isolated assembly load context the default and replaces the
+        // opt-in flag with an opt-out. Isolation is what makes native NuGet assets work (SQLite,
+        // SkiaSharp, Microsoft.Data.SqlClient - dotnet-script #763), and it also gives a script the
+        // package version it asked for rather than whichever version dotnet-script itself carries.
+        // The cost is that a type loaded via Assembly.LoadFrom is no longer reference-equal to the
+        // same type in the script's own closure. This flag restores the pre-2.0 behaviour.
+        // 1.6.0 does not recognise the flag, so it must not be passed to a customer's own
+        // locally-installed copy blindly - see RemoveLegacyIsolatedLoadContextFlag.
+        internal const string DisableIsolatedLoadContextArgument = "--disable-isolated-load-context";
+
+        // The 1.6.0 opt-in flag. 2.0 no longer recognises it, and dotnet-script forwards
+        // unrecognised options into the *script's* argument list rather than rejecting them
+        // (measured on both 1.6.0 and 2.0.1, silently and with exit 0). Left in place it would push
+        // every script argument along by one, so Env.ScriptArgs[0] becomes "--isolated-load-context".
+        internal const string LegacyIsolatedLoadContextArgument = "--isolated-load-context";
+
+        public static string FormatCommandArguments(string bootstrapFile, string? scriptParameters, string? nugetSource = null, bool disableIsolatedLoadContext = false)
         {
             var (scriptCommandArguments, scriptArguments) = RetrieveParameterValues(scriptParameters);
+            scriptCommandArguments = RemoveLegacyIsolatedLoadContextFlag(scriptCommandArguments);
             var encryptionKey = Convert.ToBase64String(VariableEncryptor.EncryptionKey);
             var source = string.IsNullOrWhiteSpace(nugetSource) ? "https://api.nuget.org/v3/index.json" : nugetSource;
             var commandArguments = new StringBuilder();
             commandArguments.Append($"-s {source} ");
+            if (disableIsolatedLoadContext) commandArguments.Append($"{DisableIsolatedLoadContextArgument} ");
             if (!string.IsNullOrWhiteSpace(scriptCommandArguments)) commandArguments.Append($"{scriptCommandArguments} ");
             commandArguments.AppendFormat("\"{0}\" -- {1} \"{2}\"", bootstrapFile, scriptArguments, encryptionKey);
             return commandArguments.ToString();
         }
 
+        /// <summary>
+        /// Drops the 1.6.0 --isolated-load-context flag from a step's script parameters. Isolation is
+        /// the default from 2.0 on, so removing the flag preserves exactly what the customer asked
+        /// for; leaving it in would instead inject the literal string as their script's first
+        /// argument. Compares whole tokens so --disable-isolated-load-context is left alone.
+        /// </summary>
+        internal static string? RemoveLegacyIsolatedLoadContextFlag(string? scriptCommandArguments)
+        {
+            if (string.IsNullOrWhiteSpace(scriptCommandArguments)
+                || scriptCommandArguments!.IndexOf(LegacyIsolatedLoadContextArgument, StringComparison.OrdinalIgnoreCase) < 0)
+                return scriptCommandArguments;
+
+            var kept = scriptCommandArguments.Split(new[] { ' ', '\t' }, StringSplitOptions.RemoveEmptyEntries)
+                                             .Where(token => !token.Equals(LegacyIsolatedLoadContextArgument, StringComparison.OrdinalIgnoreCase));
+
+            return string.Join(" ", kept);
+        }
+
+        public static bool HasLegacyIsolatedLoadContextFlag(string? scriptParameters)
+        {
+            var (scriptCommandArguments, _) = RetrieveParameterValues(scriptParameters);
+
+            return !string.IsNullOrWhiteSpace(scriptCommandArguments)
+                   && scriptCommandArguments!.Split(new[] { ' ', '\t' }, StringSplitOptions.RemoveEmptyEntries)
+                                             .Any(token => token.Equals(LegacyIsolatedLoadContextArgument, StringComparison.OrdinalIgnoreCase));
+        }
+
         [return: NotNullIfNotNull("scriptParameters")]
         static (string? scriptCommandArguments, string? scriptArguments) RetrieveParameterValues(string? scriptParameters)
         {

source/Calamari.Common/Features/Scripting/DotnetScript/DotnetScriptExecutor.cs

@@ -11,6 +11,10 @@ namespace Calamari.Common.Features.Scripting.DotnetScript
 {
     public class DotnetScriptExecutor : ScriptExecutor
     {
+        const string DotnetRollForwardVariableName = "DOTNET_ROLL_FORWARD";
+        const string RollForwardVariable = "Octopus.Action.Script.CSharp.RollForward";
+        const string DisableIsolatedLoadContextVariable = "Octopus.Action.Script.CSharp.DisableIsolatedLoadContext";
+
         readonly ICommandLineRunner commandLineRunner;
 
         public DotnetScriptExecutor(ICommandLineRunner commandLineRunner, ILog log): base(log)
@@ -33,17 +37,54 @@ protected override IEnumerable<ScriptExecution> PrepareExecution(Script script,
             var configurationFile = DotnetScriptBootstrapper.PrepareConfigurationFile(workingDirectory, variables);
             var (bootstrapFile, otherTemporaryFiles) = DotnetScriptBootstrapper.PrepareBootstrapFile(script.File, configurationFile, workingDirectory, variables);
             var nugetSource = variables.Get("Octopus.Action.Script.CSharp.NuGetSource");
-            var arguments = DotnetScriptBootstrapper.FormatCommandArguments(bootstrapFile, script.Parameters, nugetSource);
+            bool.TryParse(variables.Get(DisableIsolatedLoadContextVariable, "false"), out var disableIsolatedLoadContext);
+
+            if (DotnetScriptBootstrapper.HasLegacyIsolatedLoadContextFlag(script.Parameters))
+                Log.Verbose($"Ignoring '{DotnetScriptBootstrapper.LegacyIsolatedLoadContextArgument}' in the script parameters: "
+                            + "the isolated assembly load context is the default from dotnet-script 2.0 on, so the flag is "
+                            + $"redundant. Set {DisableIsolatedLoadContextVariable} to true to turn isolation off instead.");
+
+            var arguments = DotnetScriptBootstrapper.FormatCommandArguments(bootstrapFile, script.Parameters, nugetSource, disableIsolatedLoadContext);
             bool.TryParse(variables.Get("Octopus.Action.Script.CSharp.BypassIsolation", "false"), out var bypassDotnetScriptIsolation);
 
             var cli = CreateCommandLineInvocation(executable, arguments, !string.IsNullOrWhiteSpace(localDotnetScriptPath));
-            cli.EnvironmentVars = environmentVars;
+            cli.EnvironmentVars = WithRollForwardOverride(environmentVars, variables.Get(RollForwardVariable));
             cli.WorkingDirectory = workingDirectory;
             cli.Isolate = !bypassDotnetScriptIsolation;
 
             yield return new ScriptExecution(cli, otherTemporaryFiles.Concat(new[] { bootstrapFile, configurationFile }));
         }
-
+
+        /// <summary>
+        /// The roll-forward default ships in the vendored dotnet-script.runtimeconfig.json
+        /// (see source/IncludeDotNetScript.targets), which is process-scoped and covers both the
+        /// Windows and Linux launch paths without Calamari having to do anything.
+        ///
+        /// This only handles the per-step override. DOTNET_ROLL_FORWARD sits above the
+        /// runtimeconfig in the host's precedence order - measured on a Windows worker: a
+        /// runtimeconfig asking for LatestMajor resolved to 8.0.27 rather than 10.0.8 purely
+        /// because an inherited DOTNET_ROLL_FORWARD=Major outranked it. Unlike the runtimeconfig it
+        /// also reaches a dotnet-script the customer installed themselves and put on the PATH,
+        /// which is preferred over our bundled copy.
+        ///
+        /// Setting an environment variable leaks it into every process the customer's script goes
+        /// on to start, so we only do it when a step has explicitly asked for a policy.
+        /// </summary>
+        static Dictionary<string, string>? WithRollForwardOverride(Dictionary<string, string>? environmentVars, string? rollForward)
+        {
+            if (string.IsNullOrWhiteSpace(rollForward))
+                return environmentVars;
+
+            var vars = environmentVars == null
+                ? new Dictionary<string, string>()
+                : new Dictionary<string, string>(environmentVars);
+
+            vars[DotnetRollForwardVariableName] = rollForward;
+
+            return vars;
+        }
+
+
         private string GetExecutable(string? localDotnetScriptPath, string bundledExecutable)
         {
             return string.IsNullOrWhiteSpace(localDotnetScriptPath)

source/Calamari.Scripting/DotnetScript/dotnet-script.runtimeconfig.json

@@ -0,0 +1,14 @@
+{
+  "runtimeOptions": {
+    "tfm": "net8.0",
+    "rollForward": "Major",
+    "framework": {
+      "name": "Microsoft.NETCore.App",
+      "version": "8.0.0"
+    },
+    "configProperties": {
+      "System.Reflection.Metadata.MetadataUpdater.IsSupported": false,
+      "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false
+    }
+  }
+}

source/Calamari.Tests/Fixtures/DotnetScript/DotnetScriptBootstrapperFixture.cs

@@ -9,12 +9,16 @@ namespace Calamari.Tests.Fixtures.DotnetScript
     [TestFixture]
     public class DotnetScriptBootstrapperFixture
     {
+        // The --isolated-load-context cases expect the flag to be gone: dotnet-script 2.0 no longer
+        // recognises it and would forward it into the script's own arguments. Every other option the
+        // caller passes is left exactly where it was.
         [TestCase(null, null, null)]
         [TestCase("-- \"Parameter 1\" \"Parameter 2\"", null, "\"Parameter 1\" \"Parameter 2\"")]
         [TestCase("\"Parameter 1\" \"Parameter 2\"", null, "\"Parameter 1\" \"Parameter 2\"")]
-        [TestCase("--isolated-load-context -- \"Parameter 1\" \"Parameter 2\"", "--isolated-load-context ", "\"Parameter 1\" \"Parameter 2\"")]
-        [TestCase("--isolated-load-context -d -- \"Parameter 1\" \"Parameter 2\"", "--isolated-load-context -d ", "\"Parameter 1\" \"Parameter 2\"")]
-        [TestCase("--isolated-load-context --verbosity debug -- \"Parameter 1\" \"Parameter 2\"", "--isolated-load-context --verbosity debug ", "\"Parameter 1\" \"Parameter 2\"")]
+        [TestCase("--isolated-load-context -- \"Parameter 1\" \"Parameter 2\"", null, "\"Parameter 1\" \"Parameter 2\"")]
+        [TestCase("--isolated-load-context -d -- \"Parameter 1\" \"Parameter 2\"", "-d ", "\"Parameter 1\" \"Parameter 2\"")]
+        [TestCase("--isolated-load-context --verbosity debug -- \"Parameter 1\" \"Parameter 2\"", "--verbosity debug ", "\"Parameter 1\" \"Parameter 2\"")]
+        [TestCase("--verbosity debug -- \"Parameter 1\" \"Parameter 2\"", "--verbosity debug ", "\"Parameter 1\" \"Parameter 2\"")]
         public void FormatCommandArgumentsTest([CanBeNull] string scriptParameters, [CanBeNull] string commandArguments, [CanBeNull] string scriptArguments)
         {
             var bootstrapFile = "Bootstrap." + Guid.NewGuid().ToString().Substring(10) + "." + "Script.csx";
@@ -31,5 +35,62 @@ public void FormatCommandArguments_UsesCustomNuGetSource_WhenProvided()
             result.Should().Contain($"-s {customSource} ");
             result.Should().NotContain("api.nuget.org");
         }
+
+        [Test]
+        public void FormatCommandArguments_LeavesIsolationOn_ByDefault()
+        {
+            var result = DotnetScriptBootstrapper.FormatCommandArguments("Bootstrap.csx", null);
+            result.Should().NotContain("--disable-isolated-load-context");
+        }
+
+        [Test]
+        public void FormatCommandArguments_DisablesIsolatedLoadContext_WhenRequested()
+        {
+            var result = DotnetScriptBootstrapper.FormatCommandArguments("Bootstrap.csx", null, null, true);
+            result.Should().Contain("--disable-isolated-load-context ");
+        }
+
+        [Test]
+        public void FormatCommandArguments_PlacesDisableIsolatedLoadContextBeforeTheScriptFile()
+        {
+            // Anything after the bootstrap file is passed to the script, not to dotnet-script.
+            const string bootstrapFile = "Bootstrap.csx";
+            var result = DotnetScriptBootstrapper.FormatCommandArguments(bootstrapFile, "--verbosity debug -- \"Parameter 1\"", null, true);
+            result.IndexOf("--disable-isolated-load-context", StringComparison.Ordinal)
+                  .Should()
+                  .BeLessThan(result.IndexOf($"\"{bootstrapFile}\"", StringComparison.Ordinal));
+        }
+
+        [Test]
+        public void FormatCommandArguments_KeepsTheOptOutFlag_WhenTheCallerAlsoPassedTheLegacyOptIn()
+        {
+            // The legacy opt-in is dropped rather than treated as a conflicting instruction, so the
+            // step variable stays authoritative and the caller's flag cannot corrupt their arguments.
+            var result = DotnetScriptBootstrapper.FormatCommandArguments("Bootstrap.csx", "--isolated-load-context -- P0", null, true);
+            result.Should().Contain("--disable-isolated-load-context ");
+            result.Should().NotContain(" --isolated-load-context");
+        }
+
+        [TestCase("--isolated-load-context", "")]
+        [TestCase("--ISOLATED-LOAD-CONTEXT", "")]
+        [TestCase("--isolated-load-context -d", "-d")]
+        [TestCase("-d --isolated-load-context", "-d")]
+        [TestCase("--disable-isolated-load-context", "--disable-isolated-load-context")]
+        [TestCase("--verbosity debug", "--verbosity debug")]
+        [TestCase("", "")]
+        [TestCase(null, null)]
+        public void RemoveLegacyIsolatedLoadContextFlag_StripsWholeTokensOnly([CanBeNull] string input, [CanBeNull] string expected)
+        {
+            DotnetScriptBootstrapper.RemoveLegacyIsolatedLoadContextFlag(input).Should().Be(expected);
+        }
+
+        [TestCase("--isolated-load-context -- P0", true)]
+        [TestCase("--disable-isolated-load-context -- P0", false)]
+        [TestCase("-- P0", false)]
+        [TestCase(null, false)]
+        public void HasLegacyIsolatedLoadContextFlag_DetectsOnlyTheLegacyOptIn([CanBeNull] string scriptParameters, bool expected)
+        {
+            DotnetScriptBootstrapper.HasLegacyIsolatedLoadContextFlag(scriptParameters).Should().Be(expected);
+        }
     }
 }

source/Calamari.Tests/Fixtures/DotnetScript/DotnetScriptFixture.cs

@@ -110,26 +110,70 @@ public void ShouldConsumeParametersWithoutParametersPrefix()
             output.AssertOutput("Parameters Parameter0Parameter1");
         }
 
-        [TestCase(true)]
-        [TestCase(false)]
-        public void UsingIsolatedAssemblyLoadContext(bool enableIsolatedLoadContext)
+        /// <summary>
+        /// IsolatedLoadContext.csx asks for NuGet.Commands 6.10.0.107, whose assemblies carry
+        /// assembly version 6.10.1.5. dotnet-script loads NuGet itself to service #r "nuget:", so
+        /// the version the script observes tells you which assembly load context won.
+        ///
+        /// Isolation on (the default from 2.0): the script's own closure loads in its own context and
+        /// it sees the version it asked for. Isolation off: the script binds to whatever
+        /// dotnet-script already has loaded in the default context - 6.14.3.1 in the 2.0.1 bundle.
+        ///
+        /// This test used to assert that isolation *off* fails outright, which held only because
+        /// 1.6.0 happened to bundle NuGet 6.10.0.107 - a lower version than the script's 6.10.1.5,
+        /// and the default context refuses a downgrade while accepting an upgrade. 2.0.1 bundles a
+        /// higher version, so the same collision now resolves silently to the wrong assembly. The
+        /// assertion is on the version binding rather than on a crash so that it keeps testing the
+        /// load context rather than an accident of which version happens to be vendored.
+        /// </summary>
+        [Test]
+        public void IsolatedAssemblyLoadContext_IsOnByDefault_SoAScriptGetsTheVersionItAskedFor()
+        {
+            var (output, _) = RunScript("IsolatedLoadContext.csx",
+                                        new Dictionary<string, string>()
+                                        {
+                                            [SpecialVariables.Action.Script.ScriptParameters] = "-- Parameter0 Parameter1",
+                                        });
+
+            output.AssertSuccess();
+            output.AssertOutput("NuGet.Commands version: 6.10.1.5");
+            output.AssertOutput("Parameters Parameter0Parameter1");
+        }
+
+        [Test]
+        public void DisableIsolatedLoadContext_BindsTheScriptToDotnetScriptsOwnCopy()
+        {
+            var (output, _) = RunScript("IsolatedLoadContext.csx",
+                                        new Dictionary<string, string>()
+                                        {
+                                            [SpecialVariables.Action.Script.ScriptParameters] = "-- Parameter0 Parameter1",
+                                            ["Octopus.Action.Script.CSharp.DisableIsolatedLoadContext"] = "true",
+                                        });
+
+            output.AssertSuccess();
+            output.AssertOutput("NuGet.Commands version: 6.14.3.1");
+            output.AssertOutput("Parameters Parameter0Parameter1");
+        }
+
+        /// <summary>
+        /// 2.0 dropped --isolated-load-context, and dotnet-script forwards options it does not
+        /// recognise into the script's own argument list instead of rejecting them. Left in place the
+        /// flag would become Env.ScriptArgs[0] and shift every real argument along by one, so
+        /// Calamari strips it. Isolation is the default now, so the customer still gets what they
+        /// asked for.
+        /// </summary>
+        [Test]
+        public void LegacyIsolatedLoadContextFlag_IsStrippedAndDoesNotReachTheScriptsArguments()
         {
             var (output, _) = RunScript("IsolatedLoadContext.csx",
                                         new Dictionary<string, string>()
                                         {
-                                            [SpecialVariables.Action.Script.ScriptParameters] = $"{(enableIsolatedLoadContext ? "--isolated-load-context " : "")}-- Parameter0 Parameter1",
+                                            [SpecialVariables.Action.Script.ScriptParameters] = "--isolated-load-context -- Parameter0 Parameter1",
                                         });
-            if (enableIsolatedLoadContext)
-            {
-                output.AssertSuccess();
-                output.AssertOutput("NuGet.Commands version: 6.10.1.5");
-                output.AssertOutput("Parameters Parameter0Parameter1");
-            }
-            else
-            {
-                output.AssertFailure();
-                output.AssertErrorOutput("Could not load file or assembly 'NuGet.Protocol, Version=6.10.1.5");
-            }
+
+            output.AssertSuccess();
+            output.AssertOutput("NuGet.Commands version: 6.10.1.5");
+            output.AssertOutput("Parameters Parameter0Parameter1");
         }
     }
 }