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

mark native files in RuntimeList.xml as "DropFromSingleFile" unless from a specific list of known files. #36047

Closed
wants to merge 1 commit into from

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented May 7, 2020

mark native files in RuntimeList.xml as "DropFromSingleFile" unless from a specific list of known files.

@ghost
Copy link

ghost commented May 7, 2020

Tagging subscribers to this area: @ViktorHofer
Notify danmosemsft if you want to be subscribed.

@jkoritzinsky
Copy link
Member

I believe this change needs to be made in the Arcade version of this task. I think the one in this repo is dead code.

cc: @dagood

Here’s the arcade version: https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Build.Tasks.SharedFramework.Sdk/src/CreateFrameworkListFile.cs

@VSadov
Copy link
Member Author

VSadov commented May 7, 2020

Thanks!!
I will copy the change and add a comment here that this is dead copy. I assume it is needed, since it is here.

@VSadov VSadov closed this May 7, 2020
@@ -186,6 +186,11 @@ public override bool Execute()
}
}

if (f.IsNative && !ShouldIncludeInSingleFileHost(f.Filename))
Copy link
Member

@dagood dagood May 8, 2020

Choose a reason for hiding this comment

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

This should be done similarly to ReferencedByDefault (or metadata on Files or something), not hard-coded into the build task itself, so that putting it into Arcade doesn't prevent quickly changing it in dotnet/runtime. (Also makes the data easier to find.)

Copy link
Member Author

Choose a reason for hiding this comment

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

When it was supposed to be entirely in runtime, it could be more convenient to keep the logic in one place.
If this needs to go into arcade, I guess adding it as metadata on Files could make sense.

@dagood
Copy link
Member

dagood commented May 8, 2020

Is there an issue associated with this?

/cc @NikolaMilosavljevic

@VSadov
Copy link
Member Author

VSadov commented May 8, 2020

cc: @swaroop-sridhar

@VSadov
Copy link
Member Author

VSadov commented May 8, 2020

The associated issue: dotnet/sdk#11567

@swaroop-sridhar
Copy link
Contributor

I believe this change needs to be made in the Arcade version of this task. I think the one in this repo is dead code.

cc: @dagood

Here’s the arcade version: https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Build.Tasks.SharedFramework.Sdk/src/CreateFrameworkListFile.cs

Can we remove this file from the runtime repo? Or is there a reason to keep them here too?

@dagood
Copy link
Member

dagood commented May 8, 2020

It's already gone. #35405

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