sln-add: --include-references when adding a project #48815

edvilme · 2025-05-05T15:11:11Z

No description provided.

Update option name

Copilot

Pull Request Overview

This PR introduces a new command option (--include-references) to enable adding referenced projects along with a project in a solution.

Added IncludeReferencesOption to the command parser with a default value of true
Modified the command implementation to extract and recursively add project references when the flag is enabled

Reviewed Changes

Copilot reviewed 3 out of 21 changed files in this pull request and generated 2 comments.

File	Description
src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommandParser.cs	Added a new option for including referenced projects
src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs	Captured the new flag in the command and extended AddProject to add referenced projects conditionally

Files not reviewed (18)

src/Cli/dotnet/Commands/CliCommandStrings.resx: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.cs.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.de.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.es.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.fr.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.it.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.ja.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.ko.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.pl.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.pt-BR.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.ru.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.tr.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hans.xlf: Language not supported
src/Cli/dotnet/Commands/xlf/CliCommandStrings.zh-Hant.xlf: Language not supported
test/TestAssets/TestProjects/SlnFileWithReferencedProjects/A/A.csproj: Language not supported
test/TestAssets/TestProjects/SlnFileWithReferencedProjects/App.sln: Language not supported
test/TestAssets/TestProjects/SlnFileWithReferencedProjects/App.slnx: Language not supported
test/TestAssets/TestProjects/SlnFileWithReferencedProjects/B/B.csproj: Language not supported

Copilot · 2025-05-05T21:42:14Z

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs

@@ -202,6 +204,21 @@ private void AddProject(SolutionModel solution, string fullProjectPath, ISolutio
            project.AddProjectConfigurationRule(new ConfigurationRule(BuildDimension.BuildType, solutionBuildType, "*", projectBuildType));
        }

+        // Get referencedprojects from the project instance


[nitpick] Consider updating the comment to 'Get referenced projects from the project instance' for improved clarity.

Suggested change

// Get referencedprojects from the project instance

// Get referenced projects from the project instance

Copilot · 2025-05-05T21:42:14Z

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs

        Reporter.Output.WriteLine(CliStrings.ProjectAddedToTheSolution, solutionRelativeProjectPath);
+
+        if (_includeReferences)


Consider adding cycle detection or duplicate addition checks when recursively adding referenced projects to prevent potential infinite recursion in cases of cyclic project references.

Forgind · 2025-05-05T21:45:23Z

src/Cli/dotnet/Commands/CliCommandStrings.resx

@@ -2483,4 +2483,7 @@ To display a value, specify the corresponding command-line option without provid
  <data name="ZeroTestsRan" xml:space="preserve">
    <value>Zero tests ran</value>
  </data>
-</root>
+  <data name="SolutionAddReferencedProjectsOptionDescription" xml:space="preserve">
+    <value>Recursively add projects' ReferencedProjects to solution</value>


Referring to ReferencedProjects is clear to you but probably opaque to most people

"Recursively add project references to the solution"?

How about "Set to false to disable recursively adding all ProjectReferences"? My initial thought was that ReferencedProjects is an internal implementation detail, but looking now, it'd also be nice to include that the default is 'true'.

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs

Forgind · 2025-05-05T21:46:58Z

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs

+        var referencedProjectsFullPaths = projectInstance.EvaluatedItemElements
+            .Where(item => item.ItemType == "ProjectReference")
+            .Select(item => item.Include)
+            .Select(item => Path.GetFullPath(item, Path.GetDirectoryName(fullProjectPath)))


You don't need to pay attention to Exclude/Remove?

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs

Forgind · 2025-05-06T01:21:57Z

Currently looking exclude/remove handling and skipping already-added projects (per copilot's comment)

edvilme · 2025-05-06T17:31:56Z

Currently looking exclude/remove handling and skipping already-added projects (per copilot's comment)

The current behavior for adding existing projects is to just print a message saying it's been added already. It doesn't fail and in this case I think it should absolutely not fail (i.e., as a user I wouldn't expect it to fail if I'm being a good citizen and doing things "correctly" -- adding all needed projects). However, we could make the output message conditional. I would love to also hear @baronfel 's opinion here.

I will look into how to handle the exclude/remove and see if ProjectInstance::EvaluatedItemElements resolves that, or if it considers globbing.

Forgind · 2025-05-06T19:48:38Z

I agree that it absolutely should not fail; I just hadn't realized it was covered properly. Looks like it is, but it's halfway down the method, so I missed it. Perhaps check earlier so we can skip some extra work? I also would prefer that it not print a message when it's a recursive call, as that would add confusion when users see things that weren't added that they'd never intended to add or that they'd already added. Like:
Successfully added foo to my.sln.
Failed to add foo to my.sln because it is already there.
???

edvilme · 2025-05-06T22:12:33Z

I agree that it absolutely should not fail; I just hadn't realized it was covered properly. Looks like it is, but it's halfway down the method, so I missed it. Perhaps check earlier so we can skip some extra work? I also would prefer that it not print a message when it's a recursive call, as that would add confusion when users see things that weren't added that they'd never intended to add or that they'd already added. Like: Successfully added foo to my.sln. Failed to add foo to my.sln because it is already there. ???

I like the idea of not adding the message if in a recursive call, and how it can be very confusing to users. I will also check for a good way of identifying this beforehand to avoid unnecessary steps

edvilme · 2025-05-06T22:51:12Z

Just noticed this kindof addresses #28781

edvilme · 2025-05-08T21:12:01Z

Currently looking exclude/remove handling and skipping already-added projects (per copilot's comment)

As per offline discussion, by the time you're enumerating individual items with projectInstance.GetItems("ProjectReference") all include/exclude/remove operations are done

Copilot

Pull Request Overview

This pull request adds the --include-references option when adding a project to a solution. The key changes include new project files and solution files for test assets, updated resource strings for localized messages, and modifications to the solution add command parser and implementation to recursively add referenced projects.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

File	Description
test/TestAssets/TestProjects/SlnFileWithReferencedProjects/*	New test projects and solution files to support referenced projects
src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommandParser.cs	Added a new option for including referenced projects
src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs	Updated command implementation to process the new option and recursively add project references
src/Cli/dotnet/Commands/xlf/*, CliCommandStrings.resx	Added localized strings for the new option

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs

Forgind

Just the one tweak for when vs. if, and I think I'm happy 🙂 Thanks for working through this!

github-actions bot added the Area-CLI label May 5, 2025

build-analysis bot mentioned this pull request May 5, 2025

Template engine tests hitting known parallelism issue in GetVisualStudioInstances #44878

Open

edvilme force-pushed the edvilme-sln-add-recursive branch from 563179c to 2f429ad Compare May 5, 2025 17:07

sln-add: Add --include-references option

471d3f2

Update option name

edvilme force-pushed the edvilme-sln-add-recursive branch from 2f429ad to 471d3f2 Compare May 5, 2025 18:37

edvilme added 3 commits May 5, 2025 14:00

Add tests

69a479a

Check for flag too

38c7840

Fix tests

993c28c

edvilme marked this pull request as ready for review May 5, 2025 21:41

Copilot AI review requested due to automatic review settings May 5, 2025 21:41

Copilot AI reviewed May 5, 2025

View reviewed changes

edvilme requested a review from a team May 5, 2025 21:42

Forgind reviewed May 5, 2025

View reviewed changes

Address pr comments

bd001ce

Udpate cli completions

09abcb9

Update tests messages

1576ff5

edvilme added 2 commits May 7, 2025 09:38

Correctly get referencedprojects

34c0d37

Fix tests

266ef2e

edvilme requested review from Copilot, Forgind and a team May 8, 2025 21:12

Copilot AI reviewed May 8, 2025

View reviewed changes

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs Outdated Show resolved Hide resolved

edvilme added 2 commits May 8, 2025 14:15

Add parentheses to condition

f8e64fe

Merge branch 'main' into edvilme-sln-add-recursive

c0b6c3b

This was referenced May 9, 2025

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

Forgind reviewed May 9, 2025

View reviewed changes

src/Cli/dotnet/Commands/Solution/Add/SolutionAddCommand.cs Outdated Show resolved Hide resolved

Forgind approved these changes May 9, 2025

View reviewed changes

Address pr comments

4b810a5

edvilme enabled auto-merge (squash) May 9, 2025 04:14

edvilme merged commit 509db15 into dotnet:main May 9, 2025
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sln-add: --include-references when adding a project #48815

sln-add: --include-references when adding a project #48815

edvilme commented May 5, 2025

Copilot AI left a comment

Copilot AI May 5, 2025

Copilot AI May 5, 2025

Forgind May 5, 2025

edvilme May 5, 2025

Forgind May 6, 2025

Forgind May 5, 2025

Forgind commented May 6, 2025

edvilme commented May 6, 2025 •

edited

Loading

Forgind commented May 6, 2025

edvilme commented May 6, 2025

edvilme commented May 6, 2025

edvilme commented May 8, 2025

Copilot AI left a comment

Forgind left a comment

	// Get referencedprojects from the project instance
	// Get referenced projects from the project instance

		Reporter.Output.WriteLine(CliStrings.ProjectAddedToTheSolution, solutionRelativeProjectPath);

		if (_includeReferences)

sln-add: --include-references when adding a project #48815

sln-add: --include-references when adding a project #48815

Conversation

edvilme commented May 5, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI May 5, 2025

Choose a reason for hiding this comment

Copilot AI May 5, 2025

Choose a reason for hiding this comment

Forgind May 5, 2025

Choose a reason for hiding this comment

edvilme May 5, 2025

Choose a reason for hiding this comment

Forgind May 6, 2025

Choose a reason for hiding this comment

Forgind May 5, 2025

Choose a reason for hiding this comment

Forgind commented May 6, 2025

edvilme commented May 6, 2025 • edited Loading

Forgind commented May 6, 2025

edvilme commented May 6, 2025

edvilme commented May 6, 2025

edvilme commented May 8, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Forgind left a comment

Choose a reason for hiding this comment

edvilme commented May 6, 2025 •

edited

Loading