On Sun, Jul 16, 2017 at 10:26:25AM +0000, Robert Peichaer wrote:
> On Sun, Jul 16, 2017 at 03:37:15AM +0200, Klemens Nanni wrote:
> > Why looping over all existing archives, picking the latest version of
> > the current archive, skipping it in case it's already in our list of
> > selected latest versions or adding it otherwise?
> >
> > The current code runs ls|sort|tail about n * (v - 1) times for n
> > different libraries and v versions respectively since the globbed list
> > is almost always sorted already, effectively adding the latest versions
> > after skipping all others.
>
> Yeah well, until the globbing isn't sorting as expected.
>
> $ ls -1 libc.so.*.a
> libc.so.89.10.a
> libc.so.89.6.a
> libc.so.89.7.a
> libc.so.89.8.a
> libc.so.89.9.a
Exactly, that's why sort(1) is used.
>
> But that's not the point anyway in your otherwise perfectly valid change.
>
> > This diff makes it much clearer and simpler by sorting and picking
> > only as many versions as there are libraries to reorder (two). Globbing
> > is done within the loop so future libraries with different naming
> > schemes comes at no cost.
> >
> > Applies cleanly to both the current revision as well as my previous diff
> > but the previous one will fail on top of this one.
> >
> > Feedback? Comments?
> >
> > Index: rc
> > ===================================================================
> > RCS file: /cvs/src/etc/rc,v
> > retrieving revision 1.507
> > diff -u -p -r1.507 rc
> > --- rc 4 Jul 2017 19:02:11 -0000 1.507
> > +++ rc 16 Jul 2017 01:15:43 -0000
> > @@ -171,13 +171,10 @@ reorder_libs() {
> > echo -n 'reordering libraries:'
> >
> > # Only choose the latest version of the libraries.
> > - for _liba in /usr/lib/libc.so.*.a /usr/lib/libcrypto.so.*.a; do
> > - _liba=$(ls ${_liba%%.[0-9]*}*.a | sort -V | tail -1)
> > - for _l in $_libas; do
> > - [[ $_l == $_liba ]] && continue 2
> > - done
> > - _libas="$_libas $_liba"
> > + for _liba in 'libc.so.*.a' 'libcrypto.so.*.a'; do
> > + _libas="$_libas $(ls /usr/lib/$_liba | sort -V | tail -1)"
> > done
>
> That's definitely a much better approach.
>
> But I'd like to stay strict matching the filenames.
>
> + for _liba in /usr/lib/lib{c,crypto}; do
> + _libas="$_libas $(ls $_liba.so.+([0-9.]).a | sort -V | tail -1)"
> + done
>
> > + _libas=${_libas# }
>
> This would be another way of suppressing the blank right away.
> But it's not really necessary anyway, because the leading blank is
> removed in the list of the other for-loop.
>
> _libas="${_libas:+$_libas }$(ls $_liba.so.+([0-9.]).a | sort -V | tail
> -1)"
I explicitly avoided this since stripping the leading space in the end
seemed clearer about what's going on. Also, this check is done for each
library whereas only the first one is true.
I also thought about leaving it in but stripping it makes clear I know
what I'm doing and future changes where $_libas might get quoted will
be working as expected.
