Skip to content

Fix monodroid SDK detection that depended on non-existent file. #49

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
Jun 3, 2016
Merged

Fix monodroid SDK detection that depended on non-existent file. #49

Show file tree
Hide file tree
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
15 changes: 15 additions & 0 deletions Xamarin.Android.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Mono.Posix", "src\Mono.Posi
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.EnterpriseServices", "src\System.EnterpriseServices\System.EnterpriseServices.csproj", "{2868FC32-A4E7-4008-87C8-2C7879CACB58}"
EndProject
Project("{9344BDBB-3E7F-41FC-A0DD-8665D75EE146}") = "unix-distribution-setup", "build-tools\unix-distribution-setup\unix-distribution-setup.mdproj", "{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}"
Copy link
Contributor

@jonpryor jonpryor May 31, 2016

Choose a reason for hiding this comment

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

Invalid syntax here. The syntax is:

Project(...)
EndProject

You have:

Project(...)
Project(...)
EndProject

You need to place an EndProject before this line.

Copy link
Contributor Author

@atsushieno atsushieno Jun 1, 2016

Choose a reason for hiding this comment

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

oops, I removed it during the merge with master...!

EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|AnyCPU = Debug|AnyCPU
Expand Down Expand Up @@ -280,6 +282,18 @@ Global
{2868FC32-A4E7-4008-87C8-2C7879CACB58}.XAIntegrationDebug|AnyCPU.Build.0 = Debug|Any CPU
{2868FC32-A4E7-4008-87C8-2C7879CACB58}.XAIntegrationRelease|AnyCPU.ActiveCfg = Debug|Any CPU
{2868FC32-A4E7-4008-87C8-2C7879CACB58}.XAIntegrationRelease|AnyCPU.Build.0 = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.Debug|AnyCPU.ActiveCfg = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.Debug|AnyCPU.Build.0 = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.Release|AnyCPU.ActiveCfg = Release|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.Release|AnyCPU.Build.0 = Release|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationDebug|Any CPU.ActiveCfg = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationDebug|Any CPU.Build.0 = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationRelease|Any CPU.ActiveCfg = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationRelease|Any CPU.Build.0 = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationDebug|AnyCPU.ActiveCfg = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationDebug|AnyCPU.Build.0 = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationRelease|AnyCPU.ActiveCfg = Debug|Any CPU
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}.XAIntegrationRelease|AnyCPU.Build.0 = Debug|Any CPU
EndGlobalSection
GlobalSection(NestedProjects) = preSolution
{8FF78EB6-6FC8-46A7-8A15-EBBA9045C5FA} = {E351F97D-EA4F-4E7F-AAA0-8EBB1F2A4A62}
Expand Down Expand Up @@ -311,6 +325,7 @@ Global
{40EAD437-216B-4DF4-8258-3F47E1672C3A} = {CAB438D8-B0F5-4AF0-BEBD-9E2ADBD7B483}
{83F00D30-0AC6-40D8-834B-DD39B6CAA8B3} = {04E3E11E-B47D-4599-8AFC-50515A95E715}
{2868FC32-A4E7-4008-87C8-2C7879CACB58} = {04E3E11E-B47D-4599-8AFC-50515A95E715}
{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A} = {E351F97D-EA4F-4E7F-AAA0-8EBB1F2A4A62}
EndGlobalSection
GlobalSection(MonoDevelopProperties) = preSolution
Policies = $0
Expand Down
29 changes: 29 additions & 0 deletions build-tools/unix-distribution-setup/unix-distribution-setup.mdproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<Project DefaultTargets="Build" ToolsVersion="4.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup>
<Configuration Condition="'$(Configuration)' == ''">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<ItemType>GenericProject</ItemType>
<ProjectGuid>{2CF172E5-BDAE-4ABA-8BC8-08040ED3E77A}</ProjectGuid>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)'=='Debug'">
<OutputPath>..\..\bin\Debug\</OutputPath>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)'=='Release'">
<OutputPath>..\..\bin\Release\</OutputPath>
</PropertyGroup>

<Import Project="..\..\Configuration.props" />

<Target Name="Build">
<Copy SourceFiles="..\..\tools\scripts\generator" DestinationFiles="$(OutputPath)bin\generator" />
Copy link
Contributor

@jonpryor jonpryor May 27, 2016

Choose a reason for hiding this comment

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

This should likely also <Exec/> to a chmod +x $(OutputPath)bin\generator, in case tools/scripts/generator has the wrong +x permissions.

Copy link
Contributor Author

@atsushieno atsushieno May 30, 2016

Choose a reason for hiding this comment

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

Shouldn't git preserve executable bits? We cannot be super responsible to users messing up local checkouts by using bad clients.

Copy link
Contributor

@jonpryor jonpryor May 31, 2016

Choose a reason for hiding this comment

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

git should, yes, but this might not just be a matter of "using bad clients".

An alternate scenario: Dropbox-based git checkout, originally checked out on Windows (i.e. no +x support). If a Linux/OSX machine is attached to the same Dropbox account, will those file permissions be preserved, or will they have been "lost" because Dropbox cares not about git, it just synchronizes files, and Windows was the first one to synchronize, so that information is lost...

Yes, we could say that's not a support scenario, but (1) that's a really convenient scenario (easy source sharing), and (2) it's not that hard to support by having an explicit chmod +x after copying the file.

Copy link
Contributor Author

@atsushieno atsushieno May 31, 2016

Choose a reason for hiding this comment

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

BTW chmod is already there ;-)

Copy link
Contributor

@jonpryor jonpryor May 31, 2016

Choose a reason for hiding this comment

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

Sorry, I missed it.

<Exec
Condition=" '$(HostOS)' != 'Windows' "
Command="chmod +x $(OutputPath)bin\generator" />
</Target>

<Target Name="Clean">
<Delete Files="$(OutputPath)bin\generator" />
</Target>
</Project>

26 changes: 19 additions & 7 deletions src/Xamarin.Android.Build.Utilities/Sdks/MonoDroidSdkBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ namespace Xamarin.Android.Build.Utilities
abstract class MonoDroidSdkBase
{
protected readonly static string DebugRuntime = "Mono.Android.DebugRuntime-debug.apk";
protected readonly static string GeneratorExe = "generator.exe";
protected readonly static string GeneratorScript = "generator";

// I can never remember the difference between SdkPath and anything else...
[Obsolete ("Do not use.")]
Expand All @@ -31,16 +33,18 @@ abstract class MonoDroidSdkBase

public int SharedRuntimeVersion { get; private set; }

// runtimePath: contains Mono.Android.DebugRuntime-*.apk
// expectedRuntimePath: contains Mono.Android.DebugRuntime-*.apk
// binPath: contains mandroid
// mscorlibDir: contains mscorlib.dll
public void Initialize (string runtimePath = null, string binPath = null, string bclPath = null)
public void Initialize (string expectedRuntimePath = null, string binPath = null, string bclPath = null)
Copy link
Contributor

@jonpryor jonpryor May 27, 2016

Choose a reason for hiding this comment

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

Why rename this parameter? If it is a good idea, then the comment ~3 lines above should also be corrected.

Copy link
Contributor Author

@atsushieno atsushieno May 30, 2016

Choose a reason for hiding this comment

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

It is not just renaming, it is preserving the actual value that was passed to the method to report later as a warning.

{
runtimePath = GetValidPath ("MonoAndroidToolsPath", runtimePath, ValidateRuntime, () => FindRuntime ());
var runtimePath = GetValidPath ("MonoAndroidToolsPath", expectedRuntimePath, ValidateRuntime, () => FindRuntime ());
if (runtimePath != null) {
binPath = GetValidPath ("MonoAndroidBinPath", binPath, ValidateBin, () => FindBin (runtimePath));
bclPath = GetValidPath ("mscorlib.dll", bclPath, ValidateFramework, () => FindFramework (runtimePath));
} else {
if (expectedRuntimePath != null)
AndroidLogger.LogWarning (null, "Runtime was not found at {0}", expectedRuntimePath);
binPath = bclPath = null;
}

Expand All @@ -67,14 +71,21 @@ public void Initialize (string runtimePath = null, string binPath = null, string

static string GetValidPath (string description, string path, Func<string, bool> validator, Func<string> defaultPath)
{
if (!string.IsNullOrEmpty (path) && Directory.Exists (path)) {
if (validator (path))
return path;
AndroidLogger.LogWarning ("{0} path {1} is not valid; skipping.", description, path);
if (!string.IsNullOrEmpty (path)) {
if (Directory.Exists (path)) {
if (validator (path))
return path;
AndroidLogger.LogWarning (null, "{0} path '{1}' is explicitly specified, but it was not valid; skipping.", description, path);
} else
AndroidLogger.LogWarning (null, "{0} path '{1}' is explicitly specified, but it was not found; skipping.", description, path);
}
path = defaultPath ();
if (path != null && validator (path))
return path;
if (path != null)
AndroidLogger.LogWarning (null, "{0} path is defaulted to '{1}', but it was not valid; skipping", description, path);
else
AndroidLogger.LogWarning (null, "{0} path is not found and no default location is provided; skipping", description);
return null;
}

Expand All @@ -96,6 +107,7 @@ protected static bool ValidateRuntime (string loc)
{
return !string.IsNullOrWhiteSpace (loc) &&
(File.Exists (Path.Combine (loc, DebugRuntime)) || // Normal/expected
File.Exists (Path.Combine (loc, GeneratorExe)) || // Normal/expected
File.Exists (Path.Combine (loc, "Ionic.Zip.dll"))); // Wrench builds
}

Expand Down
13 changes: 8 additions & 5 deletions src/Xamarin.Android.Build.Utilities/Sdks/MonoDroidSdkUnix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ protected override string FindRuntime ()
string monoAndroidPath = Environment.GetEnvironmentVariable ("MONO_ANDROID_PATH");
if (!string.IsNullOrEmpty (monoAndroidPath)) {
string libMandroid = Path.Combine (monoAndroidPath, "lib", "mandroid");
if (Directory.Exists (libMandroid) && ValidateRuntime (libMandroid))
return libMandroid;
if (Directory.Exists (libMandroid)) {
if (ValidateRuntime (libMandroid))
return libMandroid;
AndroidLogger.LogInfo (null, "MONO_ANDROID_PATH points to {0}, but it is invalid.", monoAndroidPath);
} else
AndroidLogger.LogInfo (null, "MONO_ANDROID_PATH points to {0}, but it does not exist.", monoAndroidPath);
}

// check also in the users folder
Expand All @@ -38,7 +42,7 @@ protected override string FindRuntime ()
protected override bool ValidateBin (string binPath)
{
return !string.IsNullOrWhiteSpace (binPath) &&
File.Exists (Path.Combine (binPath, "generator"));
File.Exists (Path.Combine (binPath, GeneratorScript));
}

protected override string FindFramework (string runtimePath)
Expand All @@ -64,8 +68,7 @@ protected override string FindFramework (string runtimePath)
protected override string FindBin (string runtimePath)
{
string binPath = Path.GetFullPath (Path.Combine (runtimePath, "..", "..", "bin"));
Console.WriteLine (binPath);
if (File.Exists (Path.Combine (binPath, "generator")))
if (File.Exists (Path.Combine (binPath, GeneratorScript)))
return binPath;
return null;
}
Expand Down
6 changes: 6 additions & 0 deletions tools/scripts/generator
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#!/bin/sh
BINDIR=`dirname "$0"`
MANDROID_DIR="$BINDIR/../lib/mandroid"

unset MONO_PATH
exec mono $MONO_OPTIONS "$MANDROID_DIR/generator.exe" "$@"