* Re: [PATCH 1/8] Use %B for Split Subject/Body
From: greened @ 2013-01-01 22:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Techlive Zheng
In-Reply-To: <7vpq1pbg1k.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Also, please be careful about the subject line. I doubt that these
> 8 patches will stand out as relating to "contrib/subtree", when mixed
> in 200 line output of "git shortlog --no-merges".
Ah, ok. I'll be more careful next time.
-David
^ permalink raw reply
* Re: [BUG] two-way read-tree can write null sha1s into index
From: Junio C Hamano @ 2013-01-01 22:24 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git
In-Reply-To: <20121229205154.GA21058@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I think we need to update twoway_merge to recognize unmerged entries,
> which gives us two options:
>
> 1. Reject the merge.
>
> 2. Throw away the current unmerged entry in favor of the "new" entry
> (when old and new are the same, of course; otherwise we would
> reject).
>
> I think (2) is the right thing.
As "--reset" in "read-tree --reset -u A B" is a way to say "we know
we are based on A and we want to go to B no matter what", I agree we
should not reject the merge.
With -m instead of --reset, I am not sure what the right semantics
should be, though.
^ permalink raw reply
* Re: [PATCH 1/8] Use %B for Split Subject/Body
From: greened @ 2013-01-01 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Techlive Zheng
In-Reply-To: <7vtxr1bg4g.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> "David A. Greene" <greened@obbligato.org> writes:
>
>> From: Techlive Zheng <techlivezheng@gmail.com>
>>
>> Use %B to format the commit message and body to avoid an extra newline
>> if a commit only has a subject line.
>
> Is this an unconditional improvement, or is it generally an
> improvement but for some users it may be a regression? I am
> guessing it is the former but am just making sure.
The former.
>> Author: Techlive Zheng <techlivezheng@gmail.com>
>>
>> Signed-off-by: David A. Greene <greened@obbligato.org>
>
> Please don't do "Author: " which does not add anything new. That is
> what "From: " is for. Instead it needs to be a sign-off.
Ok. Unfortunately I sent a number of patches like that. Do you want me
to re-send them?
> Also, is that a real name, I have to wonder?
No idea. Not likely, I'd say.
-David
^ permalink raw reply
* Re: [PATCH 2/4] t9810: Do not use sed -i
From: Junio C Hamano @ 2013-01-01 22:12 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git, pw
In-Reply-To: <201301012240.37769.tboegi@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> sed -i is not portable on all systems.
> Use sed with different input and output files.
> Utilize a tmp file whenever needed
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
> ---
> t/t9810-git-p4-rcs.sh | 24 ++++++++++++++----------
> 1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
> index 0c2fc3e..5bf9291 100755
> --- a/t/t9810-git-p4-rcs.sh
> +++ b/t/t9810-git-p4-rcs.sh
> @@ -26,10 +26,8 @@ test_expect_success 'init depot' '
> line7
> line8
> EOF
> - cp filek fileko &&
> - sed -i "s/Revision/Revision: do not scrub me/" fileko
> - cp fileko file_text &&
> - sed -i "s/Id/Id: do not scrub me/" file_text
> + sed <filek "s/Revision/Revision: do not scrub me/" >fileko
> + sed <fileko "s/Id/Id: do not scrub me/" >file_text
Making it shorter and more correct ;-), which is good, but you are
losing the && chaining. Also it is more customary to have
redirection at the end, unless it is to redirect a numbered file
descriptor (e.g. "echo >&2 error message"). I.e.
sed "s/Revision/Revision: do not scrub me/" <filek >fileko &&
sed "s/Id/Id: do not scrub me/" <fileko >file_text
^ permalink raw reply
* Re: [PATCH] Documentation: full-ness of a bundle is significant for cloning
From: W. Trevor King @ 2013-01-01 21:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Kirill Brilliantov
In-Reply-To: <7vehi4a9za.fsf_-_@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 146 bytes --]
On Tue, Jan 01, 2013 at 01:07:05PM -0800, Junio C Hamano wrote:
> critical hint to tell readers why thhis particular bundle can be
s/thhis/this/
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Junio C Hamano @ 2013-01-01 22:07 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
In-Reply-To: <201301012240.10722.tboegi@web.de>
Torsten Bögershausen <tboegi@web.de> writes:
> Add the perl script "check-non-portable-shell.pl" to detect non-portable
> shell syntax
> Many systems use gnu tools which accept an extended syntax in shell scripts,
> which is not portable on all systems and causes the test suite to fail.
>
> To prevent contributors using e.g. Linux to add non-portable test code,
> "check-non-portable-shell.pl" is run as part of
> "make test" or "make in the t/ directory.
>
> "echo -n" is an example of a statement working on Linux,
> but not on e.g. Mac OS X.
>
> Beside "echo -n" we check for
> "sed -i",
> arrays in shell scripts (declare statement),
> "which" (use type instead),
> or "==" (bash style of =)
>
> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
What it checks looks like a good start, but the indentation of it
(and the log message) seems very screwed up.
I also have to wonder what's the false positive rate of this. When
you are preparing a new test, you would ideally want a mode that
checks only parts that you just added, without seeing noises from
existing violations and false positives from the part you did not
touch. Otherwise, it will be too cumbersome to run for developers,
and the check mechanism will end up used by nobody.
> +######################################################################
> +# Test t0000..t9999.sh for non portable shell scripts #
> +# Examples are "echo -n" or "sed -i" #
> +# This script can be called with one or more filenames as parameters #
> +#
> +######################################################################
Just a style thing (style requests are not optional, though ;-), but
these box comments are moderately annoying to read and extremely
annoying to modify. Writing it like this:
> +#
> +# Test t0000..t9999.sh for non portable shell scripts
> +# Examples are "echo -n" or "sed -i"
> +# This script can be called with one or more filenames as parameters
> +#
should be sufficiently loud.
^ permalink raw reply
* Re: [PATCH] Avoid using non-POSIX cp options
From: Junio C Hamano @ 2013-01-01 21:57 UTC (permalink / raw)
To: Ben Walton; +Cc: git
In-Reply-To: <1357071197-7927-1-git-send-email-bdwalton@gmail.com>
Thanks; I think I already have 2d3ac9a (t3600: Avoid "cp -a", which
is a GNUism, 2012-12-18)
^ permalink raw reply
* Re: [PATCH] Replace git-cvsimport with a rewrite that fixes major bugs.
From: Junio C Hamano @ 2013-01-01 21:54 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20130101172645.GA5506@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> The combination of git-cvsimport and cvsps had serious problems.
> Among these were:
>
> (1) Analysis of branchy repos was buggy in multiple ways in both
> programs, leading to incorrect repo translations.
>
> (2) Even after a correct branch analysis, extra (redundant) fileops
> would often be generated on the new-branch side.
>
> (3) Inability to report more than one tag pointing to the same revision.
>
> (4) Failure in certain cases of clock-skew reported by the t9603 test.
>
> (5) Failure to use commitids for changeset ordering in cases were this
> would have prevented clock skew from causing incorrect grouping.
>
> Problems 2-5 and portions of problem 1 have been solved by a major
> rewrite of cvsps (the 3.x release series); it now emits a git
> fast-import stream.
So..., is this a flag-day patch?
After this is merged, users who have been interoperating with CVS
repositories with the older cvsps have to install the updated cvsps
before using a new version of Git that ships with it? As long as
they update both cvsps and cvsimport, they can continue using the
existing repository to get updates from the same upstream CVS
repository without losing hisory continuity?
I would have preferred an addition of "git cvsimport-new" (or rename
of the existing one to "git cvsimport-old"), with additional tests
that compare the results of these two implemenations on simple CVS
history that cvsimport-old did *not* screw up, to ensure that (1)
people with existing set-up can choose to keep using the old one,
perhaps by tweaking their process to use cvsimport-old, and (2) the
updated one will give these people the identical conversion results,
as long as the history they have been interacting with do not have
the corner cases that trigger bugs in older cvsps.
Or am I being too conservative?
^ permalink raw reply
* Re: [PATCH 7/8] Ignore git-subtree
From: Junio C Hamano @ 2013-01-01 21:44 UTC (permalink / raw)
To: David A. Greene; +Cc: git, Michael Schubert
In-Reply-To: <1357012655-24974-8-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: Michael Schubert <mschub@elegosoft.com>
>
> Add the git-subtree command executable to .gitignore.
>
> Author: Michael Schubert <mschub@elegosoft.com>
>
> Signed-off-by: Michael Schubert <mschub@elegosoft.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
Seems sensible; I think I have this already in my tree.
> contrib/subtree/.gitignore | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/subtree/.gitignore b/contrib/subtree/.gitignore
> index 7e77c9d..91360a3 100644
> --- a/contrib/subtree/.gitignore
> +++ b/contrib/subtree/.gitignore
> @@ -1,4 +1,5 @@
> *~
> +git-subtree
> git-subtree.xml
> git-subtree.1
> mainline
^ permalink raw reply
* Re: [PATCH 6/8] Make the Manual Directory if Needed
From: Junio C Hamano @ 2013-01-01 21:44 UTC (permalink / raw)
To: David A. Greene; +Cc: git, Jesper L. Nielsen
In-Reply-To: <1357012655-24974-7-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: "Jesper L. Nielsen" <lyager@gmail.com>
>
> Before install git-subtree documentation, make sure the manpage
> directory exists.
>
> Author: Jesper L. Nielsen <lyager@gmail.com>
>
> Signed-off-by: Jesper L. Nielsen <lyager@gmail.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
> contrib/subtree/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
> index 36ae3e4..52d6fb9 100644
> --- a/contrib/subtree/Makefile
> +++ b/contrib/subtree/Makefile
> @@ -35,6 +35,7 @@ install: $(GIT_SUBTREE)
> install-doc: install-man
>
> install-man: $(GIT_SUBTREE_DOC)
> + mkdir -p $(man1dir)
We seem to use "$(INSTALL) -d -m 755" for this kind of thing (see
the Documentation/Makefile).
> $(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
>
> $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
^ permalink raw reply
* [PATCH 4/4] t9020: which is not portable
From: Torsten Bögershausen @ 2013-01-01 21:42 UTC (permalink / raw)
To: git; +Cc: tboegi
Use type instead
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t9020-remote-svn.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
index 4f2dfe0..dbaecbc 100755
--- a/t/t9020-remote-svn.sh
+++ b/t/t9020-remote-svn.sh
@@ -32,8 +32,8 @@ fi
test_debug '
git --version
- which git
- which svnrdump
+ type git
+ type svnrdump
'
test_expect_success REMOTE_SVN 'simple fetch' '
--
1.8.0.197.g5a90748
^ permalink raw reply related
* [PATCH 4/4] t9020: which is not portable
From: Torsten Bögershausen @ 2013-01-01 21:42 UTC (permalink / raw)
To: git; +Cc: tboegi
Use type instead
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t9020-remote-svn.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh
index 4f2dfe0..dbaecbc 100755
--- a/t/t9020-remote-svn.sh
+++ b/t/t9020-remote-svn.sh
@@ -32,8 +32,8 @@ fi
test_debug '
git --version
- which git
- which svnrdump
+ type git
+ type svnrdump
'
test_expect_success REMOTE_SVN 'simple fetch' '
--
1.8.0.197.g5a90748
^ permalink raw reply related
* Re: [PATCH 5/8] Honor DESTDIR
From: Junio C Hamano @ 2013-01-01 21:42 UTC (permalink / raw)
To: David A. Greene; +Cc: git, Adam Tkac
In-Reply-To: <1357012655-24974-6-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: Adam Tkac <atkac@redhat.com>
>
> Teach git-subtree's Makefile to honor DESTDIR.
>
> Author: Adam Tkac <atkac@redhat.com>
>
> Signed-off-by: Adam Tkac <atkac@redhat.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
The contents of the patch looks sensible; the above is questionable
as all the other messages in this series, though. Did any of our
tools cause this failure? If so I would like to know more about it.
> contrib/subtree/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
> index 05cdd5c..36ae3e4 100644
> --- a/contrib/subtree/Makefile
> +++ b/contrib/subtree/Makefile
> @@ -30,12 +30,12 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
> doc: $(GIT_SUBTREE_DOC)
>
> install: $(GIT_SUBTREE)
> - $(INSTALL) -m 755 $(GIT_SUBTREE) $(libexecdir)
> + $(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)
>
> install-doc: install-man
>
> install-man: $(GIT_SUBTREE_DOC)
> - $(INSTALL) -m 644 $^ $(man1dir)
> + $(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
>
> $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
> xmlto -m $(MANPAGE_NORMAL_XSL) man $^
^ permalink raw reply
* [PATCH 3/4] t4014: do not uese echo -n
From: Torsten Bögershausen @ 2013-01-01 21:41 UTC (permalink / raw)
To: git; +Cc: tboegi
echo -n is not portable on all systems.
Use printf instead
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t4014-format-patch.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 6cfad13..f460930 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -993,7 +993,7 @@ EOF
'
test_expect_success 'signoff: commit with only subject that does not end with NL' '
- echo -n subject | append_signoff >actual &&
+ printf subject | append_signoff >actual &&
cat >expected <<\EOF &&
4:Subject: [PATCH] subject
8:
--
1.8.0.197.g5a90748
^ permalink raw reply related
* [PATCH 2/4] t9810: Do not use sed -i
From: Torsten Bögershausen @ 2013-01-01 21:40 UTC (permalink / raw)
To: git, pw; +Cc: tboegi
sed -i is not portable on all systems.
Use sed with different input and output files.
Utilize a tmp file whenever needed
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t9810-git-p4-rcs.sh | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/t/t9810-git-p4-rcs.sh b/t/t9810-git-p4-rcs.sh
index 0c2fc3e..5bf9291 100755
--- a/t/t9810-git-p4-rcs.sh
+++ b/t/t9810-git-p4-rcs.sh
@@ -26,10 +26,8 @@ test_expect_success 'init depot' '
line7
line8
EOF
- cp filek fileko &&
- sed -i "s/Revision/Revision: do not scrub me/" fileko
- cp fileko file_text &&
- sed -i "s/Id/Id: do not scrub me/" file_text
+ sed <filek "s/Revision/Revision: do not scrub me/" >fileko
+ sed <fileko "s/Id/Id: do not scrub me/" >file_text
p4 add -t text+k filek &&
p4 submit -d "filek" &&
p4 add -t text+ko fileko &&
@@ -88,7 +86,8 @@ test_expect_success 'edit far away from RCS lines' '
(
cd "$git" &&
git config git-p4.skipSubmitEdit true &&
- sed -i "s/^line7/line7 edit/" filek &&
+ sed <filek "s/^line7/line7 edit/" >filek.tmp &&
+ mv -f filek.tmp filek &&
git commit -m "filek line7 edit" filek &&
git p4 submit &&
scrub_k_check filek
@@ -105,7 +104,8 @@ test_expect_success 'edit near RCS lines' '
cd "$git" &&
git config git-p4.skipSubmitEdit true &&
git config git-p4.attemptRCSCleanup true &&
- sed -i "s/^line4/line4 edit/" filek &&
+ sed <filek "s/^line4/line4 edit/" >filek.tmp &&
+ mv -f filek.tmp filek &&
git commit -m "filek line4 edit" filek &&
git p4 submit &&
scrub_k_check filek
@@ -122,7 +122,8 @@ test_expect_success 'edit keyword lines' '
cd "$git" &&
git config git-p4.skipSubmitEdit true &&
git config git-p4.attemptRCSCleanup true &&
- sed -i "/Revision/d" filek &&
+ sed <filek "/Revision/d" >filek.tmp &&
+ mv -f filek.tmp filek &&
git commit -m "filek remove Revision line" filek &&
git p4 submit &&
scrub_k_check filek
@@ -139,7 +140,8 @@ test_expect_success 'scrub ko files differently' '
cd "$git" &&
git config git-p4.skipSubmitEdit true &&
git config git-p4.attemptRCSCleanup true &&
- sed -i "s/^line4/line4 edit/" fileko &&
+ sed <fileko "s/^line4/line4 edit/" >fileko.tmp &&
+ mv -f fileko.tmp fileko &&
git commit -m "fileko line4 edit" fileko &&
git p4 submit &&
scrub_ko_check fileko &&
@@ -189,12 +191,14 @@ test_expect_success 'do not scrub plain text' '
cd "$git" &&
git config git-p4.skipSubmitEdit true &&
git config git-p4.attemptRCSCleanup true &&
- sed -i "s/^line4/line4 edit/" file_text &&
+ sed <file_text "s/^line4/line4 edit/" >file_text.tmp &&
+ mv -f file_text.tmp file_text &&
git commit -m "file_text line4 edit" file_text &&
(
cd "$cli" &&
p4 open file_text &&
- sed -i "s/^line5/line5 p4 edit/" file_text &&
+ sed <file_text "s/^line5/line5 p4 edit/" >file_text.tmp &&
+ mv -f file_text.tmp file_text &&
p4 submit -d "file5 p4 edit"
) &&
echo s | test_expect_code 1 git p4 submit &&
--
1.8.0.197.g5a90748
^ permalink raw reply related
* Re: [PATCH 4/8] Fix Synopsis
From: Junio C Hamano @ 2013-01-01 21:40 UTC (permalink / raw)
To: David A. Greene; +Cc: git
In-Reply-To: <1357012655-24974-5-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: "David A. Greene" <greened@obbligato.org>
>
> Fix the documentation of add to show that a repository can be
> specified along with a commit.
>
> Change "commit" to "refspec" in the synopsis for add.
>
> Suggested by Yann Dirson <dirson@bertin.fr>.
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
> contrib/subtree/git-subtree.sh | 3 ++-
> contrib/subtree/git-subtree.txt | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index b8a807a..ad62dfb 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -8,7 +8,8 @@ if [ $# -eq 0 ]; then
> set -- -h
> fi
> OPTS_SPEC="\
> -git subtree add --prefix=<prefix> <commit>
> +git subtree add --prefix=<prefix> <refspec>
Again, this is not <refspec> but <commit>.
> +git subtree add --prefix=<prefix> <repository> <refspec>
This is given to "fetch" and it seems to acccept any <refspec>, so
it is probably a good change (I didn't fully follow the codepath,
though).
> git subtree merge --prefix=<prefix> <commit>
> git subtree pull --prefix=<prefix> <repository> <refspec...>
> git subtree push --prefix=<prefix> <repository> <refspec...>
> diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
> index ae420aa..89c2d6e 100644
> --- a/contrib/subtree/git-subtree.txt
> +++ b/contrib/subtree/git-subtree.txt
> @@ -9,7 +9,8 @@ git-subtree - Merge subtrees together and split repository into subtrees
> SYNOPSIS
> --------
> [verse]
> -'git subtree' add -P <prefix> <commit>
> +'git subtree' add -P <prefix> <refspec>
> +'git subtree' add -P <prefix> <repository> <refspec>
> 'git subtree' pull -P <prefix> <repository> <refspec...>
> 'git subtree' push -P <prefix> <repository> <refspec...>
> 'git subtree' merge -P <prefix> <commit>
^ permalink raw reply
* [PATCH 1/4] test: Add target test-lint-shell-syntax
From: Torsten Bögershausen @ 2013-01-01 21:40 UTC (permalink / raw)
To: git; +Cc: tboegi
Add the perl script "check-non-portable-shell.pl" to detect non-portable
shell syntax
Many systems use gnu tools which accept an extended syntax in shell scripts,
which is not portable on all systems and causes the test suite to fail.
To prevent contributors using e.g. Linux to add non-portable test code,
"check-non-portable-shell.pl" is run as part of
"make test" or "make in the t/ directory.
"echo -n" is an example of a statement working on Linux,
but not on e.g. Mac OS X.
Beside "echo -n" we check for
"sed -i",
arrays in shell scripts (declare statement),
"which" (use type instead),
or "==" (bash style of =)
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/Makefile | 7 +++--
t/check-non-portable-shell.pl | 67 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 2 deletions(-)
create mode 100755 t/check-non-portable-shell.pl
diff --git a/t/Makefile b/t/Makefile
index 88e289f..7b0c4dc 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -23,7 +23,7 @@ TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
all: $(DEFAULT_TEST_TARGET)
-test: pre-clean $(TEST_LINT)
+test: pre-clean test-lint-shell-syntax $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
prove: pre-clean $(TEST_LINT)
@@ -43,7 +43,7 @@ clean-except-prove-cache:
clean: clean-except-prove-cache
$(RM) .prove
-test-lint: test-lint-duplicates test-lint-executable
+test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax
test-lint-duplicates:
@dups=`echo $(T) | tr ' ' '\n' | sed 's/-.*//' | sort | uniq -d` && \
@@ -55,6 +55,9 @@ test-lint-executable:
test -z "$$bad" || { \
echo >&2 "non-executable tests:" $$bad; exit 1; }
+test-lint-shell-syntax:
+ $(PERL_PATH) check-non-portable-shell.pl $(T)
+
aggregate-results-and-cleanup: $(T)
$(MAKE) aggregate-results
$(MAKE) clean
diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
new file mode 100755
index 0000000..de62ef0
--- /dev/null
+++ b/t/check-non-portable-shell.pl
@@ -0,0 +1,67 @@
+#!/usr/bin/perl -w
+######################################################################
+# Test t0000..t9999.sh for non portable shell scripts #
+# Examples are "echo -n" or "sed -i" #
+# This script can be called with one or more filenames as parameters #
+#
+######################################################################
+use strict;
+my $exitcode=0;
+
+sub check_one_file($) {
+ my $lineno=1;
+ my $filename=shift;
+
+ open(FINPUT, "<$filename") || die "Couldn't open filename $filename";
+ my @fdata = <FINPUT>;
+ close(FINPUT);
+
+ while (my $line = shift @fdata) {
+ do {
+ chomp $line;
+ # sed -i
+ if ($line =~ /^\s*sed\s+-i/) {
+ printf("%s:%d:error: \"sed -i not portable\" %s\n", $filename, $lineno, $line);
+ $exitcode=1;
+ }
+ # echo -n
+ if ($line =~ /^\s*echo\s+-n/) {
+ printf("%s:%d:error: \"echo -n not portable\" %s\n", $filename, $lineno, $line);
+ $exitcode=1;
+ }
+ # arrays (declare statement)
+ if ($line =~ /^\s*declare\s+/) {
+ printf("%s:%d:error: \"arrays/declare not portable\" %s\n", $filename, $lineno, $line);
+ $exitcode=1;
+ }
+ # which
+ if ($line =~ /^\s*[^#]\s*which\s/) {
+ printf("%s:%d:error: \"which is not portable (use type)\" %s\n", $filename, $lineno, $line);
+ $exitcode=1;
+ }
+
+ # == (bash style comparison)
+ if ($line =~ /test\s+[^=]*==/) {
+ printf("%s:%d:error: \"== is not portable (use =)\" %s\n", $filename, $lineno, $line);
+ $exitcode=1;
+ }
+
+ $lineno=$lineno+1;
+ }
+ }
+}
+
+
+if ($#ARGV <= 0) {
+ print STDERR "$0: Check shell scripts for non portable syntax\n";
+ print STDERR "Example: $0 t[0-9]*.sh\n";
+
+ exit(2);
+}
+
+while (@ARGV) {
+ my $arg = shift @ARGV;
+ check_one_file($arg);
+}
+
+exit($exitcode);
--
1.8.0.197.g5a90748
^ permalink raw reply related
* Re: [PATCH 3/8] Better Error Handling for add
From: Junio C Hamano @ 2013-01-01 21:39 UTC (permalink / raw)
To: David A. Greene; +Cc: git
In-Reply-To: <1357012655-24974-4-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: "David A. Greene" <greened@obbligato.org>
>
> Check refspecs for validity before passing them on to other commands.
> This lets us generate more helpful error messages.
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
> contrib/subtree/git-subtree.sh | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 7ceb413..b8a807a 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -509,8 +509,20 @@ cmd_add()
> ensure_clean
>
> if [ $# -eq 1 ]; then
> + ref=$(git check-ref-format --normalize "refs/heads/$1") ||
> + die "'$1' is not a valid refspec. Are you missing a branch?"
Is a user forbidden from passing a commit that is not at the tip of
an existing branch? In other words, is
$ subtree add origin/next~4^2
forbidden?
> + rev=$(git rev-parse --verify $1) ||
> + die "'$1' is not a valid refspec. Are you missing a branch?"
> +
> "cmd_add_commit" "$@"
If you want to make sure you give a comit to add_commit, you can
probably say something like this:
git rev-parse -q --verify "$1^{commit}" >/dev/null ||
die "'$1' does not refer to a commit"
> elif [ $# -eq 2 ]; then
> + ref=$(git check-ref-format --normalize "refs/heads/$2") ||
> + die "'$2' is not a valid refspec."
> +
> + rev=$(git rev-parse --verify $2) ||
> + die "'$2' is not a valid refspec."
> +
Likewise.
> "cmd_add_repository" "$@"
> else
> say "error: parameters were '$@'"
^ permalink raw reply
* Re: [PATCH 2/8] Add --unannotate
From: Junio C Hamano @ 2013-01-01 21:30 UTC (permalink / raw)
To: David A. Greene; +Cc: git, James Nylen
In-Reply-To: <1357012655-24974-3-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: James Nylen <jnylen@gmail.com>
>
> Teach git-subtree about --unannotate. This option strips a prefix
> from a commit message when doing a subtree split.
Hrm. This looks like a workaround for a short-sighted misdesign of
the annotate option that only allowed prefixing a fixed string. I
have to wonder if it is better to deprecate --annotate and replace
it with a more general "commit log rewriting" facility that can
cover both use cases?
^ permalink raw reply
* Re: [PATCH 1/8] Use %B for Split Subject/Body
From: Junio C Hamano @ 2013-01-01 21:25 UTC (permalink / raw)
To: David A. Greene; +Cc: git, Techlive Zheng
In-Reply-To: <1357012655-24974-2-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> Subject: Re: [PATCH 1/8] Use %B for Split Subject/Body
This needs to say "contrib/subtree" somewhere (applies to all
patches in this series).
> From: Techlive Zheng <techlivezheng@gmail.com>
>
> Use %B to format the commit message and body to avoid an extra newline
> if a commit only has a subject line.
>
> Author: Techlive Zheng <techlivezheng@gmail.com>
This needs to be a S-o-b instead; is it a real name, by the way?
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
> contrib/subtree/git-subtree.sh | 5 +++
> contrib/subtree/t/t7900-subtree.sh | 73 ++++++++++++++++++++++--------------
> 2 files changed, 49 insertions(+), 29 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 920c664..f2b6d4a 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -296,7 +296,12 @@ copy_commit()
> # We're going to set some environment vars here, so
> # do it in a subshell to get rid of them safely later
> debug copy_commit "{$1}" "{$2}" "{$3}"
> + # Use %B rather than %s%n%n%b to handle the special case of a
> + # commit that only has a subject line. We don't want to
> + # introduce a newline after the subject, causing generation of
> + # a new hash.
> git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
> +# git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |
Is it really replacing %s%n%n%b with %B, or is it still an
experiment that is disabled?
> (
> read GIT_AUTHOR_NAME
> read GIT_AUTHOR_EMAIL
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index bc2eeb0..93eeb09 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -76,6 +76,10 @@ test_expect_success 'add sub1' '
> git branch -m master subproj
> '
>
> +# Save this hash for testing later.
> +
> +subdir_hash=`git rev-parse HEAD`
> +
We prefer $() over ``; much more readable.
> # 3
> test_expect_success 'add sub2' '
> create sub2 &&
> @@ -155,7 +159,6 @@ test_expect_success 'add main-sub5' '
> create subdir/main-sub5 &&
> git commit -m "main-sub5"
> '
> -
> # 15
> test_expect_success 'add main6' '
> create main6 &&
Why?
> @@ -235,7 +238,19 @@ test_expect_success 'check split with --branch' '
> check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
Is quoting screwed up around here (and in many other places in this
patch)? What are these no-op '' doing?
> '
>
> -# 25
> +#25
Why the lossage of a SP?
It may make sense to lose these "# num" that will have to be touched
every time somebody inserts new test pieces in the middle, as a
preparatory step before any of these patches, by the way. That will
reduce noise in the patches for real changes.
^ permalink raw reply
* [PATCH] Avoid using non-POSIX cp options
From: Ben Walton @ 2013-01-01 20:13 UTC (permalink / raw)
To: gitster; +Cc: git, Ben Walton
t/3600-rm was using the -a option to cp. This option is a GNU extention
and is not portable. Instead, use just -R (no -p necessary).
Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
t/t3600-rm.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 06f6384..37bf5f1 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -474,7 +474,7 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director
git submodule update &&
(cd submod &&
rm .git &&
- cp -a ../.git/modules/sub .git &&
+ cp -R ../.git/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git merge conflict2 &&
@@ -508,7 +508,7 @@ test_expect_success 'rm of a populated submodule with a .git directory fails eve
git submodule update &&
(cd submod &&
rm .git &&
- cp -a ../.git/modules/sub .git &&
+ cp -R ../.git/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git rm submod &&
@@ -606,7 +606,7 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
git submodule update --recursive &&
(cd submod/subsubmod &&
rm .git &&
- cp -a ../../.git/modules/sub/modules/sub .git &&
+ cp -R ../../.git/modules/sub/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git rm submod &&
--
1.7.10.4
^ permalink raw reply related
* [PATCH] Documentation: full-ness of a bundle is significant for cloning
From: Junio C Hamano @ 2013-01-01 21:07 UTC (permalink / raw)
To: git; +Cc: Kirill Brilliantov
In-Reply-To: <7vvcbgaapq.fsf@alter.siamese.dyndns.org>
Not necessarily every bundle file can be cloned from. Only the ones
that do not need prerequisites can.
When 1d52b02 (Documentation: minor grammatical fixes and rewording
in git-bundle.txt, 2009-03-22) reworded this paragraph, it lost a
critical hint to tell readers why thhis particular bundle can be
cloned from. Resurrect it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* As I noticed this while I was looking at Kirill's patch...
Documentation/git-bundle.txt | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt
index 5c8ba44..bc023cc 100644
--- a/Documentation/git-bundle.txt
+++ b/Documentation/git-bundle.txt
@@ -112,10 +112,9 @@ machineA$ git bundle create file.bundle master
machineA$ git tag -f lastR2bundle master
----------------
-Then you transfer file.bundle to the target machine B. If you are creating
-the repository on machine B, then you can clone from the bundle as if it
-were a remote repository instead of creating an empty repository and then
-pulling or fetching objects from the bundle:
+Then you transfer file.bundle to the target machine B. Because this
+bundle does not require any existing object to be extracted, you can
+create a new repository on machine B by cloning from it:
----------------
machineB$ git clone -b master /home/me/tmp/file.bundle R2
--
1.8.1.209.gc32ab23
^ permalink raw reply related
* Re: git filter-branch doesn't dereference annotated tags
From: Junio C Hamano @ 2013-01-01 21:04 UTC (permalink / raw)
To: Grégory Pakosz; +Cc: git, Johannes Sixt
In-Reply-To: <CAC_01E3VWtsFd8ww+7W8DMhRAs4WgHf=bm+xoh9wszCkb-DfUA@mail.gmail.com>
Grégory Pakosz <gpakosz@visionobjects.com> writes:
> Are you suggesting $sha1 should be obtained differently before
> entering case "$rewritten" ?
> That would mean changing sha1=$(git rev-parse "$ref"^0) at line 376 to
> something like $(git cat-file -t "$ref") = 'tag' && sha1=$(git
> rev-parse "$ref") || sha1=$(git rev-parse "$ref^0") ?
I was wondering if it should be
sha1=$(git rev-parse --verify "$ref")
or something that does not dereference a tag at all.
The way I read what that loop seems to want to do is:
Read each refname that was given originally from the file
$tempdir/heads, find out the object it used to refer to and
have it in $sha1, find out what new object the object was
rewritten to and have it in $rewritten, and:
(1) if the rewrite left the object unchanged, do nothing but
warn users just in case this was a mistake;
(2) if the rewrite told us to remove it, then delete the
ref; or
(3) if the rewrite gave us a new object, replace the ref to
point to that new one.
And in the latter two cases, save the original one in
$orig_namespace so that the user can choose to recover if
this filter-branch was done by mistake.
So I do not think unwraping the ref at that point makes any sense,
unless it is not prepared to handle annotated tags at all by
unwrapping tags too early.
What am I missing?
^ permalink raw reply
* Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API
From: Junio C Hamano @ 2013-01-01 20:52 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <1356575558-2674-3-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
> index 0356d25..944fc39 100644
> --- a/Documentation/technical/api-directory-listing.txt
> +++ b/Documentation/technical/api-directory-listing.txt
> @@ -9,8 +9,11 @@ Data structure
> --------------
>
> `struct dir_struct` structure is used to pass directory traversal
> -options to the library and to record the paths discovered. The notable
> -options are:
> +options to the library and to record the paths discovered. A single
> +`struct dir_struct` is used regardless of whether or not the traversal
> +recursively descends into subdirectories.
I am somewhat lukewarm on this part of the change.
The added "regardless of..." does not seem to add as much value as
the two extra lines the patch spends. If we say something like:
A `struct dir_struct` structure is used to pass options to
traverse directories recursively, and to record all the
paths discovered by the traversal.
it might be much more direct and informative, I suspect, though.
After all, the word "traversal" pretty much implies that the library
goes in and out of the directories recursively.
> @@ -39,7 +42,7 @@ options are:
> If set, recurse into a directory that looks like a git
> directory. Otherwise it is shown as a directory.
>
> -The result of the enumeration is left in these fields::
> +The result of the enumeration is left in these fields:
Good eyes.
> diff --git a/dir.c b/dir.c
> index ee8e711..89e27a6 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2,6 +2,8 @@
> * This handles recursive filename detection with exclude
> * files, index knowledge etc..
> *
> + * See Documentation/technical/api-directory-listing.txt
> + *
> * Copyright (C) Linus Torvalds, 2005-2006
> * Junio Hamano, 2005-2006
> */
> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)
> die("cannot use %s as an exclude file", fname);
> }
>
> +/*
> + * Loads the per-directory exclude list for the substring of base
> + * which has a char length of baselen.
> + */
> static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> {
> struct exclude_list *el;
> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
> (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
> return; /* too long a path -- ignore */
>
> - /* Pop the ones that are not the prefix of the path being checked. */
> + /* Pop the directories that are not the prefix of the path being checked. */
The "one" does not refer to a "directory", but to an "exclude-list".
Pop the ones that are not for parent directories of the path
being checked
perhaps?
^ permalink raw reply
* Re: [PATCH] Correct example restore from bundle
From: Junio C Hamano @ 2013-01-01 20:51 UTC (permalink / raw)
To: Kirill Brilliantov; +Cc: git
In-Reply-To: <1357053973.22772208@f75.mail.ru>
Kirill Brilliantov <brilliantov@inbox.ru> writes:
> ----------------
> -machineB$ git clone /home/me/tmp/file.bundle R2
> +machineB$ git clone /home/me/tmp/file.bundle R2 -b master
I think the command line should follow the convention that is
suggested in "git clone -h" output, i.e.
git clone -b master /home/me/tmp/file.bundle R2
I also think 'git bundle create" should record HEAD that points at
the only branch it is packing so the person who clones from it
should not say which branch, and when that is done this patch will
become unnecessary, but that is a separate topic.
Thanks; I'll queue this after rewording some.
-- >8 --
From: Kirill Brilliantov <brilliantov@inbox.ru>
Date: Tue, 1 Jan 2013 17:54:44 +0400
Subject: [PATCH] Documentation: correct example restore from bundle
Because the bundle created in the example does not record HEAD, "git
clone" will not check out the files to the working tree:
$ git clone pr.bundle q/
Cloning into 'q'...
Receiving objects: 100% (619/619), 13.52 MiB | 18.74 MiB/s, done.
Resolving deltas: 100% (413/413), done.
warning: remote HEAD refers to nonexistent ref, unable to checkout.
Avoid alarming the readers by adding "-b master" to the example. A
better fix may be to arrange the bundle created in the earlier step
to record HEAD, so that it can be cloned without this workaround.
Signed-off-by: Brilliantov Kirill Vladimirovich <brilliantov@inbox.ru>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-bundle.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt
index 16a6b0a..5c8ba44 100644
--- a/Documentation/git-bundle.txt
+++ b/Documentation/git-bundle.txt
@@ -118,7 +118,7 @@ were a remote repository instead of creating an empty repository and then
pulling or fetching objects from the bundle:
----------------
-machineB$ git clone /home/me/tmp/file.bundle R2
+machineB$ git clone -b master /home/me/tmp/file.bundle R2
----------------
This will define a remote called "origin" in the resulting repository that
--
1.8.1.209.gc32ab23
^ 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