-
-
Notifications
You must be signed in to change notification settings - Fork 984
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
[xamarin] fix for DebugConfig and read-only file system #1509
Conversation
get | ||
{ | ||
var root = Directory.GetCurrentDirectory(); | ||
if (root == "/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the best way to check if we are running on Android? How about:
if (root == "/") | |
if (RuntimeInformation.IsOSPlatform(OSPlatform.Create("ANDROID"))) |
@jonathanpeppers could you please add a comment here that explains why we need this check (it's not obvious)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same code as here:
BenchmarkDotNet/src/BenchmarkDotNet/Configs/DefaultConfig.cs
Lines 80 to 91 in 92474e8
public string ArtifactsPath | |
{ | |
get | |
{ | |
var root = Directory.GetCurrentDirectory(); | |
if (root == "/") | |
{ | |
root = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); | |
} | |
return Path.Combine(root, "BenchmarkDotNet.Artifacts"); | |
} | |
} |
My thought was that if the current directory is /
, it's not going to be writeable on any platform.
Let me see if the IsOSPlatform
check works for me, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamsitnik neither of these checks actually work on the current Xamarin:
RuntimeInformation.IsOSPlatform(OSPlatform.Create("ANDROID"))
RuntimeInformation.IsOSPlatform(OSPlatform.Create("Android"))
I found some discussion here.
Hopefully, OSPlatform.Android
will exist in .NET 6, but we probably can't use until then.
I updated BDN's RuntimeInformation
instead.
Using `DebugInProcessConfig` results in an error on Android: System.IO.IOException: Read-only file system at System.IO.FileSystem.CreateDirectory (System.String fullPath) [0x00187] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs:317 at System.IO.Directory.CreateDirectory (System.String path) [0x0002c] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corefx/src/System.IO.FileSystem/src/System/IO/Directory.cs:40 at BenchmarkDotNet.Extensions.CommonExtensions.CreateIfNotExists (System.String directoryPath) [0x0000e] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Extensions\CommonExtensions.cs:71 at BenchmarkDotNet.Running.BenchmarkRunnerClean.GetRootArtifactsFolderPath (BenchmarkDotNet.Running.BenchmarkRunInfo[] benchmarkRunInfos) [0x00058] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Running\BenchmarkRunnerClean.cs:534 at BenchmarkDotNet.Running.BenchmarkRunnerClean.Run (BenchmarkDotNet.Running.BenchmarkRunInfo[] benchmarkRunInfos) [0x00014] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Running\BenchmarkRunnerClean.cs:40 at BenchmarkDotNet.Running.BenchmarkRunner.RunWithDirtyAssemblyResolveHelper (System.Type type, BenchmarkDotNet.Configs.IConfig config) [0x00000] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Running\BenchmarkRunnerDirty.cs:77 at BenchmarkDotNet.Running.BenchmarkRunner+<>c__DisplayClass0_0`1[T].<Run>b__0 () [0x00000] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Running\BenchmarkRunnerDirty.cs:23 at BenchmarkDotNet.Running.BenchmarkRunner.RunWithExceptionHandling (System.Func`1[TResult] run) [0x00002] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Running\BenchmarkRunnerDirty.cs:107 at BenchmarkDotNet.Running.BenchmarkRunner.Run[T] (BenchmarkDotNet.Configs.IConfig config) [0x00014] in C:\src\BenchmarkDotNet\src\BenchmarkDotNet\Running\BenchmarkRunnerDirty.cs:23 at BenchmarkDotNet.Samples.Forms.MainPage+<>c__DisplayClass1_0.<Button_Clicked>b__0 () [0x0000f] in C:\src\BenchmarkDotNet\samples\BenchmarkDotNet.Samples.Forms\MainPage.xaml.cs:32 at System.Threading.Tasks.Task.InnerInvoke () [0x0000f] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corert/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:2476 at System.Threading.Tasks.Task.Execute () [0x00000] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/external/corert/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs:2319 --- End of stack trace from previous location where exception was thrown --- at BenchmarkDotNet.Samples.Forms.MainPage.Button_Clicked (System.Object sender, System.EventArgs e) [0x00077] in C:\src\BenchmarkDotNet\samples\BenchmarkDotNet.Samples.Forms\MainPage.xaml.cs:26 In 9c0f523, I fixed `DefaultConfig.ArtifactPath`'s usage of `Directory.GetCurrentDirectory()` so that `ArtifactPath` defaults to a writeable directory on Android. We need the same fix in `DebugConfig.ArtifactPath`, but I also added `RuntimeInformation.IsAndroid()` and `RuntimeInformation.IsiOS()` to make things a bit better. I updated the Xamarin sample to use a `DebugInProcessConfig` for `Debug` configurations.
b0889c3
to
a79d91e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathanpeppers Looks very good to me. Big thanks for your work on Xamarin support!
Using
DebugInProcessConfig
results in an error on Android:In 9c0f523, I fixed
DefaultConfig.ArtifactPath
's usage ofDirectory.GetCurrentDirectory()
so thatArtifactPath
defaults to awriteable directory on Android.
We need the same fix in
DebugConfig.ArtifactPath
, but I also addedRuntimeInformation.IsAndroid()
andRuntimeInformation.IsiOS()
tomake things a bit better.
I updated the Xamarin sample to use a
DebugInProcessConfig
forDebug
configurations.