Bugfix/correct support packages #980
Conversation
Improves the packing of the core project to correctly embed the additional utils
|
(I know this is a draft, so FYI) I had difficulties to get the symbol package (snupkg) working with the custom plugin. See also the validate nuget packages job: {
"ErrorCode": 111,
"Message": "Symbol file not found",
"HelpText": null,
"FileName": "lib/netstandard2.0/Reqnroll.Utils.dll"
},
{
"ErrorCode": 111,
"Message": "Symbol file not found",
"HelpText": null,
"FileName": "lib/netstandard2.0/Reqnroll.Parser.dll"
},
{
"ErrorCode": 111,
"Message": "Symbol file not found",
"HelpText": null,
"FileName": "lib/netstandard2.0/Reqnroll.Generator.dll"
}, |
|
The custom plugin package breaks every convention around .NET projects. It doesn't even include its own assembly as package content, which breaks the support for including debug symbols. |
|
I have fudged a solution which includes an extra empty .pdb in the symbols package and I don't think we're going to get better than that without a lot more work. The Reqnroll.CustomPlugin project is a real oddity that we might want to have a longer look at and consider ways to bring it more into alignment with the .NET build system. |
Agree on this. I had the same experience. Maybe we should move back to a nuspec file for this until we refactored the project? (V4 or V5) |
|
I did consider returning to using a nuspec file for this project, but it would also require restoring a lot of property-propagation that was otherwise redundant. I also haven't yet found a way to create a symbols package using a spec file (though I feel like such a thing must exist) |
|
What kind of property-propagation do you mean? |
There was logic which passed MSBuild properties into the NuSpec file so that it would have correct values for things like author and repository URL. |
|
So an option is to hardcode them (temporary) in the nuspec file I think? |
|
Absolutely a viable alternative to what I've built. I encourage a robust examination of the packages being produced and the custom targets that twist our projects into the desired shape. |
|
I did some detail comparison between the package contents. @Code-Grump Please comment if these changes are OK/intended. For the first look, all of these seem to be OK for me, It's not a problem, but inconsistency: the main package has the 3rd party licenses in root, but the "CustomPlugin" has it in a "Licenses" subfolder. (I don't think we need the licenses in "CustomPlugin" at all, as it depends on the main package anyway.) Main package
Comparing to v2.4.1:
Plugin packages (tested: MsTest, TUnit)
Comparing to v2.4.1:
SpecFlowCompatibility
Comparing to v2.4.1:
CustomPlugin
Comparing to v2.4.1:
|
|
exploratory tests passed |
gasparnagy
left a comment
gasparnagy
left a comment
There was a problem hiding this comment.
I did a bit more testing and it seems to work. Thx!
|
Thanks for the review @gasparnagy ; the feedback is appreciated. One of my goals was to recreate the CustomPlugin package as closely as possible to its existing form. As mentioned earlier, it doesn't really follow .NET conventions at all, so it didn't seem wrong to include all the content as before. Removing those Licenses to keep the package size down would be as simple as removing an include from the definition. Let me try to address the differences as best that I can.
PDBs are generally moved into the accompanying symbols package (snupkg) - the IDE will download them independently when debug symbols are being loaded.
This file should have been present as there's a dependency here.
PDB moved to symbols package
Now using the built-in NuGet licenses format, further cutting down on shipped package sizes.
This is following general NuGet package conventions.
These dependencies are declared by the project and should be present in the package definition. Appreciate this PR has been merged, so if there's anything unsatisfactory here, I'm still happy to open a PR to fix.
This files need to be present for modern MSBuild to resolve dependencies properly.
All PDBs have been moved to the symbols package where appropriate.
I can look into this further, but I believe these files are still sensible.
These files were totally missed in the switch to project-based packing. They have been restored to their pre-upgrade structure.
This, I discovered, is a dependency of our compatibility layer that wraps up some legacy configuration-handling. It looks like this feature will not have worked in prior versions of the package due to this missing dependency. I chose to include it in as it was resolved by the updated build system.
Dependencies file included for modern MSBuild support.
Symbols moved to symbols package.
The analysis tool might have made a mistake here: no file has been renamed, but the additional dependency has been included.
This is another declared dependency that wasn't present in the package. If it isn't actually required, we should track down where it's coming from and prune it.
I believe these changes are as expected, but I can look into it further if desired.
Switched to NuGet built-in licenses support.
We don't want to include analyzers transitively.
Switched back to the previous package structure.
Restored all the bundled dependencies that were missed by the migration.
This follows the previous version of the package layout.
Restored dependency that had been dropped in the migration.
Moved symbols to the symbols package.
Switched to NuGet built-in licenses support.
This dependency made little sense, as the Reqnroll dependency provides it.
We don't want to include analyzers transitively. |
|
Appreciate this PR has been merged, so if there's anything unsatisfactory, let's open an issue and I'll do further work. |
No this is all good I think. I have created #989 for removing the Licenses folder from the CustomPlugin package. |
🤔 What's changed?
Reconfigure project packing to handle more complex scenarios required for:
⚡️ What's your motivation?
Fixes #972 and #970
🏷️ What kind of change is this?
This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.