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

Windows installer: Add Registry entry with full executable path #4783

Closed
muellesi opened this issue Sep 7, 2018 · 16 comments

Comments

Projects
None yet
5 participants
@muellesi
Copy link

commented Sep 7, 2018

Over at GIMP, there are multiple issues with the RawTherapee import plugin. One of them - RawTherapee's console not closing without user interaction is supposed to be fixed with #4514 I think?

But there is still one problem remaining: As I wrote in https://gitlab.gnome.org/GNOME/gimp/issues/1237, the plugin uses either PATH or a designated environment variable to find the RawTherapee executable which means, it will never work "out of the box". And if the user has no idea how to modify environment variables, it will just never work.

GIMP's darktable plugin uses a registry entry, created by darktable to locate the corresponding binary - maybe this would be an option for RawTherapee, too?

During installation, darktable creates two new keys in HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths, see the image below
darktable

The GIMP plugin is then able to read the darktable.exe key to start darktable.

@Beep6581 Beep6581 added the enhancement label Sep 7, 2018

@Beep6581

This comment has been minimized.

Copy link
Owner

commented Sep 7, 2018

To be tested by someone with Windows:

diff --git a/tools/win/InnoSetup/WindowsInnoSetup.iss.in b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
index 11601a53d..814e337a9 100644
--- a/tools/win/InnoSetup/WindowsInnoSetup.iss.in
+++ b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
@@ -133,3 +133,4 @@ Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee";
 Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -w ""%1"""; Flags: uninsdeletekey; Tasks: regBrowseFolder
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee"; ValueType: string; ValueData: "Open with RawTherapee"; Flags: uninsdeletekey; Tasks: regOpenFile
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -R ""%1"""; Flags: uninsdeletekey; Tasks: regOpenFile
+Root: HKCU; Subkey: "SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\rawtherapee.exe"; ValueType: string; ValueData: "{app}\{#MyAppExeName}"; Flags: uninsdeletekey
@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 7, 2018

Thanks for the quick reaction! I just tested it and the key gets created correctly. The only problem is that the GIMP helper function that does the registry lookup has HKLM hard coded for the root of the key (which seems to be the standard: On my PC, the HKLM version has 80 subkeys, the HKCU version only 5). Since this patch creates the new value in HKCU, this helper function would not work the way it is implemented right now. But if you don't want to change anything system-wide (HKCU is user-dependent, HKLM is system wide) I think it wouldn't be a big problem to simply change the helper function.

Otherwise, just change HKCU to HKLM:

diff --git a/tools/win/InnoSetup/WindowsInnoSetup.iss.in b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
index 11601a53d..814e337a9 100644
--- a/tools/win/InnoSetup/WindowsInnoSetup.iss.in
+++ b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
@@ -133,3 +133,4 @@ Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee";
 Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -w ""%1"""; Flags: uninsdeletekey; Tasks: regBrowseFolder
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee"; ValueType: string; ValueData: "Open with RawTherapee"; Flags: uninsdeletekey; Tasks: regOpenFile
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -R ""%1"""; Flags: uninsdeletekey; Tasks: regOpenFile
+Root: HKLM; Subkey: "SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\rawtherapee.exe"; ValueType: string; ValueData: "{app}\{#MyAppExeName}"; Flags: uninsdeletekey
@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 7, 2018

Oh and I think rawtherapee.exe could also be replaced with {#MyAppExeName}:

diff --git a/tools/win/InnoSetup/WindowsInnoSetup.iss.in b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
index 11601a53d..814e337a9 100644
--- a/tools/win/InnoSetup/WindowsInnoSetup.iss.in
+++ b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
@@ -133,3 +133,4 @@ Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee";
 Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -w ""%1"""; Flags: uninsdeletekey; Tasks: regBrowseFolder
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee"; ValueType: string; ValueData: "Open with RawTherapee"; Flags: uninsdeletekey; Tasks: regOpenFile
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -R ""%1"""; Flags: uninsdeletekey; Tasks: regOpenFile
+Root: HKLM; Subkey: "SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{#MyAppExeName}"; ValueType: string; ValueData: "{app}\{#MyAppExeName}"; Flags: uninsdeletekey
@Beep6581

This comment has been minimized.

Copy link
Owner

commented Sep 7, 2018

HKLM requires admin privileges. As far as I know, the RT installer does not require admin privileges, so using HKLM would force it to. If that is the case, then in my opinion it is the helper function which needs to be patched.

@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 7, 2018

Ok, good point. It shouldn't be a big problem to change that helper. Thanks again for the quick responses :)

@TooWaBoo

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

@Beep6581
The RT installer require admin privileges 'cause it's installing into the Program folder. Every application which wants to write into this folder needs admin privileges.

@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 7, 2018

@TooWaBoo I tried this before writing my last comment: You actually can install RawTherapee without admin privileges. If you are on a limited user account, it does not show the UAC panel and lets you install to any folder your current user has access to. Trying to install to the Program folder will fail with an access denied error of course.

@TooWaBoo

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

@muellesi
Yes, you are right but the Program folder is the default. I also suggest to install into the Program folder for security reason. If installing into other folders with low privileges, it's not protected by UAC.

@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 10, 2018

Alright, I proposed a patch to the GIMP project that should work with the key in HKCU: https://gitlab.gnome.org/GNOME/gimp/issues/2179

I hope I will be able to test both patches in combination within the next few days.

@TooWaBoo Of course it is always better to use the Programs folder. But if it was possible to install RT without admin privileges until now, I get why Beep6581 wouldn't want to change this for this simple patch.

@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 10, 2018

Ok, I had some spare time and thanks to user qogniw in IRC, I was able to compile and package RT to test everything. And so far it looks good. RT is detected by GIMP out of the box, there is no blocking console window anymore and I was able to open a .CR2 (Canon RAW format) file in GIMP via RT.

This is the change I did my tests with:

 tools/win/InnoSetup/WindowsInnoSetup.iss.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/win/InnoSetup/WindowsInnoSetup.iss.in b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
index 11601a53d..486a63d81 100644
--- a/tools/win/InnoSetup/WindowsInnoSetup.iss.in
+++ b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
@@ -133,3 +133,4 @@ Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee";
 Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -w ""%1"""; Flags: uninsdeletekey; Tasks: regBrowseFolder
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee"; ValueType: string; ValueData: "Open with RawTherapee"; Flags: uninsdeletekey; Tasks: regOpenFile
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -R ""%1"""; Flags: uninsdeletekey; Tasks: regOpenFile
+Root: HKCU; Subkey: "SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{#MyAppExeName}"; ValueType: string; ValueData: "{app}\{#MyAppExeName}"; Flags: uninsdeletekey

gnomesysadmins pushed a commit to GNOME/gimp that referenced this issue Sep 10, 2018

Simon Mueller Jehan
Issue #2179: Make file_rawtherapee use the registry value that is...
... provided by RawTherapee's installer (version 5.5+).

See Beep6581/RawTherapee#4783.
This patch required a small change to file_raw_get_executable_path
because the RawTherapee installer is supposed to work without admin
privileges and therefore can't write to HKLM.

Reviewer's note (Jehan): RawTherapee's installer does not add the
registry entry yet. We assume the upstream bug report will end up doing
so (someone has to make the first step!). :-)
@Jehan

This comment has been minimized.

Copy link

commented Sep 10, 2018

Hi All!

A quick note to say that we merged @muellesi's patch in GIMP's source. All we need is to make sure that RawTherapeee actually sets the key now (otherwise, it's useless). :-)

As for the HKCU vs HKLM discussion: as far as I understand, RT can be installed both with admin or user privileges. If installed as admin (hence in Program Folder), is the key set in HKCU set for all users (correct me if I am wrong, as my knowledge of Windows registry is low, but I assume that HKCU keys are per-user whereas HKLM keys are shared)? Or only for the admin account? If the later, then we would still end up with a broken link between GIMP and RawTherapee when running from an account different from the install account.

If this is confirmed, maybe it would be worth to install the key as HKLM when installing with admin privileges, and as HKCU when installing as simple user.
In GIMP code, this would be a 2 secs-change, and basically we would just give priority to HKCU key if found, then fallback to the system HKLM key.

@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 10, 2018

Oh, I just realized that the plugin also looks for the cli for thumbnail generation, so a second key would be required:

 tools/win/InnoSetup/WindowsInnoSetup.iss.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/win/InnoSetup/WindowsInnoSetup.iss.in b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
index 11601a53d..4375fb6c8 100644
--- a/tools/win/InnoSetup/WindowsInnoSetup.iss.in
+++ b/tools/win/InnoSetup/WindowsInnoSetup.iss.in
@@ -24,6 +24,7 @@
 #define MyAppPublisher "rawtherapee.com"
 #define MyAppURL "http://www.rawtherapee.com/"
 #define MyAppExeName "rawtherapee.exe"
+#define MyAppCliExeName "rawtherapee-cli.exe"
 #define MyBuildBasePath "${CMAKE_INSTALL_PREFIX}"
 #define MySourceBasePath "${PROJECT_SOURCE_DIR}"
 #define MyBitDepth "${BUILD_BIT_DEPTH}"
@@ -133,3 +134,5 @@ Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee";
 Root: HKCU; Subkey: "SOFTWARE\Classes\Directory\shell\Browse with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -w ""%1"""; Flags: uninsdeletekey; Tasks: regBrowseFolder
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee"; ValueType: string; ValueData: "Open with RawTherapee"; Flags: uninsdeletekey; Tasks: regOpenFile
 Root: HKCU; Subkey: "SOFTWARE\Classes\*\shell\Open with RawTherapee\command"; ValueType: string; ValueData: "{app}\{#MyAppExeName} -R ""%1"""; Flags: uninsdeletekey; Tasks: regOpenFile
+Root: HKCU; Subkey: "SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{#MyAppExeName}"; ValueType: string; ValueData: "{app}\{#MyAppExeName}"; Flags: uninsdeletekey;
+Root: HKCU; Subkey: "SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{#MyAppCliExeName}"; ValueType: string; ValueData: "{app}\{#MyAppCliExeName}"; Flags: uninsdeletekey;

Regarding the dynamic HKLM/HKCU switching: I just read through the inno setup documentation and some StackOverflow Posts and did not find a way to conveniently switch between install options (i.e. Creating registry keys in different root keys) depending on the user's privileges. If you allow a normal user to start the installer, you have to make sure that every task can actually be executed by a normal user.

@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 10, 2018

Ok, I think I found a solution that should work after all:

muellesi@163c3ab

This patch adds two new Radio boxes to the setup that allows the user to switch between multi user and single user setup. As you see, I also use that option to decide where to create all of the other relevant items like the desktop icon and all of the context menu entries.

If you guys are ok with those changes, I can submit a pull request :)

@heckflosse

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2018

👍 for a pull request

gnomesysadmins pushed a commit to GNOME/gimp that referenced this issue Sep 19, 2018

Simon Mueller Jehan
Issue #2179: Make file_rawtherapee use the registry value that is...
... provided by RawTherapee's installer (version 5.5+).

See Beep6581/RawTherapee#4783.
This patch required a small change to file_raw_get_executable_path
because the RawTherapee installer is supposed to work without admin
privileges and therefore can't write to HKLM.

Reviewer's note (Jehan): RawTherapee's installer does not add the
registry entry yet. We assume the upstream bug report will end up doing
so (someone has to make the first step!). :-)

(cherry picked from commit 829ca65)
@muellesi

This comment has been minimized.

Copy link
Author

commented Sep 19, 2018

The required changes to the GIMP plug-in are also merged: https://gitlab.gnome.org/GNOME/gimp/issues/2179
Feel free to test both patches together :)

@Beep6581

This comment has been minimized.

Copy link
Owner

commented Sep 19, 2018

Thank you @muellesi

@Beep6581 Beep6581 closed this Sep 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.