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

[java-interop] Fix include directory specification #47

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Jun 22, 2016

Commit b6431ac broke the build of src/java-interop:

Using task Exec from Microsoft.Build.Tasks.Exec, Microsoft.Build.Tasks.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
Executing: gcc -g -shared -std=c99 -o "obj/libjava-interop-m32.dylib" -m32 -DDEBUG -DJI_DLL_EXPORT -DMONODEVELOP -DMONO_DLL_EXPORT -fvisibility=hidden -Wl,-undefined -Wl,suppress -Wl,-flat_namespace -L /Library/Frameworks/Mono.framework/Libraries -lmonosgen-2.0   jni.c java-interop.c java-interop-mono.c java-interop-gc-bridge-mono.c
Environment variables being passed to the tool:
jni.c:7:10: fatal error: 'jni.h' file not found
#include <jni.h>
         ^
1 error generated.
In file included from java-interop-mono.c:1:
./java-interop-mono.h:30:11: fatal error: 'mono/metadata/assembly.h' file not found
        #include <mono/metadata/assembly.h>
                 ^
1 error generated.

The problem? There's no gcc -Ipath option, so the compiler doesn't
know where to find <jni.h> or <mono/metadata/assembly.h>.

The -I values are derived from the @(JdkIncludePath) and @(MonoIncludePath)` MSBuild item groups:

<!-- via bin/BuildDebug/JdkInfo.props -->
<PropertyGroup>
  <JdkJvmPath Condition=" '$(JdkJvmPath)' == '' ">/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/jre/lib/server/libjvm.dylib</JdkJvmPath>
</PropertyGroup>
<ItemGroup>
  <JdkIncludePath Condition=" '$(JdkJvmPath)' == '' " Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include" />
  <JdkIncludePath Condition=" '$(JdkJvmPath)' == '' " Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include/darwin" />
</ItemGroup>

<!-- via bin/BuildDebug/MonoInfo.props -->
<PropertyGroup>
  <MonoFrameworkPath Condition=" '$(MonoFrameworkPath)' == '' ">/Library/Frameworks/Mono.framework/Libraries/libmonosgen-2.0.1.dylib</MonoFrameworkPath>
</PropertyGroup>
<ItemGroup>
  <MonoIncludePath Condition=" '$(MonoFrameworkPath)' == '' " Include="/Library/Frameworks/Mono.framework/Headers/mono-2.0" />
</ItemGroup>

The intent to commit b6431ac is that if e.g. $(JdkJvmPath) is
overridden, then we shouldn't automatically generate the
@(JdkIncludePath) items, because they'll be for the "wrong" JDK.
If $(JdkJvmPath) is overridden, then @(JdkIncludePath) should be
as well. (Ditto $(MonoFrameworkPath) and @(MonoIncludePath).)

The problem is evaluation time:

During the evaluation phase of a build, imported files are
incorporated into the build in the order in which they appear.
Properties and items are defined in three passes in the following
order:

  • Properties are defined and modified in the order in which they appear.
  • Item definitions are defined and modified in the order in which they appear.
  • Items are defined and modified in the order in which they appear.

Properties are always evaluated before items, meaning when it came
time to evaluate //JdkIncludePath/@Condition, $(JdkJvmPath) was
always defined, and thus @(JdkIncludePath) was always empty.

Which is why there are no instances of gcc -I in the gcc compile
line, and why those headers can't be found.

The fix is to instead use the <Choose/> element to ensure that the
default items are specified when the "controlling" property isn't
overridden:

<!-- via bin/BuildDebug/JdkInfo.props -->
<Choose>
  <When Condition=" '$(JdkJvmPath)' == '' ">
    <PropertyGroup>
      <JdkJvmPath>/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/jre/lib/server/libjvm.dylib</JdkJvmPath>
    </PropertyGroup>
    <ItemGroup>
      <JdkIncludePath Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include" />
      <JdkIncludePath Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include/darwin" />
    </ItemGroup>
  </When>
</Choose>

<!-- via bin/BuildDebug/MonoInfo.props -->
<Choose>
  <When Condition=" '$(MonoFrameworkPath)' == '' ">
    <PropertyGroup>
      <MonoFrameworkPath>/Library/Frameworks/Mono.framework/Libraries/libmonosgen-2.0.1.dylib</MonoFrameworkPath>
      <MonoLibs         >-L /Library/Frameworks/Mono.framework/Libraries -lmonosgen-2.0</MonoLibs>
    </PropertyGroup>
    <ItemGroup>
      <MonoIncludePath Include="/Library/Frameworks/Mono.framework/Headers/mono-2.0" />
    </ItemGroup>
  </When>
</Choose>

This allows @(JdkIncludePath) and @(MonoIncludePath) to have
values when $(JdkJvmPath) and $(MonoFrameworkPath) aren't
overridden, which in turn allows gcc -I to be used, and allows
src/java-interop to build.

Commit b6431ac broke the build of `src/java-interop`:

	Using task Exec from Microsoft.Build.Tasks.Exec, Microsoft.Build.Tasks.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
	Executing: gcc -g -shared -std=c99 -o "obj/libjava-interop-m32.dylib" -m32 -DDEBUG -DJI_DLL_EXPORT -DMONODEVELOP -DMONO_DLL_EXPORT -fvisibility=hidden -Wl,-undefined -Wl,suppress -Wl,-flat_namespace -L /Library/Frameworks/Mono.framework/Libraries -lmonosgen-2.0   jni.c java-interop.c java-interop-mono.c java-interop-gc-bridge-mono.c
	Environment variables being passed to the tool:
	jni.c:7:10: fatal error: 'jni.h' file not found
	#include <jni.h>
	         ^
	1 error generated.
	In file included from java-interop-mono.c:1:
	./java-interop-mono.h:30:11: fatal error: 'mono/metadata/assembly.h' file not found
	        #include <mono/metadata/assembly.h>
	                 ^
	1 error generated.

The problem? There's no `gcc -Ipath` option, so the compiler doesn't
know where to find `<jni.h>` or `<mono/metadata/assembly.h>`.

The `-I` values are derived from the `@(JdkIncludePath) and
`@(MonoIncludePath)` MSBuild item groups:

	<!-- via bin/BuildDebug/JdkInfo.props -->
	<PropertyGroup>
	  <JdkJvmPath Condition=" '$(JdkJvmPath)' == '' ">/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/jre/lib/server/libjvm.dylib</JdkJvmPath>
	</PropertyGroup>
	<ItemGroup>
	  <JdkIncludePath Condition=" '$(JdkJvmPath)' == '' " Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include" />
	  <JdkIncludePath Condition=" '$(JdkJvmPath)' == '' " Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include/darwin" />
	</ItemGroup>

	<!-- via bin/BuildDebug/MonoInfo.props -->
	<PropertyGroup>
	  <MonoFrameworkPath Condition=" '$(MonoFrameworkPath)' == '' ">/Library/Frameworks/Mono.framework/Libraries/libmonosgen-2.0.1.dylib</MonoFrameworkPath>
	</PropertyGroup>
	<ItemGroup>
	  <MonoIncludePath Condition=" '$(MonoFrameworkPath)' == '' " Include="/Library/Frameworks/Mono.framework/Headers/mono-2.0" />
	</ItemGroup>

