-
-
Couldn't load subscription status.
- Fork 1k
Allow custom runtimes to use Executor #2285
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.
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
Conversation
|
I believe it would be better to introduce a separate base class or interface for such a purpose. E.g., like this: Index: src/BenchmarkDotNet/Toolchains/Executor.cs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/BenchmarkDotNet/Toolchains/Executor.cs b/src/BenchmarkDotNet/Toolchains/Executor.cs
--- a/src/BenchmarkDotNet/Toolchains/Executor.cs (revision d4de82a3f0bcd71f1f695554eef343bbfc1357ea)
+++ b/src/BenchmarkDotNet/Toolchains/Executor.cs (date 1678458305076)
@@ -148,6 +148,10 @@
start.Arguments = args;
start.WorkingDirectory = artifactsPaths.BinariesDirectoryPath;
break;
+ case CustomRuntime _:
+ start.FileName = exePath;
+ start.Arguments = args;
+ break;
default:
throw new NotSupportedException("Runtime = " + runtime);
}
Index: src/BenchmarkDotNet/Environments/Runtimes/CustomRuntime.cs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/BenchmarkDotNet/Environments/Runtimes/CustomRuntime.cs b/src/BenchmarkDotNet/Environments/Runtimes/CustomRuntime.cs
new file mode 100644
--- /dev/null (date 1678458285120)
+++ b/src/BenchmarkDotNet/Environments/Runtimes/CustomRuntime.cs (date 1678458285120)
@@ -0,0 +1,10 @@
+using BenchmarkDotNet.Jobs;
+
+namespace BenchmarkDotNet.Environments
+{
+ public abstract class CustomRuntime : Runtime
+ {
+ protected CustomRuntime(RuntimeMoniker runtimeMoniker, string msBuildMoniker, string displayName) :
+ base(runtimeMoniker, msBuildMoniker, displayName) { }
+ }
+}
\ No newline at end of file@tgjones would it cover your use case? |
|
Thanks! That's a better idea - and yes, it would cover my use-case. I'll modify my PR to look like that. |
|
@AndreyAkinshin I've applied your suggestion. |
|
@tgjones thanks! |
Hello folks! This PR makes a small change to
Executorto allow it be used for custom runtimes. The only downside I can see is that the currentNotSupportedExceptionis probably a useful reminder when adding built-in runtimes, to ensure the correct behaviour is added or re-used here.My use-case: I've written a custom runtime, and currently I need to copy/paste all of
Executorand its supporting classes into my own codebase, which seems a bit redundant when all I really want to change are these 3 lines. Of course this particular setup forProcessStartInfois not applicable for every custom runtime - but at least it allowsExecutorto be used for those runtimes for which this is applicable.But of course you'll have a better view of whether this change is a good idea and / or acceptable.