bug#38926: pcmanfm-qt unable to open files by double click

[Hartmut Goebel]

Am 05.01.20 um 23:37 schrieb Danny Milosavljevic:

Re. 
> No idea how to do that with Qt programs.  Hartmut?

For Qt/KDE I did not actually address this topic yet.

I tend to handle this case-by-case: If the coupling is "tight" and if
this is a hard requirement (or a small dependency) I'd substitute the
paths was you suggested. Otherwise I rely on the required package to be
installed my the user or some service definition.

In this special case: To avoid installing all of glib:bin (and thus
glib), we could create a new package "gio-launch-desktop", containing
only this binary.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel  | h.goe...@crazy-compilers.com   |
| www.crazy-compilers.com | compilers which you thought are impossible |




signature.asc
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Reza Alizadeh Majd]

related patch submitted: 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39004


-- 
Regards
Reza Alizadeh Majd
PantherX Team

bug#38926: pcmanfm-qt unable to open files by double click

[Reza Alizadeh Majd]

Hi Danny, 


> I don't think that will work.  According to the glib source code[1], the
> variable is read at runtime of the final program.

oh, your right !!! I incorrectly thought that GIO_LAUNCH_DESKTOP is a 
preprocessor macro. 
 
> What you could do is use your wrap-program in pcmanfm-qt, but with
> GIO_LAUNCH_DESKTOP (value like above) instead of PATH.  That should fix
> pcmanfm-qt and I don't see any forward incompatibility or side effects
> introduced by it either.  Also, glib would be unmodified.
> 
> The downside would be that we would have to patch other broken programs
> one by one (after *finding* them).
> 
> But I would still be in favor of that fix for now.  Could you try it and
> send a patch?

OK, so I fix and test pcmanfm-qt following this approach and submit the 
regarding patch.


-- 
Regards
Reza Alizadeh Majd
PantherX Team

bug#38926: pcmanfm-qt unable to open files by double click

[Danny Milosavljevic]

Hi Reza,

On Mon, 06 Jan 2020 20:15:43 +0330
"Reza Alizadeh Majd"  wrote:

> +(add-before 'build 'patch-gio-launch-desktop
> +   (lambda* (#:key outputs #:allow-other-keys)
> + (let ((bin-path (assoc-ref outputs "bin") "/bin"))
> +   (setenv "GIO_LAUNCH_DESKTOP"
> +   (string-append bin-path "/bin/gio-launch-desktop"))
> +   #t)))

I don't think that will work.  According to the glib source code[1], the
variable is read at runtime of the final program.

What you could do is use your wrap-program in pcmanfm-qt, but with
GIO_LAUNCH_DESKTOP (value like above) instead of PATH.  That should fix
pcmanfm-qt and I don't see any forward incompatibility or side effects
introduced by it either.  Also, glib would be unmodified.

The downside would be that we would have to patch other broken programs
one by one (after *finding* them).

But I would still be in favor of that fix for now.  Could you try it and
send a patch?

In the end I hope upstream realizes the insane situation that is the status
quo and fixes it on their end (see
https://gitlab.gnome.org/GNOME/glib/issues/1633 for a bug report of this
problem).

We could also use /bin/sh instead of gio-launch-desktop like the bug report
suggests, but I'm not keen on increasing the attack surface so much, not
after the security vulnerabilities in bash.

The source code of gio-launch-desktop.c is very small and DOES NOT LINK
TO GLIB so we could just make it an extra package independent of glib.

Or we could just put it into the "out" output even though it's an
executable.

I've posted a patch to core-updates that does the latter[2].

[1] 
https://gitlab.gnome.org/GNOME/glib/merge_requests/95/diffs?commit_id=742efe6232aba81c2c52c229c900a57ec2afedfd
[2] https://bugs.gnu.org/38994


pgp4pJv1uPTVT.pgp
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Reza Alizadeh Majd]

Hi, 

On Mon, Jan 6, 2020, at 8:08 PM, Danny Milosavljevic wrote:
> > Or, for a complete fix, patch glib-2.60.6/gio/gdesktopappinfo.c 
> > 
> >   tmp = "gio-launch-desktop"
> > 
> > to say
> > 
> >   tmp = "/gnu/store/xyz-glib-.../bin/gio-launch-desktop";
> > 
> > instead.  The latter has the advantage that it magically fixes ALL the 
> > applications.
> 
> When I try that, I get a circular dependency between the "bin" and the 
> "out" outputs.
> 
> To be continued...
>

I faced same issue. using `substitute*` .  

using following patch `glib` builds successfully:

--8<---cut here---start->8---
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index 528b67e6cf..9f34fe59bc 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -302,6 +302,12 @@ shared NFS home directories.")
 (number->string (parallel-job-count)))
 ;; Do not run tests marked as "flaky".
 (invoke "meson" "test" "--no-suite" "flaky")))
+(add-before 'build 'patch-gio-launch-desktop
+   (lambda* (#:key outputs #:allow-other-keys)
+ (let ((bin-path (assoc-ref outputs "bin") "/bin"))
+   (setenv "GIO_LAUNCH_DESKTOP"
+   (string-append bin-path "/bin/gio-launch-desktop"))
+   #t)))
 ;; TODO: meson does not permit the bindir to be outside of prefix.
 ;; See https://github.com/mesonbuild/meson/issues/2561
 ;; We can remove this once meson is patched.
--8<---cut here---end--->8---

but the build process takes too much time and I'm waitingfor other 
related packages to be built and test if the issue is resolved or not.


-- 
Regards
Reza Alizadeh Majd
PantherX Team

bug#38926: pcmanfm-qt unable to open files by double click

[Danny Milosavljevic]

> Or, for a complete fix, patch glib-2.60.6/gio/gdesktopappinfo.c 
> 
>   tmp = "gio-launch-desktop"
> 
> to say
> 
>   tmp = "/gnu/store/xyz-glib-.../bin/gio-launch-desktop";
> 
> instead.  The latter has the advantage that it magically fixes ALL the 
> applications.

When I try that, I get a circular dependency between the "bin" and the "out" 
outputs.

To be continued...


pgp_SiY_l1kJg.pgp
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Danny Milosavljevic]

See also https://gitlab.gnome.org/GNOME/glib/issues/1633 "should not install 
gio-launch-desktop into PATH"


pgpV7xgIARK7C.pgp
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Danny Milosavljevic]

In this case I've tried it myself.  I've invoked 

  gdb $(which pcmanfm-qt)

then 

  r

then I double-clicked an entry and waited until the error dialog window
appeared.

then I DIDN'T CLICK IT AWAY.

then I pressed Ctrl-C in gdb

then

  bt

and so I found:

[...]
#6  0x77ea1642 in Fm::FileLauncher::showError(_GAppLaunchContext*, 
Fm::GErrorPtr const&, Fm::FilePath const&, std::shared_ptr 
const&) () from 
/gnu/store/91d7kqg44cfzcav1l2wfn84hr310xvyx-libfm-qt-0.14.1/lib/libfm-qt.so.6
#7  0x77e88939 in Fm::BasicFileLauncher::launchWithApp(_GAppInfo*, 
std::vector > const&, 
_GAppLaunchContext*) () from 
/gnu/store/91d7kqg44cfzcav1l2wfn84hr310xvyx-libfm-qt-0.14.1/lib/libfm-qt.so.6
#8  0x77e8ae4d in Fm::BasicFileLauncher::launchFiles(Fm::FileInfoList 
const&, _GAppLaunchContext*) () from 
/gnu/store/91d7kqg44cfzcav1l2wfn84hr310xvyx-libfm-qt-0.14.1/lib/libfm-qt.so.6
#9  0x77ea11b7 in Fm::FileLauncher::launchFiles(QWidget*, 
Fm::FileInfoList const&) () from 
/gnu/store/91d7kqg44cfzcav1l2wfn84hr310xvyx-libfm-qt-0.14.1/lib/libfm-qt.so.6
#10 0x77ead5a2 in Fm::FolderView::onFileClicked(int, 
std::shared_ptr const&) () from 
/gnu/store/91d7kqg44cfzcav1l2wfn84hr310xvyx-libfm-qt-0.14.1/lib/libfm-qt.so.6
#11 0x0044aa55 in PCManFM::View::onFileClicked(int, 
std::shared_ptr const&) ()
[...]

Therefore, the interesting part would be either 
Fm::BasicFileLauncher::launchWithApp or Fm::BasicFileLauncher::launchFiles .

The former (inside package "libfm-qt") uses

  g_app_info_get_default_for_type

to get the app to use.  I suspect that that returns "gio-launch-desktop" 
without the full path--which is wrong. 

Checking glib source code, there's an environment variable GIO_LAUNCH_DESKTOP
that you can set (to the full path of gio-launch-desktop) for a first 
workaround.

Or, for a complete fix, patch glib-2.60.6/gio/gdesktopappinfo.c 

  tmp = "gio-launch-desktop"

to say

  tmp = "/gnu/store/xyz-glib-.../bin/gio-launch-desktop";

instead.  The latter has the advantage that it magically fixes ALL the 
applications.

I hope that helps.


pgpYagdlRKBmi.pgp
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Danny Milosavljevic]

Hi Reza,

I wouldn't change PATH in a *file manager*, of all programs.  That will make the
user hate his life should it ever pick up the wrong indirect child executable
because of that PATH change--which is opaque the user.

(Otherwise, often, your solution would have been the correct one--especially 
when
there are no indirect children)

Please try to find the actual source code line where it invokes
gio-launch-desktop .

For glib programs, I would suggest invoking

  gdb --args $(which pcmanfm-qt) --g-fatal-warnings

