[windows] now can build android-toolchain.mdproj #743
Conversation
|
@jonathanpeppers, |
|
@jonpryor is there a way I could skip an entire MSBuild project on Windows? This PR skips a couple broken targets on Windows in |
Plausibly, yes, but only for |
| @@ -135,4 +154,7 @@ | |||
| <HostOS></HostOS> | |||
| </AntItem> | |||
| </ItemGroup> | |||
| <ItemGroup> | |||
| <RequiredProgram Include="7z" Condition=" '$(HostOS)' == 'Windows' " /> | |||
There was a problem hiding this comment.
This should be in build-tools/dependencies/dependencies.projitems.
| @@ -16,7 +16,11 @@ public override bool Execute () | |||
| var licdir = Path.Combine (Path.Combine (AndroidSdkDirectory, "licenses")); | |||
| Directory.CreateDirectory (licdir); | |||
|
|
|||
| var psi = new ProcessStartInfo (Path.Combine (AndroidSdkDirectory, "tools", "bin", "sdkmanager"), "--licenses") { UseShellExecute = false, RedirectStandardInput = true }; | |||
| var path = Path.Combine (AndroidSdkDirectory, "tools", "bin", "sdkmanager"); | |||
There was a problem hiding this comment.
This should use Which.GetProgramLocation(), so that %PATHEXT% is used. This way, if sdkmanager becomes sdkmanager.cmd in the future, we'll do the sane thing.
jonathanpeppers
force-pushed
the
windows-android-toolchain
branch
from
28cbd1a to
85e231a
Compare
@jonathanpeppers this is probably down to you needing the |
|
@dellis1972 I didn't try an x64 directory, I'll try again to see if I can get it to work. I was trying something like this in |
Hmm, I feel like I should be able to get this to work, but I get Here is my directory after I added 64-bit dlls: I printed out |
|
|
@jonpryor I did mess something up on MacOS, I fixed in this PR: dotnet/android-libzipsharp#24 Is @grendello out this week? We can wait to merge this until after our call tomorrow if you like. |
jonathanpeppers
force-pushed
the
windows-android-toolchain
branch
3 times, most recently
from
aca8d93 to
a0873ba
Compare
|
@jonpryor I think this one is ready if you want to take another look. I think these test failures are something else. |
|
@jonathanpeppers: Those unit tests -- the AOT and mkbundle-related tests -- failed because -- for some reason -- the prebuilt Without a pre-existing bundle, the AOT compilers will only be built if:
Neither of which is true for this PR. If this PR had a mono bump, or had a bundle version bump (in If there was some bizarre network error, and the bundle Url was a 404 for the PR build, but otherwise exists, that would also make sense. Unfortunately, neither of the above is true: That is worrying. :-( The Ubuntu+xbuild and macOS+msbuild PR builds don't fail because they don't run the unit tests, thus the lack of AOT compilers won't be a problem. |
|
I don't understand why the hash value is changing. My current ~master build is using Scroll back to the Compare to my local files:
That's very confusing, because |
|
build |
|
I forgot about commit d891a86, which explains the change to |
| @@ -5,15 +5,15 @@ | |||
| <_AptGetInstall>apt-get -f -u install</_AptGetInstall> | |||
| </PropertyGroup> | |||
| <ItemGroup> | |||
| <RequiredProgram Include="$(HostCcName)" /> | |||
| <RequiredProgram Include="$(HostCxxName)" /> | |||
| <RequiredProgram Include="$(HostCcName)" Condition=" '$(HostOS)' != 'Windows' " /> | |||
There was a problem hiding this comment.
Let's talk about "beauty": I think code should be pretty.
"Pretty" is not well defined, but to me, it's this: things should be horizontally aligned whenever practical.
Specifically, these Condition attributes: note that every other Condition attribute in this file is horizontally aligned.
Would you mind fixing the indentation here and elsewhere in this file? :-)
jonathanpeppers
force-pushed
the
windows-android-toolchain
branch
from
a0873ba to
e9bc2a6
Compare
|
Hmm, this one downloaded the bundle and those AOT tests passed this time, but some others failed: The previous build failure with the missing bundle had these failing: |
Problems fixed on Windows: - Many `RequiredPrograms` are n/a on Windows - Setup `AndroidSdkItem` for Windows downloads - `AcceptAndroidSdkLicenses` should run `.bat` file on Windows - `UnzipDirectoryChildren` was running `/bin/mv` and hitting `PathTooLongException` while using `ZipFile.ExtractToDirectory` - references to `libzip.mdproj` are conditional on Windows, will be getting `libzip` from NuGet in the future UnzipDirectoryChildren: - on Windows we are using `System.IO.Compression` to extract directly into the destination and avoid `%TEMP%` to workaround `MAX_PATH` - Other platforms continue to use `unzip`, in order to preserve file attributes General changes: - gitignore for VS 2017
jonathanpeppers
force-pushed
the
windows-android-toolchain
branch
from
e9bc2a6 to
d1b8554
Compare
|
The macOS+xbuild PR Build job hung. I aborted it. |
Changes: dotnet/java-interop@da74abd...c3c3575 * dotnet/java-interop@c3c35753: Bump to mono/LineEditor@5d67d34e (dotnet#743) Should allow master builds to be signed once again.
Changes: dotnet/java-interop@da74abd...c3c3575 * dotnet/java-interop@c3c35753: Bump to mono/LineEditor@5d67d34e (#743) Should allow master builds to be signed once again.
Problems fixed on Windows:
RequiredProgramsare n/a on WindowsAndroidSdkItemfor Windows downloadsAcceptAndroidSdkLicensesshould run.batfile on WindowsUnzipDirectoryChildrenwas running/bin/mvand hittingPathTooLongExceptionwhile usingZipFile.ExtractToDirectorylibzip.mdprojare conditional on Windows, will begetting
libzipfrom NuGet in the futureUnzipDirectoryChildren:
System.IO.Compressionto extractdirectly into the destination and avoid
%TEMP%toworkaround
MAX_PATHunzip, in order to preservefile attributes
General changes:
You can see a successful build I setup on AppVeyor, which ran: