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 up NETCoreApp closure validation when assembling runtime packages #31805

Closed
dagood opened this issue Feb 5, 2020 · 7 comments · Fixed by #41698
Closed

Fix up NETCoreApp closure validation when assembling runtime packages #31805

dagood opened this issue Feb 5, 2020 · 7 comments · Fixed by #41698

Comments

@dagood
Copy link
Member

dagood commented Feb 5, 2020

In a recent official build binlog, it looks like VerifyClosure isn't running, and its dependency GetClosureFiles doesn't show up at all. This is an important check to ensure shared framework integrity.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Feb 5, 2020
@NikolaMilosavljevic NikolaMilosavljevic removed the untriaged New issue has not been triaged by the area owner label Feb 28, 2020
@NikolaMilosavljevic NikolaMilosavljevic self-assigned this Feb 28, 2020
@NikolaMilosavljevic NikolaMilosavljevic added this to the 5.0 milestone Feb 28, 2020
@NikolaMilosavljevic
Copy link
Member

@jkoritzinsky do you know if this is handled in the new shared FX infra?

@jkoritzinsky
Copy link
Member

The is one of the areas I need to still look at. I dont know if this is required since ASP.NET doesn't do anything similar to this. I can look at adding this in the new infra if we still feel it is valuable.

@dagood
Copy link
Member Author

dagood commented Jul 28, 2020

The is one of the areas I need to still look at. I dont know if this is required since ASP.NET doesn't do anything similar to this. I can look at adding this in the new infra if we still feel it is valuable.

@ericstj can help make this call. My understanding is it's pretty important. (I took some efforts to keep it working over time and run it in dotnet/windowsdesktop.)

@ericstj
Copy link
Member

ericstj commented Jul 28, 2020

This validation helps ensure we're shipping a complete shared framework. It guards against folks taking new dependencies without putting those in the shared framework. It's important and something we should bring back.

@ericstj
Copy link
Member

ericstj commented Aug 20, 2020

We need to do this for 5.0. It was just exposed that we've introduced cycles into the shared framework because this was missing. @NikolaMilosavljevic or @jkoritzinsky were you planning on fixing this?

@NikolaMilosavljevic
Copy link
Member

We need to do this for 5.0. It was just exposed that we've introduced cycles into the shared framework because this was missing. @NikolaMilosavljevic or @jkoritzinsky were you planning on fixing this?

Yes, this is planned. I will work on it soon.

@NikolaMilosavljevic
Copy link
Member

Validation in progress - sending PR later today.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants