GNOME Bugzilla – Bug 699326
Replace uses of fork and exec* with g_spawn*
Last modified: 2020-11-06 20:22:56 UTC
Since fork and exec* functions are not portable, we should just use g_spawn* functions provided by GLib.
Created attachment 242932 [details] [review] Use g_spawn* in anjuta-utils.c
Created attachment 242933 [details] [review] Use g_spawn* in resources.c
Created attachment 242934 [details] [review] Rewrite AnjutaLauncher to use g_spawn_async_with_pipes
Created attachment 242935 [details] [review] Remove forkpty emulation
Created attachment 242936 [details] [review] Watch output channel for password prompts Unfortunately, the prompt for getting the password now shows up in the output. It will show up in the messages pane when running "make install", for example.
Created attachment 242937 [details] [review] Add -S option to sudo This one seems not complete yet because the preferences for build-basic-autotools still shows "sudo" instead of "sudo -S".
Created attachment 242938 [details] [review] Port AnjutaLauncher to Windows
Created attachment 242939 [details] [review] Handle SIGKILL and SIGTERM signals as something like SIGTERM in Windows, ignore others
Created attachment 242940 [details] [review] Run scripts in Windows Not yet tested enough, doesn't work properly when a wrapper for autoconf and automake are present (which fails to look for the correct version of autoconf and automake, probably needs some tweaking on sh options).
Patches for Windows are not tested enough, since there are still some bugs preventing testing.
Comment on attachment 242932 [details] [review] Use g_spawn* in anjuta-utils.c Thank for your patch, it's better like this.
Comment on attachment 242933 [details] [review] Use g_spawn* in resources.c Thanks, I have committed your patch without any change.
Review of attachment 242937 [details] [review]: I don't think it's an issue if you have only sudo instead of sudo -S in the GUI. In this case, it will be better to replace "su -c" by su. Anyway, if you want to change this it is defined in the glade file of the autotools plugins: plugins/build-basic-autotools/anjuta-build-basic-autotools-plugin.ui. I have an issue with this patch though, it doesn't work here if I'm using su in the install command because su expects a terminal device. So perhaps it's better to keep a terminal at least on Linux. I don't know what is the best solution: Implement a terminal on Windows if possible? Add an option to AnjutaLauncher to use a terminal or not? Always use a terminal but only on Linux? Find a work around for su? ... What do you think? The 3 previous patches are good too but they need this one to avoid any regression, so I prefer to fix this issue first.
Created attachment 244057 [details] [review] Add -S option to sudo Changed the GtkBuilder UI file too.
(In reply to comment #13) > Review of attachment 242937 [details] [review]: > > I don't think it's an issue if you have only sudo instead of sudo -S in the > GUI. In this case, it will be better to replace "su -c" by su. Anyway, if you > want to change this it is defined in the glade file of the autotools plugins: > plugins/build-basic-autotools/anjuta-build-basic-autotools-plugin.ui. I changed the glade file so that users could easily set to use "sudo -S", otherwise they would have to use gsettings in a terminal. > I have an issue with this patch though, it doesn't work here if I'm using su in > the install command because su expects a terminal device. So perhaps it's > better to keep a terminal at least on Linux. I don't know what is the best > solution: Implement a terminal on Windows if possible? Add an option to > AnjutaLauncher to use a terminal or not? Always use a terminal but only on > Linux? Find a work around for su? ... What do you think? Did you try it in Linux Mint, using "su -c"? I tested it in Ubuntu and openSUSE and they seem to work fine. All patches except the three with Windows in its commit message is necessary for it to work. I don't think implementing a terminal in Windows is a good idea since the command prompt (i.e. Windows's terminal) will show up all the time even if we are just compiling/using git/etc. So either of the last three is the solution, I'll think more about it. Though for the last one I'm not sure if there's a good alternative to su or a work around (pkexec? gnomesu? gksu? gksu? pkexec seems the most portable, though I'm not sure if it is really pkexec's job to do something like this... but it has the best dialog :D; gksudo and gksu is in Ubuntu while gnomesu is used in openSUSE; though this alternatives provides their own password dialog). Also, I don't think we need to worry about permissions in Windows, and so a terminal for AnjutaLauncher in Windows. Windows provides some API for permissions (which I think we should avoid), and it is unusual for programs to be installed to the system directly from sources. Usually an installer would do that.
(In reply to comment #15) > Did you try it in Linux Mint, using "su -c"? I tested it in Ubuntu and > openSUSE and they seem to work fine. Yes, I have use "su -c". I have Linux Mint 13, perhaps su has been changed in newer version. > I don't think implementing a terminal in Windows is a good idea Ok, I agree that it is not needed on Windows. > Though for the last one I'm not sure if there's a good alternative to su or a > work around > Also, I don't think we need to worry about permissions in Windows. I think the best solution would be to add an option in anjuta launcher to create or not a terminal. It could be unneeded with a newer version of su but it could be necessary for another command.
Hi! > Though for the last one I'm not sure if there's a good alternative to su or a > work around (pkexec? gnomesu? gksu? gksu? pkexec seems the most portable, > though I'm not sure if it is really pkexec's job to do something like this... > but it has the best dialog :D; gksudo and gksu is in Ubuntu while gnomesu is > used in openSUSE; though this alternatives provides their own password dialog). PolicyKit (could be used directly without pkexec probably...) would be the correct and portable solution. But not sure how easy or difficult it is to implement this.
(In reply to comment #17) > Hi! > > > Though for the last one I'm not sure if there's a good alternative to su or a > > work around (pkexec? gnomesu? gksu? gksu? pkexec seems the most portable, > > though I'm not sure if it is really pkexec's job to do something like this... > > but it has the best dialog :D; gksudo and gksu is in Ubuntu while gnomesu is > > used in openSUSE; though this alternatives provides their own password dialog). > > PolicyKit (could be used directly without pkexec probably...) would be the > correct and portable solution. But not sure how easy or difficult it is to > implement this. Should I create something like anjuta-launcher then? It will then check the permission from PolicyKit and ask for the password. I can't remember (I tried doing something like this before), but I think this needs a DBus service. I'm planning to work on this this weekend.
(In reply to comment #18) > Should I create something like anjuta-launcher then? It will then check the > permission from PolicyKit and ask for the password. I can't remember (I tried > doing something like this before), but I think this needs a DBus service. I'm > planning to work on this this weekend. As you want, I agree that's it's probably the best solution but it's more complex. On my side just an option to use a terminal working on Linux only is fine.
(In reply to comment #19) > As you want, I agree that's it's probably the best solution but it's more > complex. On my side just an option to use a terminal working on Linux only is > fine. I tried looking for a work-around, but I can't find a way to create a terminal using g_spawn* functions. Do you mean we should have two ways to run? Via g_spawn* and fork/exec? By the way, I'm planning to implement the launcher such that it will be used only if sudo permissions are asked (might add some functions to AnjutaLauncher to know if it does need elevated permissions).
(In reply to comment #20) > I tried looking for a work-around, but I can't find a way to create a terminal > using g_spawn* functions. Do you mean we should have two ways to run? Via > g_spawn* and fork/exec? I haven't looked at this but I agree that it's quite annoying to use both g_spawn* and fork/exec.
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years. If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/anjuta/-/issues/ Thank you for reporting this issue and we are sorry it could not be fixed.