[MPlayer-dev-eng] [PATCH] configure: enhancement for runtime cpu detection and --target support under x86

[Reimar Döffinger]

Hello,
On Tue, Dec 12, 2006 at 10:59:25PM +0800, Zuxy Meng wrote:
> 2006/12/12, Reimar Doeffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> >Build system is not my area, but some random comments:
> >
> >On Mon, Dec 04, 2006 at 10:26:53PM +0800, Zuxy Meng wrote:
> >> +if [ "$_runtime_cpudetection" = yes ] ; then
> >
> >This applies to many, many places in the patch. I think for
> >compatibility reasons you're supposed to use the 'test' command instead
> >of []
> >
> >> +  if ! ( x86 || x86_64 || ppc ) ; then
> >
> >and "not" instead of !, there are shells that do not support '!'
> >
> >> -elif x86 || x86_64; then
> >> +elif x86 || x86_64 && [ "$_runtime_cpudetection" = no -a -z "$_target" 
> >] ; then
> >
> >And do not use -a, use two checks and combine them with && (many more
> >cases of that)
> 
> 
> Hmm...[], ! and -a/-o are used elsewhere in the configure. Forgive me
> I really didn't have any idea which alternative provides better
> compatibility.

the decision against [ and -a/-o is actually not due to compatibility I
admit, nevertheless it was once decided to do it with test and &&/||.
'!' on the other hand is not used anywhere, there is even this comment:
> # not boolean test: implement the posix shell "!" operator for a
> # non-posix /bin/sh.
> #   usage:  not {command}
> # returns exit status "success" when the execution of "command"
> # fails.

> >>       ;;
> >> -    6) iproc=686
> >> +      6) iproc=686
> >
> >cosmetics (also in some more places)
> 
> No. It's intended to accommodate a new surrounding 'case..esac' pair.

Well, that's exactly the definition of cosmetics?!?

Greetings,
Reimar Döffinger