Skip to content
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.

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

Central Package Management - Default Package Restore Failure #49

Closed
brandon-hurler opened this issue Feb 7, 2023 · 9 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@brandon-hurler
Copy link

When using Central Package Management, the default package restores fail if an explicit PackageVersion does not exist for the dependent packages the SDK is using.

e.g., I have an explicit PackageVersion set for:

  • Microsoft.AspNet.Mvc
  • RazorGenerator.Mvc
  • Microsoft.CodeDom.Providers.DotNetCompilerPlatform

However, I get the error for the two packages the SDK uses that I did not set references for.

  • Microsoft.Net.Compilers.Toolset
  • RazorGenerator.MsBuild

error NU1010: The PackageReference items Microsoft.Net.Compilers.Toolset;RazorGenerator.MsBuild do not have corresponding PackageVersion.

It looks like there needs to be additional checks to see if there is an existing PackageVersion defined for each dependent package.

@CZEMacLeod
Copy link
Owner

@brandon-hurler I'm not quite sure why you think there needs to be an additional check, or what it would do?
The existing error message from CPM is pretty clear as to what is happening/what the problem is, and how to solve it.

The RazorGenerator.MsBuild package is included if you are using the MSBuild.SDK.SystemWeb.RazorLibrary SDK to build razor pages (ensuring it will correctly update any dependent code without having any installed extensions etc. and works in all VS versions and build servers).
The Microsoft.Net.Compilers.Toolset package allows using newer C# language features in said razor pages (and ensures that the same version of the compilers are used on a build server.)

The list of packages is shown in the DefaultPackages file for each SDK - e.g. MSBuild.SDK.SystemWeb.RazorLibrary.DefaultPackages.targets and the default version numbers (if you are not using CPM) are shown in MSBuild.SDK.SystemWeb.RazorLibrary.DefaultPackages.props.

You can set the property ExcludeDefaultRazorPackages to true if you don't want the default packages included (or ExcludeASPNetCompilers in the case of the MSBuild.SDK.SystemWeb SDK)

I guess it might be possible to check if the PackageVersion is defined and if missing set it as per the defaults, but I am not aware of any other SDK or package doing this kind of logic, and I feel it would mean 'magic' was happening inside the SDK that was not clear to the user. e.g. if there were 2 projects referencing different SDK versions, they might end up with different package versions (which kind of negates the point of CPM).

@brandon-hurler
Copy link
Author

If you have CPM enabled but do not have an existing dependency on the packages included in the SDK, then you won't have any version for the project / solution likely at all. Using PackageReference in combination with PackageVersion, you do not define any version on the PackageReference directly.

For that use-case, if someone using CPM were to install the SDK and not already have a dependency on all the packages the SDK would need, it would break the restore.

It's not that I can't fix it on my end, that would be pretty easy to do. But without checking for the PackageVersion as well, the SDK can get out of state with its own dependencies.

@CZEMacLeod
Copy link
Owner

I am aware of how CPM works w.r.t. defining the version numbers (there is an open issue #46 about the new CPM included in nuget vs. the version in Microsoft.Build.CentralPackageVersions which is already supported)

If you add a project using the template for the SDK, then you will need to modify/add the code from that issue at present if you are using the new CPM.

The package restore will fail in this case if any of the packages do not have a central package version defined, correct, however, there is a sensible error message provided and appropriate versions can be added (as a one off for the solution) to your central package versions file.

If you have an example of another SDK that has this additional logic you are talking about, please let me know so I can look at it.

I'm not quite sure what you mean by 'if someone were to install the SDK' - it is not a nuget pacakge you install - see the How can I use these SDKs? section.

Also, what you mean about the SDK getting out of sync with its dependencies? Do you mean, that if it requires a specific minimum version of one of these it could cause a problem? The default versions can be overridden and set if required (for instance to a newer release of the compilers) without a need for a new version of the SDK as it stands, but there are no 'checks' if the version will work as intended in that case either.

I don't believe that any of the packages will fail if they are downgraded, or upgraded as it stands - they are mainly there to quickly add the main references required, in a similar way to the net6+ SDK, such that you don't need any additional package references. They can also be disabled as I said before, if you want to manually add all the package references (and package versions) to you project/solution manually.

If there are any specific checks that would make this easier to use, feel free to create a PR with them, and we can look at merging that (along with the general changes to resolve #46). I think it might just be a case of improving the documentation for the project - again I welcome PRs to flesh out any of the functionality/documentation.

A lot of the SDK is designed around ease of use in the basic case, along with the ability to override functions and setting for an advanced user. I don't think CPM is a basic case, and people using it would know how to resolve the issue - but I could be wrong about that.

brandon-hurler added a commit to brandon-hurler/MSBuild.SDK.SystemWeb that referenced this issue Feb 8, 2023
…ng RazorLibrary to support predefined PackageVersions.
@brandon-hurler
Copy link
Author

I added a PR to address this issue and #46 for the RazorLibrary. Here's what I wanted to address:

  • Added ManagePackageVersionsCentrally to the Razor definitions for CPM and non-CPM groups
  • Add default PackageVersions matching existing dependency versions when using CPM
  • Control package versions
    1. Use ExcludeDefaultRazorPackageVersions if you want to define your own within CPM with Include
    2. Or, update CPM from Include to Update within your solution if you only want to target a single package

@julealgon
Copy link

Is there a reason this is still open? Hasn't it been solved by #54 above?

@CZEMacLeod CZEMacLeod added the enhancement New feature or request label Jul 5, 2023
@CZEMacLeod CZEMacLeod self-assigned this Jul 5, 2023
@CZEMacLeod
Copy link
Owner

@julealgon #54 has been closed without merging as the design and structure of the solution moved quite a way in the discussions.
#62 / #59 has an update for the default version used, and the 'workaround' of defining the versions in the CPM packages file should work.
However, there are no 'default versions' applied when using a CPM system for now.
Once the latest PRs are pulled in, I'll try and build something from the discussion in #56, although, as I pointed out in the comments, I don't actually think this is a bug as such, much more an enhancement, with a fairly limited use case.

@julealgon
Copy link

@julealgon #54 has been closed without merging

Oh I completely missed that fact, my bad.

Thanks for the clarifications @CZEMacLeod . The reason I went over some of these issues now is that we'll potentially begin a substantial migration towards SDK-style projects on our side here and we do need to make sure a few old webforms and MVC Razor projects will not only retain full compatibility but also that we know about any challenges coming up. Central package management is also something I'd like to introduce ASAP.

Will apply the workaround in the meantime but will also keep track of this.

Appreciate your work on this library very much.

@willbush
Copy link

willbush commented Jul 12, 2023

I just ran into an CPM issue with 4.0.88 of this library due to default packages Microsoft.Net.Compilers.Toolset and Microsoft.CodeDom.Providers.DotNetCompilerPlatform getting included. The error said the version was not specified. I worked around the issue by simply setting ExcludeSDKDefaultPackages true.

@CZEMacLeod
Copy link
Owner

@willbush You can either set ExcludeSDKDefaultPackages to true, or include versions for those two packages for now.

<ItemGroup>
  <PackageVersion Include="Microsoft.Net.Compilers.Toolset" Version="$(MicrosoftNetCompilersToolset_Version)" />
  <PackageVersion Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" Version="$(MicrosoftCodeDomProvidersDotNetCompilerPlatform_Version)" />
</ItemGroup>

Should work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants