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

Bundled multidex jar #124

Merged
merged 3 commits into from
Aug 9, 2016
Merged

Bundled multidex jar #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
18444e4
[msbuild] switch to bundled multidex jar
atsushieno Jul 21, 2016
c947524
Copy android-support-multidex.jar into mandroid directory.
atsushieno Jul 21, 2016
fb44adb
Copy support library license file to lib/mandroid as MULTIDEX_JAR_LIC…
atsushieno Jul 22, 2016
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: 4 additions & 0 deletions build-tools/android-toolchain/android-toolchain.projitems
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@
<HostOS></HostOS>
<DestDir>platforms\android-24</DestDir>
</AndroidSdkItem>
<AndroidSdkItem Include="android_m2repository_r16.zip">
<HostOS></HostOS>
<DestDir>extras\android\m2repository</DestDir>
</AndroidSdkItem>
</ItemGroup>
<ItemGroup>
<_NdkToolchain Include="arm-linux-androideabi-clang" Condition="$(AndroidSupportedTargetJitAbisForConditionalChecks.Contains(':armeabi:')) Or $(AndroidSupportedTargetJitAbisForConditionalChecks.Contains(':armeabi-v7a:'))">
Expand Down
24 changes: 24 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
<?xml version="1.0" encoding="UTF-8" ?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<UsingTask TaskName="Xamarin.Android.Tools.BootstrapTasks.GenerateProfile" AssemblyFile="..\..\bin\Build$(Configuration)\Xamarin.Android.Tools.BootstrapTasks.dll" />
<UsingTask TaskName="Xamarin.Android.Tools.BootstrapTasks.UnzipDirectoryChildren" AssemblyFile="..\..\bin\Build$(Configuration)\Xamarin.Android.Tools.BootstrapTasks.dll" />
<PropertyGroup>
<_SharedRuntimeBuildPath Condition=" '$(_SharedRuntimeBuildPath)' == '' ">..\..\bin\$(Configuration)\lib\xbuild-frameworks\MonoAndroid\</_SharedRuntimeBuildPath>
<_GeneratedProfileClass>$(IntermediateOutputPath)Profile.g.cs</_GeneratedProfileClass>
<BuildDependsOn>
$(BuildDependsOn);
_CopyExtractedMultiDexJar;
</BuildDependsOn>
<_AndroidSdkLocation>$(ANDROID_SDK_PATH)</_AndroidSdkLocation>
Copy link
Member

@jonpryor jonpryor Aug 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

This should probably be the $(AndroidSdkFullPath) MSBuild property. Nothing is going to export an $(ANDROID_SDK_PATH) environment variable.

The $(AndroidSdkFullPath) property is brought in through the importing of Configuration.props in Xamarin.Android.Build.Tasks.csproj.

Copy link
Member

Choose a reason for hiding this comment

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

Unrelatedly: Please fix your editor to expand tabs into spaces on .targets files and other XML files, with 2 space indents.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. monodroid does export that environment variable. Want me to eliminate the variables that are used ONLY in monodroid in the same manner? For example, InstantRunEnabled.

Copy link
Member

Choose a reason for hiding this comment

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

What should happen for "pure" xamarin-android builds? $(ANDROID_SDK_PATH) won't be set, so e.g. $(_AndroidSdkLocation)\$(_MultiDexAarInAndroidSdk) will be /extras/android/..., which won't be readable or writable, and might result in a xamarin-android build error.

That seems less than desirable.

Perhaps this should use a fallback?

<_AndroidSdkLocation Condition=" '$(AndroidSdkFullPath)' != '' ">$(AndroidSdkFullPath)</_AndroidSdkLocation>
<_AndroidSdkLocation Condition=" '$(_AndroidSdkLocation)' == '' ">$(ANDROID_SDK_PATH)</_AndroidSdkLocation>

This should allow xamarin-android to build while allowing the monodroid build to override.

Copy link
Member

Choose a reason for hiding this comment

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

Want me to eliminate the variables that are used ONLY in monodroid in the same manner? For example, InstantRunEnabled.

If they can be removed, that would likely be best. If they can't be removed -- which may be the case for $(_InstantRunEnabled), which is used with the <DetermineJavaLibrariesToCompile/> task -- then perhaps it should be renamed to be something more meaningful w/o context/memory/history.

For example, instead of DetermineJavaLibrariesToCompile.EnableInstantRun, rename it to DetermineJavaLibrariesToCompile.SkipMonoPlatformJar. For those just reading Xamarin.Android.Common.targets, I assume that seeing SkipMonoPlatformJar="$(_SkipMonoPlatformJar)" would be more meaningful than EnableInstantRun="$(_InstantRunEnabled)".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pure xamarin-android configuration for _AndroidSdkLocation is defined immediately below.

