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.

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

[create-vsix] Create .vsix files #541

Merged
merged 1 commit into from
Apr 5, 2017
Merged

Conversation

jonpryor
Copy link
Member

@jonpryor jonpryor commented Mar 30, 2017

Visual Studio 2017 uses .vsix files for Xamarin.Android SDK support.
A .vsix file is ZIP container with additional metadata, and the
Microsoft.VSSDK.BuildTools NuGet package contains various MSBuild
targets and tools to assist in creating .vsix files.

Add a new build-tools/create-vsix/create-vsix.csproj project to
create the bin/Build$(Configuration)/Xamarin.Android.Sdk*.vsix file,
so that we can plausibly provide per-build OSS Xamarin.Android
releases that work with Visual Studio 2017.

Unfortunately those tools were written on Windows, and not really well
tested on macOS or Linux... In particular, there are case-sensitivity
and directory-separator-char issues with the tooling, necessitating
that MONO_IOMAP=all be exported in order to run them:

    MONO_IOMAP=all MONO_OPTIONS=--arch=64 msbuild \
            build-tools/create-vsix/create-vsix.csproj /p:CreateVsixContainer=True

Meanwhile, we're still attempting to allow things to be built with
xbuild 1. Thread this needle by "special-casing" the
$(BuildDependsOn), $(CopyVsixManifestFileDependsOn), and
$(DetokenizeVsixManifestFileDependsOn) MSBuild properties so that
when the $(CreateVsixContainer) MSBuild property is False -- the
default -- no .vsix package will be created, and xbuild will be
able to build the project.

Similar needle threading is needed to "support" building on Linux: the
Microsoft.VSSDK.BuildTools NuGet package uses case in an inconsistent
fashion -- or is it nuget? -- which results in
failures when building on Linux because nuget extracts e.g.:

    packages/Microsoft.VSSDK.BuildTools.15.0.26201/tools/vssdk/Microsoft.VsSDK.targets

while Microsoft.VSSDK.BuildTools attempts to <Import/> the file:

    packages/Microsoft.VSSDK.BuildTools.15.0.26201/tools/VSSDK/Microsoft.VsSDK.targets

vssdk != VSSDK on case-sensitive filesystems, so this results in
an error on Ubuntu (and presumably case-senstive macOS as well).

Handle the Linux case by extending the xbuild case: even when
$(CreateVsixContainer) is True, we won't actually build the .vsix
unless the $(VsSDKInstall) directory exists, which is the
tools/VSSDK path.

Building with $(CreateVsixContainer) set to True will require using
msbuild with MONO_IOMAP=all and MONO_OPPTIONS=--arch=64
exported, while using a case-insensitive filesystem.

The new make create-vsix CONFIGURATIONS=Debug target can be used to
explicitly create the Xamarin.Android.Sdk*.vsix file.

One problem with creating .vsix files: macOS still defaults to using
a 32-bit process for mono, and make create-vsix regularly fails
for me with an OutOfMemoryException when attempting to generate a
.vsix file ~600-700+MB in size. (Why so large? In part because for
Debug configuration we're including un-strip'd native libraries;
libmonosgen-2.0.dll is 237MB in size (!); it's closer to 5MB when
strip'd, but any un-strip'd MXE-generated native libraries to the
.vsix file quickly results in OutOfMemoryExceptions.)

A 64-bit mono can be used by using mono64 -- which isn't easily done
with msbuild -- or by using mono --arch=64, which can be done
with msbuild by exporting MONO_OPTIONS=--arch=64.
The make create-vsix target does this.

Finally, once we have a .visx being created, we then examine the
contents of the .vsix file, which is...weird:

    $MSBuild/Xamarin/Android/Xamarin.Android.CSharp.targets                                           # Desirable
    $ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v1.0/Xamarin.Android.CSharp.targets          # wat?
    $ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v1.0/Facades/Xamarin.Android.CSharp.targets  # really?!
    $ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v7.1/Xamarin.Android.CSharp.targets          # !!!!

Turns Out™, the problem was due to @(ProjectReference). We're
(ab)using @(ProjectReference) to explicitly specify project
dependencies, and the (implicit) project build order.

Unexpectedly (forgetten?), MSBuild also copies the outputs of
@(ProjectReference)s into the $(OutputPath) of the current
project.

Since e.g. Mono.Posix.csproj had a reference on
Xamarin.Android.Build.Tasks.csproj, the result of this is that the
$(OutputPath) of Mono.Posix.csproj --
$prefix/lib/xbuild-frameworks/MonoAndroid/v1.0 -- also contained the
%(None.CopyToOutputDirectory) items from
Xamarin.Android.Build.Tasks.csproj.

Oops.

The fix is to set %(ProjectReference.Private) to False, which
disables this copying of additional and undesirable files.

Footnotes

  1. But for how much longer?

@jonpryor jonpryor added the do-not-merge PR should not be merged. label Mar 30, 2017
@jonpryor
Copy link
Member Author

TODO: Double-check how the XamarinVS repo creates the .vsix file, and add the Windows-specific MSBuild targets.

@jonpryor jonpryor force-pushed the jonp-vsix branch 3 times, most recently from c390259 to 3513aa6 Compare March 30, 2017 13:55
<Installation AllUsers="true">
<InstallationTarget Version="[15.0,)" Id="Microsoft.VisualStudio.Community" />
</Installation>
<Dependencies>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this node altogether. It's not needed.

<?xml version="1.0" encoding="utf-8"?>
<PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
<Metadata>
<Identity Id="4489ebd9-10e4-42d0-bc3c-0639c608aaea" Version="1.0" Language="en-US" Publisher="Company" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Note how the version can be fetched dynamically by invoking an MSBuild target that can read it from wherever: |Android.Sdk;GetVsixVersion| (that's |PROJECT_NAME;TARGET_NAME|)

@jonpryor jonpryor force-pushed the jonp-vsix branch 3 times, most recently from 4db94ef to 6b0dd12 Compare March 30, 2017 19:24
@jonpryor jonpryor removed the do-not-merge PR should not be merged. label Mar 30, 2017
<_BuildVsix Condition=" '$(_BuildVsix)' == '' ">False</_BuildVsix>
</PropertyGroup>
<Import Project="..\..\Configuration.props" />
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that no code will ever run from the VSIX itself, how about just removing entirely the two property groups that are conditioned and just add one property below _BuildVsix as <OutputPath>bin\$(Configuration)</OutputPath> and just declare the two conditioned property groups empty just to satisfy VS?

<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' " />
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' " />

Copy link
Member Author

Choose a reason for hiding this comment

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

Historically, Xamarin Studio gets rather cross if It doesn't have an explicit Debug property group and Release property group. I haven't tried in this case, but odds are against me.

<WarningLevel>4</WarningLevel>
<ConsolePause>false</ConsolePause>
</PropertyGroup>
<ItemGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

This ItemGroup (and the AssemblyInfo.cs itself) shouldn't be needed if we aren't including any built outputs?

-->
<Project InitialTargets="RedirectMonoAndroidSdkPaths;RemoveSdksCache" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole file was intended only to work on VS2017+, which is true as long as '$(VsInstallRoot)' != ''. Maybe the property groups and targets in this file should have the condition, just to make it more explicit and avoid unintentional usage outside of that scenario?

Visual Studio 2017 uses `.vsix` files for Xamarin.Android SDK support.
A `.vsix` file is [ZIP container with additional metadata][0], and the
[Microsoft.VSSDK.BuildTools NuGet package][1] contains various MSBuild
targets and tools to assist in creating `.vsix` files.

Add a new `build-tools/create-vsix/create-vsix.csproj` project to
create the `bin/Build$(Configuration)/Xamarin.Android.Sdk*.vsix` file,
so that we can plausibly provide per-build OSS Xamarin.Android
releases that work with Visual Studio 2017.

Unfortunately those tools were written on Windows, and not really well
tested on macOS or Linux... In particular, there are case-sensitivity
and directory-separator-char issues with the tooling, necessitating
that `MONO_IOMAP=all` be exported in order to run them:

	MONO_IOMAP=all MONO_OPTIONS=--arch=64 msbuild \
		build-tools/create-vsix/create-vsix.csproj /p:CreateVsixContainer=True

Meanwhile, we're still attempting to allow things to be built with
`xbuild` [^3]. Thread this needle by "special-casing" the
`$(BuildDependsOn)`, `$(CopyVsixManifestFileDependsOn)`, and
`$(DetokenizeVsixManifestFileDependsOn)` MSBuild properties so that
when the `$(CreateVsixContainer)` MSBuild property is False -- the
default -- no `.vsix` package will be created, and `xbuild` will be
able to build the project.

Similar needle threading is needed to "support" building on Linux: the
Microsoft.VSSDK.BuildTools NuGet package uses case in an inconsistent
fashion -- or is it `nuget`? -- which results in
[failures when building on Linux][2] because `nuget` extracts e.g.:

	packages/Microsoft.VSSDK.BuildTools.15.0.26201/tools/vssdk/Microsoft.VsSDK.targets

while Microsoft.VSSDK.BuildTools attempts to `<Import/>` the file:

	packages/Microsoft.VSSDK.BuildTools.15.0.26201/tools/VSSDK/Microsoft.VsSDK.targets

`vssdk` != `VSSDK` on case-sensitive filesystems, so this results in
an error on Ubuntu (and presumably case-senstive macOS as well).

Handle the Linux case by extending the `xbuild` case: *even when*
`$(CreateVsixContainer)` is True, we won't actually build the `.vsix`
unless the `$(VsSDKInstall)` directory exists, which is the
`tools/VSSDK` path.

Building with `$(CreateVsixContainer)` set to True will require using
`msbuild` with `MONO_IOMAP=all` and `MONO_OPPTIONS=--arch=64`
exported, while using a case-insensitive filesystem.

The new `make create-vsix CONFIGURATIONS=Debug` target can be used to
explicitly create the `Xamarin.Android.Sdk*.vsix` file.

One problem with creating `.vsix` files: macOS still defaults to using
a 32-bit process for `mono`, and `make create-vsix` regularly fails
for me with an `OutOfMemoryException` when attempting to generate a
`.vsix` file ~600-700+MB in size. (Why so large? In part because for
Debug configuration we're including un-`strip`'d native libraries;
`libmonosgen-2.0.dll` is *237MB* in size (!); it's closer to 5MB when
`strip`'d, but any un-`strip`'d MXE-generated native libraries to the
`.vsix` file quickly results in `OutOfMemoryException`s.)

A 64-bit mono can be used by using `mono64` -- which isn't easily done
with `msbuild` -- or by using `mono --arch=64`, which *can* be done
with `msbuild` by exporting `MONO_OPTIONS=--arch=64`.
The `make create-vsix` target does this.

Finally, once we have a `.visx` being created, we then examine the
*contents* of the `.vsix` file, which is...weird:

	$MSBuild/Xamarin/Android/Xamarin.Android.CSharp.targets	                                          # Desirable
	$ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v1.0/Xamarin.Android.CSharp.targets          # wat?
	$ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v1.0/Facades/Xamarin.Android.CSharp.targets  # really?!
	$ReferenceAssemblies/Microsoft/Framework/MonoAndroid/v7.1/Xamarin.Android.CSharp.targets          # !!!!

Turns Out™, the problem was due to `@(ProjectReference)`. We're
(ab)using `@(ProjectReference)` to explicitly specify project
dependencies, and the (implicit) project build order.

Unexpectedly (forgetten?), MSBuild *also* copies the *outputs* of
`@(ProjectReference)`s into the `$(OutputPath)` of the current
project.

Since e.g. `Mono.Posix.csproj` had a reference on
`Xamarin.Android.Build.Tasks.csproj`, the result of this is that the
`$(OutputPath)` of `Mono.Posix.csproj` --
`$prefix/lib/xbuild-frameworks/MonoAndroid/v1.0` -- also contained the
`%(None.CopyToOutputDirectory)` items from
`Xamarin.Android.Build.Tasks.csproj`.

Oops.

The fix is to set `%(ProjectReference.Private)` to False, which
disables this copying of additional and undesirable files.

[0]: https://msdn.microsoft.com/en-us/library/dd997148.aspx
[1]: https://www.nuget.org/packages/Microsoft.VSSDK.BuildTools/
[2]: https://jenkins.mono-project.com/view/Xamarin.Android/job/xamarin-anroid-linux-pr-builder/297/consoleText
[^3]: But for how much longer?
@jonpryor jonpryor merged commit a08cd80 into dotnet:master Apr 5, 2017
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Dec 19, 2019
Changes: dotnet/java-interop@f3553f4...d61b329

  * dotnet/java-interop@d61b329 [Java.Interop-Tests] Relax Exception.Message checks (dotnet#544)
  * dotnet/java-interop@d778204 [Java.Interop] Better linker-friendly JavaObjectArray<T>.ValueMarshaler (dotnet#546)
  * dotnet/java-interop@4bd07e0 [vs-code] Recommend the zbecknell.t4-support extension (dotnet#545)
  * dotnet/java-interop@3b24a2c [Java.Interop] apply AggressiveInlining where profiler shows calls (dotnet#541)
jonpryor added a commit that referenced this pull request Dec 20, 2019
Changes: dotnet/java-interop@f3553f4...d61b329

  * dotnet/java-interop@d61b329 [Java.Interop-Tests] Relax Exception.Message checks (#544)
  * dotnet/java-interop@d778204 [Java.Interop] Better linker-friendly JavaObjectArray<T>.ValueMarshaler (#546)
  * dotnet/java-interop@4bd07e0 [vs-code] Recommend the zbecknell.t4-support extension (#545)
  * dotnet/java-interop@3b24a2c [Java.Interop] apply AggressiveInlining where profiler shows calls (#541)
@github-actions github-actions bot locked and limited conversation to collaborators Feb 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge PR should not be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants