From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id 2K6bCrYYVmDDeAAA0tVLHw (envelope-from ) for ; Sat, 20 Mar 2021 15:45:58 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id +FJ8BrYYVmAsUgAA1q6Kng (envelope-from ) for ; Sat, 20 Mar 2021 15:45:58 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 78567F026 for ; Sat, 20 Mar 2021 16:45:57 +0100 (CET) Received: from localhost ([::1]:57328 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lNdnX-0005SW-Ch for larch@yhetil.org; Sat, 20 Mar 2021 11:45:55 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48174) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lNdn6-0005SD-8E for emacs-orgmode@gnu.org; Sat, 20 Mar 2021 11:45:28 -0400 Received: from ciao.gmane.io ([116.202.254.214]:59898) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lNdn4-0003R2-Ku for emacs-orgmode@gnu.org; Sat, 20 Mar 2021 11:45:28 -0400 Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1lNdn1-0009yD-DY for emacs-orgmode@gnu.org; Sat, 20 Mar 2021 16:45:23 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: emacs-orgmode@gnu.org From: Maxim Nikulin Subject: Re: bug#44824: [PATCH] org.el: Avoid xdg-open silent failure Date: Sat, 20 Mar 2021 22:45:16 +0700 Message-ID: <214cd672-a9c6-b7d4-754e-df2e552f2b2e@gmail.com> References: <87y2gfcape.fsf_-_@gnus.org> <87a6st7oi1.fsf@gnus.org> <108399a5-66ad-eee6-572b-b3f2181e4e6c@gmail.com> <87lfccxs5a.fsf@gnus.org> <875z3f2bwx.fsf@gnus.org> <838s8aak8j.fsf@gnu.org> <83sg6i8rht.fsf@gnu.org> <5f1a0018-56a4-7f00-68bc-eeb93631f102@gmail.com> <83lfca8k4e.fsf@gnu.org> <83y2g96ta6.fsf@gnu.org> <7635bde2-8590-f555-0d3b-7fa818d812c6@gmail.com> <83o8h56p7o.fsf__8661.17158891342$1612110869$gmane$org@gnu.org> <87zgyzls41.fsf@kyleam.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------8A9F5E7AADCBE8693FAE8F3D" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: <87zgyzls41.fsf@kyleam.com> Content-Language: en-US Received-SPF: pass client-ip=116.202.254.214; envelope-from=geo-emacs-orgmode@m.gmane-mx.org; helo=ciao.gmane.io X-Spam_score_int: 5 X-Spam_score: 0.5 X-Spam_bar: / X-Spam_report: (0.5 / 5.0 requ) BAYES_00=-1.9, DKIM_ADSP_CUSTOM_MED=0.001, FORGED_GMAIL_RCVD=1, FREEMAIL_FORGED_FROMDOMAIN=0.249, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.25, NICE_REPLY_A=-0.001, NML_ADSP_CUSTOM_MED=0.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: 44824@debbugs.gnu.org Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1616255157; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=7IDt7rtMz3R6+AVQq+zFTuxL8nuA+KWhUVAJ+1Dh1Dw=; b=qrqj17njoQTocTeCIp8rcWrR8RkHXtpDuKWHNFlZ9cnYwFaKcDvJ91IMyxE0LkqjcI3sSK GAFeUTw+TcOTCp938h0UjufV5HxXJXmrTUK2WEwlJF5ySOC3U4xnzcB6pbKS/DbVWGRyiQ Ycq4t7daOAa4EHuwsED0y4YnZDAZ4Zl9W1a5dZXkeiAxXCz/YaC4aFsLywkRouTjTyh+JK wZ80BsoXPaJRH0gF55kN4B8fPbQMbJ3Ry4iBSWxHXU9Ta4deHNG0TuFZ0UlpeMU62VVpSq dpzCf9JOLgUoLd7DdwWdVxzQqGb5J9WFqpm9SJTnPDd0EDzSHr31B9OHrHziQg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1616255157; a=rsa-sha256; cv=none; b=WFx3VZ08trIf5lLKa5cph4M+l4yo45UGXloKczOlzvbTOU1XjWGYwJY0r8j05DJG/yGbgc FoqXq3OWmHjh1FFGdxk69nB93hEDXhDPgCB5pDp6EA6KWruSajBV6XFkwgPB0eVwcL3oTV qlnBJCtMHu9pMVSUAOlOrCj0BYv1Y/oDN4Eny/68nUW4vWsp6r5qkbr9hd74T7t7DwQfve YMPzN2cW/TGZ+NkTr2CCX+6Bgv5nd7oZl52EOBKC91Z3YseXyL17P+X4UcMF151DohXDUW i8215Wlihj4veJV9JzbyJmOiw1YOq81+x4eNDgWI5OQ+bnCFkDau/tj3UrLYWw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Spam-Score: -1.81 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of emacs-orgmode-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=emacs-orgmode-bounces@gnu.org X-Migadu-Queue-Id: 78567F026 X-Spam-Score: -1.81 X-Migadu-Scanner: scn0.migadu.com X-TUID: o3dg0l19cHbj This is a multi-part message in MIME format. --------------8A9F5E7AADCBE8693FAE8F3D Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 19/03/2021 10:50, Kyle Meyer wrote: > Maxim Nikulin writes: > A few comments in addition to Eli's advice to drop the > (eq system-type 'gnu/linux) condition... Feel free to commit the change suggested in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=44824#82 instead of this patch. >> +(defun org--error-process-sentinel (proc event) >> + "Show a message if process failed (exited with non-zero code >> +or killed by a signal. Pass the function as :SENTINEL argument > > Please rework the first sentence so that it fits on the first line, > though I'd be in favor dropping the function and using a lambda in the > make-process call. My impression is that org-open-file function is already too long and complex. Another reason to use standalone function is that I am unsure if elisp compiler and interpreter are smart enough to reuse single instance of lambda. I was afraid that every opened file caused creation of new sentinel possibly with a closure containing chain of stack frames. On the other hand even in worst case memory footprint is negligible in comparison to any GUI viewer. >> + (unless (string-match "finished" event) > > There's no need for substring matching, right? So it could be > > (equal event "finished\n") I was surprised by final "\n" that is not always suitable and I was in doubts concerning its stability. I would prefer something like (starts-with event "finished") Certainly match-data is not necessary, so even match-string-p is better. > (when (and (memq (process-status proc) '(exit signal)) > (/= (process-exit-status proc) 0)) Thank you, I was too lazy to implement such kind of check myself. Certainly this variant is better. I hope, I have addressed other your comments in the updated patch. --------------8A9F5E7AADCBE8693FAE8F3D Content-Type: text/x-patch; charset=UTF-8; name="org-open-file-make-process-v2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="org-open-file-make-process-v2.patch" commit 5eca7764d94dd46b9f9a7792d1b786a3f03b20b6 Author: Max Nikulin Date: Wed Feb 17 16:35:58 2021 +0000 org.el: Avoid xdg-open silent failure * lisp/org.el (org-open-file): Use 'pipe :connection-type instead of 'pty to prevent killing of background process on handler exit. (Bug#44824) Problem happens only in some desktop environments where configured through `org-file-apps' or mailcap handlers launches actual viewer (as defined in .desktop files and obtained from mimeapps.list) in background. E.g. xdg-open invokes "gio open" or kde-open5 for Gnome or KDE accordingly and these handlers launches e.g. eog or okular in background. As soon as main process exits, temporary terminal session created by `start-process-shell-command' is terminated. As a result background processes receive SIGHUP. Previously command were executed with no buffer, so the change does not affect "needsterminal" and "copiousoutput" mailcap features, they are not supported as earlier. If handler main process fails then show a message with exit reason. Output (including error messages) is ignored as before. Gtk application tends to report significant amount of failed asserts hardly informative for majority of users. diff --git a/lisp/org.el b/lisp/org.el index 4db2dbe04..c58708a5f 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -8768,7 +8768,21 @@ If the file does not exist, throw an error." (save-window-excursion (message "Running %s...done" cmd) - (start-process-shell-command cmd nil cmd) + ;; Handlers such as "gio open" and kde-open5 start viewer in background + ;; and exit immediately. Avoid `start-process' since it assumes + ;; :connection-type 'pty and kills children processes with SIGHUP + ;; when temporary terminal session is finished. + (make-process + :name "org-open-file" :connection-type 'pipe :noquery t + :buffer nil ; use "*Messages*" for debugging + :sentinel (lambda (proc event) + (when (and (memq (process-status proc) '(exit signal)) + (/= (process-exit-status proc) 0)) + (message + "Command %s: %s." + (mapconcat #'identity (process-command proc) " ") + (substring event 0 -1)))) + :command (list shell-file-name shell-command-switch cmd)) (and (boundp 'org-wait) (numberp org-wait) (sit-for org-wait)))) ((or (stringp cmd) (eq cmd 'emacs)) --------------8A9F5E7AADCBE8693FAE8F3D--