Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VS2019 16.11.2 Failing to build Android binding libraries #6271

Closed
kristijan-boskovic opened this issue Sep 8, 2021 · 10 comments
Closed

VS2019 16.11.2 Failing to build Android binding libraries #6271

kristijan-boskovic opened this issue Sep 8, 2021 · 10 comments
Assignees
Labels
Area: App+Library Build Issues when building Library projects or Application projects. need-info Issues that need more information from the author.

Comments

@kristijan-boskovic
Copy link

kristijan-boskovic commented Sep 8, 2021

After switching from Debug to Release in Visual Studio and trying to build the solution, machine in our office was not able to build Android binding libraries anymore (BoradRadio.FaceSdk and BroadRadio.RfidSdk). We tried switching back to Debug, cleaning&rebuilding, re-installing VS/Android Studio/All Android SDKs/Java JDKs, and nothing helped. During the build, /lang/something is always added to these castings within CreateDelegate, which leads to many build errors.

Log File

MyProjectOutput.log

@kristijan-boskovic kristijan-boskovic added Area: App+Library Build Issues when building Library projects or Application projects. needs-triage Issues that need to be assigned. labels Sep 8, 2021
@dellis1972
Copy link
Contributor

Are you able to provide a test project?

Or show the code generated for obj\Debug\generated\src\Android_serialport_api.SerialPort.cs ?
It might help us figure out why we are generating invalid C# code.

@dellis1972 dellis1972 added need-info Issues that need more information from the author. and removed needs-triage Issues that need to be assigned. labels Sep 8, 2021
@kristijan-boskovic
Copy link
Author

kristijan-boskovic commented Sep 8, 2021

Here is the test project that contains test Anroid binding library with files generated after failed build on our machine:
Testapp.zip

@dellis1972
Copy link
Contributor

The test app Binding project produces this error for me

obj/Debug/generated/src/Com.MV.Nfe.Version.cs(33,23): error CS0542: 'Version': member names cannot be the same as their enclosing type

Is that what you are seeing?

@kristijan-boskovic
Copy link
Author

kristijan-boskovic commented Sep 8, 2021

Sorry, we forgot to add one <remove-node ... /> in the Metadata.xaml of the BindingLibrary. Here is a version that builds on all the company machines besides the affected one. Screenshot related, that's some of the generated output with errors that we get. You should be able to find the generated files inside TestBindingLibrary\obj\Debug\generated\src.

Testapp.zip

Errors

@dellis1972
Copy link
Contributor

@jpobst any ideas on this one? If you look at the screen shot ^ we are generating delegates with method names which include / characters.

I can't repo this issue on my Mac, and the NamespaceMapping.cs file does not contain any entries with a / in them.

@jpobst
Copy link
Contributor

jpobst commented Sep 8, 2021

We've had one report of this before, but I could not reproduce it. It built fine locally and on our CI: https://developercommunity.visualstudio.com/t/XamarinAndroid-binding-compiling-proble/1477963.

I'll see if this test case shows anything to help us track this down.

@jpobst
Copy link
Contributor

jpobst commented Sep 8, 2021

Same as the other one, it works fine here. And the code that generates this referenced in the other ticket still looks like there is no possible way this could get output.

image

@jpobst
Copy link
Contributor

jpobst commented Sep 8, 2021

This does indeed seem to be a string-culture issue. That is:

var ci = new CultureInfo ("hr");
Console.WriteLine ("Ljava".StartsWith ("L", false, ci));

returns False for Bosnian, Croatian, and Serbian because "lj" is considered a single letter.

I've opened a PR which fixes this issue and enables the string localization code analyzers to help ensure we catch other issues like this and do not create any new ones in the future:
dotnet/java-interop#879

I think the only workaround is to use a different culture for now.

@kristijan-boskovic
Copy link
Author

@jpobst yes, you are right! The issue was in Regional format being set to Croatian in Windows 10 settings. Now that we switched it to English, everything builds correctly. Just to inform you, in case you are not familiar, along with "lj", there are two more two-character letters in Croatian/Bosnian/Serbian: "dž" and "nj", which could cause the same issue. We tested it, and they also return False in StartsWith test.

I can't thank you guys enough for helping us resolve this issue. Cheers!

jonpryor pushed a commit to dotnet/java-interop that referenced this issue Sep 9, 2021
Context: dotnet/android#6271
Context: https://developercommunity.visualstudio.com/t/XamarinAndroid-binding-compiling-proble/1477963
Context: https://www.learncroatian.eu/blog/the-croatian-letters

If you attempt to build an Android Binding project on Windows within
a Bosnian, Croatian, or Serbian locale, the build may break as the
delegate type names created by 56955d9 may not be valid C#, e.g.:

	delegate IntPtr _JniMarshal_PP_Ljava/Lang/String; (IntPtr jnienv, IntPtr klass)

It *should* be declaringn a `_JniMarshal_PP_L` type, *not*
`_JniMarshal_PP_Ljava/Lang/String;`, the latter of which results in
numerous C# errors:

	error CS1003: Syntax error, '(' expected
	error CS1001: Identifier expected
	error CS1001: Identifier expected
	error CS1003: Syntax error, ',' expected
	error CS1003: Syntax error, ',' expected
	error CS1001: Identifier expected
	error CS1026: ) expected

The problem is caused by the interplay of two factors:

 1. Commit 56955d9 uses the culture-sensitive
    [`string.StartsWith(string)`][0] method to determine if a JNI
    type name starts with `L`:

        if (jni_name.StartsWith ("L") || jni_name.StartsWith ("["))
            return "L";

 2. In the `bs`, `hr`, and `sr` locales, the strings `Lj` and `lj`
    are treated as a single letter, *distinct from* `L` or `l`.
    In those locales, this expression is *false*, not true:
    
        "Ljava/lang/String;".StartsWith ("L") // false in bs, hr, sr; true everywhere else

Additionally, this issue only arises when Java package names
starting with `java` are used, e.g. `Ljava/lang/Object;`.  Java types
from packages that *don't* start with `java` don't encounter this bug.

Fix this issue by enabling the [CA1307][1] and [CA1309][2] rules,
previously disabled in commit ac914ce.  These code analysis rules
require the use of string methods which use the
[`StringComparison`][3] enumeration, for which we then specify
`StringComparison.Ordinal`, which is *not* culture-sensitive.

One complication with enabling these rules is that the .NET 6+
version of these rules are stricter and require overloads that do not
exist in .NET Framework or .NET Standard to fix the violations.
Enabling these rules in `.editorconfig` affects all
`$(TargetFrameworkMoniker)`s; we will instead use
`Directory.Build.props` to only enable them for non-.NET 6+ builds.

Finally, add a new `.yaml` step that shuts down the cached `dotnet`
MSBuild processes between invocations, to fix the error that has been
happening on Windows - NET Core:

	error MSB3027: Could not copy "obj\\Release\Java.Interop.BootstrapTasks.dll" to "D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll". Exceeded retry count of 10. Failed.  [D:\a\1\s\build-tools\Java.Interop.BootstrapTasks\Java.Interop.BootstrapTasks.csproj]
	error MSB3021: Unable to copy file "obj\\Release\Java.Interop.BootstrapTasks.dll" to "D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll". The process cannot access the file 'D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll' because it is being used by another process. [D:\a\1\s\build-tools\Java.Interop.BootstrapTasks\Java.Interop.BootstrapTasks.csproj]

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.string.startswith?view=net-5.0#System_String_StartsWith_System_String_
[1]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1307
[2]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1309
[3]: https://docs.microsoft.com/en-us/dotnet/api/system.stringcomparison?view=net-5.0
@jpobst
Copy link
Contributor

jpobst commented Sep 9, 2021

Glad you got it working!

Fix committed in dotnet/java-interop#879.

@jpobst jpobst closed this as completed Sep 9, 2021
jonpryor pushed a commit that referenced this issue Sep 16, 2021
Fixes: #6271

Context: 2d81740
Context: #5749
Context: dotnet/java-interop@d16b1e5

