Skip to content

Update NuGetCommand to use nuget.config #1434

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

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Conversation

manodasanW
Copy link
Member

Previous PR adding nuget.config wasn't enough as a couple nuget restore tasks still used nuget.org. Updating the tasks to use the nuget.config.

@DefaultRyan DefaultRyan merged commit 64d2a52 into master Sep 10, 2024
79 of 82 checks passed
@DefaultRyan DefaultRyan deleted the manodasanw/nugetcommand branch September 10, 2024 21:30
@@ -51,6 +51,8 @@ jobs:
inputs:
command: 'restore'
restoreSolution: '$(Build.SourcesDirectory)\cppwinrt.sln'
feedsToUse: config
Copy link
Member

@walbourn walbourn Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my experience, a better solution is to not have the private ADO feed link in the public repo in a nuget.config.

Instead, delete the nuget.config from the repo, add a pipeline variable set in the ADO interface that points to the feed, and then have in the ADO YAML:

  - task: NuGetCommand@2
    inputs:
      command: 'restore'
      restoreSolution: '$(Build.SourcesDirectory)\cppwinrt.sln'
      feedRestore: $(GUID_FEED)
      includeNuGetOrg: false

The only time I've had to use a NuGet.config for just a source has been doing command - custom to use nuget install.

The 'trick' if there any is that the feed has to be described as GUID or rather two GUIDS with the projectid/feedid.

For the URL checked into nuget.config here, you'd set the GUID_FEED variable to 0e1afd68-1a41-4bd2-9a93-ad91fb9c76d5/2cb8c784-833f-471a-a386-9be37cb4d900

This approach has the benefit of not forcing external developers to use the ADO artifacts feed instead of nuget.org or their own secure feed. The existing implementation causes problems if someone wants to use this repo as a submodule and they are trying to follow the guidance of only using a single feed that isn't the hard-coded one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feed is a public feed hosted on ADO.

Copy link
Member

@walbourn walbourn Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't need to be public, and again if someone submodules this repo that needs other NuGet packages and they follow security guidance, they should be using their own feed and NOT use the ADO feed hard-coded in this repo.

For private repos, checking in the nuget.config with the ADO feed URL is fine. For a public repo, it's not ideal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do have a good point about submodules that will need to be evaluated. And I do agree for public repos, it is not ideal, but the nuget.config is needed as part of one of the current guidance. If you or someone else has a repo that this repo is currently a submodule of, I can look at starting a thread with folks involved with the guidance to get further guidance on how to support it.

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.

3 participants