Dynamically Pack MVVM SourceGen project outputs #220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 tasks
|
Please merge this only after merging #85 as it depends on some changes from that PR. All the 3 PRs are correlated since the changes cascade into the other. |
Nirmal4G
force-pushed
the
feature/dynamic-pack-sourcegen
branch
3 times, most recently
from
f435d10 to
f985d3f
Compare
Nirmal4G
force-pushed
the
feature/dynamic-pack-sourcegen
branch
2 times, most recently
from
db97327 to
ff6f9b2
Compare
Nirmal4G
force-pushed
the
feature/dynamic-pack-sourcegen
branch
5 times, most recently
from
a1962c0 to
556a8b5
Compare
Nirmal4G
force-pushed
the
feature/dynamic-pack-sourcegen
branch
from
556a8b5 to
628c035
Compare
Nirmal4G
force-pushed
the
feature/dynamic-pack-sourcegen
branch
3 times, most recently
from
cf281c0 to
849cc27
Compare
- Rename `build` folder to `eng`:
- This is a standard build infra directory used in official dotnet projects.
- Rename NuGet Icon to `Icon.png`:
- This is no longer used as a public reference point for NuGet icon URL.
- Also, Icon URL is deprecated. Hence, it's safe to change.
- Normalize casing for `ReadMe.md`:
- Repository information files such as ReadMe, License, etc... are only UPPER_CASE
if they are without an extension. With extension, the casing becomes PascalCase
or Kebab-Case. The primary reason is attention to the presentation of file names.
- Do Kebab-Case when a phrase is presented. E.g., `Code-of-Conduct.md`.
- Rename solution file to `CommunityToolkit.sln`:
- The `dotnet` seems implied and also doesn't stand-out in the file list because of the lower casing and `d` char.
- Spaces are a main issue when doing automation (_like using `*.sln` in build scripts and in URLs it adds `%20`_).
- Move `toolkit.snk` file to `eng` sub-directory.
- Remove un-needed and deleted files from solution.
- Update Git Ignore entries to latest from upstream.
- Indent text in `ThirdPartyNotices.txt` with spaces instead.
- Fix the text flow warping in the MSBuild Console logging. - Use checked version properties instead of hard-coded checks. - Update the structure of the projects list in the Solution file. - Refactor Roslyn multi-targeting to use multiple projects in the same project directory without using Shared projects. This refactoring is made in preparation for the solution to use the NuGet's CPVM (Central Package Versions Management) feature. This also slightly improves IDE load time and Build performance.
- Move the T4 MSBuild logic to a new file.
- Move Public Key to a new file 'toolkit.spk'.
- Move MSBuild logic to near-by `Directory.Build.{props|targets}` files.
Follow the following rules: - SOF (_Start Of File_) Imports, - Core properties, Package properties, build properties, - Build items, Resource items, Content items, Misc. items, - In-place Imports, Choose (_elements within follows outer sort order_) - `ProjectReference` items, `PackageReference` items, - Targets and Properties and Items close to said Targets, - EOF (_End Of File_) Imports. Where there's a condition by target properties, we should group them together under `Choose`. For multiple target values, we should sort them by the order in which they are defined. And the order in which they should be defined is either ascending or descending in terms of compatibility layering.
- Add necessary guard to check for pack. - Remove redundant properties and values. - Remove and adjust quotes in property functions. - Use wildcards to generalize and reduce items declared.
- Remove redundant comments. - Add missing comments on certain code blocks. - Format code in comments with proper quote style.
Some property groups have conditions that also self-explain their purpose. So, Add labels to bare property groups only to differentiate among themselves. Then, when contributors add any additional properties, they'll know where to put them.
When the '$(IncludeContentInPack)' property is false, files specified via '@(None)', '@(Content)' items are excluded from the NuGet package. Adding '@(PackageFile)' items directly to '%(_PackageFiles)' item via the following target ensures that they will always be included in the package. So, Use '%(PackageFile)' item to always include files in the package. By default, they are included in the root of the package but can be overridden via '%(PackageFile.TargetPath)' metadata.
Previously, static output path to the MVVM SourceGen assembly was used to pack the MVVM project. This leads to error when OutputPath was updated dynamically when testing or in forks. So, here, we'll update the build so that the SourceGen build outputs will be dynamically packed.
- MSBuild Item update logic within target broke during 17.3-17.4! Previous logic referencing direct Item names worked fine before, but now needs a proxy/temporary item in order to process the includes. Same with the undefined Metadata, previously it returned empty string for items with the metadata undefined but now throws error. This may be correct behavior for items under target but this difference hinders sharing logic within and out of targets. This has a side-effect of needing to specify fully qualified name "%(Item.Metadata)" which makes it verbose.
Nirmal4G
force-pushed
the
feature/dynamic-pack-sourcegen
branch
from
849cc27 to
9b508f8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #219
Changes
Previously, static output path to the MVVM SourceGen assembly was used to pack the MVVM project.
This leads to error when OutputPath was updated dynamically when testing a new feature (see #96).
So, here, we'll update the build so that the SourceGen build outputs will be dynamically packed. We first get the build outputs dynamically via
GetBuildOutputsMSBuild target mirroring NuGet's Pack targets. Then, we add it to the package via_PackageFiles(an internal MSBuild item used by NuGet's Pack targets). This is brittle only if @NuGet team decides to modify/remove_PackageFileswhich is highly unlikely.PR Checklist
featurebranch in your forkNotes
Also, this is brittle only if @NuGet team decides to modify/remove
_PackageFileswhich is highly unlikely.One interesting side-effect of this implementation is that when we decide to multi-target, we can easily modify our solution to be more resilient without having to tackle the problem again since we already output the artifacts by TFM. This should save hours of headbutting against the wall 😉!!
Merge pull request #xxxx from repo/branch, and commit message to either PR message or messages of individual commits. Theauto-mergeoption does this by default.