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

[Vcxproj] Improve NuGet handling in packages.config mode & add necessary logic for package reference mode #399

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

zyzyz
Copy link

@zyzyz zyzyz commented Jan 16, 2025

1. Improve .targets import logic when using NuGet packages.config mode

Old Behaviour: /build/native/*.targets and /build/*.targets are both imported.

Problem: in common, /build/native/*.targets for C++ project is firstly chosen if presented, old logic will make /build/*.targets also be imported, and in before of /build/native/*.targets, causing troubles.
e.g. Microsoft.WindowsAppSDK

Current Behaviour: /build/native/*.targets import is made before /build/*.targets , it will check if /build/native/*.targets
is not presented, then do the /build/*.targets import.


2. Add support codes for NuGet package reference mode in .vcxproj

The current NuGet support for C++ project is incomplete.

here and NuGet/NuGet.Client#3145 have shown the way to force package reference mode for .vcxproj.
To achieve this in Sharpmake, it requires

  1. A custom configuration of Directory.Build.props
  2. <PackageReference> element generation support inside Sharpmake

This PR contains the 2. part.

I haven't figure out the best way to automatically handle 1., but since Directory.Build.props is originally a thing that users manually add to their projects, so it should be no harm to let them also add that custom configuration manually (for now?)

P.S. the default NuGet mode for C++ project still keeps as packages.config.

- added 6 commits January 5, 2025 18:36
@zyzyz zyzyz mentioned this pull request Jan 16, 2025
@jspelletier
Copy link
Collaborator

Hi
We've started to check this PR. We ran regression tests on our big engines and it doesn't change anything(we don't use the features that this PR changes).

The next step is we need some samples using this if you ever want this to land eventually.

Thanks.

@jspelletier
Copy link
Collaborator

jspelletier commented Feb 7, 2025

It looks like you need to update some samples reference files(see CI failure)

@zyzyz
Copy link
Author

zyzyz commented Feb 15, 2025

Hi,
I have updated the reference files in sample PackageReferences, and add a new sample CPPForcePackageReference.

Run .\RunSample.ps1 CPPForcePackageReference -framework "net6.0" -VsVersionSuffix "vs2022" -configuration "debug" -os "windows-2022" in local environment emit expected output for me.

Yet the doc about what needs to be done in order to add a new sample is fairly unclear to me. There is a Samples.md but it is not enough, many other things are not mentioned like how to properly make reference files, how to update launchSettings.json, etc.

Currently the new sample involves a copy action of Directory.Build.props file from codebase folder to project file location. I don't know if it is necessary to also include that copied one into reference folder for your CI to work in this specific case.

P.S. there is one thing needs to be beware of: in SamplesDef.json the target of ./RunProcess.ps1 -exeToRun is msbuild not nuget, because Microsoft deliberately prevents nuget.exe from working with such case, msbuild must be used.

@jspelletier
Copy link
Collaborator

you need to edit UpdateSamplesOutput.bat and run it. this will generate new references files.

@jspelletier
Copy link
Collaborator

the changes looks fine but the CI fails possibly due to the fact that you need to regenerate references(execute updatesamplesoutput.bat after edit)

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

Successfully merging this pull request may close these issues.

2 participants