automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tests: check the depmodes 'msvisualcpp' and 'msvcmsys'


From: Stefano Lattarini
Subject: Re: [PATCH] tests: check the depmodes 'msvisualcpp' and 'msvcmsys'
Date: Wed, 08 Feb 2012 12:42:54 +0100

On 02/08/2012 11:03 AM, Peter Rosin wrote:
> * tests/defs (cygpath): New requirement, checking that cygpath
> is working.
> (mingw): New requirement, checking that the build system is
> MSYS (in its normal MinGW mode).
> * tests/gen-testsuite-part (depmodes): Add entries for depmodes
> 'msvisualcpp' and 'msvcmsys'.
> ---
>  tests/defs               |   10 ++++++++++
>  tests/gen-testsuite-part |    4 ++++
>  2 files changed, 14 insertions(+), 0 deletions(-)
> 
> Hi!
> 
> Can I assume that 'uname' works?
>
I guess so.  But see a suggestion for an extra-safety measure, below.

> If so, ok for master?
>
OK with the very minor nits below addressed.  And thanks!

> Cheers,
> Peter
> 
> 
> diff --git a/tests/defs b/tests/defs
> index 6d3810b..6031033 100644
> --- a/tests/defs
> +++ b/tests/defs
> @@ -729,6 +729,10 @@ do
>        cscope --version </dev/null \
>          || skip_all_ "required program \`cscope' not available"
>        ;;
> +    cygpath)
> +      echo "$me: running cygpath --version"
> +      cygpath --version || skip_all_ "cygpath not available"
> +      ;;
>      etags)
>        # Exuberant Ctags will create a TAGS file even
>        # when asked for --help or --version.  (Emacs's etags
> @@ -852,6 +856,12 @@ do
>          || skip_all_ "cannot find a makeinfo program that groks the" \
>                   "\`--html' option"
>        ;;
> +    mingw)
> +      case `uname -s` in
>
For extra safety in the face of 'set -e', and in order to simplify prospective
future debugging, what about using something like this instead?

  uname_s=`uname -s || echo UNKNOWN`
  echo "$me: system name: $uname_s"
  case $uname_s in
  ...

> +      MINGW*) ;;
> +      *) skip_all_ "this test requires MSYS in MinGW mode" ;;
>
Missing indentation: the "...)" patterns in a "case" statements should
be indented with two spaces.

> +      esac
> +      ;;
>      non-root)
>        # Skip this test case if the user is root.
>        # We try to append to a read-only file to detect this.
> diff --git a/tests/gen-testsuite-part b/tests/gen-testsuite-part
> index 40e6dfc..ad00657 100755
> --- a/tests/gen-testsuite-part
> +++ b/tests/gen-testsuite-part
> @@ -332,6 +332,10 @@ my %depmodes =
>  # This is for older (pre-3.x) GCC versions.  Newer versions
>  # have depmode "gcc3".
>      gcc          => ["gcc"],
> +# This is for older (pre-7) msvc versions.  Newer versions
> +# have depmodes "msvc7" and "msvc7msys".
> +    msvisualcpp  => ["cl", "cygpath"],
> +    msvcmsys     => ["cl", "mingw"],
>    );
>  
>  foreach my $lt (TRUE, FALSE)
>

Thanks,
  Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]