* [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file @ 2023-08-05 5:18 Roshan Shariff 2023-08-05 10:23 ` Ihor Radchenko 0 siblings, 1 reply; 8+ messages in thread From: Roshan Shariff @ 2023-08-05 5:18 UTC (permalink / raw) To: emacs-orgmode; +Cc: Roshan Shariff * org-macs.el (org-compile-file, org-compile-file-commands): Avoid converting the source path to be relative to the default-directory, which breaks for absolute source paths when the current directory is a symlink. Commit 5a8a1d4ff [1] changed org-compile-file to use `file-relative-name` for the source argument. This was intended to fix bug [2] by expanding ~ directories in the source path, like a shell. Unfortunately, this forced use of relative paths breaks when default-directory is a symlink, and the source to be compiled has an absolute path. For example, on macOS Ventura, ~/Dropbox is a symlink to ~/Library/CloudStorage/Dropbox. Suppose `default-directory` is /Users/username/Dropbox and you try to compile /var/tmp/test.org. Its relative path is ../../../var/tmp/test.org. But the working directory of a compilation process is actually ~/Library/CloudStorage/Dropbox, relative to which the source path resolves to "/Users/username/var/tmp/test.org". The process thus cannot find the source file. This commit changes `org-compile-file` and its helper function `org-compile-file-commands` to avoid the use of relative paths. Instead, to address bug [2], `expand-file-name` is used (only on absolute paths) for ~ expansion. Otherwise, the source path is passed unchanged to the compilation command. The `file-truename` of the source directory is used to construct absolute source and output paths if needed by the command. [1] https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5a8a1d4ff [2] https://orgmode.org/list/25528.42190.53674.62381@gargle.gargle.HOWL --- lisp/org-macs.el | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index e102f01c3..5d8f65193 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -1606,16 +1606,20 @@ filename. Otherwise, it raises an error. When PROCESS is a list of commands, optional argument LOG-BUF can be set to a buffer or a buffer name. `shell-command' then uses it for output." + (when (file-name-absolute-p source) + ;; Expand "~" and "~user" . Shell expansion will be disabled + ;; in the shell command call. + (setq source (expand-file-name source))) (let* ((commands (org-compile-file-commands source process ext spec err-msg)) - (output (expand-file-name (concat (file-name-base source) "." ext) - (file-name-directory source))) + (output (file-name-concat (file-name-directory source) + (concat (file-name-base source) "." ext))) (log-buf (and log-buf (get-buffer-create log-buf))) (time (file-attribute-modification-time (file-attributes output)))) (save-window-excursion (dolist (command commands) (cond ((functionp command) - (funcall command (shell-quote-argument (file-relative-name source)))) + (funcall command (shell-quote-argument source))) ((stringp command) (shell-command command log-buf))))) ;; Check for process failure. Output file is expected to be ;; located in the same directory as SOURCE. @@ -1658,22 +1662,33 @@ argument SPEC, as an alist following the pattern Throw an error if PROCESS does not satisfy the described patterns. The error string will be appended with ERR-MSG, when it is a string." + (when (file-name-absolute-p source) + ;; Expand "~" and "~user" . Shell expansion will be disabled + ;; in the shell command call. + (setq source (expand-file-name source))) (let* ((base-name (file-name-base source)) - (full-name (file-truename source)) - (relative-name (file-relative-name source)) - (out-dir (if (file-name-directory source) - ;; Expand "~". Shell expansion will be disabled - ;; in the shell command call. - (file-name-directory full-name) - "./")) - (output (expand-file-name (concat (file-name-base source) "." ext) out-dir)) + (out-dir (file-name-directory source)) + ;; Don't use (expand-file-name SOURCE) for the absolute path, + ;; in case SOURCE starts with ../ and default-directory is a + ;; symlink. Instead, resolve symlinks in the directory + ;; component of SOURCE... + (true-out-dir (file-truename out-dir)) + ;; but use the original file name component of SOURCE in case + ;; it is a symlink; we want %f and %F to have the same file + ;; name component: + (full-name (file-name-concat true-out-dir + (file-name-nondirectory source))) + ;; The absolute path OUTPUT is the same as FULL-NAME, except + ;; with extension EXT: + (output (file-name-concat true-out-dir + (concat base-name "." ext))) (err-msg (if (stringp err-msg) (concat ". " err-msg) ""))) (pcase process ((pred functionp) (list process)) ((pred consp) (let ((spec (append spec `((?b . ,(shell-quote-argument base-name)) - (?f . ,(shell-quote-argument relative-name)) + (?f . ,(shell-quote-argument source)) (?F . ,(shell-quote-argument full-name)) (?o . ,(shell-quote-argument out-dir)) (?O . ,(shell-quote-argument output)))))) base-commit: 73cb528c24322fef2d05142d36bc48bb9dac962e -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-05 5:18 [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file Roshan Shariff @ 2023-08-05 10:23 ` Ihor Radchenko 2023-08-05 16:15 ` Roshan Shariff 0 siblings, 1 reply; 8+ messages in thread From: Ihor Radchenko @ 2023-08-05 10:23 UTC (permalink / raw) To: Roshan Shariff; +Cc: emacs-orgmode Roshan Shariff <roshan.shariff@gmail.com> writes: > * org-macs.el (org-compile-file, org-compile-file-commands): Avoid > converting the source path to be relative to the default-directory, > which breaks for absolute source paths when the current directory is a > symlink. > > Commit 5a8a1d4ff [1] changed org-compile-file to use > `file-relative-name` for the source argument. This was intended to fix > bug [2] by expanding ~ directories in the source path, like a shell. Thanks for the report and for providing a detailed explanation with a patch! I have a few comments. First, minor one: please put two spaces between sentences in the commit message. It is our convention. > - (output (expand-file-name (concat (file-name-base source) "." ext) > - (file-name-directory source))) > + (output (file-name-concat (file-name-directory source) > + (concat (file-name-base source) "." ext))) `file-name-concat' is only available since Emacs 28. Please use `org-file-name-concat'. > - (?f . ,(shell-quote-argument relative-name)) > + (?f . ,(shell-quote-argument source)) This will break user expectations. The PROCESS argument can, for example, be `org-latex-pdf-process', which promises that "%f in the command will be replaced by the relative file name" (see the docstring). -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-05 10:23 ` Ihor Radchenko @ 2023-08-05 16:15 ` Roshan Shariff 2023-08-06 6:49 ` Ihor Radchenko 0 siblings, 1 reply; 8+ messages in thread From: Roshan Shariff @ 2023-08-05 16:15 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1390 bytes --] Hi Ihor, On Sat, 5 Aug 2023 at 04:23, Ihor Radchenko <yantar92@posteo.net> wrote: > First, minor one: please put two spaces between sentences in the commit > message. It is our convention. > `file-name-concat' is only available since Emacs 28. > Please use `org-file-name-concat'. Thanks for the feedback! I'm attaching a new version of the patch that improves the commit message and avoids the use of file-name-concat. > The PROCESS argument can, for example, be `org-latex-pdf-process', which > promises that "%f in the command will be replaced by the relative file > name" (see the docstring). The patch now converts the source to a relative path if it is absolute. Note that, according to the documentation of file-relative-name, the path will still be absolute on Microsoft Windows if the source and the current directory have different drive letters; this is probably unavoidable. I have also updated the docstring to clarify which paths are supposed to be relative and which are absolute. By the way, I forgot to mention that I ran into this bug while testing tecosaur's latex-preview patch set on macOS. When running org-latex-export-to-pdf on an Org file in ~/Dropbox, I got a warning message about the preamble precompilation failing because it couldn't find a temporary .tex file in /var. With this patch applied, the PDF export completes without warnings. Regards, Roshan [-- Attachment #2: v2-0001-org-macs-Fix-incorrect-use-of-relative-paths-in-o.patch --] [-- Type: text/x-patch, Size: 5794 bytes --] From 6e1f120818582695da9018f2111156b00c87104a Mon Sep 17 00:00:00 2001 From: Roshan Shariff <roshan.shariff@gmail.com> Date: Fri, 4 Aug 2023 22:10:25 -0600 Subject: [PATCH v2] org-macs: Fix incorrect use of relative paths in org-compile-file * org-macs.el (org-compile-file, org-compile-file-commands): Resolve symlinks in default-directory before computing relative source path Commit 5a8a1d4ff [1] changed org-compile-file to use `file-relative-name` for the SOURCE argument. This was intended to fix bug [2] by expanding ~ directories, like a shell. Unfortunately, this breaks when DEFAULT-DIRECTORY is a symlink and SOURCE has an absolute path. For example, on macOS Ventura, ~/Dropbox is a symlink to ~/Library/CloudStorage/Dropbox. Suppose DEFAULT-DIRECTORY is /Users/username/Dropbox and SOURCE is /var/tmp/test.org, so its relative path is ../../../var/tmp/test.org. But the working directory of a compilation process is actually ~/Library/CloudStorage/Dropbox, relative to which the source path resolves to /Users/username/var/tmp/test.org. The process thus cannot find the source file. This commit changes `org-compile-file` and its helper function `org-compile-file-commands` to resolve symlinks in DEFAULT-DIRECTORY before computing the relative path of SOURCE. If SOURCE is already relative, it is used as-is. The absolute path is processed by `expand-file-name`, avoiding bug [1]. [1] https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5a8a1d4ff [2] https://orgmode.org/list/25528.42190.53674.62381@gargle.gargle.HOWL --- lisp/org-macs.el | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/lisp/org-macs.el b/lisp/org-macs.el index e102f01c3..dc5dbeab5 100644 --- a/lisp/org-macs.el +++ b/lisp/org-macs.el @@ -1607,15 +1607,18 @@ When PROCESS is a list of commands, optional argument LOG-BUF can be set to a buffer or a buffer name. `shell-command' then uses it for output." (let* ((commands (org-compile-file-commands source process ext spec err-msg)) - (output (expand-file-name (concat (file-name-base source) "." ext) - (file-name-directory source))) + (output (concat (file-name-sans-extension source) "." ext)) + (relname (if (file-name-absolute-p source) + (let ((pwd (file-truename default-directory))) + (file-relative-name source pwd)) + source)) (log-buf (and log-buf (get-buffer-create log-buf))) (time (file-attribute-modification-time (file-attributes output)))) (save-window-excursion (dolist (command commands) (cond ((functionp command) - (funcall command (shell-quote-argument (file-relative-name source)))) + (funcall command (shell-quote-argument relname))) ((stringp command) (shell-command command log-buf))))) ;; Check for process failure. Output file is expected to be ;; located in the same directory as SOURCE. @@ -1649,33 +1652,35 @@ the SOURCE file. If PROCESS is a list of commands, each of them is called using `shell-command'. By default, in each command, %b, %f, %F, %o and -%O are replaced with, respectively, SOURCE base name, name, full -name, directory and absolute output file name. It is possible, -however, to use more place-holders by specifying them in optional -argument SPEC, as an alist following the pattern +%O are replaced with, respectively, SOURCE base name, relative +file name, absolute file name, relative directory and absolute +output file name. It is possible, however, to use more +place-holders by specifying them in optional argument SPEC, as an +alist following the pattern (CHARACTER . REPLACEMENT-STRING). Throw an error if PROCESS does not satisfy the described patterns. The error string will be appended with ERR-MSG, when it is a string." - (let* ((base-name (file-name-base source)) - (full-name (file-truename source)) - (relative-name (file-relative-name source)) - (out-dir (if (file-name-directory source) - ;; Expand "~". Shell expansion will be disabled - ;; in the shell command call. - (file-name-directory full-name) - "./")) - (output (expand-file-name (concat (file-name-base source) "." ext) out-dir)) + (let* ((basename (file-name-base source)) + ;; Resolve symlinks in default-directory to correctly handle + ;; absolute source paths or relative paths with .. + (pwd (file-truename default-directory)) + (absname (expand-file-name source pwd)) + (relname (if (file-name-absolute-p source) + (file-relative-name source pwd) + source)) + (relpath (or (file-name-directory relname) "./")) + (output (concat (file-name-sans-extension absname) "." ext)) (err-msg (if (stringp err-msg) (concat ". " err-msg) ""))) (pcase process ((pred functionp) (list process)) ((pred consp) (let ((spec (append spec - `((?b . ,(shell-quote-argument base-name)) - (?f . ,(shell-quote-argument relative-name)) - (?F . ,(shell-quote-argument full-name)) - (?o . ,(shell-quote-argument out-dir)) + `((?b . ,(shell-quote-argument basename)) + (?f . ,(shell-quote-argument relname)) + (?F . ,(shell-quote-argument absname)) + (?o . ,(shell-quote-argument relpath)) (?O . ,(shell-quote-argument output)))))) (mapcar (lambda (command) (format-spec command spec)) process))) (_ (error "No valid command to process %S%s" source err-msg))))) base-commit: c7e1f78326581e8d994feaee69d725d3e073f89f -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-05 16:15 ` Roshan Shariff @ 2023-08-06 6:49 ` Ihor Radchenko 2023-08-06 15:07 ` Roshan Shariff 0 siblings, 1 reply; 8+ messages in thread From: Ihor Radchenko @ 2023-08-06 6:49 UTC (permalink / raw) To: Roshan Shariff; +Cc: emacs-orgmode Roshan Shariff <roshan.shariff@gmail.com> writes: > Thanks for the feedback! I'm attaching a new version of the patch that > improves the commit message and avoids the use of file-name-concat. Thanks! Applied, onto main, with minor amendments. https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4ea9a98f8 I added a TINYCHANGE cookie as you do not appear to have FSF copyright assignment. I also duplicated your comment about "pwd" usage to `org-compile-file' to avoid accidental edits in future. Please note that your total contribution have now exceeded the allowed limit we can accept without copyright paper work. You may consider FSF copyright assignment in future. See https://orgmode.org/worg/org-contribute.html#copyright -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-06 6:49 ` Ihor Radchenko @ 2023-08-06 15:07 ` Roshan Shariff 2023-08-06 15:10 ` Ihor Radchenko 0 siblings, 1 reply; 8+ messages in thread From: Roshan Shariff @ 2023-08-06 15:07 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Thanks again Ihor! For the future, I wanted to clarify that I do have a copyright assignment to the FSF "for the suite of programs known as GNU EMACS". I have a signed copy of the assignment form dated 3 April, 2022. I should be in the list as "Roshan Shariff", but if not I can follow up with the FSF to see if they missed adding me. On Sun, 6 Aug 2023 at 00:49, Ihor Radchenko <yantar92@posteo.net> wrote: > > Roshan Shariff <roshan.shariff@gmail.com> writes: > > > Thanks for the feedback! I'm attaching a new version of the patch that > > improves the commit message and avoids the use of file-name-concat. > > Thanks! > Applied, onto main, with minor amendments. > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=4ea9a98f8 > > I added a TINYCHANGE cookie as you do not appear to have FSF copyright > assignment. I also duplicated your comment about "pwd" usage to > `org-compile-file' to avoid accidental edits in future. > > Please note that your total contribution have now exceeded the allowed > limit we can accept without copyright paper work. > You may consider FSF copyright assignment in future. See > https://orgmode.org/worg/org-contribute.html#copyright > > -- > Ihor Radchenko // yantar92, > Org mode contributor, > Learn more about Org mode at <https://orgmode.org/>. > Support Org development at <https://liberapay.com/org-mode>, > or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-06 15:07 ` Roshan Shariff @ 2023-08-06 15:10 ` Ihor Radchenko 2023-08-06 15:26 ` Bastien Guerry 0 siblings, 1 reply; 8+ messages in thread From: Ihor Radchenko @ 2023-08-06 15:10 UTC (permalink / raw) To: Roshan Shariff, Bastien; +Cc: emacs-orgmode Roshan Shariff <roshan.shariff@gmail.com> writes: > For the future, I wanted to clarify that I do have a copyright > assignment to the FSF "for the suite of programs known as GNU EMACS". > I have a signed copy of the assignment form dated 3 April, 2022. I > should be in the list as "Roshan Shariff", but if not I can follow up > with the FSF to see if they missed adding me. Thanks for the update. I do not have access to the FSF records, and you are not yet listed on https://orgmode.org/worg/contributors.html Bastien, may you please confirm Roshan's copyright status? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-06 15:10 ` Ihor Radchenko @ 2023-08-06 15:26 ` Bastien Guerry 2023-08-08 8:54 ` Ihor Radchenko 0 siblings, 1 reply; 8+ messages in thread From: Bastien Guerry @ 2023-08-06 15:26 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Roshan Shariff, emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Bastien, may you please confirm Roshan's copyright status? Yes, I do confirm Roshan is a FSF registered contributor for Emacs. -- Bastien Guerry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file 2023-08-06 15:26 ` Bastien Guerry @ 2023-08-08 8:54 ` Ihor Radchenko 0 siblings, 0 replies; 8+ messages in thread From: Ihor Radchenko @ 2023-08-08 8:54 UTC (permalink / raw) To: Bastien Guerry; +Cc: Roshan Shariff, emacs-orgmode Bastien Guerry <bzg@gnu.org> writes: > Ihor Radchenko <yantar92@posteo.net> writes: > >> Bastien, may you please confirm Roshan's copyright status? > > Yes, I do confirm Roshan is a FSF registered contributor for Emacs. Updated our records. https://git.sr.ht/~bzg/worg/commit/9015a8ea -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-08-08 8:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-05 5:18 [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file Roshan Shariff 2023-08-05 10:23 ` Ihor Radchenko 2023-08-05 16:15 ` Roshan Shariff 2023-08-06 6:49 ` Ihor Radchenko 2023-08-06 15:07 ` Roshan Shariff 2023-08-06 15:10 ` Ihor Radchenko 2023-08-06 15:26 ` Bastien Guerry 2023-08-08 8:54 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).