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.

Sign up for GitHub

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

[java-interop] Windows build system support #816

Merged
merged 29 commits into from
Mar 17, 2021
Merged

[java-interop] Windows build system support #816

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
dfa97b2
[java-interop] Add `cmake`-based build system
jonpryor Mar 11, 2021
6ede47a
Enable tests on Windows+.NET Core!
jonpryor Mar 11, 2021
e134baf
Upload `bin` dir fir windows_dotnet_build
jonpryor Mar 12, 2021
0969a84
Explicitly set RunConfiguration.TargetPlatform=x64 ?
jonpryor Mar 12, 2021
6babd68
What *is* dotnet.exe? 32-bit or 64-bit?
jonpryor Mar 12, 2021
c14ebb6
Take 2?
jonpryor Mar 12, 2021
08348d3
*Are* we in a 32-bit process?
jonpryor Mar 12, 2021
e4be4ed
Install .NET 5.0.103.
jonpryor Mar 12, 2021
f4d1109
Build java-interop.dll for x86, x64
jonpryor Mar 12, 2021
ea91ce2
Revert "*Are* we in a 32-bit process?"
jonpryor Mar 13, 2021
c45db89
Revert "Take 2?"
jonpryor Mar 13, 2021
53531d8
Revert "What *is* dotnet.exe? 32-bit or 64-bit?"
jonpryor Mar 13, 2021
d1eadab
Revert "Explicitly set RunConfiguration.TargetPlatform=x64 ?"
jonpryor Mar 13, 2021
94c694a
Fix NativeTiming build
jonpryor Mar 13, 2021
300f1c4
Now it doesn't build on Windows. Why?
jonpryor Mar 13, 2021
e17afe3
Build is failing because the `&&` isn't present.
jonpryor Mar 14, 2021
9468871
Doh; NativeTiming was missing &&
jonpryor Mar 14, 2021
e30a583
Cleanup!
jonpryor Mar 14, 2021
958372c
Revert "Now it doesn't build on Windows. Why?"
jonpryor Mar 14, 2021
95ec6a1
Revert "Upload `bin` dir fir windows_dotnet_build"
jonpryor Mar 14, 2021
cc2ef8b
Use target batching
jonpryor Mar 16, 2021
7016a28
"Multitarget" NativeTiming as well
jonpryor Mar 16, 2021
eb4eab1
Use Ninja instead of Makefiles.
jonpryor Mar 16, 2021
14d3b11
Revert "Use Ninja instead of Makefiles."
jonpryor Mar 16, 2021
5b1b04c
C++ warning cleanup
jonpryor Mar 16, 2021
44f83ce
Avoid $(OS)
jonpryor Mar 16, 2021
4f46058
set CMAKE_C_STANDARD to 11
jonpryor Mar 16, 2021
0bccb2d
Remove diagnostic messages.
jonpryor Mar 16, 2021
edf6d9d
ITEM GROUP
jonpryor Mar 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion build-tools/automation/azure-pipelines.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pr:
variables:
RunningOnCI: true
Build.Configuration: Release
DotNetCoreVersion: 3.1.300
DotNetCoreVersion: 5.0.103
HostedMacImage: macOS-10.15
HostedWinVS2019: Hosted Windows 2019 with VS2019
NetCoreTargetFrameworkPathSuffix: -netcoreapp3.1
Expand Down Expand Up @@ -90,6 +90,8 @@ jobs:
- template: templates\core-build.yaml

- template: templates\core-tests.yaml
parameters:
runNativeDotnetTests: true

- template: templates\fail-on-issue.yaml

Expand Down
2 changes: 1 addition & 1 deletion build-tools/automation/templates/core-tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ steps:

- task: DotNetCoreCLI@2
displayName: 'Tests: Java.Interop'
condition: eq('${{ parameters.runNativeTests }}', 'true')
condition: or(eq('${{ parameters.runNativeDotnetTests }}', 'true'), eq('${{ parameters.runNativeTests }}', 'true'))
inputs:
command: test
arguments: bin/Test$(Build.Configuration)$(NetCoreTargetFrameworkPathSuffix)/Java.Interop-Tests.dll
Expand Down
18 changes: 18 additions & 0 deletions build-tools/scripts/NativeToolchain.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>
<Target Name="GetNativeBuildCommands">
<ItemGroup Condition=" '$(VSINSTALLROOT)' != '' And Exists('$(VSINSTALLROOT)') ">
<_Vcvarsall
Include="$(VSINSTALLROOT)\VC\Auxiliary\Build\vcvarsall.bat"
/>
</ItemGroup>
<PropertyGroup Condition=" '@(_Vcvarsall->Count())' != '0' ">
<_Vcvarsall>%(_Vcvarsall.Identity)</_Vcvarsall>
<PrepareNativeToolchain>call "$(_Vcvarsall)" </PrepareNativeToolchain>
</PropertyGroup>
<PropertyGroup>
<CmakeGenerator Condition=" '$(OS)' != 'Windows_NT' ">-G "Unix Makefiles"</CmakeGenerator>
Copy link
Contributor

Choose a reason for hiding this comment

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

If you can use Ninja, it tends to be faster

Copy link
Member Author

Choose a reason for hiding this comment

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

