bugGNU roff - Bugs: bug #64061, pdfpic.tmac requires non-standard...

 
 

bug #64061: pdfpic.tmac requires non-standard sed feature

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Mon 17 Apr 2023 08:33:25 AM UTC
   
 
Category:  Macro - others/general Severity:  3 - Normal
Item Group:  Incorrect behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.24.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 10 Jul 2023 08:55:45 AM UTC, comment #19: 


commit f60452a44c1109a9315042514d310a057b4666f3
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Apr 17 16:41:33 2023 -0500

    [pdfpic]: Fix Savannah #64061.

    * tmac/pdfpic.tmac: Refactor to make comprehensible some woefully
      undocumented cleverness and improve efficiency.

      (PDFPIC): Break out flaming-hoop-leaping "clever" bit of `sy` usage
      into its own macro, calling from here and relocating its requests from
      here...

      (pdfpic@get-image-dimensions): ...to here.  When using `sy` request to
      collect and munge output of pdfinfo(1), (a) disable the escape
      character while defining the macro; (b) construct the command in a
      roff string, appending to it in discrete, hopefully comprehensible
      chunks; (c) disable the escape character during macro interpretation
      wherever possible (most of it); (d) retain doubled backslashes so that
      they survive subsequent string interpolation; (e) stop using grep(1)
      in the pipeline when sed(1) is perfectly capable of performing its own
      input filtering; (f) invoke sed with '-n' option and emit output only
      upon a successful substitution; (g) replace unportable(!) POSIX BRE
      character class '[:digit:]' in substitution match text with '[0-9]';
      and most importantly (h) replace multi-line sed 's' replacement text
      (see below for the reason we can't use it) with single roff control
      line employing the groff extension escape sequence `\R` to assign
      multiple registers.  Annotate portability and escaping challenges.
      Tested on GNU/Linux, macOS 12, and (with simulated pdfinfo(1) output)
      Solaris 11.

    There is a problem with trying to embed true newlines into the arguments
    of a `sy` request.  The C++ function that GNU troff uses to assemble the
    command string (character by character) _does not recognize C/C++ string
    literal escape sequences_.  This means that you _cannot_ embed "\n" in
    `sy`'s arguments and have it survive, as a newline character, into the
    command string passed to the standard C library's system(3) function.
    ("A\nB" gets encoded as 'A', '\\', 'n', 'B', not 'A', '\n', 'B'.)
    Unfortunately, this appears to be AT&T troff-compatible behavior.  But
    it means that you _cannot_ portably construct multi-line replacement
    text for sed's 's' command.  (Other sed commands like 'a', 'c', and 'i'
    will be similarly affected.)  See Savannah #64071.

    * PROBLEMS: Drop item.

    Fixes <https://savannah.gnu.org/bugs/?64061>.  Thanks to Bruno Haible
    for the report, and to him and Ralph Corderoy for the discussion of
    portable and efficient sed constructs.


G. Branden Robinson <gbranden>
Group administrator
Mon 08 May 2023 12:40:13 AM UTC, comment #18: 

Updated fix on branch (does not expand test cases as mused about in comment #17).


commit 15ca5031a84d98a195403de89bf33a0b96c032a7
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Apr 17 16:41:33 2023 -0500

    [pdfpic]: Fix Savannah #64061.

    * tmac/pdfpic.tmac: Refactor to make comprehensible some woefully
      undocumented cleverness and improve efficiency.

      (PDFPIC): Break out flaming-hoop-leaping "clever" bit of `sy` usage
      into its own macro, calling from here and relocating its requests from
      here...

      (pdfpic@get-image-dimensions): ...to here.  When using `sy` request to
      collect and munge output of pdfinfo(1), (a) disable the escape
      character while defining the macro; (b) construct the command in a
      roff string, appending to it in discrete, hopefully comprehensible
      chunks; (c) disable the escape character during macro interpretation
      wherever possible (most of it); (d) retain doubled backslashes so that
      they survive subsequent string interpolation; (e) stop using grep(1)
      in the pipeline when sed(1) is perfectly capable of performing its own
      input filtering; (f) invoke sed with '-n' option and emit output only
      upon a successful substitution; (g) replace unportable(!) POSIX BRE
      character class '[:digit:]' in substitution match text with '[0-9]';
      and most importantly (h) replace multi-line sed 's' replacement text
      (see below for the reason we can't use it) with single roff control
      line employing the groff extension escape sequence `\R` to assign
      multiple registers.  Annotate portability and escaping challenges.
      Tested on GNU/Linux, macOS 12, and (with simulated pdfinfo(1) output)
      Solaris 11.

    There is a problem with trying to embed true newlines into the arguments
    of a `sy` request.  The C++ function that GNU troff uses to assemble the
    command string (character by character) _does not recognize C/C++ string
    literal escape sequences_.  This means that you _cannot_ embed "\n" in
    `sy`'s arguments and have it survive, as a newline character, into the
    command string passed to the standard C library's system(3) function.
    ("A\nB" gets encoded as 'A', '\\', 'n', 'B', not 'A', '\n', 'B'.)
    Unfortunately, this appears to be AT&T troff-compatible behavior.  But
    it means that you _cannot_ portably construct multi-line replacement
    text for sed's 's' command.  (Other sed commands like 'a', 'c', and 'i'
    will be similarly affected.)  See Savannah #64071.

    * PROBLEMS: Drop item.

    Fixes <https://savannah.gnu.org/bugs/?64061>.  Thanks to Bruno Haible
    for the report, and to him and Ralph Corderoy for the discussion of
    portable and efficient sed constructs.


And for grins...


commit 8de67a3bf1163b18ce8bfce54075fca8a6fd379b
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Wed Apr 26 04:36:05 2023 -0500

    [pdfpic]: Refactor (`sy` -> `pso`).

    * tmac/pdfpic.tmac: Migrate gathering of image dimensions from `sy` and
      a temporary file to `pso`.

      (pdfpic@cleanup): Drop `pdfpic*temporary-file` string.

      (pdfpic@get-image-dimensions): Remove redirection.  Invoke `pso`, not
      `sy`.

      (PDFPIC): Stop constructing `pdfpic*temporary-file` string.  Stop
      testing `systat` register.  Stop sourcing and deleting temporary file.

    We keep the temporary directory handling because we will need it for the
    `PSPIC` fallback logic, but it also promises to be really painful to fix
    that before we have more formatter support for string traversal.  See
    <https://savannah.gnu.org/bugs/index.php?64114>.


G. Branden Robinson <gbranden>
Group administrator
Sat 06 May 2023 09:45:28 PM UTC, comment #17: 

Hi Deri,

> The switch from using grep to sed, which seems to have caused issues,


Not exactly; it's more like it caused us to pay a lot more attention to something that was a bit jinky in the first place.

Here's what the code looked like when Bernd added it in 2015 (after groff 1.22.3).


+.\" get image dimensions
+.  ec @
+.  sy pdfinfo @$1 | \
+grep "Page *size" | \
+sed -e 's/Page *size: *\\([[:digit:].]*\\) *x *\\([[:digit:].]*\\).*$/\
+.nr pdf-wid (p;\\1)\\n\
+.nr pdf-ht  (p;\\2)/' \
+> /tmp/pdfpic\n[$$]
+.  so /tmp/pdfpic\n[$$]
+.  sy rm /tmp/pdfpic\n[$$]
+.  ec


The piece that isn't portable sed(1) is this bit:


.nr pdf-wid (p;\\1)\\n\


That '\\n' does not go into the shell command as an escape newline, but as the character sequence '\', 'n' (or, in C, '\\', 'n'), which is a GNU sed extension that the sed in macOS 12 also supports.  But other seds don't.

> given that the example pdf provided in bug #58206 was in fact an invalid pdf, which is why pdfinfo did not handle it correctly.


That may be true, but I was able to reproduce the problem using conventional Linux tools, and did so in my regression test.

https://git.savannah.gnu.org/cgit/groff.git/tree/tmac/tests/pdfpic_does-not-choke-on-bad-pdfinfo-output.sh?h=1.23.0.rc4#n68

...unless you're saying that my contrived /Title annotation is similarly defective in lacking a byte order mark, which you might be.

Maybe we should test both, since evidently a lot of PDFs have been produced by faulty tools that omitted the BOM.

> So, if it makes things any easier we could go back to a simple grep,


I'm afraid we can't.  All this byte order mark business is a side issue from the mission of the PDFPIC macro's call to `sy`: to scrape the image dimensions out of pdfinfo(1)'s output into two separate *roff registers.

Even if nobody ever produced PDFs with BOMs missing from their encoded text strings, or pdfinfo(1) never behaved badly (from our perspective) if they did, we'd still need to turn one line of pdfinfo output like this:


Page size:      612 x 792 pts


into valid (g)roff syntax--and that's going to take more than grep(1).

Does this clear things up?


G. Branden Robinson <gbranden>
Group administrator
Sat 06 May 2023 08:30:35 PM UTC, comment #16: 

The switch from using grep to sed, which seems to have caused issues, may well have been unnecessary, given that the example pdf provided in bug #58206 was in fact an invalid pdf, which is why pdfinfo did not handle it correctly. The pdf reference says:-

"For text strings encoded in Unicode, the first two bytes must be 254 followed by
255, representing the Unicode byte order marker, U+FEFF ."

From the byte dump in that bug you can see no such BOM is present. Please also note that later versions of pdfinfo (checked with v. 0.26.4) now handles the mangled title correctly (must be recognising the alternating zero bytes and "guessing" it is UTF-16 with a missing BOM."

So, if it makes things any easier we could go back to a simple grep, since if it fails we know it is a non-standard pdf and they are using an older version of pdfinfo.

Deri James <deri>
Group Member
Fri 28 Apr 2023 07:26:46 PM UTC, comment #15: 

Fixed on branch.  Still subject to change.


commit 66b3077b7c265525ab72aa807e43047e5b86de8c
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Mon Apr 17 16:41:33 2023 -0500

    [pdfpic]: Fix Savannah #64061.

    * tmac/pdfpic.tmac: Refactor to make comprehensible some woefully
      undocumented cleverness and improve efficiency.

      (PDFPIC): Break out flaming-hoop-leaping "clever" bit of `sy` usage
      into its own macro, calling from here and relocating its requests from
      here...

      (pdfpic@system): ...to here.  When using `sy` request to collect and
      munge output of pdfinfo(1), (a) disable the escape character while
      defining the macro; (b) construct the command in a roff string,
      appending to it in discrete, hopefully comprehensible chunks; (c)
      disable the escape character during macro interpretation wherever
      possible (most of it); (d) retain doubled backslashes so that they
      survive subsequent string interpolation; (e) stop using grep(1) in the
      pipeline when sed(1) is perfectly capable of performing its own input
      filtering; (f) invoke sed with '-n' option and emit output only upon a
      successful substitution; (g) replace unportable(!) POSIX BRE character
      class '[:digit:]' in substitution match text with '[0-9]'; and most
      importantly (h) replace multi-line sed 's' replacement text (see below
      for the reason we can't use it) with single roff control line
      employing the groff extension escape sequence `\R` to assign multiple
      registers.  Annotate portability and escaping challenges.  Tested on
      GNU/Linux, macOS 12, and (with simulated pdfinfo(1) output), on
      Solaris 11.

    There is a problem with trying to embed true newlines into the arguments
    of a `sy` request.  The C++ function that GNU troff uses to assemble the
    command string (character by character) _does not recognize C/C++ string
    literal escape sequences_.  This means that you _cannot_ embed "\n" in
    `sy`'s arguments and have it survive, as a newline character, into the
    command string passed to the standard C library's system(3) function.
    ("A\nB" gets encoded as 'A', '\\', 'n', 'B', not 'A', '\n', 'B'.)
    Unfortunately, this appears to be AT&T troff-compatible behavior.  But
    it means that you _cannot_ portably construct multi-line replacement
    text for sed's 's' command.  (Other sed commands like 'a', 'c', and 'i'
    will be similarly affected.)  See Savannah #64071.

    * PROBLEMS: Drop item.

    Fixes <https://savannah.gnu.org/bugs/?64061>.  Thanks to Bruno Haible
    for the report, and to him and Ralph Corderoy for the discussion of
    portable and efficient sed constructs.


G. Branden Robinson <gbranden>
Group administrator
Mon 24 Apr 2023 12:53:23 AM UTC, comment #14: 

comment #11:

> comment #9:
> True, but I thought you might have a setup that generated an expected temp file for the fake-pdfinfo input, which you could diff against mine, bypassing the eyeballs.


Ah, no...that didn't occur to me.

I don't really want to bypass eyeballs.  Many brains are better than few.
 

> Adding an output line to the temp file and groffing it also gives the same results on both Mac and Gentoo Linux:


Good deal!

comment #12:

> comment #5:
> > 1. Run pdfinfo twice and scrape out a different datum on each
> > pass.  I fear this would give up more than was gained by
> > eliminating grep from the pipeline.
>
> Not that we need more options at this point, since two others now have working implementations, but I don't think this particular objection has much weight: the grep wasn't eliminated because it was incurring a noticeable performance penalty, but because it was deemed unnecessary on code inspection.  I doubt the overhead of two invocations of pdfinfo would make a significant runtime difference even in a document that included dozens of PDFs.


I think that's fair.  The recovered efficiency is more significant in human brain cycles than CPU clock cycles.

G. Branden Robinson <gbranden>
Group administrator
Mon 24 Apr 2023 12:50:46 AM UTC, comment #13: 

Yet another realization has come to me.

Since we're in unsafe mode anyway (duh, `sy`), why are we messing with a temporary file only to source it?

This is what the groff `pso` request was made for.

So I expect I'll grind on this a bit harder before its eventual merge to master (which won't happen until 1.23.0 final is tagged or Bertrand green-lights it as okay for the RC series).

G. Branden Robinson <gbranden>
Group administrator
Sun 23 Apr 2023 09:06:08 PM UTC, comment #12: 

comment #5:

> 1. Run pdfinfo twice and scrape out a different datum on each
> pass.  I fear this would give up more than was gained by
> eliminating grep from the pipeline.


Not that we need more options at this point, since two others now have working implementations, but I don't think this particular objection has much weight: the grep wasn't eliminated because it was incurring a noticeable performance penalty, but because it was deemed unnecessary on code inspection.  I doubt the overhead of two invocations of pdfinfo would make a significant runtime difference even in a document that included dozens of PDFs.

Ultimately, though, this seems the least elegant of the three proposals, so even though it was mine initially, I'm happy to see it discarded.

Dave <barx>
Group Member
Sun 23 Apr 2023 08:29:07 PM UTC, comment #11: 

comment #9:

> I forgot to point you to my "private" branch on Savannah.


Right.  I did know about that branch, so I should have checked there.

> It looks good; of course one of the the challenges with this sort of
> cleverness is that it's easy for problems to escape the eyeballs.


True, but I thought you might have a setup that generated an expected temp file for the fake-pdfinfo input, which you could diff against mine, bypassing the eyeballs.

Adding an output line to the temp file and groffing it also gives the same results on both Mac and Gentoo Linux:

$ echo '.tm pdfpic*width \n[pdfpic*width] pdfpic*height \n[pdfpic*height]' >> /tmp/pdfpic-temporary-file
$ groff /tmp/pdfpic-temporary-file
pdfpic*width 612000 pdfpic*height 792000
$ nroff /tmp/pdfpic-temporary-file
pdfpic*width 2040 pdfpic*height 2640


Dave <barx>
Group Member
Sat 22 Apr 2023 07:08:32 PM UTC, comment #10: 


comment #8:

> comment #6:
> > And because I couldn't resist testing my mad sed stunta
> > skillz, here's a proof of concept.
>
> I got identical results to yours piping fake-pdfinfo into stunt.sed on OS X.  So your stunt is even portable (to at least one of the outlier seds).


As well it should be!  This is old-school sed that should be portable everywhere.

(He said, tempting fate.)

G. Branden Robinson <gbranden>
Group administrator
Sat 22 Apr 2023 07:07:25 PM UTC, comment #9: 

comment #7:

> comment #5:
> > Dave, can you give this a crack on your Mac OS X 10.11.6 host?
>
> I think I got it to work?
>
> You'll have to check my work, because I appear to be missing a step.  I took the pdfpic*system macro from the file "64061.diff" attached to http://lists.gnu.org/r/groff/2023-04/msg00190.html, but the patch in comment #5 is not against the same version of that macro; there appears to be an intermediate revision I'm missing.  So I interpolated, hopefully correctly.


Ah, yes, whoops.  I forgot to point you to my "private" branch on Savannah.

https://git.savannah.gnu.org/cgit/groff.git/commit/?h=branden-2023-04-20&id=4dc2c23f0c2b72517be62e60090f30b214299650

> First, I set up a small input file that is basically the "64061.diff" version of pdfpic*system with a couple edits to (a) call your fake-pdfinfo script from comment #6, rather than pdfinfo itself, so that we know exactly what data we're working with, and (b) hard-code a path in place of \*[pdfpic*temporary-file].  All the test file does is define the macro and then invoke it:


> $ cat pdfpic.test
> .eo
> .de pdfpic*system
> .  ds pdfpic*command ./fake-pdfinfo
> .  eo
> .  as pdfpic*command " | tr -d '\\000'
> .  as pdfpic*command " | sed -n -e '/Page *size:/
> .  as pdfpic*command s/Page *size: *\\([0-9.]*\\) *x *\([0-9.]*\\).*$/
> .  as pdfpic*command .nr pdfpic*width  (p;\\1)\\n
> .  as pdfpic*command .nr pdfpic*height (p;\\2)/'
> .  as pdfpic*command " -e tprint -e b -e :print -e p
> .  ec
> .  as pdfpic*command " > /tmp/pdfpic-temporary-file
> .  sy \*[pdfpic*command]
> .  rm pdfpic*command
> ..
> .ec
> .
> .  pdfpic*system
> $ groff -U pdfpic.test
> $ cat /tmp/pdfpic-temporary-file
> .nr pdfpic*width  (p;612)n.nr pdfpic*height (p;792)


> That didn't work and we knew it wouldn't, so no surprises there.


Your testing approach seems sound to me.
 

> Now's where I had to do a little guesswork to apply the patch.  Here's how my version of pdfpic.patched.test differs from the original test:


> $ diff -u pdfpic.test pdfpic.patched.test
> --- pdfpic.test 2023-04-22 07:41:13.000000000 -0500
> +++ pdfpic.patched.test 2023-04-22 07:43:54.000000000 -0500
> @@ -5,9 +5,9 @@
>  .  as pdfpic*command " | tr -d '\\000'
>  .  as pdfpic*command " | sed -n -e '/Page *size:/
>  .  as pdfpic*command s/Page *size: *\\([0-9.]*\\) *x *\([0-9.]*\\).*$/
> -.  as pdfpic*command .nr pdfpic*width  (p;\\1)\\n
> -.  as pdfpic*command .nr pdfpic*height (p;\\2)/'
> -.  as pdfpic*command " -e tprint -e b -e :print -e p
> +.  as pdfpic*command . \\\\R@pdfpic*width  (p;\\1)@
> +.  as pdfpic*command " \\\\R@pdfpic*height (p;\\2)@
> +.  as pdfpic*command /p'
>  .  ec
>  .  as pdfpic*command " > /tmp/pdfpic-temporary-file
>  .  sy \*[pdfpic*command]
> $ groff -U pdfpic.patched.test
> $ cat /tmp/pdfpic-temporary-file
> . \R@pdfpic*width  (p;612)@ \R@pdfpic*height (p;792)@


> So, presuming that
> 0 my patch is functionally equivalent to yours, and
> 0 the contents of the temp file are what you expected
> then it looks like we have a winner!


It looks good; of course one of the the challenges with this sort of cleverness is that it's easy for problems to escape the eyeballs.
 

> pdfpic.patched.test generates an identical temp file on my Linux box, so that's an encouraging sign.


Very much so!

G. Branden Robinson <gbranden>
Group administrator
Sat 22 Apr 2023 01:47:34 PM UTC, comment #8: 

comment #6:

> And because I couldn't resist testing my mad sed stunta
> skillz, here's a proof of concept.


I got identical results to yours piping fake-pdfinfo into stunt.sed on OS X.  So your stunt is even portable (to at least one of the outlier seds).

> I'm sure the FSF wouldn't appreciate me putting a "licensable"
> photograph of a skateboarder's skull about to meet high-speed concrete,


How about an ASCII art rendition?

Dave <barx>
Group Member
Sat 22 Apr 2023 01:24:20 PM UTC, comment #7: 

comment #5:

> Dave, can you give this a crack on your Mac OS X 10.11.6 host?


I think I got it to work?

You'll have to check my work, because I appear to be missing a step.  I took the pdfpic*system macro from the file "64061.diff" attached to http://lists.gnu.org/r/groff/2023-04/msg00190.html, but the patch in comment #5 is not against the same version of that macro; there appears to be an intermediate revision I'm missing.  So I interpolated, hopefully correctly.

First, I set up a small input file that is basically the "64061.diff" version of pdfpic*system with a couple edits to (a) call your fake-pdfinfo script from comment #6, rather than pdfinfo itself, so that we know exactly what data we're working with, and (b) hard-code a path in place of \*[pdfpic*temporary-file].  All the test file does is define the macro and then invoke it:

$ cat pdfpic.test
.eo
.de pdfpic*system
.  ds pdfpic*command ./fake-pdfinfo
.  eo
.  as pdfpic*command " | tr -d '\\000'
.  as pdfpic*command " | sed -n -e '/Page *size:/
.  as pdfpic*command s/Page *size: *\\([0-9.]*\\) *x *\([0-9.]*\\).*$/
.  as pdfpic*command .nr pdfpic*width  (p;\\1)\\n
.  as pdfpic*command .nr pdfpic*height (p;\\2)/'
.  as pdfpic*command " -e tprint -e b -e :print -e p
.  ec
.  as pdfpic*command " > /tmp/pdfpic-temporary-file
.  sy \*[pdfpic*command]
.  rm pdfpic*command
..
.ec
.
.  pdfpic*system
$ groff -U pdfpic.test
$ cat /tmp/pdfpic-temporary-file
.nr pdfpic*width  (p;612)n.nr pdfpic*height (p;792)

That didn't work and we knew it wouldn't, so no surprises there.

Now's where I had to do a little guesswork to apply the patch.  Here's how my version of pdfpic.patched.test differs from the original test:

$ diff -u pdfpic.test pdfpic.patched.test
--- pdfpic.test 2023-04-22 07:41:13.000000000 -0500
+++ pdfpic.patched.test 2023-04-22 07:43:54.000000000 -0500
@@ -5,9 +5,9 @@
 .  as pdfpic*command " | tr -d '\\000'
 .  as pdfpic*command " | sed -n -e '/Page *size:/
 .  as pdfpic*command s/Page *size: *\\([0-9.]*\\) *x *\([0-9.]*\\).*$/
-.  as pdfpic*command .nr pdfpic*width  (p;\\1)\\n
-.  as pdfpic*command .nr pdfpic*height (p;\\2)/'
-.  as pdfpic*command " -e tprint -e b -e :print -e p
+.  as pdfpic*command . \\\\R@pdfpic*width  (p;\\1)@
+.  as pdfpic*command " \\\\R@pdfpic*height (p;\\2)@
+.  as pdfpic*command /p'
 .  ec
 .  as pdfpic*command " > /tmp/pdfpic-temporary-file
 .  sy \*[pdfpic*command]
$ groff -U pdfpic.patched.test
$ cat /tmp/pdfpic-temporary-file
. \R@pdfpic*width  (p;612)@ \R@pdfpic*height (p;792)@

So, presuming that

  1. my patch is functionally equivalent to yours, and
  2. the contents of the temp file are what you expected

then it looks like we have a winner!

pdfpic.patched.test generates an identical temp file on my Linux box, so that's an encouraging sign.

Dave <barx>
Group Member
Sat 22 Apr 2023 02:39:16 AM UTC, comment #6: 

And because I couldn't resist testing my mad sed stunta skillz, here's a proof of concept.


$ cat ATTIC/fake-pdfinfo
#!/bin/sh

cat <<EOF
Creator:        groff version 1.23.0.rc3.81-9e3e4
Producer:       gropdf version 1.23.0.rc3.81-9e3e4
CreationDate:   Sat Mar 25 18:43:18 2023 CDT
ModDate:        Sat Mar 25 18:43:18 2023 CDT
Tagged:         no
UserProperties: no
Suspects:       no
Form:           none
JavaScript:     no
Pages:          1
Encrypted:      no
Page size:      612 x 792 pts (letter)
Page rot:       0
File size:      106745 bytes
Optimized:      no
PDF version:    1.4
EOF
$ cat ATTIC/stunt.sed
/Page  *size:/s/Page  *size:  *//;h;s/\([0-9.]*\)  *x  *\([0-9.]*\).*$/.nr pdfpic*width  (p;\1)/p;x;s/\([0-9.]*\)  *x  *\([0-9.]*\).*$/.nr pdfpic*height (p;\2)/p
$ ./ATTIC/fake-pdfinfo | sed -n -f ATTIC/stunt.sed
.nr pdfpic*width  (p;612)
.nr pdfpic*height (p;792)


It worked on the first try.  Maybe I'm a less crap sed programmer than I thought.

But at any rate I feel this is a bit lengthy, so I think I prefer solution 3, `\R`.

I'm sure the FSF wouldn't appreciate me putting a "licensable" photograph of a skateboarder's skull about to meet high-speed concrete, so the reader will just have to imagine one.

G. Branden Robinson <gbranden>
Group administrator
Sat 22 Apr 2023 02:25:54 AM UTC, comment #5: 

As discussed on the list, there are a few tricks available to us.

1. Run pdfinfo twice and scrape out a different datum on each pass.  I fear this would give up more than was gained by eliminating grep from the pipeline.

2. Perform a sed stunt involving the hold space.

3. Use groff's `\R` escape sequence to sneak two register assignments onto the same input line.

Here's an illustration of the last.


commit 0bad05526a72e63fd189babb166c0ad981370c42 (HEAD)
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Fri Apr 21 20:16:21 2023 -0500

    squash Fix Savannah #64061. (\R trick)

diff --git a/tmac/pdfpic.tmac b/tmac/pdfpic.tmac
index 024c10cd4..90a183774 100644
--- a/tmac/pdfpic.tmac
+++ b/tmac/pdfpic.tmac
@@ -77,8 +77,9 @@
 .  as pdfpic*command " | tr -d '\\000'
 .  as pdfpic*command " | sed -n -e '/Page  *size:/
 .  as pdfpic*command s/Page  *size:  *\\([0-9.]*\\)  *x  *\([0-9.]*\\).*$/
-.  as pdfpic*command .nr pdfpic*width  (p;\\1)\\n
-.  as pdfpic*command .nr pdfpic*height (p;\\2)/p'
+.  as pdfpic*command . \\\\R@pdfpic*width  (p;\\1)@
+.  as pdfpic*command " \\\\R@pdfpic*height (p;\\2)@
+.  as pdfpic*command /p'
 .  ec
 .  as pdfpic*command " > \*[pdfpic*temporary-file]
 .  sy \*[pdfpic*command]


This works for me on Solaris 11 (running my "fake-pdfinfo" output generator instead of the real thing, which isn't installed).

Dave, can you give this a crack on your Mac OS X 10.11.6 host?

G. Branden Robinson <gbranden>
Group administrator
Wed 19 Apr 2023 10:03:17 PM UTC, comment #4: 
G. Branden Robinson <gbranden>
Group administrator
Mon 17 Apr 2023 05:33:00 PM UTC, comment #3: 


comment #1:

> original submission:
> > 2.  It uses the sequence '\n' in the replacement part of a sed
> > 's' command.  This is non-standard and undefined on some seds.
>
> I raised this as a side note in comment 11 of bug 58206, but once that bug's primary issue was addressed, I didn't follow up on this point.  So I apologize for dropping the ball there.


It's okay.  It turns out that the side note was misleading.  But you could be forgive to draw the conclusion you did; I think almost anyone would.  There's deviltry afoot, but I now have a cast of the offending hoofprint.

> Some of the email threads linked elsewhere in that bug report point out the redundancy of the grep command, and offer other ways to skin that cat, such as an awk equivalent of the sed (which may or may not be more portable).


Awk is also notorious for its number of dialects, and requires Autoconf checks.

sed is practically always available and as far as I know there is a portable dialect of it that is still feature-complete as far as POSIX goes--you just have to be insanely careful.  Often this seems to mean breaking lines at every opportunity.

G. Branden Robinson <gbranden>
Group administrator
Mon 17 Apr 2023 05:29:22 PM UTC, comment #2: 

It turns out some of the "excess" escaping isn't excess, and the "\n" is not only portable but also necessary.  (Hint: sed never sees the "\n".)

More on this when I finish proving things out and have a tested patch ready.

G. Branden Robinson <gbranden>
Group administrator
Mon 17 Apr 2023 05:25:19 PM UTC, comment #1: 

original submission:

> 2.  It uses the sequence '\n' in the replacement part of a sed
> 's' command.  This is non-standard and undefined on some seds.


I raised this as a side note in comment 11 of bug 58206, but once that bug's primary issue was addressed, I didn't follow up on this point.  So I apologize for dropping the ball there.

Some of the email threads linked elsewhere in that bug report point out the redundancy of the grep command, and offer other ways to skin that cat, such as an awk equivalent of the sed (which may or may not be more portable).

Dave <barx>
Group Member
Mon 17 Apr 2023 08:33:25 AM UTC, original submission:  

Affects groff 1.22.4.

pdfpic.tmac has several problems.

1.  It unnecessarily uses double backslashes in a single-quoted sed expression.  Since the roff escape character has been changed (to '@'), this means that the doubled backslashes appear as-is to sed's input, and some seds don't support this non-standard usage.

2.  It uses the sequence '\n' in the replacement part of a sed 's' command.  This is non-standard and undefined on some seds.

3.  It wastefully uses grep to filter the output when sed is readily capable of performing this task itself without the expense of launching a separate process in the pipeline.

See recent discussion on the groff mailing list.

G. Branden Robinson <gbranden>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by deri (Posted a comment)
  • -email is unavailable- added by barx (Posted a comment)
  • -email is unavailable- added by gbranden (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-07-10 gbranden StatusReady for Merge Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.24.0
    2023-04-28 gbranden StatusIn Progress Ready for Merge
    2023-04-22 gbranden Summarypdfpic.tmac resorts to undocumented magic pdfpic.tmac requires non-standard sed feature
    2023-04-19 gbranden Summarypdfpic.tmac uses sed badly and non-portably pdfpic.tmac resorts to undocumented magic

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code