Convert the artifacts folder layout to match Arcade #6850
Conversation
Changes: * IsProductPackage => IsShippingPackage * artifacts/$config/packages => artifacts/packages/$config * packages/product => packages/Shipping * packages/internal => packages/NonShipping * Renamed MSBuild properties used for output folders * Update build tools to use 2 spaces in global.json This repo will be converting soon to use Arcade. This updates the layout the artifacts folder, and uses Arcade property names
natemcmaster
requested review from
jkotalik,
SteveSandersonMS and
Tratcher
as code owners
|
cc @tmat @markwilkie |
Eilon
added
the
area-infrastructure
natemcmaster
requested review from
dougbu
and removed request for
jkotalik,
SteveSandersonMS and
pranavkm
| <ProductPackageOutputPath>$(ArtifactsDir)$(Configuration)\packages\product\</ProductPackageOutputPath> | ||
| <InternalPackageOutputPath>$(ArtifactsDir)$(Configuration)\packages\internal\</InternalPackageOutputPath> | ||
| <InstallersOutputPath>$(ArtifactsDir)$(Configuration)\installers\</InstallersOutputPath> | ||
| <ArtifactsShippingPackagesDir>$(ArtifactsDir)\packages\$(Configuration)\Shipping\</ArtifactsShippingPackagesDir> |
There was a problem hiding this comment.
Suggest moving these properties to one file and importing that from here, the root Directory.Build.props file and anywhere else they're needed e.g. the templating Directory.Build.props file
There was a problem hiding this comment.
This can happen once we implement #4257
There was a problem hiding this comment.
What's the connection? (These properties aren't framework-specific or conditional, $(ArtifactsDir) could be moved to the common .props file, and %(Configuration) is fine.)
There was a problem hiding this comment.
Once we do #4247, we will be able to build the shared framework without having to build, pack, and then restore against the packages we just made.
%(ArtifactsDir) could be moved to the common
It could be, but I'm trying to reduce the number of things imported into the KoreBuild context and push as much down into the project-context as possible.
By the way, variables use the $ sign - $(ArtifactsDir). Percent sign %( means batch an item group.
There was a problem hiding this comment.
Got it. And, I apologize for my fumble with the %-sign. I'll edit my comment 😈
| @@ -8,10 +8,10 @@ | |||
| By default, all projects which produce packages are not intended to ship to NuGet.org as a product package. | |||
| Packages which are intended to ship to NuGet.org must opt-in by setting this to true in the project file. | |||
| --> | |||
| <IsProductPackage Condition=" '$(IsProductPackage)' == '' ">false</IsProductPackage> | |||
| <IsShippingPackage Condition=" '$(IsShippingPackage)' == '' ">false</IsShippingPackage> | |||
There was a problem hiding this comment.
Suggest moving these properties to one file and importing that from here, the root Directory.Build.targets file and anywhere else they're needed
There was a problem hiding this comment.
Yup, that will happen as a part of #4246.
There was a problem hiding this comment.
src/Templating/ still builds as if it was a git submodule in its own repo. This file doesn't import the parent Directory.Build.targets file.
There was a problem hiding this comment.
I understand that. But, it could import a new centralized file that's used elsewhere. Then again, I get that the .targets duplication is only between two files and that'll go away when Templating does import the root Directory.Build.targets file
There was a problem hiding this comment.
I tried doing that a while ago, but it made things more complicated. There are too many assumptions in both src/Templating/ and the repo-root D.B.targets which conflict. It was easier to keep these two projects in separate scopes with a little code duplication while we work on finishing up #4246.
Changes:
Mark the following packages as shipping. They are listed in artifacts.props, but are missing the right setting in .csproj
- Microsoft.AspNetCore.Mvc.NewtonsoftJsonalready fixed in Put Mvc.NewtonsoftJson in the right section #6842This repo will be converting soon to use Arcade. This updates the layout the artifacts folder, and uses Arcade property names