Re: [PATCH] org-macs: Fix incorrect use of relative paths in org-compile-file

[Roshan Shariff <roshan.shariff@gmail.com>]

[-- 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