The -v option has been ignored for a very long time (did only provide information relevant for operations on a.out libraries).
The local variable "verbose" is only set and never read and not made available to any function in ldconfig, it can therefore be removed.
To support the unlikely case that the variable is passed in some script, it can be accepted and ignored (as it already effectively was).
The man-page is updated to no longer mention -v as a valid option.
There are 3 ignored options: "-elf", "-s", and "-v" that are ignored but accepted for historical reason: list these variables at the end of the HISTORY section.
(While here fix the cross-reference to the ld-elf.so.1 man-page by adding the missing section "1".)
Details
Apply patch and verify that ldconfig operates as before.
Verify changes to the man-page result in useful and correct information.
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Fix cross-reference to the ld-elf.so.1 man-page - there was no blank between the section number and the comma.
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
44 | Why not just remove? |
My main question regarding this review is whether the comment regarding ignored options should be in the HISTORY section or rather moved to the end of the options section.
I'll update the diff to be based on the actual ldconfig sources after commit of D44093.
And I'll move the text to the end of the DESCRIPTION section, behind the list of options, since I think that it is not of sufficient relevance for the HISTORY section.
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
44 | This was a just a reminder for me that the -elf option was also accepted and ignored - but that it still is in the usage() output. | |
161 | I have already committed this fix in the previous update that added the -B option. | |
sbin/ldconfig/ldconfig.c | ||
127 | A -B option has been added in a previous commit. |
While reading the FILES section I noticed that there was some outdated/wrong information:
- /var/run/ld-elf32.so.hints is a binary hints file, not a configuration file like /etc/ld-elf.so.conf
- /etc/ld-elf.so.conf does not exist by default, anymore
- Files in /usr/local/libdata/ldconfig/ are actually used to initialize the standard hints file when rc.d/ldconfig is executed
The information about ignored options has been moved to the end of the DESCRIPTION section.
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | There is some $LOCALPREFIX knob or such, and user.localbase sysctl, affecting the paths. I believe you was involved with it? |
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | Yes, I was involved in the implementation of the getlocalbase() function, which uses these parameters, and I have implemented the "user.localbase" sysctl variable for this purpose in commit 147eea393fdaf. Do you want to change how this path is referenced in the man-page and/or a change of rc.d/ldconfig to adapt to a non-standard $LOCALBASE? |
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | I think that default rc.conf should indeed source the usr.localbase value for _localbase instead of hardcoding /usr/local, and man page for ldconfig should provide some reference to that. |
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | See line 58 of /etc/rc on -CURRENT: _localbase=`/sbin/sysctl -n user.localbase 2> /dev/null` The assignment of /usr/local in defaults/rc.conf is only a fall-back in case the sysctl variable has not been set. # Set default value of _localbase if not previously set : ${_localbase:="/usr/local"} |
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | Great. Might be add some hint in rc.conf? |
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | I'm not sure about where to put additional information regarding a non-default $LOCALBASE. It is of course possible to add another line of comments to defaults/rc.conf that explains what "not previously set" means. Some explanatory text could also be added to the rc.conf man-page. But there are a lot of other places that refer /usr/local by literally writing this path out (instead of referring to it by e.g. $LOCALPATH), which could be checked whether they cross reference with getlocalbase() or some other man-page that hints at the possibility to build a system with a non-default $LOCALBASE. But there had been opposition to making users aware of the possibility to change $LOCALBASE on one of my earlier commits in this area (introduction oft the user.localbase sysctl, IIRC). The reasoning was that this would make it seem a good idea to change this path, while a lot of sources, documentation, and config files hard-code /usr/local (and have to be identified and changed). My suggestion to better document this specifically for rc.conf:
(BTW: There was an issue that prevented setting of user.localbase in loader.conf, that could lead to a panic - I'm not sure that I have made this a writable tunable after I found out about the possible panic.) |
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 |
My confusion was due to not knowing that _locabase was already set in /etc/rc. I read the statement in rc.conf as initialization, and not as a fallback.
|
sbin/ldconfig/ldconfig.8 | ||
---|---|---|
161 | There are too many programs in the base system that do only support the LOCALBASE directory compiled in during "make buildworld" (e.g. cron, which compiles in that base directory as: I had converted a number of utilities to use getlocalbase() instead of a compiled in localbase directory, but many more still use the compiled-in value only. I'll change the comment to better describe the feature and its limitations. |