Skip to content
This repository was archived by the owner on Nov 21, 2018. It is now read-only.

Add Microsoft.AspNetCore.All metapackage and build package cache #18

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

JunTaoLuo
Copy link
Contributor

WIP

@JunTaoLuo JunTaoLuo requested a review from pranavkm April 5, 2017 21:18
@dnfclas
Copy link

dnfclas commented Apr 5, 2017

@JunTaoLuo,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

build.ps1 Outdated
@@ -33,7 +33,7 @@ cd $PSScriptRoot
$repoFolder = $PSScriptRoot
$env:REPO_FOLDER = $repoFolder

$koreBuildZip="https://github.com/aspnet/KoreBuild/archive/dev.zip"
$koreBuildZip="https://github.com/aspnet/KoreBuild/archive/stable-cache.zip"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you file individual follow up items for each of the hacks we have here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is going to cause issues in universe builds. The CLI and shared runtime are locked by the outer msbuild process, so this will fail. What does it take to not do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no longer needed, will remove.

@@ -1,6 +1,9 @@
<Project>
<PropertyGroup>
<AspNetCoreVersion>2.0.0-*</AspNetCoreVersion>
<AspNetCore01Version>0.1.0-*</AspNetCore01Version>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pretty meaningless. Can you use package names instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rather annoying that 03 is used for Diagnostics.Elm, Proxy and Buffering but sure I'll name them.


<ItemGroup>
<!-- set PrivateAssets=None to ensure that all assets, including Build and Analyzer, are included in the nuspec -->
<PackageReference Include="Microsoft.AspNetCore.Antiforgery" Version="$(AspNetCoreVersion)" PrivateAssets="None" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Follow up to use packages.csv to generate this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#19

<PackageReference Include="Microsoft.AspNetCore.Cryptography.KeyDerivation" Version="$(AspNetCoreVersion)" PrivateAssets="None" />
<PackageReference Include="Microsoft.AspNetCore.DataProtection" Version="$(AspNetCoreVersion)" PrivateAssets="None" />
<PackageReference Include="Microsoft.AspNetCore.DataProtection.Abstractions" Version="$(AspNetCoreVersion)" PrivateAssets="None" />
<!--<PackageReference Include="Microsoft.AspNetCore.DataProtection.AzureStorage" Version="$(AspNetCoreVersion)" PrivateAssets="None" />-->
Copy link
Contributor

Choose a reason for hiding this comment

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

No commented out lines.

<ItemGroup>
<!-- set PrivateAssets=None to ensure that all assets, including Build and Analyzer, are included in the nuspec -->
<PackageReference Include="Microsoft.AspNetCore.Antiforgery" Version="$(AspNetCoreVersion)" PrivateAssets="None" />
<!--<PackageReference Include="Microsoft.AspNetCore.ApplicationInsightsLightup" Version="$(AspNetCoreVersion)" PrivateAssets="None" />-->
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually these shouldn't be removed. A few of the packages need to be in the cache but not the metapackage and the others need to be included but are blocked based on other issues. I'll add some notes linking to the individual issues.


<PropertyGroup>
<IncludeBuildOutput>false</IncludeBuildOutput>
<TargetFramework>netstandard2.0</TargetFramework>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be netstandard1.6? Does it have to be a 2.0 package?


<RemoveDir Directories="$(OutputPath)" />
<RemoveDir Directories="$(WorkingDirectory)" />
<Exec Command="dotnet cache --entries $(MetaPackageFile) --framework netcoreapp2.0 --runtime win7-x64 --output $(OutputPath) --framework-version 2.0.0-beta-001776-00 --working-dir $(WorkingDirectory)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Follow up to fix

  1. The hardcoded rid
  2. The hardcoded fx-version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually going to fix the hardcoded rid right now to be determined by the platform type. Will file issue with the hardcoded fx-version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Star works for --framework-version apparently

NuGet.config Outdated
<!-- Workaround for missing package on our feed -->
<add key="DotNetCore" value="https://dotnet.myget.org/F/dotnet-core/api/v3/index.json" />
<!-- Additional feeds for dotnet cache -->
<add key="Roslyn" value="https://dotnet.myget.org/F/roslyn/api/v3/index.json" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't need this yet, since it's pulled in via M.N.A for apps but we will need it once we move to 2.0.

NuGet.config Outdated
@@ -3,6 +3,10 @@
<packageSources>
<add key="AspNetCore" value="https://dotnet.myget.org/F/aspnetcore-ci-dev/api/v3/index.json" />
<add key="AspNetCoreTools" value="https://dotnet.myget.org/F/aspnetcore-tools/api/v3/index.json" />
<!-- Workaround for missing package on our feed -->
<add key="DotNetCore" value="https://dotnet.myget.org/F/dotnet-core/api/v3/index.json" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Which one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The System.Numerics.Vector that we spoke about yesterday. https://github.com/aspnet/CoreCLR/pull/175. Did this get fixed?

@JunTaoLuo JunTaoLuo merged commit 36cd2b8 into dev Apr 6, 2017
@JunTaoLuo JunTaoLuo deleted the johluo/all branch April 6, 2017 00:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants