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.

Sign up for GitHub

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
DefaultRyan merged 1 commit into from
Sep 10, 2024
Merged

Update NuGetCommand to use nuget.config #1434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .pipelines/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ jobs:

- task: NuGetCommand@2
displayName: NuGet restore
inputs:
command: 'restore'
feedsToUse: config
nugetConfigPath: NuGet.config

- task: CmdLine@2
displayName: Build Tools
Expand Down Expand Up @@ -322,6 +326,10 @@ jobs:

- task: NuGetCommand@2
displayName: NuGet restore
inputs:
command: 'restore'
feedsToUse: config
nugetConfigPath: NuGet.config

- task: DownloadPipelineArtifact@1
displayName: Download x86 Artifacts
Expand Down
4 changes: 4 additions & 0 deletions .pipelines/jobs/OneBranchBuild.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ jobs:
inputs:
command: 'restore'
restoreSolution: '$(Build.SourcesDirectory)\cppwinrt.sln'
feedsToUse: config
Copy link
Member

@walbourn walbourn Sep 11, 2024 •
edited
Loading

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

@manodasanW manodasanW Sep 11, 2024

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 •
edited
Loading

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

@manodasanW manodasanW Sep 11, 2024

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.

nugetConfigPath: NuGet.config

- task: VSBuild@1
displayName: Build fast_fwd
Expand All @@ -72,6 +74,8 @@ jobs:
inputs:
command: 'restore'
restoreSolution: '$(Build.SourcesDirectory)\natvis\cppwinrtvisualizer.sln'
feedsToUse: config
nugetConfigPath: NuGet.config

- task: VSBuild@1
displayName: Build Component visualizer
Expand Down
2 changes: 2 additions & 0 deletions .pipelines/jobs/OneBranchTest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ jobs:
inputs:
command: 'restore'
restoreSolution: '$(Build.SourcesDirectory)\cppwinrt.sln'
feedsToUse: config
nugetConfigPath: NuGet.config

- task: PowerShell@2
displayName: Remove cppwinrt dependency from test projects
Expand Down
2 changes: 2 additions & 0 deletions .pipelines/jobs/OneBranchVsix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ jobs:
inputs:
command: 'restore'
restoreSolution: '$(Build.SourcesDirectory)\vsix\vsix.sln'
feedsToUse: config
nugetConfigPath: NuGet.config

- task: DownloadPipelineArtifact@2
displayName: 'Download x86 binaries'
Expand Down