Commit c942ab6
authored
[java-source-utils] Build one
Context: dotnet/android#7157
Ever since commit 69e1b80, `java-source-utils.csproj` used the
`$(TargetFrameworks)` plural form, even though it only specified a
single framework. I don't remember underlying reason for this, other
than "that's what I needed for it to build," which might have been
because the `_BuildJava` target was "wrong".
This arrangement was "fine", until dotnet/android@2197a459,
after which the xamarin-android/main CI builds started behaving
"weird": frequently `make all-tests` would fail, because
`make jenkins` would produce an invalid or corrupt
`java-source-utils.jar`, *apparently* because it was attempting to
*concurrently build* `java-source-utils.csproj` (?!).
One of these concurrent builds *appeared* to be an "outer build" from
`Xamarin.Android.sln`, while another one appeared to be an "inner
build" via `apksigner.csproj` and
`%(ProjectReference.AdditionalProperties)`:
17:53:44.526 59:11>Target "_BuildJava: (TargetId:490)" in file "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.targets" from project "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.csproj" (entry point):
17:53:44.526 59:11>Building target "_BuildJava" completely.
Output file "/Users/runner/work/1/s/xamarin-android/bin/Release/lib/packs/Microsoft.Android.Sdk.Darwin/33.0.0/tools/java-source-utils.jar" does not exist.
…
17:54:19.564 59:12>Target "DispatchToInnerBuilds: (TargetId:1637)" in file "/Users/runner/work/1/s/xamarin-android/bin/Release/dotnet/sdk/7.0.100-preview.7.22354.2/Microsoft.Common.CrossTargeting.targets" from project "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.csproj" (target "Build" depends on it):
Task "MSBuild" (TaskId:1099)
Task Parameter:BuildInParallel=True (TaskId:1099)
Task Parameter:Targets=Build (TaskId:1099)
Task Parameter:
Projects=
java-source-utils.csproj
AdditionalProperties=TargetFramework=net7.0 (TaskId:1099)
Additional Properties for project "java-source-utils.csproj": (TaskId:1099)
TargetFramework=net7.0 (TaskId:1099)
…
17:54:19.592 59:12>Target "_BuildJava: (TargetId:1640)" in file "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.targets" from project "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/tools/java-source-utils/java-source-utils.csproj" (entry point):
Building target "_BuildJava" completely.
Output file "/Users/runner/work/1/s/xamarin-android/external/Java.Interop/../../bin/Release/lib/xamarin.android/xbuild/Xamarin/Android/java-source-utils.jar" does not exist.
…
17:54:41.034 59:11>Done building target "_BuildJava" in project "java-source-utils.csproj".: (TargetId:490)
We attempted to fix this by removing
`%(ProjectReference.AdditionalProperties)`, which only slightly
changed things: the "outer build via `Xamarin.Android.sln`" build was
removed, but we instead saw a scenario in which
`java-source-utils.csproj` was built "once", and as part of that
build the "outer" and "inner" builds were run concurrently.
A commonality here is `$(TargetFrameworks)` requires "outer" and
"inner" builds, and that is a complication that we should remove.
Update `java-source-utils.csproj` so that singular
`$(TargetFramework)` is used, not plural `$(TargetFrameworks)`.
Update the `_BuildJava` target so that it runs before the
`GetCopyToOutputDirectoryItems` target. This is consistent with how
the [`_BuildGradle` target in `apksigner.csproj`][0] works. Without
this change -- and the removal of the empty `Build` target -- the
`java-source-utils.csproj` build didn't behave correctly
(`gradlew` wasn't run).
Unfortunately, this seemingly simple change hits a little "snag":
`Directory.Build.props` is imported [very early][1]:
> *Directory.Build.props* is imported very early in
> *Microsoft.Common.props*, and properties defined later are
> unavailable to it.
"Properties defined later are unavailable to it." Properties such as
`$(TargetFramework)`. Which means that every property we have in
`Directory.Build.props` which "depends" on `$(TargetFramework)`
*are not **actually** usable*. They've only *appeared* to work
because they would default to "classic" paths, but as soon as you try
to build with `$(TargetFramework)`=net7.0 -- as was attempted with
`java-source-utils.csproj`, and previously with `Java.Base.csproj`
(bc5bcf4) -- you'll find that the "wrong" directories are used for
`$(OutputPath)`.
This has been a long-standing deficiency in my MSBuild understanding.
Fix this by adding a new `TargetFrameworkDependentValues.props` file,
and `<Import/>`ing this file in *every* `.csproj` which uses MSBuild
properties with values dependent upon `$(TargetFramework)` *before*
those properties are set.
Old and busted:
<PropertyGroup>
<TargetFramework>net7.0</TargetFramework>
<OutputPath>$(UtilityOutputFullPath)</OutputPath>
</PropertyGroup>
New hawtness:
<PropertyGroup>
<TargetFramework>net7.0</TargetFramework>
</PropertyGroup>
<Import Project="..\..\TargetFrameworkDependentValues.props" />
<PropertyGroup>
<OutputPath>$(UtilityOutputFullPath)</OutputPath>
</PropertyGroup>
[0]: https://github.com/xamarin/xamarin-android/blob/c537dd28c30f482f365ef756214be35aa1553da2/src/apksigner/apksigner.targets#L3-L13
[1]: https://docs.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2022#import-order$(TargetFramework) (#1007)1 parent b7caa78 commit c942ab6
File tree
50 files changed
+168
-62
lines changed- build-tools
- Java.Interop.BootstrapTasks
- jnienv-gen
- src
- Java.Base
- Java.Interop.Dynamic
- Java.Interop.Export
- Java.Interop.GenericMarshaler
- Java.Interop.Localization
- Java.Interop.Tools.Cecil
- Java.Interop.Tools.Diagnostics
- Java.Interop.Tools.Generator
- Java.Interop.Tools.JavaCallableWrappers
- Java.Interop.Tools.JavaSource
- Java.Interop.Tools.JavaTypeSystem
- Java.Interop
- Java.Runtime.Environment
- Xamarin.Android.Tools.AnnotationSupport.Cecil
- Xamarin.Android.Tools.AnnotationSupport
- Xamarin.Android.Tools.ApiXmlAdjuster
- Xamarin.Android.Tools.Bytecode
- Xamarin.SourceWriter
- java-interop
- tests
- Java.Base-Tests
- Java.Interop-PerformanceTests
- Java.Interop-Tests
- Java.Interop.Dynamic-Tests
- Java.Interop.Export-Tests
- Java.Interop.Tools.Generator-Tests
- Java.Interop.Tools.JavaCallableWrappers-Tests
- Java.Interop.Tools.JavaSource-Tests
- Java.Interop.Tools.JavaTypeSystem-Tests
- NativeTiming
- TestJVM
- Xamarin.Android.Tools.ApiXmlAdjuster-Tests
- Xamarin.Android.Tools.Bytecode-Tests
- Xamarin.SourceWriter-Tests
- generator-Tests
- invocation-overhead
- logcat-parse-Tests
- tools
- class-parse
- generator
- java-source-utils
- jcw-gen
- jnimarshalmethod-gen
- logcat-parse
- param-name-importer
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
50 files changed
+168
-62
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
| 64 | + | |
87 | 65 | | |
88 | 66 | | |
89 | 67 | | |
| |||
102 | 80 | | |
103 | 81 | | |
104 | 82 | | |
105 | | - | |
106 | | - | |
107 | | - | |
108 | | - | |
109 | | - | |
110 | | - | |
111 | | - | |
112 | 83 | | |
113 | 84 | | |
114 | 85 | | |
| |||
126 | 97 | | |
127 | 98 | | |
128 | 99 | | |
129 | | - | |
130 | | - | |
131 | | - | |
132 | | - | |
133 | | - | |
134 | | - | |
135 | | - | |
136 | | - | |
137 | | - | |
138 | | - | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | 100 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
Lines changed: 6 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
6 | 11 | | |
7 | 12 | | |
8 | 13 | | |
9 | 14 | | |
| 15 | + | |
10 | 16 | | |
11 | 17 | | |
12 | 18 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | 7 | | |
9 | 8 | | |
| 9 | + | |
| 10 | + | |
10 | 11 | | |
| 12 | + | |
11 | 13 | | |
12 | 14 | | |
13 | 15 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
4 | | - | |
| 4 | + | |
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
9 | 14 | | |
10 | 15 | | |
11 | 16 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
11 | | - | |
12 | 11 | | |
| 12 | + | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
| |||
Lines changed: 1 addition & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| |||
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
| 13 | + | |
| 14 | + | |
13 | 15 | | |
14 | 16 | | |
15 | 17 | | |
| |||
Lines changed: 3 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
| 12 | + | |
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
| 19 | + | |
18 | 20 | | |
19 | 21 | | |
20 | 22 | | |
| |||
0 commit comments