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

fix: `Application.ExecutablePath` returns dll instead of exe (#2801) #2838

Closed

Conversation

@RussKie
Copy link
Member

RussKie commented Feb 10, 2020

Resolves #1143

For discussions and tests refer to #2801
(cherry picked from commit 2af3af9)

Proposed changes

In .NET artifacts are DLLs even for executable projects. With some automagic they get bundled into executables.
However Assembly.GetEntryAssembly() always returns the dll instead of the exe.

Following the guidance from the Runtime team retrieve the path to the executable via GetModuleFileNameW call.

Customer Impact

  • Customers can now retrieve a path to the applications executable instead of the application's dll.

Regression?

  • Yes

Risk

  • Small-medium due to possibly unaccounted use cases

Test methodology

Microsoft Reviewers: Open in CodeFlow
@RussKie RussKie added this to the 3.1.x milestone Feb 10, 2020
@RussKie RussKie requested a review from dotnet/dotnet-winforms as a code owner Feb 10, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 10, 2020

Codecov Report

Merging #2838 into release/3.1 will increase coverage by 0.00998%.
The diff coverage is 0%.

@@                 Coverage Diff                  @@
##           release/3.1      #2838         +/-   ##
====================================================
+ Coverage     24.85382%   24.8638%   +0.00997%     
====================================================
  Files              844        844                 
  Lines           260290     260274         -16     
  Branches         36896      36894          -2     
====================================================
+ Hits             64692      64714         +22     
+ Misses          190874     190837         -37     
+ Partials          4724       4723          -1
Flag Coverage Δ
#Debug 24.8638% <0%> (+0.00998%) ⬆️
#production 24.8638% <0%> (+0.00998%) ⬆️
#test ?
In .NET artifacts are DLLs even for executable projects. With some automagic
they get bundled into executables.
However `Assembly.GetEntryAssembly()` always returns the dll instead of the exe.

Following the guidance from the Runtime team retrieve the path to the
executable via `GetModuleFileNameW` call.

Resolves #1143

(cherry picked from commit 2af3af9)
@RussKie RussKie force-pushed the RussKie:fix_Application.ExecutablePath branch from a17e689 to abd221b Feb 11, 2020
}
}
StringBuilder sb = UnsafeNativeMethods.GetModuleFileNameLongPath(NativeMethods.NullHandleRef);
executablePath = Path.GetFullPath(sb.ToString());

This comment has been minimized.

Copy link
@davkean

davkean Feb 11, 2020

Member

If this does return a relative path at any point, GetFullPath is going to resolve it by whatever the current directory is, which may or may not be the application directory:

C:\>cd NotApplicationDir
C:\NotApplicationDir> C:\MyApp\App.exe

If GetModuleFileNameLongPath actually returns a relative path at any point, this will return C:\NotApplicationDir\App.exe.

This comment has been minimized.

Copy link
@davkean

davkean Feb 11, 2020

Member

From my reading, it probably doesn't, and you are doing this just to normalize the path, right?

This comment has been minimized.

Copy link
@RussKie

RussKie Feb 11, 2020

Author Member

I have tested a number of different configurations and posted results here, here and here.

Please let me know if I missed anything.

This comment has been minimized.

Copy link
@RussKie

RussKie Feb 11, 2020

Author Member

From my reading, it probably doesn't, and you are doing this just to normalize the path, right?

I believe so too. That's how the original code had it. It may be redundant but I'm not going to change it on a service branch. :)

This comment has been minimized.

Copy link
@weltkante

weltkante Feb 11, 2020

Contributor

doc says

Retrieves the fully qualified path

which rules out relative paths (actually I commented on this on the other PR)

Copy link
Member

Shyam-Gupta left a comment

LGTM

@merriemcgaw

This comment has been minimized.

Copy link
Member

merriemcgaw commented Feb 13, 2020

This is good for 5.0, but until we find out that it's blocking servicing, I think that this won't meet a servicing bar for 3.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.