bugGNU roff - Bugs: bug #62950, [gropdf] revise 'download'...

 
 

bug #62950: [gropdf] revise 'download' resolution process

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Wed 24 Aug 2022 10:12:36 AM UTC
   
 
Category:  Driver gropdf Severity:  1 - Wish
Item Group:  Build/Installation Status:  Fixed
Privacy:  Public Assigned to:  deri
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 17 Dec 2023 09:37:08 PM UTC, comment #11: 

Commit c51ccf799 fixed the issue raised in comment #7.

Dave <barx>
Group Member
Mon 28 Nov 2022 04:46:54 AM UTC, comment #10: 


commit 7e5d433ba5ddc2389986a5c02f91eb57fc1de47d
Author: Deri James <deri@chuzzlewit.myzen.co.uk>
Date:   Tue Nov 8 21:03:49 2022 +0000

    [gropdf]: Cater for invalid entries in download file.

    * src/devices/gropdf/gropdf.pl: Test if path in the download
    file points to a readable file. Also change order so that the
    first valid entry is used as the font to embed. This replaces
    previous order where last found entry is used.

    This change is discussed in <https://savannah.gnu.org/bugs/?62950>.


G. Branden Robinson <gbranden>
Group administrator
Tue 08 Nov 2022 09:15:05 PM UTC, comment #9: 

I have pushed the changes. Will close as the freeeuro thing is not something I can fix in gropdf.

Deri James <deri>
Group Member
Tue 08 Nov 2022 07:54:37 PM UTC, comment #8: 


comment #7:

> I have finished testing and found an issue, not really related to this issue but found during testing. Since the groff_man_pages.pdf uses the freeeuro font the download file in build/font/devpdf correctly has the entry "../devps/freeeuro.pfa" which is correct once installed in the system directory, but while building it is in font rather than build/font which is not searched, so an error is given.
>
> Fixes can either be:-
>
> "build" freeeuro.pfa/afm by copying to the build directory, the same as is currently done for zapfdr and symbolsl, or
>
> Change the GROFF_FONT_PATH used during the build to include the src_directory/font as well.
>
> I will soon release my changes, but I thought I should document this problem.


Thanks!  If you want to reassign the ticket to me after you push, or simply resolve it as fixed, I'll deal with the issue you raise.  Thanks for the heads up.

G. Branden Robinson <gbranden>
Group administrator
Tue 08 Nov 2022 06:29:12 PM UTC, comment #7: 

I have finished testing and found an issue, not really related to this issue but found during testing. Since the groff_man_pages.pdf uses the freeeuro font the download file in build/font/devpdf correctly has the entry "../devps/freeeuro.pfa" which is correct once installed in the system directory, but while building it is in font rather than build/font which is not searched, so an error is given.

Fixes can either be:-

"build" freeeuro.pfa/afm by copying to the build directory, the same as is currently done for zapfdr and symbolsl, or

Change the GROFF_FONT_PATH used during the build to include the src_directory/font as well.

I will soon release my changes, but I thought I should document this problem.

Deri James <deri>
Group Member
Tue 08 Nov 2022 04:23:54 PM UTC, comment #6: 

Making this is a 1.23 release goal since people keep tripping over it.  gropdf didn't get any worse since 1.22.4 (quite the contrary).

What I think has happened is that my introduction of the groff-man-pages.pdf artifact (with all fonts embedded) has exposed the sensitivity of the previous approach to file system layout issues.

Between:

1. Increased determinism in URW font directory location;
2. Improved documentation of the gropdf Foundry and download files;
3. Improved diagnostic messages in gropdf itself; and
4. What Deri has in the works for to resolve this ticket,

I think we'll have a substantially improved gropdf font handling story for groff 1.23.

G. Branden Robinson <gbranden>
Group administrator
Fri 04 Nov 2022 06:25:34 PM UTC, comment #5: 

I am still testing, but the coding is done. I am still lazy, but we can live in hope!

As regards grops the simplest implementation would be to change my two hashes into two arrays of fontname fontpath structures (as it probably does now), for valid and invalid entries. Then just a loop over the array to find matching font name. Most of the code is probably already there, would need a pointer to operate on either the valid or invalid array.

I do like Ralph's algorithm and it is more efficient, but the saving would be miniscule compared to over all run time, and it would require a bit more changes to gropdf and I refer to my second sentence above.

Deri James <deri>
Group Member
Fri 04 Nov 2022 04:35:22 PM UTC, comment #4: 

Hmm, since grops should one day follow gropdf's lead on this (bug #62938), a consideration besides "how hard is this to do in perl?" might be "how hard is it to replicate this algorithm in C++?"  Or it might not: from the user's viewpoint, the behavior of Deri's plan and Ralph's should look the same, so the implementation details could be left up to the coder.

Dave <barx>
Group Member
Fri 04 Nov 2022 02:28:58 PM UTC, comment #3: 

That sounds good.  Is this something you've already implemented but just not pushed yet?

One possible advantage to Ralph's idea is that no validity checking need be done until a font is actually called upon in a document, whereas all must be checked up front with the system sketched in comment #2.  Still, I imagine the performance difference, even on a system that has several hundred entries in its download files, would be negligible.

A nice thing about perl is it makes "complicated" data structures easy to implement.  I read Ralph's idea as using a hash where each value is an array rather than a scalar--that is, more a hash of arrays than an array of hashes.  (On the other hand, I've written perl code that used the latter structure but not the former, so I perhaps shouldn't blithely declare they're both equally simple to implement.)

Dave <barx>
Group Member
Thu 03 Nov 2022 12:50:57 PM UTC, comment #2: 

I think this is a bit more than a wish. The current gropdf behaviour (choose last) causes build to fail if site or system download file contains "stale" entries for the groff system fonts. Staleness can occur if ghostscript is updated without making amendments to the download files (or re-running BuildFoundries).

If the priority order is simply changed to "choose first" you run into the problem that if a user downloads groff as a package, and the site-font contains stale entries for the standard groff fonts, jobs which previously completed successfully would now fail because the first matching entry is the stale entry in site-font. Whereas with the current "choose last" the (possibly) valid system entry would be used, i.e. a code change which would result in groff no longer working on a previously functioning system. Ralph's suggestion (an array of hashes, one for each download file) would certainly work, but I think there is a slightly easier method.

The goal is to find the first valid entry, i.e. the font file entry points to a file which exists and is readable, plus inform user if they have tried to use a font which points to a file which does not exist. Since this is a binary issue, whether the file pointed to is readable or not, I create two hashes from the various download files, valid entries and invalid entries. When gropdf is required to embed a font, and it is not found in the valid entries, one of two messages are output. If it is present in the invalid hash a message identifying the font and particular download file is issued, otherwise the existing message concerning possible corrupt download file.

I have documented the method I have adopted for gropdf in case someone wishes to do something similar to grops.

Deri James <deri>
Group Member
Mon 19 Sep 2022 01:34:23 AM UTC, comment #1: 

I should note here that elsewhere in that thread (http://lists.gnu.org/r/groff/2022-08/msg00194.html), Ralph proposed a more sophisticated method of handling duplicate entries, though that's an adjacent issue to which of those dupes should take precedence, and Deri's suggestion to reverse the present order makes sense regardless of whether the method is changed.

However, Ralph's method would seem to be more important if that order is in fact reversed: a user font directory is more likely to have an invalid "download" entry than the system one is, and keeping the current method--if I'm understanding it correctly--while reversing the order would cause the font to fail to load altogether in this case, whereas Ralph's method potentially provides other chances to resolve it.

But that's a lot of reengineering to cover a case Deri characterizes as "unexpected."

Dave <barx>
Group Member
Wed 24 Aug 2022 10:12:36 AM UTC, original submission:  

Deri wrote in https://lists.gnu.org/archive/html/groff/2022-08/msg00202.html :


> 1.  As noted above, the "last one wins"; this is even more different
>     from other groff path-searching behavior than continuing to search
>     if an initially matching resource is found to be invalid.

Having multiple entries for the same font would be unexpected. There is no way
of knowing which the user determines is the correct one, and one way round is
a little less typing. :-)

However, at the moment it favours the system directory which is checked last,
and we should favour font directories provided by the user. So I am happy to
change to first wins.

> 2.  There _is_ no validity check; the `download` hash is populated
>     without checking to see if the font file exists or is readable.

This is the wrong place for a validity check of this type. It is quite valid
to have download entries for files that don't exist. The error should come if
you try to embed a font which is unreadable, which it does.

Imagine you do some house keeping and remove some fonts and their .pfas but
don't edit the download file, possibly because you are unaware of it, because
install-fonts.sh did it all for you, why should gropdf nag every time it is
run for something which does not affect its output?


This all makes sense to me.

There are further comments but they relate to grops so I'll file a separate ticket for those.

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

Digest:
   bug dependencies.

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2022-11-08 deri Item GroupFeature change Build/Installation
        Open/ClosedOpen Closed
    2022-11-08 deri StatusIn Progress Fixed
    2022-11-08 gbranden StatusNone In Progress
        Assigned toNone deri
        Planned ReleaseNone 1.23.0
    2022-08-24 gbranden Dependencies- bugs #62938 is dependent

    Back to the top

    Powered by Savane 3.13-4e86.
    Corresponding source code