-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
GenerateResources task is not incremental on .NET Core #1197
Comments
IIRC there's a plan to make binary serialization available everywhere (just not portable between .NET Core and Full Framework). If that is already done this might be easy. If not, we might have to devise a new serialization format 👎 |
I believe binary serialization is coming to .NET Core with netstandard2.0. But that won't be available for a while yet. So we may have to devise a way around this for Dev15. |
That's correct, binary serialization will come back. @danmosemsft, who is working on it and can provide an update where we are with it? |
BinaryFormatter is already implemented for 1.2. It's in master. It works. And most of the core types that should be serializable now are, though there are still a bunch of types across corefx that need some work done to restore their serialization implementations. |
@stephentoub: Sweet! @rainersigwald, what gurantees do you need for incremental build? I suppose you don't care about binary serialization having different output between .NET Framework and .NET Core? |
@terrajobst it uses BinaryFormatter for 2 purposes
|
Just realized we can't create AppDomains so that's not an issue. Another reason we shouldn't run on .resx's with serialized user types - we'll lock them. |
I think the ifdef can be narrowed to give us pretty good incremental build. Let me see. |
A sketch (or possibly the actual) fix for incrmentality without BinarySerialization is in the PR #1202 |
Ok, so my current understanding is: .NET Core assemblies cannot use linked resources (that's what it sounds like from comments @DamianEdwards made on https://github.com/dotnet/corefx/issues/8200 which is now https://github.com/dotnet/cli/issues/3695). This implies that it's fine to drop all of the code in That could create a behavior difference when using .NET Core MSBuild to build assemblies for Desktop. CLI currently does that, but the plan has been to always use Desktop MSBuild if it's available. Given this, I think it's ok to do something like #1202. @danmosemsft and others--is this a reasonable read on the situation? |
@rainersigwald yes I think so. BTW I don't have cycles right now to test/commit #1202 ... for some reason I can't get MSBuild to actually build for me right now. |
Closes dotnet#1197. This implementation is sufficient for current .NET Core--see dotnet#1247 for details. If we start supporting linked resources, this will produce false up-to-date results and must be changed.
Fixed in xplat packages from |
The
CoreResGen
target is not incremental, because it assumes that theGenerateResources
task handles incrementality internally. It does . . . on full framework, whenFEATURE_BINARY_SERIALIZATION
is enabled andNeedToRebuildSourceFile
gets called.On .NET Core, though, that codepath falls back to the conservative-but-slow "always just rebuild".
(noticed by @eerhardt in dotnet/sdk#284)
The text was updated successfully, but these errors were encountered: