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 7 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.

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