* [BUG] Tangle with symbolic links don't work
@ 2023-11-06 16:08 Cletip Cletip
2023-11-06 16:17 ` Ihor Radchenko
0 siblings, 1 reply; 7+ messages in thread
From: Cletip Cletip @ 2023-11-06 16:08 UTC (permalink / raw)
To: Org Mode List
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
Hello everyone,
I'm reaching out to discuss a challenge I've encountered while working with
Org-mode, specifically during the tangling process of code blocks.
The Issue:
I have an Org file, test.org, from which I tangle code blocks into test.py.
The complication arises because test.py is a symbolic link. Each time I
perform the tangling operation, the symbolic link is unfortunately
overwritten.
Interestingly, when exporting documents to formats such as .tex, .html, or
others, Org-mode respects the symbolic link, which is the desired behavior.
However, this is not the case with tangling, which is quite perplexing.
My Org mode version : 9.6.10 (9.6.10-n/a-g902975...)
Proposed Solution:
I've identified a potential fix that involves a minor adjustment in the
ob-tangle.el file:
Original line (259):
(let ((file-name (car by-fn)))
Modified line:
(let ((file-name (file-truename (car by-fn))))
While this solution appears effective at first glance, I haven't
extensively tested all edge cases. I would greatly appreciate your insights
on whether this is a robust solution ^^.
Thanks in advance for your answer.
[-- Attachment #2: Type: text/html, Size: 1271 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] Tangle with symbolic links don't work
2023-11-06 16:08 [BUG] Tangle with symbolic links don't work Cletip Cletip
@ 2023-11-06 16:17 ` Ihor Radchenko
2023-11-07 10:55 ` Max Nikulin
0 siblings, 1 reply; 7+ messages in thread
From: Ihor Radchenko @ 2023-11-06 16:17 UTC (permalink / raw)
To: Cletip Cletip, Max Nikulin; +Cc: Org Mode List
Cletip Cletip <clement020302@gmail.com> writes:
> The Issue:
> I have an Org file, test.org, from which I tangle code blocks into test.py.
> The complication arises because test.py is a symbolic link. Each time I
> perform the tangling operation, the symbolic link is unfortunately
> overwritten.
> ...
> Proposed Solution:
> I've identified a potential fix that involves a minor adjustment in the
> ob-tangle.el file:
>
> Original line (259):
>
> (let ((file-name (car by-fn)))
>
> Modified line:
>
> (let ((file-name (file-truename (car by-fn))))
>
> While this solution appears effective at first glance, I haven't
> extensively tested all edge cases. I would greatly appreciate your insights
> on whether this is a robust solution ^^.
Looks reasonable.
Max, do you see any pitfalls using `file-truename'?
--
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] 7+ messages in thread
* Re: [BUG] Tangle with symbolic links don't work
2023-11-06 16:17 ` Ihor Radchenko
@ 2023-11-07 10:55 ` Max Nikulin
2023-11-07 11:30 ` Ihor Radchenko
0 siblings, 1 reply; 7+ messages in thread
From: Max Nikulin @ 2023-11-07 10:55 UTC (permalink / raw)
To: Cletip Cletip; +Cc: Org Mode List
On 06/11/2023 23:17, Ihor Radchenko wrote:
> Cletip Cletip writes:
>
>> I have an Org file, test.org, from which I tangle code blocks into test.py.
>> The complication arises because test.py is a symbolic link.
I suggest to change your workflow to not depend on particular means to
write tangled files. Perhaps you may set tangle directory. Unfortunately
the description is too concise to figure out if it would be convenient
enough.
> Max, do you see any pitfalls using `file-truename'?
Sorry, I am not familiar with related code path. That is why I can not
reason what way to deal with file name is safer.
If there is a world-writable directory in the file path (usually
$TMPDIR) then `file-truename' is less safe, see
https://www.kernel.org/doc/html/latest/admin-guide/sysctl/fs.html#protected-symlinks
echo content >/home/ubuntu/tmp/file.txt
ls -l /home/ubuntu/tmp/file.txt
-rw-r--r-- 1 ubuntu ubuntu 8 Nov 7 17:29 /home/ubuntu/tmp/file.txt
ls -l /tmp/ln
lrwxrwxrwx 1 test test 25 Nov 7 17:10 /tmp/ln -> /home/ubuntu/tmp/file.txt
Notice different owner of the symlink.
echo overwrite >/tmp/ln
bash: /tmp/ln: Permission denied
cat /home/ubuntu/tmp/file.txt
content
`file-truename':
echo overwrite>"$(readlink -f /tmp/ln)"
cat /home/ubuntu/tmp/file.txt
overwrite
/usr/sbin/sysctl fs.protected_symlinks
fs.protected_symlinks = 1
In general, I am never sure that Org code follows best practices in
respect to security in general and in respect to /tmp in particular. The
following citation is unrelated to /tmp, but the same proposed patch has
an issue with predictable name in /tmp:
Visuwesh… Re: [BUG] [PATCH] Add yank-media and DND handler. Wed, 27 Sep
2023 13:59:49 +0530. https://list.orgmode.org/878r8sj9xe.fsf@gmail.com
> That would be quite annoying IMO. I say we let the user shoot
> themselves in the foot.
Even when /tmp or similar directories are not involved, a proper
strategy to replace file content should be carefully chosen. E.g. cp(1)
preserves inode number while install(1) replaces target file atomically
(create a temporary one and rename). The latter way is more suitable for
shared libraries since it allows running application to continue call
function from the deleted file.
I know, it is not an answer you expected from me, but giving a better
one require to much efforts to read the code and to debug it.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] Tangle with symbolic links don't work
2023-11-07 10:55 ` Max Nikulin
@ 2023-11-07 11:30 ` Ihor Radchenko
2023-11-20 9:55 ` Cletip Cletip
0 siblings, 1 reply; 7+ messages in thread
From: Ihor Radchenko @ 2023-11-07 11:30 UTC (permalink / raw)
To: Max Nikulin; +Cc: Cletip Cletip, Org Mode List
Max Nikulin <manikulin@gmail.com> writes:
>> Max, do you see any pitfalls using `file-truename'?
>
> Sorry, I am not familiar with related code path. That is why I can not
> reason what way to deal with file name is safer.
>
> If there is a world-writable directory in the file path (usually
> $TMPDIR) then `file-truename' is less safe, see
> https://www.kernel.org/doc/html/latest/admin-guide/sysctl/fs.html#protected-symlinks
Thanks!
> In general, I am never sure that Org code follows best practices in
> respect to security in general and in respect to /tmp in particular. The
> following citation is unrelated to /tmp, but the same proposed patch has
> an issue with predictable name in /tmp:
We have to compromise between usability and safety... but probably not
in this case.
> Even when /tmp or similar directories are not involved, a proper
> strategy to replace file content should be carefully chosen. E.g. cp(1)
> preserves inode number while install(1) replaces target file atomically
> (create a temporary one and rename). The latter way is more suitable for
> shared libraries since it allows running application to continue call
> function from the deleted file.
What we actually use is Elisp API. For export and tangling, we use
`write-region' - it correctly handles TRAMP files with lower-level
details taken care of.
I can now see that blindly expanding to `file-truename' may not be wise.
Without `file-truename', the difference between ox.el (that works for
Cletip) and ob-tangle.el is that ob-tangle explicitly deletes the tangle
target before tangling:
`org-babel-tangle':
;; erase previous file
(when (file-exists-p file-name)
(delete-file file-name))
(write-region nil nil file-name)
(mapc (lambda (mode) (set-file-modes file-name mode)) modes)
Rather than using `file-truename', we may instead remove the
`delete-file' part. This way, we will not risk changing file modes in
the original files and always modify the symlink, if the tangle target
is an existing symlink.
> I know, it is not an answer you expected from me, but giving a better
> one require to much efforts to read the code and to debug it.
It is exactly an answer I expected, actually :)
--
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] 7+ messages in thread
* Re: [BUG] Tangle with symbolic links don't work
2023-11-07 11:30 ` Ihor Radchenko
@ 2023-11-20 9:55 ` Cletip Cletip
2023-12-04 12:58 ` Ihor Radchenko
0 siblings, 1 reply; 7+ messages in thread
From: Cletip Cletip @ 2023-11-20 9:55 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: Max Nikulin, Org Mode List
[-- Attachment #1: Type: text/plain, Size: 3012 bytes --]
Hello !
Sorry to bring up the subject again, but I didn't quite understand what the
solution was: should I modify the function ? Is it modified in a new
version of org-mode ?
Thank you in advance for your response.
Le mar. 7 nov. 2023 à 12:28, Ihor Radchenko <yantar92@posteo.net> a écrit :
> Max Nikulin <manikulin@gmail.com> writes:
>
> >> Max, do you see any pitfalls using `file-truename'?
> >
> > Sorry, I am not familiar with related code path. That is why I can not
> > reason what way to deal with file name is safer.
> >
> > If there is a world-writable directory in the file path (usually
> > $TMPDIR) then `file-truename' is less safe, see
> >
> https://www.kernel.org/doc/html/latest/admin-guide/sysctl/fs.html#protected-symlinks
>
> Thanks!
>
> > In general, I am never sure that Org code follows best practices in
> > respect to security in general and in respect to /tmp in particular. The
> > following citation is unrelated to /tmp, but the same proposed patch has
> > an issue with predictable name in /tmp:
>
> We have to compromise between usability and safety... but probably not
> in this case.
>
> > Even when /tmp or similar directories are not involved, a proper
> > strategy to replace file content should be carefully chosen. E.g. cp(1)
> > preserves inode number while install(1) replaces target file atomically
> > (create a temporary one and rename). The latter way is more suitable for
> > shared libraries since it allows running application to continue call
> > function from the deleted file.
>
> What we actually use is Elisp API. For export and tangling, we use
> `write-region' - it correctly handles TRAMP files with lower-level
> details taken care of.
>
> I can now see that blindly expanding to `file-truename' may not be wise.
>
> Without `file-truename', the difference between ox.el (that works for
> Cletip) and ob-tangle.el is that ob-tangle explicitly deletes the tangle
> target before tangling:
>
> `org-babel-tangle':
>
> ;; erase previous file
> (when (file-exists-p file-name)
> (delete-file file-name))
> (write-region nil nil file-name)
> (mapc (lambda (mode) (set-file-modes file-name mode))
> modes)
>
> Rather than using `file-truename', we may instead remove the
> `delete-file' part. This way, we will not risk changing file modes in
> the original files and always modify the symlink, if the tangle target
> is an existing symlink.
>
> > I know, it is not an answer you expected from me, but giving a better
> > one require to much efforts to read the code and to debug it.
>
> It is exactly an answer I expected, actually :)
>
> --
> 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>
>
[-- Attachment #2: Type: text/html, Size: 4066 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [BUG] Tangle with symbolic links don't work
2023-11-20 9:55 ` Cletip Cletip
@ 2023-12-04 12:58 ` Ihor Radchenko
2023-12-15 11:26 ` Ihor Radchenko
0 siblings, 1 reply; 7+ messages in thread
From: Ihor Radchenko @ 2023-12-04 12:58 UTC (permalink / raw)
To: Cletip Cletip; +Cc: Max Nikulin, Org Mode List
[-- Attachment #1: Type: text/plain, Size: 354 bytes --]
Cletip Cletip <clement020302@gmail.com> writes:
> Sorry to bring up the subject again, but I didn't quite understand what the
> solution was: should I modify the function ? Is it modified in a new
> version of org-mode ?
The tentative solution is attached. May you test it?
The previous discussion was mostly about how to best approach the solution.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-babel-tangle-Do-note-erase-the-existing-tangle-t.patch --]
[-- Type: text/x-patch, Size: 1650 bytes --]
From cf9db7b5a602c2c0d4970c69a95d98640cefc9a0 Mon Sep 17 00:00:00 2001
Message-ID: <cf9db7b5a602c2c0d4970c69a95d98640cefc9a0.1701694627.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Mon, 4 Dec 2023 13:55:05 +0100
Subject: [PATCH] org-babel-tangle: Do note erase the existing tangle target
before overwriting
* lisp/ob-tangle.el (org-babel-tangle): Do not remove the existing
tangle target file, if any. `write-region' later will overwrite it
anyway, while removing may be unexpected if the existing target is a
symlink.
Reported-by: Cletip Cletip <clement020302@gmail.com>
Link: https://list.orgmode.org/orgmode/CAPHku6O9NfVMAfmE3_ahmpJea_2Qm0mJMFX6qPpT8uiQ94KMZA@mail.gmail.com/
---
lisp/ob-tangle.el | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el
index b30fd9274..b48269897 100644
--- a/lisp/ob-tangle.el
+++ b/lisp/ob-tangle.el
@@ -310,9 +310,8 @@ (defun org-babel-tangle (&optional arg target-file lang-re)
(compare-buffer-substrings
nil nil nil
tangle-buf nil nil)))))))
- ;; erase previous file
- (when (file-exists-p file-name)
- (delete-file file-name))
+ ;; We do not erase, but overwrite previous file
+ ;; to preserve any existing symlinks.
(write-region nil nil file-name)
(mapc (lambda (mode) (set-file-modes file-name mode)) modes))
(push file-name path-collector))))))
--
2.42.0
[-- Attachment #3: Type: text/plain, Size: 224 bytes --]
--
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 related [flat|nested] 7+ messages in thread
* Re: [BUG] Tangle with symbolic links don't work
2023-12-04 12:58 ` Ihor Radchenko
@ 2023-12-15 11:26 ` Ihor Radchenko
0 siblings, 0 replies; 7+ messages in thread
From: Ihor Radchenko @ 2023-12-15 11:26 UTC (permalink / raw)
To: Cletip Cletip; +Cc: Max Nikulin, Org Mode List
Ihor Radchenko <yantar92@posteo.net> writes:
> Subject: [PATCH] org-babel-tangle: Do note erase the existing tangle target
> before overwriting
Applied.
Fixed, on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fbcd71e85
--
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] 7+ messages in thread
end of thread, other threads:[~2023-12-15 11:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-06 16:08 [BUG] Tangle with symbolic links don't work Cletip Cletip
2023-11-06 16:17 ` Ihor Radchenko
2023-11-07 10:55 ` Max Nikulin
2023-11-07 11:30 ` Ihor Radchenko
2023-11-20 9:55 ` Cletip Cletip
2023-12-04 12:58 ` Ihor Radchenko
2023-12-15 11:26 ` 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).