Add AppHost Binary transformation Utilities to HostModel #6831
Conversation
The code that transforms AppHost binary in order to update the path to the App.dll binary lives in multiple repositories (SDK, CLI and partly in core-setup tests). This change adds the code to HostModel library, so that other copies can be removed. This also facilitates implementing a similar mechanism to locate the offset of the bundle header within the binary (so that the bundle headers are not required to live at the end of the file).
| // Copy resources from managed dll to the apphost | ||
| new ResourceUpdater(appHostDestinationFilePath) | ||
| .AddResourcesFromPEImage(intermediateAssembly) | ||
| .Update(); |
| string appHostDestinationFilePath, | ||
| string appBinaryFilePath, | ||
| bool windowsGraphicalUserInterface = false, | ||
| string intermediateAssembly = null) |
| { | ||
| if (!appHostIsPEImage) | ||
| { | ||
| throw new BinaryUpdateException($"Unable to use '{appHostSourceFilePath}' as application host executable because it's not a Windows executable for the CUI (Console) subsystem"); |
|
|
||
| private static void ThrowExceptionForInvalidUpdate() | ||
| { | ||
| throw new InvalidOperationException("Update handle is invalid. This instance may not be used for further updates"); |
There was a problem hiding this comment.
this type of developer exception is ok to be just in english
|
Do we want to use the new non-OS-bound PE resource APIs that @davidwrighton did for xplat IBCMerge or do we want to wait on that until a future PR/post-3.0? |
1) Throw different exceptions for each error (instead of passing the message) 2) Add the GetGUIBit method.
swaroop-sridhar
changed the title
|
@wli3, thanks for the comments, I've made the changes now. |
| /// </summary> | ||
| public class AppNameTooLongException : AppHostUpdateException | ||
| { | ||
| public string LongName; |
There was a problem hiding this comment.
Fixed it in this commit: ff295f5
|
👍 |
I think we should bring the code over as-is. If the resource-updater APIs can be made public, then we can change it. |
|
@dotnet-bot test dotnet.core-setup (Build_OSX release) |
Ooh. Yes, please!
OK. |
|
Yes, I would continue this PR without pulling in the non OS bound PE resource apis. The work that I did around the non OS bound PE resource apis should be just about drop in compatible with the OS api surface, so it should be easy, but its a large pile of really terrible code which could be destabilizing, and would certainly distract from the core thrust of this PR. If you're interested in doing that, it should be a separate PR. |
…-setup#6831) * Add AppHost Binary transformatuin Utilities to HostModel The code that transforms AppHost binary in order to update the path to the App.dll binary lives in multiple repositories (SDK, CLI and partly in core-setup tests). This change adds the code to HostModel library, so that other copies can be removed. This also facilitates implementing a similar mechanism to locate the offset of the bundle header within the binary (so that the bundle headers are not required to live at the end of the file). This change also adds the GetGUIBit method, as requested by the SDK team. Commit migrated from dotnet/core-setup@daf720b
The code that transforms AppHost binary in order to update the
path to the App.dll binary lives in multiple repositories
(SDK, CLI and partly in core-setup tests).
This change adds the code to HostModel library, so that other
copies can be removed.
This also facilitates implementing a similar mechanism to
locate the offset of the bundle header within the binary
(so that the bundle headers are not required to live at the end
of the file).