From: Evgenii Klimov <eugene.dev@lipklim.org>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: emacs-orgmode@gnu.org
Subject: [PATCH v3] ob-tangle.el: Blocks overwrite each other when grouping before tangling
Date: Mon, 24 Jul 2023 13:28:11 +0100 [thread overview]
Message-ID: <87bkg1h4q3.fsf@lipklim.org> (raw)
In-Reply-To: <87ttu6j2zr.fsf@localhost>
[-- Attachment #1: Type: text/plain, Size: 3079 bytes --]
Hi
Here are the new tests that demonstrate the bug in block grouping during
block collection, along with the patch to address the issue, taking your
previous remarks into account.
I split it into two patches so you can apply the tests first to see the
bug. And probably tests should be rewritten as they look too complex
and mostly duplicate each other. I'd appreciate your suggestions
on how to enhance them.
Ihor Radchenko <yantar92@posteo.net> writes:
> Evgenii Klimov <eugene.dev@lipklim.org> writes:
>
>> In this version I just updated the docstrings for the relevant
>> functions, because prior to that it wasn't clear: does this "default
>> export file for *all* source blocks" influence blocks with :tangle
>> "yes"/FILENAME?
>
> Thanks for the patch, but we need to be careful changing things in
> ob-tangle. Not everything is well-documented there.
>
>> Optional argument TARGET-FILE can be used to specify a default
>> -export file for all source blocks.
>> +export file for all source blocks without :tangle header
>> +argument.
>
> This is confusing.
> Is :tangle yes "without"?
> What about inheritance?
> What about default header args?
I just find current lack of details confusing as well and want to
express the place of TARGET-FILE in the lineage of :tangle in
~org-babel-get-src-block-info~:
1. org-babel-default-header-args
1. TANGLE-FILE of ~org-babel-tangle~
2. org-babel-default-header-args:<lang>
3. org-babel-params-from-properties
4. org-element-property :parameters datum
5. org-element-property :header datum
It wasn't clear for me: will ":tangle yes" or explicit ":tangle no" be
affected by TARGET-FILE. Maybe if we rephrase as follows it will be
clear for both of us:
Optional argument TARGET-FILE can be used to overwrite a default
export file in `org-babel-default-header-args' for all source
blocks.
> What if we have :tangle "/path/to/foo" and TARGET-FILE = "/path/to/foo"?
> What if they are :tangle "./foo" and TARGET-FILE = "/full/path/to/foo"?
See the new tests in the patch, I tried to take it into account.
>> (defun org-babel-effective-tangled-filename (buffer-fn src-lang src-tfile)
>> - "Return effective tangled filename of a source-code block.
>> + "Return effective tangled absolute filename of a source-code block.
>
> This will likely cause breakage.
> There are two callers of `org-babel-effective-tangled-filename:
> 1. `org-babel-tangle-collect-blocks'
> 2. `org-babel-tangle-single-block'
>
> `org-babel-tangle-single-block' passes (nth 1 result) as BUFFER-FN.
> Its value is
>
> (if org-babel-tangle-use-relative-file-links
> (file-relative-name file)
> file)
>
> So,
>
>> + (let* ((fnd (file-name-directory (buffer-file-name
>> + (get-buffer buffer-fn))))
>
> will fail when FILE contains file path.
> And it does: (file (buffer-file-name (buffer-base-buffer)))
Thanks, fixed: both `org-babel-tangle-single-block' and
`org-babel-tangle-collect-blocks' now pass absolute value to
`org-babel-effective-tangled-filename'.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v3-0001-testing-lisp-test-ob-tangle.el-Test-block-collect.patch --]
[-- Type: text/x-diff, Size: 6493 bytes --]
From 7235bf0306d12f6644838ad8542ac8822bcde258 Mon Sep 17 00:00:00 2001
From: Evgenii Klimov <eugene.dev@lipklim.org>
Date: Fri, 21 Jul 2023 22:40:06 +0100
Subject: [PATCH v3 1/2] testing/lisp/test-ob-tangle.el: Test block collection
into groups for tangling
* testing/lisp/test-ob-tangle.el (ob-tangle/collect-blocks): Test
block collection into groups for tangling.
(ob-tangle/collect-blocks-with-target-file): The same but with
TARGET-FILE.
---
testing/examples/babel.org | 75 ++++++++++++++++++++++++++++++++++
testing/lisp/test-ob-tangle.el | 60 +++++++++++++++++++++++++++
2 files changed, 135 insertions(+)
diff --git a/testing/examples/babel.org b/testing/examples/babel.org
index d46afeb5e..2d7b39d4e 100644
--- a/testing/examples/babel.org
+++ b/testing/examples/babel.org
@@ -490,3 +490,78 @@ The =[[= causes a false positive which ~org-babel-detangle~ should handle proper
:END:
#+begin_src emacs-lisp :tangle yes :comments link
#+end_src
+* tangle collect blocks
+:PROPERTIES:
+:ID: fae6bb5b-555a-4d68-9658-a30ac5d1b2ba
+:END:
+** with :tangle in properties
+:PROPERTIES:
+:ID: b2021d51-253c-4b26-9988-dac9193eb00b
+:header-args: :tangle relative.el
+:END:
+#+begin_src emacs-lisp
+"H1: no :tangle, but :tangle relative.el in properties"
+#+end_src
+
+#+begin_src emacs-lisp :tangle yes
+"H1: :tangle yes (to babel.el)"
+#+end_src
+
+#+begin_src emacs-lisp :tangle no
+"H1: should be ignored"
+#+end_src
+
+#+begin_src emacs-lisp :tangle ./babel.el
+"H1: :tangle ./babel.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle relative.el
+"H1: :tangle relative.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle ./relative.el
+"H1: :tangle ./relative.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle /tmp/absolute.el
+"H1: :tangle /tmp/absolute.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle ~/../../tmp/absolute.el
+"H1: :tangle ~/../../tmp/absolute.el"
+#+end_src
+** without :tangle in properties
+:PROPERTIES:
+:ID: 9f9afb0e-ba6d-4f63-9735-71af48ecd4e6
+:END:
+#+begin_src emacs-lisp
+"H2: no :tangle"
+#+end_src
+
+#+begin_src emacs-lisp :tangle yes
+"H2: :tangle yes (to babel.el)"
+#+end_src
+
+#+begin_src emacs-lisp :tangle no
+"H2: should be ignored"
+#+end_src
+
+#+begin_src emacs-lisp :tangle babel.el
+"H2: :tangle babel.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle relative.el
+"H2: :tangle relative.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle ./relative.el
+"H2: :tangle ./relative.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle /tmp/absolute.el
+"H2: :tangle /tmp/absolute.el"
+#+end_src
+
+#+begin_src emacs-lisp :tangle ~/../../tmp/absolute.el
+"H2: :tangle ~/../../tmp/absolute.el"
+#+end_src
diff --git a/testing/lisp/test-ob-tangle.el b/testing/lisp/test-ob-tangle.el
index 07e75f4d3..09eeffed7 100644
--- a/testing/lisp/test-ob-tangle.el
+++ b/testing/lisp/test-ob-tangle.el
@@ -569,6 +569,66 @@ another block
(set-buffer-modified-p nil))
(kill-buffer buffer))))
+(ert-deftest ob-tangle/collect-blocks ()
+ "Test block collection into groups for tangling."
+ (org-test-at-id "fae6bb5b-555a-4d68-9658-a30ac5d1b2ba"
+ (org-narrow-to-subtree)
+ (let ((expected-targets (cons '("/tmp/absolute.el" . 4)
+ (map-apply
+ (lambda (file numblocks)
+ (cons (expand-file-name file
+ org-test-example-dir)
+ numblocks))
+ '(("relative.el" . 5) ("babel.el" . 4)))))
+ (collected-blocks (org-babel-tangle-collect-blocks)))
+ (should (= (length expected-targets)
+ (length (map-keys collected-blocks))))
+ (let ((collected-targets (map-apply (lambda (file blocks) ; full blocks itself
+ (cons (expand-file-name file
+ org-test-example-dir)
+ (length blocks)))
+ collected-blocks)))
+ (should (equal (length expected-targets)
+ (length (map-filter
+ (lambda (file numblocks)
+ (= numblocks
+ (cdr (assoc-string file collected-targets))))
+ expected-targets))))))))
+
+(ert-deftest ob-tangle/collect-blocks-with-target-file ()
+ "Test block collection into groups for tangling with TARGET-FILE
+as `org-babel-tangle' would do."
+ (org-test-at-id "fae6bb5b-555a-4d68-9658-a30ac5d1b2ba"
+ (org-narrow-to-subtree)
+ (let* ((expected-targets (cons '("/tmp/absolute.el" . 4)
+ (map-apply
+ (lambda (file numblocks)
+ (cons (expand-file-name file
+ org-test-example-dir)
+ numblocks))
+ '(("relative.el" . 5) ("babel.el" . 5)))))
+ ;; simulate `org-babel-tangle' and `org-babel-load-file'
+ ;; TARGET-FILE
+ (org-babel-default-header-args
+ (org-babel-merge-params
+ org-babel-default-header-args
+ (list (cons :tangle
+ (expand-file-name "babel.el" org-test-example-dir)))))
+ (collected-blocks (org-babel-tangle-collect-blocks)))
+ (should (= (length expected-targets)
+ (length (map-keys collected-blocks))))
+ (let ((collected-targets (map-apply (lambda (file blocks) ; full blocks itself
+ (cons (expand-file-name file
+ org-test-example-dir)
+ (length blocks)))
+ collected-blocks)))
+ (should (equal (length expected-targets)
+ (length (map-filter
+ (lambda (file numblocks)
+ (= numblocks
+ (cdr (assoc-string file collected-targets))))
+ expected-targets))))))))
+
(provide 'test-ob-tangle)
;;; test-ob-tangle.el ends here
--
2.34.1
[-- Attachment #3: /home/eugene/git/org-mode/patches/v3-0002-ob-tangle.el-Avoid-relative-file-names-when-group.patch --]
[-- Type: message/external-body, Size: 103 bytes --]
next prev parent reply other threads:[~2023-07-24 12:31 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-12 20:59 [BUG] ob-tangle.el: Blocks overwrite each other when grouping before tangling Evgenii Klimov
2023-07-12 22:51 ` [PATCH] " Evgenii Klimov
2023-07-13 10:52 ` [PATCH v2] " Evgenii Klimov
2023-07-14 8:45 ` Ihor Radchenko
2023-07-24 12:28 ` Evgenii Klimov [this message]
2023-07-25 7:02 ` [PATCH v3] " Ihor Radchenko
2023-07-25 16:12 ` [PATCH v4] " Evgenii Klimov
2023-07-26 7:20 ` Ihor Radchenko
2023-07-26 15:07 ` [PATCH v5] " Evgenii Klimov
2023-07-28 7:29 ` Ihor Radchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bkg1h4q3.fsf@lipklim.org \
--to=eugene.dev@lipklim.org \
--cc=emacs-orgmode@gnu.org \
--cc=yantar92@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).