Changes: dotnet/java-interop@f359e73...8f7ddcd

  * dotnet/java-interop@8f7ddcdd: [build] Fix 'BuildVersionInfo_g_cs' build target. (#881)
  * dotnet/java-interop@7068f4b2: [build] Enable string operations globalization code analyzers. (#879)
  * dotnet/java-interop@3e6a6232: [build] Use GitInfo to Generate Version information. (#875)

In order to better support building solutions which reference both
.NET 6 and legacy projects within Visual Studio, we have begun
strong-naming all the MSBuild-related assemblies; see also 2d81740.
This allows loading two different version of the "same" assembly, e.g.
`Java.Interop.Tools.Diagnostics.dll`, from two different filesystem
locations in the same AppDomain/AssemblyLoadContext.

However, strong-naming is only part of the solution.  The other part
is that, for sanity and reliability, the "two different versions of
the 'same' assembly" should *also* have different assembly *versions*.
If they have the same assembly version, are they truly different?

Multiple sub-modules have been updated to use the [`GitInfo`][0]
[NuGet Package][1] so that assembly versions are unique per-commit:

  * `external/xamarin-android-tools`: 25c5cb5
  * `.external` & `monodroid`: 2d81740

Update the `external/Java.Interop` submodule to follow suit.

This in turn requires updates to `xaprepare` and the CI YAML files to
ensure that `GitInfo` is configured correctly within Java.Interop.

[0]: https://github.com/devlooped/GitInfo
[1]: https://www.nuget.org/packages/GitInfo/2.1.2
jpobst added a commit to dotnet/java-interop that referenced this issue Sep 30, 2021
Context: dotnet/android#6271
Context: https://developercommunity.visualstudio.com/t/XamarinAndroid-binding-compiling-proble/1477963
Context: https://www.learncroatian.eu/blog/the-croatian-letters

If you attempt to build an Android Binding project on Windows within
a Bosnian, Croatian, or Serbian locale, the build may break as the
delegate type names created by 56955d9 may not be valid C#, e.g.:

	delegate IntPtr _JniMarshal_PP_Ljava/Lang/String; (IntPtr jnienv, IntPtr klass)

It *should* be declaringn a `_JniMarshal_PP_L` type, *not*
`_JniMarshal_PP_Ljava/Lang/String;`, the latter of which results in
numerous C# errors:

	error CS1003: Syntax error, '(' expected
	error CS1001: Identifier expected
	error CS1001: Identifier expected
	error CS1003: Syntax error, ',' expected
	error CS1003: Syntax error, ',' expected
	error CS1001: Identifier expected
	error CS1026: ) expected

The problem is caused by the interplay of two factors:

 1. Commit 56955d9 uses the culture-sensitive
    [`string.StartsWith(string)`][0] method to determine if a JNI
    type name starts with `L`:

        if (jni_name.StartsWith ("L") || jni_name.StartsWith ("["))
            return "L";

 2. In the `bs`, `hr`, and `sr` locales, the strings `Lj` and `lj`
    are treated as a single letter, *distinct from* `L` or `l`.
    In those locales, this expression is *false*, not true:
    
        "Ljava/lang/String;".StartsWith ("L") // false in bs, hr, sr; true everywhere else

Additionally, this issue only arises when Java package names
starting with `java` are used, e.g. `Ljava/lang/Object;`.  Java types
from packages that *don't* start with `java` don't encounter this bug.

Fix this issue by enabling the [CA1307][1] and [CA1309][2] rules,
previously disabled in commit ac914ce.  These code analysis rules
require the use of string methods which use the
[`StringComparison`][3] enumeration, for which we then specify
`StringComparison.Ordinal`, which is *not* culture-sensitive.

One complication with enabling these rules is that the .NET 6+
version of these rules are stricter and require overloads that do not
exist in .NET Framework or .NET Standard to fix the violations.
Enabling these rules in `.editorconfig` affects all
`$(TargetFrameworkMoniker)`s; we will instead use
`Directory.Build.props` to only enable them for non-.NET 6+ builds.

Finally, add a new `.yaml` step that shuts down the cached `dotnet`
MSBuild processes between invocations, to fix the error that has been
happening on Windows - NET Core:

	error MSB3027: Could not copy "obj\\Release\Java.Interop.BootstrapTasks.dll" to "D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll". Exceeded retry count of 10. Failed.  [D:\a\1\s\build-tools\Java.Interop.BootstrapTasks\Java.Interop.BootstrapTasks.csproj]
	error MSB3021: Unable to copy file "obj\\Release\Java.Interop.BootstrapTasks.dll" to "D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll". The process cannot access the file 'D:\a\1\s\bin\BuildRelease\Java.Interop.BootstrapTasks.dll' because it is being used by another process. [D:\a\1\s\build-tools\Java.Interop.BootstrapTasks\Java.Interop.BootstrapTasks.csproj]

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.string.startswith?view=net-5.0#System_String_StartsWith_System_String_
[1]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1307
[2]: https://docs.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1309
[3]: https://docs.microsoft.com/en-us/dotnet/api/system.stringcomparison?view=net-5.0
jonpryor pushed a commit that referenced this issue Oct 8, 2021
Context: #6271
Context: dotnet/java-interop@7068f4b

Making culture-aware string comparisons when not intended can result
in very hard to diagnose bugs (See/#6271).

As we did in dotnet/java-interop@7068f4b2, update the appropriate
Roslyn code analyzers so that errors are generated around "unsafe"
string usage, and to ensure that we don't introduce any new issues
around string & globalization support in the future.

One complication with enabling these rules is that the .NET 6+ version
of these rules are stricter and require overloads that do not exist in
.NET Framework or .NET Standard to fix the violations.  Enabling these
rules in `.editorconfig` affects all `$(TargetFrameworkMoniker)`s;
we will instead use `Configuration.props` to only enable them for
non-.NET 6+ builds.

Additionally, tweak and reuses the existing Roslyn Analyzers logic to
exclude external code, as many libraries that are used for testing
(like `Mono.Debugg[ing|er].Soft)` have issues surfaced by these
analyzers.

Note that some non-test comparisons on filenames/paths were switched
from case-sensitive to case-insensitive compares.
@ghost ghost locked as resolved and limited conversation to collaborators Jun 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App+Library Build Issues when building Library projects or Application projects. need-info Issues that need more information from the author.
Projects
None yet
Development

No branches or pull requests

4 participants