[Xamarin.Android.Build.Tasks] fixes for case-insensitive FS #3463
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A simple C# code change could break our build:
class Foo : Java.Lang.Object { }
Changed to:
class fOO : Java.Lang.Object { }
`javac` will fail on Windows (and likely macOS if you have a case
insensitive file system) with:
obj\Debug\android\src\md54a1bd985b0effc3cb8b7fa6ec8bb4028\Foo.java(4,8): error: class fOO is public, should be declared in a file named fOO.java
public class fOO
The only way to get the project working again is to do a `Clean` or
`Rebuild`.
This C# snippet illustrates the issue further:
File.WriteAllText("Foo.txt", "Foo");
File.WriteAllText("fOO.txt", "fOO");
foreach (var file in Directory.GetFiles (".", "*.txt"))
{
Console.WriteLine(file + " -> " + File.ReadAllText (file));
}
The output is:
.\Foo.txt -> fOO
The only solution is to put a `File.Delete()` prior to the second file
write -- there is no other way to sanely correct the casing on a file!
I added a call to `File.Delete()` in any of the `Files.Copy*IfChanged`
methods, which appears to solve the problem. I wrote a small battery
of tests to be sure things are working. One thing to note is that
`File.Delete()` does not throw if the file does not exist, and so we
don't need a `File.Exists` check. There are tests covering this case
as well.
We have been trying to *reduce* disk I/O, and so this is going to be a
slight performance hit. My thought is we could remove any calls to
`File.SetLastAccessTimeUtc` as they seem duplicative of
`File.SetLastWriteTimeUtc`.
After the fixes and removal of `File.SetLastAccessTimeUtc`, the
performance seems to be roughly the same if we look at two MSBuild
tasks that use `Copy*IfChanged` a lot. Here is three runs of a "fresh"
build of the Xamarin.Forms project in this repo:
Before:
172 ms CopyIfChanged 5 calls
151 ms CopyIfChanged 5 calls
161 ms CopyIfChanged 5 calls
4603 ms GenerateJavaStubs 1 calls
4368 ms GenerateJavaStubs 1 calls
4529 ms GenerateJavaStubs 1 calls
After:
157 ms CopyIfChanged 5 calls
180 ms CopyIfChanged 5 calls
150 ms CopyIfChanged 5 calls
4241 ms GenerateJavaStubs 1 calls
4496 ms GenerateJavaStubs 1 calls
4319 ms GenerateJavaStubs 1 calls
Note that the `<CopyIfChanged/>` calls `Files.CopyIfChanged` for each
input file internally, and the task itself was called 5 times here.
`<GenerateJavaStubs/>` calls `CopyIfStringChanged` for each Java stub.
Other changes:
* I removed all usage of `File.SetLastAccessTimeUtc` -- they seemed to
be duplicate everywhere.
* Fixed a bug I found, where if the destination file is readonly,
`Files.CopyIfChanged` failed...
* Removed a comment in `Files.cs`
jonathanpeppers
force-pushed
the
fs-case-sensitivity
branch
from
4824154
to
e9516d1
Compare
dellis1972
reviewed
Aug 9, 2019
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs
Show resolved
Hide resolved
jonpryor
pushed a commit
that referenced
this pull request
Aug 12, 2019
A simple C# code change could break our build. Consider:
class Foo : Java.Lang.Object { }
Changed it to (renaming `Foo` to `fOO`; note: capital-O, not zero):
class fOO : Java.Lang.Object { }
`javac` will fail on Windows (and likely macOS if you have a case
insensitive file system) with:
obj\Debug\android\src\md54a1bd985b0effc3cb8b7fa6ec8bb4028\Foo.java(4,8): error: class fOO is public, should be declared in a file named fOO.java
public class fOO
The only way to get the project working again is to do a `Clean` or
`Rebuild`.
This C# snippet illustrates the issue further:
File.WriteAllText("Foo.txt", "Foo"); // or File.Copy()...
File.WriteAllText("fOO.txt", "fOO"); // or File.Copy()...
foreach (var file in Directory.GetFiles (".", "*.txt")) {
Console.WriteLine(file + " -> " + File.ReadAllText (file));
}
The output is:
.\Foo.txt -> fOO
i.e. even though we wrote to the file `fOO.txt`,
`Directory.GetFiles()` only returns the *first* filename casing used.
The only solution is to put a `File.Delete()` prior to the second file
write -- there is no other way to sanely change the casing on a file!
I added a call to `File.Delete()` to many of the
`Files.Copy*IfChanged()` methods, which appears to solve the problem.
I wrote a small battery of tests to be sure things are working. One
thing to note is that `File.Delete()` does not throw if the file does
not exist, and so we don't need a `File.Exists()` check. There are
tests covering this case as well.
We have been trying to *reduce* disk I/O, and so this is going to be
a slight performance hit. My thought is we could remove any calls to
`File.SetLastAccessTimeUtc()` as they seem duplicative of
`File.SetLastWriteTimeUtc()`.
After the fixes and removal of `File.SetLastAccessTimeUtc`(), the
performance seems to be roughly the same if we look at two MSBuild
tasks that use `Files.Copy*IfChanged()` a lot. Here is three runs of
a "fresh" build of the Xamarin.Forms project in this repo:
Before:
172 ms CopyIfChanged 5 calls
151 ms CopyIfChanged 5 calls
161 ms CopyIfChanged 5 calls
4603 ms GenerateJavaStubs 1 calls
4368 ms GenerateJavaStubs 1 calls
4529 ms GenerateJavaStubs 1 calls
After:
157 ms CopyIfChanged 5 calls
180 ms CopyIfChanged 5 calls
150 ms CopyIfChanged 5 calls
4241 ms GenerateJavaStubs 1 calls
4496 ms GenerateJavaStubs 1 calls
4319 ms GenerateJavaStubs 1 calls
Note that the `<CopyIfChanged/>` calls `Files.CopyIfChanged()` for
each input file internally, and the task itself was called 5 times
here. `<GenerateJavaStubs/>` calls `Files.CopyIfStringChanged()` for
each Java stub.
Other changes:
* I removed all usage of `File.SetLastAccessTimeUtc()` -- they
seemed to be duplicate everywhere.
* Fixed a bug I found, where if the destination file is readonly,
`Files.CopyIfChanged()` failed...
* Removed a comment in `Files.cs`
|
A new Preview version has now been published that includes a fix for this item. Fix included in Xamarin.Android 10.0.0.40. Fix included on Windows in Visual Studio 2019 version 16.3 Preview 3. To try the Preview version that includes the fix, check for the latest updates in Visual Studio Preview. Fix included on macOS in Visual Studio 2019 for Mac version 8.3 Preview 3. To try the Preview version that includes the fix, check for the latest updates on the Preview updater channel. |
|
Release status update A new Release version has now been published that includes the fix for this item. Fix included in Xamarin.Android 10.0.0.43 Fix included on Windows in Visual Studio 2019 version 16.3. To get the new version that includes the fix, check for the latest updates or install the latest version from https://visualstudio.microsoft.com/downloads/. Fix included on macOS in Visual Studio 2019 for Mac version 8.3. To get the new version that includes the fix, check for the latest updates on the Stable updater channel. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A simple C# code change could break our build:
Changed to:
javacwill fail on Windows (and likely macOS if you have a caseinsensitive file system) with:
The only way to get the project working again is to do a
CleanorRebuild.This C# snippet illustrates the issue further:
The output is:
The only solution is to put a
File.Delete()prior to the second filewrite -- there is no other way to sanely correct the casing on a file!
I added a call to
File.Delete()in any of theFiles.Copy*IfChangedmethods, which appears to solve the problem. I wrote a small battery
of tests to be sure things are working. One thing to note is that
File.Delete()does not throw if the file does not exist, and so wedon't need a
File.Existscheck. There are tests covering this caseas well.
We have been trying to reduce disk I/O, and so this is going to be a
slight performance hit. My thought is we could remove any calls to
File.SetLastAccessTimeUtcas they seem duplicative ofFile.SetLastWriteTimeUtc.After the fixes and removal of
File.SetLastAccessTimeUtc, theperformance seems to be roughly the same if we look at two MSBuild
tasks that use
Copy*IfChangeda lot. Here is three runs of a "fresh"build of the Xamarin.Forms project in this repo:
Note that the
<CopyIfChanged/>callsFiles.CopyIfChangedfor eachinput file internally, and the task itself was called 5 times here.
<GenerateJavaStubs/>callsCopyIfStringChangedfor each Java stub.Other changes:
File.SetLastAccessTimeUtc-- they seemed tobe duplicate everywhere.
Files.CopyIfChangedfailed...Files.cs