Skip to content

Misc improvements to workflow documentation #816

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Nov 2, 2023
Merged

Misc improvements to workflow documentation #816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4cf5a5
Misc improvements to workflow documentation
MichaelSimons Nov 1, 2023
bc93bbf
Shorten line lengths
MichaelSimons Nov 1, 2023
d1e5e53
Improve the guidance for regenerating existing packages
MichaelSimons Nov 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 32 additions & 23 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ to time in issues and some documentation.

## Supported Platforms

.NET source build currently only supports Linux but generating a source-build reference or text-only package is supported on both Windows and Unix based operating systems.
.NET source build currently only supports Linux but generating a source-build reference or text-only package
is supported on both Windows and Unix based operating systems.

## Building

Expand All @@ -30,8 +31,8 @@ to time in issues and some documentation.
New packages are needed from time to time as
[existing dependency versions are upgraded](https://github.com/dotnet/source-build/blob/main/Documentation/sourcebuild-in-repos/update-dependencies.md)
and [new dependencies are added](https://github.com/dotnet/source-build/blob/main/Documentation/sourcebuild-in-repos/new-dependencies.md)
to .NET. The [generate script](https://github.com/dotnet/source-build-reference-packages/blob/main/generate.sh) supports
generating new packages. Run `generate.sh --help` for usage details.
to .NET. The [generate script](https://github.com/dotnet/source-build-reference-packages/blob/main/generate.sh)
supports generating new packages. Run `generate.sh --help` for usage details.

When generating a package(s), the tooling will detect and generate all dependent packages.

Expand All @@ -43,34 +44,40 @@ When generating a package(s), the tooling will detect and generate all dependent

After generating new reference packages, all new projects must be referenced as a
[DependencyPackageProjects](https://github.com/dotnet/source-build-reference-packages/blob/main/eng/Build.props#L9).
These must be defined in dependency order. There is a [tracking issue](https://github.com/dotnet/source-build/issues/1690)
to address this manual step.
These must be defined in dependency order. There is a
[tracking issue](https://github.com/dotnet/source-build/issues/1690) to address this manual step.

The tooling does not handle all situations and sometimes the generated code will need manual tweeks to get it to compile.
If this occurs when generating a newer version of an existing package, it can be helpful to regenerate the older version
to see what changes were made.

The tooling has evolved over time and therefore when generating new packages, you may see edits made to existing packages.
This is because the new package has a dependency on an existing package, it was regenerated and changes were detected from
when it was originally generated.
The tooling does not handle all situations and sometimes the generated code will need manual tweeks to get
it to compile. If this occurs when generating a newer version of an existing package, it can be helpful to
regenerate the older version to see what customizations to the generated code were made.

#### Workflow

* Generate reference package and its depencencies running the `./generate.sh --package <package>,<version>` script.
* Revert changes for packages that were already existed in the repository.
* Add `DependencyPackageProjects` for all new projects in the [eng/Build.props](https://github.com/dotnet/source-build-reference-packages/blob/main/eng/Build.props#L9)
* Inspect any changes to packages that already existed in the repository. There are two reasons why previously
generated packages show changes when being regenerated.
1. The package contains intentional code modifications on top of the generated code. This may be upgrading a
project reference to address a CVE or code fixups because the generate tooling does not support a scenario.
When this occurs, there should be code comments explaining why the code modification was made. If this is
the case, the changes to the existing package should be reverted.
2. The generate tooling has changed since the last time this package was generated. The new changes should
be considered better/correct and should be committed.
Comment on lines +59 to +64
Copy link
Member

Choose a reason for hiding this comment

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

This may end up being difficult for newbies to differentiate. We should strive to add comments to the source whenever making manual modifications so they are easy to spot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Hopefully the last step I added will help ensure comments are added.

* Add `DependencyPackageProjects` for all new projects in the
[eng/Build.props](https://github.com/dotnet/source-build-reference-packages/blob/main/eng/Build.props#L9)
in the correct dependency order.
* Run build with the `./build.sh -sb` command.
* If the compilation produces numerous compilation issue - run the `./build.sh --projects <path to .csproj file>` command for each
generated reference package separately. It may be necessary to manually tweak the code to address compilation issues. When this occurs,
please ensure there is an [tracking issue](#filing-issues) to address the underlying problem with the generator.
* If the compilation produces numerous compilation issue - run the `./build.sh --projects <path to .csproj file>`
command for each generated reference package separately. It may be necessary to manually tweak the code to
address compilation issues. When this occurs, please ensure there is an [tracking issue](#filing-issues) to
address the underlying problem with the generator.
* Add comments calling out any modifications to the generated code that were necessary.

You can search for known issues in the [Known Generator Issues Markdown file](docs/known_generator_issues.md).

### Targeting

Generating new targeting packages is not supported. No new targeting packs should be needed/added. If you feel a new
targeting pack is needed, please [open a new issue](#filing-issues) to discuss.
Generating new targeting packages is not supported. No new targeting packs should be needed/added. If you feel
a new targeting pack is needed, please [open a new issue](#filing-issues) to discuss.

### Text Only

Expand All @@ -80,8 +87,10 @@ targeting pack is needed, please [open a new issue](#filing-issues) to discuss.

## Vulnerable Packages

CVEs may exist for reference packages included in this repo. If they are mitigated by a newer version, the newer version should be added, the vulnerable version should be removed, and references to the vulnerable package within other reference
packages should be upgraded. A comment should be added to indicate when packages were manually upgraded.
CVEs may exist for reference packages included in this repo. If they are mitigated by a newer version, the
newer version should be added, the vulnerable version should be removed, and references to the vulnerable
package within other reference packages should be upgraded. A comment should be added to indicate when
packages were manually upgraded.

``` xml
<!-- Manually updated version from 4.3.0 to address CVE-2017-0247 -->
Expand All @@ -95,8 +104,8 @@ This repo does not accept issues. Please file issues in the

## Cleanup

Periodically, a query is ran in a source-built environment to detect unused reference packages. These unreferenced packages
will be deleted.
Periodically, a query is ran in a source-built environment to detect unused reference packages. These
unreferenced packages will be deleted.

## License

Expand Down
16 changes: 7 additions & 9 deletions eng/Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@

<ItemGroup Condition="'$(GeneratePackageSource)' != 'true' and '$(Test)' != 'true'">
<!--
The following DependencyPackageProjects are ones on which other packages depend on that do
not exist in the source-build package cache. Adding them to this ItemGroup will ensure that
they get built first and in order of inclusion. Also, packages included here will be added
to the source-build package cache when building with source-build to allow them to be
considered in prebuilt reporting.
All new projects must be added to DependencyPackageProjects. This will ensure that they get built first
and in order of inclusion. The resulting packages from these projects will get added the source-build
package cache when building with source-build to prevent prebuilts.

All newly added packages should be grouped together at the end of the list. The reason this is
important is that when previous source-built artifacts are updated, the entries for the new projects
it includes can be removed. New PRs may miss that cut and if they are not grouped at the bottom,
this management becomes more difficult.
All newly added packages should be grouped together at the end of the list separated by empty lines.
The reason this is important is that when previous source-built artifacts are updated, the entries
for the new projects it includes can be removed. New PRs may miss that cut and if they are not grouped
at the bottom, this management becomes more difficult.

Format:
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\Microsoft.Extensions.Options.5.0.0.csproj" />
Expand Down