Add apphost customization #2545

sbomer · 2018-09-20T21:39:00Z

This adds logic to copy resources from the managed dll to the apphost.

_ComputeNETCoreBuildOutputFiles now runs later in the build, at a point where IntermediateAssembly is available.
The resource updating can only be done on windows hosts. When building for windows from a unix host, we log a warning but don't fail the build.
- I introduced a string resource (AppHostCustomizationRequiresWindowsHostWarning) for this case, but not for errors specific to the ResourceUpdater
No attempt is made to update resources when targeting unix, and no warning is shown in this case. Resources will just end up in the managed dll like before.
I added a testcase that uses the ResourceUpdater to copy over and check a version resource.
I did manual testing to verify that app icons and manifests work correctly with the change (the apphost icon shows up in the file explorer, and I was able to make an apphost ask for admin privileges on launch using an embedded app manifest).
This also includes a method that @AaronRobinsonMSFT should be able to use for adding a manifest for COM activation (see AddResource).

Note that this feature doesn't yet work with incremental build (see the discussion here #2467).

Addresses #1899.

This will emit a warning if host customization is requested but we are building on non-Windows (where the required APIs don't exist).

Add proper error handling and documentation

src/Tasks/Microsoft.NET.Build.Tasks/AppHost.cs

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs

jkotas · 2018-09-20T22:38:10Z

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs

+            IntPtr hResource = FindResourceEx(hModule, lpType, lpName, wLang);
+            if (hResource == IntPtr.Zero)
+            {
+                ThrowExceptionForLastWin32Error();


It is dangerous to throw exceptions out of C callback. The C callback should always catch the exceptions and return the error via lParam.

Thank you. Fixed in ea4f506. I also moved away from ThrowExceptionForHR due to #2552.

src/Tasks/Microsoft.NET.Build.Tasks/AppHost.cs

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToCustomizeTheApphost.cs

vitek-karas · 2018-09-21T20:05:28Z

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs

+    /// in a PE image. It currently only works on Windows, because it
+    /// requires various kernel32 APIs.
+    /// </summary>
+    public class ResourceUpdater


I think this should implement IDisposable since it owns the update handle. In case of exceptions this will likely leak that handle.

I made it IDisposable, using a new SafeUpdateHandle.

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs

src/Tasks/Microsoft.NET.Build.Tasks/CreateAppHost.cs

nguerrera · 2018-09-21T22:42:48Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

-          DependsOnTargets="ResolvePackageAssets"
-          AfterTargets="ResolveReferences"
-          BeforeTargets="AssignTargetPaths"
+          DependsOnTargets="ResolvePackageAssets;CoreCompile"


CoreCompile [](start = 49, length = 11)

I'm not sure about this dependency on CoreCompile. I know we need the intermediate assembly available, but there could be scenarios where we end up triggering a re-build when we were supposed to be using what is already on disk. This has come up a few times with other targets. I have to think about the best approach here.

Still thinking about it, but I think the best approach following the existing idioms in common targets would be to generate the apphost to intermediate directory upon/after compile and designate the intermediate item as something to copy separately.

In reply to: 219644421 [](ancestors = 219644421)

@nguerrera generating the apphost into the intermediate directory would still have a dependency on CoreCompile, wouldn't it?

Correct.

I was thinking about changing the paradigm to react to compilation instead of depending on compilation.

I am concerned that there may be places where this target is invoked without expecting compilation to happen even if the source files are out of date. Look at the long discussion on https://github.com/dotnet/cli/issues/5331 for some background on why this is worth worrying about.

What I'm wondering is: can the apphost generation happen in a separate target that is AfterTargets=CoreCompile (after we compile a managed exe to the intermediate directory, create the matching apphost next to it). And this target would just add the intermediate location to the set of files to be copied.

Adding @rainersigwald who may have other ideas here.

Agreed that if the apphost creation will be invoked in a way that doesn't expect compilation to happen, it shouldn't depend on CoreCompile. But the current target logically depends on an up-to-date IntermediateAssembly, and I think "DependsOnTargets=CoreCompile" captures that.

If the scenario you're thinking about is one where we do "publish --no-build", and expect that this will create an apphost for the previously-built intermediate assembly (even if source files are out of date), then I can see your concern. To me it makes sense to implement this by expressing the dependency on the outputs of a target that logically produces IntermediateAssembly, and having that target internally respect the --no-build option by using the existing IntermediateAssembly in that case. This way the --no-build behavior is transparent to targets that depend on build outputs.

AfterTargets=CoreCompile could work, but that has other issues. Any inputs to the apphost generator that don't come from CoreCompile should trigger apphost re-generation if they change. For example, setting the target to WinExe should cause the apphost to be re-generated with the gui bit, even if we don't want to re-compile.

If the scenario you're thinking about is one where we do "publish" --no-build

I'm thinking even more generally. This will cause any caller of GetCopyToOutputDirectoryItems to trigger compilation and I don't think that's OK.

Agree that AfterTargets="CoreCompile" alone doesn't solve it. I'm still thinking about what's best here.

In reply to: 220379741 [](ancestors = 220379741)

It generally makes sense for GetCopyToOutputDirectoryItems to trigger compilation, because it logically depends on the compilation output through the apphost. If we want to optimize for "publish --no-build", we need to cut out some some dependency edges from Publish -> CoreCompile in this case (or alternatively make CoreCompile do nothing).

In #2111, it looks like we already cut one edge from Publish -> CoreCompile by making Publish not depend on Build in the "--no-build" case. Maybe we want to do something similar here? We could make GetCopyToOutputDirectoryItems depend on _ComputeNETCoreBuildOutputFiles unless NoBuild is specified. If NoBuild is specified, GetCopyToOutputDirectoryItems could just try to get the last-built apphost from the intermediate directory.

Just an idea, but I'm sure there are more constraints I don't know about. I'll let you decide how you think this should be done - let me know if there's anything I can do to help!

GetCopyToOutputDirectoryItems should not do actual work and should not depend on CoreCompile (and friends), because it is called on references in the /p:BuildProjectReferences=false case, which should not trigger compilation in the referenced projects. I don't think the NoBuild case applies here for that reason--you're building, but some other project.

Unfortunately, I can't think of a great way to accomplish this. Best I can think of is to move the actual work into a target (CreateAppHost?) and append ;CreateAppHost to $(CompileDependsOn), which would insert it in the normal flow shortly after CoreCompile but not require it to express the dependency. Then, do no real work in _ComputeNETCoreBuildOutputFiles and run it BeforeTargets="GetCopyToOutputDirectoryItems".

Thanks, @rainersigwald. This sounds like a good approach. @sbomer, can you look at doing it that way?

Thanks @rainersigwald and @nguerrera. I think the latest change does the trick - PTAL.

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs

Instead, use the extra parameter to capture error information, and throw the appropriate error from the caller of EnumResourceTypes.

nguerrera

Waiting for the issue with the CoreCompile dependency to be resolved.

This cuts the dependency from _ComputeNETCoreBuildOutputFiles on CoreCompile. Instead, _CreateAppHost will run after compilation via CompileDependson. Locating the restored apphost has been factored into another target _GetAppHostPaths, which is a dependency of both _ComputeNETCoreBuildOutputFiles and _CreateAppHost. The destination apphost path is also computed here, and passed to the CreateAppHost task as an input. This way, during a "publish --no-build", the apphost destintation path (of the previously-generated apphost) is computed as a dependency of _ComputeNETCoreBuildOutputFiles, and it is copied to the publish directory without causing CoreCompile to run.

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

nguerrera · 2018-09-27T18:26:21Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

+    </AssignTargetPath>
+    <ItemGroup>
+      <_NoneWithTargetPath Include="@(_AllNETCoreCopyLocalItemsWithTargetPath)" />
+    </ItemGroup>


With the create/get split, can this go before AssignTargetPaths again so that we don't need to do this?

I don't think so, unless we change when AssignTargetPaths runs. I see this dependency chain putting AssignTargetPaths before build, which is too early for the AppHost: CoreBuild -> PrepareResources -> PrepareResourceNames -> AssignTargetPaths.

I don't follow yet. We're just computing paths here in this target now, not actually generating the apphost. Why can't that be done sooner?

Oh, I think I understand now. It seems strange that we would add the apphost to None potentially before it exists. Won't that cause some item metadata to be missing?

I don't know if there's a problem with that. @rainersigwald ?

No, I don't think there'd be a problem. Well-known/built-in metadata is evaluated just-in-time, so if you create an item, create the file, and then ask for %(Whatever.ModifiedTime) it should still work.

I'm ok looking at this after the initial commit, btw. If this is green, we can merge as is.

I'll leave it as-is then. Thanks for all the help!

Thank you for the contribution of this feature!

src/Tasks/Common/Resources/Strings.resx

nguerrera · 2018-09-27T18:47:47Z

I will investigate the tests failing on full msbuild leg. (At the moment, the new CI system does not publish test logs nor can it re-run individual legs, but I'm told this is getting resolved next week.)

nguerrera · 2018-09-27T18:58:05Z

Test failure:

Unit.net Console Runner (64-bit .NET Core 4.6.26515.07)
  Discovering: Microsoft.NET.Pack.Tests
  Discovered:  Microsoft.NET.Pack.Tests
  Starting:    Microsoft.NET.Pack.Tests
    Microsoft.NET.Pack.Tests.GivenThatWeWantToPackANetFrameworkLibrary.ImplicitReferencesAreNotIncludedAsFrameworkReferences [FAIL]
      Expected command to pass but it did not.
      File Name: C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\msbuild.exe
      Arguments: /t:Pack D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj
      Exit Code: 1
      StdOut:
      Microsoft (R) Build Engine version 15.9.14-preview+gae560092f5 for .NET Framework
      Copyright (C) Microsoft Corporation. All rights reserved.
      Build started 9/27/2018 11:50:44 AM.
      Project "D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj" on node 1 (Pack target(s)).
      _CheckForNETCoreSdkIsPreview:
      D:\Src\sdk\artifacts\Debug\bin\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(145,5): message NETSDK1057: You are using a preview version of .NET Core. See: https://aka.ms/dotnet-core-preview [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
      PrepareForBuild:
        Creating directory "bin\Debug\net461\".
        Creating directory "obj\Debug\net461\".
      CoreCompile:
        C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Roslyn\csc.exe /noconfig /unsafe- /checked- /nowarn:1701,1702,1701,1702 /nostdlib+ /errorreport:prompt /warn:4 /define:TRACE;DEBUG;NETFRAMEWORK;NET461 /highentropyva+ /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\mscorlib.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Core.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Data.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Drawing.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.IO.Compression.FileSystem.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Numerics.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Runtime.Serialization.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Xml.dll" /reference:"C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.1\System.Xml.Linq.dll" /debug+ /debug:portable /filealign:512 /optimize- /out:obj\Debug\net461\PackImplicitReferences.dll /ruleset:"C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\Team Tools\Static Analysis Tools\\Rule Sets\MinimumRecommendedRules.ruleset" /subsystemversion:6.00 /target:library /warnaserror- /utf8output /deterministic+ PackImplicitReferences.cs "C:\Users\nicholg\AppData\Local\Temp\.NETFramework,Version=v4.6.1.AssemblyAttributes.cs" obj\Debug\net461\PackImplicitReferences.AssemblyInfo.cs /warnaserror+:NU1605
        Using shared compilation with compiler from directory: C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Roslyn
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3261,7): error MSB4057: The target "_AfterCompileWinFXInternal [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3261,7): error MSB4057:      [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3261,7): error MSB4057:       _CreateAppHost" does not exist in the project. [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
      Done Building Project "D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj" (Pack target(s)) -- FAILED.
      Build FAILED.
      "D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj" (Pack target) (1) ->
        C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3261,7): error MSB4057: The target "_AfterCompileWinFXInternal [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3261,7): error MSB4057:      [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\15.0\Bin\Microsoft.Common.CurrentVersion.targets(3261,7): error MSB4057:       _CreateAppHost" does not exist in the project. [D:\Src\sdk\artifacts\Debug\tmp\PackImplicitReferences\PackImplicitReferences\PackImplicitReferences.csproj]
          0 Warning(s)
          1 Error(s)

~~I think this has something to do with two-phase WPF builds. Investigating further.~~

nguerrera · 2018-09-27T19:05:02Z

Oh, it looks like it's just a missing semicolon here:

    <CompileDependsOn>
-      $(CompileDependsOn)
+      $(CompileDependsOn);
      _CreateAppHost;
    </CompileDependsOn>

nguerrera · 2018-09-27T20:13:45Z

After fixing that, there's another failure on my box. Investigating

      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error APPX0002: Task 'ValidateConfiguration' failed. Value cannot be null. [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error APPX0002: Parameter name: input [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error APPX0002:  [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018: The "ValidateConfiguration" task failed unexpectedly. [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018: System.ArgumentNullException: Value cannot be null. [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018: Parameter name: input [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at System.Version.Parse(String input) [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at Microsoft.Build.AppxPackage.ValidateConfiguration.ExecuteImplementation() [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at Microsoft.Build.AppxPackage.AppxPackagingTaskHelper.Execute(String file) [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018: --- End of stack trace from previous location where exception was thrown --- [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at Microsoft.Build.AppxPackage.AppxPackagingTaskHelper.Execute(String file) [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at Microsoft.Build.AppxPackage.ValidateConfiguration.Execute() [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]
      C:\Program Files (x86)\Microsoft Visual Studio\Preview\Enterprise\MSBuild\Microsoft\VisualStudio\v15.0\AppxPackage\Microsoft.AppXPackage.Targets(6068,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() [D:\Src\sdk\artifacts\Debug\tmp\PCL_Project_r---890FB1DF\Dependency\Dependency.csproj]

nguerrera · 2018-09-27T20:19:08Z

That second failure is internal bug 686365, which is fixed but my dogfood build hasn't updated. I believe the CI machine would not have a build of VS with that bug so this should be green once the semicolon is fixed.

And fix a typo

The apphost is (optionally) customized by copying resources from the managed dll, and setting the GUI bit based on the Target property (which also modifies the managed dll). Therefore, we use the managed dll as an input for incremental build (the apphost needs to be updated iff the managed dll was updated). This fixes dotnet#2554. This also removes the unnecessary overwriteExisting flag from AppHost.Create, fixing dotnet#2473.

sbomer

@nguerrera thank you for investigating the CI failures!

sbomer · 2018-10-01T16:34:52Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets

+    </AssignTargetPath>
+    <ItemGroup>
+      <_NoneWithTargetPath Include="@(_AllNETCoreCopyLocalItemsWithTargetPath)" />
+    </ItemGroup>


I don't think so, unless we change when AssignTargetPaths runs. I see this dependency chain putting AssignTargetPaths before build, which is too early for the AppHost: CoreBuild -> PrepareResources -> PrepareResourceNames -> AssignTargetPaths.

peterhuene · 2018-10-01T17:34:23Z

It looks like some of the tests didn't get updated with the removal of overwriteExisting.

sbomer · 2018-10-01T17:40:12Z

Oops, left those changes out of my commit for some reason. Thanks.

nguerrera · 2018-10-01T23:35:33Z

👏 🎉

sbomer added 9 commits September 14, 2018 12:49

Add apphost customization

dcacdc0

Pass intermediate assembly to apphost

25bb0d6

Change task order to make IntermediateAssembly available to app host

0bf8fa5

Make intermediate assembly optional and fix version check test

888026b

Factor out PE image check

2e776e9

Check PE image and host to provide appropriate warning

6e40454

This will emit a warning if host customization is requested but we are building on non-Windows (where the required APIs don't exist).

Clean up resource updater a bit

f61feae

Clean up ResourceUpdater

7a342b5

Add proper error handling and documentation

Run the resource test on windows only

0574dcc

AaronRobinsonMSFT reviewed Sep 20, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/AppHost.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed Sep 20, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed Sep 20, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs Outdated Show resolved Hide resolved

jkotas reviewed Sep 20, 2018

View reviewed changes

vitek-karas requested changes Sep 21, 2018

View reviewed changes

nguerrera reviewed Sep 21, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs Outdated Show resolved Hide resolved

nguerrera reviewed Sep 21, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CreateAppHost.cs Outdated Show resolved Hide resolved

nguerrera reviewed Sep 21, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/CreateAppHost.cs Show resolved Hide resolved

nguerrera reviewed Sep 21, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/ResourceUpdater.cs Outdated Show resolved Hide resolved

livarcocc mentioned this pull request Sep 22, 2018

.NETCore apps cannot specify a Windows application manifest and other things that must go in exe #1899

Closed

sbomer added 10 commits September 24, 2018 10:57

Use resource strings for all error messages

e8fcf28

Use a private sealed class for pinvokes

156e0c3

Add comment about PE header

5833c67

Document new parameters to AppHost.Create

b029ce0

Use more descriptive callback names

c455384

Implement IDisposable using a SafeHandle for hUpdate

ecba305

Use custom exception type for LockResource failure

49ee502

Remove AppHostOptions

20f4c1e

Make IntermediateAssembly required in CreateAppHost task

4199da9

Make ResourceUpdater internal

3af93e1

sbomer added 2 commits September 25, 2018 16:30

Don't throw managed exceptions from callbacks

ea4f506

Instead, use the extra parameter to capture error information, and throw the appropriate error from the caller of EnumResourceTypes.

Move language-neutral constant to Kernel32 class

982d4ab

vitek-karas approved these changes Sep 26, 2018

View reviewed changes

nguerrera suggested changes Sep 26, 2018

View reviewed changes

sbomer mentioned this pull request Sep 26, 2018

Make apphost generation work correctly during incremental build #2554

Closed

nguerrera reviewed Sep 27, 2018

View reviewed changes

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets Outdated Show resolved Hide resolved

rainersigwald approved these changes Sep 27, 2018

View reviewed changes

nguerrera reviewed Sep 27, 2018

View reviewed changes

peterhuene reviewed Sep 27, 2018

View reviewed changes

src/Tasks/Common/Resources/Strings.resx Outdated Show resolved Hide resolved

nguerrera approved these changes Sep 27, 2018

View reviewed changes

sbomer added 5 commits October 1, 2018 09:25

Place customized apphost into IntermediateOutputPath

9e5b84b

Make UseWindowsGraphicalUserInterface property private

801cacd

And fix a typo

Add missing semicolon in CompileDependsOn

dce92c7

Improve warning message

45eef49

sbomer commented Oct 1, 2018

View reviewed changes

nguerrera approved these changes Oct 1, 2018

View reviewed changes

Remove overwriteExisting option from tests

c3aba73

livarcocc merged commit c202205 into dotnet:master Oct 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add apphost customization #2545

Add apphost customization #2545

sbomer commented Sep 20, 2018

jkotas Sep 20, 2018

sbomer Sep 25, 2018

vitek-karas Sep 21, 2018

sbomer Sep 25, 2018

nguerrera Sep 21, 2018

nguerrera Sep 21, 2018 •

edited

Loading

sbomer Sep 25, 2018

nguerrera Sep 25, 2018

sbomer Sep 25, 2018

nguerrera Sep 25, 2018

sbomer Sep 26, 2018

rainersigwald Sep 26, 2018

nguerrera Sep 26, 2018

sbomer Sep 27, 2018

nguerrera left a comment

nguerrera Sep 27, 2018

sbomer Oct 1, 2018

nguerrera Oct 1, 2018

sbomer Oct 1, 2018

nguerrera Oct 1, 2018

rainersigwald Oct 1, 2018

nguerrera Oct 1, 2018

sbomer Oct 1, 2018

nguerrera Oct 1, 2018

nguerrera commented Sep 27, 2018

nguerrera commented Sep 27, 2018 •

edited

Loading

nguerrera commented Sep 27, 2018 •

edited

Loading

nguerrera commented Sep 27, 2018

nguerrera commented Sep 27, 2018

sbomer left a comment

sbomer Oct 1, 2018

peterhuene commented Oct 1, 2018

sbomer commented Oct 1, 2018

nguerrera commented Oct 1, 2018

Add apphost customization #2545

Add apphost customization #2545

Conversation

sbomer commented Sep 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nguerrera Sep 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nguerrera left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nguerrera commented Sep 27, 2018

nguerrera commented Sep 27, 2018 • edited Loading

nguerrera commented Sep 27, 2018 • edited Loading

nguerrera commented Sep 27, 2018

nguerrera commented Sep 27, 2018

sbomer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterhuene commented Oct 1, 2018

sbomer commented Oct 1, 2018

nguerrera commented Oct 1, 2018

nguerrera Sep 21, 2018 •

edited

Loading

nguerrera commented Sep 27, 2018 •

edited

Loading

nguerrera commented Sep 27, 2018 •

edited

Loading