From: Leo Butler <Leo.Butler@umanitoba.ca>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: tbanelwebmin <tbanelwebmin@free.fr>,
"emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: [PATCH] ob-C.el compile-only header argument, was Re: How to use mpirun with C or C++ Org-babel?
Date: Tue, 19 Dec 2023 22:14:19 +0000 [thread overview]
Message-ID: <87h6kdyh52.fsf_-_@t14.reltub.ca> (raw)
In-Reply-To: <87r0joq3kr.fsf@localhost> (Ihor Radchenko's message of "Thu, 14 Dec 2023 14:08:36 +0000")
[-- Attachment #1: Type: text/plain, Size: 4889 bytes --]
On Thu, Dec 14 2023, Ihor Radchenko <yantar92@posteo.net> wrote:
> Leo Butler <Leo.Butler@umanitoba.ca> writes:
>
>> From 7d8e406bc4a92e2e2eab772b2671dcd72ca8c202 Mon Sep 17 00:00:00 2001
>> From: Leo Butler <leo.butler@umanitoba.ca>
>> Date: Tue, 12 Dec 2023 12:32:41 -0600
>> Subject: [PATCH] lisp/ob-C.el: add :compile-only header to compile to a named
>> target
>
> Thanks for the patch!
Thank you for the feedback.
>
>> * lisp/ob-C.el (org-babel-C-execute): The new header argument,
>> `:compile-only', causes source and compiled binary files to be named
>> using the `:file' header argument. When `:compile-only' is set,
>> execution of source block ends at compilation. The naming of source
>> and binary filenames is factored out to `org-babel-C-src/bin-file'.
>
> What will happen if we have something like :results value or :results
> output instead of :results file link?
Originally, I felt that only ":results file" makes sense. I have adopted
your suggestion, though, and added test cases so that the compiler
stderr output is caught.
>
>> * lisp/ob-C.el (org-babel-C-src/bin-file): A new function that factors
>> out the setting of source and binary filenames. It also signals an
>> error if `:compile-only' is set, but `:file' is not.
>> * testing/examples/ob-C-test.org: Add three example that exercise the
>> `:compile-only' header argument, including one that causes an error.
>> * testing/lisp/test-ob-C.el: Add three tests of the `:compile-only'
>> header argument. New tests: ob-C/set-src+bin-file-name-{1,2,3}.
>
> You should also announce the new feature in ORG-NEWS and document it in
> WORG.
I have added the announcement in this patch. I will submit a separate
patch for worg.
>
>> + (compile-only . (nil no t yes)))
>
> Why nil/t? No other header argument allow "nil" or "t". Just yes/no.
Ok. I also noticed (in a separate thread) that it should be
(compile-only . ((no yes))
>
>> +(defun org-babel-C-src/bin-file (params src? compile-only?)
>> + "Return the src or bin filename to `org-babel-C-execute'.
>> +
>> +If `SRC?' is T, a file extension is added to the filename. By
>
> Just SRC?. You should only quote Elisp symbols and upcase the function
> arguments. See
> https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html
>
> Also, why upcase "T"?
Corrected.
>
>> +default, the filename is created by `org-babel-temp-file'. If
>> +`COMPILE-ONLY?' is T, the filename is taken from the `:file'
>
> I think quoting :file is not necessary.
Yes.
>
>> +field in `PARAMS'; if that is NIL, an error occurs."
>
> No need to upcase NIL.
Yes.
> Also, "if that is nil, throw an error" - this is more common style
> (saying what function does).
Done.
>
>> + (let ((f (cdr (assq :file params))))
>
> Please avoid short variable names - they are harder to understand and
> search in code.
Done.
>
>> + (when (and compile-only? (null f))
>> + (error "Error: When COMPILE-ONLY is T or YES, output FILE needs to be set"))
>
> t or "yes". Also, what does "output FILE" refer to? Upcasing implies
> function argument, but you are referring to :file header argument in PARAMS.
Ok, I think the current error message is consistent with Gnu
standards.
>
>> + (let* ((file (cond (compile-only? f) (src? "C-src-") (t "C-bin-")))
>> + (ext (if src? (pcase org-babel-c-variant
>> + (`c ".c") (`cpp ".cpp") (`d ".d"))
>
> We usually split `cond' and `case' into multiple lines for readability.
> Otherwise, it is confusing, especially in `let' forms where one can
> confuse `cond' forms with `let' bindings.
Ok. I guess, to paraphrase, that readability is in the eye of the beholder.
>
>> + (unless compile-only?
>> + (let ((results
>> + (org-babel-eval
>> ...
>
> No return value at all? I'd expect file link to be returned, as we
> discussed in another thread. Also, see the above considerations about
> :results value/output.
See my comment above.
> We might want return the compiler output for
> :results output.
Done.
> Or maybe even arrange `org-babel-eval-error-notify' to
> display compile-mode window when there are compilation warnings.
Yes, this is already done by `org-babel-eval' in `org-babel-C-execute'.
>
>> --- a/testing/examples/ob-C-test.org
>> +++ b/testing/examples/ob-C-test.org
>> @@ -174,3 +174,29 @@ std::cout << "\"line 1\"\n";
>> std::cout << "\"line 2\"\n";
>> std::cout << "\"line 3\"\n";
>> #+end_src
>
> If you can, please avoid adding tests as Org files where possible.
> Instead, we prefer using `org-test-with-temp-text' or
> `org-test-with-temp-text-in-file' when the Org fragment for the test is
> not too long.
Ok.
Patch attached.
Leo
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-ob-C.el-add-compile-only-header-to-compile-to-a.patch --]
[-- Type: text/x-diff; name="0001-lisp-ob-C.el-add-compile-only-header-to-compile-to-a.patch", Size: 15253 bytes --]
From 37de68b264bca5f8103a1664626f1572d43f07d4 Mon Sep 17 00:00:00 2001
From: Leo Butler <leo.butler@umanitoba.ca>
Date: Tue, 12 Dec 2023 12:32:41 -0600
Subject: [PATCH] lisp/ob-C.el: add :compile-only header to compile to a named
target
* lisp/ob-C.el (org-babel-C-execute): The new header argument,
:compile-only, causes source and compiled binary files to be named
using the :file header argument. When :compile-only is set, execution
of source block ends at compilation. If :results is set to `output',
then any compiler errors or warnings are returned. The naming of
source and binary filenames is factored out to
org-babel-C-src/bin-file.
* lisp/ob-C.el (org-babel-C-src/bin-file): A new function that factors
out the setting of source and binary filenames. It also signals an
error if :compile-only is set, but :file is not.
* testing/examples/ob-C-test.org: Add six examples that exercise the
:compile-only header argument, including one that throws an error, one
that returns a compiler error message, and one that returns a compiler
warning.
* testing/lisp/test-ob-C.el: Add five tests of the :compile-only
header argument. New tests: ob-C/set-src+bin-file-name-{1,2,3,4,5}.
* etc/ORG-NEWS: Announce the new header argument.
Refs: https://list.orgmode.org/87fs81egk5.fsf@t14.reltub.ca/
https://list.orgmode.org/87msukbadu.fsf@localhost/
---
etc/ORG-NEWS | 10 +++
lisp/ob-C.el | 116 ++++++++++++++++++++-------------
testing/examples/ob-C-test.org | 57 ++++++++++++++++
testing/lisp/test-ob-C.el | 97 +++++++++++++++++++++++++++
4 files changed, 236 insertions(+), 44 deletions(-)
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 6c81221c1..09474151d 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -735,6 +735,16 @@ Completion is enabled for links to man pages added using ~org-insert-link~:
=C-c C-l man RET emacscl TAB= to get =emacsclient=. Of course, the ~ol-man~
library should be loaded first.
+*** =ob-C.el=: add ~:compile-only~ header to compile to a named target
+
+=ob-C= has a new header argument: ~:compile-only~.
+
+The new header argument, ~:compile-only~, causes source and compiled
+binary files to be named using the ~:file~ header argument. When
+~:compile-only~ is set, execution of source block ends at compilation.
+If ~:results~ is set to ~output~ then any compiler errors or warnings
+are returned.
+
** New functions and changes in function arguments
*** ~org-fold-hide-drawer-all~ is now interactive
diff --git a/lisp/ob-C.el b/lisp/ob-C.el
index 0278fc02a..c07c4585c 100644
--- a/lisp/ob-C.el
+++ b/lisp/ob-C.el
@@ -53,7 +53,8 @@
(main . :any)
(flags . :any)
(cmdline . :any)
- (libs . :any))
+ (libs . :any)
+ (compile-only . ((no yes))))
"C/C++-specific header arguments.")
(defconst org-babel-header-args:C++
@@ -128,17 +129,35 @@ This function is called by `org-babel-execute-src-block'."
"Expand C BODY according to its header arguments PARAMS."
(let ((org-babel-c-variant 'c)) (org-babel-C-expand-C body params)))
+(defun org-babel-C-src/bin-file (params src? compile-only?)
+ "Return the src or bin filename to `org-babel-C-execute'.
+
+If SRC? is t, a file extension is added to the filename. By
+default, the filename is created by `org-babel-temp-file'. If
+COMPILE-ONLY? is t, the filename is taken from the :file
+field in PARAMS; if that is nil, throw an error."
+ (let ((file (cdr (assq :file params))))
+ (when (and compile-only? (null file))
+ (error "Error: When compile-only header argument is \"yes\", file header argument needs to be set"))
+ (let* ((basename (cond (compile-only? file)
+ (src? "C-src-")
+ (t "C-bin-")))
+ (ext (if src? (pcase org-babel-c-variant
+ (`c ".c")
+ (`cpp ".cpp")
+ (`d ".d"))
+ org-babel-exeext)))
+ (org-babel-process-file-name
+ (if compile-only? (concat basename ext)
+ (org-babel-temp-file basename ext))))))
+
(defun org-babel-C-execute (body params)
"Execute C/C++/D BODY according to its header arguments PARAMS.
This function should only be called by `org-babel-execute:C' or
`org-babel-execute:C++' or `org-babel-execute:D'."
- (let* ((tmp-src-file (org-babel-temp-file
- "C-src-"
- (pcase org-babel-c-variant
- (`c ".c") (`cpp ".cpp") (`d ".d"))))
- (tmp-bin-file ;not used for D
- (org-babel-process-file-name
- (org-babel-temp-file "C-bin-" org-babel-exeext)))
+ (let* ((compile-only? (string= (cdr (assq :compile-only params)) "yes"))
+ (tmp-src-file (org-babel-C-src/bin-file params t compile-only?))
+ (tmp-bin-file (org-babel-C-src/bin-file params nil compile-only?)) ;not used for D
(cmdline (cdr (assq :cmdline params)))
(cmdline (if cmdline (concat " " cmdline) ""))
(flags (cdr (assq :flags params)))
@@ -155,46 +174,55 @@ This function should only be called by `org-babel-execute:C' or
(pcase org-babel-c-variant
(`c (org-babel-C-expand-C body params))
(`cpp (org-babel-C-expand-C++ body params))
- (`d (org-babel-C-expand-D body params)))))
+ (`d (org-babel-C-expand-D body params))))
+ compilation-results)
(with-temp-file tmp-src-file (insert full-body))
(pcase org-babel-c-variant
((or `c `cpp)
- (org-babel-eval
- (format "%s -o %s %s %s %s"
- (pcase org-babel-c-variant
- (`c org-babel-C-compiler)
- (`cpp org-babel-C++-compiler))
- tmp-bin-file
- flags
- (org-babel-process-file-name tmp-src-file)
- libs)
- ""))
+ (let ((compile-cmd (format "%s -o %s %s %s %s"
+ (pcase org-babel-c-variant
+ (`c org-babel-C-compiler)
+ (`cpp org-babel-C++-compiler))
+ tmp-bin-file
+ flags
+ (org-babel-process-file-name tmp-src-file)
+ libs)))
+ (message compile-cmd)
+ (setq compilation-results
+ (org-babel-eval compile-cmd "")))
+ (if (and (string= compilation-results "") (get-buffer org-babel-error-buffer-name))
+ (setq compilation-results (with-current-buffer (get-buffer org-babel-error-buffer-name)
+ (buffer-substring (point-min) (point-max))))))
(`d nil)) ;; no separate compilation for D
- (let ((results
- (org-babel-eval
- (pcase org-babel-c-variant
- ((or `c `cpp)
- (concat tmp-bin-file cmdline))
- (`d
- (format "%s %s %s %s"
- org-babel-D-compiler
- flags
- (org-babel-process-file-name tmp-src-file)
- cmdline)))
- "")))
- (when results
- (setq results (org-remove-indentation results))
- (org-babel-reassemble-table
- (org-babel-result-cond (cdr (assq :result-params params))
- results
- (let ((tmp-file (org-babel-temp-file "c-")))
- (with-temp-file tmp-file (insert results))
- (org-babel-import-elisp-from-file tmp-file)))
- (org-babel-pick-name
- (cdr (assq :colname-names params)) (cdr (assq :colnames params)))
- (org-babel-pick-name
- (cdr (assq :rowname-names params)) (cdr (assq :rownames params)))))
- )))
+ (cond (compile-only?
+ (org-babel-result-cond (cdr (assq :result-params params))
+ compilation-results))
+ (t
+ (let ((results
+ (org-babel-eval
+ (pcase org-babel-c-variant
+ ((or `c `cpp)
+ (concat tmp-bin-file cmdline))
+ (`d
+ (format "%s %s %s %s"
+ org-babel-D-compiler
+ flags
+ (org-babel-process-file-name tmp-src-file)
+ cmdline)))
+ "")))
+ (when results
+ (setq results (org-remove-indentation results))
+ (org-babel-reassemble-table
+ (org-babel-result-cond (cdr (assq :result-params params))
+ results
+ (let ((tmp-file (org-babel-temp-file "c-")))
+ (with-temp-file tmp-file (insert results))
+ (org-babel-import-elisp-from-file tmp-file)))
+ (org-babel-pick-name
+ (cdr (assq :colname-names params)) (cdr (assq :colnames params)))
+ (org-babel-pick-name
+ (cdr (assq :rowname-names params)) (cdr (assq :rownames params)))))
+ )))))
(defun org-babel-C-expand-C++ (body params)
"Expand C/C++ BODY with according to its header arguments PARAMS."
diff --git a/testing/examples/ob-C-test.org b/testing/examples/ob-C-test.org
index c7a96f665..2212192fb 100644
--- a/testing/examples/ob-C-test.org
+++ b/testing/examples/ob-C-test.org
@@ -174,3 +174,60 @@ std::cout << "\"line 1\"\n";
std::cout << "\"line 2\"\n";
std::cout << "\"line 3\"\n";
#+end_src
+
+* File naming
+:PROPERTIES:
+:ID: 1a691f36-f9c1-4531-8fc0-ee7b21ef5975
+:END:
+
+The binary file is saved in =./hello-world=.
+
+#+name: compile-only-1
+#+begin_src C++ :includes <iostream> :results none :compile-only yes :file ./hello-world
+std::cout << "Hello World!\n";
+#+end_src
+
+A link to the binary file is inserted when =:results file= is set.
+
+#+name: compile-only-2
+#+begin_src C++ :includes <iostream> :results file :compile-only yes :file ./hello-world
+std::cout << "Hello World!\n";
+#+end_src
+
+#+RESULTS: compile-only-2
+[[file:./hello-world]]
+
+Error! The =:file= header is unset.
+
+#+name: compile-only-3
+#+begin_src C++ :includes <iostream> :results none :compile-only yes
+std::cout << "Hello World!\n";
+#+end_src
+
+Syntax Error! =*Org-Babel Error Output*= buffer is popped open. The
+contents of that buffer are also the results of this block.
+
+#+name: compile-only-4.0
+#+begin_src C++ :includes <iostream> :results output :compile-only yes :file ./hello-world
+std::cout << "Hello World!\n" <== syntax error
+#+end_src
+
+Same, except results are silenced.
+
+#+name: compile-only-4.1
+#+begin_src C++ :includes <iostream> :results none :compile-only yes :file ./hello-world
+std::cout << "Hello World!\n" <== syntax error
+#+end_src
+
+Warnings.
+
+#+name: compile-only-5
+#+begin_src C++ :includes <iostream> :flags -Wall :results output :wrap example :compile-only yes :file ./hello-world
+int i{0};
+std::cout << "Hello World!\n";
+#+end_src
+
+
+
+
+
diff --git a/testing/lisp/test-ob-C.el b/testing/lisp/test-ob-C.el
index c70534a51..559a3aa05 100644
--- a/testing/lisp/test-ob-C.el
+++ b/testing/lisp/test-ob-C.el
@@ -200,5 +200,102 @@ std::cout << (x == y);
"\"line 1\"\n\"line 2\"\n\"line 3\"\n"
(org-babel-execute-src-block))))))
+(ert-deftest ob-C/compile-only-1 ()
+ "Test `:compile-only' header argument."
+ (if (executable-find org-babel-C++-compiler)
+ (unwind-protect
+ (let* ((file (make-temp-name "hello-world-"))
+ (file.cpp (concat file ".cpp")))
+ (org-test-with-temp-text
+ (format "#+source: compile-only-1
+#+begin_src cpp :includes <iostream> :results none :compile-only yes :file %s
+std::cout << \"Hello World!\\n\";
+#+end_src
+" file)
+ (should (null (org-babel-execute-src-block)))
+ (should (file-exists-p file))
+ (should (file-exists-p file.cpp))))
+ (ignore-errors (delete-file file))
+ (ignore-errors (delete-file file.cpp)))))
+
+(ert-deftest ob-C/compile-only-2 ()
+ "Test `:compile-only' header argument.
+A link to the binary file should be inserted."
+ (if (executable-find org-babel-C++-compiler)
+ (unwind-protect
+ (let* ((file (make-temp-name "hello-world-"))
+ (file.cpp (concat file ".cpp")))
+ (org-test-with-temp-text
+ (format "#+source: compile-only-2
+#+begin_src cpp :includes <iostream> :results file :compile-only yes :file %s
+std::cout << \"Hello World!\\n\";
+#+end_src
+" file)
+ (should (string= file (org-babel-execute-src-block)))
+ (should (file-exists-p file))
+ (should (file-exists-p file.cpp))))
+ (ignore-errors (delete-file file))
+ (ignore-errors (delete-file file.cpp)))))
+
+(ert-deftest ob-C/compile-only-3 ()
+ "Test `:compile-only' header argument.
+The :file header is unset, which throws an error."
+ (if (executable-find org-babel-C++-compiler)
+ (org-test-with-temp-text
+ (format "#+source: compile-only-3
+#+begin_src cpp :includes <iostream> :results file :compile-only yes
+std::cout << \"Hello World!\\n\";
+#+end_src
+")
+ (should-error (org-babel-execute-src-block)))))
+
+(ert-deftest ob-C/compile-only-4 ()
+ "Test `:compile-only' header argument.
+
+A C++ syntax error throws a compiler error. The compiler's error
+output is caught in `org-babel-error-buffer-name'; that buffer's
+contents are the result of the source block."
+ (if (executable-find org-babel-C++-compiler)
+ (unwind-protect
+ (let* ((file (make-temp-name "hello-world-"))
+ (file.cpp (concat file ".cpp")))
+ (org-test-with-temp-text
+ (format "#+source: compile-only-4.0
+#+begin_src cpp :includes <iostream> :results output :compile-only yes :file %s
+std::cout << \"Hello World!\\n\" <== syntax error
+#+end_src
+" file)
+ (should (string-match "error:" (org-babel-execute-src-block)))
+ (should (get-buffer org-babel-error-buffer-name))
+ (should (file-exists-p file.cpp))))
+ (ignore-errors (kill-buffer org-babel-error-buffer-name))
+ (ignore-errors (delete-file file))
+ (ignore-errors (delete-file file.cpp)))))
+
+(ert-deftest ob-C/compile-only-5 ()
+ "Test `:compile-only' header argument.
+
+A C++ warning is emitted on stderr. The compiler's warning is
+caught in `org-babel-error-buffer-name'; that buffer's contents
+are the result of the source block."
+ (if (executable-find org-babel-C++-compiler)
+ (unwind-protect
+ (let* ((file (make-temp-name "hello-world-"))
+ (file.cpp (concat file ".cpp")))
+ (org-test-with-temp-text
+ (format "#+source: compile-only-4.0
+#+begin_src cpp :includes <iostream> :flags -Wall :results output :compile-only yes :file %s
+int i;
+std::cout << \"Hello World!\\n\";
+#+end_src
+" file)
+ (should (string-match "warning:" (org-babel-execute-src-block)))
+ (should (get-buffer org-babel-error-buffer-name))
+ (should (file-exists-p file.cpp))))
+ (ignore-errors (kill-buffer org-babel-error-buffer-name))
+ (ignore-errors (delete-file file))
+ (ignore-errors (delete-file file.cpp)))))
+
+
(provide 'test-ob-C)
;;; test-ob-C.el ends here
--
2.42.0
next prev parent reply other threads:[~2023-12-19 22:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-07 23:10 How to use mpirun with C or C++ Org-babel? Edgar Lux
2023-12-08 10:36 ` tbanelwebmin
2023-12-08 15:47 ` Leo Butler
2023-12-08 22:24 ` Ihor Radchenko
2023-12-13 16:09 ` Leo Butler
2023-12-14 14:08 ` Ihor Radchenko
2023-12-19 22:14 ` Leo Butler [this message]
2023-12-22 12:20 ` [PATCH] ob-C.el compile-only header argument, was " Ihor Radchenko
2024-01-04 17:53 ` Leo Butler
2024-01-05 12:47 ` Ihor Radchenko
2023-12-20 18:15 ` Leo Butler
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=87h6kdyh52.fsf_-_@t14.reltub.ca \
--to=leo.butler@umanitoba.ca \
--cc=emacs-orgmode@gnu.org \
--cc=tbanelwebmin@free.fr \
--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).