Re: Failure message when inconsistent X11_TYPE

[Frederic Fauberteau <triaxx%NetBSD.org@localhost>]

Greg Troxel wrote:
> Frederic Fauberteau <triaxx%NetBSD.org@localhost> writes:
>> Greg Troxel wrote:
>>> Joerg Sonnenberger <joerg%bec.de@localhost> writes:
>>>> On Tue, Jul 13, 2021 at 06:21:22PM -0400, Greg Troxel wrote:
>>>>> Frederic Fauberteau <triaxx%NetBSD.org@localhost> writes:
>>>>>
>>>>>>   .if empty(MACHINE_PLATFORM:MNetBSD-[0-5].*)
>>>>>>   X11_TYPE?=             native
>>>>>>  +.  if !empty(X11_TYPE:Mnative) && !exists(/etc/mtree/set.xbase)
>>>>>>  +PKG_FAIL_REASON+=      "Native X11 distribution cannot be used if xbase set has not been installed. You should set X11_TYPE=modular or install xbase set."
>>>>>>  +.  endif
>>>>>>   .endif
>>>>>
>>>>> I'm not really a fan, but I can't really argue against this.
>>>>
>>>> I'm not really in favor of this either. Especially since the feature
>>>> test is looking for something completely unrelated to what anything
>>>> cares about.
>>>
>>> Good point; I realized that and didn't say it becuase checking for 100
>>> .h and 10 .so is not something we want to do in platform.mk
>>>
>>>
>>> Maybe, x11-links should at build time actually check for all the stuff
>>> it wants to symlink to (if it doesn't) and fail, and I think everything X
>>> depends on x11-links anyway.
>>
>> I totally agree that testing of tens or hundreds of files is not
>> reasonable. The problem I was trying to address was the tools that
>> depend on xbase set in their native version. In any case, to link a
>> tool from work/.tools/bin to ${X11BASE}/bin will result in a
>> configure/build/installation error if ${X11BASE} doesn't exist. I
>> would like to be directly notified that it is a misconfiguration
>> instead of greping for minutes... The test I would like to put in
>> mk/tools/replace.mk would be:
> 
> So this is only about tools that try to use things in base, and is not
> about building packaging that use X?
> 
> Can you list the tools that do this?
Sure. There are iceauth, mkfontdir, mkfontscale, bdftopcf, ucs2any, bdftruncate, xauth, xinit, xmessage and makedepend.
>> --- pkgsrc/mk/tools/replace.mk  6 Oct 2020 17:36:50 -0000       1.291
>> +++ pkgsrc/mk/tools/replace.mk  15 Jul 2021 09:44:15 -0000
>> @@ -1355,6 +1355,16 @@
>>  .  endif
>>  .endfor
>>  
>> +###
>> +### For each tool that depends on X11 for its native version, make
>> +### sure it is really installed (i.e. xbase has been installed).
>> +###
>> +.for _t_ in ${_USE_TOOLS}
>> +.  if !empty(TOOLS_PATH.${_t_}:M${X11BASE}*) && !exists(${TOOLS_PATH.${_t_}})
>> +PKG_FAIL_REASON+=      "X11_TYPE=native but xbase set is not installed.  Either set X11_TYPE to modular or install xbase."
>> +.  endif
>> +.endfor
>> +
>>  ######################################################################
>>  
>>  # Create a TOOLS_ENV variable that holds a shell environment that
>>
>> If you disagree with this approach, I could close the PR with message "Issue not relevant for pkgsrc".
> 
> I am not sure what I really think yet, but:
> 
>   It's somewhat or more, perhaps entirely, reasonable for pkgsrc to
>   assume that an xbase set file implies that the sets are present.
> 
>   If one sets X11_TYPE to modular, do the tools that you listed depend
>   on the pkgsrc versions in a clean manner?  (Surely this must be true.)
I think this is one of the aims of mk/tools/replace.mk. For instance, we can find code like:

.  if defined(X11_TYPE) && !empty(X11_TYPE:Mmodular)
TOOLS_DEPENDS.makedepend?=  makedepend-[0-9]*:../../devel/makedepend
TOOLS_PATH.makedepend=  ${LOCALBASE}/bin/makedepend
.  else # !empty(X11_TYPE:Mnative)
TOOLS_PATH.makedepend=  ${X11BASE}/bin/makedepend
.  endif

>   Files that are evaluated for every package should strive to be lean
> 
> 
> So if the only problem is that some tools are taken from native X if
> X11_TYPE=native (and work fine with modular), then a single if for the
> set config file in tools seems preferable to looping over all tools and
> checking, as a tradeoff of avoiding runtime at the expense of assuming
> consistency.
> 
> Which I think brings me back to your original suggestion, which is to
> just check for the set file in platform.mk
> 
> I would really like to hear what other people think; this is not an easy
> issue to deal with.