build: remove package-benchmark from the dependencies #165
Conversation
|
Removing the benchmark plugin just like that is not okey. You can conditionalize it on being built in windows for example, but just removing like that isn't very helpful, we do want this plugin. |
|
Conditionalised behind |
This package is not very Windows friendly. A partial port was attempted but revealed further Unixisms that will be difficult to port. There are alternative packages for benchmarking, e.g. google/swift-benchmark which may be a better choice.
| func sjEnableBenchmarking() -> Bool { | ||
| if let value = ProcessInfo.processInfo.environment["SWIFT_JAVA_ENABLE_BENCHMARKING"] { | ||
| switch value { | ||
| case "TRUE", "YES", "1": return true | ||
| case "FALSE", "NO", "0": return false | ||
| default: break | ||
| } | ||
| } | ||
| fatalError("Please set 'SWIFT_JAVA_ENABLE_BENCHMARKING' to 'TRUE' or 'FALSE'") | ||
| } |
There was a problem hiding this comment.
We shouldn't force all clients to set an environment variable here. I think the best option here is to disable benchmarking on Windows until we find an appropriate solution.
There was a problem hiding this comment.
Is there a way to do that within the confines of package.swift? #if os(...) uses the build not the host.
There was a problem hiding this comment.
I'll repeat the request about the "opt out with an env var" on windows.
I don't understand what you meant by it being hard on windows. There is "set ...=..." and surely IDEs can set env when starting processes as well...? Can we do that instead?
There was a problem hiding this comment.
No, IDEs don't do very well with setting environment variables (at least VSCode in some cases and VS do not). The environment variable setting then permeates into everything and has different syntax in different places. It doesn't fully propagate between subshells, which adds additional surprises. I think that we should either fully drop the benchmarking or require setting the environment everywhere - it avoids any surprises.
There was a problem hiding this comment.
Then I propose we put the benchmarking into a sub-package that we add in a sub directory. Lemme try that.
This package is not very Windows friendly. A partial port was attempted but revealed further Unixisms that will be difficult to port. There are alternative packages for benchmarking, e.g. google/swift-benchmark which may be a better choice.