Build bug report: 'make check' needs sparse, but configure doesn't check it

Build bug report: 'make check' needs sparse, but configure doesn't check it

[Ed Avis]

When you build git-1.6.0.2 and 'make check', it tries to run sparse and fails if
sparse is not there.  But the configure script does not check sparse is
installed.

I suggest that 'make check' skip the sparse tests if sparse is not there.  As I
understand it, they are really more for the developers to get a report of
possible errors in the source code and not to test that the built executables
work, so it would not be dangerous to just skip running those tests for ordinary
users who don't have the tool.

Alternatively, if running sparse is really important for a thorough 'make
check', the configure script should check for it and warn you to install it.

-- 
Ed Avis <ed@membled.com>

Re: Build bug report: 'make check' needs sparse, but configure doesn't check it

[Dmitry Potapov]

On Sat, Oct 04, 2008 at 01:19:56PM +0000, Ed Avis wrote:
> When you build git-1.6.0.2 and 'make check', it tries to run sparse and fails if
> sparse is not there.  But the configure script does not check sparse is
> installed.
> 
> I suggest that 'make check' skip the sparse tests if sparse is not there.  As I
> understand it, they are really more for the developers to get a report of
> possible errors in the source code and not to test that the built executables
> work, so it would not be dangerous to just skip running those tests for ordinary
> users who don't have the tool.

The whole point of 'make check' is to run 'sparce' on all files.
Granted, you may not have 'sparce' installed, but then why do you
want to run 'make check'? It makes no sense. If you want to build
and to test the resulting binaries then you run 'make test'.


Dmitry

Re: Build bug report: 'make check' needs sparse, but configure doesn't check it

[Ed Avis]

Dmitry Potapov <dpotapov <at> gmail.com> writes:

>The whole point of 'make check' is to run 'sparce' on all files.

>If you want to build
>and to test the resulting binaries then you run 'make test'.

Thanks, I didn't realize that.  I assumed that 'make check' was to run the test
suite.

That is the convention used by most autoconf-based programs: for example if you
download coreutils then 'make check' runs the tests, so the normal way to build
and install is something like 'configure && make check && make install'.

To reduce confusion I suggest renaming the target to 'make sparse'.  Then it's
obvious what it does and it can't get confused with the more common usage of
'make check'.

-- 
Ed Avis <eda@waniasset.com>

Re: Build bug report: 'make check' needs sparse, but configure doesn't check it

[Shawn O. Pearce]

Ed Avis <eda@waniasset.com> wrote:
> Dmitry Potapov <dpotapov <at> gmail.com> writes:
> 
> >The whole point of 'make check' is to run 'sparce' on all files.
> 
> >If you want to build
> >and to test the resulting binaries then you run 'make test'.
> 
> To reduce confusion I suggest renaming the target to 'make sparse'.  Then it's
> obvious what it does and it can't get confused with the more common usage of
> 'make check'.

Nah.  We're not going to rename the target to "make sparse".
First off that sounds like we are compiling the sparse binary for
the user, and we will never do that.  And existing developers who
use "make check" to run sparse are used to that calling convention.

-- 
Shawn.

Re: Build bug report: 'make check' needs sparse, but configure doesn't check it

[Ed Avis]

Shawn O. Pearce <spearce <at> spearce.org> writes:

>Nah.  We're not going to rename the target to "make sparse".

Fair enough.  Can I suggest a one line patch to avoid any misunderstanding in 
the future?

diff --git a/Makefile b/Makefile
index 3c0664a..c98b921 100644
--- a/Makefile
+++ b/Makefile
@@ -1354,6 +1354,7 @@ check-sha1:: test-sha1$X
        ./test-sha1.sh

 check: common-cmds.h
+       @echo "'make check' runs the 'sparse' tool on all source files; say 
'make test' to run the test suite."
        for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done

 remove-dashes: