Install issues

Install issues

[H.Merijn Brand]

1. OpenSUSE 11.0's yast2 does not have "ignore dependency" anymore in
   the gui, which makes installing git from the GIT RPMS to fail as it
   seems to depend on openssh-clients, a package that has been
   deprecated in SUSE, and is not available anymore

   In 10.3 you could still go on after "ignore this dependency
   locally", and all worked fine.

So now I have to build from source, or download the rpm and look up the
flags to rpm to make --no-deps work. That feels dirty, so I rather build

Builds fine. Now 'make test', which I often mistype as 'make check'
because the GNU folk refuse to add 'make test' as an alias for 'make
check' and switching between all the sourse distributions that do it
right (perl, git, ...) and those that do it wrong (GNU) I often type
the wrong one

git-1.6.0.4 112 > make check
for i in *.c; do sparse -g -O2 -Wall  -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRLCPY -D__BIG_ENDIAN__ -D__powerpc__ $i || exit; done
/bin/sh: sparse: command not found
make: *** [check] Error 127

Maybe the Makefile could be a little more user-friendly with a message
like

	"Cannot run 'make check', as you do not have 'sparse' installed.
	 Did you mean to run 'make test' instead?"

FYI
'/bin/sh' ./aggregate-results.sh test-results/t*-*
fixed   1
success 3726
failed  0
broken  2
total   3729
rm -f -r 'trash directory' test-results

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Install issues

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Mon, Nov 10, 2008 at 12:17:39PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
> git-1.6.0.4 112 > make check
> for i in *.c; do sparse -g -O2 -Wall  -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRLCPY -D__BIG_ENDIAN__ -D__powerpc__ $i || exit; done
> /bin/sh: sparse: command not found
> make: *** [check] Error 127
> 
> Maybe the Makefile could be a little more user-friendly with a message
> like
> 
> 	"Cannot run 'make check', as you do not have 'sparse' installed.
> 	 Did you mean to run 'make test' instead?"

Care to send a patch?

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Install issues

[H.Merijn Brand]

On Mon, 10 Nov 2008 12:39:24 +0100, Miklos Vajna
<vmiklos@frugalware.org> wrote:

> On Mon, Nov 10, 2008 at 12:17:39PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
> > git-1.6.0.4 112 > make check
> > for i in *.c; do sparse -g -O2 -Wall  -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRLCPY -D__BIG_ENDIAN__ -D__powerpc__ $i || exit; done
> > /bin/sh: sparse: command not found
> > make: *** [check] Error 127
> > 
> > Maybe the Makefile could be a little more user-friendly with a message
> > like
> > 
> > 	"Cannot run 'make check', as you do not have 'sparse' installed.
> > 	 Did you mean to run 'make test' instead?"
> 
> Care to send a patch?

--- Makefile.org	2008-11-10 17:29:53.000000000 +0100
+++ Makefile	2008-11-10 17:29:39.000000000 +0100
@@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
 	./test-sha1.sh
 
 check: common-cmds.h
+	@`sparse </dev/null 2>/dev/null` || (\
+	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
+	    echo "Did you mean 'make test' instead?" ;\
+	    exit 1 )
 	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
 
 remove-dashes:

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Install issues

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>  	./test-sha1.sh
>  
>  check: common-cmds.h
> +	@`sparse </dev/null 2>/dev/null` || (\
> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> +	    echo "Did you mean 'make test' instead?" ;\
> +	    exit 1 )
>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done

Please read Documentation/SubmittingPatches, your patch lacks a signoff
and a commit message.

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: Install issues

[H.Merijn Brand]

On Mon, 10 Nov 2008 18:51:23 +0100, Miklos Vajna
<vmiklos@frugalware.org> wrote:

> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
> > --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> > +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> > @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
> >  	./test-sha1.sh
> >  
> >  check: common-cmds.h
> > +	@`sparse </dev/null 2>/dev/null` || (\
> > +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> > +	    echo "Did you mean 'make test' instead?" ;\
> > +	    exit 1 )
> >  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
> 
> Please read Documentation/SubmittingPatches, your patch lacks a signoff
> and a commit message.

You're not making things easier for people that do not use git from a
git repo, something that happens quite a lot when you build from a
released tarball.

git-1.6.0.4 $ git format-patch -M
fatal: Not a git repository
Exit 128

I don't like this at all. How much more work is it for you to add the
subject and sign-off yourself, instead of requiring that from people
that like to help?

In the perl development, the only thing we *require* is a diff that
either uses unified (preferred) or context diff.

I did follow the ideal patch flow so far:
--8<---
An ideal patch flow

Here is an ideal patch flow for this project the current maintainer
suggests to the contributors:

 (0) You come up with an itch.  You code it up.

 (1) Send it to the list and cc people who may need to know about
     the change.
-->8---

Code speaks louder than words, so I proposed a patch.
This might be my last patch. git is not just another project I want to
loose so much time in.

--8<---
Author: H.Merijn Brand <h.m.brand@xs4all.nl>
Date:   Mon, 10 Nov 2008 17:31:01 +0100

    Make check needs sparse. If sparse is not available, it might as
    well be a user error who really wanted make test.

Signed-off-by: H.Merijn Brand <h.m.brand@xs4all.nl>
---

diff --git a/Makefile.org b/Makefile
index becd008..718ddf2 100644
--- a/Makefile
+++ b/Makefile
@@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
        ./test-sha1.sh

 check: common-cmds.h
+       @`sparse </dev/null 2>/dev/null` || (\
+           echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
+           echo "Did you mean 'make test' instead?" ;\
+           exit 1 )
        for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done

 remove-dashes:
-->8---

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Install issues

[Andreas Ericsson]

H.Merijn Brand wrote:
> On Mon, 10 Nov 2008 18:51:23 +0100, Miklos Vajna
> <vmiklos@frugalware.org> wrote:
> 
>> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
>>> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
>>> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
>>> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>>>  	./test-sha1.sh
>>>  
>>>  check: common-cmds.h
>>> +	@`sparse </dev/null 2>/dev/null` || (\
>>> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
>>> +	    echo "Did you mean 'make test' instead?" ;\
>>> +	    exit 1 )
>>>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
>> Please read Documentation/SubmittingPatches, your patch lacks a signoff
>> and a commit message.
> 
> You're not making things easier for people that do not use git from a
> git repo, something that happens quite a lot when you build from a
> released tarball.
> 

I think it's assumed that most git developers will clone the git repo so
they can send their patches against the very latest code. Usually, that's
part of "how to submit patches" of all projects.

> git-1.6.0.4 $ git format-patch -M
> fatal: Not a git repository
> Exit 128
> 
> I don't like this at all. How much more work is it for you to add the
> subject and sign-off yourself, instead of requiring that from people
> that like to help?
> 

"Ask not what a bunch of oss devs can do for you, but what you can do
for them. Especially if you want your changes accepted".

The harsh reality is that this is your itch. Noone else cares very
much either way, so if you want your change included, you'll have to
do it the way the project maintainer wants it.

> In the perl development, the only thing we *require* is a diff that
> either uses unified (preferred) or context diff.
> 

Nothing's required here, but since nobody else seems to have problems
with the check/test stuff here, noone else will fix it up for you.

Besides that, I'm sure you require a teensy bit more than that. If I
send you a 14k line patch without a reasoning behind it, you won't
just go ahead and apply it, will you? If I send an algorithm for a new
hash that might actually be proprietary code from an anonymous email
address, wouldn't you want some sort of verification that I'm actually
allowed to send that algorithm to you?

In git, those two steps are formalized into "Sign your patches!" and
"Write a proper commit message!". Since they *are* formalized, we
provide handy tools for doing just that. It's up to you if you want to
use them. Dictating the patch submission (or rather, acceptancy policy)
is not. Patches submitted incorrectly will have one of the following
things happen to them:
1. The patch is forgotten entirely, since nobody cares about it enough
   to make it a "proper" patch (by git standards).
2. The patch author resubmits it in the proper format.
3. Someone else resubmits it in the proper format.
4. Junio accepts it anyway.

4 is the least likely to happen, and will pretty much only occur if
someone finds some really horrible bug that absolutely and obviously
has to go in the git code. Even in those cases, 3 usually happens
before Junio wakes up.

3 is not very likely to happen unless the feature you're suggesting
is of interest to a lot of people.

Ofcourse, *after* the patch has been submitted in the proper format,
it can still get rejected. Submitting it properly is only the first
step towards getting any kind of review on it (basically).

> I did follow the ideal patch flow so far:
> --8<---
> An ideal patch flow
> 
> Here is an ideal patch flow for this project the current maintainer
> suggests to the contributors:
> 
>  (0) You come up with an itch.  You code it up.
> 
>  (1) Send it to the list and cc people who may need to know about
>      the change.
> -->8---
> 
> Code speaks louder than words, so I proposed a patch.
> This might be my last patch. git is not just another project I want to
> loose so much time in.
> 

That would be sad. We're a tad short on perl folks, although I guess
that's not much of an issue since we're rewriting most of it in C
anyways.

If you should decide to relent though, I seriously suggest using the
tools git provides for sending patches. If you do, you'll be able to
able to send (properly formatted) patches very easily indeed.

Rant aside, thanks for the re-send. I'm sure Junio can sed s/Author/From/
on the mail and have git accept it properly in case he accepts it.
Otherwise, I'll resubmit it, faking you as the author.

> --8<---
> Author: H.Merijn Brand <h.m.brand@xs4all.nl>
> Date:   Mon, 10 Nov 2008 17:31:01 +0100
> 
>     Make check needs sparse. If sparse is not available, it might as
>     well be a user error who really wanted make test.
> 
> Signed-off-by: H.Merijn Brand <h.m.brand@xs4all.nl>
> ---
> 
> diff --git a/Makefile.org b/Makefile
> index becd008..718ddf2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>         ./test-sha1.sh
> 
>  check: common-cmds.h
> +       @`sparse </dev/null 2>/dev/null` || (\
> +           echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> +           echo "Did you mean 'make test' instead?" ;\
> +           exit 1 )
>         for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
> 
>  remove-dashes:
> -->8---
> 


-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: Install issues

[Junio C Hamano]

Miklos Vajna <vmiklos@frugalware.org> writes:

> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
>> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
>> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
>> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>>  	./test-sha1.sh
>>  
>>  check: common-cmds.h
>> +	@`sparse </dev/null 2>/dev/null` || (\
>> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
>> +	    echo "Did you mean 'make test' instead?" ;\
>> +	    exit 1 )
>>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
>
> Please read Documentation/SubmittingPatches, your patch lacks a signoff
> and a commit message.

Heh, for something small and obvious like this, that's asking a tad too
much, although a properly formatted message does reduce my workload and is
appreciated.

I said "obvious" not in the sense that it is "obviously good".  It is
obvious what issue the patch wants to address.

