GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-17 05:11:13 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,803 Commits 101 Branches 105 Tags
master
Commit Graph

2 Commits

Author SHA1 Message Date
Marc Schier 00664404c0 Ship the OPC Foundation license file from every package and fix the NU5033 pack failure (#4174) 
# Description

The `preview-pack` stage of the Azure DevOps pipeline fails with nine `NU5033` errors, which fails the whole build:

```
NuGet.Build.Tasks.Pack.targets(222,5): error NU5033: Invalid metadata.
Cannot specify both a PackageLicenseExpression and a PackageLicenseFile.
[src\Opc.Ua.Robotics\Opc.Ua.Robotics.csproj]
```

`common.props` gives every package the shared `LICENSE.txt` as its `<PackageLicenseFile>`, but **19** packages also declared `<PackageLicenseExpression>MIT</PackageLicenseExpression>`. Nine of them hit `NU5033`; the other ten only avoided it by carrying an empty `<PackageLicenseFile></PackageLicenseFile>` workaround.

That declaration was wrong on two counts.

**It advertised the wrong license.** `LICENSE.txt` is the **OPC Foundation MIT License 1.00**, which carries the OPC Foundation copyright and preamble — it is a different text than the plain SPDX `MIT` license. The affected packages therefore published `<license type="expression">MIT</license>` plus a `licenses.nuget.org/MIT` URL on nuget.org, instead of the OPC Foundation text that every other package in the stack ships.

**It also broke the build,** because NuGet refuses to pack a project carrying both a license expression and a license file, failing `pack` solution-wide rather than just for the offending projects.

## Changes

The expression is dropped from all 19 packages so every one uses the shared OPC Foundation license file uniformly, like the rest of the stack:

* `src/Opc.Ua.OpenUsd`, `.Client`, `.Server`
* `src/Opc.Ua.OpenUsdScene`, `.Server`
* `src/Opc.Ua.Robotics`, `.Client`, `.Server`
* `src/Opc.Ua.Positioning`, `.Client`, `.Server`
* `src/Opc.Ua.Di`, `src/Opc.Ua.ISA95`, `src/Opc.Ua.WotCon`, `src/Opc.Ua.Gds.Common`
* `tools/Opc.Ua.OpenUsd.Connector.Viewer`, `tools/Opc.Ua.SourceGeneration.Core`
* `tools/Opc.Ua.SourceGeneration.Pack`, `tools/Opc.Ua.SourceGeneration.Stack.Pack` — these two set the license not in their own `.csproj` but in the shared `tools/SourceGeneratorPack.targets`, so they were only caught once the new guard ran in CI.

The eight per-project empty `<PackageLicenseFile></PackageLicenseFile>` workarounds go away with it.

To stop the whole class of failure from recurring, `Directory.Build.targets` now *enforces* the rule rather than papering over it. Silently clearing the inherited license file would have fixed the build while still publishing the wrong license metadata, so instead a violation fails fast with an actionable error naming the project:

```
error OPCUA0001: Opc.Ua.Foo sets PackageLicenseExpression=MIT. Every package must
ship the OPC Foundation MIT License 1.00 via the shared LICENSE.txt that
common.props sets as PackageLicenseFile, which the SPDX expression neither
matches nor is allowed to coexist with (NU5033). Remove the
PackageLicenseExpression property.
```

`common.props` cannot catch this itself: it is imported from `Directory.Build.props`, i.e. **before** the project body, so it never observes a project-level license expression. `Directory.Build.targets` is imported **after** the project body and is the earliest place that can. The `common.props` comment that previously hinted the expression was an option now points at this rule.

### Verification

* Every packable project evaluates to `PackageLicenseFile=LICENSE.txt` with an empty `PackageLicenseExpression`; file-licensed packages such as `Opc.Ua.Client` are unchanged.
* `dotnet pack -c Release` succeeds for `Opc.Ua.Robotics` (previously failing) and `Opc.Ua.SourceGeneration.Core` (previously relying on the removed workaround).
* Replicated the CI build commands locally — `dotnet build` of `UA.slnx` (net48/Debug), `tools/SourceGeneration.slnx` (net472/Release) and `tools/MigrationAnalyzer.slnx` (net48/Debug) all succeed with 0 errors.
* Inspected the produced `.nupkg`: now `<license type="file">LICENSE.txt</license>` with the license text at the package root, matching the rest of the stack (it was `<license type="expression">MIT</license>` before).
* Confirmed the guard fires on a simulated violation (`-p:PackageLicenseExpression=MIT`) and is inert otherwise.

## Related Issues

No tracking issue — this fixes the `NU5033` failure in the OPC Foundation Azure DevOps pipeline (build 16247, `preview-pack` stage) and corrects the license metadata published for the affected packages. No product code or public API changes.

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._

- [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc.
- [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. _(Build-infrastructure change with no code surface. Validated by `dotnet pack` of the affected projects, by inspecting the resulting package metadata, and by confirming the new `OPCUA0001` guard fires on a simulated violation. The pipeline `preview-pack` stage is the regression test.)_
- [x] I have added all necessary documentation. _(The rule is documented in place, on the new `Directory.Build.targets` guard and at the `PackageLicenseFile` definition in `common.props`.)_
- [x] I have verified that my changes do not introduce (new) build or analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.
 2026-08-04 18:49:50 +00:00
Marc Schier 76d33cb139 Remove SourceGenerator.Foundations, ship analyzers under Roslyn-versioned folders for VS2022 and VS2026, and validate the packages (#4120) 
# Description

The two OPC UA source generators were built on
`SourceGenerator.Foundations` (SGF), whose only job was getting the
generator's dependency closure loaded inside the compiler. It does so
with a **process-wide, first-generator-wins `AppDomain.AssemblyResolve`
hook backed by a non-concurrent `Dictionary` mutated from `AssemblyLoad`
callbacks**. Concurrent compilations in a shared `VBCSCompiler` race it,
which is the root cause of the intermittent clean-build failure:

```
CSC : error CS8784: Generator 'StackSourceGeneratorHoist' failed to initialize. ...
Exception was of type 'FileNotFoundException' with message 'Could not load file or assembly
'SourceGenerator.Foundations.Contracts, Version=2.0.14.0, ...'
```

The repository already solves that problem deterministically — #4041
made the NuGet packages ship the full closure under
`analyzers/dotnet/.../cs`, and `Directory.Build.targets` registers the
same closure as `Analyzer` items for in-repo `ProjectReference`
consumers. SGF was therefore pure liability and is removed.

## What changed

**1. SourceGenerator.Foundations removed.** `ModelSourceGenerator` and
`StackSourceGenerator` are plain `[Generator(LanguageNames.CSharp)]
IIncrementalGenerator` implementations again. SGF's implicit crash
handling is replaced by an explicit `SourceGenerator.Guard` that reports
`MODELGEN003` / `STACKGEN003` and rethrows cancellation. Its logger sink
was already dead code and is dropped.
`Opc.Ua.SourceGeneration.Stack.dll` shrank from **18 MB** (embedded
closure) to **29 KB**.

**2. Analyzers now ship under Roslyn-versioned folders.**
`analyzers/dotnet/roslyn4.14/cs` and `analyzers/dotnet/roslyn5.0/cs`
instead of `analyzers/dotnet/cs`, driven by `roslyn.props` at the
repository root, with new `*.Pack` projects owning the package identity.
The versioned folder matters: a host older than the band **skips** the
analyzer instead of loading it and failing inside generator
initialization.

**3. Dependencies flow automatically.** Both delivery paths discover the
payload by globbing the generator's output directory, so adding a
package to a generator's dependency tree requires **no** `.csproj`,
`.targets`, `.nuspec` or validation-script edit. Verified by temporarily
adding a `PackageReference` and watching it appear in the payload with
no other file touched.

## Two Roslyn bands: VS2022 and VS2026

Analyzers ship twice, once per band. The .NET SDK loads the highest band
its compiler supports and **silently skips** anything above it, so a
single-band package leaves every consumer below that band with no
diagnostics and no generated code.

| Band | Built against | Loaded by |
| --- | --- | --- |
| `roslyn4.14` | `Microsoft.CodeAnalysis.CSharp 4.14.0` | Visual Studio
2022 17.14+ / .NET 9 SDK |
| `roslyn5.0` | `Microsoft.CodeAnalysis.CSharp 5.0.0` | Visual Studio
2026 18.0+ / .NET 10 SDK |

Getting this wrong is invisible, which is most of why it took three
attempts. The rule:

> An analyzer must reference the **lowest**
`System.Collections.Immutable` / `System.Reflection.Metadata` across all
hosts it claims to support, and must never ship them. .NET binds a
reference *upward* but never downward.

Every failure mode is a compiler **warning**:

| Diagnostic | Cause |
| --- | --- |
| `CS9057` | Built against a newer Roslyn than the host, so it is
skipped entirely |
| `CS8784` | Loaded, then `MissingMethodException` - a shipped S.C.I
bound a second `ImmutableArray<T>` |
| `CS8032` | Analyzer instance could not be created |

This PR first shipped a `roslyn4.8` band that **never loaded** -
confirmed against a real Roslyn 4.8 compiler:

```
warning CS8784: Generator 'ModelSourceGenerator' failed to initialize ...
MissingMethodException: Method not found:
'IncrementalValueProvider`1<ImmutableArray`1<!!0>>
 IncrementalValueProviderExtensions.Collect(IncrementalValuesProvider`1<!!0>)'
```

That band was removed, and **this is not a regression**: master had the
same defect, pinned to Roslyn 4.14 while referencing
`System.Collections.Immutable 10.0.0.0`, so no 4.x host ever worked
there either.

The band that landed instead is `4.14`, and it is nearly free:
`Microsoft.CodeAnalysis 4.14` depends on the **same S.C.I 9.0.0 as
5.0.0** and its `csc` runs on `net9.0`. Both bands therefore share
`$(RoslynRuntimeVersion)`, so `Opc.Ua.Types` and
`Opc.Ua.SourceGeneration.Core` are compiled **once** and serve both;
only the five Roslyn-referencing assemblies get a second build, each
from a 12-line project that glob-links the primary sources and overrides
`RoslynApiVersion`. Going lower still - 4.8 wants S.C.I 7.x - would mean
rebuilding that whole closure, `Opc.Ua.Types` included, and remains out
of scope. (`System.Collections.Immutable 7.0.0` also does not resolve in
this repository: `NU1603`, 8.0.0 substituted.)

**Nothing else in the repository exercises the down-level payload**, and
since every failure is a warning, a broken band would ship silently -
exactly as the 4.8 one did. So `validate-source-generator-packages.ps1`
now restores a real `Microsoft.Net.Compilers.Toolset 4.14.0`, runs its
`csc.dll` over the packed `roslyn4.14` payload, and asserts **both**
that none of `CS9057`/`CS8784`/`CS8032`/`CS8034` is emitted **and** that
`/reportanalyzer` lists the generator as having executed - silence alone
would also pass if the generator were never handed to the compiler. The
migration analyzer, whose payload comes from a hand-written nuspec
rather than `SourceGeneratorPack.targets`, gets the same check plus a
band/assembly manifest assertion.

To stop the trap recurring, `Microsoft.CodeAnalysis*`,
`System.Collections.Immutable` and `System.Reflection.Metadata` are
excluded from both the package payload and the in-repo analyzer closure,
and asserted against in the validation script. `roslyn.props` and
`docs/DeveloperGuide.md` document what adding a band requires.
## The shipped package set is unchanged

Evaluating `PackageId`/`IsPackable` across every project in
`preview-pack.slnx` gives the **same 47 package ids** on `master` and on
this branch, and a full Release build plus `dotnet pack --no-build`
produces exactly those 47. Only the owning project changed:

| Package | master | this PR |
| --- | --- | --- |
| `…Opc.Ua.SourceGeneration` | `Opc.Ua.SourceGeneration.csproj` |
`Opc.Ua.SourceGeneration.Pack.csproj` |
| `…Opc.Ua.SourceGeneration.Stack` |
`Opc.Ua.SourceGeneration.Stack.csproj` |
`Opc.Ua.SourceGeneration.Stack.Pack.csproj` |

`.azurepipelines/expected-packages.txt` pins the expected set and the
validation script fails when the packed output drifts.

## Two new package checks

**`build/<PackageId>.props` naming.** NuGet only auto-imports that exact
name, and the file declares every `CompilerVisibleProperty` /
`CompilerVisibleItemMetadata` the model generator relies on. It was
previously packed as
`build/OPCFoundation.Opc.Ua.SourceGeneration.props`, which does **not**
match the package id — so **no NuGet consumer ever imported it**, and
every `ModelSourceGenerator*` property and `AdditionalFiles` metadata
was silently invisible. In-repo projects were unaffected because they
import it by relative path.

**An end-to-end consumer that actually drives the generator.** The
pre-existing clean-consumer check only proved the analyzer *loads*.
`Test-SourceGeneratingConsumer` builds a standalone project that feeds a
real NodeSet2 in as an `AdditionalFile`, pins a
`ModelSourceGeneratorPrefix` that cannot be derived from the model, and
references the emitted types from hand-written code. It deliberately
does **not** import the props by path, so the package's auto-import is
what is under test. Both checks were verified to fail against a package
rebuilt with the misnamed props.

## CI: parallelism re-enabled

`buildandtest.yml` pinned three `dotnet build` invocations to
`/p:UseSharedCompilation=false` and `-maxcpucount:1`. `dotnet build`
defaults to all logical processors, so `-maxcpucount:1` actively
disabled parallelism. Both flags were introduced silently in unrelated
PRs (`f358078b1`, `208c601b0`) and suppress exactly the conditions the
SGF race needed — now structurally gone. Compilation is ~72 % of what
that workflow executes (233 of 323 job-minutes in run `29916768976`), so
the queue-independent `Build` step durations are the baseline:

| Measurement | Baseline |
| --- | --- |
| `Build` step, 26 `test-*` jobs | mean 5.53 min, sum 143.9 min |
| `build-linux-all-tfm` | 56.3 min |
| `build-windows-all-tfm` | 32.5 min |

`codeql-analysis.yml` keeps the flag — the CodeQL tracer requires it.

## Incidental fixes

- `Microsoft.CodeAnalysis.CSharp` declares an exact `[version]` range on
`Microsoft.CodeAnalysis.Common`, so with
`CentralPackageTransitivePinningEnabled` the central pins must move in
lockstep or restore fails with `NU1109`. They now derive from
`$(RoslynApiVersionVS2026)`.
- `MigrationGeneratorTests` created its generator driver without parse
options — silently fine under Roslyn 4.14, `Inconsistent language
versions` under 5.0.
- The migration analyzer props injected `<Analyzer>` items by hand from
`analyzers/dotnet/cs`; NuGet registers analyzers itself and selects the
matching folder, so the injection was obsolete and would now force the
wrong assembly to load.
- Exception diagnostics reported `ex.StackTrace` (null for an exception
that never left its throw site, and carrying neither type nor inner
chain) — now `ex.ToString()` at all three call sites.
- The pack guard tested the flattened assembly list, so it only fired
when *no* variant produced output; it now checks each declared variant
contributed assemblies.
- `GeneratePackageOnBuild` is disabled for the generators; packing
happens in CI or on an explicit `dotnet pack`.

## Note for reviewers

**Building this repository requires a Roslyn 5.x host** (the .NET 10 SDK
or Visual Studio 2026). That is already the documented prerequisite and
every CI leg installs `10.0.x` last, but it is now a hard requirement.

## Related Issues

_No tracking issue: this is a follow-up to the CS8784 clean-build
failure reported against #4041, which fixed the packaged delivery path
but left the in-repo project-reference path relying on the SGF
resolver._

- Follow-up to #4041

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

### Verification detail

| Check | Result |
| --- | --- |
| `dotnet build UA.slnx` (clean, and after merging master) | 0 errors;
18 warnings, all pre-existing `CA1873` in `Opc.Ua.Client` |
| `Opc.Ua.SourceGeneration.Tests` | 81/81 on net10.0 and net48 |
| `Opc.Ua.SourceGeneration.Stack.Tests` | 92/92 on net10.0, 89/89 on
net48 |
| `Opc.Ua.SourceGeneration.Core.Tests` | 3722 passed, 8 skipped |
| `Opc.Ua.MigrationAnalyzer.Tests` | 121/121 on net10.0 and net48 |
| Package id set, master vs branch | identical, 47 packages |
| Release build + `dotnet pack --no-build` | 47 packages, matching the
manifest; both `roslyn4.14` and `roslyn5.0` present; no host-provided
assemblies shipped |
| `validate-source-generator-packages.ps1` | passes, including manifest,
props-naming, end-to-end generator consumer and down-level host checks |
| Down-level band under real csc 4.14.0 | all three analyzer packages
load **and run** |
| Negative tests | misnamed props → both checks fail as designed; hidden
variant output → pack guard fires; 5.0 build placed in `roslyn4.14` →
real `CS9057`; non-running generator name → execution assertion fires;
expecting a `roslyn4.8` band → manifest check fires |

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9ed53262-5cfa-429a-820e-bf6146a00d8b
 2026-07-31 15:46:17 +00:00