* Re: [PATCH 3/3] Makefile: use -Wdeclaration-after-statement if supported
From: Junio C Hamano @ 2012-12-17 1:52 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1355686561-1057-4-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> If we adopt this approach,...
> diff --git a/Makefile b/Makefile
> index a49d1db..aae70d4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -331,8 +331,13 @@ endif
> # CFLAGS and LDFLAGS are for the users to override from the command line.
>
> CFLAGS = -g -O2 -Wall
> +GCC_DECL_AFTER_STATEMENT = \
> + $(shell $(CC) --help -v 2>&1 | \
> + grep -q -- -Wdeclaration-after-statement && \
> + echo -Wdeclaration-after-statement)
> +GCC_FLAGS = $(GCC_DECL_AFTER_STATEMENT)
> +ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) $(GCC_FLAGS)
> LDFLAGS =
> -ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
> ALL_LDFLAGS = $(LDFLAGS)
Please do not do this.
People cannot disable it from the command line, like:
$ make V=1 CFLAGS='-g -O0 -Wall'
If anything, this should be part of the default CFLAGS.
More importantly, this will run the $(shell ...) struct once for
every *.o file we produce, I think, in addition to running it twice
for the whole build. If you add this:
@@ -345,7 +345,8 @@ CFLAGS = -g -O2 -Wall
GCC_DECL_AFTER_STATEMENT = \
$(shell $(CC) --help -v 2>&1 | \
grep -q -- -Wdeclaration-after-statement && \
- echo -Wdeclaration-after-statement)
+ echo -Wdeclaration-after-statement; \
+ echo >&2 GCC_DECL_AFTER_STATEMENT CRUFT)
GCC_FLAGS = $(GCC_DECL_AFTER_STATEMENT)
ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) $(GCC_FLAGS)
LDFLAGS =
remove git.o and dir.o from a fully built tree, and then try to
rebuild these two files, you will get this:
$ make V=1 git.o dir.o
GCC_DECL_AFTER_STATEMENT CRUFT
GCC_DECL_AFTER_STATEMENT CRUFT
GCC_DECL_AFTER_STATEMENT CRUFT
cc -o git.o -c -MF ./.depend/git.o.d -MMD -MP -g -O2 -Wall \
-Wdeclaration-after-statement -I. -DHAVE_PATHS_H -DHAVE_DEV_TTY \
-DXDL_FAST_HASH -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRLCPY \
-DNO_MKSTEMPS -DSHELL_PATH='"/bin/sh"' \
'-DGIT_HTML_PATH="share/doc/git-doc"' '-DGIT_MAN_PATH="share/man"' \
'-DGIT_INFO_PATH="share/info"' git.c
GCC_DECL_AFTER_STATEMENT CRUFT
cc -o dir.o -c -MF ./.depend/dir.o.d -MMD -MP -g -O2 -Wall \
-Wdeclaration-after-statement -I. -DHAVE_PATHS_H -DHAVE_DEV_TTY \
-DXDL_FAST_HASH -DSHA1_HEADER='<openssl/sha.h>' -DNO_STRLCPY \
-DNO_MKSTEMPS -DSHELL_PATH='"/bin/sh"' dir.c
$ make V=1 git.o dir.o
GCC_DECL_AFTER_STATEMENT CRUFT
GCC_DECL_AFTER_STATEMENT CRUFT
make: `dir.o' is up to date.
^ permalink raw reply
* $PATH pollution and t9902-completion.sh
From: Adam Spiers @ 2012-12-17 1:05 UTC (permalink / raw)
To: git mailing list
t/t9902-completion.sh is currently failing for me because I happen to
have a custom shell-script called git-check-email in ~/bin, which is
on my $PATH. This is different to a similar-looking case reported
recently, which was due to an unclean working tree:
http://thread.gmane.org/gmane.comp.version-control.git/208085
It's not unthinkable that in the future other tests could break for
similar reasons. Therefore it would be good to sanitize $PATH in the
test framework so that it cannot destabilize tests, although I am
struggling to think of a good way of doing this. Naively stripping
directories under $HOME would not protect against git "plugins" such
as the above being installed into places like /usr/bin. Thoughts?
^ permalink raw reply
* Re: [PATCH 12/12] Add git-check-ignore sub-command
From: Adam Spiers @ 2012-12-17 0:10 UTC (permalink / raw)
To: git
In-Reply-To: <7vmwzmtmyd.fsf@alter.siamese.dyndns.org>
On Tue, Oct 16, 2012 at 09:12:58AM -0700, Junio C Hamano wrote:
> Adam Spiers <git@adamspiers.org> writes:
> > On Mon, Oct 15, 2012 at 3:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> >>> +For each pathname given via the command-line or from a file via
> >>> +`--stdin`, this command will list the first exclude pattern found (if
> >>> +any) which explicitly excludes or includes that pathname. Note that
> >>> +within any given exclude file, later patterns take precedence over
> >>> +earlier ones, so any matching pattern which this command outputs may
> >>> +not be the one you would immediately expect.
> >>
> >> "The first exclude pattern" is very misleading, isn't it?
> >
> > I don't think so, because of the second sentence.
> >
> >> For example, with these in $GIT_DIR/info/exclude, I would get:
> >>
> >> $ cat -n .git/info/exclude
> >> 1 *~
> >> 2 Makefile~
> >> $ git check-ignore -v Makefile~
> >> .git/info/exclude:2:Makefile~ Makefile~
> >>
> >> which is the correct result (the last one in a single source decides
> >> the fate of the path), but it hardly is "first one found" and the
> >> matching pattern in the output would not be something unexpected for
> >> the users, either.
> >>
> >> The reason it is "the first one found" is because the implementation
> >> arranges the loop in such a way that it can stop early when it finds
> >> a match---it simply checks matches from the end of the source.
> >>
> >> But that is not visible to end-users,
> >
> > Correct; that's precisely why I wrote the second sentence which
> > explicitly explains this.
> >
> >> and they will find the above description just wrong, no?
> >
> > It's not wrong AFAICS, but suggestions for rewording this more clearly
> > are of course welcome. Maybe s/immediately/intuitively/ ?
>
> I think this is sufficient:
>
> For each pathname given via the command-line or from a file
> via `--stdin`, show the pattern from .gitignore (or other
> input files to the exclude mechanism) that decides if the
> pathname is excluded.
>
> and without "Note that" at all.
I don't think this is quite sufficient. Firstly, it does not cover
negated patterns (my original text contained "or includes").
Secondly, I think there is still potential for this rewording to
result in confused users. If the "backwards-ness" of the internal
algorithm is kept hidden from them, then in your example above, most
users would be more likely to intuitively expect check-ignore to
return the first line of .git/info/exclude ("*~"). When they saw it
returning the second, they might draw the conclusion that the first
line failed to match (e.g. by mistakenly thinking that the file format
requires regular expressions rather than globs), rather than that git
starts at the end of the file.
This is precisely why I chose not to hide this aspect of the
implementation when initially writing this documentation.
Unfortunately my wording managed to confuse several of you, so clearly
it was not adequate. Therefore I propose an extension of your
version:
For each pathname given via the command-line or from a file
via `--stdin`, show the pattern from .gitignore (or other
input files to the exclude mechanism) that decides if the
pathname is excluded or included. Later patterns within a
file take precedence over earlier ones.
^ permalink raw reply
* Re: [PATCH 1/3] SubmittingPatches: add convention of prefixing commit messages
From: Junio C Hamano @ 2012-12-16 23:15 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1355686561-1057-2-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> Conscientious newcomers to git development will read SubmittingPatches
> and CodingGuidelines, but could easily miss the convention of
> prefixing commit messages with a single word identifying the file
> or area the commit touches.
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
> Documentation/SubmittingPatches | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 0dbf2c9..c107cb1 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -9,6 +9,14 @@ Checklist (and a short version for the impatient):
> - the first line of the commit message should be a short
> description (50 characters is the soft limit, see DISCUSSION
> in git-commit(1)), and should skip the full stop
> + - it is also conventional in most cases to prefix the
> + first line with "area: " where the area is a filename
> + or identifier for the general area of the code being
> + modified, e.g.
> + . archive: ustar header checksum is computed unsigned
> + . git-cherry-pick.txt: clarify the use of revision range notation
> + (if in doubt which identifier to use, run "git log --no-merges"
> + on the files you are modifying to see the current conventions)
Thanks; I have to wonder if these details should be left in the
longer version to keep the "short" one short, though.
We should probably add "learn from good examples." (aka "read 'git
log' output and the pattern should be obvious to you") as the first
item to this list, too.
> - the body should provide a meaningful commit message, which:
> . explains the problem the change tries to solve, iow, what
> is wrong with the current code without the change.
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Junio C Hamano @ 2012-12-16 23:11 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <CAOkDyE9B_HfUZmqNqO35mtjTvdihBTiW=uOV2oEQgLUw1xyf=A@mail.gmail.com>
Adam Spiers <git@adamspiers.org> writes:
> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Adam Spiers <git@adamspiers.org> writes:
>>
>>> This series of commits attempts to make test output coloring
>>> more intuitive,...
>>
>> Thanks; I understand that this is to replace the previous one
>> b465316 (tests: paint unexpectedly fixed known breakages in bold
>> red, 2012-09-19)---am I correct?
>
> Correct. AFAICS I have incorporated all feedback raised in previous
> reviews.
Seemed clean from a cursory look. Will replace. Thanks.
^ permalink raw reply
* compiler checks
From: Adam Spiers @ 2012-12-16 23:04 UTC (permalink / raw)
To: git list
In-Reply-To: <7vehlv5hg8.fsf@alter.siamese.dyndns.org>
On Fri, Sep 21, 2012 at 12:00:55PM -0700, Junio C Hamano wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
> > It has been rebased on the latest master, and passed a full test run.
>
> FYI, I applied the attached on top before queuing it in 'pu'.
>
> Points to note:
>
> * We match the underline and the title of documentation header;
>
> * a few type mismatches (constness of full_path and treat_gitlink()
> signature) that broke compilation;
Of course I will incorporate these tweaks in my re-roll, but it
worries me that my environment yielded no compilation issues even
without these tweaks. Obviously I wouldn't have dreamed of submitting
a patch series which didn't even compile! I'm using a modern gcc, and
I guess you probably are too? Which would suggest to me that either
your environment is somehow set up to perform stricter type checking
than mine[1], or that there's a weird compiler-oriented bug somewhere
(e.g. in Makefile). Or maybe I'm missing something obvious ...
[1] I'm in favour of stricter compiler checks where possible:
http://article.gmane.org/gmane.comp.version-control.git/211607
^ permalink raw reply
* [PATCH v2] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2012-12-16 22:20 UTC (permalink / raw)
To: git; +Cc: Manlio Perillo
The git-completion.bash script did not implemented full support for
completion, for git commands that operate on files from the current
working directory or the index.
For these commands, only options completion was available.
Full support for completion is now implemented, for git commands where
the non-option arguments always refer to paths on the current working
directory or the index, as the follow:
* the path completion for the "git mv" and "git rm" commands is provided
using "git ls-files --exclude-standard"
* the path completion for the "git add" command is provided using
"git ls-files --exclude-standard -o -m"
* the path completion for the "git clean" command is provided using
"git ls-files --exclude-standard -o"
* the path completion for the "git commit" command is provides using
"git diff-index --name-only HEAD"
Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
Updated the script documentation.
contrib/completion/git-completion.bash | 40 +++++++++++++++++++++++-----------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b77eb1..3bd7fc8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,6 +13,7 @@
# *) .git/remotes file names
# *) git 'subcommands'
# *) tree paths within 'ref:path/to/file' expressions
+# *) working directory and index file names
# *) common --long-options
#
# To use these routines:
@@ -233,6 +234,25 @@ __gitcomp_nl ()
COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
}
+__git_files ()
+{
+ local dir="$(__gitdir)"
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" ls-files --exclude-standard $*
+ return
+ fi
+}
+
+# Return all staged files with modification from current HEAD
+__git_commit_files ()
+{
+ local dir="$(__gitdir)"
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" diff-index --name-only HEAD
+ return
+ fi
+}
+
__git_heads ()
{
local dir="$(__gitdir)"
@@ -770,8 +790,6 @@ _git_apply ()
_git_add ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "
@@ -780,7 +798,8 @@ _git_add ()
"
return
esac
- COMPREPLY=()
+ # XXX should we care for --update and --all options ?
+ __gitcomp_nl "$(__git_files -o -m)" "" "$cur" ""
}
_git_archive ()
@@ -930,15 +949,14 @@ _git_cherry_pick ()
_git_clean ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--dry-run --quiet"
return
;;
esac
- COMPREPLY=()
+ # TODO: check for -x option
+ __gitcomp_nl "$(__git_files -o)" "" "$cur" ""
}
_git_clone ()
@@ -969,8 +987,6 @@ _git_clone ()
_git_commit ()
{
- __git_has_doubledash && return
-
case "$cur" in
--cleanup=*)
__gitcomp "default strip verbatim whitespace
@@ -998,7 +1014,7 @@ _git_commit ()
"
return
esac
- COMPREPLY=()
+ __gitcomp_nl "$(__git_commit_files)" "" "$cur" ""
}
_git_describe ()
@@ -1362,7 +1378,7 @@ _git_mv ()
return
;;
esac
- COMPREPLY=()
+ __gitcomp_nl "$(__git_files)" "" "$cur" ""
}
_git_name_rev ()
@@ -2068,15 +2084,13 @@ _git_revert ()
_git_rm ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
return
;;
esac
- COMPREPLY=()
+ __gitcomp_nl "$(__git_files)" "" "$cur" ""
}
_git_shortlog ()
--
1.8.1.rc1.18.g9db0d25
^ permalink raw reply related
* [PATCH] git-completion.bash: update obsolete code.
From: Manlio Perillo @ 2012-12-16 21:50 UTC (permalink / raw)
To: git; +Cc: Manlio Perillo
The git-completion.bash script was using the git ls-tree command
without the --name-only option, with a sed filter to parse path names;
use the --name-only option, instead.
Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
contrib/completion/git-completion.bash | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b77eb1..85d9051 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -397,20 +397,7 @@ __git_complete_revlist_file ()
*) pfx="$ref:$pfx" ;;
esac
- __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" \
- | sed '/^100... blob /{
- s,^.* ,,
- s,$, ,
- }
- /^120000 blob /{
- s,^.* ,,
- s,$, ,
- }
- /^040000 tree /{
- s,^.* ,,
- s,$,/,
- }
- s/^.* //')" \
+ __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree --name-only "$ls")" \
"$pfx" "$cur_" ""
;;
*...*)
--
1.8.1.rc1.18.g9db0d25
^ permalink raw reply related
* [PATCH] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2012-12-16 21:24 UTC (permalink / raw)
To: git; +Cc: Manlio Perillo
The git-completion.bash script did not implemented full support for
completion, for git commands that operate on files from the current
working directory or the index.
For these commands, only options completion was available.
Full support for completion is now implemented, for git commands where
the non-option arguments always refer to paths on the current working
directory or the index, as the follow:
* the path completion for the "git mv" and "git rm" commands is provided
using "git ls-files --exclude-standard"
* the path completion for the "git add" command is provided using
"git ls-files --exclude-standard -o -m"
* the path completion for the "git clean" command is provided using
"git ls-files --exclude-standard -o"
* the path completion for the "git commit" command is provides using
"git diff-index --name-only HEAD"
Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
contrib/completion/git-completion.bash | 39 ++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 13 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b77eb1..8b348c3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,25 @@ __gitcomp_nl ()
COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
}
+__git_files ()
+{
+ local dir="$(__gitdir)"
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" ls-files --exclude-standard $*
+ return
+ fi
+}
+
+# Return all staged files with modification from current HEAD
+__git_commit_files ()
+{
+ local dir="$(__gitdir)"
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" diff-index --name-only HEAD
+ return
+ fi
+}
+
__git_heads ()
{
local dir="$(__gitdir)"
@@ -770,8 +789,6 @@ _git_apply ()
_git_add ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "
@@ -780,7 +797,8 @@ _git_add ()
"
return
esac
- COMPREPLY=()
+ # XXX should we care for --update and --all options ?
+ __gitcomp_nl "$(__git_files -o -m)" "" "$cur" ""
}
_git_archive ()
@@ -930,15 +948,14 @@ _git_cherry_pick ()
_git_clean ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--dry-run --quiet"
return
;;
esac
- COMPREPLY=()
+ # TODO: check for -x option
+ __gitcomp_nl "$(__git_files -o)" "" "$cur" ""
}
_git_clone ()
@@ -969,8 +986,6 @@ _git_clone ()
_git_commit ()
{
- __git_has_doubledash && return
-
case "$cur" in
--cleanup=*)
__gitcomp "default strip verbatim whitespace
@@ -998,7 +1013,7 @@ _git_commit ()
"
return
esac
- COMPREPLY=()
+ __gitcomp_nl "$(__git_commit_files)" "" "$cur" ""
}
_git_describe ()
@@ -1362,7 +1377,7 @@ _git_mv ()
return
;;
esac
- COMPREPLY=()
+ __gitcomp_nl "$(__git_files)" "" "$cur" ""
}
_git_name_rev ()
@@ -2068,15 +2083,13 @@ _git_revert ()
_git_rm ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
return
;;
esac
- COMPREPLY=()
+ __gitcomp_nl "$(__git_files)" "" "$cur" ""
}
_git_shortlog ()
--
1.8.1.rc1.18.g9db0d25
^ permalink raw reply related
* Re: [PATCH] Move api-command.txt to the end of API list in api-index.txt
From: Junio C Hamano @ 2012-12-16 20:01 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git
In-Reply-To: <1702872710.62174.1355660592713.JavaMail.ngmail@webmail12.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> - because it describes a different form of API than the other api-* documents
Drop that "- "; it is not like you are enumerating many reasons.
It makes me wonder if a more correct "fix" is to move this document
to the ../howto/ hierarchy.
>
> Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
> ---
> Documentation/technical/api-index.sh | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/api-index.sh b/Documentation/technical/api-index.sh
> index 9c3f413..c2c68ed 100755
> --- a/Documentation/technical/api-index.sh
> +++ b/Documentation/technical/api-index.sh
> @@ -10,12 +10,16 @@
> while read filename
> do
> case "$filename" in
> - api-index-skel.txt | api-index.txt) continue ;;
> + api-index-skel.txt | api-index.txt | api-command.txt) continue ;;
> esac
> title=$(sed -e 1q "$filename")
> html=${filename%.txt}.html
> echo "* link:$html[$title]"
> done
> + filename=api-command.txt
> + title=$(sed -e 1q "$filename")
> + html=${filename%.txt}.html
> + echo "* link:$html[$title]"
> echo "$c"
> sed -n -e '/^\/\/ table of contents end/,$p' "$skel"
> ) >api-index.txt+
^ permalink raw reply
* Re: [PATCH] Remove misleading date form api-index-skel.txt
From: Junio C Hamano @ 2012-12-16 19:59 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Thomas Ackermann, git
In-Reply-To: <m2ip828jtx.fsf@linux-m68k.org>
Andreas Schwab <schwab@linux-m68k.org> writes:
> s/form/from/
>
> Andreas.
Thanks; will apply.
^ permalink raw reply
* [PATCH v2 2/2] Makefile: detect when PYTHON_PATH changes
From: Christian Couder @ 2012-12-16 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When make is run, the python scripts are created from *.py files that
are changed to use the python given by PYTHON_PATH. And PYTHON_PATH
is set by default to /usr/bin/python on Linux.
This is nice except when you run make another time setting a
different PYTHON_PATH, because, as the python scripts have already
been created, make finds nothing to do.
The goal of this patch is to detect when the PYTHON_PATH changes and
to create the python scripts again when this happens. To do that we
use the same trick that is done to track other variables like prefix,
flags, tcl/tk path and shell path. We update a GIT-PYTHON-VARS file
with the PYTHON_PATH and check if it changed.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
.gitignore | 1 +
Makefile | 16 ++++++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/.gitignore b/.gitignore
index 6d69ae1..086c5af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
/GIT-CFLAGS
/GIT-LDFLAGS
/GIT-PREFIX
+/GIT-PYTHON-VARS
/GIT-SCRIPT-DEFINES
/GIT-USER-AGENT
/GIT-VERSION-FILE
diff --git a/Makefile b/Makefile
index 585b2eb..7db8445 100644
--- a/Makefile
+++ b/Makefile
@@ -2245,7 +2245,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
endif # NO_PERL
ifndef NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
$(QUIET_GEN)$(RM) $@ $@+ && \
INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
@@ -2624,6 +2624,18 @@ ifdef GIT_PERF_MAKE_OPTS
@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_MAKE_OPTS)))'\' >>$@
endif
+### Detect Python interpreter path changes
+ifndef NO_PYTHON
+TRACK_PYTHON = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
+
+GIT-PYTHON-VARS: FORCE
+ @VARS='$(TRACK_PYTHON)'; \
+ if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+ echo 1>&2 " * new Python interpreter location"; \
+ echo "$$VARS" >$@; \
+ fi
+endif
+
test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -2899,7 +2911,7 @@ ifndef NO_TCLTK
$(MAKE) -C git-gui clean
endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
- $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES
+ $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES GIT-PYTHON-VARS
.PHONY: all install profile-clean clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
--
1.8.1.rc1.2.g8740035
^ permalink raw reply related
* [PATCH v2 1/2] Makefile: remove tracking of TCLTK_PATH
From: Christian Couder @ 2012-12-16 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
It looks like we are tracking the value of TCLTK_PATH in the main
Makefile for no good reason, as this is done in git-gui too and the
GIT-GUI-VARS is not used in the Makefile.
This patch removes the useless code used to do this tracking.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
.gitignore | 1 -
Makefile | 14 +-------------
2 files changed, 1 insertion(+), 14 deletions(-)
diff --git a/.gitignore b/.gitignore
index f702415..6d69ae1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,7 +1,6 @@
/GIT-BUILD-OPTIONS
/GIT-CFLAGS
/GIT-LDFLAGS
-/GIT-GUI-VARS
/GIT-PREFIX
/GIT-SCRIPT-DEFINES
/GIT-USER-AGENT
diff --git a/Makefile b/Makefile
index 4ad6fbd..585b2eb 100644
--- a/Makefile
+++ b/Makefile
@@ -2624,18 +2624,6 @@ ifdef GIT_PERF_MAKE_OPTS
@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_MAKE_OPTS)))'\' >>$@
endif
-### Detect Tck/Tk interpreter path changes
-ifndef NO_TCLTK
-TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
-
-GIT-GUI-VARS: FORCE
- @VARS='$(TRACK_VARS)'; \
- if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
- echo 1>&2 " * new Tcl/Tk interpreter location"; \
- echo "$$VARS" >$@; \
- fi
-endif
-
test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -2910,7 +2898,7 @@ ifndef NO_TCLTK
$(MAKE) -C gitk-git clean
$(MAKE) -C git-gui clean
endif
- $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS GIT-BUILD-OPTIONS
+ $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES
.PHONY: all install profile-clean clean strip
--
1.8.1.rc1.2.g8740035
^ permalink raw reply related
* [PATCH 2/3] Documentation: move support for old compilers to CodingGuidelines
From: Adam Spiers @ 2012-12-16 19:36 UTC (permalink / raw)
To: git list
In-Reply-To: <1355686561-1057-1-git-send-email-git@adamspiers.org>
The "Try to be nice to older C compilers" text is clearly a guideline
to be borne in mind whilst coding rather than when submitting patches.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
Documentation/CodingGuidelines | 8 ++++++++
Documentation/SubmittingPatches | 13 -------------
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 57da6aa..69f7e9b 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -112,6 +112,14 @@ For C programs:
- We try to keep to at most 80 characters per line.
+ - We try to support a wide range of C compilers to compile git with,
+ including old ones. That means that you should not use C99
+ initializers, even if a lot of compilers grok it.
+
+ - Variables have to be declared at the beginning of the block.
+
+ - NULL pointers shall be written as NULL, not as 0.
+
- When declaring pointers, the star sides with the variable
name, i.e. "char *string", not "char* string" or
"char * string". This makes it easier to understand code
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index c107cb1..c34c9d1 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -127,19 +127,6 @@ in templates/hooks--pre-commit. To help ensure this does not happen,
run git diff --check on your changes before you commit.
-(1a) Try to be nice to older C compilers
-
-We try to support a wide range of C compilers to compile
-git with. That means that you should not use C99 initializers, even
-if a lot of compilers grok it.
-
-Also, variables have to be declared at the beginning of the block
-(you can check this with gcc, using the -Wdeclaration-after-statement
-option).
-
-Another thing: NULL pointers shall be written as NULL, not as 0.
-
-
(2) Generate your patch using git tools out of your commits.
git based diff tools generate unidiff which is the preferred format.
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
* [PATCH 3/3] Makefile: use -Wdeclaration-after-statement if supported
From: Adam Spiers @ 2012-12-16 19:36 UTC (permalink / raw)
To: git list
In-Reply-To: <1355686561-1057-1-git-send-email-git@adamspiers.org>
CodingGuidelines requests that code should be nice to older C compilers.
Since modern gcc can warn on code written using newer dialects such as C99,
it makes sense to take advantage of this by auto-detecting this capability
and enabling it when found.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
If we adopt this approach, it may make sense to enable other flags
where available (e.g. -Wzero-as-null-pointer-constant, maybe even
-ansi). In that case, something like this might be a more efficient
way of writing it:
GCC_FLAGS=-Wdeclaration-after-statement,-Wanother-flag,-Wand-another
GCC_FLAGS_REGEXP=$(shell echo $(GCC_FLAGS) | sed 's/,/\\|/g')
GCC_SUPPORTED_FLAGS=$(shell cc --help -v 2>&1 | \
sed -n '/.* \($(GCC_FLAGS_REGEXP)\) .*/{s//\1/;p}')
Makefile | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index a49d1db..aae70d4 100644
--- a/Makefile
+++ b/Makefile
@@ -331,8 +331,13 @@ endif
# CFLAGS and LDFLAGS are for the users to override from the command line.
CFLAGS = -g -O2 -Wall
+GCC_DECL_AFTER_STATEMENT = \
+ $(shell $(CC) --help -v 2>&1 | \
+ grep -q -- -Wdeclaration-after-statement && \
+ echo -Wdeclaration-after-statement)
+GCC_FLAGS = $(GCC_DECL_AFTER_STATEMENT)
+ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) $(GCC_FLAGS)
LDFLAGS =
-ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
STRIP ?= strip
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
* [PATCH 1/3] SubmittingPatches: add convention of prefixing commit messages
From: Adam Spiers @ 2012-12-16 19:35 UTC (permalink / raw)
To: git list
In-Reply-To: <1355686561-1057-1-git-send-email-git@adamspiers.org>
Conscientious newcomers to git development will read SubmittingPatches
and CodingGuidelines, but could easily miss the convention of
prefixing commit messages with a single word identifying the file
or area the commit touches.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
Documentation/SubmittingPatches | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 0dbf2c9..c107cb1 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -9,6 +9,14 @@ Checklist (and a short version for the impatient):
- the first line of the commit message should be a short
description (50 characters is the soft limit, see DISCUSSION
in git-commit(1)), and should skip the full stop
+ - it is also conventional in most cases to prefix the
+ first line with "area: " where the area is a filename
+ or identifier for the general area of the code being
+ modified, e.g.
+ . archive: ustar header checksum is computed unsigned
+ . git-cherry-pick.txt: clarify the use of revision range notation
+ (if in doubt which identifier to use, run "git log --no-merges"
+ on the files you are modifying to see the current conventions)
- the body should provide a meaningful commit message, which:
. explains the problem the change tries to solve, iow, what
is wrong with the current code without the change.
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
* [PATCH 0/3] Help newbie git developers avoid obvious pitfalls
From: Adam Spiers @ 2012-12-16 19:35 UTC (permalink / raw)
To: git list
In-Reply-To: <7vobl0804s.fsf@alter.siamese.dyndns.org>
I fell into various newbie pitfalls when submitting my first patches
to git, despite my best attempts to adhere to documented guidelines.
This small patch series attempts to reduce the chances of other
developers making the same mistakes I did.
Adam Spiers (3):
SubmittingPatches: add convention of prefixing commit messages
Documentation: move support for old compilers to CodingGuidelines
Makefile: use -Wdeclaration-after-statement if supported
Documentation/CodingGuidelines | 8 ++++++++
Documentation/SubmittingPatches | 21 ++++++++-------------
Makefile | 7 ++++++-
3 files changed, 22 insertions(+), 14 deletions(-)
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Adam Spiers @ 2012-12-16 19:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list
In-Reply-To: <7v8v8xrfnp.fsf@alter.siamese.dyndns.org>
On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> This series of commits attempts to make test output coloring
>> more intuitive,...
>
> Thanks; I understand that this is to replace the previous one
> b465316 (tests: paint unexpectedly fixed known breakages in bold
> red, 2012-09-19)---am I correct?
Correct. AFAICS I have incorporated all feedback raised in previous
reviews.
> Will take a look; thanks.
Thanks. Sorry again for the delay. I'm now (finally) resuming work
on as/check-ignore.
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Junio C Hamano @ 2012-12-16 18:54 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1355682495-22382-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> This series of commits attempts to make test output coloring
> more intuitive,...
Thanks; I understand that this is to replace the previous one
b465316 (tests: paint unexpectedly fixed known breakages in bold
red, 2012-09-19)---am I correct?
> - red is only used for things which have gone unexpectedly wrong:
> test failures, unexpected test passes, and failures with the
> framework,
>
> - yellow is only used for known breakages,
>
> - green is only used for things which have gone to plan and
> require no further work to be done,
>
> - blue is only used for skipped tests, and
>
> - cyan is used for other informational messages.
OK.
> Since unexpected test passes are no longer treated as passes, the
> summary lines displayed at the end of a test run have enough different
> possible outputs to warrant them being covered in the test framework's
> self-tests. Therefore this series also refactors and extends the
> self-tests.
>
> Adam Spiers (7):
> tests: test number comes first in 'not ok $count - $message'
> tests: paint known breakages in bold yellow
> tests: paint skipped tests in bold blue
> tests: change info messages from yellow/brown to bold cyan
> tests: refactor mechanics of testing in a sub test-lib
> tests: test the test framework more thoroughly
> tests: paint unexpectedly fixed known breakages in bold red
>
> t/t0000-basic.sh | 211 ++++++++++++++++++++++++++++++++++++++++++-------------
> t/test-lib.sh | 25 ++++---
> 2 files changed, 180 insertions(+), 56 deletions(-)
Will take a look; thanks.
^ permalink raw reply
* Re: [PATCH v2] Documentation: don't link to example mail addresses
From: Junio C Hamano @ 2012-12-16 18:48 UTC (permalink / raw)
To: John Keeping; +Cc: Jeff King, git
In-Reply-To: <20121216140029.GE2725@river.lan>
John Keeping <john@keeping.me.uk> writes:
> Email addresses in documentation are converted into mailto: hyperlinks
> in the HTML output and footnotes in man pages. This isn't desirable for
> cases where the address is used as an example and is not valid.
>
> Particularly annoying is the example "jane@laptop.(none)" which appears
> in git-shortlog(1) as "jane@laptop[1].(none)", with note 1 saying:
>
> 1. jane@laptop
> mailto:jane@laptop
>
> Fix this by escaping these email addresses with a leading backslash, to
> prevent Asciidoc expanding them as inline macros.
>
> In the case of mailmap.txt, render the address monospaced so that it
> matches the block examples surrounding that paragraph.
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: John Keeping <john@keeping.me.uk>
> ---
>
> On Sun, Dec 16, 2012 at 07:04:05AM -0500, Jeff King wrote:
>> Furthermore, the right way to suppress
>> expansion of macros is with a backslash escape.
> [snipped an example]
>> I think it's a little less ugly
>> than the "$$" quoting, but not by much. No clue if one is accepted by
>> more asciidoc versions or not.
>
> From a quick reading of the Asciidoc changelog, I think backslash
> escaping should be supported just as well as "$$" quoting, which leaves
> the minimal patch looking like this.
The patch looks reasonable to me, too. We were bitten by relying on
the description in AsciiDoc documentation (which shows the state of
their latest software) before; between the constructs that work, it
is safer to use the older, more vanilla and more common one.
Thanks.
^ permalink raw reply
* [PATCH v6 6/7] tests: test the test framework more thoroughly
From: Adam Spiers @ 2012-12-16 18:28 UTC (permalink / raw)
To: git list
In-Reply-To: <1355682495-22382-1-git-send-email-git@adamspiers.org>
Add 5 new full test suite runs each with a different number of
passing/failing/broken/fixed tests, in order to ensure that the
correct exit code and output are generated in each case. As before,
these are run in a subdirectory to avoid disrupting the metrics for
the parent tests.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
t/t0000-basic.sh | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index fc5200f..5c1dde0 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -79,6 +79,55 @@ check_sub_test_lib_test () {
)
}
+test_expect_success 'pretend we have a fully passing test suite' "
+ run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
+ for i in 1 2 3; do
+ test_expect_success \"passing test #\$i\" 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test full-pass <<-\\EOF
+ > ok 1 - passing test #1
+ > ok 2 - passing test #2
+ > ok 3 - passing test #3
+ > # passed all 3 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'pretend we have a partially passing test suite' "
+ test_must_fail run_sub_test_lib_test \
+ partial-pass '2/3 tests passing' <<-\\EOF &&
+ test_expect_success 'passing test #1' 'true'
+ test_expect_success 'failing test #2' 'false'
+ test_expect_success 'passing test #3' 'true'
+ test_done
+ EOF
+ check_sub_test_lib_test partial-pass <<-\\EOF
+ > ok 1 - passing test #1
+ > not ok 2 - failing test #2
+ # false
+ > ok 3 - passing test #3
+ > # failed 1 among 3 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'pretend we have a known breakage' "
+ run_sub_test_lib_test failing-todo 'A failing TODO test' <<-\\EOF &&
+ test_expect_success 'passing test' 'true'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_done
+ EOF
+ check_sub_test_lib_test failing-todo <<-\\EOF
+ > ok 1 - passing test
+ > not ok 2 - pretend we have a known breakage # TODO known breakage
+ > # still have 1 known breakage(s)
+ > # passed all remaining 1 test(s)
+ > 1..2
+ EOF
+"
+
test_expect_success 'pretend we have fixed a known breakage' "
run_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\EOF &&
test_expect_failure 'pretend we have fixed a known breakage' 'true'
@@ -92,6 +141,61 @@ test_expect_success 'pretend we have fixed a known breakage' "
EOF
"
+test_expect_success 'pretend we have a pass, fail, and known breakage' "
+ test_must_fail run_sub_test_lib_test \
+ mixed-results1 'mixed results #1' <<-\\EOF &&
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'failing test' 'false'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_done
+ EOF
+ check_sub_test_lib_test mixed-results1 <<-\\EOF
+ > ok 1 - passing test
+ > not ok 2 - failing test
+ > # false
+ > not ok 3 - pretend we have a known breakage # TODO known breakage
+ > # still have 1 known breakage(s)
+ > # failed 1 among remaining 2 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'pretend we have a mix of all possible results' "
+ test_must_fail run_sub_test_lib_test \
+ mixed-results2 'mixed results #2' <<-\\EOF &&
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'failing test' 'false'
+ test_expect_success 'failing test' 'false'
+ test_expect_success 'failing test' 'false'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_expect_failure 'pretend we have fixed a known breakage' 'true'
+ test_done
+ EOF
+ check_sub_test_lib_test mixed-results2 <<-\\EOF
+ > ok 1 - passing test
+ > ok 2 - passing test
+ > ok 3 - passing test
+ > ok 4 - passing test
+ > not ok 5 - failing test
+ > # false
+ > not ok 6 - failing test
+ > # false
+ > not ok 7 - failing test
+ > # false
+ > not ok 8 - pretend we have a known breakage # TODO known breakage
+ > not ok 9 - pretend we have a known breakage # TODO known breakage
+ > ok 10 - pretend we have fixed a known breakage # TODO known breakage
+ > # fixed 1 known breakage(s)
+ > # still have 2 known breakage(s)
+ > # failed 3 among remaining 8 test(s)
+ > 1..10
+ EOF
+"
+
test_set_prereq HAVEIT
haveit=no
test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
* [PATCH v6 7/7] tests: paint unexpectedly fixed known breakages in bold red
From: Adam Spiers @ 2012-12-16 18:28 UTC (permalink / raw)
To: git list
In-Reply-To: <1355682495-22382-1-git-send-email-git@adamspiers.org>
Change color of unexpectedly fixed known breakages to bold red. An
unexpectedly passing test indicates that the test code is somehow
broken or out of sync with the code it is testing. Either way this is
an error which is potentially as bad as a failing test, and as such is
no longer portrayed as a pass in the output.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
t/t0000-basic.sh | 30 ++++++++++++++++++++++++------
t/test-lib.sh | 13 +++++++++----
2 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 5c1dde0..bd6127f 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -134,13 +134,31 @@ test_expect_success 'pretend we have fixed a known breakage' "
test_done
EOF
check_sub_test_lib_test passing-todo <<-\\EOF
- > ok 1 - pretend we have fixed a known breakage # TODO known breakage
- > # fixed 1 known breakage(s)
- > # passed all 1 test(s)
+ > ok 1 - pretend we have fixed a known breakage # TODO known breakage vanished
+ > # 1 known breakage(s) vanished; please update test(s)
> 1..1
EOF
"
+test_expect_success 'pretend we have fixed one of two known breakages (run in sub test-lib)' "
+ run_sub_test_lib_test partially-passing-todos \
+ '2 TODO tests, one passing' <<-\\EOF &&
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_expect_success 'pretend we have a passing test' 'true'
+ test_expect_failure 'pretend we have fixed another known breakage' 'true'
+ test_done
+ EOF
+ check_sub_test_lib_test partially-passing-todos <<-\\EOF
+ > not ok 1 - pretend we have a known breakage # TODO known breakage
+ > ok 2 - pretend we have a passing test
+ > ok 3 - pretend we have fixed another known breakage # TODO known breakage vanished
+ > # 1 known breakage(s) vanished; please update test(s)
+ > # still have 1 known breakage(s)
+ > # passed all remaining 1 test(s)
+ > 1..3
+ EOF
+"
+
test_expect_success 'pretend we have a pass, fail, and known breakage' "
test_must_fail run_sub_test_lib_test \
mixed-results1 'mixed results #1' <<-\\EOF &&
@@ -188,10 +206,10 @@ test_expect_success 'pretend we have a mix of all possible results' "
> # false
> not ok 8 - pretend we have a known breakage # TODO known breakage
> not ok 9 - pretend we have a known breakage # TODO known breakage
- > ok 10 - pretend we have fixed a known breakage # TODO known breakage
- > # fixed 1 known breakage(s)
+ > ok 10 - pretend we have fixed a known breakage # TODO known breakage vanished
+ > # 1 known breakage(s) vanished; please update test(s)
> # still have 2 known breakage(s)
- > # failed 3 among remaining 8 test(s)
+ > # failed 3 among remaining 7 test(s)
> 1..10
EOF
"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5d9d0fc..b1acdfc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -308,7 +308,7 @@ test_failure_ () {
test_known_broken_ok_ () {
test_fixed=$(($test_fixed+1))
- say_color "" "ok $test_count - $@ # TODO known breakage"
+ say_color error "ok $test_count - $@ # TODO known breakage vanished"
}
test_known_broken_failure_ () {
@@ -406,13 +406,18 @@ test_done () {
if test "$test_fixed" != 0
then
- say_color pass "# fixed $test_fixed known breakage(s)"
+ say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
fi
if test "$test_broken" != 0
then
say_color warn "# still have $test_broken known breakage(s)"
- msg="remaining $(($test_count-$test_broken)) test(s)"
+ fi
+ if test "$test_broken" != 0 || test "$test_fixed" != 0
+ then
+ test_remaining=$(( $test_count - $test_broken - $test_fixed ))
+ msg="remaining $test_remaining test(s)"
else
+ test_remaining=$test_count
msg="$test_count test(s)"
fi
case "$test_failure" in
@@ -426,7 +431,7 @@ test_done () {
if test $test_external_has_tap -eq 0
then
- if test $test_count -gt 0
+ if test $test_remaining -gt 0
then
say_color pass "# passed all $msg"
fi
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
* [PATCH v6 0/7] make test output coloring more intuitive
From: Adam Spiers @ 2012-12-16 18:28 UTC (permalink / raw)
To: git list
This series of commits attempts to make test output coloring
more intuitive, so that:
- red is only used for things which have gone unexpectedly wrong:
test failures, unexpected test passes, and failures with the
framework,
- yellow is only used for known breakages,
- green is only used for things which have gone to plan and
require no further work to be done,
- blue is only used for skipped tests, and
- cyan is used for other informational messages.
Since unexpected test passes are no longer treated as passes, the
summary lines displayed at the end of a test run have enough different
possible outputs to warrant them being covered in the test framework's
self-tests. Therefore this series also refactors and extends the
self-tests.
Adam Spiers (7):
tests: test number comes first in 'not ok $count - $message'
tests: paint known breakages in bold yellow
tests: paint skipped tests in bold blue
tests: change info messages from yellow/brown to bold cyan
tests: refactor mechanics of testing in a sub test-lib
tests: test the test framework more thoroughly
tests: paint unexpectedly fixed known breakages in bold red
t/t0000-basic.sh | 211 ++++++++++++++++++++++++++++++++++++++++++-------------
t/test-lib.sh | 25 ++++---
2 files changed, 180 insertions(+), 56 deletions(-)
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply
* [PATCH v6 1/7] tests: test number comes first in 'not ok $count - $message'
From: Adam Spiers @ 2012-12-16 18:28 UTC (permalink / raw)
To: git list
In-Reply-To: <1355682495-22382-1-git-send-email-git@adamspiers.org>
The old output to say "not ok - 1 messsage" was working by accident
only because the test numbers are optional in TAP.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
t/t0000-basic.sh | 4 ++--
t/test-lib.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 562cf41..46ccda3 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -189,13 +189,13 @@ test_expect_success 'tests clean up even on failures' "
! test -s err &&
! test -f \"trash directory.failing-cleanup/clean-after-failure\" &&
sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF &&
- > not ok - 1 tests clean up even after a failure
+ > not ok 1 - tests clean up even after a failure
> # Z
> # touch clean-after-failure &&
> # test_when_finished rm clean-after-failure &&
> # (exit 1)
> # Z
- > not ok - 2 failure to clean up causes the test to fail
+ > not ok 2 - failure to clean up causes the test to fail
> # Z
> # test_when_finished \"(exit 2)\"
> # Z
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f50f834..d0b236f 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -298,7 +298,7 @@ test_ok_ () {
test_failure_ () {
test_failure=$(($test_failure + 1))
- say_color error "not ok - $test_count $1"
+ say_color error "not ok $test_count - $1"
shift
echo "$@" | sed -e 's/^/# /'
test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
* [PATCH v6 3/7] tests: paint skipped tests in bold blue
From: Adam Spiers @ 2012-12-16 18:28 UTC (permalink / raw)
To: git list
In-Reply-To: <1355682495-22382-1-git-send-email-git@adamspiers.org>
Skipped tests indicate incomplete test coverage. Whilst this is not a
test failure or other error, it's still not a complete success.
Other testsuite related software like automake, autotest and prove
seem to use blue for skipped tests, so let's follow suit.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 710f051..220b172 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -212,7 +212,7 @@ then
error)
tput bold; tput setaf 1;; # bold red
skip)
- tput bold; tput setaf 2;; # bold green
+ tput bold; tput setaf 4;; # bold blue
warn)
tput bold; tput setaf 3;; # bold brown/yellow
pass)
--
1.7.12.1.396.g53b3ea9
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox