-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add support for .net core target .exes #1692
Conversation
Would love to see this pulled in! |
@mungojam |
@anaisbetts this one seems to be popular, do you think you will have a chance to look at this? |
@anaisbetts This issue is keeping us from moving our Squirrel distributed app to .NET Core (or 6 when it is available). CreateDump.exe is now included (and required without some json editing) in published builds, so even if our app is one exe Squirrel sees both and adds a shortcut for "Microsoft .NET" to the desktop. A fix is needed to mark our app as "SquirrelAware". This PR seems to address this. If there is a better workaround (i.e. some other way of telling Squirrel to only create shortcuts for one 'non C#' app - which seems to be the recommended way to use Squirrel with .NET Core apps) we would be very grateful for the guidance. |
Part 1 is an easy win, I'd take that PR. Part 2 I still do not understand why .NET Core does this, and I'm a little squicked out opening up random DLLs and hoping they're related |
I could split the PR if that would help, but note that with just Part 1, people would need to add a post-build step to actually make use of it by calling rcedit to add the SquirrelAware attribute.
@anaisbetts, with .net core 1 and 2, I think it originally forced you to run the DLLs and didn't even have an exe. With .net core 3.0 (I think), they added the launcher .exe that then loads the DLL. So the DLL is really where your code lives and will always be named the same as the exe. Would it help if it somehow detected for sure that the .exe is a .net core wrapper exe? I don't know how easy that is though or if it would be fragile to different .net core versions. |
Yeah, I think that would be a bit safer |
@ComtelJeremy, with your offer of help. If there is any research you are able to do into this aspect that would be helpful. I will try and get to looking at it in the next few days, but no guarantees |
@mungojam Taking a quick look, I noticed that in our project in the properties for the .exe there is a property called "Original filename". It has the .dll name and extension in it... Would this be enough to make us look if there is a matching .dll with that name in the package? This might not be bulletproof, or futureproof, but might be a good sanity check before checking the .dll for attributes. See below: |
Nice, thanks! That would work for me, how about you @anaisbetts? I can try and implement that in the next few days. |
I would actually try to analyze the executable itself to try to find some sign that it is a renamed dotnet.exe. I'll look into it soon |
A few things to help with the decision:
|
@mungojam Were you going to split the PR? With Part 1 at least we would be able to move forward with marking one of the exes in our app as SquirrelAware. We don't mind a post-build step...I already do one to releasify. I would simply add that step to the script I have already. @anaisbetts Anything we can do to help with a possible Part 2? |
Sure, I'll try and put that in this afternoon. It's good to know it would be used |
@mungojam This PR doesn't solve the CreateDump.exe problem as far as I can see. What was your plan to solve that part? |
Never mind. I see that this PR works when combined with an app that includes: |
I don't see much traction on this repo. To get unblocked I built this PR locally and pushed the package to my own public feed so I can consume it. I share here in case anyone else wants to consume it as well. |
* Add status bar * Build squirrel installers * Push installer to github releases * Suppress pushing to nuget.org * Bump version to 0.2 This is necessary because 0.1 has history in `src` directly, so versions were inconsistent across the repo. * Calculate the version only once, from the repo root * Consume Squirrel from Squirrel/Squirrel.Windows#1692
Note that .net 6 resolves the specific createdump problem to a degree as it no longer lists it as a dependency so it can be deleted as a post build, pre squirrelise step. |
@anaisbetts Just wondering if this might be merged and released and/or #1732 might be released in the near future... We are in the middle of upgrading our .net framework squirrel installed app to core, and it contains two exes (one called by the first exe for elevated tasks). Having a working SquirrelAwareVersion assembly attribute will prevent us having to do some exe renaming before and after install. Also happy for any other workaround you or others can suggest. We love squirrel and want to keep using it! That being said, if any help is needed, please point me in the right direction and I'll do my very best. |
@anaisbetts I've just spotted the comments on #1752 (comment) about this one. I wonder if I can help resolve the concern around it. It only goes looking for the partner dll if it doesn't see the squirrel aware flag on the .exe. And it will only then report the DLL if the DLL has the squirrel aware flag. I don't see what the risk of doing that would be with other languages. If for some reason they have flagged the DLL then it wouldn't serve any purpose at present. |
@mungojam FYI my fork has removed the dependency on mono.cecil and now has several other ways of detecting a binary as squirrel aware that are friendly to net framework, dotnet core, publishsinglefile, and are even consistent with how it's done for native binaries, so this change is not needed. The preferred way to mark a binary as SquirrelAware is now an added line in the assembly manifest - which is easy to do for dotnet and native binaries alike. There are also many other fixes in this repo and it is being actively maintained. |
Closing this to reduce my open PR noise and as it doesn't look likely to get merged ever. The fork mentioned by @caesay has now become https://github.com/velopack/velopack which is worth a look |
Fixes .net core .exes are not detected as squirrel apps even with SquirrelAwareVersion #1644, adding direct support for .net core exes by looking for the assembly custom attribute in the actual .net backing DLL rather than in the native .exe entry point that .net core builds create.
I've added a .dll and some .exes as test fixtures, but I can understand if you want to reproduce these yourself for security reasons.