<_AndroidSdkLocation Condition="'$(_AndroidSdkLocation)'==''">$(AndroidToolchainDirectory)\sdk</_AndroidSdkLocation>
<_MultiDexAarInAndroidSdk>extras\android\m2repository\com\android\support\multidex\1.0.1\multidex-1.0.1.aar</_MultiDexAarInAndroidSdk>
<_SupportLicenseInAndroidSdk>extras\android\m2repository\NOTICE.txt</_SupportLicenseInAndroidSdk>
</PropertyGroup>
<ItemGroup>
<_SharedRuntimeAssemblies Include="$(_SharedRuntimeBuildPath)v1.0\*.dll;$(_SharedRuntimeBuildPath)$(AndroidFrameworkVersion)\*.dll"/>
Expand All @@ -19,4 +28,19 @@
Lines="$(_GeneratedProfileClass)"
Overwrite="false"/>
</Target>

<Target Name="_CopyExtractedMultiDexJar"
Inputs="$(_AndroidSdkLocation)\$(_MultiDexAarInAndroidSdk);$(_AndroidSdkLocation)\$(_SupportLicenseInAndroidSdk)"
Outputs="$(OutputPath)..\..\..\mandroid\android-support-multidex.jar;$(OutputPath)..\..\..\mandroid\MULTIDEX_JAR_LICENSE">
Copy link
Member

Choose a reason for hiding this comment

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

Should MULTIDEX_JAR_LICENSE have a .txt extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Too cosmetic to change.

<UnzipDirectoryChildren
NoSubdirectory="true"
SourceFiles="$(_AndroidSdkLocation)\$(_MultiDexAarInAndroidSdk)"
DestinationFolder="$(IntermediateOutputPath)multidex-aar" />
<Copy
SourceFiles="$(IntermediateOutputPath)multidex-aar\classes.jar"
DestinationFiles="$(OutputPath)..\..\..\mandroid\android-support-multidex.jar" />
<Copy
SourceFiles="$(_AndroidSdkLocation)\$(_SupportLicenseInAndroidSdk)"
DestinationFiles="$(OutputPath)..\..\..\mandroid\MULTIDEX_JAR_LICENSE" />
</Target>
</Project>
9 changes: 7 additions & 2 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 @@ -195,7 +195,8 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
<DeployExternal Condition="'$(DeployExternal)' == ''">False</DeployExternal>
<UseShortFileNames Condition="'$(UseShortFileNames)' != 'True'">False</UseShortFileNames>

<AndroidMultiDexSupportJar>extras\android\support\multidex\library\libs\android-support-multidex.jar</AndroidMultiDexSupportJar>
<!-- Obsolete build property: should be removed in the future releases -->
<AndroidMultiDexSupportJar></AndroidMultiDexSupportJar>

<AndroidSupportedAbis Condition=" '$(AndroidSupportedAbis)' == '' ">armeabi-v7a</AndroidSupportedAbis>

Expand Down Expand Up @@ -1072,7 +1073,11 @@ because xbuild doesn't support framework reference assemblies.

<Target Name="_AddMultiDexDependencyJars">
<CreateItem Include="$(AndroidSdkDirectory)\$(AndroidMultiDexSupportJar)"
Condition="'$(AndroidEnableMultiDex)' == 'True'">
Condition="'$(AndroidEnableMultiDex)' == 'True' AND '$(AndroidMultiDexSupportJar)' != ''">
<Output TaskParameter="Include" ItemName="AndroidJavaLibrary" />
</CreateItem>
<CreateItem Include="$(MonoAndroidToolsDirectory)\android-support-multidex.jar"
Condition="'$(AndroidEnableMultiDex)' == 'True' AND '$(AndroidMultiDexSupportJar)' == ''">
<Output TaskParameter="Include" ItemName="AndroidJavaLibrary" />
</CreateItem>
</Target>
Expand Down
6 changes: 5 additions & 1 deletion ...droid.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/UnzipDirectoryChildren.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public UnzipDirectoryChildren ()
[Required]
public ITaskItem DestinationFolder { get; set; }

public bool NoSubdirectory { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this property name. Let's try to find a better one...

The reason the task is UnzipDirectoryChildren and not Unzip is precisely because it "skips"/"ignores" the topmost extracted directory, hence the Directory.EnumerateDirectories(nestedTemp, "*").

In an ideal world, I think we'd want a separate task, but that's also ugly.

Perhaps IncludeRootDirectory would be clearer?

I'll I'm sure of is that NoSubdirectory doesn't seem clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change follows the "keep changes minimum" principle. If you make your suggested change, that affects other build targets. Such a semantic change should be done in separate PR.


public override bool Execute ()
{
Log.LogMessage (MessageImportance.Low, "Unzip:");
Expand Down Expand Up @@ -101,7 +103,9 @@ async TTask ExtractFile (string tempDir, string sourceFile, string relativeDestD
p.WaitForExit ();
}

foreach (var dir in Directory.EnumerateDirectories (nestedTemp, "*")) {
var dirs = NoSubdirectory ? new string [] { nestedTemp } : Directory.EnumerateDirectories (nestedTemp, "*");

foreach (var dir in dirs) {
foreach (var fse in Directory.EnumerateFileSystemEntries (dir)) {
var name = Path.GetFileName (fse);
var destDir = string.IsNullOrEmpty (relativeDestDir)
Expand Down