[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.
From: |
Deri |
Subject: |
Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message. |
Date: |
Fri, 18 Nov 2022 17:16:44 +0000 |
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.
- Re: gropdf landscape orientation support (was: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.), (continued)
- Re: gropdf landscape orientation support (was: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.), Deri, 2022/11/17
- Re: gropdf landscape orientation support (was: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.), G. Branden Robinson, 2022/11/17
- Re: gropdf landscape orientation support (was: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.), Deri, 2022/11/18
- Re: gropdf landscape orientation support (was: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.), Blake McBride, 2022/11/18
- gropdf paper format diagnostic message and rev{i, er}sion plans (was: gropdf landscape orientation support), G. Branden Robinson, 2022/11/18
- Re: gropdf paper format diagnostic message and rev{i, er}sion plans (was: gropdf landscape orientation support), Deri, 2022/11/18
- Re: gropdf paper format diagnostic message and rev{i,er}sion plans (was: gropdf landscape orientation support), G. Branden Robinson, 2022/11/24
- Re: gropdf paper format diagnostic message and rev{i, er}sion plans (was: gropdf landscape orientation support), Deri, 2022/11/25
- Re: gropdf paper format diagnostic message and rev{i,er}sion plans (was: gropdf landscape orientation support), G. Branden Robinson, 2022/11/25
- Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message., G. Branden Robinson, 2022/11/17
- Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message.,
Deri <=
- Re: [groff] 04/40: [gropdf]: Provide more info in diagnostic message., G. Branden Robinson, 2022/11/18