Re: [ft-devel] Two freetype-2.3.9 problems

[Werner LEMBERG]

> The attached patch modifies the value of "--srcdir" option. It adds
> "builds/unix/" to tell the location of the autoconf configure.
> Other options are passed to builds/unix/configure transparently.

Great!

> BTW, after short tests of this patch, I'm afraid that some make
> files of FreeType2 are not transparent to the pathnames including
> white space.  [...]
>
> Werner, how do you think about the support of the pathnames
> including white spaces?

It should work in all cases, so it would be good to fix it

Some comments to your patch:

> +     Filter --srcdir= option before invoking builds/unix/configure

Please end the sentence with a full stop.

> +     * configure: when builds/unix/configure is invoked with
> +     --srcdir option, [...]

Please start with uppercase after the double colon.

> +     the option should take "builds/unix" directory instead
> +     of the top source directory. Thus configure in the top directory
> +     should modify --srcdir= option when builds/unix/configure is
> +     invoked.

Please use two spaces after a full stop.

> +# "--srcdir=" option can override abs_ft2_dir.
> +
> +if [ $# -gt 0 ]; then

For consistency (and handling broken shells) I think it's better to
say

  if test $# -gt 0; then

> +   for x in "$@"; do
> +      echo $x

This `echo' is only for debugging, right?  Please remove it.

> +      case x"$x" in
> +      x--srcdir=* )

Please omit the space before `)' for consistency.

> +        abs_ft2_dir=`echo $x | sed 's/^--srcdir=//'` ;;
> +      * ) ;;

The `*)' case is redundant and can be omitted.

> +      esac
> +   done
> +fi
> +
>  # build a dummy Makefile if we are not building in the source tree
>
>  if test "$abs_curr_dir" != "$abs_ft2_dir"; then
>    mkdir reference
>    echo "Copying \`modules.cfg'"
> -  cp $abs_ft2_dir/modules.cfg $abs_curr_dir
> +  cp "$abs_ft2_dir/modules.cfg" "$abs_curr_dir"
>    echo "Generating \`Makefile'"
> -  echo "TOP_DIR   := $abs_ft2_dir"               > Makefile
> -  echo "OBJ_DIR   := $abs_curr_dir"             >> Makefile
> +  echo $abs_ft2_dir  | sed 's/ /\\ /;s/^/TOP_DIR   := /'  > Makefile
> +  echo $abs_curr_dir | sed 's/ /\\ /;s/^/OBJ_DIR   := /' >> Makefile

I think it's cleaner to define new variables before using `echo'.


    Thanks for your efforts!


        Werner