Having said that, it is far from clear if special casing "make check" like
this is a good thing, though.  The crufts resulting from "Four extra lines
won't hurt" kind of reasoning can accumulate and snowball.  Is reading the
Makefile when your build fails in order to see if the target was what you
really wanted to invoke (ideally, it should rater be "_before_ running
make, reading the Makefile to find out what you want to run") a lost art
these days?

Re: Install issues

[H.Merijn Brand]

On Tue, 11 Nov 2008 09:47:00 -0800, Junio C Hamano <gitster@pobox.com>
wrote:

> Miklos Vajna <vmiklos@frugalware.org> writes:
> 
> > On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
> >> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> >> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> >> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
> >>  	./test-sha1.sh
> >>  
> >>  check: common-cmds.h
> >> +	@`sparse </dev/null 2>/dev/null` || (\
> >> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> >> +	    echo "Did you mean 'make test' instead?" ;\
> >> +	    exit 1 )
> >>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
> >
> > Please read Documentation/SubmittingPatches, your patch lacks a signoff
> > and a commit message.
> 
> Heh, for something small and obvious like this, that's asking a tad too
> much, although a properly formatted message does reduce my workload and is
> appreciated.

Junio++

> I said "obvious" not in the sense that it is "obviously good".  It is
> obvious what issue the patch wants to address.
> 
> Having said that, it is far from clear if special casing "make check" like
> this is a good thing, though.  The crufts resulting from "Four extra lines
> won't hurt" kind of reasoning can accumulate and snowball.  Is reading the
> Makefile when your build fails in order to see if the target was what you
> really wanted to invoke (ideally, it should rater be "_before_ running
> make, reading the Makefile to find out what you want to run") a lost art
> these days?

Not at all, and for me it was clear from the start, so I typed 'make
test' and went ahead.

It was that I am just all to aware of the GNU world that I can easily
imagine other people making the same mistake, and just thought it
end-user-friendly to do as I proposed.

I'm by now way offended or scared away if you reject these kind of
patches

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Install issues

[Boyd Lynn Gerber]

On Tue, 11 Nov 2008, H.Merijn Brand wrote:
> On Tue, 11 Nov 2008 09:47:00 -0800, Junio C Hamano <gitster@pobox.com>
> wrote:
>> Miklos Vajna <vmiklos@frugalware.org> writes:
>>> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
>>>> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
>>>> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
>>>> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>>>>  	./test-sha1.sh
>>>>
>>>>  check: common-cmds.h
>>>> +	@`sparse </dev/null 2>/dev/null` || (\
>>>> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
>>>> +	    echo "Did you mean 'make test' instead?" ;\
>>>> +	    exit 1 )
>>>>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
>>>
>>> Please read Documentation/SubmittingPatches, your patch lacks a signoff
>>> and a commit message.

Here is a check list I use to make sure my patches are OK.

http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD

...
> I'm by now way offended or scared away if you reject these kind of
> patches

The crowed here may seem harsh but they really are not.  They just have a 
lot to do and getting things to them in a format that is easy to follow 
and uses the above URL guildlines assists them in looking at the patches, 
commit messages and sign-off.  Git requires a sign-off in order for them 
to use the patches.  It is a way to make sure everything is kept above 
board and proper.  This allow ever line of code to have a resource as to 
where and whence it ...

Do not give up.  Just try to do things in a manner that is easy to allow 
your patches into Git.

Good Luck,

--
Boyd Gerber <gerberb@zenez.com>
ZENEZ	1042 East Fort Union #135, Midvale Utah  84047

Re: Install issues

[H.Merijn Brand]

On Tue, 11 Nov 2008 16:03:50 -0700, Boyd Lynn Gerber <gerberb@zenez.com> wrote:

> Here is a check list I use to make sure my patches are OK.
> 
> http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD
> ...
> > I'm by now way offended or scared away if you reject these kind of
             ^
/me meant "no" where he wrote "now" :( :(
I can imagine people interpreting that sentence very very wrong.

> > patches
> 
> The crowed here may seem harsh but they really are not.  They just have a 
> lot to do and getting things to them in a format that is easy to follow 
> and uses the above URL guildlines assists them in looking at the patches, 
> commit messages and sign-off.  Git requires a sign-off in order for them 
> to use the patches.  It is a way to make sure everything is kept above 
> board and proper.  This allow ever line of code to have a resource as to 
> where and whence it ...
> 
> Do not give up.  Just try to do things in a manner that is easy to allow 
> your patches into Git.

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Install issues

[Junio C Hamano]

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> On Tue, 11 Nov 2008 16:03:50 -0700, Boyd Lynn Gerber <gerberb@zenez.com> wrote:
>
>> Here is a check list I use to make sure my patches are OK.
>> 
>> http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD
>> ...
>> > I'm by now way offended or scared away if you reject these kind of
>              ^
> /me meant "no" where he wrote "now" :( :(
> I can imagine people interpreting that sentence very very wrong.

That is not even "interpretation".  You _literally_ wrote the exact
opposite of what you meant ;-).

Re: Install issues

[H.Merijn Brand]

On Tue, 11 Nov 2008 15:42:26 -0800, Junio C Hamano <gitster@pobox.com>
wrote:

> "H.Merijn Brand" <h.m.brand@xs4all.nl> writes:
> 
> > On Tue, 11 Nov 2008 16:03:50 -0700, Boyd Lynn Gerber <gerberb@zenez.com> wrote:
> >
> >> Here is a check list I use to make sure my patches are OK.
> >> 
> >> http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD
> >> ...
> >> > I'm by now way offended or scared away if you reject these kind of
> >              ^
> > /me meant "no" where he wrote "now" :( :(
> > I can imagine people interpreting that sentence very very wrong.
> 
> That is not even "interpretation".  You _literally_ wrote the exact
> opposite of what you meant ;-).

I'm really really sorry. That must have sounded harsh.

-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

Re: Install issues

[Andreas Ericsson]

Junio C Hamano wrote:
> Miklos Vajna <vmiklos@frugalware.org> writes:
> 
>> On Mon, Nov 10, 2008 at 05:31:01PM +0100, "H.Merijn Brand" <h.m.brand@xs4all.nl> wrote:
>>> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
>>> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
>>> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>>>  	./test-sha1.sh
>>>  
>>>  check: common-cmds.h
>>> +	@`sparse </dev/null 2>/dev/null` || (\
>>> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
>>> +	    echo "Did you mean 'make test' instead?" ;\
>>> +	    exit 1 )
>>>  	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
>> Please read Documentation/SubmittingPatches, your patch lacks a signoff
>> and a commit message.
> 
> Heh, for something small and obvious like this, that's asking a tad too
> much, although a properly formatted message does reduce my workload and is
> appreciated.
> 
> I said "obvious" not in the sense that it is "obviously good".  It is
> obvious what issue the patch wants to address.
> 
> Having said that, it is far from clear if special casing "make check" like
> this is a good thing, though.  The crufts resulting from "Four extra lines
> won't hurt" kind of reasoning can accumulate and snowball.  Is reading the
> Makefile when your build fails in order to see if the target was what you
> really wanted to invoke (ideally, it should rater be "_before_ running
> make, reading the Makefile to find out what you want to run") a lost art
> these days?
> 

Why not "make help" with as friendly a message as we can muster, like the
linux kernel does it?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: Install issues

[Junio C Hamano]

"H.Merijn Brand" <h.m.brand@xs4all.nl> writes:

> --- Makefile.org	2008-11-10 17:29:53.000000000 +0100
> +++ Makefile	2008-11-10 17:29:39.000000000 +0100
> @@ -1329,6 +1329,10 @@ check-sha1:: test-sha1$X
>  	./test-sha1.sh
>  
>  check: common-cmds.h
> +	@`sparse </dev/null 2>/dev/null` || (\
> +	    echo "The 'sparse' command is not available, so I cannot make the 'check' target" ;\
> +	    echo "Did you mean 'make test' instead?" ;\
> +	    exit 1 )

When you mean "grouping", using {} is much clearer to convey your
intention.  Use of needless (subshell) forces the reader to wonder if you
wanted to do something that affects the environment for later commands
inside, and in this case you didn't.

Why do you have sparse check inside a backtick to produce a string to be
interpreted as a command to be executed?

How about doing this instead?  'sparse' without any parameter exits with
success status silently; when you do not have the command, the shell will
complain with "sparse: command not found" anyway, so you only need to
suggest "make 'test'" and nothing else.

-- >8 --
Subject: Makefile: help people who run 'make check' by mistake

The target to run self test is 'make test', but there are people who try
'make check' and worse yet do not have sparse installed.

Suggest 'make test' target when they do not have 'sparse'.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git c/Makefile w/Makefile
index 40309e1..d3137ca 100644
--- c/Makefile
+++ w/Makefile
@@ -1355,7 +1355,16 @@ check-sha1:: test-sha1$X
 	./test-sha1.sh
 
 check: common-cmds.h
-	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
+	if sparse; \
+	then \
+		for i in *.c; \
+		do \
+			sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; \
+		done; \
+	else \
+		echo 2>&1 "Did you mean 'make test'?"; \
+		exit 1; \
+	fi
 
 remove-dashes:
 	./fixup-builtins $(BUILT_INS) $(PROGRAMS) $(SCRIPTS)