[PATCH] Add Fedora location of DejaVu SANS font

[PATCH] Add Fedora location of DejaVu SANS font

[Robbie Harwood]

From: fluteze <fluteze@gmail.com>

In Fedora 35, and possibly earlier, grub would fail to configure with a
complaint about DejaVu being "not found" even though it was installed.
The DejaVu sans font search path is updated to reflect the
distribution's current install path.

Signed-off-by: Erik Edwards <fluteze@gmail.com>
[rharwood@redhat.com: slight commit message edits]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 8d1c81a73..5186eab2c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1725,7 +1725,7 @@ fi
 
 if test x"$starfield_excuse" = x; then
    for ext in pcf pcf.gz bdf bdf.gz ttf ttf.gz; do
-     for dir in . /usr/src /usr/share/fonts/X11/misc /usr/share/fonts/truetype/ttf-dejavu /usr/share/fonts/dejavu /usr/share/fonts/truetype; do
+     for dir in . /usr/src /usr/share/fonts/X11/misc /usr/share/fonts/truetype/ttf-dejavu /usr/share/fonts/dejavu /usr/share/fonts/truetype /usr/share/fonts/dejavu-sans-fonts; do
         if test -f "$dir/DejaVuSans.$ext"; then
           DJVU_FONT_SOURCE="$dir/DejaVuSans.$ext"
           break 2
-- 
2.33.0

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Nicholas Vinson]

On 12/6/21 17:25, Robbie Harwood wrote:
> From: fluteze <fluteze@gmail.com>
> 
> In Fedora 35, and possibly earlier, grub would fail to configure with a
> complaint about DejaVu being "not found" even though it was installed.
> The DejaVu sans font search path is updated to reflect the
> distribution's current install path.
> 
> Signed-off-by: Erik Edwards <fluteze@gmail.com>
> [rharwood@redhat.com: slight commit message edits]
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>   configure.ac | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 8d1c81a73..5186eab2c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1725,7 +1725,7 @@ fi
>   
>   if test x"$starfield_excuse" = x; then
>      for ext in pcf pcf.gz bdf bdf.gz ttf ttf.gz; do
> -     for dir in . /usr/src /usr/share/fonts/X11/misc /usr/share/fonts/truetype/ttf-dejavu /usr/share/fonts/dejavu /usr/share/fonts/truetype; do
> +     for dir in . /usr/src /usr/share/fonts/X11/misc /usr/share/fonts/truetype/ttf-dejavu /usr/share/fonts/dejavu /usr/share/fonts/truetype /usr/share/fonts/dejavu-sans-fonts; do

Wouldn't it be better to modify configure.ac so the location could be 
passed in instead of having one-off patches for alternate locations? 
Something like --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?

>           if test -f "$dir/DejaVuSans.$ext"; then
>             DJVU_FONT_SOURCE="$dir/DejaVuSans.$ext"
>             break 2
> 

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

Nicholas Vinson <nvinson234@gmail.com> writes:

> Wouldn't it be better to modify configure.ac so the location could be 
> passed in instead of having one-off patches for alternate locations? 
> Something like --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?

That would mean that anyone using grub2 that wants the dejavu fonts will
have to figure out that the option exists and pass it to the
buildscripts.  That's a pretty bad user experience for anyone building
their own grub.

I get that maintaining a list of where everyone has put their fonts is
somewhat obnoxious, but in the absence of a standard way of doing it,
I'm not sure there's anything better.  The only other alternative I
could think of was running find in /usr/share/fonts, which struck me as
fraught.

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Nicholas Vinson]

On 12/7/21 15:04, Robbie Harwood wrote:
> Nicholas Vinson <nvinson234@gmail.com> writes:
> 
>> Wouldn't it be better to modify configure.ac so the location could be
>> passed in instead of having one-off patches for alternate locations?
>> Something like --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?
> 
> That would mean that anyone using grub2 that wants the dejavu fonts will
> have to figure out that the option exists and pass it to the
> buildscripts.  That's a pretty bad user experience for anyone building
> their own grub.

On the contrary, it doesn't have to mean that at all. It could mean that 
when a path is given, configure would use the provided path; otherwise, 
it falls back to its default list to find the font. Such an approach 
would mimic configure's current behavior for values such as bootdir and 
grubdir.

> 
> I get that maintaining a list of where everyone has put their fonts is
> somewhat obnoxious, but in the absence of a standard way of doing it,
> I'm not sure there's anything better.  The only other alternative I

See above and configure.ac's solutions to similar issues for better 
approaches.

> could think of was running find in /usr/share/fonts, which struck me as
> fraught.

Because it's the same solution you're already doing. The use of find 
won't fix anything because you'd still have to hard-code the patterns 
find uses to locate the font file.

Regards,
Nicholas Vinson

> 
> Be well,
> --Robbie
> 

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

Nicholas Vinson <nvinson234@gmail.com> writes:

> On 12/7/21 15:04, Robbie Harwood wrote:
>> Nicholas Vinson <nvinson234@gmail.com> writes:
>> 
>>> Wouldn't it be better to modify configure.ac so the location could
>>> be passed in instead of having one-off patches for alternate
>>> locations?  Something like
>>> --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?
>> 
>> That would mean that anyone using grub2 that wants the dejavu fonts
>> will have to figure out that the option exists and pass it to the
>> buildscripts.  That's a pretty bad user experience for anyone
>> building their own grub.
>
> On the contrary, it doesn't have to mean that at all. It could mean
> that when a path is given, configure would use the provided path;
> otherwise, it falls back to its default list to find the font.

But then we still have to keep the default list... and I'd still be here
as a distro maintainer wanting my distro's path in the default list.  I
don't see how this is any better.

> Such an approach would mimic configure's current behavior for values
> such as bootdir and grubdir.

But those are *install* paths, not *detection* paths.  We're not
installing the font - we're figuring out where it is on the system.

>> I get that maintaining a list of where everyone has put their fonts is
>> somewhat obnoxious, but in the absence of a standard way of doing it,
>> I'm not sure there's anything better.  The only other alternative I
>
> See above and configure.ac's solutions to similar issues for better 
> approaches.

configure.ac is 2189 lines of code.  You're going to have to be more
specific :)

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Nicholas Vinson]

On 12/8/21 12:58, Robbie Harwood wrote:
> Nicholas Vinson <nvinson234@gmail.com> writes:
> 
>> On 12/7/21 15:04, Robbie Harwood wrote:
>>> Nicholas Vinson <nvinson234@gmail.com> writes:
>>>
>>>> Wouldn't it be better to modify configure.ac so the location could
>>>> be passed in instead of having one-off patches for alternate
>>>> locations?  Something like
>>>> --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?
>>>
>>> That would mean that anyone using grub2 that wants the dejavu fonts
>>> will have to figure out that the option exists and pass it to the
>>> buildscripts.  That's a pretty bad user experience for anyone
>>> building their own grub.
>>
>> On the contrary, it doesn't have to mean that at all. It could mean
>> that when a path is given, configure would use the provided path;
>> otherwise, it falls back to its default list to find the font.
> 
> But then we still have to keep the default list... and I'd still be here
> as a distro maintainer wanting my distro's path in the default list.  I
> don't see how this is any better.

And you want it in the default path because? Most likely because you're 
patching configure or using tools like sed or awk to change configure or 
Makefile to use your path? You know that approach is brittle and you 
want something better.

Adding your distro's choice location to a pre-defined list works until 
the distro changes where it stores the font. At that point you are back 
where you started. Manually patching GRUB until a version of GRUB is 
released with your new path in the list.

The same is true with any other distro that uses a location not in the 
list. The approach you're suggesting is that submit a patch, get GRUB 
updated.

What I am asking for puts an end to that. Once the flag is in place, the 
builder can pick whatever path is desirable, pass it into the configure 
script, and have GRUB builds.  No need to craft a patch, send it to the 
mailing list, and wait for the update to make it into a GRUB release.

> 
>> Such an approach would mimic configure's current behavior for values
>> such as bootdir and grubdir.
> 
> But those are *install* paths, not *detection* paths.  We're not
> installing the font - we're figuring out where it is on the system.

In the context of building GRUB, bootdir and grubdir are *not* 
installation directories. They are simply default paths that get written 
into the built code.

Furthermore, autotools makes no distinction between such paths. The 
approach used to handle both types in configure.ac is the same.

> 
>>> I get that maintaining a list of where everyone has put their fonts is
>>> somewhat obnoxious, but in the absence of a standard way of doing it,
>>> I'm not sure there's anything better.  The only other alternative I
>>
>> See above and configure.ac's solutions to similar issues for better
>> approaches.
> 
> configure.ac is 2189 lines of code.  You're going to have to be more
> specific :)

Effort is often times its own reward. I would not want to deprive you of it.

Regards,
Nicholas Vinson

> 
> Be well,
> --Robbie
> 

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 3658 bytes --]

Nicholas Vinson <nvinson234@gmail.com> writes:

> On 12/8/21 12:58, Robbie Harwood wrote:
>> Nicholas Vinson <nvinson234@gmail.com> writes:
>>> On 12/7/21 15:04, Robbie Harwood wrote:
>>>> Nicholas Vinson <nvinson234@gmail.com> writes:
>>>>
>>>>> Wouldn't it be better to modify configure.ac so the location could
>>>>> be passed in instead of having one-off patches for alternate
>>>>> locations?  Something like
>>>>> --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?
>>>>
>>>> That would mean that anyone using grub2 that wants the dejavu fonts
>>>> will have to figure out that the option exists and pass it to the
>>>> buildscripts.  That's a pretty bad user experience for anyone
>>>> building their own grub.
>>>
>>> On the contrary, it doesn't have to mean that at all. It could mean
>>> that when a path is given, configure would use the provided path;
>>> otherwise, it falls back to its default list to find the font.
>> 
>> But then we still have to keep the default list... and I'd still be here
>> as a distro maintainer wanting my distro's path in the default list.  I
>> don't see how this is any better.
>
> And you want it in the default path because? Most likely because you're 
> patching configure or using tools like sed or awk to change configure or 
> Makefile to use your path? You know that approach is brittle and you 
> want something better.

No, that's not it.  Don't put words in other people's mouths.

We're sitting on more than 220 downstream patches that I want
upstreamed.  The relative brittleness of this particular change is
*nothing* in comparison, believe me :)

The sheer number here means I'm starting with the easy stuff.  The
one-liners, simple conceptual things, *stuff that should be
non-contentious*.

> Adding your distro's choice location to a pre-defined list works until 
> the distro changes where it stores the font. At that point you are back 
> where you started. Manually patching GRUB until a version of GRUB is 
> released with your new path in the list.
>
> The same is true with any other distro that uses a location not in the 
> list. The approach you're suggesting is that submit a patch, get GRUB 
> updated.
>
> What I am asking for puts an end to that. Once the flag is in place, the 
> builder can pick whatever path is desirable, pass it into the configure 
> script, and have GRUB builds.  No need to craft a patch, send it to the 
> mailing list, and wait for the update to make it into a GRUB release.

But my point is this hurts everyone who's *not* the distro maintainer
but building their own grub.  That's the user experience issue I was
talking about.  Everyone developing on Fedora now has to remember
another flag or it won't work the same.

>>> Such an approach would mimic configure's current behavior for values
>>> such as bootdir and grubdir.
>> 
>> But those are *install* paths, not *detection* paths.  We're not
>> installing the font - we're figuring out where it is on the system.
>
> In the context of building GRUB, bootdir and grubdir are *not* 
> installation directories. They are simply default paths that get written 
> into the built code.

Right, that's what I mean by "install" path.  It's a place where you put
stuff, not a place where you find stuff.

> Effort is often times its own reward. I would not want to deprive you
> of it.

Wow, okay.

You know this comes across as rather hostile, right?  Is this really the
approach you want to take when someone *shows up with code* and asks you
to clarify your feedback?  Tell them to go away and RTFM?

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH] Add Fedora location of DejaVu SANS font

[Nicholas Vinson]

On 12/8/21 18:42, Robbie Harwood wrote:
> Nicholas Vinson <nvinson234@gmail.com> writes:
> 
>> On 12/8/21 12:58, Robbie Harwood wrote:
>>> Nicholas Vinson <nvinson234@gmail.com> writes:
>>>> On 12/7/21 15:04, Robbie Harwood wrote:
>>>>> Nicholas Vinson <nvinson234@gmail.com> writes:
>>>>>
>>>>>> Wouldn't it be better to modify configure.ac so the location could
>>>>>> be passed in instead of having one-off patches for alternate
>>>>>> locations?  Something like
>>>>>> --with-dejavu-font=/usr/share/fonts/dejavu-sans-fonts?
>>>>>
>>>>> That would mean that anyone using grub2 that wants the dejavu fonts
>>>>> will have to figure out that the option exists and pass it to the
>>>>> buildscripts.  That's a pretty bad user experience for anyone
>>>>> building their own grub.
>>>>
>>>> On the contrary, it doesn't have to mean that at all. It could mean
>>>> that when a path is given, configure would use the provided path;
>>>> otherwise, it falls back to its default list to find the font.
>>>
>>> But then we still have to keep the default list... and I'd still be here
>>> as a distro maintainer wanting my distro's path in the default list.  I
>>> don't see how this is any better.
>>
>> And you want it in the default path because? Most likely because you're
>> patching configure or using tools like sed or awk to change configure or
>> Makefile to use your path? You know that approach is brittle and you
>> want something better.
> 
> No, that's not it.  Don't put words in other people's mouths.
> 
> We're sitting on more than 220 downstream patches that I want
> upstreamed.  The relative brittleness of this particular change is
> *nothing* in comparison, believe me :)
> 
> The sheer number here means I'm starting with the easy stuff.  The
> one-liners, simple conceptual things, *stuff that should be
> non-contentious*.
> 
>> Adding your distro's choice location to a pre-defined list works until
>> the distro changes where it stores the font. At that point you are back
>> where you started. Manually patching GRUB until a version of GRUB is
>> released with your new path in the list.
>>
>> The same is true with any other distro that uses a location not in the
>> list. The approach you're suggesting is that submit a patch, get GRUB
>> updated.
>>
>> What I am asking for puts an end to that. Once the flag is in place, the
>> builder can pick whatever path is desirable, pass it into the configure
>> script, and have GRUB builds.  No need to craft a patch, send it to the
>> mailing list, and wait for the update to make it into a GRUB release.
> 
> But my point is this hurts everyone who's *not* the distro maintainer
> but building their own grub.  That's the user experience issue I was
> talking about.  Everyone developing on Fedora now has to remember
> another flag or it won't work the same.

If someone wants to use a path that is not in the list how would that 
person do it?

> 
>>>> Such an approach would mimic configure's current behavior for values
>>>> such as bootdir and grubdir.
>>>
>>> But those are *install* paths, not *detection* paths.  We're not
>>> installing the font - we're figuring out where it is on the system.
>>
>> In the context of building GRUB, bootdir and grubdir are *not*
>> installation directories. They are simply default paths that get written
>> into the built code.
> 
> Right, that's what I mean by "install" path.  It's a place where you put
> stuff, not a place where you find stuff.
> 
>> Effort is often times its own reward. I would not want to deprive you
>> of it.
> 
> Wow, okay.
> 
> You know this comes across as rather hostile, right?  Is this really the > approach you want to take when someone *shows up with code* and asks you
> to clarify your feedback?  Tell them to go away and RTFM?

You should consider your own responses first before criticizing the 
responses of others.

Responding with the equivalent of 'tldr; you'll have to do better' is 
not how to properly ask someone to clarify a statement. Such a response, 
however, is extremely rude as it suggests your time and effort is more 
valuable than others. Suggesting that you believe you should get some 
sort of special treatment simply because you "showed up with code" does 
as well.

That said if you have a specific question about something I've said or 
wish for me to elaborate on something, ask appropriately and I (or 
someone else on this list) will answer.

and just to be clear, I don't speak on behalf of the GRUB maintainers or 
in any way represent them.

Regards,
Nicholas Vinson

> 
> Be well,
> --Robbie
>