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

Make apphost generation work correctly during incremental build #2554

Closed
sbomer opened this issue Sep 26, 2018 · 4 comments
Closed

Make apphost generation work correctly during incremental build #2554

sbomer opened this issue Sep 26, 2018 · 4 comments
Labels
Milestone

Comments

@sbomer
Copy link
Member

sbomer commented Sep 26, 2018

The apphost generation currently will not overwrite an existing apphost. The result is that changing properties that influence the apphost (setting the target to WinExe/Exe, or adding resources to the managed dll that get copied over to the apphost via #2545) won't result in a new apphost being created for incremental builds, which seems incorrect.

See the discussion in #2467 and the related issue #2473.

@nguerrera
Copy link
Contributor

I wonder if we can actually solve this on top of #2545 with just use Inputs="$(IntermediateAssembly)" Outputs="$(ApphostDestinationPath)" on _CreateAppHost + removing the File.Exists check. If the output type changes, the managed assembly will also change. Are there other properties that can impact the apphost?

cc @peterhuene

@nguerrera nguerrera added this to the 3.0.1xx milestone Sep 27, 2018
@nguerrera nguerrera added the Bug label Sep 27, 2018
@rainersigwald
Copy link
Member

rainersigwald commented Sep 27, 2018

If there are additional properties or items that matter beyond the assembly, you can use a pattern like we do for C#: collect all relevant properties into an item and write its hash to a file with WriteOnlyWhenDifferent="True".

See https://github.com/Microsoft/msbuild/blob/90506c8f6c5e4b82a09bafa6ae62a41e9e7367bb/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3404-L3431 (right now that only does items but you could add properties too).

@wli3
Copy link

wli3 commented Sep 27, 2018

https://github.com/dotnet/sdk/pull/2413/files#diff-fc9ca72fce3be7d1f0119f6666a307bdR148

I think genshim's incremental build is correct. Normal apphost should be similar to that

@nguerrera
Copy link
Contributor

It should be similar to that if necessary, but I think the standard apphost might be a pure function of the managed assembly.

wli3 pushed a commit that referenced this issue Feb 7, 2020
….3 (#2554)

- Microsoft.DotNet.Cli.Runtime - 3.0.100-rc1.19451.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants