Skip to content

Commit 6a9e068

Browse files
Misc improvements to workflow documentation (#816)
* Misc improvements to workflow documentation * Shorten line lengths * Improve the guidance for regenerating existing packages
1 parent 3311314 commit 6a9e068

File tree

2 files changed

+40
-33
lines changed

2 files changed

+40
-33
lines changed

README.md

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ to time in issues and some documentation.
1717

1818
## Supported Platforms
1919

20-
.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.
20+
.NET source build currently only supports Linux but generating a source-build reference or text-only package
21+
is supported on both Windows and Unix based operating systems.
2122

2223
## Building
2324

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

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

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

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

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

5754
#### Workflow
5855

5956
* Generate reference package and its depencencies running the `./generate.sh --package <package>,<version>` script.
60-
* Revert changes for packages that were already existed in the repository.
61-
* 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)
57+
* Inspect any changes to packages that already existed in the repository. There are two reasons why previously
58+
generated packages show changes when being regenerated.
59+
1. The package contains intentional code modifications on top of the generated code. This may be upgrading a
60+
project reference to address a CVE or code fixups because the generate tooling does not support a scenario.
61+
When this occurs, there should be code comments explaining why the code modification was made. If this is
62+
the case, the changes to the existing package should be reverted.
63+
2. The generate tooling has changed since the last time this package was generated. The new changes should
64+
be considered better/correct and should be committed.
65+
* Add `DependencyPackageProjects` for all new projects in the
66+
[eng/Build.props](https://github.com/dotnet/source-build-reference-packages/blob/main/eng/Build.props#L9)
6267
in the correct dependency order.
6368
* Run build with the `./build.sh -sb` command.
64-
* If the compilation produces numerous compilation issue - run the `./build.sh --projects <path to .csproj file>` command for each
65-
generated reference package separately. It may be necessary to manually tweak the code to address compilation issues. When this occurs,
66-
please ensure there is an [tracking issue](#filing-issues) to address the underlying problem with the generator.
69+
* If the compilation produces numerous compilation issue - run the `./build.sh --projects <path to .csproj file>`
70+
command for each generated reference package separately. It may be necessary to manually tweak the code to
71+
address compilation issues. When this occurs, please ensure there is an [tracking issue](#filing-issues) to
72+
address the underlying problem with the generator.
73+
* Add comments calling out any modifications to the generated code that were necessary.
6774

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

7077
### Targeting
7178

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

7582
### Text Only
7683

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

8188
## Vulnerable Packages
8289

83-
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
84-
packages should be upgraded. A comment should be added to indicate when packages were manually upgraded.
90+
CVEs may exist for reference packages included in this repo. If they are mitigated by a newer version, the
91+
newer version should be added, the vulnerable version should be removed, and references to the vulnerable
92+
package within other reference packages should be upgraded. A comment should be added to indicate when
93+
packages were manually upgraded.
8594

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

96105
## Cleanup
97106

98-
Periodically, a query is ran in a source-built environment to detect unused reference packages. These unreferenced packages
99-
will be deleted.
107+
Periodically, a query is ran in a source-built environment to detect unused reference packages. These
108+
unreferenced packages will be deleted.
100109

101110
## License
102111

eng/Build.props

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,14 @@
1010

1111
<ItemGroup Condition="'$(GeneratePackageSource)' != 'true' and '$(Test)' != 'true'">
1212
<!--
13-
The following DependencyPackageProjects are ones on which other packages depend on that do
14-
not exist in the source-build package cache. Adding them to this ItemGroup will ensure that
15-
they get built first and in order of inclusion. Also, packages included here will be added
16-
to the source-build package cache when building with source-build to allow them to be
17-
considered in prebuilt reporting.
18-
19-
All newly added packages should be grouped together at the end of the list. The reason this is
20-
important is that when previous source-built artifacts are updated, the entries for the new projects
21-
it includes can be removed. New PRs may miss that cut and if they are not grouped at the bottom,
22-
this management becomes more difficult.
13+
All new projects must be added to DependencyPackageProjects. This will ensure that they get built first
14+
and in order of inclusion. The resulting packages from these projects will get added the source-build
15+
package cache when building with source-build to prevent prebuilts.
16+
17+
All newly added packages should be grouped together at the end of the list separated by empty lines.
18+
The reason this is important is that when previous source-built artifacts are updated, the entries
19+
for the new projects it includes can be removed. New PRs may miss that cut and if they are not grouped
20+
at the bottom, this management becomes more difficult.
2321
2422
Format:
2523
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\Microsoft.Extensions.Options.5.0.0.csproj" />

0 commit comments

Comments
 (0)