* Gary V. Vaughan wrote on Sat, Jun 12, 2010 at 12:34:04PM CEST: > On 12 Jun 2010, at 15:48, Ralf Wildenhues wrote: > > The variable preserve_args is not properly initialized any more, > > so it could pick up junk from the environment. > > Okay. I did that to avoid the initial space, otherwise > ${preserve_args+.... } is set and expands to the part with the > space. But since a spurious leading space is better that picking > up garbage from the environment, that's an obvious change.
Well, these are two orthogonal issues. But the leading space is still not a problem. :-) > > The addition of > > --debug to preserve_args is moved out of the parsing loop, unlike > > the other preserved args, which is inconsistent. > > That's because M4SH_GETOPTS adds a consistent (wrt other clients of > M4SH_GETOPTS) --debug|-x -> $opt_debug case branch to the option > parsing loop automatically, and the preserved_args handling is bespoke > to ltmain.m4sh. Does the change of argument ordering in the case of > --debug really matter? No. I simply wasn't aware that the inconsistency had a reason other than oversight. Never mind then. > If it could cause problems, then I'll remove > automatic --debug processing from M4SH_GETOPTS, and will have to add > it back manually in each of our other scripts. Not needed, thanks. > > I'm not sure why some of the variable renamings are done, e.g., > > opt_preserve_dup_deps, or execute_dlfiles, or mode. These could easily > > be done in a separate patch (and then, they would be trivial to verify, > > and maybe easier to understand why they are needed). > > When M4SH_GETOPT generates the code for a given command line option > that you specify to it, the associated variable name is generated by > stripping the leading `--' from the long option name (or short option > if there is no long option, but that is irrelevant for now), converting > non alphanumeric characters to `_' and adding a leading `opt_'. So, if > the long option name is --preserve-dup-deps, then M4SH_GETOPTS generates > code that fills opt_preserve_dup_deps appropriately, and that is less of > an impact than changing the option name to --duplicate-deps to retain > opt_duplicate_deps. Similarly for --mode to opt_mode. This seems more > consistent to me (whatever was specified to the script as a command line > option can be found in an associated opt_option_name variable) and is > an improvement over the arbitrary names we were using until now. Point well taken. Oversight of mine in reviewing. > > Why do you now separate entries in opt_dlopen (formerly execute_dlfiles) > > with newlines, rather than, as formerly, with spaces? > > The M4SH_GETOPTS ';' specifier does newline separation in additive > options for more flexibility (and is required to support multiword > additive options such as --header='Reply-To: bug-libt...@gnu.org' in > announce-gen). > > We could add another specifier for space separated additive options, > but that seems like busy work to me when the existing ';' works very > well, and is more general. Ah. Newline is fine with me then. > > The handling of --dlopen=..., --mode=..., --tag=..., now increased by > > two forks and one exec per such flag on systems with decent shells, and > > it doesn't use safe $ECHO any more even on systems with indecent shells, > > because you don't use func_opt_split any more. My guess is that this > > would increase libtool execution time for typical compile commands by > > a noticeable amount, since we got them down to about half a dozen forks. > > Please fix this. > > That's quite an invasive change, since the core of the M4SH_GETOPTS > generated code will then rely on the XSI functions you add to libtool at > configure time. I suppose we need to put them in, say, > libltd/config/xsi.m4sh where libtoolize, commit, announce-gen, > mailnotify and any future m4sh scripts can reap the benefits. This > in turn means we need to move it into a separate macro to contribute > back to Autoconf when we move getopt.m4sh and supporting code. I'm fine with not using XSI for all other scripts; the func_opt_split could be trivial forwarders for the sed scripts there; that would avoid having to beef up infrastructure here, IIUC. But the libtool command line parsing really was one of the time critical issues in user builds, and --mode and --tag are pervasive, so this would be a real regression. Thanks, Ralf