Skip to content

Conversation

@jonathanpeppers
Copy link
Member

Context:
https://devdiv.visualstudio.com/DevDiv/_build/index?buildId=1047263

There are a couple fixes to get the BCL tests (and
Xamarin.Android-Tests.sln) compiling on Windows.

The SystemUnzip task is passing some file paths to
Directory.EnumerateFiles. Since it does not appear that this would
work; in general, it seems that GetExtractedSourceEntries should only
return directories as a simple fix to this problem.

Next, MSBuild is getting a duplicate <Compile /> entry passed to Csc
for obj/Debug/App.cs. There was a conditional <Compile /> entry as
well as one added during the _GenerateApp_cs task. It seems simpler to
just remove the condition and the duplicate <Compile /> entry, since
<BuildDependsOn /> causes the _GenerateApp_cs task to run first.

@dnfclas
Copy link

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

build

}
}

IEnumerable<string> GetExtractedSourceEntries (string root)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename this method to GetExtractedSourceDirectories(), as it now only returns directories, not file system entries.

Context:
https://devdiv.visualstudio.com/DevDiv/_build/index?buildId=1047263

There are a couple fixes to get the BCL tests (and
Xamarin.Android-Tests.sln) compiling on Windows.

The `SystemUnzip` task is passing some file paths to
`Directory.EnumerateFiles`. Since it does not appear that this would
work; in general, it seems that `GetExtractedSourceEntries` should only
return directories as a simple fix to this problem.

Next, MSBuild is getting a duplicate `<Compile />` entry passed to `Csc`
for `obj/Debug/App.cs`. There was a conditional `<Compile />` entry as
well as one added during the `_GenerateApp_cs` task. It seems simpler to
just remove the condition and the duplicate `<Compile />` entry, since
`<BuildDependsOn />` causes the `_GenerateApp_cs` task to run first.
@jonpryor jonpryor merged commit ca3644e into dotnet:master Oct 11, 2017
@jonathanpeppers jonathanpeppers deleted the windows-bcl-tests branch October 11, 2017 21:18
Redth pushed a commit to Redth/xamarin-android that referenced this pull request Oct 30, 2017
Context: https://devdiv.visualstudio.com/DevDiv/_build/index?buildId=1047263

A couple of fixes to get `Xamarin.Android.Bcl-Tests` and
`Xamarin.Android-Tests.sln` compiling on Windows:

The `<SystemUnzip/>` task is passing some file paths to
`Directory.EnumerateFiles()`:

	System.IO.IOException: The directory name is invalid.
	at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
	at System.IO.FileSystemEnumerableIterator`1.CommonInit()
	at System.IO.FileSystemEnumerableIterator`1..ctor(String path, String originalUserPath, String searchPattern, SearchOption searchOption, SearchResultHandler`1 resultHandler, Boolean checkHost)
	at System.IO.Directory.EnumerateFiles(String path, String searchPattern, SearchOption searchOption)
	at Xamarin.Android.BuildTools.PrepTasks.SystemUnzip.<ExtractFile>d__22.MoveNext() in E:\A\_work\1\s\build-tools\xa-prep-tasks\Xamarin.Android.BuildTools.PrepTasks\SystemUnzip.cs:line 128


Fix this by renaming `SystemUnzip.GetExtractedSourceEntries()` to
`SystemUnzip.GetExtractedSourceDirectories()` and have it return
only directories.

MSBuild is getting a duplicate `@(Compile)` entry passed to `<Csc/>`
for `obj/Debug/App.cs`. There was a conditional `@(Compile)` entry as
well as one added during the `_GenerateApp_cs` target. It seems simpler to
just remove the condition and the duplicate `@(Compile)` entry, since
`$(BuildDependsOn)` causes the `_GenerateApp_cs` target to run first.
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
@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