Re: [PATCH] handle misquoting of target in build-scripts
> On Mon, Oct 31, 2022 at 05:46:55PM +0100, felix.winkelm...@bevuta.com wrote: > > Thanks, pushed. I had to hand-apply the patch as git-am didn't like it. I > > also > > fixed a scrutinizer warning. > > I suppose the warning was about target-librarian-options not being a > list? Unfortunately, splicing in these options wouldn't work properly - > they're taken from a baked in foreign value which may contain spaces. > > We'll have to figure out a way to keep certain arguments "as-is". > Attached is a pretty straightforward proposal to make this work. Thanks - pushed. > > PS: I noticed two more mistakes that have nothing to do with this - just > the platform was still being passed in to "filelist" and was *not* being > passed in to print-build-command. I've fixed these already and pushed > them as trivial patches. Excellent. cheers, felix
Re: [PATCH] handle misquoting of target in build-scripts
On Mon, Oct 31, 2022 at 05:46:55PM +0100, felix.winkelm...@bevuta.com wrote: > Thanks, pushed. I had to hand-apply the patch as git-am didn't like it. I also > fixed a scrutinizer warning. I suppose the warning was about target-librarian-options not being a list? Unfortunately, splicing in these options wouldn't work properly - they're taken from a baked in foreign value which may contain spaces. We'll have to figure out a way to keep certain arguments "as-is". Attached is a pretty straightforward proposal to make this work. PS: I noticed two more mistakes that have nothing to do with this - just the platform was still being passed in to "filelist" and was *not* being passed in to print-build-command. I've fixed these already and pushed them as trivial patches. Cheers, Peter From 651f70a66ed2ab3e70334466ae8a93b57ffbaa10 Mon Sep 17 00:00:00 2001 From: Peter Bex Date: Tue, 1 Nov 2022 10:39:32 +0100 Subject: [PATCH] Add a way to pass in already-quoted arguments to qs* The librarian options are kept around in the binary as a foreign string which may contain multiple flags separated by spaces. These should be taken verbatim by print-build-command. The way we do this is by wrapping such verbatim snippets in a "raw" record type and detecting these values in qs* and unpacking their strings directly from the struct instead of quoting them. --- egg-compile.scm | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/egg-compile.scm b/egg-compile.scm index 662fad3d..c1f2ceb0 100644 --- a/egg-compile.scm +++ b/egg-compile.scm @@ -650,7 +650,7 @@ link-objects (print-build-command (list out3) `(,out2 ,@lobjs) -`(,target-librarian ,target-librarian-options ,out3 ,out2 ,@lobjs) +`(,target-librarian ,(raw-arg target-librarian-options) ,out3 ,out2 ,@lobjs) platform))) (print-end-command platform))) @@ -1152,10 +1152,19 @@ EOF ;; have to undo that here again. It can also convert slashes to ;; backslashes on Windows, which is necessary in many cases when ;; running programs via "cmd". +;; +;; It also supports already-quoted arguments which can be taken as-is. (define (qs* arg platform #!optional slashify?) - (let* ((arg (->string arg)) -(path (if slashify? (slashify arg platform) arg))) -(qs path (if (eq? platform 'windows) 'mingw32 platform + (if (raw-arg? arg) + (raw-arg-value arg) + (let* ((arg (->string arg)) +(path (if slashify? (slashify arg platform) arg))) + (qs path (if (eq? platform 'windows) 'mingw32 platform) + +(define-record-type raw-arg + (raw-arg value) + raw-arg? + (value raw-arg-value)) (define (slashify str platform) (if (eq? platform 'windows) -- 2.36.2 signature.asc Description: PGP signature
Re: [PATCH] handle misquoting of target in build-scripts
> On Sun, Oct 30, 2022 at 02:46:49PM +0100, felix.winkelm...@bevuta.com wrote: > > Another follow-up patch on the recent change of quoting in chicken-install, > > which causes > > chicken-do to always fire (and thus rebuilding any egg, regardless of > > status). > > I noticed there were still inconsistencies here. > How about the attached patch instead? It may be a little bit > trickier to read (but not by much), but should eliminate the > problem entirely for chicken-do calls. > Thanks, pushed. I had to hand-apply the patch as git-am didn't like it. I also fixed a scrutinizer warning. felix
Re: [PATCH] handle misquoting of target in build-scripts
On Sun, Oct 30, 2022 at 02:46:49PM +0100, felix.winkelm...@bevuta.com wrote:
> Another follow-up patch on the recent change of quoting in chicken-install,
> which causes
> chicken-do to always fire (and thus rebuilding any egg, regardless of status).
I noticed there were still inconsistencies here.
How about the attached patch instead? It may be a little bit
trickier to read (but not by much), but should eliminate the
problem entirely for chicken-do calls.
Cheers,
Peter
From 730c74994a5feb96ef5d1d7ac2afdefbad65db7a Mon Sep 17 00:00:00 2001
From: Peter Bex
Date: Mon, 31 Oct 2022 11:44:52 +0100
Subject: [PATCH] Hopefully completely fix quoting hell in generated build
commands
Instead of using a mess of "qs*", "joins", "slashify" and "filelist"
and hoping for the best when generating chicken-do commands, use a
more principled method by way of a helper procedure to print the build
command.
This helper will receive *only* unquoted arguments: the list of
targets, list of dependencies and the build command line with flags as
a list. It then calls "qs*" on all of these where needed and emits a
corresponding chicken-do line.
By doing it this way, it's much more obvious where quotation happens:
only in print-build-command, and never in the procedure that calls it.
For consistency, also change prepare-custom-command so that it accepts
an unquoted filename, so that quotation is delegated to it.
For now, leave the code that emits installation commands untouched.
We'll probably want to do the same for these though.
---
egg-compile.scm | 333 +---
1 file changed, 142 insertions(+), 191 deletions(-)
diff --git a/egg-compile.scm b/egg-compile.scm
index 23af8b4e..6818c6e4 100644
--- a/egg-compile.scm
+++ b/egg-compile.scm
@@ -595,9 +595,8 @@
link-objects modules
custom types-file inline-file)
srcdir platform)
- (let* ((cmd (qs* (or (custom-cmd custom srcdir platform)
- default-csc)
- platform))
+ (let* ((cmd (or (custom-cmd custom srcdir platform)
+ default-csc))
(sname (prefix srcdir name))
(tfile (prefix srcdir (conc types-file ".types")))
(ifile (prefix srcdir (conc inline-file ".inline")))
@@ -613,51 +612,46 @@
(list "-emit-inline-file" ifile)
'(
(out1 (conc sname ".static"))
- (out2 (qs* (target-file (conc out1
- (object-extension platform))
- mode)
-platform))
+ (out2 (target-file (conc out1
+ (object-extension platform))
+mode))
(out3 (if (null? link-objects)
out2
- (qs* (target-file (conc out1
- (archive-extension platform))
- mode)
-platform)))
+ (target-file (conc out1
+ (archive-extension platform))
+mode)))
(targets (append (list out3 lfile)
(maybe types-file tfile)
(maybe inline-file ifile)
(map (lambda (m)
(prefix srcdir (conc m ".import.scm")))
(or modules '()
- (src (qs* (or source (conc name ".scm")) platform)))
+ (src (or source (conc name ".scm"
(when custom
(prepare-custom-command cmd platform))
-(print "\n" (qs* default-builder platform #t) " "
- (joins targets platform) " : "
- src " " (qs* eggfile platform) " "
- (if custom cmd "") " "
- (filelist srcdir source-dependencies platform)
- " : " cmd
- (if keep-generated-files " -k" "")
- " -regenerate-import-libraries"
- (if modules " -J" "") " -M"
- " -setup-mode -static -I " srcdir
- " -emit-link-file " (qs* lfile platform)
- (if (eq? mode 'host) " -host" "")
- " -D compiling-extension -c -unit " name
- " -D compiling-static-extension"
- " -C -I" srcdir " " (joins opts platform)
- " " src " -o " out2)
+(print-build-command targets
+`(,@(filelist srcdir source-dependencies) ,src ,eggfile
+ ,@(if custom (list cmd) '()))
+`(,cmd ,@(if keep-generated-files '("-k") '())
+ "-regenerate-import-libraries"
+ ,@(if modules '("-J") '()) "-M"
+ "-setup-mode" "-static" "-I" ,srcdir
+ "-emit-link-file" ,lfile
+
[PATCH] handle misquoting of target in build-scripts
Another follow-up patch on the recent change of quoting in chicken-install, which causes chicken-do to always fire (and thus rebuilding any egg, regardless of status). felix From 942b63f2a08dad36a3d73a68a3bac43bf7469ab2 Mon Sep 17 00:00:00 2001 From: felix Date: Sun, 30 Oct 2022 14:38:38 +0100 Subject: [PATCH] handle another double-quoting case which forced recompilation of eggs --- egg-compile.scm | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/egg-compile.scm b/egg-compile.scm index 14c93be0..6c9b21c5 100644 --- a/egg-compile.scm +++ b/egg-compile.scm @@ -613,16 +613,16 @@ (list "-emit-inline-file" ifile) '( (out1 (conc sname ".static")) - (out2 (qs* (target-file (conc out1 - (object-extension platform)) - mode) -platform)) + (out2 (target-file (conc out1 + (object-extension platform)) +mode)) (out3 (if (null? link-objects) out2 - (qs* (target-file (conc out1 - (archive-extension platform)) - mode) -platform))) + (target-file (conc out1 + (archive-extension platform)) +mode))) + (out2q (qs* out2 platform)) + (out3q (qs* out3 platform)) (targets (append (list out3 lfile) (maybe types-file tfile) (maybe inline-file ifile) @@ -647,16 +647,16 @@ " -D compiling-extension -c -unit " name " -D compiling-static-extension" " -C -I" srcdir (arglist opts platform) - " " src " -o " out2) + " " src " -o " out2q) (when (pair? link-objects) (let ((lobjs (filelist srcdir (map (cut conc <> ".static" (object-extension platform)) link-objects) platform))) -(print (qs* default-builder platform #t) " " out3 " : " - out2 " " lobjs " : " +(print (qs* default-builder platform #t) " " + out3q " : " out2q " " lobjs " : " (qs* target-librarian platform) " " - target-librarian-options " " out3 " " out2 " " + target-librarian-options " " out3q " " out2q " " lobjs))) (print-end-command platform))) @@ -684,7 +684,7 @@ (if inline-file (list "-emit-inline-file" ifile) '( - (out (qs* (target-file (conc sname ".so") mode) platform)) + (out (target-file (conc sname ".so") mode)) (src (qs* (or source (conc name ".scm")) platform)) (lobjs (map (lambda (lo) (target-file (conc lo -- 2.28.0
