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

Provide a way to override project properties without forking #209

Closed
kzu opened this issue Jul 1, 2016 · 8 comments
Closed

Provide a way to override project properties without forking #209

kzu opened this issue Jul 1, 2016 · 8 comments
Milestone

Comments

@kzu
Copy link

kzu commented Jul 1, 2016

Given that Rx may (and actually is) used in scenarios where multiple components may have been built and tested against different versions of Rx, and given that AppDomain/Process level isolation is not always an option (i.e. VS extensions). See #97 and #99.

Therefore, sometimes the only way to guarantee runtime consistency is to do one of two things:

  1. ILMerge your dependencies
  2. Rename or re-sign the assemblies
  1. is fairly easy to do and some may prefer that way
  2. is typically way more involved, requiring forking the repo, tweaking the places where signing happens, and maintaining all that in sync with upstream.

In order to make 2) more straightforward, I propose you follow the same approach we contributed to Mono.Cecil in jbevain/cecil#217.

This "settings" file is basically a targets that's imported at the bottom of all Cecil projects (i.e. https://github.com/jbevain/cecil/blob/master/Mono.Cecil.csproj#L136). Most projects already have one such global import shared across all projects, which should simplify the implementation.

This common targets file then attempts to lookup another targets that provides overriden MSBuild values, by searching ancestor directories:

  <PropertyGroup>
    <CecilOverrides Condition="'$(CecilOverrides)' == ''">$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildProjectDirectory), Mono.Cecil.overrides))\Mono.Cecil.overrides</CecilOverrides>
  </PropertyGroup>  
  <Import Project="$(CecilOverrides)" Condition="Exists('$(CecilOverrides)')" />

Another repo consuming Rx that wants to isolate itself from other extensions, could simply submodule the Rx repo, and provide this override targets in a folder outside the submodule (therefore never dirty-ing it).

In Xamarin's case, this is what it looks like:

<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup>
    <AssemblyName>$(AssemblyName.Replace('Mono', 'Xamarin.VisualStudio'))</AssemblyName>
  </PropertyGroup>

  <ItemGroup>
    <Compile Include="$(MsBuildThisFileDirectory)Cecil.AssemblyInfo.cs" />
  </ItemGroup>

  <PropertyGroup>
    <DefineConstants>TRACE;NET_3_5;NET_4_0</DefineConstants>
    <ErrorReport>prompt</ErrorReport>
    <WarningLevel>4</WarningLevel>
    <TargetFrameworkVersion>v4.0</TargetFrameworkVersion>
  <PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
    <DebugSymbols>true</DebugSymbols>
    <DebugType>full</DebugType>
    <Optimize>false</Optimize>
    <OutputPath>bin\net_4_0_Debug\</OutputPath>
    <DefineConstants>$(DefineConstants);DEBUG</DefineConstants>
  </PropertyGroup>
  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
    <DebugType>pdbonly</DebugType>
    <Optimize>true</Optimize>
    <OutputPath>bin\net_4_0_Release\</OutputPath>
  </PropertyGroup>

</Project>

You could of course override signing:

    <PropertyGroup>
        <SignAssembly>true</SignAssembly>
        <AssemblyOriginatorKeyFile>mykey.snk</AssemblyOriginatorKeyFile>
    </PropertyGroup>

In our case, we're even providing a Cecil.AssemblyInfo.cs where we add InternalsVisibleTo attributes for testing purposes too, which are typically problematic also when consuming an external project.

@kzu kzu changed the title Provide a way to override projects without forking Provide a way to override project properties without forking Jul 1, 2016
@clairernovotny
Copy link
Member

Not sure I understand the needs here? Wouldn't #205 address this? It is no different than the main parts of CoreFX in the way it needs to handle api surface area across different capabilities and versions?

@clairernovotny
Copy link
Member

clairernovotny commented Jul 1, 2016

This also doesn't currently work as we use project.json and xproj. Something like this may be possible after things migrate back to MSBuild, but I'm not sure it should be necessary anyway.

@kzu
Copy link
Author

kzu commented Jul 1, 2016

It's not the same thing. I'm just sharing something that works for us in other plug-in related scenarios really well, without having to do any kind of potentially costly babysitting of APIs and version numbers. You know you'll always get the version you tested and compiled against, regardless of where you're being re-hosted/consumed, and regardless of the hosting process binding policies (which #205 doesn't fix in the VS case since someone could still provide a binding redirect policy that breaks someone else that depends on a specific version containing specific fixes, even if they are only behavioral and not API).

Feel free to disregard if not applicable ;).

I never understood the rush to move to xproj anyway. Had no idea you guys did that at all, so again, more reason to probably discard this, and just let others keep complaining ;)

@clairernovotny
Copy link
Member

clairernovotny commented Jul 1, 2016

@kzu thanks for the tip!

One of the main reasons to use xproj today is it's cross-compile capability. As we need to optimize for as many platforms as possible, using "single-headed" csproj's would be unwieldy.

I think this is still a useful idea though but I'm going to defer it until after the xproj->csproj transition is complete, where we can cross-compile there. Once back on csproj, I think we can have a conditionally imported props/target to enable easy overriding.

@clairernovotny clairernovotny added this to the Future milestone Jul 1, 2016
@kzu
Copy link
Author

kzu commented Jul 1, 2016

I wonder why shared projects + multiple platform-specific heads with platform-specific code didn't work for you? Not familiar with how xproj does the conditional compilation of stuff depending on target platform, or if you just have to resort to ifdefs..

@clairernovotny
Copy link
Member

At the end of the day, xproj does a "for each framework, evaluate these dependencies, defines and build an output". It's far simpler to maintain than having a 6-7 heads per project.

If you look at the source here, we are heavily feature-flagged to ensure maximum capabilities per platform. This has the effect of creating between 2-6 outputs per library to ensure things line-up appropriately. All of this is transparent to the end user as NuGet pulls in the best version for them, but it does mean that anything that adds "heads" would be hugely challenging to deal with.

@kzu
Copy link
Author

kzu commented Jul 1, 2016

Interesting. I'll look it up now closely since this might be useful
information down the road.

/cc @mhutch

@clairernovotny
Copy link
Member

Closing this as you can now do this with MSBuild in Preview3 and later. So this will be implemented once the tooling supports this project.

@shiftkey shiftkey closed this as completed Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants