* [PATCH] mergetool: Use args as pathspec to unmerged files
From: Jonathon Mah @ 2011-09-16 2:12 UTC (permalink / raw)
To: git; +Cc: Dan McGee, David Aguilar, Junio C Hamano
In-Reply-To: <7vsjnysuyl.fsf@alter.siamese.dyndns.org>
Mergetool now treats its path arguments as a pathspec (like other git
subcommands), restricting action to the given files and directories.
Files matching the pathspec are filtered so mergetool only acts on
unmerged paths; previously it would assume each path argument was in an
unresolved state, and get confused when it couldn't check out their
other stages.
Running "git mergetool subdir" will prompt to resolve all conflicted
blobs under subdir.
Signed-off-by: Jonathon Mah <me@JonathonMah.com>
---
With Junio's change, I'm happy with the code in git-mergetool.sh.
I've tried to clarify the commit message since the first submission.
Documentation/git-mergetool.txt | 7 ++--
git-mergetool.sh | 76 ++++++++++++++------------------------
t/t7610-mergetool.sh | 58 ++++++++++++++++++++---------
3 files changed, 72 insertions(+), 69 deletions(-)
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 3470910..2a49de7 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -17,9 +17,10 @@ Use `git mergetool` to run one of several merge utilities to resolve
merge conflicts. It is typically run after 'git merge'.
If one or more <file> parameters are given, the merge tool program will
-be run to resolve differences on each file. If no <file> names are
-specified, 'git mergetool' will run the merge tool program on every file
-with merge conflicts.
+be run to resolve differences on each file (skipping those without
+conflicts). Specifying a directory will include all unresolved files in
+that path. If no <file> names are specified, 'git mergetool' will run
+the merge tool program on every file with merge conflicts.
OPTIONS
-------
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 3c157bc..12a2706 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -342,64 +342,44 @@ merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo fa
last_status=0
rollup_status=0
-rerere=false
-
-files_to_merge() {
- if test "$rerere" = true
- then
- git rerere remaining
- else
- git ls-files -u | sed -e 's/^[^ ]* //' | sort -u
- fi
-}
-
+files=
if test $# -eq 0 ; then
cd_to_toplevel
if test -e "$GIT_DIR/MERGE_RR"
then
- rerere=true
+ files=$(git rerere remaining)
+ else
+ files=$(git ls-files -u | sed -e 's/^[^ ]* //' | sort -u)
fi
+else
+ files=$(git ls-files -u -- "$@" | sed -e 's/^[^ ]* //' | sort -u)
+fi
- files=$(files_to_merge)
- if test -z "$files" ; then
- echo "No files need merging"
- exit 0
- fi
+if test -z "$files" ; then
+ echo "No files need merging"
+ exit 0
+fi
- # Save original stdin
- exec 3<&0
+# Save original stdin
+exec 3<&0
- printf "Merging:\n"
- printf "$files\n"
+printf "Merging:\n"
+printf "$files\n"
- files_to_merge |
- while IFS= read i
- do
- if test $last_status -ne 0; then
- prompt_after_failed_merge <&3 || exit 1
- fi
- printf "\n"
- merge_file "$i" <&3
- last_status=$?
- if test $last_status -ne 0; then
- rollup_status=1
- fi
- done
-else
- while test $# -gt 0; do
- if test $last_status -ne 0; then
- prompt_after_failed_merge || exit 1
- fi
- printf "\n"
- merge_file "$1"
- last_status=$?
- if test $last_status -ne 0; then
- rollup_status=1
- fi
- shift
- done
-fi
+IFS='
+'; for i in $files
+do
+ if test $last_status -ne 0; then
+ prompt_after_failed_merge <&3 || exit 1
+ fi
+ printf "\n"
+ merge_file "$i" <&3
+ last_status=$?
+ if test $last_status -ne 0; then
+ rollup_status=1
+ fi
+done
exit $rollup_status
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index cbc08e3..4aab2a7 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -16,6 +16,7 @@ Testing basic merge tool invocation'
test_expect_success 'setup' '
git config rerere.enabled true &&
echo master >file1 &&
+ echo master spaced >"spaced name" &&
echo master file11 >file11 &&
echo master file12 >file12 &&
echo master file13 >file13 &&
@@ -30,13 +31,14 @@ test_expect_success 'setup' '
git commit -m "Add foo"
) &&
git submodule add git://example.com/submod submod &&
- git add file1 file1[1-4] subdir/file3 .gitmodules submod &&
+ git add file1 "spaced name" file1[1-4] subdir/file3 .gitmodules submod &&
git commit -m "add initial versions" &&
git checkout -b branch1 master &&
git submodule update -N &&
echo branch1 change >file1 &&
echo branch1 newfile >file2 &&
+ echo branch1 spaced >"spaced name" &&
echo branch1 change file11 >file11 &&
echo branch1 change file13 >file13 &&
echo branch1 sub >subdir/file3 &&
@@ -47,7 +49,7 @@ test_expect_success 'setup' '
git commit -m "Add bar on branch1" &&
git checkout -b submod-branch1
) &&
- git add file1 file11 file13 file2 subdir/file3 submod &&
+ git add file1 "spaced name" file11 file13 file2 subdir/file3 submod &&
git rm file12 &&
git commit -m "branch1 changes" &&
@@ -55,6 +57,7 @@ test_expect_success 'setup' '
git submodule update -N &&
echo master updated >file1 &&
echo master new >file2 &&
+ echo master updated spaced >"spaced name" &&
echo master updated file12 >file12 &&
echo master updated file14 >file14 &&
echo master new sub >subdir/file3 &&
@@ -65,7 +68,7 @@ test_expect_success 'setup' '
git commit -m "Add bar on master" &&
git checkout -b submod-master
) &&
- git add file1 file12 file14 file2 subdir/file3 submod &&
+ git add file1 "spaced name" file12 file14 file2 subdir/file3 submod &&
git rm file11 &&
git commit -m "master updates" &&
@@ -78,8 +81,8 @@ test_expect_success 'custom mergetool' '
git checkout -b test1 branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
- ( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
- ( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file1 ) &&
+ ( yes "" | git mergetool file2 "spaced name" >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
@@ -97,6 +100,7 @@ test_expect_success 'mergetool crlf' '
test_must_fail git merge master >/dev/null 2>&1 &&
( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool "spaced name" >/dev/null 2>&1 ) &&
( yes "" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
@@ -126,7 +130,7 @@ test_expect_success 'mergetool on file in parent dir' '
(
cd subdir &&
( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
- ( yes "" | git mergetool ../file2 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
( yes "d" | git mergetool ../file11 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool ../file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool ../submod >/dev/null 2>&1 ) &&
@@ -180,6 +184,24 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' '
git reset --hard
'
+test_expect_success 'mergetool takes partial path' '
+ git config rerere.enabled false &&
+ git checkout -b test12 branch1 &&
+ git submodule update -N &&
+ test_must_fail git merge master &&
+
+ #shouldnt need these lines
+ #( yes "d" | git mergetool file11 >/dev/null 2>&1 ) &&
+ #( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
+ #( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&
+ #( yes "" | git mergetool file1 file2 >/dev/null 2>&1 ) &&
+
+ ( yes "" | git mergetool subdir ) &&
+
+ test "$(cat subdir/file3)" = "master new sub" &&
+ git reset --hard
+'
+
test_expect_success 'deleted vs modified submodule' '
git checkout -b test6 branch1 &&
git submodule update -N &&
@@ -189,7 +211,7 @@ test_expect_success 'deleted vs modified submodule' '
git checkout -b test6.a test6 &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
rmdir submod && mv submod-movedaside submod &&
@@ -205,7 +227,7 @@ test_expect_success 'deleted vs modified submodule' '
git submodule update -N &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test ! -e submod &&
@@ -218,7 +240,7 @@ test_expect_success 'deleted vs modified submodule' '
git submodule update -N &&
test_must_fail git merge test6 &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
test ! -e submod &&
@@ -233,7 +255,7 @@ test_expect_success 'deleted vs modified submodule' '
git submodule update -N &&
test_must_fail git merge test6 &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test "$(cat submod/bar)" = "master submodule" &&
@@ -256,7 +278,7 @@ test_expect_success 'file vs modified submodule' '
git checkout -b test7.a branch1 &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
rmdir submod && mv submod-movedaside submod &&
@@ -271,7 +293,7 @@ test_expect_success 'file vs modified submodule' '
git checkout -b test7.b test7 &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
git submodule update -N &&
@@ -286,7 +308,7 @@ test_expect_success 'file vs modified submodule' '
git submodule update -N &&
test_must_fail git merge test7 &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "r" | git mergetool submod ) &&
test -d submod.orig &&
@@ -301,7 +323,7 @@ test_expect_success 'file vs modified submodule' '
git submodule update -N &&
test_must_fail git merge test7 &&
test -n "$(git ls-files -u)" &&
- ( yes "" | git mergetool file1 file2 subdir/file3 >/dev/null 2>&1 ) &&
+ ( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod ) &&
test "$(cat submod/bar)" = "master submodule" &&
@@ -388,7 +410,7 @@ test_expect_success 'directory vs modified submodule' '
test "$(cat submod/file16)" = "not a submodule" &&
rm -rf submod.orig &&
- git reset --hard &&
+ git reset --hard >/dev/null 2>&1 &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
test ! -e submod.orig &&
@@ -400,7 +422,7 @@ test_expect_success 'directory vs modified submodule' '
( cd submod && git clean -f && git reset --hard ) &&
git submodule update -N &&
test "$(cat submod/bar)" = "master submodule" &&
- git reset --hard && rm -rf submod-movedaside &&
+ git reset --hard >/dev/null 2>&1 && rm -rf submod-movedaside &&
git checkout -b test11.c master &&
git submodule update -N &&
@@ -410,7 +432,7 @@ test_expect_success 'directory vs modified submodule' '
git submodule update -N &&
test "$(cat submod/bar)" = "master submodule" &&
- git reset --hard &&
+ git reset --hard >/dev/null 2>&1 &&
git submodule update -N &&
test_must_fail git merge test11 &&
test -n "$(git ls-files -u)" &&
@@ -418,7 +440,7 @@ test_expect_success 'directory vs modified submodule' '
( yes "r" | git mergetool submod ) &&
test "$(cat submod/file16)" = "not a submodule" &&
- git reset --hard master &&
+ git reset --hard master >/dev/null 2>&1 &&
( cd submod && git clean -f && git reset --hard ) &&
git submodule update -N
'
--
1.7.5.4
^ permalink raw reply related
* Re: git-rebase skips automatically no more needed commits
From: Martin von Zweigbergk @ 2011-09-16 1:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: Martin von Zweigbergk, Francis Moreau, Michael J Gruber, git
In-Reply-To: <7vwrdaqxpk.fsf@alter.siamese.dyndns.org>
Hi,
On Wed, 14 Sep 2011, Junio C Hamano wrote:
> What I said is "all 'am' need to use from its input while rebasing is the
> commit object name"; that is very different from "we have only commit
> object name so we must use cherry-pick" or "because we have commit object
> name, we can afford to use cherry-pick".
Right. I didn't mean to suggest that we should use cherry-pick.
> Look for $rebasing in git-am.sh and notice that:
Thanks! I had no idea about these "tricks" in git-am.sh. Now I
understand much better what you meant. So we are currently getting the
metainfo from the commit rather than from the mailbox. With your
patch, we would also get the patch body from the commit. The only
thing remaining after that is the commit log message, correct?
Once all that git-am reads from the mailbox while in $rebasing mode is
the commit name, it would be as small change to teach it to read a
list of commit names instead. Is this what you meant or did you really
mean for format-patch to generate output without patch body? At this
point the name 'am' becomes quite misleading, but maybe that's not
worth worrying about.
Thanks,
Martin
^ permalink raw reply
* Re* [PATCH 4/4] Add documentation for ref namespaces
From: Junio C Hamano @ 2011-09-16 0:22 UTC (permalink / raw)
To: Jamey Sharp
Cc: Ævar Arnfjörð Bjarmason, Shawn O. Pearce,
Johannes Schindelin, Jeff King, Jakub Narebski, git,
Josh Triplett
In-Reply-To: <CACBZZX4bow8vwrwSL5uRJQtAgeg10_K+5ss8u-HvCHOn3+0Tuw@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On Tue, Jun 7, 2011 at 20:21, Jamey Sharp <jamey@minilop.net> wrote:
>
>> Document the namespace mechanism in a new gitnamespaces(7) page.
>> Reference it from receive-pack and upload-pack.
>
> This breaks the build on older asciidoc versions, the fix is to do
> what I did in f5008f56d5aba06598e1c6272f4f55b4ee4bb016.
Earlier I sent out a tongue-in-cheek "like this" that had a completely
bogus SYNOPSIS section that is unusable in a released version of Git,
hoping that somebody who _care_ more about the feature that the page
describes would give us a better wording, but unfortunately it never
happened.
So here is a more realistic replacement, so that we won't have to suffer
by complaints from people with older AsciiDoc saying "the release does not
build". If we were to include this in 1.7.7 final, we now need to make
sure we won't to have to hear from people with newer AsciiDoc saying "why
do we have nonsense in SYNOPSIS section", so your help is needed here.
I explicitly avoided saying:
[verse]
export GIT_NAMESPACE=<namespace>
as nothing other than the selected transports seems to pay attention to
this environment variable.
-- >8 --
Subject: [PATCH] Documentation/gitnamespaces.txt: cater to older asciidoc
Older asciidoc (e.g. 8.2.5 on Centos 5.5) is unhappy if a manpage does not
have a SYNOPSIS section. Show a sample (and a possibly bogus) command line
of running two commands that pay attention to this environment variable
with a customized value.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/gitnamespaces.txt | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/Documentation/gitnamespaces.txt b/Documentation/gitnamespaces.txt
index ed8924e..c6713cf 100644
--- a/Documentation/gitnamespaces.txt
+++ b/Documentation/gitnamespaces.txt
@@ -5,6 +5,13 @@ NAME
----
gitnamespaces - Git namespaces
+SYNOPSIS
+--------
+[verse]
+GIT_NAMESPACE=<namespace> 'git upload-pack'
+GIT_NAMESPACE=<namespace> 'git receive-pack'
+
+
DESCRIPTION
-----------
--
1.7.7.rc1.3.g559357
^ permalink raw reply related
* Re: [PATCH/RFC] add lame win32 credential-helper
From: Erik Faye-Lund @ 2011-09-15 21:48 UTC (permalink / raw)
To: Jeff King; +Cc: git, jaysoffian, gitster
In-Reply-To: <20110915214026.GA18623@sigill.intra.peff.net>
On Thu, Sep 15, 2011 at 11:40 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Sep 15, 2011 at 10:25:24PM +0200, Erik Faye-Lund wrote:
>
>> Not that it's useful yet, since the core-git code for the
>> credential-helper support doesn't compile on Windows. So
>> it's not fully tested, I've only read the interface
>> documentation and experimented with it from the command
>> line.
>
> Which parts of the credential-helper code don't compile? I wouldn't be
> surprised if the cache helper doesn't work (because it uses unix domain
> sockets for communication).
That's exactly it:
unix-socket.c:12: warning: 'struct sockaddr_un' declared inside parameter list
unix-socket.c:12: warning: its scope is only this definition or declaration, whi
ch is probably not what you want
unix-socket.c: In function 'unix_sockaddr_init':
unix-socket.c:15: error: dereferencing pointer to incomplete type
unix-socket.c:17: error: dereferencing pointer to incomplete type
unix-socket.c:18: error: dereferencing pointer to incomplete type
unix-socket.c:19: error: dereferencing pointer to incomplete type
unix-socket.c: In function 'unix_stream_connect':
unix-socket.c:25: error: storage size of 'sa' isn't known
unix-socket.c: In function 'unix_stream_listen':
unix-socket.c:39: error: storage size of 'sa' isn't known
make: *** [unix-socket.o] Error 1
make: *** Waiting for unfinished jobs....
> If it's too hard to adapt it to whatever IPC
> mechanism would be appropriate on Windows, we can just leave it out on
> that platform.
>
> But the core code in git itself should be pretty straight forward.
I didn't mean that it was impossible to port, just that it didn't compile
as-is. I haven't looked into fixing up the code so it compiles on Windows
again myself. And I'm not really planning to; I have little git-time these
days, and little knowledge of how unix-sockets works...
^ permalink raw reply
* Re: [PATCH/RFC] add lame win32 credential-helper
From: Jeff King @ 2011-09-15 21:40 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: git, jaysoffian, gitster
In-Reply-To: <1316118324-6164-1-git-send-email-kusmabite@gmail.com>
On Thu, Sep 15, 2011 at 10:25:24PM +0200, Erik Faye-Lund wrote:
> Not that it's useful yet, since the core-git code for the
> credential-helper support doesn't compile on Windows. So
> it's not fully tested, I've only read the interface
> documentation and experimented with it from the command
> line.
Which parts of the credential-helper code don't compile? I wouldn't be
surprised if the cache helper doesn't work (because it uses unix domain
sockets for communication). If it's too hard to adapt it to whatever IPC
mechanism would be appropriate on Windows, we can just leave it out on
that platform.
But the core code in git itself should be pretty straight forward. Let
me know what I can do to help.
-Peff
^ permalink raw reply
* [PATCH v3 20/22] resolve_ref(): also treat a too-long SHA1 as invalid
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
If the SHA1 in a reference file is not terminated by a space or
end-of-file, consider it malformed and emit a warning.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 2387f4e..0baa500 100644
--- a/refs.c
+++ b/refs.c
@@ -593,7 +593,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag |= REF_ISSYMREF;
}
- if (get_sha1_hex(buffer, sha1)) {
+ /* Please note that FETCH_HEAD has a second line containing other data. */
+ if (get_sha1_hex(buffer, sha1) || (buffer[40] != '\0' && !isspace(buffer[40]))) {
warning("reference in %s is formatted incorrectly", path);
return NULL;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 17/22] remote: avoid passing NULL to read_ref()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
read_ref() can (and in test t5800, actually *does*) return NULL.
Don't pass the NULL along to read_ref(). Coincidentally, this mistake
didn't make resolve_ref() blow up, but upcoming changes to
resolve_ref() will make it less forgiving.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
transport-helper.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 0713126..6f227e2 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -446,8 +446,10 @@ static int fetch_with_import(struct transport *transport,
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
private = xstrdup(posn->name);
- read_ref(private, posn->old_sha1);
- free(private);
+ if (private) {
+ read_ref(private, posn->old_sha1);
+ free(private);
+ }
}
strbuf_release(&buf);
return 0;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
While resolving references, if a reference is found that is in an
unrecognized format, emit a warning (and then fail, as before).
Wouldn't *you* want to know?
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index ee3e0cc..2387f4e 100644
--- a/refs.c
+++ b/refs.c
@@ -500,6 +500,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
ssize_t len;
char buffer[256];
static char ref_buffer[256];
+ char path[PATH_MAX];
if (flag)
*flag = 0;
@@ -508,7 +509,6 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
for (;;) {
- char path[PATH_MAX];
struct stat st;
char *buf;
int fd;
@@ -593,8 +593,10 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag |= REF_ISSYMREF;
}
- if (get_sha1_hex(buffer, sha1))
+ if (get_sha1_hex(buffer, sha1)) {
+ warning("reference in %s is formatted incorrectly", path);
return NULL;
+ }
return ref;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 15/22] resolve_ref(): do not follow incorrectly-formatted symbolic refs
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Emit a warning and fail if a symbolic reference refers to an
incorrectly-formatted refname.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index 473f7f6..b055501 100644
--- a/refs.c
+++ b/refs.c
@@ -581,6 +581,11 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
buf = buffer + 4;
while (isspace(*buf))
buf++;
+ if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
+ warning("symbolic reference in %s is formatted incorrectly",
+ path);
+ return NULL;
+ }
ref = strcpy(ref_buffer, buf);
if (flag)
*flag |= REF_ISSYMREF;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 13/22] resolve_ref(): turn buffer into a proper string as soon as possible
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Immediately strip off trailing spaces and null-terminate the string
holding the contents of the reference file; this allows the use of
string functions and avoids the need to keep separate track of the
string's length. (get_sha1_hex() fails automatically if the string is
too short.)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index 8f0b871..79ab0eb 100644
--- a/refs.c
+++ b/refs.c
@@ -546,25 +546,25 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
len = read_in_full(fd, buffer, sizeof(buffer)-1);
close(fd);
+ if (len < 0)
+ return NULL;
+ while (len && isspace(buffer[len-1]))
+ len--;
+ buffer[len] = '\0';
/*
* Is it a symbolic ref?
*/
- if (len < 4 || memcmp("ref:", buffer, 4))
+ if (prefixcmp(buffer, "ref:"))
break;
buf = buffer + 4;
- len -= 4;
- while (len && isspace(*buf))
- buf++, len--;
- while (len && isspace(buf[len-1]))
- len--;
- buf[len] = 0;
- memcpy(ref_buffer, buf, len + 1);
- ref = ref_buffer;
+ while (isspace(*buf))
+ buf++;
+ ref = strcpy(ref_buffer, buf);
if (flag)
*flag |= REF_ISSYMREF;
}
- if (len < 40 || get_sha1_hex(buffer, sha1))
+ if (get_sha1_hex(buffer, sha1))
return NULL;
return ref;
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 11/22] resolve_ref(): use prefixcmp()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Terminate the link content string one step earlier, allowing
prefixcmp() to be used instead of the less clear memcmp().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index c51fd45..da9737f 100644
--- a/refs.c
+++ b/refs.c
@@ -520,8 +520,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
len = readlink(path, buffer, sizeof(buffer)-1);
if (len < 0)
return NULL;
- if (len >= 5 && !memcmp("refs/", buffer, 5)) {
- buffer[len] = 0;
+ buffer[len] = 0;
+ if (!prefixcmp(buffer, "refs/")) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 12/22] resolve_ref(): only follow a symlink that contains a valid, normalized refname
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index da9737f..8f0b871 100644
--- a/refs.c
+++ b/refs.c
@@ -521,7 +521,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (len < 0)
return NULL;
buffer[len] = 0;
- if (!prefixcmp(buffer, "refs/")) {
+ if (!prefixcmp(buffer, "refs/") &&
+ !check_refname_format(buffer, 0)) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 14/22] resolve_ref(): extract a function get_packed_ref()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Making it a function and giving it a name makes the code clearer. I
also have a strong suspicion that the function will find other uses in
the future.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 47 ++++++++++++++++++++++++++++++++++-------------
1 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/refs.c b/refs.c
index 79ab0eb..473f7f6 100644
--- a/refs.c
+++ b/refs.c
@@ -466,6 +466,23 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
}
/*
+ * Try to read ref from the packed references. On success, set sha1
+ * and return 0; otherwise, return -1.
+ */
+static int get_packed_ref(const char *ref, unsigned char *sha1)
+{
+ struct ref_list *list = get_packed_refs(NULL);
+ while (list) {
+ if (!strcmp(ref, list->name)) {
+ hashcpy(sha1, list->sha1);
+ return 0;
+ }
+ list = list->next;
+ }
+ return -1;
+}
+
+/*
* If the "reading" argument is set, this function finds out what _object_
* the ref points at by "reading" the ref. The ref, if it is not symbolic,
* has to exist, and if it is symbolic, it has to point at an existing ref,
@@ -497,22 +514,26 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return NULL;
git_snpath(path, sizeof(path), "%s", ref);
- /* Special case: non-existing file. */
+
if (lstat(path, &st) < 0) {
- struct ref_list *list = get_packed_refs(NULL);
- while (list) {
- if (!strcmp(ref, list->name)) {
- hashcpy(sha1, list->sha1);
- if (flag)
- *flag |= REF_ISPACKED;
- return ref;
- }
- list = list->next;
+ if (errno != ENOENT)
+ return NULL;
+ /*
+ * The loose reference file does not exist;
+ * check for a packed reference.
+ */
+ if (!get_packed_ref(ref, sha1)) {
+ if (flag)
+ *flag |= REF_ISPACKED;
+ return ref;
}
- if (reading || errno != ENOENT)
+ /* The reference is not a packed reference, either. */
+ if (reading) {
return NULL;
- hashclr(sha1);
- return ref;
+ } else {
+ hashclr(sha1);
+ return ref;
+ }
}
/* Follow "normalized" - ie "refs/.." symlinks by hand */
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 10/22] resolve_ref(): explicitly fail if a symlink is not readable
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Previously the failure came later, after a few steps in which the
length was treated like the actual length of a string. Even though
the old code gave the same answers, it was somewhat misleading.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index d2aac24..c51fd45 100644
--- a/refs.c
+++ b/refs.c
@@ -518,6 +518,8 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
/* Follow "normalized" - ie "refs/.." symlinks by hand */
if (S_ISLNK(st.st_mode)) {
len = readlink(path, buffer, sizeof(buffer)-1);
+ if (len < 0)
+ return NULL;
if (len >= 5 && !memcmp("refs/", buffer, 5)) {
buffer[len] = 0;
strcpy(ref_buffer, buffer);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 16/22] remote: use xstrdup() instead of strdup()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
remote.c | 2 +-
transport-helper.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index 6fcf809..e52aa9b 100644
--- a/remote.c
+++ b/remote.c
@@ -815,7 +815,7 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
refspec->dst, &ret))
return ret;
} else if (!strcmp(refspec->src, name))
- return strdup(refspec->dst);
+ return xstrdup(refspec->dst);
}
return NULL;
}
diff --git a/transport-helper.c b/transport-helper.c
index 4eab844..0713126 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -183,7 +183,7 @@ static struct child_process *get_helper(struct transport *transport)
ALLOC_GROW(refspecs,
refspec_nr + 1,
refspec_alloc);
- refspecs[refspec_nr++] = strdup(capname + strlen("refspec "));
+ refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec "));
} else if (!strcmp(capname, "connect")) {
data->connect = 1;
} else if (!prefixcmp(capname, "export-marks ")) {
@@ -445,7 +445,7 @@ static int fetch_with_import(struct transport *transport,
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
else
- private = strdup(posn->name);
+ private = xstrdup(posn->name);
read_ref(private, posn->old_sha1);
free(private);
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 18/22] resolve_ref(): verify that the input refname has the right format
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/refs.c b/refs.c
index b055501..ee3e0cc 100644
--- a/refs.c
+++ b/refs.c
@@ -504,6 +504,9 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (flag)
*flag = 0;
+ if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
+ return NULL;
+
for (;;) {
char path[PATH_MAX];
struct stat st;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 21/22] resolve_ref(): expand documentation
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Record information about resolve_ref(), hard-won via reverse
engineering, in a comment for future spelunkers.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
cache.h | 34 +++++++++++++++++++++++++++++++++-
refs.c | 12 ------------
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/cache.h b/cache.h
index 607c2ea..aea8685 100644
--- a/cache.h
+++ b/cache.h
@@ -822,7 +822,39 @@ static inline int get_sha1_with_context(const char *str, unsigned char *sha1, st
extern int get_sha1_hex(const char *hex, unsigned char *sha1);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref(const char *filename, unsigned char *sha1);
-extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
+
+/*
+ * Resolve a reference, recursively following symbolic refererences.
+ *
+ * Store the referred-to object's name in sha1 and return the name of
+ * the non-symbolic reference that ultimately pointed at it. The
+ * return value, if not NULL, is a pointer into either a static buffer
+ * or the input ref.
+ *
+ * If the reference cannot be resolved to an object, the behavior
+ * depends on the "reading" argument:
+ *
+ * - If reading is set, return NULL.
+ *
+ * - If reading is not set, clear sha1 and return the name of the last
+ * reference name in the chain, which will either be a non-symbolic
+ * reference or an undefined reference. If this is a prelude to
+ * "writing" to the ref, the return value is the name of the ref
+ * that will actually be created or changed.
+ *
+ * If flag is non-NULL, set the value that it points to the
+ * combination of REF_ISPACKED (if the reference was found among the
+ * packed references) and REF_ISSYMREF (if the initial reference was a
+ * symbolic reference).
+ *
+ * If ref is not a properly-formatted, normalized reference, return
+ * NULL. If more than MAXDEPTH recursive symbolic lookups are needed,
+ * give up and return NULL.
+ *
+ * errno is sometimes set on errors, but not always.
+ */
+extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
extern int interpret_branch_name(const char *str, struct strbuf *);
diff --git a/refs.c b/refs.c
index 0baa500..096b42c 100644
--- a/refs.c
+++ b/refs.c
@@ -482,18 +482,6 @@ static int get_packed_ref(const char *ref, unsigned char *sha1)
return -1;
}
-/*
- * If the "reading" argument is set, this function finds out what _object_
- * the ref points at by "reading" the ref. The ref, if it is not symbolic,
- * has to exist, and if it is symbolic, it has to point at an existing ref,
- * because the "read" goes through the symref to the ref it points at.
- *
- * The access that is not "reading" may often be "writing", but does not
- * have to; it can be merely checking _where it leads to_. If it is a
- * prelude to "writing" to the ref, a write to a symref that points at
- * yet-to-be-born ref will create the real ref pointed by the symref.
- * reading=0 allows the caller to check where such a symref leads to.
- */
const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag)
{
int depth = MAXDEPTH;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 22/22] add_ref(): verify that the refname is formatted correctly
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
In add_ref(), verify that the refname is formatted correctly before
adding it to the ref_list. Here we have to allow refname components
that start with ".", since (for example) the remote protocol uses
synthetic reference name ".have". So add a new REFNAME_DOT_COMPONENT
flag that can be passed to check_refname_format() to allow leading
dots.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 18 ++++++++++++++----
refs.h | 6 +++++-
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index 096b42c..832a52f 100644
--- a/refs.c
+++ b/refs.c
@@ -56,6 +56,8 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,
entry = xmalloc(sizeof(struct ref_list) + len);
hashcpy(entry->sha1, sha1);
hashclr(entry->peeled);
+ if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
+ die("Reference has invalid format: '%s'", name);
memcpy(entry->name, name, len);
entry->flag = flag;
entry->next = list;
@@ -900,7 +902,7 @@ static inline int bad_ref_char(int ch)
* the length of the component found, or -1 if the component is not
* legal.
*/
-static int check_refname_component(const char *ref)
+static int check_refname_component(const char *ref, int flags)
{
const char *cp;
char last = '\0';
@@ -919,8 +921,16 @@ static int check_refname_component(const char *ref)
}
if (cp == ref)
return -1; /* Component has zero length. */
- if (ref[0] == '.')
- return -1; /* Component starts with '.'. */
+ if (ref[0] == '.') {
+ if (!(flags & REFNAME_DOT_COMPONENT))
+ return -1; /* Component starts with '.'. */
+ /*
+ * Even if leading dots are allowed, don't allow "."
+ * as a component (".." is prevented by a rule above).
+ */
+ if (ref[1] == '\0')
+ return -1; /* Component equals ".". */
+ }
if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
return -1; /* Refname ends with ".lock". */
return cp - ref;
@@ -932,7 +942,7 @@ int check_refname_format(const char *ref, int flags)
while (1) {
/* We are at the start of a path component. */
- component_len = check_refname_component(ref);
+ component_len = check_refname_component(ref, flags);
if (component_len < 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
ref[0] == '*' &&
diff --git a/refs.h b/refs.h
index b0da5fc..d5ac133 100644
--- a/refs.h
+++ b/refs.h
@@ -99,6 +99,7 @@ extern int for_each_reflog(each_ref_fn, void *);
#define REFNAME_ALLOW_ONELEVEL 1
#define REFNAME_REFSPEC_PATTERN 2
+#define REFNAME_DOT_COMPONENT 4
/*
* Return 0 iff ref has the correct format for a refname according to
@@ -106,7 +107,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
- * components. No leading or repeated slashes are accepted.
+ * components. No leading or repeated slashes are accepted. If
+ * REFNAME_DOT_COMPONENT is set in flags, then allow refname
+ * components to start with "." (but not a whole component equal to
+ * "." or "..").
*/
extern int check_refname_format(const char *ref, int flags);
--
1.7.6.8.gd2879
^ permalink raw reply related
* Re: vcs-svn and friends
From: Stephen Bash @ 2011-09-15 21:13 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: David Michael Barr, Dmitry Ivankov, git, Junio C Hamano
In-Reply-To: <20110915204815.GA31508@elie>
----- Original Message -----
> From: "Jonathan Nieder" <jrnieder@gmail.com>
> Sent: Thursday, September 15, 2011 4:48:15 PM
> Subject: Re: vcs-svn and friends
>
> Stephen Bash wrote:
>
> > For those of us interested but out of the loop, does this mean you
> > have a working example where I can point it at a SVN repo and see
> > what happens? Having done our SVN to Git conversion last year, I
> > know our repo has a lot of the common SVN screw cases (non-branching
> > copies, partial merges, mis-merges, *lots* of retagging, changes
> > committed to tags, etc.) so if it's relatively easy to setup a test
> > I'm happy to run one.
>
> Thanks. It's very bare-bones at the moment: it just imports each
> revision as a whole tree, with no branch and merge tracking at all.
> So it would be very interesting to get this basic stuff out into the
> wild and then add some code implementing those things for you to break
> on top of it.
Okay, that matches my last known state of the project, guess I just got optimistic ;)
Since last summer I've been meaning to take a crack at svn-filter-root.py, but it keeps sliding off the priority list. If you guys don't beat me to it, maybe I'll eventually get around to it...
Thanks,
Stephen
^ permalink raw reply
* [PATCH v3 03/22] Change bad_ref_char() to return a boolean value
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Previously most bad characters were indicated by returning 1, but "*"
was special-cased to return 2 instead of 1. One caller examined the
return value to see whether the special case occurred.
But it is easier (to document and understand) for bad_ref_char()
simply to return a boolean value, treating "*" like any other bad
character. Special-case the handling of "*" (which only occurs in
very specific circumstances) at the caller. The resulting calling
code thereby also becomes more transparent.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index a615043..fd29d89 100644
--- a/refs.c
+++ b/refs.c
@@ -860,22 +860,21 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
* - it contains a "\" (backslash)
*/
+/* Return true iff ch is not allowed in reference names. */
static inline int bad_ref_char(int ch)
{
if (((unsigned) ch) <= ' ' || ch == 0x7f ||
ch == '~' || ch == '^' || ch == ':' || ch == '\\')
return 1;
/* 2.13 Pattern Matching Notation */
- if (ch == '?' || ch == '[') /* Unsupported */
+ if (ch == '*' || ch == '?' || ch == '[') /* Unsupported */
return 1;
- if (ch == '*') /* Supported at the end */
- return 2;
return 0;
}
int check_ref_format(const char *ref)
{
- int ch, level, bad_type, last;
+ int ch, level, last;
int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
@@ -890,9 +889,8 @@ int check_ref_format(const char *ref)
/* we are at the beginning of the path component */
if (ch == '.')
return CHECK_REF_FORMAT_ERROR;
- bad_type = bad_ref_char(ch);
- if (bad_type) {
- if (bad_type == 2 && (!*cp || *cp == '/') &&
+ if (bad_ref_char(ch)) {
+ if (ch == '*' && (!*cp || *cp == '/') &&
ret == CHECK_REF_FORMAT_OK)
ret = CHECK_REF_FORMAT_WILDCARD;
else
@@ -902,8 +900,7 @@ int check_ref_format(const char *ref)
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
- bad_type = bad_ref_char(ch);
- if (bad_type)
+ if (bad_ref_char(ch))
return CHECK_REF_FORMAT_ERROR;
if (ch == '/')
break;
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 04/22] Change check_ref_format() to take a flags argument
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Change check_ref_format() to take a flags argument that indicates what
is acceptable in the reference name (analogous to "git
check-ref-format"'s "--allow-onelevel" and "--refspec-pattern"). This
is more convenient for callers and also fixes a failure in the test
suite (and likely elsewhere in the code) by enabling "onelevel" and
"refspec-pattern" to be allowed independently of each other.
Also rename check_ref_format() to check_refname_format() to make it
obvious that it deals with refnames rather than references themselves.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 21 +---------------
builtin/checkout.c | 2 +-
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/show-ref.c | 2 +-
builtin/tag.c | 4 +-
connect.c | 2 +-
environment.c | 2 +-
fast-import.c | 7 +----
git_remote_helpers/git/git.py | 2 +-
notes-merge.c | 5 ++-
pack-refs.c | 2 +-
refs.c | 42 ++++++++++++++-----------------
refs.h | 17 +++++++++----
remote.c | 53 +++++++++++------------------------------
sha1_name.c | 4 +-
t/t1402-check-ref-format.sh | 6 +----
transport.c | 16 +++---------
walker.c | 2 +-
20 files changed, 69 insertions(+), 126 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 7295954..8f41696 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -53,9 +53,6 @@ static void refname_format_print(const char *arg)
printf("%s\n", refname);
}
-#define REFNAME_ALLOW_ONELEVEL 1
-#define REFNAME_REFSPEC_PATTERN 2
-
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
@@ -83,24 +80,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
if (! (i == argc - 1))
usage(builtin_check_ref_format_usage);
- switch (check_ref_format(argv[i])) {
- case CHECK_REF_FORMAT_OK:
- break;
- case CHECK_REF_FORMAT_ERROR:
+ if (check_refname_format(argv[i], flags))
return 1;
- case CHECK_REF_FORMAT_ONELEVEL:
- if (!(flags & REFNAME_ALLOW_ONELEVEL))
- return 1;
- else
- break;
- case CHECK_REF_FORMAT_WILDCARD:
- if (!(flags & REFNAME_REFSPEC_PATTERN))
- return 1;
- else
- break;
- default:
- die("internal error: unexpected value from check_ref_format()");
- }
if (print)
refname_format_print(argv[i]);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3bb6525..574d2b6 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -882,7 +882,7 @@ static int parse_branchname_arg(int argc, const char **argv,
new->name = arg;
setup_branch_path(new);
- if (check_ref_format(new->path) == CHECK_REF_FORMAT_OK &&
+ if (!check_refname_format(new->path, 0) &&
resolve_ref(new->path, branch_rev, 1, NULL))
hashcpy(rev, branch_rev);
else
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 412bd32..b51e478 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -544,7 +544,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_ref_format(ref->name + 5))
+ check_refname_format(ref->name + 5, 0))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ae164da..0600efa 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -356,7 +356,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_ref_format(name + 5)) {
+ if (prefixcmp(name, "refs/") || check_refname_format(name + 5, 0)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
diff --git a/builtin/replace.c b/builtin/replace.c
index fe3a647..517fa10 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_ref_format(ref))
+ if (check_refname_format(ref, 0))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 45f0340..fafb6dd 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -145,7 +145,7 @@ static int exclude_existing(const char *match)
if (strncmp(ref, match, matchlen))
continue;
}
- if (check_ref_format(ref)) {
+ if (check_refname_format(ref, 0)) {
warning("ref '%s' ignored", ref);
continue;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index 667515e..48be745 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -407,12 +407,12 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
{
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_ref_format(sb->buf);
+ return check_refname_format(sb->buf, 0);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
diff --git a/connect.c b/connect.c
index ee1d4b4..51990fa 100644
--- a/connect.c
+++ b/connect.c
@@ -22,7 +22,7 @@ static int check_ref(const char *name, int len, unsigned int flags)
len -= 5;
/* REF_NORMAL means that we don't want the magic fake tag refs */
- if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
+ if ((flags & REF_NORMAL) && check_refname_format(name, 0))
return 0;
/* REF_HEADS means that we want regular branch heads */
diff --git a/environment.c b/environment.c
index e96edcf..8174b70 100644
--- a/environment.c
+++ b/environment.c
@@ -106,7 +106,7 @@ static char *expand_namespace(const char *raw_namespace)
if (strcmp((*c)->buf, "/") != 0)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
- if (check_ref_format(buf.buf) != CHECK_REF_FORMAT_OK)
+ if (check_refname_format(buf.buf, 0))
die("bad git namespace path \"%s\"", raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
diff --git a/fast-import.c b/fast-import.c
index 742e7da..f9347f5 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -722,13 +722,8 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
- switch (check_ref_format(name)) {
- case 0: break; /* its valid */
- case CHECK_REF_FORMAT_ONELEVEL:
- break; /* valid, but too few '/', allow anyway */
- default:
+ if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
die("Branch name doesn't conform to GIT standards: %s", name);
- }
b = pool_calloc(1, sizeof(struct branch));
b->name = pool_strdup(name);
diff --git a/git_remote_helpers/git/git.py b/git_remote_helpers/git/git.py
index a383e6c..007a1bf 100644
--- a/git_remote_helpers/git/git.py
+++ b/git_remote_helpers/git/git.py
@@ -54,7 +54,7 @@ def valid_git_ref (ref_name):
# The following is a reimplementation of the git check-ref-format
# command. The rules were derived from the git check-ref-format(1)
# manual page. This code should be replaced by a call to
- # check_ref_format() in the git library, when such is available.
+ # check_refname_format() in the git library, when such is available.
if ref_name.endswith('/') or \
ref_name.startswith('.') or \
ref_name.count('/.') or \
diff --git a/notes-merge.c b/notes-merge.c
index e1aaf43..3bbcc9d 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -570,7 +570,8 @@ int notes_merge(struct notes_merge_options *o,
/* Dereference o->local_ref into local_sha1 */
if (!resolve_ref(o->local_ref, local_sha1, 0, NULL))
die("Failed to resolve local notes ref '%s'", o->local_ref);
- else if (!check_ref_format(o->local_ref) && is_null_sha1(local_sha1))
+ else if (!check_refname_format(o->local_ref, 0) &&
+ is_null_sha1(local_sha1))
local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
else if (!(local = lookup_commit_reference(local_sha1)))
die("Could not parse local commit %s (%s)",
@@ -583,7 +584,7 @@ int notes_merge(struct notes_merge_options *o,
* Failed to get remote_sha1. If o->remote_ref looks like an
* unborn ref, perform the merge using an empty notes tree.
*/
- if (!check_ref_format(o->remote_ref)) {
+ if (!check_refname_format(o->remote_ref, 0)) {
hashclr(remote_sha1);
remote = NULL;
} else {
diff --git a/pack-refs.c b/pack-refs.c
index 1290570..23bbd00 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -72,7 +72,7 @@ static void try_remove_empty_parents(char *name)
for (i = 0; i < 2; i++) { /* refs/{heads,tags,...}/ */
while (*p && *p != '/')
p++;
- /* tolerate duplicate slashes; see check_ref_format() */
+ /* tolerate duplicate slashes; see check_refname_format() */
while (*p == '/')
p++;
}
diff --git a/refs.c b/refs.c
index fd29d89..aaa8730 100644
--- a/refs.c
+++ b/refs.c
@@ -872,10 +872,9 @@ static inline int bad_ref_char(int ch)
return 0;
}
-int check_ref_format(const char *ref)
+int check_refname_format(const char *ref, int flags)
{
int ch, level, last;
- int ret = CHECK_REF_FORMAT_OK;
const char *cp = ref;
level = 0;
@@ -884,41 +883,42 @@ int check_ref_format(const char *ref)
; /* tolerate duplicated slashes */
if (!ch)
/* should not end with slashes */
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
/* we are at the beginning of the path component */
if (ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (bad_ref_char(ch)) {
- if (ch == '*' && (!*cp || *cp == '/') &&
- ret == CHECK_REF_FORMAT_OK)
- ret = CHECK_REF_FORMAT_WILDCARD;
+ if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
+ (!*cp || *cp == '/'))
+ /* Accept one wildcard as a full refname component. */
+ flags &= ~REFNAME_REFSPEC_PATTERN;
else
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
}
last = ch;
/* scan the rest of the path component */
while ((ch = *cp++) != 0) {
if (bad_ref_char(ch))
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (ch == '/')
break;
if (last == '.' && ch == '.')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
if (last == '@' && ch == '{')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
last = ch;
}
level++;
if (!ch) {
if (ref <= cp - 2 && cp[-2] == '.')
- return CHECK_REF_FORMAT_ERROR;
- if (level < 2)
- return CHECK_REF_FORMAT_ONELEVEL;
+ return -1;
+ if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
+ return -1;
if (has_extension(ref, ".lock"))
- return CHECK_REF_FORMAT_ERROR;
- return ret;
+ return -1;
+ return 0;
}
}
}
@@ -1103,7 +1103,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
{
char refpath[PATH_MAX];
- if (check_ref_format(ref))
+ if (check_refname_format(ref, 0))
return NULL;
strcpy(refpath, mkpath("refs/%s", ref));
return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
@@ -1111,13 +1111,9 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
{
- switch (check_ref_format(ref)) {
- default:
+ if (check_refname_format(ref, REFNAME_ALLOW_ONELEVEL))
return NULL;
- case 0:
- case CHECK_REF_FORMAT_ONELEVEL:
- return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
- }
+ return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
}
static struct lock_file packlock;
diff --git a/refs.h b/refs.h
index dfb086e..48540c0 100644
--- a/refs.h
+++ b/refs.h
@@ -97,11 +97,18 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi
*/
extern int for_each_reflog(each_ref_fn, void *);
-#define CHECK_REF_FORMAT_OK 0
-#define CHECK_REF_FORMAT_ERROR (-1)
-#define CHECK_REF_FORMAT_ONELEVEL (-2)
-#define CHECK_REF_FORMAT_WILDCARD (-3)
-extern int check_ref_format(const char *target);
+#define REFNAME_ALLOW_ONELEVEL 1
+#define REFNAME_REFSPEC_PATTERN 2
+
+/*
+ * Return 0 iff ref has the correct format for a refname according to
+ * the rules described in Documentation/git-check-ref-format.txt. If
+ * REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
+ * reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
+ * allow a "*" wildcard character in place of one of the name
+ * components.
+ */
+extern int check_refname_format(const char *ref, int flags);
extern const char *prettify_refname(const char *refname);
extern char *shorten_unambiguous_ref(const char *ref, int strict);
diff --git a/remote.c b/remote.c
index b8ecfa5..6fcf809 100644
--- a/remote.c
+++ b/remote.c
@@ -493,23 +493,6 @@ static void read_config(void)
}
/*
- * We need to make sure the remote-tracking branches are well formed, but a
- * wildcard refspec in "struct refspec" must have a trailing slash. We
- * temporarily drop the trailing '/' while calling check_ref_format(),
- * and put it back. The caller knows that a CHECK_REF_FORMAT_ONELEVEL
- * error return is Ok for a wildcard refspec.
- */
-static int verify_refname(char *name, int is_glob)
-{
- int result;
-
- result = check_ref_format(name);
- if (is_glob && result == CHECK_REF_FORMAT_WILDCARD)
- result = CHECK_REF_FORMAT_OK;
- return result;
-}
-
-/*
* This function frees a refspec array.
* Warning: code paths should be checked to ensure that the src
* and dst pointers are always freeable pointers as well
@@ -532,13 +515,13 @@ static void free_refspecs(struct refspec *refspec, int nr_refspec)
static struct refspec *parse_refspec_internal(int nr_refspec, const char **refspec, int fetch, int verify)
{
int i;
- int st;
struct refspec *rs = xcalloc(sizeof(*rs), nr_refspec);
for (i = 0; i < nr_refspec; i++) {
size_t llen;
int is_glob;
const char *lhs, *rhs;
+ int flags;
is_glob = 0;
@@ -576,6 +559,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
rs[i].pattern = is_glob;
rs[i].src = xstrndup(lhs, llen);
+ flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
if (fetch) {
/*
@@ -585,26 +569,20 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
*/
if (!*rs[i].src)
; /* empty is ok */
- else {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_refname_format(rs[i].src, flags))
+ goto invalid;
/*
* RHS
* - missing is ok, and is same as empty.
* - empty is ok; it means not to store.
* - otherwise it must be a valid looking ref.
*/
- if (!rs[i].dst) {
+ if (!rs[i].dst)
; /* ok */
- } else if (!*rs[i].dst) {
+ else if (!*rs[i].dst)
; /* ok */
- } else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
- goto invalid;
- }
+ else if (check_refname_format(rs[i].dst, flags))
+ goto invalid;
} else {
/*
* LHS
@@ -616,8 +594,7 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
if (!*rs[i].src)
; /* empty is ok */
else if (is_glob) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].src, flags))
goto invalid;
}
else
@@ -630,14 +607,12 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
* - otherwise it must be a valid looking ref.
*/
if (!rs[i].dst) {
- st = verify_refname(rs[i].src, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].src, flags))
goto invalid;
} else if (!*rs[i].dst) {
goto invalid;
} else {
- st = verify_refname(rs[i].dst, is_glob);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL)
+ if (check_refname_format(rs[i].dst, flags))
goto invalid;
}
}
@@ -1427,8 +1402,8 @@ int get_fetch_map(const struct ref *remote_refs,
for (rmp = &ref_map; *rmp; ) {
if ((*rmp)->peer_ref) {
- int st = check_ref_format((*rmp)->peer_ref->name + 5);
- if (st && st != CHECK_REF_FORMAT_ONELEVEL) {
+ if (check_refname_format((*rmp)->peer_ref->name + 5,
+ REFNAME_ALLOW_ONELEVEL)) {
struct ref *ignore = *rmp;
error("* Ignoring funny ref '%s' locally",
(*rmp)->peer_ref->name);
@@ -1620,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_ref_format(refname + 5))
+ if (check_refname_format(refname + 5, 0))
return 0;
len = strlen(refname) + 1;
diff --git a/sha1_name.c b/sha1_name.c
index ff5992a..143fd97 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -972,9 +972,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
{
strbuf_branchname(sb, name);
if (name[0] == '-')
- return CHECK_REF_FORMAT_ERROR;
+ return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
- return check_ref_format(sb->buf);
+ return check_refname_format(sb->buf, 0);
}
/*
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index f551eef..1cad88f 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -87,11 +87,7 @@ valid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='*'
invalid_ref "$ref"
-
-#invalid_ref "$ref" --allow-onelevel
-test_expect_failure "ref name '$ref' is invalid with options --allow-onelevel" \
- "test_must_fail git check-ref-format --allow-onelevel '$ref'"
-
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
diff --git a/transport.c b/transport.c
index fa279d5..feb2ff5 100644
--- a/transport.c
+++ b/transport.c
@@ -754,18 +754,10 @@ void transport_verify_remote_names(int nr_heads, const char **heads)
continue;
remote = remote ? (remote + 1) : local;
- switch (check_ref_format(remote)) {
- case 0: /* ok */
- case CHECK_REF_FORMAT_ONELEVEL:
- /* ok but a single level -- that is fine for
- * a match pattern.
- */
- case CHECK_REF_FORMAT_WILDCARD:
- /* ok but ends with a pattern-match character */
- continue;
- }
- die("remote part of refspec is not a valid name in %s",
- heads[i]);
+ if (check_refname_format(remote,
+ REFNAME_ALLOW_ONELEVEL|REFNAME_REFSPEC_PATTERN))
+ die("remote part of refspec is not a valid name in %s",
+ heads[i]);
}
}
diff --git a/walker.c b/walker.c
index dce7128..be389dc 100644
--- a/walker.c
+++ b/walker.c
@@ -190,7 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
{
if (!get_sha1_hex(target, sha1))
return 0;
- if (!check_ref_format(target)) {
+ if (!check_refname_format(target, 0)) {
struct ref *ref = alloc_ref(target);
if (!walker->fetch_ref(walker, ref)) {
hashcpy(sha1, ref->old_sha1);
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 01/22] t1402: add some more tests
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
The new tests reflect the status quo. Soon the rule for "*.lock" in
refname components will be tightened up.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
t/t1402-check-ref-format.sh | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index ed4275a..dc43171 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -13,6 +13,8 @@ invalid_ref() {
"test_must_fail git check-ref-format '$1'"
}
+invalid_ref ''
+invalid_ref '/'
valid_ref 'heads/foo'
invalid_ref 'foo'
valid_ref 'foo/bar/baz'
@@ -27,6 +29,9 @@ invalid_ref 'heads/foo..bar'
invalid_ref 'heads/foo?bar'
valid_ref 'foo./bar'
invalid_ref 'heads/foo.lock'
+invalid_ref 'heads///foo.lock'
+valid_ref 'foo.lock/bar'
+valid_ref 'foo.lock///bar'
valid_ref 'heads/foo@bar'
invalid_ref 'heads/v@{ation'
invalid_ref 'heads/foo\bar'
@@ -83,5 +88,9 @@ invalid_ref_normalized '/foo'
invalid_ref_normalized 'heads/foo/../bar'
invalid_ref_normalized 'heads/./foo'
invalid_ref_normalized 'heads\foo'
+invalid_ref_normalized 'heads/foo.lock'
+invalid_ref_normalized 'heads///foo.lock'
+valid_ref_normalized 'foo.lock/bar' 'foo.lock/bar'
+valid_ref_normalized 'foo.lock///bar' 'foo.lock/bar'
test_done
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 05/22] Refactor check_refname_format()
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Among other things, extract a function check_refname_component().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 95 +++++++++++++++++++++++++++++++++++++---------------------------
1 files changed, 55 insertions(+), 40 deletions(-)
diff --git a/refs.c b/refs.c
index aaa8730..5259724 100644
--- a/refs.c
+++ b/refs.c
@@ -872,55 +872,70 @@ static inline int bad_ref_char(int ch)
return 0;
}
+/*
+ * Try to read one refname component from the front of ref. Return
+ * the length of the component found, or -1 if the component is not
+ * legal.
+ */
+static int check_refname_component(const char *ref)
+{
+ const char *cp;
+ char last = '\0';
+
+ for (cp = ref; ; cp++) {
+ char ch = *cp;
+ if (ch == '\0' || ch == '/')
+ break;
+ if (bad_ref_char(ch))
+ return -1; /* Illegal character in refname. */
+ if (last == '.' && ch == '.')
+ return -1; /* Refname contains "..". */
+ if (last == '@' && ch == '{')
+ return -1; /* Refname contains "@{". */
+ last = ch;
+ }
+ if (cp == ref)
+ return -1; /* Component has zero length. */
+ if (ref[0] == '.')
+ return -1; /* Component starts with '.'. */
+ return cp - ref;
+}
+
int check_refname_format(const char *ref, int flags)
{
- int ch, level, last;
- const char *cp = ref;
+ int component_len, component_count = 0;
- level = 0;
while (1) {
- while ((ch = *cp++) == '/')
- ; /* tolerate duplicated slashes */
- if (!ch)
- /* should not end with slashes */
- return -1;
-
- /* we are at the beginning of the path component */
- if (ch == '.')
- return -1;
- if (bad_ref_char(ch)) {
- if ((flags & REFNAME_REFSPEC_PATTERN) && ch == '*' &&
- (!*cp || *cp == '/'))
+ while (*ref == '/')
+ ref++; /* tolerate leading and repeated slashes */
+
+ /* We are at the start of a path component. */
+ component_len = check_refname_component(ref);
+ if (component_len < 0) {
+ if ((flags & REFNAME_REFSPEC_PATTERN) &&
+ ref[0] == '*' &&
+ (ref[1] == '\0' || ref[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
- else
- return -1;
- }
-
- last = ch;
- /* scan the rest of the path component */
- while ((ch = *cp++) != 0) {
- if (bad_ref_char(ch))
- return -1;
- if (ch == '/')
- break;
- if (last == '.' && ch == '.')
- return -1;
- if (last == '@' && ch == '{')
- return -1;
- last = ch;
- }
- level++;
- if (!ch) {
- if (ref <= cp - 2 && cp[-2] == '.')
- return -1;
- if (level < 2 && !(flags & REFNAME_ALLOW_ONELEVEL))
- return -1;
- if (has_extension(ref, ".lock"))
+ component_len = 1;
+ } else {
return -1;
- return 0;
+ }
}
+ component_count++;
+ if (ref[component_len] == '\0')
+ break;
+ /* Skip to next component. */
+ ref += component_len + 1;
}
+
+ if (ref[component_len - 1] == '.')
+ return -1; /* Refname ends with '.'. */
+ if (component_len >= 5 && !memcmp(&ref[component_len - 5], ".lock", 5))
+ return -1; /* Refname ends with ".lock". */
+ if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
+ return -1; /* Refname has only one component. */
+ return 0;
}
const char *prettify_refname(const char *name)
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 07/22] Make collapse_slashes() allocate memory for its result
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
This will make upcoming changes a tiny bit easier.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/check-ref-format.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 8f41696..989ee5c 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -12,25 +12,28 @@ static const char builtin_check_ref_format_usage[] =
" or: git check-ref-format --branch <branchname-shorthand>";
/*
- * Remove leading slashes and replace each run of adjacent slashes in
- * src with a single slash, and write the result to dst.
+ * Return a copy of refname but with leading slashes removed and runs
+ * of adjacent slashes replaced with single slashes.
*
* This function is similar to normalize_path_copy(), but stripped down
* to meet check_ref_format's simpler needs.
*/
-static void collapse_slashes(char *dst, const char *src)
+static char *collapse_slashes(const char *refname)
{
+ char *ret = xmalloc(strlen(refname) + 1);
char ch;
char prev = '/';
+ char *cp = ret;
- while ((ch = *src++) != '\0') {
+ while ((ch = *refname++) != '\0') {
if (prev == '/' && ch == prev)
continue;
- *dst++ = ch;
+ *cp++ = ch;
prev = ch;
}
- *dst = '\0';
+ *cp = '\0';
+ return ret;
}
static int check_ref_format_branch(const char *arg)
@@ -47,9 +50,7 @@ static int check_ref_format_branch(const char *arg)
static void refname_format_print(const char *arg)
{
- char *refname = xmalloc(strlen(arg) + 1);
-
- collapse_slashes(refname, arg);
+ char *refname = collapse_slashes(arg);
printf("%s\n", refname);
}
--
1.7.6.8.gd2879
^ permalink raw reply related
* [PATCH v3 09/22] Change check_refname_format() to reject unnormalized refnames
From: Michael Haggerty @ 2011-09-15 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1316121043-29367-1-git-send-email-mhagger@alum.mit.edu>
Since much of the infrastructure does not work correctly with
unnormalized refnames, change check_refname_format() to reject them.
Similarly, change "git check-ref-format" to reject unnormalized
refnames by default. But add an option --normalize, which causes "git
check-ref-format" to normalize the refname before checking its format,
and print the normalized refname. This is exactly the behavior of the
old --print option, which is retained but deprecated.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 26 ++++++++++++++++++--------
builtin/check-ref-format.c | 15 +++++++--------
refs.c | 3 ---
refs.h | 2 +-
t/t1402-check-ref-format.sh | 31 +++++++++++++++++++++++--------
5 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 9114751..103e7b1 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -8,8 +8,9 @@ git-check-ref-format - Ensures that a reference name is well formed
SYNOPSIS
--------
[verse]
-'git check-ref-format' [--print]
- [--[no-]allow-onelevel] [--refspec-pattern] <refname>
+'git check-ref-format' [--normalize]
+ [--[no-]allow-onelevel] [--refspec-pattern]
+ <refname>
'git check-ref-format' --branch <branchname-shorthand>
DESCRIPTION
@@ -45,7 +46,11 @@ git imposes the following rules on how references are named:
bracket `[` anywhere. See the `--refspec-pattern` option below for
an exception to this rule.
-. They cannot end with a slash `/` nor a dot `.`.
+. They cannot begin or end with a slash `/` or contain multiple
+ consecutive slashes (see the `--normalize` option below for an
+ exception to this rule)
+
+. They cannot end with a dot `.`.
. They cannot contain a sequence `@{`.
@@ -70,10 +75,6 @@ reference name expressions (see linkgit:gitrevisions[7]):
. at-open-brace `@{` is used as a notation to access a reflog entry.
-With the `--print` option, if 'refname' is acceptable, it prints the
-canonicalized name of a hypothetical reference with that name. That is,
-it prints 'refname' with any extra `/` characters removed.
-
With the `--branch` option, it expands the ``previous branch syntax''
`@{-n}`. For example, `@{-1}` is a way to refer the last branch you
were on. This option should be used by porcelains to accept this
@@ -95,6 +96,15 @@ OPTIONS
in place of a one full pathname component (e.g.,
`foo/{asterisk}/bar` but not `foo/bar{asterisk}`).
+--normalize::
+ Normalize 'refname' by removing any leading slash (`/`)
+ characters and collapsing runs of adjacent slashes between
+ name components into a single slash. Iff the normalized
+ refname is valid then print it to standard output and exit
+ with a status of 0. (`--print` is a deprecated way to spell
+ `--normalize`.)
+
+
EXAMPLES
--------
@@ -107,7 +117,7 @@ $ git check-ref-format --branch @{-1}
* Determine the reference name to use for a new branch:
+
------------
-$ ref=$(git check-ref-format --print "refs/heads/$newbranch") ||
+$ ref=$(git check-ref-format --normalize "refs/heads/$newbranch") ||
die "we do not like '$newbranch' as a branch name."
------------
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index f5df9aa..28a7320 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -8,7 +8,7 @@
#include "strbuf.h"
static const char builtin_check_ref_format_usage[] =
-"git check-ref-format [--print] [options] <refname>\n"
+"git check-ref-format [--normalize] [options] <refname>\n"
" or: git check-ref-format --branch <branchname-shorthand>";
/*
@@ -51,7 +51,7 @@ static int check_ref_format_branch(const char *arg)
int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
{
int i;
- int print = 0;
+ int normalize = 0;
int flags = 0;
const char *refname;
@@ -62,8 +62,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
return check_ref_format_branch(argv[2]);
for (i = 1; i < argc && argv[i][0] == '-'; i++) {
- if (!strcmp(argv[i], "--print"))
- print = 1;
+ if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
+ normalize = 1;
else if (!strcmp(argv[i], "--allow-onelevel"))
flags |= REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--no-allow-onelevel"))
@@ -77,13 +77,12 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
usage(builtin_check_ref_format_usage);
refname = argv[i];
+ if (normalize)
+ refname = collapse_slashes(refname);
if (check_refname_format(refname, flags))
return 1;
-
- if (print) {
- refname = collapse_slashes(refname);
+ if (normalize)
printf("%s\n", refname);
- }
return 0;
}
diff --git a/refs.c b/refs.c
index 5a0bd0f..d2aac24 100644
--- a/refs.c
+++ b/refs.c
@@ -908,9 +908,6 @@ int check_refname_format(const char *ref, int flags)
int component_len, component_count = 0;
while (1) {
- while (*ref == '/')
- ref++; /* tolerate leading and repeated slashes */
-
/* We are at the start of a path component. */
component_len = check_refname_component(ref);
if (component_len < 0) {
diff --git a/refs.h b/refs.h
index 48540c0..b0da5fc 100644
--- a/refs.h
+++ b/refs.h
@@ -106,7 +106,7 @@ extern int for_each_reflog(each_ref_fn, void *);
* REFNAME_ALLOW_ONELEVEL is set in flags, then accept one-level
* reference names. If REFNAME_REFSPEC_PATTERN is set in flags, then
* allow a "*" wildcard character in place of one of the name
- * components.
+ * components. No leading or repeated slashes are accepted.
*/
extern int check_refname_format(const char *ref, int flags);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 419788f..710fcca 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -28,11 +28,17 @@ invalid_ref() {
invalid_ref ''
invalid_ref '/'
invalid_ref '/' --allow-onelevel
+invalid_ref '/' --normalize
+invalid_ref '/' '--allow-onelevel --normalize'
valid_ref 'foo/bar/baz'
-valid_ref 'refs///heads/foo'
+valid_ref 'foo/bar/baz' --normalize
+invalid_ref 'refs///heads/foo'
+valid_ref 'refs///heads/foo' --normalize
invalid_ref 'heads/foo/'
-valid_ref '/heads/foo'
-valid_ref '///heads/foo'
+invalid_ref '/heads/foo'
+valid_ref '/heads/foo' --normalize
+invalid_ref '///heads/foo'
+valid_ref '///heads/foo' --normalize
invalid_ref './foo'
invalid_ref './foo/bar'
invalid_ref 'foo/./bar'
@@ -60,12 +66,15 @@ invalid_ref "$ref"
valid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
ref='foo/bar'
valid_ref "$ref"
valid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+valid_ref "$ref" --normalize
ref='foo/*'
invalid_ref "$ref"
@@ -78,6 +87,8 @@ invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--refspec-pattern --normalize'
ref='foo/*/bar'
invalid_ref "$ref"
@@ -105,9 +116,13 @@ invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
ref='/foo'
invalid_ref "$ref"
-valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
-valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" --normalize
+valid_ref "$ref" '--allow-onelevel --normalize'
+invalid_ref "$ref" '--refspec-pattern --normalize'
+valid_ref "$ref" '--refspec-pattern --allow-onelevel --normalize'
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
@@ -141,12 +156,12 @@ test_expect_success 'check-ref-format --branch from subdir' '
valid_ref_normalized() {
test_expect_success "ref name '$1' simplifies to '$2'" "
- refname=\$(git check-ref-format --print '$1') &&
+ refname=\$(git check-ref-format --normalize '$1') &&
test \"\$refname\" = '$2'"
}
invalid_ref_normalized() {
- test_expect_success "check-ref-format --print rejects '$1'" "
- test_must_fail git check-ref-format --print '$1'"
+ test_expect_success "check-ref-format --normalize rejects '$1'" "
+ test_must_fail git check-ref-format --normalize '$1'"
}
valid_ref_normalized 'heads/foo' 'heads/foo'
--
1.7.6.8.gd2879
^ 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