Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.

[G. Branden Robinson]

Hi Deri,

At 2022-11-17T01:22:48+0000, Deri wrote:
> On Wednesday, 16 November 2022 22:57:45 GMT G. Branden Robinson wrote:
> > I therefore propose something like:
> > 
> > gropdf: warning: cannot embed font for 'TR':
> > "/usr/share/groff/site-font/devpdf/download" has invalid entry for
> > Times-Roman; could not open ".../timesrom.pfa" (Permission denied)
> > 
> > (lines broken only for email)
> 
> Very close, but I still fail to see the benefit of naming the file,

Because a failing operation on the file name is how we decide the entry
is invalid.

            if (!-r $pth)
            {
                if (exists($missing{"$foundry $name"}))
                {
                    $missing{"$foundry $name"}.=":$pth";
                }
                else
                {
                    $missing{"$foundry $name"}="$pth";
                }
                next;
            }

In the shell, when a "test -r" fails and I don't have a way to cope with
it, I write a diagnostic like "$pth does not exist or is not readable".
We can do a little better in Perl because we have the equivalent of
`strerror(errno)` in `$!` instead of just an exit status of 1.

> the problem is the entry, the most likely scenario is a new
> ghostscript install which has changed all the paths/filenames so the
> entry is stale. 
> 
> How about:-
> 
> gropdf: warning: cannot embed font for 'TR':
> Line nn of "/usr/share/groff/site-font/devpdf/download" has invalid
> entry for Times-Roman; could not open the file (Permission denied)

Could not open _what_ file?  You don't see the utility in saying?

If we're going to treat the "download" file as an input file (which is a
reasonable enough claim) and go so far as to add line counting logic
while reading it (which I am willing to do), then we might as well
format this message the way the GNU Coding Standards say--groff is
growing increasingly conformant with them anyway (in this respect).

gropdf:/usr/share/groff/site-font/devpdf/download:16: warning: cannot
embed font for 'TR': invalid entry for Times-Roman; could not open the
file (Permission denied)

And as I've emphasized above, I'd strong preferly to see:

gropdf:/usr/share/groff/site-font/devpdf/download:16: warning: cannot
embed font for 'TR': invalid entry for Times-Roman; unable to open
"/mnt/most/unreliable/nfs/host/type1/timesr.pfa" (Permission denied)

Can you live with that?

> So we drop multiple paths, yes?
> 
> If the situation arises that the same postscript font is incorrect in
> more than one download file (rare) they will receive a message for the
> first one, then after correction they will receive a message for the
> second one.

I can live with that.  It will keep gropdf simpler.  There may be some
user frustration in the scenario you envision as most common, when
Ghostscript gets upgraded and changes the names of all its font files
for fun.  Still, a user won't have to be overladen with insight to
realize that they might just want to `ls` the directory in question
update all the entries prospectively instead of repeatedly running
gropdf to be fed the problem children one by one.

> Given that what I said was "Will close as the freeeuro thing is not
> something I can fix in gropdf." I was surprised to see changes in the
> gropdf directory, particularly when my preferred fix was:-
> 
> "build" freeeuro.pfa/afm by copying to the build directory, the same
> as is currently done for zapfdr and symbolsl

...except the groff 'pdf' device doesn't need the latter two fonts,
right?  I thought I asked about this not too long ago...

(mailing list search ensures)

Ah, I did.  And apparently that's true for ZDR but _not_ SS.[1]  But
groff has never shipped gropdf with these font names in its DESC file.

https://git.savannah.gnu.org/cgit/groff.git/tree/font/devpdf/DESC.in?id=ac1fbe277660750e8caaf3f2b38a1149cf79aabc

This file _does_ get transformed, but just to set the default paper
format, not to add these font names (and the latter wouldn't need to be
determined at build time anyway).

https://git.savannah.gnu.org/cgit/groff.git/tree/font/devpdf/devpdf.am?id=ac1fbe277660750e8caaf3f2b38a1149cf79aabc#n163

Nevertheless users seem to add them by hand.[2]

> (I admit my other suggestion was complete baloney, altering
> GROFF_FONT_PATH affects the search for download files, not the
> directories holding pfas.!!).

Unfortunately I tried your baloney suggestion first and became fixated
on my goal, deciding to resolve the problem come hell or high water.
Hence my application of the Sledge-O-Matic to the walnut.

> That would be very kind, thank you. Please note as well as the rotated
> landscape hotspot fix, it includes appending L or l to papersizes,
> i.e. -p LetterL would specify a landscape letter paper size.

Okay.  I split them into two commits since, per my testing, the new
`$rot` feature resolved the hotspot problem all by itself.

commit 3e1c246e22746ce59106c70b2912e9f3472116cc
Author: Deri James <deri@chuzzlewit.myzen.co.uk>
Date:   Wed Nov 16 22:02:49 2022 +0000

    [gropdf]: Rotate MediaBox if '-p' arg L-suffixed.

    * src/devices/gropdf/gropdf.pl: If the argument to the paper format
      option '-p' matches a recognized format but includes a trailing 'L' or
      'l' ("legalL" or "a4l", for example), rotate the document's MediaBox.

commit 0ce521b5fce66f8065f1b7d0dc721fd44d8ae40b
Author: Deri James <deri@chuzzlewit.myzen.co.uk>
Date:   Wed Nov 16 22:02:49 2022 +0000

    [gropdf]: Fix Savannah #63380.

    * src/devices/gropdf/gropdf.pl: Fix incorrect hotspot placement if page
      is in landscape orientation.

      (FixRect): Perform coordinate transform if page is rotated.

      (Rotate): New function performs relevant trigonometry.

    Fixes <https://savannah.gnu.org/bugs/?63380>.  Thanks to Blake McBride
    for the report.

Please correct me if I misstated anything in the foregoing.  (These are
from my working copy so the commit IDs are not guaranteed to be the same
when I push.)

> > As noted in private, I feel gated on the reversions because I
> > haven't yet heard from you on whether you approve of any of my
> > proposed mechanisms for causing the build to fail if font embedding
> > fails when generating "groff-man-pages.pdf" (and thereby "keeping
> > the tree green" in this respect).  To me, running Poppler's
> > pdffonts(1) to scan the generated file is extremely undesirable
> > because doing so doubles groff's build time on my system (and surely
> > others').  I'm open to suggestions.
> 
> What about a gropdf exit code based on whether any warnings or error
> messages are emitted during the run,

This is pretty close to what I wanted to do in the first place; since
exiting with a non-zero exit status is, for make(1)'s purposes, strictly
equivalent to failing.

> could you then test this in the make file, setting your green or red
> flag.

Nothing so fancy.  The build would simply fail, never getting as far as
the tests.  Better, even, for my purposes.[3]

It is easy enough to not die _immediately_ upon encountering a missing
font to be embedded.  A flag can be set and checked before eventual
exit, and gropdf will have done a best-effort job to construct the
document anyway.  I guess I'll have to create groff-man-pages.pdf as a
temporary file first in this case, because .DELETE_ON_ERROR is a GNU
make extension.  (Ingo hasn't posted to the list in a while, but he's
not forgotten.)

Are you okay with making this the new default behavior or would you
rather it be gated behind a new option, say '-E'?  (Feel free to propose
an alternative option letter if the latter.)  I have no preference.

Let me know.  I am anxious to get this implemented so we can get master
back into a condition where you're happy with it.

Regards,
Branden

[1] https://lists.gnu.org/archive/html/groff/2022-07/msg00145.html
[2] https://lists.gnu.org/archive/html/bug-groff/2022-07/msg00105.html

[3] I believe in failing fast, failing hard, and leaving a detailed
    autopsy report on top of the carcass.

signature.asc
Description: PGP signature