Alas, attempting to use ninja broke on the macOS CI. :-(

<CmakeGenerator Condition=" '$(OS)' == 'Windows_NT' ">-G "NMake Makefiles"</CmakeGenerator>
</PropertyGroup>
</Target>
</Project>
73 changes: 73 additions & 0 deletions build-tools/scripts/RunCmake.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Prepare" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Target Name="Cmake"
DependsOnTargets="_ValidateCmake;_RunCmake"
/>
<Target Name="_ValidateCmake">
<Error
Condition=" '$(CmakePath)' == '' "
Text="Set the `%24(CmakePath)` property."
/>
<Error
Condition=" '$(CmakeGenerator)' == '' "
Text="Set the `%24(CmakeGenerator)` property."
/>
<Error
Condition=" '$(CmakeSourceDir)' == '' "
Text="Set the `%24(CmakeSourceDir)` property."
/>
<Error
Condition=" '$(CmakeBuildDir)' == '' "
Text="Set the `%24(CmakeBuildDir)` property."
/>
</Target>

<Target Name="_RunCmake">
<PropertyGroup>
<_Prepare>$(PrepareNativeToolchain)</_Prepare>
<_Prepare Condition=" '$(_Prepare)' != '' And !$(_Prepare.Trim().EndsWith('&amp;&amp;')) ">$(_Prepare) &amp;&amp;</_Prepare>
<_SourceDir>$(CmakeSourceDir.Replace('%5c', '/'))</_SourceDir>
<_BuildDir>$(CmakeBuildDir.Replace('%5c', '/'))</_BuildDir>
<_ExtraArgs>$(CmakeExtraArgs.Replace('%5c', '/'))</_ExtraArgs>
Comment on lines +29 to +31
Copy link
Member

Choose a reason for hiding this comment

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

Since this is already working, this is fine probably fine. But was the main issue here the <Exec/> task?

Maybe it would work to make a small <UnixExec/> task that subclasses <Exec/> and replaces all the \ with / in Command? Then base.Execute() would succeed?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jonathanpeppers : the problem isn't "unix", the problem is double quotes. :-)

"General guidelines", at least on Unix, is to quote anything which could possibly have a space…such as path names! The problem is when:

  1. The "path name" is a directory name, and
  2. The directory name ends with \.

The result is that you escape the quote!

$(_Prepare) $(CmakePath) $(CmakeGenerator) -S &quot;$(_SourceDir)&quot;

Here, $(_SourceDir) will be e.g. $(MSBuildThisFileDirectory), which aways ends with \. If we didn't "fix" $(_SourceDir), we'd have:

make -S "C:\some\directory\" …

The attempt to "sanely" quote the directory name results in an invalid command line.

Does this mean that s,\,/,g is "best"? No; we could instead do:

<_SourceDir>$(CmakeSourceDir.TrimEnd('%5c'))</_SourceDir>

But! If a value can be "part of" the Cmake files, e.g. Cmake variables, those values must use /, not \, because Cmake otherwise attempts to treat \ as an escape character. Thus, $(CmakeExtraArgs) must replace \ with /, as it contains Cmake property overrides that contain paths.

In neither case does a <UnixExec/> class help, unless a "command global" s,\,/,g is desired. Note however that we don't "fixup" $(PrepareNativeToolchain) or $(CmakePath).

</PropertyGroup>
<Exec
ContinueOnError="WarnAndContinue"
Command="$(_Prepare) $(CmakePath) $(CmakeGenerator) -S &quot;$(_SourceDir)&quot; -B &quot;$(_BuildDir)&quot; $(_ExtraArgs) &amp;&amp; $(CmakePath) --build &quot;$(_BuildDir)&quot; -v"
/>
<PropertyGroup>
<_CmakeStatus>$(MSBuildLastTaskResult)</_CmakeStatus>
</PropertyGroup>
<ReadLinesFromFile
Condition=" '$(_CmakeStatus)' == 'false' "
File="$(CmakeBuildDir)CMakeFiles/CMakeOutput.log">
<Output TaskParameter="Lines" ItemName="_CmakeLog" />
</ReadLinesFromFile>
<Message
Condition=" '$(_CmakeStatus)' == 'false' "
Text="CMakeOutput.log"
/>
<Message
Condition=" '$(_CmakeStatus)' == 'false' "
Text="@(_CmakeLog, '
')"
/>
<ReadLinesFromFile
Condition=" '$(_CmakeStatus)' == 'false' "
File="$(CmakeBuildDir)CMakeFiles/CMakeError.log">
<Output TaskParameter="Lines" ItemName="_CmakeErrorLog" />
</ReadLinesFromFile>
<Message
Condition=" '$(_CmakeStatus)' == 'false' "
Text="CMakeError.log"
/>
<Message
Condition=" '$(_CmakeStatus)' == 'false' "
Text="@(_CmakeErrorLog, '
')"
/>
<Error
Condition=" '$(_CmakeStatus)' == 'false' "
Text="`cmake` failed. See previous messages."
/>
</Target>
</Project>
12 changes: 12 additions & 0 deletions src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ public JreRuntime CreateJreVM ()

public class JreRuntime : JniRuntime
{
static JreRuntime ()
{
if (Environment.OSVersion.Platform == PlatformID.Win32NT) {
var baseDir = Path.GetDirectoryName (typeof (JreRuntime).Assembly.Location);
var newDir = Path.Combine (baseDir, Environment.Is64BitProcess ? "win-x64" : "win-x86");
NativeMethods.AddDllDirectory (newDir);
}
}

static int CreateJavaVM (out IntPtr javavm, out IntPtr jnienv, ref JavaVMInitArgs args)
{
return NativeMethods.java_interop_jvm_create (out javavm, out jnienv, ref args);
Expand Down Expand Up @@ -168,6 +177,9 @@ partial class NativeMethods {

[DllImport (JavaInteropLib, CharSet=CharSet.Ansi, CallingConvention=CallingConvention.Cdecl)]
internal static extern int java_interop_jvm_create (out IntPtr javavm, out IntPtr jnienv, ref JavaVMInitArgs args);

[DllImport ("kernel32", CharSet=CharSet.Unicode)]
internal static extern int AddDllDirectory (string NewDirectory);
}
}

60 changes: 60 additions & 0 deletions src/java-interop/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
set(CMAKE_OSX_ARCHITECTURES x86_64 arm64)

project(
java-interop
DESCRIPTION "Java.Interop native support"
HOMEPAGE_URL "https://github.com/xamarin/java.interop/"
LANGUAGES CXX C
)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_VISIBILITY_PRESET hidden)

jonpryor marked this conversation as resolved.
Show resolved Hide resolved
option(ENABLE_MONO_INTEGRATION "Require Mono runtime" OFF)

cmake_minimum_required(VERSION 3.10.2)

set(JAVA_INTEROP_CORE_SOURCES
java-interop-dlfcn.cc
java-interop-jvm.cc
java-interop-logger.cc
java-interop-util.cc
java-interop.cc
${JNI_C_PATH}
)
set(JAVA_INTEROP_MONO_SOURCES
java-interop-gc-bridge-mono.cc
java-interop-mono.cc
)

add_compile_definitions("JAVA_INTEROP_DLL_EXPORT")
add_compile_definitions("JI_DLL_EXPORT")

foreach(dir in ${JDK_INCLUDE_LIST})
include_directories(${dir})
endforeach()

set(LINK_FLAGS "")

if(ENABLE_MONO_INTEGRATION)
foreach(dir in ${MONO_INCLUDE_LIST})
include_directories(${dir})
endforeach()
list(APPEND LINK_FLAGS ${MONO_LINK_FLAGS})
list(APPEND LINK_FLAGS "-Wl,-undefined -Wl,suppress -Wl,-flat_namespace")
set(JAVA_INTEROP_SOURCES ${JAVA_INTEROP_CORE_SOURCES} ${JAVA_INTEROP_MONO_SOURCES})
else()
set(JAVA_INTEROP_SOURCES ${JAVA_INTEROP_CORE_SOURCES})
endif()

add_library(
java-interop
SHARED
${JAVA_INTEROP_SOURCES}
)
target_link_libraries(
java-interop
${LINK_FLAGS}
)
101 changes: 101 additions & 0 deletions src/java-interop/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<Project>

<Import Project="..\..\build-tools\scripts\NativeToolchain.targets" />

<PropertyGroup>
<_JavaInteropLibName Condition=" '$(OS)' != 'Windows_NT' And Exists ('/Library/Frameworks/') ">libjava-interop.dylib</_JavaInteropLibName>
<_JavaInteropLibName Condition=" '$(OS)' != 'Windows_NT' And !Exists ('/Library/Frameworks/') ">libjava-interop.so</_JavaInteropLibName>
<_JavaInteropLibName Condition=" '$(OS)' == 'Windows_NT' ">java-interop.dll</_JavaInteropLibName>
Copy link
Member

Choose a reason for hiding this comment

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

We have a lot of existing code using $(OS), but I think the proper check is to use these:

$([MSBuild]::IsOSPlatform ('osx'))
$([MSBuild]::IsOSPlatform ('windows'))
$([MSBuild]::IsOSPlatform ('linux'))

I've seen DevCom issues where someone had %OS% set to Windows on their Dell PC!

</PropertyGroup>

<ItemGroup Condition=" '$(OS)' == 'Windows_NT' ">
<_JavaInteropNativeLib Include="CMakeLists.txt">
<Arch>x86_amd64</Arch>
<Dir>win-x64\</Dir>
</_JavaInteropNativeLib>
<_JavaInteropNativeLib Include="CMakeLists.txt">
<Arch>x86</Arch>
<Dir>win-x86\</Dir>
</_JavaInteropNativeLib>
</ItemGroup>

<ItemGroup Condition=" '$(OS)' != 'Windows_NT' ">
<_JavaInteropNativeLib Include="CMakeLists.txt" />
</ItemGroup>

<ItemGroup>
<None Include="@(_JavaInteropNativeLib->'$(OutputPath)%(Dir)$(_JavaInteropLibName)')">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
<Link>%(Dir)$(_JavaInteropLibName)</Link>
</None>
</ItemGroup>

<ItemGroup>
<ClInclude Include="*.h" />
</ItemGroup>

<ItemGroup>
<ClCompile Include="$(IntermediateOutputPath)jni.c" />
<ClCompile Include="*.cc" />
</ItemGroup>

<Target Name="_BuildJni_c"
Inputs="$(_JNIEnvGenPath)"
Outputs="$(IntermediateOutputPath)jni.c">
<MakeDir Directories="$(OutputPath)" />
<Exec Command="$(_RunJNIEnvGen) $(IntermediateOutputPath)jni.g.cs $(IntermediateOutputPath)jni.c" />
</Target>

<Target Name="_GetCmakeOptions">
<PropertyGroup Condition=" '$(TargetFramework)' == 'net472' And '@(MonoIncludePath->Count())' != '0' ">
<_MonoDirs>"-DMONO_INCLUDE_LIST=@(MonoIncludePath, ';')"</_MonoDirs>
<_MonoLib>"-DMONO_LINK_FLAGS=$(MonoLibs)"</_MonoLib>
<_EnableMono>-DENABLE_MONO_INTEGRATION=ON</_EnableMono>
</PropertyGroup>
<PropertyGroup>
<_JdkDirs>"-DJDK_INCLUDE_LIST=@(JdkIncludePath, ';')"</_JdkDirs>
<_Jni_c>"-DJNI_C_PATH=$(MSBuildThisFileDirectory)$(IntermediateOutputPath)jni.c"</_Jni_c>
<_ExtraArgs>$([MSBuild]::Escape('$(_JdkDirs) $(_Jni_c) $(_EnableMono) $(_MonoDirs) $(_MonoLib)'))</_ExtraArgs>
</PropertyGroup>
</Target>

<Target Name="_BuildLibs"
DependsOnTargets="GetNativeBuildCommands;_BuildJni_c;_GetCmakeOptions"
BeforeTargets="Build"
Inputs="@(_JavaInteropNativeLib);$(MSBuildThisFileFullPath);java-interop.csproj;@(ClInclude);$(ClCompile)"
Outputs="$(OutputPath)%(_JavaInteropNativeLib.Dir)$(_JavaInteropLibName)">
<Message Text="# jonp: _Test: Building: @(_JavaInteropNativeLib->'%(Identity) for %(Arch)', ' ')" />
<MakeDir Directories="$(IntermediateOutputPath)%(_JavaInteropNativeLib.Dir)" />
<ItemGroup>
<_Cmake
Condition=" '$(PrepareNativeToolchain)' != '' "
Include="PrepareNativeToolchain=$(PrepareNativeToolchain) %(_JavaInteropNativeLib.Arch)"
/>
<_Cmake Include="CmakePath=$(CmakePath)" />
<_Cmake Include="CmakeGenerator=$(CmakeGenerator)" />
<_Cmake Include="CmakeSourceDir=$(MSBuildThisFileDirectory)" />
<_Cmake Include="CmakeBuildDir=$(MSBuildThisFileDirectory)$(IntermediateOutputPath)%(_JavaInteropNativeLib.Dir)" />
<_Cmake Include="CmakeExtraArgs=$(_ExtraArgs)" />
</ItemGroup>
<MSBuild
Projects="..\..\build-tools\scripts\RunCmake.proj"
Properties="@(_Cmake)"
Targets="Cmake"
/>
<MakeDir Directories="$(OutputPath)%(_JavaInteropNativeLib.Dir)" />
<ItemGroup>
<_Libs Include="$(IntermediateOutputPath)%(_JavaInteropNativeLib.Dir)$(_JavaInteropLibName)*" />
</ItemGroup>
<Copy
SourceFiles="@(_Libs)"
DestinationFolder="$(OutputPath)%(_JavaInteropNativeLib.Dir)"
/>
<Touch Files="$(OutputPath)%(_JavaInteropNativeLib.Dir)$(_JavaInteropLibName)" />
</Target>

<Target Name="_Clean"
AfterTargets="Clean">
<Delete Files="@(None)" />
</Target>

</Project>
Loading