The *intent* to commit b6431ac is that if e.g. `$(JdkJvmPath)` is
overridden, then we shouldn't automatically generate the
`@(JdkIncludePath)` items, because they'll be for the "wrong" JDK.
If `$(JdkJvmPath)` is overridden, then `@(JdkIncludePath)` should be
as well. (Ditto `$(MonoFrameworkPath)` and `@(MonoIncludePath)`.)

The problem is [evaluation time][0]:

> During the evaluation phase of a build, imported files are
> incorporated into the build in the order in which they appear.
> Properties and items are defined in three passes in the following
> order:
>
> * Properties are defined and modified in the order in which they appear.
> * Item definitions are defined and modified in the order in which they appear.
> * Items are defined and modified in the order in which they appear.

Properties are *always* evaluated before items, meaning when it came
time to evaluate `//JdkIncludePath/@Condition`, `$(JdkJvmPath)` was
*always* defined, and thus `@(JdkIncludePath)` was *always* empty.

Which is why there are no instances of `gcc -I` in the `gcc` compile
line, and why those headers can't be found.

The fix is to instead use the `<Choose/>` element to ensure that the
default items are specified when the "controlling" property isn't
overridden:

	<!-- via bin/BuildDebug/JdkInfo.props -->
	<Choose>
	  <When Condition=" '$(JdkJvmPath)' == '' ">
	    <PropertyGroup>
	      <JdkJvmPath>/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/jre/lib/server/libjvm.dylib</JdkJvmPath>
	    </PropertyGroup>
	    <ItemGroup>
	      <JdkIncludePath Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include" />
	      <JdkIncludePath Include="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home/include/darwin" />
	    </ItemGroup>
	  </When>
	</Choose>

	<!-- via bin/BuildDebug/MonoInfo.props -->
	<Choose>
	  <When Condition=" '$(MonoFrameworkPath)' == '' ">
	    <PropertyGroup>
	      <MonoFrameworkPath>/Library/Frameworks/Mono.framework/Libraries/libmonosgen-2.0.1.dylib</MonoFrameworkPath>
	      <MonoLibs         >-L /Library/Frameworks/Mono.framework/Libraries -lmonosgen-2.0</MonoLibs>
	    </PropertyGroup>
	    <ItemGroup>
	      <MonoIncludePath Include="/Library/Frameworks/Mono.framework/Headers/mono-2.0" />
	    </ItemGroup>
	  </When>
	</Choose>

This allows `@(JdkIncludePath)` and `@(MonoIncludePath)` to have
values when `$(JdkJvmPath)` and `$(MonoFrameworkPath)` aren't
overridden, which in turn allows `gcc -I` to be used, and allows
`src/java-interop` to build.

[0]: https://msdn.microsoft.com/en-us/library/dd997067.aspx#Anchor_2
@radical radical merged commit 04f9e3e into dotnet:master Jun 23, 2016
radekdoulik added a commit to radekdoulik/java.interop that referenced this pull request Aug 27, 2018
Changes in xamarin-android-tools between 75530565b6aa903b3a0e52b61df4dd94475a19fc and 9e78d6ee586b498d0ea082b3bc00432c23583dd1:

9e78d6e (HEAD, origin/master, origin/HEAD, master) [tests] fix test failures on Windows (dotnet#47)
bdf0158 Better support no installed JDKs on macOS (dotnet#48)
6353659 Log what is happening during path selection (dotnet#46)
3ef860b Take BUILD_NUMBER into consideration for Version sorting (dotnet#45)
d3de054 Allow an optional locator to be provided to JdkInfo (dotnet#43)
917d3b3 Don't require quotes around `release` values (dotnet#41)
7427692 [tests] Unit tests for finding NDK location based on $PATH (dotnet#40)
dbc517b Merge pull request dotnet#38 from jonpryor/jonp-ndk-via-path
511d580 Allow finding NDK location based on $PATH
b42c217 [tests] Fix DetectAndSetPreferredJavaSdkPathToLatest() test (dotnet#37)
a4aad18 Add AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest() (dotnet#35)
fae7e0a [tests] Remove temporary directories (dotnet#36)
07c4c2b [Xamarin.Android.Tools.AndroidSdk] Revert JDK validation (dotnet#34)
radekdoulik added a commit that referenced this pull request Aug 27, 2018
Changes in xamarin-android-tools between 75530565b6aa903b3a0e52b61df4dd94475a19fc and 9e78d6ee586b498d0ea082b3bc00432c23583dd1:

9e78d6e (HEAD, origin/master, origin/HEAD, master) [tests] fix test failures on Windows (#47)
bdf0158 Better support no installed JDKs on macOS (#48)
6353659 Log what is happening during path selection (#46)
3ef860b Take BUILD_NUMBER into consideration for Version sorting (#45)
d3de054 Allow an optional locator to be provided to JdkInfo (#43)
917d3b3 Don't require quotes around `release` values (#41)
7427692 [tests] Unit tests for finding NDK location based on $PATH (#40)
dbc517b Merge pull request #38 from jonpryor/jonp-ndk-via-path
511d580 Allow finding NDK location based on $PATH
b42c217 [tests] Fix DetectAndSetPreferredJavaSdkPathToLatest() test (#37)
a4aad18 Add AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest() (#35)
fae7e0a [tests] Remove temporary directories (#36)
07c4c2b [Xamarin.Android.Tools.AndroidSdk] Revert JDK validation (#34)
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 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