search

NuGet / Home

Repo for NuGet Client issues
Other
1.49k stars 249 forks source link

NU5129 is not so clever to detect a file renamed by the nuspec #10162

Open ncook-hxgn opened 3 years ago

ncook-hxgn commented 3 years ago

This warning confused me because it is straight up gaslighting me.

WARNING: NU5129: - At least one .props file was found in 'build/native/include/', but 'build/native/BCGControlBarPro-Win32.props' was not.

Except, it was: image

    <file src="build\BCGControlBarPro.props" target="build\BCGControlBarPro-Win32.props"/>
    <file src="build\BCGControlBarPro.targets" target="build\BCGControlBarPro-Win32.targets"/>

Could you please either get nuget.exe to acknowledge my nuspec has renamed the file appropriately, or make build\BCGControlBarPro-Win32.props a folder, not a file, so that the warning is accurate?

The docs say I'm doing the right thing

Ta.

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

zivkan commented 3 years ago

This is a product issue, not a docs issue. Moving to NuGet/Home.

donnie-msft commented 3 years ago

Looks like some validation problem, perhaps in UpholdBuildConventionRule.cs ? @ncook-hxgn Which version of nuget.exe are you using? Ideally, fill out our Issue Template: https://github.com/NuGet/Home/issues/new /cc @zkat

ncook-hxgn commented 3 years ago

@donnie-msft We're often using the latest version. Back when I created this? I'm not sure, but likely 5.x though.

I update nuget about once per month, and the first thing I do when nuget starts appearing to misbehave is nuget update -self.

zivkan commented 3 years ago

@ncook-hxgn I'm not sure why NuGet ever used build/<package_id>.props, but in general we recommend using build/<tfm>/<package_id>.props. In your case <tfm> is native. This matches the path that the warning message NuGet generated is reporting. This should also prevent your package being being installable into .NET projects, whereas currently your package advertises itself as being compatible with all target frameworks, as that's what build/ with no TFM means. This will become more useful as NuGet starts to implement "search by TFM".

Alternatively, if you rename your build/native/ folder to something that is not a folder name that NuGet recognises as a supported framework, I would expect this warning to disappear as well.

So, in this case, I believe the validation rule is working as designed. This validation was created to help package authors detect when they have props/targets in the package that are not being imported, because it's not correctly following NuGet's naming convention.

By (accidentally?) creating a subdirectory under build/ that happens to match a supported target framework (native), NuGet can't tell the difference between:

I wish I noticed this back in October, but I no longer think there's a bug in the validation rule.

ncook-hxgn commented 3 years ago

@zivkan indeed I've been creating a lot of native recently.. I've built ~60 like this 😁 (BCGControlBarPro is special though, in that it's the only source so far with a .props file in a place that confuses nuget.. don't ask me why it's there).

I guess I missed this rule/guideline changing.. when did nuget move from recommending build/<package_id>.props to build/<tfm>/<package_id>.props?

So, what nuget is trying to say is "Hey, I found something in the <tfm> folder: so if this is <tfm>, why isn't your <package_id>.props a <tfm>/<package_id>.props?!".

Either way, what it's actually telling me is that is that there is a problem/something-to-be-aware-of because build/native/bcgcontrolbarpro-win32.props is not in the build/native/include folder: If I just copy that file there "to fix the warning", I'll not be helping anyone I suspect. I think the warning is a bit misleading in that respect.