Skip to content

[Xamarin.Android.Build.Tasks] Move MonoAndroidAssetsDirIntermediate #8166

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

Merged
merged 1 commit into from
Jul 6, 2023
Merged

[Xamarin.Android.Build.Tasks] Move MonoAndroidAssetsDirIntermediate #8166

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 8 additions & 8 deletions src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,12 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<AndroidPackagingOptionsExclude Include="$([MSBuild]::Escape('*.kotlin_*'))" />
</ItemGroup>

<!-- Assets build properties -->
<PropertyGroup>
<MonoAndroidAssetsDirIntermediate>$(IntermediateOutputPath)assets\</MonoAndroidAssetsDirIntermediate>
<MonoAndroidAssetsPrefix Condition="'$(MonoAndroidAssetsPrefix)' == ''">Assets</MonoAndroidAssetsPrefix>
</PropertyGroup>

<!--
*******************************************
Imports
Expand Down Expand Up @@ -439,7 +445,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
Outputs="$(MonoAndroidAssetsDirIntermediate)xa-internal/xa-mam-mapping.xml">
<MakeDir
Condition=" '@(_AndroidMamMappingFile->Count())' != '0' "
Directories="$(MonoAndroidAssetsDirIntermediate)/xa-internal"
Directories="$(MonoAndroidAssetsDirIntermediate)xa-internal"
/>
Comment on lines 446 to 449
Copy link
Member

@jonathanpeppers jonathanpeppers Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Wait, does this really fix it? MSBuild does things in passes, a simplification/example of the ordering:

  1. Imports & Properties
  2. Item Groups
  3. Targets

Would $(MonoAndroidAssetsDirIntermediate) have always have been set in pass 1, and these targets are pass 3?

Copy link
Contributor Author

@jonpryor jonpryor Jul 5, 2023

Choose a reason for hiding this comment

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

While $(MonoAndroidAssetsDirIntermediate) would always be set in pass 1, there is still an ordering issue between properties, because properties are set in file order:

  1. $(_XARemapMembersFilePath) was set first: https://github.com/xamarin/xamarin-android/blob/69b4ab056badcb5d25a5faf39d08baeb1b8b7010/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L458
  2. $(MonoAndroidAssetsDirIntermediate) was set afterward: https://github.com/xamarin/xamarin-android/blob/69b4ab056badcb5d25a5faf39d08baeb1b8b7010/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L885

When (1) is evaluated, $(MonoAndroidAssetsDirIntermediate) hasn't been set yet, thus $(_XARemapMembersFilePath) will be xa-internal/xa-remap-members.xml (which has the nice property of matching the bug report).

Indeed, when viewing the diagnostic build log, we see:

_XARemapMembersFilePath = xa-internal/xa-remap-members.xml

By moving $(MonoAndroidAssetsDirIntermediate) to be defined before $(_XARemapMembersFilePath) is defined, we ensure that $(MonoAndroidAssetsDirIntermediate) doesn't expand to the empty string, because it will have been set by the time it is used.

<MamJsonToXml
MappingFiles="@(_AndroidMamMappingFile)"
Expand All @@ -464,7 +470,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
Outputs="$(_XARemapMembersFilePath)">
<MakeDir
Condition=" '@(_AndroidRemapMembers->Count())' != '0' "
Directories="$(MonoAndroidAssetsDirIntermediate)/xa-internal"
Directories="$(MonoAndroidAssetsDirIntermediate)xa-internal"
/>
<MergeRemapXml
InputRemapXmlFiles="@(_AndroidRemapMembers)"
Expand Down Expand Up @@ -880,12 +886,6 @@ because xbuild doesn't support framework reference assemblies.
/>
</Target>

<!-- Assets build properties -->
<PropertyGroup>
<MonoAndroidAssetsDirIntermediate>$(IntermediateOutputPath)assets\</MonoAndroidAssetsDirIntermediate>
<MonoAndroidAssetsPrefix Condition="'$(MonoAndroidAssetsPrefix)' == ''">Assets</MonoAndroidAssetsPrefix>
</PropertyGroup>

<!-- Assets Build -->

<Target Name="UpdateAndroidAssets"
Expand Down