bug#50789: syncthing-gtk creates autostart file with wrong bin

bug#50789: syncthing-gtk creates autostart file with wrong bin

[John Kehayias via Bug reports for GNU Guix]

Hello,

syncthing-gtk has an option to enable autostart, which it does by creating the .desktop file in ~/.config/autostart However, this has the wrong exec line, getting the -real script instead of syncthing-gtk. This won't work as it needs to be run as syncthing-gtk. Namely, it produces:

Exec=/gnu/store/vf5h9jqhq40x8r46afaa0jgw7awg1361-syncthing-gtk-0.9.4.4-1.c46fbd8/bin/.syncthing-gtk-real

Instead of

Exec=/gnu/store/vf5h9jqhq40x8r46afaa0jgw7awg1361-syncthing-gtk-0.9.4.4-1.c46fbd8/bin/syncthing-gtk

This is due to syncthing-gtk getting its executable name to write to the .desktop file in get_executable(): https://salsa.debian.org/debian/syncthing-gtk/-/blob/master/syncthing_gtk/tools.py#L409 where due to wrapping it will find something that won't work when run directly.

How should this be solved in Guix? Should we patch this part of the code to explicitly rewrite the path to have "syncthing-gtk" instead of ".syncthing-gtk-real"? Related would be the discussion at https://lists.gnu.org/r/guix-devel/2021-09/msg00088.html which I will message separately.

Thanks,
John

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Liliana Marie Prikler]

Hi,

Am Freitag, den 24.09.2021, 21:33 +0000 schrieb John Kehayias:
> Hello,
> 
> syncthing-gtk has an option to enable autostart, which it does by
> creating the .desktop file in ~/.config/autostart However, this has
> the wrong exec line, getting the -real script instead of syncthing-
> gtk. This won't work as it needs to be run as syncthing-gtk. Namely,
> it produces:
> 
> Exec=/gnu/store/vf5h9jqhq40x8r46afaa0jgw7awg1361-syncthing-gtk-
> 0.9.4.4-1.c46fbd8/bin/.syncthing-gtk-real
> 
> Instead of
> 
> Exec=/gnu/store/vf5h9jqhq40x8r46afaa0jgw7awg1361-syncthing-gtk-
> 0.9.4.4-1.c46fbd8/bin/syncthing-gtk
> 
> This is due to syncthing-gtk getting its executable name to write to
> the .desktop file in get_executable(): 
> https://salsa.debian.org/debian/syncthing-gtk/-/blob/master/syncthing_gtk/tools.py#L409
> where due to wrapping it will find something that won't work when run
> directly.
> 
> How should this be solved in Guix? Should we patch this part of the
> code to explicitly rewrite the path to have "syncthing-gtk" instead
> of ".syncthing-gtk-real"? Related would be the discussion at 
> https://lists.gnu.org/r/guix-devel/2021-09/msg00088.html which I will
> message separately.
We should patch syncthing to not write a desktop file at all.  Note how
even if the binary name itself was correct, syncthing would link
directly into the store, potentially producing a stale desktop file
that breaks with garbage collection.

Cheers

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Leo Famulari]

On Sat, Sep 25, 2021 at 12:55:56AM +0200, Liliana Marie Prikler wrote:
> We should patch syncthing to not write a desktop file at all.  Note how
> even if the binary name itself was correct, syncthing would link
> directly into the store, potentially producing a stale desktop file
> that breaks with garbage collection.

Maybe we could make the desktop file execute a path like
"$HOME/.guix-profile/bin/syncthing-gtk"?

bug#50789: syncthing-gtk creates autostart file with wrong bin

[John Kehayias via Bug reports for GNU Guix]

Hi Leo, Liliana, et al,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Friday, September 24th, 2021 at 7:15 PM, Leo Famulari wrote:

> Maybe we could make the desktop file execute a path like
>
> "$HOME/.guix-profile/bin/syncthing-gtk"?

Hmm...these are both good points. And I think I may have run into stale files produced by some programs due to this. Actually, checking now, I see the same for redshift-gtk in the autostart .desktop file it makes (a /gnu/store link). So this is perhaps more common when programs create files like this.

I think there are a few possibilities and assumptions that go with them.

1. Rely on $PATH so that it can just be Exec=syncthing-gtk This is pretty common (non-Guix, at least) I think, but is an assumption. Seems safe for a program like this, but ambiguous, especially with Guix allowing multiple of the same-named bin to be installed at the same time.

2. Something like what was suggested by Leo, but we have to consider different profiles. Is that easy to account for in a build time patch? In my case it ends up $HOME/.config/guix/profiles/desktop/desktop/bin/syncthing-gtk Though I guess this happens at install. Perhaps the Python code can be patched to make something like this, based on its runtime path.

3. Remove the ability for these types of files to be created, as Liliana suggested. I think this is drastic though, changing the expected behavior in programs and difficult to enforce evenly. The point raised is important though, that direct /gnu/store links shouldn't be used in such a way.

Perhaps there are others. What do we currently do in Guix, or what would be preferred? From a user standpoint, I'd expect creating an autostart file (for instance) to work as created and to continue to do so.

John

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Liliana Marie Prikler]

Hi John,

Am Samstag, den 25.09.2021, 00:45 +0000 schrieb John Kehayias:
> [...]
> 
> 3. Remove the ability for these types of files to be created, as
> Liliana suggested. I think this is drastic though, changing the
> expected behavior in programs and difficult to enforce evenly. The
> point raised is important though, that direct /gnu/store links
> shouldn't be used in such a way.
> 
> Perhaps there are others. What do we currently do in Guix, or what
> would be preferred? From a user standpoint, I'd expect creating an
> autostart file (for instance) to work as created and to continue to
> do so.
I think the answer to that one is fairly simple.  Guix should provide a
mechanism to generate/link autostart services, ideally via (guix home),
which will be upstreamed soon (idk if it has one such service already,
just throwing it out there).  However, the preferred solution is to not
bother with autostart at all and instead use shepherd to manage user
services – there is a tradeoff to be made between those management
styles somewhere.

Cheers

bug#50789: syncthing-gtk creates autostart file with wrong bin

[John Kehayias via Bug reports for GNU Guix]

Hi everyone,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Saturday, September 25th, 2021 at 3:02 AM, Liliana Marie Prikler wrote:

> I think the answer to that one is fairly simple. Guix should provide a
> mechanism to generate/link autostart services, ideally via (guix home),
> which will be upstreamed soon (idk if it has one such service already,
> just throwing it out there). However, the preferred solution is to not
> bother with autostart at all and instead use shepherd to manage user
> services – there is a tradeoff to be made between those management
> styles somewhere.
>

I agree we can have a better "Guix way" of doing autostart, documented with examples (perhaps a Cookbook chapter on Desktop usage of Guix). User run shepherd works, at least for some basic background services I run. I don't think it is as documented or as fleshed out as XDG autostart stuff. I too am interested in guix home (just merged!) and will take a look at that.

But back to the matter at hand. Medium to long-term I support better Guix services for autostart, but that doesn't address the problem of having packages run as intended by upstream, at least with reasonable expectations. I think this is expected and reasonable behavior, that a program can create a proper .desktop file in Guix.

Looking at another non-Guix system, the autostart files I have in  ~/.confg/autostart mostly (syncthing-gtk being the main exception) use just

Exec=program-name

I see this mostly true for /etc/xdg/autostart as well (non-Guix system). So I think this is an easy and typical behavior we can implement. In this case patching syncthing-gtk to produce Exec=syncthing-gtk. Perhaps upstream would consider it as well, unless they have good reason for a full path here, as opposed to other programs. (Upstream is a bit quiet in activity though.)

What do we think?

John

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Liliana Marie Prikler]

Hi,

Am Montag, den 27.09.2021, 19:04 +0000 schrieb John Kehayias:
> [...]
> 
> But back to the matter at hand. Medium to long-term I support better
> Guix services for autostart, but that doesn't address the problem of
> having packages run as intended by upstream, at least with reasonable
> expectations. I think this is expected and reasonable behavior, that
> a program can create a proper .desktop file in Guix.
Unless you are running a dedicated desktop file/autostart editor like
alacarte or whatever gnome-tweaks has going for it, writing to
.config/autostart is not reasonable behaviour.

> Looking at another non-Guix system, the autostart files I have
> in  ~/.confg/autostart mostly (syncthing-gtk being the main
> exception) use just
> 
> Exec=program-name
The "full path" desktop files used in Guix do have some advantages. 
Also, on other distros when using stuff like systemd in a similar
manner to shepherd, you have full paths again.

> I see this mostly true for /etc/xdg/autostart as well (non-Guix
> system). So I think this is an easy and typical behavior we can
> implement. In this case patching syncthing-gtk to produce
> Exec=syncthing-gtk. Perhaps upstream would consider it as well,
> unless they have good reason for a full path here, as opposed to
> other programs. (Upstream is a bit quiet in activity though.)
> 
> What do we think?
I think upstream as good reasons to use full paths, e.g. to prevent the
wrong syncthing from being used when there are two in /usr and
/usr/local.  Were Guix to police upstream on this matter, the decision
would clearly be to make this feature optional at the click of a button
– and not one that annoyingly pops up if Icecat is not your default
browser, if you understand what I'm trying to say.

Regards

bug#50789: syncthing-gtk creates autostart file with wrong bin

[John Kehayias via Bug reports for GNU Guix]

Hello,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Monday, September 27th, 2021 at 5:23 PM, Liliana Marie Prikler wrote:

> Hi,
>
> Am Montag, den 27.09.2021, 19:04 +0000 schrieb John Kehayias:
>
> > [...]
> >
> > But back to the matter at hand. Medium to long-term I support better
> > Guix services for autostart, but that doesn't address the problem of
> > having packages run as intended by upstream, at least with reasonable
> > expectations. I think this is expected and reasonable behavior, that
> > a program can create a proper .desktop file in Guix.
>
> Unless you are running a dedicated desktop file/autostart editor like
> alacarte or whatever gnome-tweaks has going for it, writing to
> .config/autostart is not reasonable behaviour.
>

Sorry, I don't understand this comment. I think this is just the XDG specification, user autorun desktop files go in ~/.config/autostart. If a program wants to autostart (e.g. user enables the option) this I think is the expected and conformant behavior. I see many programs doing this (and is where you'd write if you want to change the per-user behavior of something in the system autostart, which might also be relevant). Anyway, we are getting sidetracked and that's not important for the matter at hand, I don't think.

> > Looking at another non-Guix system, the autostart files I have
> > in ~/.confg/autostart mostly (syncthing-gtk being the main
> > exception) use just
> >
> > Exec=program-name
>
> The "full path" desktop files used in Guix do have some advantages.
> Also, on other distros when using stuff like systemd in a similar
> manner to shepherd, you have full paths again.
>
> > I see this mostly true for /etc/xdg/autostart as well (non-Guix
> > system). So I think this is an easy and typical behavior we can
> > implement. In this case patching syncthing-gtk to produce
> > Exec=syncthing-gtk. Perhaps upstream would consider it as well,
> > unless they have good reason for a full path here, as opposed to
> > other programs. (Upstream is a bit quiet in activity though.)
> >
> > What do we think?
>
> I think upstream as good reasons to use full paths, e.g. to prevent the
> wrong syncthing from being used when there are two in /usr and
> /usr/local. Were Guix to police upstream on this matter, the decision
> would clearly be to make this feature optional at the click of a button
> – and not one that annoyingly pops up if Icecat is not your default
> browser, if you understand what I'm trying to say.
>

So then I think we end up wanting to patch syncthing-gtk to create something like Exec=/path/to/profile/bin/syncthing-gtk I'm not sure off the top of my head how to do that (without ending up as it currently does with the direct /gnu/store/.../.syncthing-gtk-real since that is what the program will see as the origin of the process). I can take a look if no one has the Python incantation ready. Or else default to using $PATH and/or which; probably the right thing most of the time but I'm not sure.

John

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Liliana Marie Prikler]

Hi,

Am Donnerstag, den 30.09.2021, 19:29 +0000 schrieb John Kehayias:
> Hello,
> 
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> 
> On Monday, September 27th, 2021 at 5:23 PM, Liliana Marie Prikler
> wrote:
> 
> > Hi,
> > 
> > Am Montag, den 27.09.2021, 19:04 +0000 schrieb John Kehayias:
> > 
> > > [...]
> > > 
> > > But back to the matter at hand. Medium to long-term I support
> > > better Guix services for autostart, but that doesn't address the
> > > problem of having packages run as intended by upstream, at least
> > > with reasonable expectations. I think this is expected and
> > > reasonable behavior, that a program can create a proper .desktop
> > > file in Guix.
> > 
> > Unless you are running a dedicated desktop file/autostart editor
> > like alacarte or whatever gnome-tweaks has going for it, writing to
> > .config/autostart is not reasonable behaviour.
> > 
> 
> Sorry, I don't understand this comment. I think this is just the XDG
> specification, user autorun desktop files go in ~/.config/autostart.
> If a program wants to autostart (e.g. user enables the option) this I
> think is the expected and conformant behavior. I see many programs
> doing this (and is where you'd write if you want to change the per-
> user behavior of something in the system autostart, which might also
> be relevant). Anyway, we are getting sidetracked and that's not
> important for the matter at hand, I don't think.
Just because many programs do this doesn't mean they have a good
rationale to do so.  Managing autostarts joins the ranks of having an
updater for the program itself and messing with context menus or
mimetype associations.  This is not the Microsoft OS, we have better
ways of managing things here.

> > > Looking at another non-Guix system, the autostart files I have
> > > in ~/.confg/autostart mostly (syncthing-gtk being the main
> > > exception) use just
> > > 
> > > Exec=program-name
> > 
> > The "full path" desktop files used in Guix do have some advantages.
> > Also, on other distros when using stuff like systemd in a similar
> > manner to shepherd, you have full paths again.
> > 
> > > I see this mostly true for /etc/xdg/autostart as well (non-Guix
> > > system). So I think this is an easy and typical behavior we can
> > > implement. In this case patching syncthing-gtk to produce
> > > Exec=syncthing-gtk. Perhaps upstream would consider it as well,
> > > unless they have good reason for a full path here, as opposed to
> > > other programs. (Upstream is a bit quiet in activity though.)
> > > 
> > > What do we think?
> > 
> > I think upstream as good reasons to use full paths, e.g. to prevent
> > the wrong syncthing from being used when there are two in /usr and
> > /usr/local. Were Guix to police upstream on this matter, the
> > decision would clearly be to make this feature optional at the
> > click of a button – and not one that annoyingly pops up if Icecat
> > is not your default browser, if you understand what I'm trying to
> > say.
> > 
> 
> So then I think we end up wanting to patch syncthing-gtk to create
> something like Exec=/path/to/profile/bin/syncthing-gtk I'm not sure
> off the top of my head how to do that (without ending up as it
> currently does with the direct /gnu/store/.../.syncthing-gtk-real
> since that is what the program will see as the origin of the
> process). I can take a look if no one has the Python incantation
> ready. Or else default to using $PATH and/or which; probably the
> right thing most of the time but I'm not sure.
As I stated initially you could hardcode the store path to syncthing-
gtk in its stead but it's still a store path in the end.  It must go
stale by design.  The only reasonable thing is to not install the file
and instead give users the tools to do so on their own.

Regards

bug#50789: syncthing-gtk creates autostart file with wrong bin

[John Kehayias via Bug reports for GNU Guix]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Thursday, September 30th, 2021 at 4:00 PM, Liliana Marie Prikler wrote:

>
> As I stated initially you could hardcode the store path to syncthing-
> gtk in its stead but it's still a store path in the end. It must go
> stale by design. The only reasonable thing is to not install the file
> and instead give users the tools to do so on their own.
>

For a medium to long-term solution, what would you advocate for? A note in a package's description that they should see the manual or cookbook for how to autostart this program if they want (since it will show up in the preferences menu)? Of course, we'll need that documentation first :) Maybe an example piece of code for using Shepherd to start, or guix home (I still need to look at that)? Perhaps this should be raised more generally in the devel list to be consistent in Guix, since this won't be the only one.

In the meantime, I'll try to find if getting the profile to bin path is not too tricky, if not at least make it so the path it does use points to syncthing-gtk and not .syncthing-gtk-real, since that is incorrect in every way.

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Liliana Marie Prikler]

Hi,

Am Donnerstag, den 30.09.2021, 20:13 +0000 schrieb John Kehayias:
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> 
> On Thursday, September 30th, 2021 at 4:00 PM, Liliana Marie Prikler
> wrote:
> 
> > As I stated initially you could hardcode the store path to
> > syncthing-gtk in its stead but it's still a store path in the end.
> > It must go stale by design. The only reasonable thing is to not
> > install the file and instead give users the tools to do so on their
> > own.
> > 
> 
> For a medium to long-term solution, what would you advocate for? A
> note in a package's description that they should see the manual or
> cookbook for how to autostart this program if they want (since it
> will show up in the preferences menu)? Of course, we'll need that
> documentation first :) Maybe an example piece of code for using
> Shepherd to start, or guix home (I still need to look at that)?
> Perhaps this should be raised more generally in the devel list to be
> consistent in Guix, since this won't be the only one.
Long-term I'd advocate having an autostart service for putting .desktop
files into the autostart folder, documented in the home-services
section with the concrete example of syncthing-gtk perhaps there or in
the Cookbook.  We shouldn't change the package description, we don't do
that for packages we need at the system level (within or without a
service).

> In the meantime, I'll try to find if getting the profile to bin path
> is not too tricky, if not at least make it so the path it does use
> points to syncthing-gtk and not .syncthing-gtk-real, since that is
> incorrect in every way.
The string you need to replace is "get_executable\\(\\)" in
syncthing_gtk/uisettingsdialog.py

Happy hacking

bug#50789: syncthing-gtk creates autostart file with wrong bin

[John Kehayias via Bug reports for GNU Guix]

Hello again,

I should have sent this here directly, but I just submitted a patch for this issue. I opted to just use a plain exec line of "syncthing-gtk". Please see https://debbugs.gnu.org/cgi/bugreport.cgi?bug=53036

As we had discussed, it is not perfect, but it will work and won't break on a store path change. I think the chance of conflicting/multiple versions of syncthing-gtk is rare and not worth worrying about over this not working at all (syncthing-gtk is just a simple frontend to the web interface of the real syncthing, after all). We can work on a more robust autostart in Guix with guix home for a future system to wrangle all this better.

Thanks,
John

bug#50789: syncthing-gtk creates autostart file with wrong bin

[Leo Famulari]

On Fri, Sep 24, 2021 at 09:33:56PM +0000, John Kehayias via Bug reports for GNU Guix wrote:
> Hello,
> 
> syncthing-gtk has an option to enable autostart, which it does by creating the .desktop file in ~/.config/autostart However, this has the wrong exec line, getting the -real script instead of syncthing-gtk. This won't work as it needs to be run as syncthing-gtk. Namely, it produces:
> 
> Exec=/gnu/store/vf5h9jqhq40x8r46afaa0jgw7awg1361-syncthing-gtk-0.9.4.4-1.c46fbd8/bin/.syncthing-gtk-real
> 
> Instead of
> 
> Exec=/gnu/store/vf5h9jqhq40x8r46afaa0jgw7awg1361-syncthing-gtk-0.9.4.4-1.c46fbd8/bin/syncthing-gtk

Fixed with commit c37559e81979232feee07aa1eb39faacb093c5ca