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

Update referenced projects during a run of NuGetUpdater. #9097

Merged
merged 6 commits into from
Feb 23, 2024

Conversation

JoeRobich
Copy link
Collaborator

There was an assumption being made in the Ruby side of the NuGet updater that runs of the native updater were updating the target project and referenced projects. The ruby side would then mark the referenced projects as processed and not attempt to update them separately. Whereas this assumption was true for dirs.proj files it was not true for .csproj, .vbproj, etc... This PR bring the two different ways of processing into alignment and makes the assumption true for all project files.

  • Changed native NuGetUpdater to test against project files instead of always wrapping them with a solution. This brings the tests in line with how the ruby side invokes the native updater. A later PR will add test coverage around updating solutions.
  • Renamed the dirs.proj unit tests to align it with the other UpdateWorkerTests.
  • Refactors the SdkPackageUpdater by extracting logic into methods.

@JoeRobich JoeRobich requested a review from a team as a code owner February 22, 2024 09:08
@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Feb 22, 2024
@JoeRobich JoeRobich force-pushed the joerobich/updateReferencedProjects branch from 764cfc1 to 7eacd94 Compare February 22, 2024 09:10

// update all dependencies, including transitive
var buildFiles = await MSBuildHelper.LoadBuildFiles(repoRootPath, projectPath);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The changes in this file are mostly just grouping logic into methods to make this more readable.

var projectRootElement = ProjectRootElement.Open(projFilePath);
projectStack.Push((Path.GetFullPath(Path.GetDirectoryName(projFilePath)!), projectRootElement));
}
catch (InvalidProjectFileException)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed to guard against invalid projects in this method because we are now looking for project paths before processing the project.

Comment on lines +114 to +120
var projectFilePaths = MSBuildHelper.GetProjectPathsFromProject(projectPath);
foreach (var projectFullPath in projectFilePaths.Concat([projectPath]))
{
// If there is some MSBuild logic that needs to run to fully resolve the path skip the project
if (File.Exists(projectFullPath))
{
await RunUpdaterAsync(repoRootPath, projectFullPath, dependencyName, previousDependencyVersion, newDependencyVersion, isTransitive);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the heart of the change. We now collect referenced projects and update them as we do for dirs.proj.

@Nishnha
Copy link
Member

Nishnha commented Feb 23, 2024

Hi I'll merge this once CI is passing

@Nishnha Nishnha merged commit 4229759 into main Feb 23, 2024
67 checks passed
@Nishnha Nishnha deleted the joerobich/updateReferencedProjects branch February 23, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dotnet:nuget NuGet packages via nuget or dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants