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

[mono] Don't build System.Utf8String.Experimental.Tests.csproj tests for mono #34445

Merged
merged 1 commit into from
Apr 3, 2020

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Apr 2, 2020

It fails when we build repo with --subsetCategory mono-libraries

System.Utf8String.Experimental.csproj was already disabled:
https://github.com/dotnet/runtime/blob/master/src/libraries/src.builds#L5

@@ -6,6 +6,10 @@
<TargetFramework>$(NetCoreAppCurrent)</TargetFramework>
</PropertyGroup>

<ItemGroup Condition="'$(RuntimeFlavor)' == 'Mono'">
<ProjectExclusions Include="$(MSBuildThisFileDirectory)\System.Utf8String.Experimental\tests\System.Utf8String.Experimental.Tests.csproj" />
</ItemGroup>
Copy link
Member

Choose a reason for hiding this comment

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

Is this the best / only place to do this? We've built the engineering system here such that core files like this one don't need to know about individual test projects, but now we're adding a specific suppression to this general file. There's no way to do this as part of System.Utf8String.Experimental?

Copy link
Member

Choose a reason for hiding this comment

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

The project shouldn't be loaded at all, hence excluding it in the traversals (tests.proj, src.builds, ref.builds, packages.builds) is fine.

Copy link
Member

Choose a reason for hiding this comment

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

I understand, but why can't something be put into the System.Utf8String.Experimental directory that would prevent that as well?

Copy link
Member

@ViktorHofer ViktorHofer Apr 2, 2020

Choose a reason for hiding this comment

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

We currently just glob on csproj files and invoke them in tests.proj and other traversals. We could do an exclusion check based on a file being present in every item's project dir but I doubt that's worth the hassle. Just doing the file globbing in msbuild is already slow. (as an example, we keep a static list of references for the facades in the netstandard package to avoid globbing)

Copy link
Member

@stephentoub stephentoub Apr 2, 2020

Choose a reason for hiding this comment

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

Then how bad is it if we just load the project? We're talking about one project out of hundreds that will get loaded and ignored in a test build.

Copy link
Member

Choose a reason for hiding this comment

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

I'm sure @EgorBo can try that out. To be consistent, the same needs to be done for the ref and the src project which are also excluded.

Copy link
Member

Choose a reason for hiding this comment

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

I'd appreciate if we could merge this and file an issue to make it better to unblock our test build :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am not a build system expert, how can I disable build for certain libraries in their folders? what property can I use in Directory.Build.props ?

Copy link
Member

Choose a reason for hiding this comment

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

I'd appreciate if we could merge this and file an issue to make it better to unblock our test build :)

Ok.

Copy link
Member

Choose a reason for hiding this comment

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

Filed #34499

@stephentoub stephentoub merged commit 0b246c0 into dotnet:master Apr 3, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants