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

Project.WithProjectReferences and friends allows you to give duplicate references #12101

Closed
jasonmalinowski opened this issue Jun 20, 2016 · 7 comments

Comments

@jasonmalinowski
Copy link
Member

@jasonmalinowski jasonmalinowski commented Jun 20, 2016

We guard against duplicate references being created in Project.AddProjectReferences and similar APIs, but no similar guarding is done if you call WithProjectReferences. It seems the checks should be added there instead.

@KirillOsenkov

This comment has been minimized.

Copy link
Member

@KirillOsenkov KirillOsenkov commented Jul 20, 2016

There are incorrectly authored projects out there with duplicate project references. SourceBrowser is "finding" them in droves (by hitting the key already exists exception in CompilationTracker).

What's the best way to fix it to not break existing incorrectly authored projects? Dedup the references in WithProjectReferences/AddProjectReferences?

I've already filed a project system suggestion to issue a warning on duplicate references:
dotnet/project-system#311

@jasonmalinowski

This comment has been minimized.

Copy link
Member Author

@jasonmalinowski jasonmalinowski commented Jul 20, 2016

I would say AddProjectReferences/WithProjectReferences should throw (since it's bad data), and MSBuildWorkspace can do the dedup there.

@KirillOsenkov

This comment has been minimized.

Copy link
Member

@KirillOsenkov KirillOsenkov commented Aug 11, 2016

If it helps, this is what I wrote to work around the crash (and it seems to have helped):

        private Solution DeduplicateProjectReferences(Solution solution)
        {
            foreach (var projectId in solution.ProjectIds.ToArray())
            {
                var project = solution.GetProject(projectId);

                var distinctProjectReferences = project.AllProjectReferences.Distinct().ToArray();
                if (distinctProjectReferences.Length < project.AllProjectReferences.Count)
                {
                    var newProject = project.WithProjectReferences(distinctProjectReferences);
                    solution = newProject.Solution;
                }
            }

            return solution;
        }

@mattwar maybe Matt can look into fixing MSBuildWorkspace? I'd send a PR myself but I'm on vacation. Also it probably needs a test.

@mattwar

This comment has been minimized.

Copy link
Contributor

@mattwar mattwar commented Aug 11, 2016

Alas @KirillOsenkov I am also on vacation.

@brunotag

This comment has been minimized.

Copy link

@brunotag brunotag commented Nov 7, 2016

@KirillOsenkov thanks for the workaround, I was getting this same "task was canceled" error mentioned here #12872.
So I turned on first chance exceptions and caught a System.ArgumentException in mscorlib.dll!System.Collections.Generic.Dictionary<Microsoft.CodeAnalysis.MetadataReference, Microsoft.CodeAnalysis.ProjectId>.Insert(...) and ended up here.
So, 12872 can be due to duplicate references, @pieroci

@Pilchie

This comment has been minimized.

Copy link
Member

@Pilchie Pilchie commented Nov 7, 2016

Interesting - tagging @CyrusNajmabadi too.

@Pilchie Pilchie modified the milestones: 3.0, 2.0 (RTM) Jan 19, 2017
@jasonmalinowski jasonmalinowski removed their assignment Jan 17, 2019
@jasonmalinowski jasonmalinowski modified the milestones: 16.0, Backlog Jan 17, 2019
jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this issue Feb 8, 2019
The workspace isn't supposed to allow duplicate ProjectReferences, but
currently due to dotnet#12101 it does.
This dedups (correctly) at the surface and also adds a test to ensure
this isn't broken.

Fixes dotnet#31390.
jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this issue Feb 8, 2019
The workspace isn't supposed to allow duplicate ProjectReferences, but
currently due to dotnet#12101 it does.
This dedups (correctly) at the surface and also adds a test to ensure
this isn't broken.

Fixes dotnet#31390.
@jasonmalinowski jasonmalinowski modified the milestones: Backlog, 16.1.P1 Feb 8, 2019
@jasonmalinowski jasonmalinowski self-assigned this Feb 8, 2019
@jasonmalinowski

This comment has been minimized.

Copy link
Member Author

@jasonmalinowski jasonmalinowski commented Feb 8, 2019

Pulling this back in as we've realized it does actually cause problems.

jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this issue Feb 11, 2019
The workspace isn't supposed to allow duplicate ProjectReferences, but
currently due to dotnet#12101 it does.
This dedups (correctly) at the surface and also adds a test to ensure
this isn't broken.

Fixes dotnet#31390.
jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this issue Feb 11, 2019
The workspace isn't supposed to allow duplicate ProjectReferences, but
currently due to dotnet#12101 it does.
This dedups (correctly) at the surface and also adds a test to ensure
this isn't broken.

Fixes dotnet#31390.
@jasonmalinowski jasonmalinowski modified the milestones: 16.1.P1, 16.2 Apr 23, 2019
@jinujoseph jinujoseph modified the milestones: 16.2, Backlog Jun 9, 2019
@tmat tmat closed this in #42451 Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.