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

Ignore invalid import when expression empty #6222

Merged

Conversation

@jeffkl
Copy link
Contributor

@jeffkl jeffkl commented Mar 3, 2021

Fixes #6221

Context

When the project load setting IgnoreInvalidImports is set, the caller expects imports that couldn't be loaded to just be ignored in a best effort attempt to load the project. However, the code path of the project expression evaluating to an empty string was overlooked in #2720

Changes Made

I've checked the ProjectLoadSettings in the evaluator when the project import expression evaluates to an empty string and logged an event and skipped the import

Testing

I've added a unit test

Notes

The first commit is the implementation, the second commit is just a conversion to Shouldly

@cdmihai
cdmihai approved these changes Mar 4, 2021
@Forgind
Forgind approved these changes Mar 5, 2021
Copy link
Member

@Forgind Forgind left a comment

Thanks for all the changes to Shouldly!

src/Build/Evaluation/Evaluator.cs Outdated Show resolved Hide resolved
Copy link
Member

@BenVillalobos BenVillalobos left a comment

LGTM

@BenVillalobos BenVillalobos merged commit 2da32a0 into dotnet:master Mar 13, 2021
7 checks passed
7 checks passed
license/cla All CLA requirements met.
Details
@azure-pipelines
msbuild-pr Build #20210305.6 succeeded
Details
@azure-pipelines
msbuild-pr (Linux Core) Linux Core succeeded
Details
@azure-pipelines
msbuild-pr (Windows Core) Windows Core succeeded
Details
@azure-pipelines
msbuild-pr (Windows Full Release (no bootstrap)) Windows Full Release (no bootstrap) succeeded
Details
@azure-pipelines
msbuild-pr (Windows Full) Windows Full succeeded
Details
@azure-pipelines
msbuild-pr (macOS Core) macOS Core succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants