Add DotNet-First packaging layout support and build extensibility - #1106
Add DotNet-First packaging layout support and build extensibility#1106DYH1319 wants to merge 3 commits into
Conversation
pr-comment: Run #140
🎉 All tests passed!Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
- Detect and launch the Electron host from an electron/ subdirectory when using a DotNet-First packaged app layout. - Make ElectronPackageId and Title overridable from MSBuild properties. - Allow overriding the intermediate PublishDir via ElectronPublishDir. - Add ElectronSkipExecCommands to suppress npm/npx build steps. Co-Authored-By: Mark Deng <1245687900@qq.com>
- Add 1000ms timeout to process.WaitForExit() call to avoid potential hangs - Ensure process cleanup doesn't block indefinitely on mono implementations
…kipExecCommands in logging - Remove redundant "Electron setup/installation failed!" messages that duplicate MSBuild's built-in error reporting - Add ElectronSkipExecCommands condition to Message tasks to prevent logging when commands are skipped - Increase electron install timeout from 600000ms to 1800000ms for Electron 42+ to accommodate slower installations
| { | ||
| // This shouldn't throw here, but the mono process implementation doesn't always behave as it should. | ||
| this.process.WaitForExit(); | ||
| this.process.WaitForExit(1000); |
There was a problem hiding this comment.
I really don't like having a number here - if we do it should be configurable (and the default should be at relatively high, at least 5_000, i.e., smaller than the indefinite one we have right now, but still high).
There was a problem hiding this comment.
Agreed. 1000ms was arbitrary. I'll make it configurable with a safer default of at least 5000ms.
| } | ||
| else | ||
| { | ||
| // Fallback to the legacy Electron-First directory layout. |
There was a problem hiding this comment.
This should not be "legacy". It should be the primary. Why do we have two different paths here? Can't this be just configurable ("electron-root-dir" - default is ".")?
There was a problem hiding this comment.
Agreed. The existing resources/bin layout is still the primary layout in Electron.NET, and it shouldn't be called "legacy". I'll change this to an explicit configuration, e.g. an ElectronRootDir property. The default behavior will preserve the existing dir.Parent.Parent logic; when explicitly set, the path will be resolved relative to the .NET executable directory.
FlorianRappl
left a comment
FlorianRappl
left a comment
There was a problem hiding this comment.
I miss a bit why we need that extra complexity and what problem this solves.
In addition, to accept this PR we also need changes to the docs (how to activate, when to use, ...).
|
Thanks for the review.
Electron.NET's documentation already supports packaged .NET-first startup: the .NET executable starts first and then launches Electron. However, in the current code, when packaged startup runs, ElectronProcessActive assumes the .NET executable is under resources/bin, so it uses dir.Parent.Parent to walk up two directories to locate Electron. Our packaging scenario looks like this: In this layout, the .NET executable sits at the package root, and Electron is placed in an electron/ subdirectory. At this point, dir.Parent.Parent jumps outside the package root, so it cannot find Electron. UnpackagedDetector also misidentifies the app as unpackaged because it doesn't see resources/bin. So the core goal of this PR is: let the .NET runtime know where to find Electron, so that a .NET-first layout with the .NET entry point at the package root can actually work.
I'll change the docs to reflect the changes. |
|
Understood - my point was that the default packaging is Electron-first - so while you can always "compose arbitrarily to use the different modes" I was wondering if this affects / would be good to extend deployment / package building as documented at https://github.com/ElectronNET/Electron.NET/wiki/Package-Building Let's refine the two things / make them configurable and then merge this. Many thanks for your efforts! |
2 participants
Summary
This PR makes the Electron.NET runtime and MSBuild targets support an additional "DotNet-First" packaged app layout and gives packaging builds a few more escape hatches.
Motivation
Today the runtime probes for the packaged layout
resources/bin/<app>(Electron-first). When the .NET publish output is the app root and the Electron host is placed under anelectronsubdirectory (DotNet-First),UnpackagedDetectormis-identifies the app as unpackaged andElectronProcessActivecannot find the Electron binary.In addition,
ElectronPackageId,Title, the intermediatePublishDir, and thenpm/npxexec steps are hard-coded, which makes it harder to plug Electron.NET into custom packaging pipelines or CI.Changes
DotNet-First layout detection in
UnpackagedDetector.CheckUnpackaged2:electronsubdirectory, treat the app as packaged.resources/bin(Electron-first) and.electron(development) checks are preserved.DotNet-First binary startup in
ElectronProcessActive.StartCore:dir/electronexists, startelectronfrom that directory and use it as the working directory.dir.Parent.Parentlogic for the Electron-first layout.Overridable build properties in
ElectronNET.Core.props:ElectronPackageIdandTitleonly default fromMSBuildProjectNamewhen the properties are not already set.Configurable publish/exec targets in
ElectronNET.LateImport.targets:ElectronPublishDircan override the intermediatePublishDir.ElectronSkipExecCommands(defaultfalse) can suppressnpm/npxexecution steps.Exectasks fornpm/npxnow respectElectronSkipExecCommands.Files changed
src/ElectronNET.API/Runtime/Helpers/UnpackagedDetector.cssrc/ElectronNET.API/Runtime/Services/ElectronProcess/ElectronProcessActive.cssrc/ElectronNET/build/ElectronNET.Core.propssrc/ElectronNET/build/ElectronNET.LateImport.targets