Skip to content

Conversation

@jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Oct 31, 2017

Context in #992

The lint.bat tool installed to ~\android-toolchain\sdk\tools\bin
appears to be broken on Windows, so a few tests are failing that use it.

There is a workaround to set the JAVA_OPTS environment variable
that gets these tests passing on Windows. Hopefully, this is a temporary
workaround until a new version of the SDK tools comes out that fixes
this issue.

One other thing to note here is that the Android SDK installed by Visual
Studio uses the SDK tools version 25.x, which does not appear to have
this issue on windows.

@dnfclas
Copy link

dnfclas commented Oct 31, 2017

@jonathanpeppers,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@jonpryor
Copy link
Contributor

jonpryor commented Nov 1, 2017

@jonathanpeppers: Please file an upstream issue at https://issuetracker.google.com/issues?q=componentid:192633%2B (the BROWSE ALL DEVELOPER TOOLS ISSUES link here), reporting that lint.bat --version reports unknown version.

@dellis1972: Is ignoring the lint-based tests on Windows the right thing to do? Or should we instead update the <Lint/> task so that if lint --version reports unknown version we instead treat it as e.g. v1.0.0 or whatever the current version is?

@jonathanpeppers
Copy link
Member Author

Issue filed here: https://issuetracker.google.com/issues/68753324

@dellis1972
Copy link
Contributor

Lint unknown version check was added in #996

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Nov 3, 2017

#996 is helpful, but further failures happen, such as:

~\android-toolchain\sdk\tools\bin\lint.bat --quiet --disable "StaticFieldLeak,ObsoleteSdkInt" --resources obj\Debug\res\ obj\Debug\  (TaskId:347)
~\Desktop\Git\xamarin-android\bin\Debug\lib\xamarin.android\xbuild\Xamarin\Android\Xamarin.Android.Common.targets(925,2): error : Can't find API database; API check not performed [LintError] [~\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckLintErrorsAndWarnings\UnnamedProject.csproj]
obj\Debug\android\AndroidManifest.xml(6,142): error XA0103:  Avoid hardcoding the debug mode; leaving it out allows debug and release builds to automatically assign one [HardcodedDebugMode] [~\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckLintErrorsAndWarnings\UnnamedProject.csproj]
obj\Debug\obj\Debug\res\layout\main.xml(1): warning XA0102:  The resource R.layout.main appears to be unused [UnusedResources] [~\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckLintErrorsAndWarnings\UnnamedProject.csproj]
obj\Debug\obj\Debug\res\values\strings.xml(1,71): warning XA0102:  The resource R.string.app_name appears to be unused [UnusedResources] [~\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckLintErrorsAndWarnings\UnnamedProject.csproj]
obj\Debug\obj\Debug\res\values\strings.xml(1,19): warning XA0102:  The resource R.string.hello appears to be unused [UnusedResources] [~\Desktop\Git\xamarin-android\bin\TestDebug\temp\CheckLintErrorsAndWarnings\UnnamedProject.csproj]
  2 errors, 3 warnings (TaskId:347)

I even installed latest Android Studio & its SDK tools, and the issue is present on Windows.

There seem to be two ways to fix it:

  1. Modify lint.bat, which is what I would do as a user
  2. Set JAVA_OPTS env var to set the full path to the tools directory "-Dcom.android.tools.lint.bindir=ANDROID_SDK_PATH\tools"

It depends on if people are really running into this, if we want to go as far as doing number 2.

It seems if I use the Android SDK installed with Visual Studio, it uses the older SDK tools (25.x) that are working fine on Windows.

Context in dotnet#992

The lint.bat tool installed to ~\android-toolchain\sdk\tools\bin
appears to be broken on Windows, so a few tests are failing that use it.

There is a workaround to set the `JAVA_OPTS` environment variable
that gets these tests passing on Windows. Hopefully, this is a temporary
workaround until a new version of the SDK tools comes out that fixes
this issue.

One other thing to note here is that the Android SDK installed by Visual
Studio uses the SDK tools version 25.x, which does not appear to have
this issue on windows.
@jonathanpeppers jonathanpeppers force-pushed the windows-ignore-lint-tests branch from fce86c1 to ce3d766 Compare November 3, 2017 14:47
@jonathanpeppers jonathanpeppers changed the title [tests] ignore lint tests on Windows [tests] workaround broken lint.bat on Windows Nov 3, 2017
@dellis1972
Copy link
Contributor

Generally we have shy'd away from modifying the .bat files. We ended up bypassing the proguard/multidex stuff completely with our own calls in the end.

The EnvVar we can do fairly easily.

I suspect people are not running the lint tool at all on windows. Its not on by default so I imagine most users don't bother.

@dellis1972
Copy link
Contributor

@jonathanpeppers is there a way to detect this issue and apply the fix in the Lint.cs Task ? I assume it might be needed by end users if they do use it?

@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Nov 3, 2017

After looking at this, I think we could do this in the Lint task:

  • If the "unknown version" state is detected
  • Set the environment variable
  • Try to check the version again

That being said, I don't know how high of a priority this is compared to other things. It doesn't seem like many people are using this feature.

It might be better to just ignore these tests on Windows for now, but I'm open to trying to implement this change to the Lint task if you guys think we should.

@jonpryor jonpryor merged commit b10508a into dotnet:master Nov 7, 2017
@jonathanpeppers jonathanpeppers deleted the windows-ignore-lint-tests branch June 12, 2019 03:14
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants