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

Fix project.json search in DotNetProjectSystem #673

Merged

Conversation

DustinCampbell
Copy link
Contributor

@DustinCampbell DustinCampbell commented Nov 22, 2016

This change fixes several issues with locating project.json projects:

  1. The top-level folders defined in global.json were not searched. So, if a user tried to add "projects": ["MyProject1"] where MyProject1 contained a project.json file, the project wouldn't be found. Instead, only child folders of the top-level folders would be search. So, src/MyProject1 would work. Now, we'll search the top-level folders and their immediate children, which should match how dotnet restore behaves a bit more closely. A test has been added for this scenario.

  2. We had infrastructure for falling back to a recursive directory search when a project.json or global.json doesn't exist in the current directory. However, we weren't using it! There was an issue with the recursive search (it wouldn't search a directory's sub-directories if the directory contained a project.json file), but that is now fixed.

  3. The test for recursively searching directories wasn't correct. This is now fixed.

This fixes dotnet/vscode-csharp#962 and dotnet/vscode-csharp#904

This change fixes several issues with locating project.json projects:

1. The top-level folders defined in global.json were not searched. So, if
a user tried to add `"projects": ["MyProject1"]` where `MyProject1`
contained a project.json file, the project wouldn't be found. Instead,
only child folders of the top-level folders would be search. So,
`src/MyProject1` would work. Now, we'll search the top-level folders and
their immediate children, which should match how `dotnet restore` behaves
a bit more closely. A test has been added for this scenario.

2. We had infrastructure for falling back to a recursive directory search
when a project.json or global.json doesn't exist in the current directory.
However, we weren't using it! There was an issue with the recursive search
(it wouldn't search a directory's sub-directories if the directory
contained a project.json file), but that is now fixed.

3. The test for recursively searching wasn't correct. This is now fixed.
@@ -156,7 +112,7 @@ private static IEnumerable<ProjectDescription> GetProjectReferences(ProjectConte
continue;
}

// if this is an assembly reference then don't threat it as project reference
// if this is an assembly reference then don't treat it as project reference
Copy link
Member

Choose a reason for hiding this comment

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

lol

}

public static IEnumerable<string> Search(string solutionRoot, int maxDepth)
public static IEnumerable<string> Search(string directory, int maxDepth)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this function was inconsistent before my changes. Sometimes it would return a directory path to a project.json files, and sometimes it would return multiple file paths to project.json files. Now, it just returns file paths to project.json files.

Copy link
Member

@david-driscoll david-driscoll left a comment

Choose a reason for hiding this comment

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

:shipit:

Looks good, after I wrote out my sanity comment, it made sense, lol.

var projectPaths = new SortedSet<string>(StringComparer.OrdinalIgnoreCase);
var searchDirectories = new Queue<string>();

// Look in global.json 'projects' search paths and their immediate children
Copy link
Member

Choose a reason for hiding this comment

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

For my sanity (because project.json is starting to fade with csproj out soon 😜).

global.json never supported wild cards, but instead just looked to see if it a any project.json in the folder or it's children. ie the common case was projects: ["src", "test"].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Were you looking for a change here?

@DustinCampbell DustinCampbell merged commit a4dced2 into OmniSharp:dev Nov 23, 2016
@DustinCampbell DustinCampbell deleted the fix-dotnet-project-search branch January 19, 2017 14:10
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