[Xamarin.Android.Build.Tasks] Only Ignore Lint Checks if its supported. #924
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
Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=59861 We started to ignore the `StaticFieldLeak` lint check. But this was introduced in 26.0.2 of tools. So if we try to ignore this check in older versions it will error. Invalid id or category "StaticFieldLeak". So we need to version the checks we ignore by default. This commit puts these in a dictionary which includes the version number you can run them from.
jonpryor
reviewed
Oct 9, 2017
| return Version.Parse (MinimumSupportedJavaVersion); | ||
| } | ||
|
|
||
| Version GetLintVersion (string tool) { |
Contributor
There was a problem hiding this comment.
Code convention: { on new line for methods.
jonpryor
reviewed
Oct 9, 2017
|
|
||
| Version GetLintVersion (string tool) { | ||
| var sb = new StringBuilder (); | ||
| MonoAndroidHelper.RunProcess (tool, "--version", (s, e) => { |
Contributor
There was a problem hiding this comment.
Should this check the return value?
jonpryor
reviewed
Oct 9, 2017
| foreach (var dir in MonoAndroidHelper.AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) { | ||
| Version lintVersion = new Version (); | ||
| if (!string.IsNullOrEmpty (LintToolPath)) { | ||
| lintVersion = GetLintVersion (Path.Combine (LintToolPath, Lint)); |
Contributor
There was a problem hiding this comment.
Do we actually want to do the lint version check in <ResolveSdks/>? We won't always need to run lint, but we always run <ResolveSdks/>, and lint --version takes ~0.25s for me.
I think this should be moved to the <Lint/> task.
jonpryor
reviewed
Oct 9, 2017
| public string IntermediateOutputPath { get; set; } | ||
|
|
||
| [Required] | ||
| public string LintToolVersion { get; set; } |
Contributor
There was a problem hiding this comment.
Does this need to be a required property when we now execute lint --version within the task? I think we can remove this property, no?
Contributor
There was a problem hiding this comment.
The presence of [Required] should result in a build failure...
Redth
pushed a commit
to Redth/xamarin-android
that referenced
this pull request
Oct 30, 2017
…d. (dotnet#924) Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=59861 We started to ignore the `StaticFieldLeak` lint check. But this was introduced in 26.0.2 of tools. So if we try to ignore this check in older versions it will error. Invalid id or category "StaticFieldLeak". So we need to version the checks we ignore by default. This commit puts these in a dictionary which includes the version number you can run them from.
jonpryor
added a commit
to jonpryor/xamarin-android
that referenced
this pull request
Dec 7, 2021
Changes: dotnet/java-interop@7f1a5ab...bc5bcf4 * dotnet/java-interop@bc5bcf4f: [Java.Base] Begin binding JDK-11 java.base module (dotnet#909) * dotnet/java-interop@af91b9c2: [ci] Use descriptive test run titles. (dotnet#926) * dotnet/java-interop@aae8f251: [Java.Interop.Tools.Generator] Add some enumification helper methods (dotnet#866) * dotnet/java-interop@111ebca8: [Java.Interop] Treat warnings as errors. (dotnet#925) * dotnet/java-interop@7a32bb97: [java-source-utils] Transform XML using XSLT (dotnet#924) * dotnet/java-interop@7f55b2dc: [logcat-parse] Use C# verbatim strings for paths (dotnet#927) * dotnet/java-interop@2601146b: [java-source-utils] Fix regresion from 77c9c5fa (dotnet#923)
jonpryor
added a commit
that referenced
this pull request
Dec 9, 2021
Changes: dotnet/java-interop@7f1a5ab...aac3e9a * dotnet/java-interop@aac3e9ac: [Java.Interop, Java.Base] Fix xamarin-android integration issues (#929) * dotnet/java-interop@bc5bcf4f: [Java.Base] Begin binding JDK-11 java.base module (#909) * dotnet/java-interop@af91b9c2: [ci] Use descriptive test run titles. (#926) * dotnet/java-interop@aae8f251: [Java.Interop.Tools.Generator] Add some enumification helper methods (#866) * dotnet/java-interop@111ebca8: [Java.Interop] Treat warnings as errors. (#925) * dotnet/java-interop@7a32bb97: [java-source-utils] Transform XML using XSLT (#924) * dotnet/java-interop@7f55b2dc: [logcat-parse] Use C# verbatim strings for paths (#927) * dotnet/java-interop@2601146b: [java-source-utils] Fix regresion from 77c9c5fa (#923) (So many inadvertent issues as part of this bump…) dotnet/java-interop@bc5bcf4f added an *altered* `Java.Interop.JavaTypeParametersAttribute` custom attribute, altered to "appease" various Code Analysis warnings such as [CA1813][0] "Avoid unsealed attributes". This introduction caused some unit tests to fail to build: error CS0433: The type 'JavaTypeParametersAttribute' exists in both 'Java.Interop, Version=0.1.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065' and 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065' [C:\…\temp\LibraryProjectZipWithLint\BindingsProject\BindingsProject.csproj] The original idea had been to use [type forwarders][1], so that `Java.Interop.JavaTypeParametersAttribute` from `Mono.Android.dll` would be forwarded to the same type in `Java.Interop.dll`. This idea was rejected after further consideration, as it would likely break [forward compatibility][2] (dotnet/java-interop@e56a8c8e). dotnet/java-interop@aac3e9ac fixes this by making `JavaTypeParametersAttribute` conditional on `NET`, causing the type to only be present when targeting .NET 6+. `Mono.Android.dll` for .NET 6 -- and *not* Classic Xamarin.Android -- can then use a type forwarder. This can be done because there's no forward compatibility constraint on .NET 6. However, by making `JavaTypeParametersAttribute` conditional on `NET`, the `_CheckApiCompatibility` target started failing -- rightfully! -- because the `JavaTypeParametersAttribute` type had been removed. "Worse", there was no way to accept this change in e.g. `tests/api-compatibility/acceptable-breakages-vReference.txt`, because the `<CheckApiCompatibility/>` task requires that there be no extra entries. Square this circle by adding a `CheckApiCompatibility.TargetFramework` property, and looking for `acceptable-breakages-*-{TargetFramework}.txt` *before* looking for `acceptable-breakages-*.txt`. This allows us to special-case the acceptable breakages based on `$(TargetFramework)`. Finally, update `azure-pipelines.yaml` so that the `java-source-utils` unit tests are *not* run on Windows. This is because they currently fail on Windows, which is something we'll worry about later. For now, disable the `java-source-utils` tests when building on Windows. [0]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1813 [1]: https://docs.microsoft.com/en-us/dotnet/standard/assembly/type-forwarding [2]: https://en.wikipedia.org/wiki/Forward_compatibility
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 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.
Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=59861
We started to ignore the
StaticFieldLeaklint check. But thiswas introduced in 26.0.2 of tools. So if we try to ignore this
check in older versions it will error.
So we need to version the checks we ignore by default. This
commit puts these in a dictionary which includes the version
number you can run them from.