install_man: add support for building man pages via internal custom_target

[eli-schwartz]

This is completely terrible, because really you should use a custom_target and then feed it to install_man. However, that has been decided to never happen. So, reluctantly, if install_man won't accept a custom_target, then let install_man copy the entire interface and internally make one.

This solves a massive usability problem where custom targets with install_dir are strictly inferior to install_man() which automatically handles sectioned or localized subdirectories of mandir, and results in nontrivial code to painfully work around this. One alternative approach was to hack in @MANDIR@ support to install_dir itself (which is itself an inconsistent interface), but after adding a locale kwarg to install_man it is no longer possible to infer all information from the filename alone, so that's a total wash.

Fixes: 1550 2244 4778 4981 5935 8291

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 66.88%. Comparing base (3feaea6) to head (ffcb039). Report is 3266 commits behind head on master.

Files	Patch %	Lines
mesonbuild/interpreter/interpreter.py	86.66%	1 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9342      +/-   ##
==========================================
+ Coverage   66.87%   66.88%   +0.01%     
==========================================
  Files         388      388              
  Lines       85495    85527      +32     
  Branches    17559    17569      +10     
==========================================
+ Hits        57173    57205      +32     
+ Misses      23579    23577       -2     
- Partials     4743     4745       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[jpakkane]

An alternative would be to have a custom_man_target that takes the same arguments as custom_target (except install_dir), validates the output name, computes the mandir and then calls custom_target internally.

[eli-schwartz]

That is almost the same thing as this PR, except for complaining when people add useless arguments but don't specify a command.

On the other hand it adds an entirely new function which is rather annoying on its own. Would you prefer this?

[jpakkane]

Well that's the thing: I don't really know. OTOH having generation functionality in install_man is nonstandard because none of the other ones have it. On the other hand additional functions are not nice either, for exactly the reasons you mention.

Do other people have opinions? Or better suggestions for exposing this functionality?

[jpakkane]

Well how about this then:

custom_target(...,
    install_true,
    is_manpage: true) # Defining both this and an install_dir would be an error

[anarazel]

custom_target(...,
    install_true,
    is_manpage: true) # Defining both this and an install_dir would be an error

If you went with something like this it seems like it might be better to have a target_type: 'manpage', so that it can later be extended to other types of things.

[jpakkane]

That is an option I considered, but the only real use case we have thus far is for man pages. Still, I guess something like executable or static_library would make sense. But still, a question is whether the kwarg should be target_type or something like 'install_as`.

[dcbaker]

I prefer the is_man or my previous install_dir : '@MANDIR@' approach, personally. either that or we could do something like add a unix module with a man_target method. 🤷🏻

[eli-schwartz]

@dcbaker this despite the fact that my commit message references your own decision that @MANDIR@ cannot work?

@jpakkane an is_manpage kwarg would share the exact same problem as @MANDIR@ since it would not come with an accompanying manpage_locale kwarg.

I mean, I guess we could add that too, if you'd like... It feels like we'd be adding a lot of manpage specific kwargs to a highly generic function like custom_target when it would make more sense to add the generic kwargs to a highly specific function like install_man.

[eli-schwartz]

Does anyone have further thoughts on this? I don't want to merge a solution that doesn't actually have feature compatibility for the existing uses of install_man() without built outputs.

[jpakkane]

Thinking about this I'm leaning towards install_dir : '@MANDIR@' , because it simplifies code from:

install_dir: get_option('libdir') / 'foobar'

to

install_dir: '@LIBDIR@/foobar'

Technically this is an API change, but I've never heard of anyone installing things to a directory with a @ in it. Has anyone?

[eli-schwartz]

Once again, I do not understand how this solves the feature compatibility with install_man(..., locale: 'fr').

[jpakkane]

In the only test case we have the filename (foo.fr.1) does have all the information needed. It even removes the .fr part from the middle. Could we say that if you want the automatic locale based thing to work, you must name your generated files as bob.langname.number?

[jpakkane]

Or possibly @FR_MANDIR@?

[Tachi107]

Could we say that if you want the automatic locale based thing to work, you must name your generated files as bob.langname.number?

This is a sane requirement IMO. I can't think of a reason why you would want to name localized manpages differently anyway