Prepare uap package #15866

joperezr · 2017-02-06T17:43:55Z

These changes will move packaging assets to bin\$(TargetGroup)\pkg into bin\pkg\$(TargetGroup).
It will also mark all of the applicable libraries as UAP and will deploy them to be ready for packaging. Both folders have closure complete except for System.IO.IsolatedStorage

jkotas · 2017-02-06T19:16:10Z

external/netfx/netfx.depproj

@@ -25,5 +25,11 @@
    <FileToExclude Include="System.EnterpriseServices.Thunk" />
    <FileToExclude Include="System.EnterpriseServices.Wrapper" />
  </ItemGroup>
+  <ItemGroup Condition="'$(TargetGroup)'=='uap' or '$(TargetGroup)'=='uapaot'">


Does this really belong to this file? It does not sound right to have coupling between netfx and uap/uapaot.

The problem is that we need this targeting pack in order to build the desktop and netstandard shims, which are required in order to be able to build the uap tree. We should probably consider renaming this project in order to avoid confusion.

The filtering shouldn't be done here. It should be done in the shims.proj file.

I believe these lines I added are required anyway, or else the depproj won't resolve those assemblies, but what we can do is that in the shims.proj we can remove them from non-uap builds.

No you shouldn't need them in this depproj any more we need to keep the set of things resolve across the different configurations consistent otherwise BuildAll will keep thrashing this directory.

So then you want me to add these lines to the itemgroup above and remove the condition?

No I would like you to remove this entire section in this depproj and only have the filtering in the shims.proj

ok, I finally see what you mean, seems like we recently changed the default on depproj targets. I'll rebase and fix this.

jkotas · 2017-02-06T19:18:42Z

src/Microsoft.Win32.Registry/src/Microsoft.Win32.Registry.csproj

@@ -31,7 +31,7 @@
    <Compile Include="Microsoft\Win32\SafeHandles\SafeRegistryHandle.cs" />
    <Compile Include="System\Security\AccessControl\RegistryRights.cs" />
  </ItemGroup>
-  <ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp' AND '$(TargetsWindows)' == 'true'">
+  <ItemGroup Condition="('$(TargetGroup)' == 'netcoreapp' or '$(TargetGroup)' == 'uap') AND '$(TargetsWindows)' == 'true'">


This condition should rather be:

'$(TargetGroup)' != 'netfx' and '$(TargetsWindows)' == 'true'

I agree it would be cleaner, but @weshaggard suggested that we should always do inclusive conditions as opposed to exclusive, I'm fine with either.

Yes my preference is to generally be inclusive instead of exclusive but we should keep the conditions within reason. I wonder if we should set some other property in here that means TargetsCore and use that in the various places.

jkotas · 2017-02-06T19:19:57Z

src/Microsoft.Win32.Registry/src/Microsoft.Win32.Registry.csproj

@@ -16,7 +16,7 @@
  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netcoreapp-Windows_NT-Release|AnyCPU'" />
  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netfx-Debug|AnyCPU'" />
  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netfx-Release|AnyCPU'" />
-  <ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp'">
+  <ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp' or '$(TargetGroup)' == 'uap'">


This should be '$(TargetGroup)' != 'netfx'

weshaggard · 2017-02-06T19:38:28Z

binplace.targets

+      <RefPath Condition="'$(IsUAPRef)'=='true'">$(UAPPackageRefPath)</RefPath>
+      <RuntimePath>$(UAPPackageRuntimePath)</RuntimePath>
+    </BinplaceConfiguration>
+	<!-- binplace targeting packs which may be different from BuildConfiguration -->


nit: indention.

weshaggard · 2017-02-06T19:38:38Z

binplace.targets

@@ -7,6 +7,8 @@
         https://github.com/dotnet/corefx/issues/14291 is tracking cleaning this up -->
    <IsRuntimeAndReferenceAssembly Condition="'$(IsRuntimeAndReferenceAssembly)' == '' and '$(IsRuntimeAssembly)' == 'true' and Exists('$(SourceDir)/$(AssemblyName)') and !Exists('$(SourceDir)/$(AssemblyName)/ref') and !$(AssemblyName.StartsWith('System.Private'))">true</IsRuntimeAndReferenceAssembly>
    <IsNETCoreAppRef Condition="'$(IsNETCoreAppRef)' == ''">$(IsNETCoreApp)</IsNETCoreAppRef>
+    <IsUAPRef Condition="'$(IsUAPRef)' == ''">$(IsUAP)</IsUAPRef>
+


nit: extra empty line.

weshaggard · 2017-02-06T19:39:41Z

src/Microsoft.Win32.Registry/dir.props

@@ -5,6 +5,8 @@
    <AssemblyVersion>4.1.0.0</AssemblyVersion>
    <IsNETCoreApp>true</IsNETCoreApp>
    <IsNETCoreAppRef>false</IsNETCoreAppRef>
+    <IsUAP>true</IsUAP>


What pulls in registry into the closure?

System.Diagnostics.Process and System.Private.Xml. Implementation only, it is not required for refs.

OK we can keep it for now but I suspect this will be one of the libraries flagged once we enable the PInvoke checker.

weshaggard · 2017-02-06T19:41:11Z

src/System.Diagnostics.DiagnosticSource/dir.props

@@ -4,5 +4,7 @@
  <PropertyGroup>
    <AssemblyVersion>4.0.2.0</AssemblyVersion>
    <IsNETCoreApp>true</IsNETCoreApp>
+    <IsUAP>true</IsUAP>
+    <IsUAPRef>false</IsUAPRef>


We will eventually want to revisit these IsUAPRef=false choices because we will likely just include them as refs as well.

sounds good, for now I only selected the ones required to satisfy the following:

Support our shipped UAP package.

All dependencies of netstandard.dll facade.

Required assemblies to have a full closure (Excluding classic assembly facades)

weshaggard · 2017-02-06T19:43:10Z

dir.props

@@ -255,8 +255,10 @@
    <RuntimeProjectFile Condition="'$(RuntimeProjectFile)' == ''">$(ProjectDir)\external\runtime\runtime.depproj</RuntimeProjectFile>

    <!-- Paths to binplace package content -->
-    <NETCoreAppPackageRefPath>$(BinDir)netcoreapp\pkg\ref</NETCoreAppPackageRefPath>
-    <NETCoreAppPackageRuntimePath>$(BinDir)netcoreapp\pkg\lib</NETCoreAppPackageRuntimePath>
+    <NETCoreAppPackageRefPath>$(BinDir)pkg\netcoreapp\ref</NETCoreAppPackageRefPath>


Should we generalize these and just call the PackageRefPath and PackageRuntimePath?

Sure I can do that and use $(TargetGroup) instead

I've been trying to address this, but I'm having problems with the binplacing logic. The reason is that today we use the Target group specific paths in the following snippet:

<BinplaceConfiguration Condition="'$(IsNETCoreApp)' == 'true'" Include="netcoreapp-$(OSGroup)"> <RefPath Condition="'$(IsNETCoreAppRef)' == 'true'">$(NETCoreAppPackageRefPath)</RefPath> <RuntimePath>$(NETCoreAppPackageRuntimePath)</RuntimePath> </BinplaceConfiguration> <BinplaceConfiguration Condition="'$(IsUAP)' == 'true'" Include="uap-$(OSGroup)"> <RefPath Condition="'$(IsUAPRef)'=='true'">$(UAPPackageRefPath)</RefPath> <RuntimePath>$(UAPPackageRuntimePath)</RuntimePath> </BinplaceConfiguration>

Basically the problem is that if we are building a netstandard asset, today we are binplacing it in both the uap package paths and the netcoreapp ones. If we decide to unify these paths, then we would be giving that up unless we change the logic of how we do that here.

Ok don't let this hang you up. We can look at cleaning this up later.

weshaggard · 2017-02-06T19:43:35Z

src/shims/shims.proj

@@ -74,8 +74,8 @@
    <!-- Copy the facades to the package ref and lib folders to be included in the packages -->
    <!-- TODO: replace with BinPlacing targets -->
    <ItemGroup>
-      <PackageOutputPaths Include="$(BinDir)$(TargetGroup)/pkg/ref" />
-      <PackageOutputPaths Include="$(BinDir)$(TargetGroup)/pkg/lib" />
+      <PackageOutputPaths Include="$(BinDir)pkg/$(TargetGroup)/ref" />


I would like to genralize the Package paths so we can share them here.

those are shared already, as they use $(TargetGroup) in their definition.

I realize these will be correct but I would still prefer we share a common path from dir.props here instead of maintaining the pkg output directory structure in multiple locations.

weshaggard · 2017-12-12T19:14:58Z

src/System.Collections.Immutable/dir.props

@@ -5,5 +5,6 @@
    <PackageVersion>1.4.0</PackageVersion>
    <AssemblyVersion>1.2.2</AssemblyVersion>
    <IsNETCoreApp>true</IsNETCoreApp>
+    <IsUAP>true</IsUAP>


@joperezr do you know why Immutable and Reflection.Metadata needed to be pulled into the UAP platform package?

IIRC it was because they were in the previous stable package, so we needed to have it in there for the new one as well. This was the same case for the WCF assemblies.

Prepare uap package Commit migrated from dotnet/corefx@afaaf45

joperezr added 2 commits February 6, 2017 09:30

Binplacing assemblies for the UAP Identity Package

9bc4d97

Setting up bin/pkg/uap/ref and lib to prepare uap package generation

f309378

dnfclas added the cla-already-signed label Feb 6, 2017

karelz added the area-Infrastructure-libraries label Feb 6, 2017

karelz assigned joperezr Feb 6, 2017

jkotas reviewed Feb 6, 2017

View reviewed changes

weshaggard reviewed Feb 6, 2017

View reviewed changes

PR Feedback

d84b1fa

joperezr merged commit afaaf45 into dotnet:master Feb 8, 2017

joperezr deleted the PrepareUAPPackage branch February 8, 2017 18:39

karelz modified the milestone: 2.0.0 Feb 9, 2017

weshaggard reviewed Dec 12, 2017

View reviewed changes

macrogreg pushed a commit to open-telemetry/opentelemetry-dotnet-instrumentation that referenced this pull request Sep 24, 2020

Prepare uap package (dotnet/corefx#15866)

ae1aa33

Prepare uap package Commit migrated from dotnet/corefx@afaaf45

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

Prepare uap package (dotnet/corefx#15866)

d9c497b

Prepare uap package Commit migrated from dotnet/corefx@afaaf45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare uap package #15866

Prepare uap package #15866

joperezr commented Feb 6, 2017

jkotas Feb 6, 2017

joperezr Feb 6, 2017

weshaggard Feb 6, 2017

joperezr Feb 6, 2017

weshaggard Feb 6, 2017

joperezr Feb 7, 2017

weshaggard Feb 7, 2017

joperezr Feb 7, 2017

jkotas Feb 6, 2017

joperezr Feb 6, 2017

weshaggard Feb 6, 2017

jkotas Feb 6, 2017

weshaggard Feb 6, 2017

weshaggard Feb 6, 2017

weshaggard Feb 6, 2017

joperezr Feb 6, 2017 •

edited

Loading

weshaggard Feb 6, 2017

weshaggard Feb 6, 2017

joperezr Feb 6, 2017

weshaggard Feb 6, 2017

joperezr Feb 6, 2017

joperezr Feb 8, 2017

weshaggard Feb 8, 2017

weshaggard Feb 6, 2017

joperezr Feb 6, 2017

weshaggard Feb 6, 2017

weshaggard Dec 12, 2017

joperezr Dec 18, 2017

Prepare uap package #15866

Prepare uap package #15866

Conversation

joperezr commented Feb 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joperezr Feb 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joperezr Feb 6, 2017 •

edited

Loading