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

[Deri]

On Friday, 18 November 2022 01:31:22 GMT G. Branden Robinson wrote:
> 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.

Hi Branden,

99% of time this message will be triggered when entries in the system download 
file go stale because a new version of ghostscript is installed with different 
paths to its fonts. So I don't see the benefit of making the message longer to 
include stale information. There is nothing special with this path it points 
to a file which no longer exists, the real problem is line n of the particular 
download file which will need editing to whatever is the new path, and of 
course, when they locate the offending line in download they will see the 
pathname. I will concede that if the reason the file can't be opened is 
because of access rights, then there is a case for including the offending 
path. But, by far the most likely is the file has gone missing. Perhaps we 
need two versions, with path for !-r and without for !-e.

By far the easiest for the user if they receive either message would be to run 
BuildFoundries, you have not given me your thinking on that yet.

> > 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?
>
It has told me which file it could not open, the one which contains the Times-
Roman font.

> 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).

Thank you for this offer but I am quite happy to continue supporting gropdf, 
the sooner you revert this change means the sooner I can do the changes I've 
discussed.

> 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)

Perfect. :-)

> 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=ac1f
> be277660750e8caaf3f2b38a1149cf79aabc
> 
> 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=ac
> 1fbe277660750e8caaf3f2b38a1149cf79aabc#n163
> 
> Nevertheless users seem to add them by hand.[2]

This has nothing to do with what I said. Symbolsl and zapfdr do already get 
"built" from font/devps to build/font/devps, I'm asking for freeeuro to be 
treated the same way (end up in build/font/devps). There is no need to change 
anything in devpdf/, so the above seems to be a misunderstanding.

> > (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.)

This is very good, thank you for doing this for me.

> > > 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.

The big difference was that you proposed to exit immediately, but I propose to 
exit at the end of the run, which allows writing the pdf to complete even if a 
font is missing, since the pdf viewer may have access to the correct font 
itself

> > 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.

As soon as you let me know the revert is done I will be as quick as I can.

Cheers 

Deri

> 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.