then

  r

and then wait until it crashes, then

  bt

.  It will tell you the exact location in the source code where the invocation
is.

No idea how to do that with Qt programs.  Hartmut?


pgp6MiXtnEy9I.pgp
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Reza Alizadeh Majd]

Hi,

> Unfortunately, that way is not acceptable.
> 
> Long story short, please make it a regular input.  The reasoning is below:
> 
> Guix has better modularity than most of the other distributions that exist.

OK, I got your point, so I think following patch could be acceptable now:

--8<---cut here---start->8---
diff --git a/gnu/packages/lxqt.scm b/gnu/packages/lxqt.scm
index 5a3708e30a..fdadb03dda 100644
--- a/gnu/packages/lxqt.scm
+++ b/gnu/packages/lxqt.scm
@@ -1000,7 +1000,17 @@ components to build desktop file managers which belongs 
to LXDE.")
  (substitute* '("autostart/CMakeLists.txt")
(("DESTINATION \"\\$\\{LXQT_ETC_XDG_DIR\\}")
 "DESTINATION \"etc/xdg"))
- #t)
+ #t))
+ (add-after 'install 'wrap-executable
+   (lambda* (#:key outputs inputs #:allow-other-keys)
+ (let ((out (assoc-ref outputs "out"))
+   (glib-bin (string-append (assoc-ref inputs "glib") "/bin")))
+   (display "[=] ")
+   (display glib-bin)
+   (newline)
+   (wrap-program (string-append out "/bin/pcmanfm-qt")
+ `("PATH" ":" prefix (,glib-bin)))
+   #t))
 (home-page "https://lxqt.org/;)
 (synopsis "File manager and desktop icon manager")
 (description "PCManFM-Qt is the Qt port of PCManFM, the file manager of
(END)
--8<---cut here---end--->8---

so please let me know, if any other issues are still exists. 

-- 
Regards
Reza Alizadeh Majd
PantherX Team

bug#38926: pcmanfm-qt unable to open files by double click

[Danny Milosavljevic]

Hi,

thanks for the patch.

Unfortunately, that way is not acceptable.

Long story short, please make it a regular input.  The reasoning is below:

Guix has better modularity than most of the other distributions that exist.

One of the main uses of modularity is isolation.  If one module uses something
internally, another module must not be disturbed by that fact.

If (executable) package Q requires (library) package B@0.0.1 internally and
   (executable) package R requires (library) package B@0.0.2 internally

then it works just fine to install both Q and R into the same profile.

Why?  Because B is not propagated into the profile (usually when B
is an implementation detail of Q and R and not part of their public
interface).

If, on the other hand, B were propagated then the profile would not be able to
build because the two Bs would conflict.

Debian, Gentoo, Redhat etcetc are different in that they basically ONLY have
propagated-inputs and don't have regular inputs.

They don't have the concept of an "internal" dependency.  All their
dependencies are external, changing the interface.

Looking at https://fossies.org/linux/glib/gio/gio-launch-desktop.c ,
gio-launch-desktop is very small and not connected to anything else.
Propagating it, with all the versioning problems that introduces, would be
unwise.

I propose instead to make glib bin a regular input of pcmanfm-qt and then
substitute* the entire path to gio-launch-desktop in it inside a phase
of pcmanfm-qt.

As a rule of thumb, if something that a regular user would have never heard
of ("glib" "bin" ... ???) makes it directly into his profile (by propagation),
it's probably the wrong thing to do.


pgpOFvLWsR5a4.pgp
Description: OpenPGP digital signature

bug#38926: pcmanfm-qt unable to open files by double click

[Reza Alizadeh Majd]

Hi, 

there is an issue with `pcmanfm-qt`. when we try to open a known file using
double click, we receive following error:

--8<---cut here---start->8---
Failed to execute child process “gio-launch-desktop” (No such file or directory)
--8<---cut here---end--->8---


since `gio-launch-desktop` is located in `bin` output of `glib` package,
adding that as a propagated input for `pcmanfm-qt` fixes this issue.

--8<---cut here---start->8---
diff --git a/gnu/packages/lxqt.scm b/gnu/packages/lxqt.scm
index 5a3708e30a..2337daa7c3 100644
--- a/gnu/packages/lxqt.scm
+++ b/gnu/packages/lxqt.scm
@@ -991,6 +991,8 @@ components to build desktop file managers which belongs to 
LXDE.")
  `(("pkg-config" ,pkg-config)
("qttools" ,qttools)
("lxqt-build-tools" ,lxqt-build-tools)))
+(propagated-inputs 
+  `(("glib" ,glib "bin")))
 (arguments
  '(#:tests? #f  ; no tests
#:phases
--8<---cut here---end--->8---

is this change is acceptable? if so I can submit a regarding patch to
`guix-patches` mailing list.


--
Regards
Reza Alizadeh Majd
PantherX Team