[Xamarin.Android.Build.Tests] Fix the processing of the build.log files. #968
Conversation
This file contains hidden or 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
akoeplinger
reviewed
Oct 24, 2017
| data.Initialize (); | ||
| fs.Read (data, 0, 1024); | ||
| var line = System.Text.Encoding.UTF8.GetString (data); | ||
| Console.WriteLine (line); |
Member
There was a problem hiding this comment.
is this a debugging leftover or why do we print the line here?
jonpryor
reviewed
Oct 24, 2017
| var data =new byte [1024]; | ||
| while (fs.Position > 0) { | ||
| data.Initialize (); | ||
| fs.Read (data, 0, 1024); |
Contributor
There was a problem hiding this comment.
This should be data.Length, not 1024.
jonpryor
reviewed
Oct 24, 2017
| using (var fs = File.OpenRead (buildLogFullPath)) { | ||
| var data = new byte [1024]; | ||
| fs.Seek (data.Length, SeekOrigin.End); | ||
| while (fs.Position > 0) { |
Contributor
There was a problem hiding this comment.
Once concern here; assume a file that is < 1KB in size. The preceding fs.Seek(data.Length, SeekOrigin.End) will throw an ArgumentException (doh!), so it should probably instead be:
fs.Seek (Math.Min (data.Length, fs.Length), SeekOrigin.End);Additionally, if/when that happens, fs.Position will be 0 (beginning of file), which means while (fs.Position > 0) will skip over the file contents and nothing will be parsed.
So long as the file is > 1KB in size, we're fine, but if it isn't...
Contributor
Author
There was a problem hiding this comment.
Changed to a do while. and changed the seek too.
jonpryor
reviewed
Oct 24, 2017
| sb.AppendFormat ("\n#stderr begin\n{0}\n#stderr end\n", p.StandardError.ReadToEnd ()); | ||
|
|
||
| Console.WriteLine ("Searching for Time Elapsed"); | ||
| using (var fs = File.OpenRead (buildLogFullPath)) { |
Contributor
There was a problem hiding this comment.
Can buildLogFullPath be null? It's checked against null both earlier and later in this method, but it's not checked here, and thus could plausibly result in an ArgumentNullException.
Should this using block be merged with the one above, on lines 228-235 of the new file? The earlier loop reads each line individually; we could simply merge the Time Elapsed check there, and use line.StartsWith("Time Elapsed") for good measure.
Commit 029c86f kinda caused a perforanace problem because it was regex'ing the entire build log. Some of those logs can grow to 300+ meg in size. This commit reworks the build log processing to 1) Use a StringBuilder rather than concat strings. 2) Search for the build time by checking each line rarther than regex'ing the whole file.
Contributor
Author
|
build |
Redth
pushed a commit
to Redth/xamarin-android
that referenced
this pull request
Oct 30, 2017
…#968) Attempt to quasi-"optimize" `Builder.BuildInternal()`: if we're going to read `buildLogFullPath` a line at a time *anyway* -- to collect the log file for later parsing into `Builder.LastBuildOutput` -- then we should also do any associated line-oriented parsing as well. Update `buildLogFullPath` processing to use a `StringBuilder` instead of string concatenation, and search for the `Time Elapsed` message while reading the file, instead of separately, *after* having read the entire build log into memory.
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Apr 19, 2022
Fixes: dotnet/java-interop#967 Changes: dotnet/java-interop@05bfece...05eddd9 * dotnet/java-interop@05eddd9a: [generator] Add string cast to prevent CS1503 (dotnet#970) * dotnet/java-interop@37cff251: [Java.Base, generator] Bind all of package java.io (dotnet#968) * dotnet/java-interop@a65d6fb4: [Java.Base, generator] Bind all of package java.lang (dotnet#966) * dotnet/java-interop@ed9c2abf: [Java.Interop-MonoAndroid] Set Version after Directory.Build.props (dotnet#965)
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Apr 19, 2022
Fixes: dotnet/java-interop#967 Changes: dotnet/java-interop@05bfece...05eddd9 * dotnet/java-interop@05eddd9a: [generator] Add string cast to prevent CS1503 (dotnet#970) * dotnet/java-interop@37cff251: [Java.Base, generator] Bind all of package java.io (dotnet#968) * dotnet/java-interop@a65d6fb4: [Java.Base, generator] Bind all of package java.lang (dotnet#966) * dotnet/java-interop@ed9c2abf: [Java.Interop-MonoAndroid] Set Version after Directory.Build.props (dotnet#965)
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Apr 21, 2022
Fixes: dotnet/java-interop#967 Changes: dotnet/java-interop@05bfece...2a882d2 * dotnet/java-interop@2a882d2d: [generator] Fix xamarin-android/src/Mono.Android build (dotnet#972) * dotnet/java-interop@968e0f5f: [Directory.Build.props] Set dummy $(PackageVersion) to appease NuGet (dotnet#971) * dotnet/java-interop@05eddd9a: [generator] Add string cast to prevent CS1503 (dotnet#970) * dotnet/java-interop@37cff251: [Java.Base, generator] Bind all of package java.io (dotnet#968) * dotnet/java-interop@a65d6fb4: [Java.Base, generator] Bind all of package java.lang (dotnet#966) * dotnet/java-interop@ed9c2abf: [Java.Interop-MonoAndroid] Set Version after Directory.Build.props (dotnet#965)
jonpryor
added a commit
that referenced
this pull request
Apr 21, 2022
Fixes: dotnet/java-interop#967 Changes: dotnet/java-interop@05bfece...2a882d2 * dotnet/java-interop@2a882d2d: [generator] Fix xamarin-android/src/Mono.Android build (#972) * dotnet/java-interop@968e0f5f: [Directory.Build.props] Set dummy $(PackageVersion) to appease NuGet (#971) * dotnet/java-interop@05eddd9a: [generator] Add string cast to prevent CS1503 (#970) * dotnet/java-interop@37cff251: [Java.Base, generator] Bind all of package java.io (#968) * dotnet/java-interop@a65d6fb4: [Java.Base, generator] Bind all of package java.lang (#966) * dotnet/java-interop@ed9c2abf: [Java.Interop-MonoAndroid] Set Version after Directory.Build.props (#965)
jonathanpeppers
pushed a commit
that referenced
this pull request
Apr 25, 2022
Fixes: dotnet/java-interop#967 Changes: dotnet/java-interop@05bfece...2a882d2 * dotnet/java-interop@2a882d2d: [generator] Fix xamarin-android/src/Mono.Android build (#972) * dotnet/java-interop@968e0f5f: [Directory.Build.props] Set dummy $(PackageVersion) to appease NuGet (#971) * dotnet/java-interop@05eddd9a: [generator] Add string cast to prevent CS1503 (#970) * dotnet/java-interop@37cff251: [Java.Base, generator] Bind all of package java.io (#968) * dotnet/java-interop@a65d6fb4: [Java.Base, generator] Bind all of package java.lang (#966) * dotnet/java-interop@ed9c2abf: [Java.Interop-MonoAndroid] Set Version after Directory.Build.props (#965)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Commit 029c86f kinda caused a perforanace problem because it
was regex'ing the entire build log. Some of those logs can
grow to 300+ meg in size.
This commit reworks the build log processing to
Hopefully this will improve things.