Skip to content

Conversation

@jonathanpeppers
Copy link
Member

Changes as needed by #949

@dnfclas
Copy link

dnfclas commented Oct 25, 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

I think something isn't quite right:

An unexpected exception was thrown : System.UnauthorizedAccessException: Access to the path '/Users/builder/android-toolchain/sdk/docs/reference' is denied.

It's causing the Xamarin.Android.Tools.BytecodeTests.ParameterFixupTests.XmlDeclaration_FixedUpFromDocumentation() tests to fail, which isn't a previously failing test as far as I can tell.

@jonathanpeppers
Copy link
Member Author

I'll see if it happens for me locally tomorrow.

That test looks like it gets skipped on Java.Interop: https://jenkins.mono-project.com/view/Xamarin.Android/job/Java.Interop/164/testReport/Xamarin.Android.Tools.BytecodeTests/ParameterFixupTests/XmlDeclaration_FixedUpFromDocumentation/

The `ANDROID_SDK_PATH` environment variable isn't set; cannot test importing parameter names from HTML. Skipping...

I hope it means this test only runs under Xamarin.Android, and I didn't break something else.

@jonpryor
Copy link
Contributor

I hope it means this test only runs under Xamarin.Android, and I didn't break something else.

That is correct: that test only runs when executed within xamarin-android, not separately.

jonathanpeppers added a commit to jonathanpeppers/java.interop that referenced this pull request Oct 26, 2017
Context: dotnet/android#978

When bumping java.interop in xamarin-android, a test failure occurred
in the `ClassPath.GetDocletType` method. This was added in dotnet#188, but
since `ANDROID_SDK_PATH` is not set during java.interop’s test runs the
test was skipped.

The incoming `path` can be a directory, so the `File.OpenText` call
will fail in that case. Added a `File.Exists` check as a fix.
jonpryor pushed a commit to dotnet/java-interop that referenced this pull request Oct 27, 2017
Context: dotnet/android#978 (comment)

Commit 4d6c5a9 had a minor problem: it updated
`ClassPath.GetDocletType()` to check to see if the `path` parameter
was an `api.xml`-compatible XML file, but *earlier in the method*
it was *required* that `path` be a *directory*.
These semantics can't be intermixed, and resulted in an error from
`Xamarin.Android.Tools.BytecodeTests.ParameterFixupTests.XmlDeclaration_FixedUpFromDocumentation()`
during a xamarin-android integration PR:

	An unexpected exception was thrown : System.UnauthorizedAccessException: Access to the path '/Users/builder/android-toolchain/sdk/docs/reference' is denied.

Additionally, further code review demonstrated suggested that the
`ClassPath.DocletType` property was fundamentally *broken*:
`ClassPath.DocumentationPaths` can contain *multiple* paths, of
possibly *different* types, but `ClassPath.DocletType` -- if set --
was used for *all* `ClassPath.DocumentationPaths` values.
This doesn't make sense, in any way.

Remove `ClassPath.DocletType`, and update `class-parse --docstype`
documentation to note that it's obsolete.

Move `ClassPath.GetDocletType()` to
`AndroidDocScraper.GetDocletType()`, and make it public, so that it
can now be unit tested.

Fix the semantics of `AndroidDocScraper.GetDocletType(string path)`
so that `path` can be a directory or a file, as appropriate, without
generating an error in `XmlDeclaration_FixedUpFromDocumentation()`.

Cleanup the unit tests.
@jonathanpeppers jonathanpeppers changed the title Bump to Java.Interop/master/5cf61f5 Bump to Java.Interop/master/00ad8d6 Oct 27, 2017
@jonpryor jonpryor merged commit b2cf680 into dotnet:master Oct 27, 2017
@jonathanpeppers jonathanpeppers deleted the java-interop-bump branch October 27, 2017 22:14
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 5, 2022
Changes: dotnet/java-interop@2a882d2...4787e01

  * dotnet/java-interop@4787e017: [Java.Base-Tests] Test Java-to-Managed invocations for Java.Base (dotnet#975)
  * dotnet/java-interop@59716252: [build] `main` *conceptually* targets .NET 7 (dotnet#978)
  * dotnet/java-interop@61cdb40d: [generator] Fix reserved keywords binary search (dotnet#977)
jonpryor added a commit that referenced this pull request May 6, 2022
Changes: dotnet/java-interop@2a882d2...843f3c7

  * dotnet/java-interop@843f3c78: [Java.Base-Tests] Use $(UtilityOutputFullPath)/jcw-gen.dll (#979)
  * dotnet/java-interop@4787e017: [Java.Base-Tests] Test Java-to-Managed invocations for Java.Base (#975)
  * dotnet/java-interop@59716252: [build] `main` *conceptually* targets .NET 7 (#978)
  * dotnet/java-interop@61cdb40d: [generator] Fix reserved keywords binary search (#977)
jonpryor added a commit that referenced this pull request May 6, 2022
Changes: dotnet/java-interop@2a882d2...843f3c7

  * dotnet/java-interop@843f3c78: [Java.Base-Tests] Use $(UtilityOutputFullPath)/jcw-gen.dll (#979)
  * dotnet/java-interop@4787e017: [Java.Base-Tests] Test Java-to-Managed invocations for Java.Base (#975)
  * dotnet/java-interop@59716252: [build] `main` *conceptually* targets .NET 7 (#978)
  * dotnet/java-interop@61cdb40d: [generator] Fix reserved keywords binary search (#977)
@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.

3 participants