Manpages rootprefix #39
Conversation
Replace some /usr/lib occurences in man/ with &rootprefix;/lib.
Strip trailing slashes from options such as --with-rootprefix, so that building
with rootprefix="/" results in paths like "/lib" instead of "//lib".
Also handle paths such as "/usr/" gracefully.
Use m4/ax_normalize_path.m4 from the autoconf-archive project, which is now
included in our tree as per usual practices in using autoconf-archive macros.
Tested with the following configure options:
./configure \
--with-rootprefix=/ \
--with-rootlibdir=/lib64/ \
--prefix=/usr/ \
--libdir=/lib/ \
--with-bashcompletiondir=/bash-completion/completions/
(The "prefix" and "libdir" are already automatically normalized by Autoconf,
this command is testing the others.)
Compared the config.log and resulting trees (in particular man pages) to
confirm double slashes were not present in the latter.
Also tested that a configuration using default options is not affected and that
`make distcheck` still works as expected.
|
He seems to be @mbiebl on GitHub ... |
|
I noticed, that this macro replaces empty paths with '.'. I think this is problematic, because up until now, we were advocating the use of --with-rootprefix= for split-usr setups (see e.g. autogen.sh). With the patch applied, this would result in rootprefix = '.' and roolibdir './lib', etc. So this has the potential of breaking existing setups. E.g. in Debian/Ubuntu we'd have to replace --with-rootprefix= with --with-rootprefix=/ in our build scripts when updating to v221. |
|
@mbiebl Yes you are correct. Now looking at my notes in the commit it seems that I tested it with I could swear that I did test it with Not really sure how to proceed here... Since in a way "empty" does not equal "root" in most cases... Should we update the advice to use Or should we handle "empty" as special? In particular, should we check it for Suggestions? |
|
@filbranden I think we should change the macro to not turn an empty argument into '.' at least for commands which set a prefix to not break existing usages of --with-rootprefix= Looking at what autoconfi does: I think we should do at least a) in our macro, b) would be an optional nicety. Erroring out in that case makes more sense to me then setting it to '.'. That said, I doubt any of our --with-foodir configure switches will ever be set to an empty string. So it's probably not relevant in practice. @martinpitt what do you have think? |
|
IMHO defaulting to a relative path like "." for an empty argument is almost always unintended and a trap. I really don't mind adjusting our debian/rules to set the argument to "/", but we are by far not the only ones building systemd :-) If it's hard/impractical to handle an empty value appropriately, then erroring out in configure with "no value set" or so would be a fine safeguard to avoid this trap. Thanks @filbranden for working on this! |
|
I fail to see any good reason for replacing "" with ".", am I missing anything? I think the best solution here is to remove that special case handling from AX_NORMALIZE_PATH(). After all, all we wanted to achieve is removing trailing slashes. |
|
So whatever we do, I think we should not modify the definition of I think the rationale to transforming (empty) to "." is that that's what it means for example when you have an empty entry in variables like Though I see the point that using "." doesn't really make a lot of sense for paths passed to So, my proposal is:
Does that sound good? I'll start working on steps 1 and 2 right now and send you another PR. Cheers, |
backporting journal changes from master to v229
4 participants
These are the two last pending items for the XML entities with the paths when generating the man pages.
@zonque @martinpitt
I wanted to also cc Biebl but couldn't find him on GitHub, will send him an e-mail instead...
Cheers,
Filipe