* Re: [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal()
From: Johan Herland @ 2007-06-09 21:39 UTC (permalink / raw)
To: git; +Cc: Alex Riesen, Junio C Hamano, Johannes Schindelin
In-Reply-To: <81b0412b0706091431h3a786aaew342693a667938d9@mail.gmail.com>
On Saturday 09 June 2007, Alex Riesen wrote:
> On 6/9/07, Johan Herland <johan@herland.net> wrote:
> > + /* Verify the tag-name: we don't allow control characters or spaces in it */
> > + for (i = 4;;) {
> > + unsigned char c = tag_line[i++];
> > + if (c == '\n')
> > + break;
> > + if (c > ' ')
> > + continue;
> > + return error("char" PD_FMT ": could not verify tag name", tag_line + i - data);
> > + }
>
> This looks very familiar. Haven't you just made a very useless patch
> which had this very same code? How about putting it in its own
> function and just call it from these two places? And what problem
> do you have with pointers?!
I just answered your comment on the previous patch, and that answer should
apply here as well.
I'm probably splitting this up into too small pieces, since I keep getting
comments that fail to see the overall picture of what I'm trying to do,
namely taking two similar pieces of code and slowly unifying them to the
point where I can replace one of them by a call to the other (see the two
next patches).
Hope this helps,
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH 09/21] Remove unneeded code from mktag.c
From: Alex Riesen @ 2007-06-09 21:39 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090216.18823.johan@herland.net>
On 6/9/07, Johan Herland <johan@herland.net> wrote:
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
> mktag.c | 94 ---------------------------------------------------------------
> 1 files changed, 0 insertions(+), 94 deletions(-)
So, you do some useless changes just to remove the
function completely afterwards?
^ permalink raw reply
* Re: [PATCH 10/21] Free mktag's buffer before dying
From: Alex Riesen @ 2007-06-09 21:37 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090216.41856.johan@herland.net>
On 6/9/07, Johan Herland <johan@herland.net> wrote:
> + if (parse_and_verify_tag_buffer(0, buffer, size, 1)) {
> + free(buffer);
> + die("invalid tag data file");
This, and the similar one below are useless. You're destroying the
process, what do you free that buffer for? Either handle the error
case or do not needlessly complicate your change, which really
also absolutely unneeded.
^ permalink raw reply
* [PATCH 3/3] t7400: barf if git-submodule removes or replaces a file
From: Lars Hjemli @ 2007-06-09 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven Verdoolaege, git
In-Reply-To: <1181425132239-git-send-email-hjemli@gmail.com>
The test for an unmolested file wouldn't fail properly if the file had been
removed or replaced by something other than a regular file. This fixes it.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
t/t7400-submodule-basic.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 3940433..74fafce 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -72,7 +72,7 @@ test_expect_success 'update should fail when path is used by a file' '
then
echo "[OOPS] update should have failed"
false
- elif test -f lib && test "$(cat lib)" != "hello"
+ elif test "$(cat lib)" != "hello"
then
echo "[OOPS] update failed but lib file was molested"
false
--
1.5.2.1.914.gbd3a7
^ permalink raw reply related
* [PATCH 1/3] git-submodule: allow submodule name and path to differ
From: Lars Hjemli @ 2007-06-09 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven Verdoolaege, git
In-Reply-To: <1181425132239-git-send-email-hjemli@gmail.com>
This teaches git-submodule to check module.*.path when looking for the
config for a submodule path. If no match is found it falls back to the
current behaviour (module.$path).
With this change a submodule can be checked out at different paths in
different revisions of the superproject without changing the submodule
properties in .git/config.
While at it, add a new testscript for named submodules. This is basically
the same tests as can be found in t7400, but modified for a submodule
with name != path.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/git-submodule.txt | 16 ++++---
git-submodule.sh | 44 +++++++++++++------
...submodule-basic.sh => t7401-submodule-named.sh} | 27 ++++++++----
3 files changed, 59 insertions(+), 28 deletions(-)
copy t/{t7400-submodule-basic.sh => t7401-submodule-named.sh} (82%)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index f8fb80f..176e16d 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -24,8 +24,8 @@ status::
init::
Initialize the submodules, i.e. register in .git/config each submodule
- path and url found in .gitmodules. The key used in git/config is
- `submodule.$path.url`. This command does not alter existing information
+ name and url found in .gitmodules. The key used in git/config is
+ `submodule.$name.url`. This command does not alter existing information
in .git/config.
update::
@@ -50,11 +50,15 @@ OPTIONS
FILES
-----
-When initializing submodules, a .gitmodules file in the top-level directory
-of the containing repository is used to find the url of each submodule.
-This file should be formatted in the same way as $GIR_DIR/config. The key
-to each submodule url is "module.$path.url".
+The .gitmodules file in the top-level directory of the containing repository
+is used to map submodule path to submodule name. The map consist of keys
+named `module.$name.path` with a value matching the path found in the index of
+the containing repository. If no such key is found for a specific path, the
+submodule path is used as submodule name.
+During `git-submodule init` the url for each submodule is registered in
+.git/config of the containing repository. The url is located under the key
+`module.$name.url` in .gitmodules.
AUTHOR
------
diff --git a/git-submodule.sh b/git-submodule.sh
index 8bdd99a..d9f0c91 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -25,6 +25,18 @@ say()
fi
}
+#
+# Resolve submodule name from path, use path as name if no mapping exist
+#
+# $1 = path
+#
+module_name()
+{
+ name=$(GIT_CONFIG=.gitmodules git-config --get-regexp '^module\..*\.path$' "$1" |
+ sed -nre 's/^module\.(.+)\.path .+$/\1/p')
+ test -z "$name" && name="$1"
+ echo $name
+}
#
# Clone a submodule
@@ -62,18 +74,19 @@ modules_init()
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
do
- # Skip already registered paths
- url=$(git-config submodule."$path".url)
+ # Skip already registered submodules
+ name=$(module_name "$path")
+ url=$(git-config submodule."$name".url)
test -z "$url" || continue
- url=$(GIT_CONFIG=.gitmodules git-config module."$path".url)
+ url=$(GIT_CONFIG=.gitmodules git-config module."$name".url)
test -z "$url" &&
- die "No url found for submodule '$path' in .gitmodules"
+ die "No url found for submodule '$name', path '$path' in .gitmodules"
- git-config submodule."$path".url "$url" ||
- die "Failed to register url for submodule '$path'"
+ git-config submodule."$name".url "$url" ||
+ die "Failed to register url for submodule '$name', path '$path'"
- say "Submodule '$path' registered with url '$url'"
+ say "Submodule '$name', path '$path' registered with url '$url'"
done
}
@@ -87,13 +100,14 @@ modules_update()
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
do
- url=$(git-config submodule."$path".url)
+ name=$(module_name "$path")
+ url=$(git-config submodule."$name".url)
if test -z "$url"
then
# Only mention uninitialized submodules when its
# path have been specified
test "$#" != "0" &&
- say "Submodule '$path' not initialized"
+ say "Submodule '$name', path '$path' not initialized"
continue
fi
@@ -104,15 +118,15 @@ modules_update()
subsha1=$(unset GIT_DIR && cd "$path" &&
git-rev-parse --verify HEAD) ||
- die "Unable to find current revision of submodule '$path'"
+ die "Unable to find current revision of submodule '$name', path '$path'"
if test "$subsha1" != "$sha1"
then
(unset GIT_DIR && cd "$path" && git-fetch &&
git-checkout -q "$sha1") ||
- die "Unable to checkout '$sha1' in submodule '$path'"
+ die "Unable to checkout '$sha1' in submodule '$name', path '$path'"
- say "Submodule '$path': checked out '$sha1'"
+ say "Submodule '$name', path '$path': checked out '$sha1'"
fi
done
}
@@ -132,10 +146,12 @@ modules_list()
git ls-files --stage -- "$@" | grep -e '^160000 ' |
while read mode sha1 stage path
do
- if ! test -d "$path"/.git
+ name=$(module_name "$path")
+ url=$(git-config submodule."$name".url)
+ if test -z "$url" || ! test -d "$path"/.git
then
say "-$sha1 $path"
- continue;
+ continue
fi
revname=$(unset GIT_DIR && cd "$path" && git-describe $sha1)
if git diff-files --quiet -- "$path"
diff --git a/t/t7400-submodule-basic.sh b/t/t7401-submodule-named.sh
similarity index 82%
copy from t/t7400-submodule-basic.sh
copy to t/t7401-submodule-named.sh
index 3940433..3c3d195 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7401-submodule-named.sh
@@ -3,10 +3,10 @@
# Copyright (c) 2007 Lars Hjemli
#
-test_description='Basic porcelain support for submodules
+test_description='Basic porcelain support for named submodules
This test tries to verify basic sanity of the init, update and status
-subcommands of git-submodule.
+subcommands of git-submodule when a submodule name differs from its path.
'
. ./test-lib.sh
@@ -18,7 +18,7 @@ subcommands of git-submodule.
# -add directory lib to 'superproject', this creates a DIRLINK entry
# -add a couple of regular files to enable testing of submodule filtering
# -mv lib subrepo
-# -add an entry to .gitmodules for path 'lib'
+# -add an entry to .gitmodules for submodule 'foo'
#
test_expect_success 'Prepare submodule testing' '
mkdir lib &&
@@ -40,7 +40,7 @@ test_expect_success 'Prepare submodule testing' '
git-add a lib z &&
git-commit -m "super commit 1" &&
mv lib .subrepo &&
- GIT_CONFIG=.gitmodules git-config module.lib.url git://example.com/lib.git
+ GIT_CONFIG=.gitmodules git-config module.foo.url git://example.com/libfoo.git
'
test_expect_success 'status should only print one line' '
@@ -52,14 +52,25 @@ test_expect_success 'status should initially be "missing"' '
git-submodule status | grep "^-$rev1"
'
+test_expect_success 'init should fail when map from path to name is missing' '
+ if git-submodule init
+ then
+ echo "[OOPS] init should have failed"
+ false
+ elif ! GIT_CONFIG=.gitmodules git-config module.foo.path lib
+ then
+ echo "[OOPS] init failed but so did git-config"
+ fi
+'
+
test_expect_success 'init should register submodule url in .git/config' '
git-submodule init &&
- url=$(git-config submodule.lib.url) &&
- if test "$url" != "git://example.com/lib.git"
+ url=$(git-config submodule.foo.url) &&
+ if test "$url" != "git://example.com/libfoo.git"
then
echo "[OOPS] init succeeded but submodule url is wrong"
false
- elif ! git-config submodule.lib.url ./.subrepo
+ elif ! git-config submodule.foo.url ./.subrepo
then
echo "[OOPS] init succeeded but update of url failed"
false
@@ -72,7 +83,7 @@ test_expect_success 'update should fail when path is used by a file' '
then
echo "[OOPS] update should have failed"
false
- elif test -f lib && test "$(cat lib)" != "hello"
+ elif test "$(cat lib)" != "hello"
then
echo "[OOPS] update failed but lib file was molested"
false
--
1.5.2.1.914.gbd3a7
^ permalink raw reply related
* [PATCH 0/3] submodule improvements
From: Lars Hjemli @ 2007-06-09 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven Verdoolaege, git
This is a series of somewhat unrelated changes, but all related to
git-submodule.
1/3 implements a mapping between submodule path and submodule name
2/3 adds a gitmodules(5) document
3/3 fixes a bug in testscript t7400, noticed when making 1/3
The stats:
Documentation/Makefile | 2 +-
Documentation/git-submodule.txt | 16 +++--
Documentation/gitmodules.txt | 63 ++++++++++++++++++++
git-submodule.sh | 44 +++++++++----
t/t7400-submodule-basic.sh | 2 +-
...submodule-basic.sh => t7401-submodule-named.sh} | 27 ++++++---
6 files changed, 124 insertions(+), 30 deletions(-)
^ permalink raw reply
* [PATCH 2/3] Add gitmodules(5)
From: Lars Hjemli @ 2007-06-09 21:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sven Verdoolaege, git
In-Reply-To: <1181425132239-git-send-email-hjemli@gmail.com>
This adds documentation for the .gitmodules file.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/Makefile | 2 +-
Documentation/gitmodules.txt | 63 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 1 deletions(-)
create mode 100644 Documentation/gitmodules.txt
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 9cef480..2ad18e0 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -2,7 +2,7 @@ MAN1_TXT= \
$(filter-out $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
$(wildcard git-*.txt)) \
gitk.txt
-MAN5_TXT=gitattributes.txt gitignore.txt
+MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt
MAN7_TXT=git.txt
DOC_HTML=$(patsubst %.txt,%.html,$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT))
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
new file mode 100644
index 0000000..37888c9
--- /dev/null
+++ b/Documentation/gitmodules.txt
@@ -0,0 +1,63 @@
+gitmodules(5)
+=============
+
+NAME
+----
+gitmodules - defining submodule properties
+
+SYNOPSIS
+--------
+.gitmodules
+
+
+DESCRIPTION
+-----------
+
+The `.gitmodules` file, located in the top-level directory of a
+gitlink:git[7] working tree, is a text file with a layout matching the
+requirements of gitlink:git-config[1].
+
+The file consists of sections named `module`, divided into one subsection
+per submodule. The subsections are named with the logical name of the
+submodule it describes.
+
+Each submodule can contain the following keys.
+
+module.$name.path::
+ Define a path, relative to the top-level directory of the git
+ working tree, where the submodule is expected to be checked out.
+
+module.$name.url::
+ Define a url from where the submodule repository can be cloned.
+
+
+EXAMPLES
+--------
+
+Consider the following .gitmodules file:
+
+ [module 'libfoo']
+ path = include/foo
+ url = git://example1.com/git/libfoo.git
+
+ [module 'libbar']
+ url = git://example2.com/pub/git/libbar.git
+
+
+This defines two submodules, `libfoo` and `libbar`. The former specifies
+both a checkout path and a suggested url, while the latter only specifies
+a url. This file would make gitlink:git-submodule[1] map the path
+`include/foo` to the submodule `libfoo` and the path `libbar` to the
+submodule `libbar`.
+
+SEE ALSO
+--------
+gitlink:git-submodule[1] gitlink:git-config[1]
+
+DOCUMENTATION
+-------------
+Documentation by Lars Hjemli <hjemli@gmail.com>
+
+GIT
+---
+Part of the gitlink:git[7] suite
--
1.5.2.1.914.gbd3a7
^ permalink raw reply related
* Re: [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line'
From: Johan Herland @ 2007-06-09 21:34 UTC (permalink / raw)
To: git; +Cc: Alex Riesen, Junio C Hamano, Johannes Schindelin
In-Reply-To: <81b0412b0706091426x5cc496aft788376872fc56995@mail.gmail.com>
On Saturday 09 June 2007, Alex Riesen wrote:
> On 6/9/07, Johan Herland <johan@herland.net> wrote:
> > Signed-off-by: Johan Herland <johan@herland.net>
> > ---
> > mktag.c | 29 ++++++++++++++++-------------
> > 1 files changed, 16 insertions(+), 13 deletions(-)
>
> What is this change good for?
> How did you justify the type selection for your
> loop index variable?
>
> IOW, the patch looks very useless.
I agree. By itself, the patch is useless.
However, if you look at the next patch, you'll see that this exact piece of
code is moved from verify_tag() to parse_and_verify_tag_buffer(), and in
the new context, we can't increment tag_line, since the code that follows
depends on tag_line not being moved.
In other words this patch is here so that the next patch will be easier to
follow. because it's _literally_ moving copying code from verify_tag() and
pasting it in parse_and_verify_tag_buffer().
I'm sorry if this is not clear from the patches.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH 07/21] Copy the remaining differences from verify_tag() to parse_tag_buffer_internal()
From: Alex Riesen @ 2007-06-09 21:31 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090215.31731.johan@herland.net>
On 6/9/07, Johan Herland <johan@herland.net> wrote:
> + /* Verify the tag-name: we don't allow control characters or spaces in it */
> + for (i = 4;;) {
> + unsigned char c = tag_line[i++];
> + if (c == '\n')
> + break;
> + if (c > ' ')
> + continue;
> + return error("char" PD_FMT ": could not verify tag name", tag_line + i - data);
> + }
This looks very familiar. Haven't you just made a very useless patch
which had this very same code? How about putting it in its own
function and just call it from these two places? And what problem
do you have with pointers?!
^ permalink raw reply
* Re: [PATCH 06/21] Refactor tag name verification loop to use index 'i' instead of incrementing pointer 'tag_line'
From: Alex Riesen @ 2007-06-09 21:26 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706090215.05318.johan@herland.net>
On 6/9/07, Johan Herland <johan@herland.net> wrote:
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
> mktag.c | 29 ++++++++++++++++-------------
> 1 files changed, 16 insertions(+), 13 deletions(-)
What is this change good for?
How did you justify the type selection for your
loop index variable?
IOW, the patch looks very useless.
^ permalink raw reply
* Re: fatal: serious inflate inconsistency
From: Thomas Glanzmann @ 2007-06-09 21:12 UTC (permalink / raw)
To: Marco Roeland; +Cc: Linus Torvalds, Nicolas Pitre, GIT, Michael Gernoth
In-Reply-To: <20070609001333.GA4058@cip.informatik.uni-erlangen.de>
Hello again,
the machine that had the issues just got down because of a broken power
supply and took half the rack with it. So it was definitive a hardware
fault. Sorry for all the trouble. And the bottom line is: It was _not_
git, it was broken hardware!
Thomas
^ permalink raw reply
* Re: [PATCH 1/2] filter-branch: Simplify parent computation.
From: Junio C Hamano @ 2007-06-09 20:44 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Johannes Schindelin
In-Reply-To: <200706082328.39758.johannes.sixt@telecom.at>
Johannes Sixt <johannes.sixt@telecom.at> writes:
> @@ -332,18 +321,19 @@ mkdir ../map # map old->new commit ids for rewriting
> parents
>
Crap. Why is this patch line-wrapped X-<.
> ...
> i=0
> -while read commit; do
> +while read commit parents; do
> i=$(($i+1))
> printf "\rRewriting commits... ($i/$commits)"
>
Crap. Where did that "\rRewriting commits..." come from?
The other Johannes had the same in his patch.
> @@ -398,7 +388,7 @@ while read commit; do
> done > ../map/$commit
> done <../revs
>
> -src_head=$(tail -n 1 ../revs)
> +src_head=$(tail -n 1 ../revs | sed -e 's/ .*//')
> target_head=$(head -n 1 ../map/$src_head)
> case "$target_head" in
> '')
Crap. I seem to have "tee" there that stores them to ../map/$commit.
Anyway, I've manually fixed up the offending three patches (two
from you and one from the other Johannes) and pushed the results
out on 'next'.
^ permalink raw reply
* [PATCH] git-remote show: Also shorten non-fast-forward refs in the 'push' listing
From: Johannes Sixt @ 2007-06-09 20:34 UTC (permalink / raw)
To: git
'git-remote show remote-name' lists the refs that are pushed to the remote
by showing the 'Push' line from the config file. But before showing it,
it shortened 'refs/heads/here:refs/heads/there' to 'here:there'. However,
if the Push line is prefixed with a plus, the ref was not shortened.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-remote.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/git-remote.perl b/git-remote.perl
index 5763799..b59cafd 100755
--- a/git-remote.perl
+++ b/git-remote.perl
@@ -258,6 +258,7 @@ sub show_remote {
if ($info->{'PUSH'}) {
my @pushed = map {
s|^refs/heads/||;
+ s|^\+refs/heads/|+|;
s|:refs/heads/|:|;
$_;
} @{$info->{'PUSH'}};
--
1.5.2
^ permalink raw reply related
* Re: git-svn dcommit failure
From: Florian Weimer @ 2007-06-09 20:22 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20070609200527.GC32225@muzzle>
* Eric Wong:
>> If such bugs turn up, I'd try to debug them on
>> my own and submit a fix, or at least a reproduction recipe. (The
>> repository itself may contain personally identifiable information in
>> test cases and hence, cannot be shared.)
>
> Was the commit modifying a file that was just one directory deep from
> the project you were working on?
Yes, it was.
^ permalink raw reply
* Re: git-svn "cannot lock ref" error during fetch
From: Eric Wong @ 2007-06-09 20:06 UTC (permalink / raw)
To: James Peach; +Cc: git
In-Reply-To: <6C4E89FC-CE40-4A76-A297-E0AB7A1F2F75@mac.com>
James Peach <jamespeach@mac.com> wrote:
> On Jun 6, 2007, at 11:31 PM, Eric Wong wrote:
>
> >James Peach <jamespeach@mac.com> wrote:
> >>Hi all,
> >>
> >>I'm new to git, and I'm experimenting with using git-svn to interact
> >>with a large SVN repository with lots of branches.
> >>
> >>I initially did an init like this:
> >>
> >>git-svn init -t tags -b branches -T trunk svn+ssh://server/svn/
> >>project
> >>
> >>Then I did a git-svn fetch, which started pulling all the branches.
> >>After a while, however, it hit a branch that it couldn't pull:
> >>
> >>Found branch parent: (tags/project-92~9)
> >>767f1f1601a4deae459c99ea6c1d1b9ba8f57a65
> >>Following parent with do_update
> >>...
> >>Successfully followed parent
> >>fatal: refs/remotes/tags/project-92~9: cannot lock the ref
> >>update-ref -m r13726 refs/remotes/tags/project-92~9
> >>950638ff72acc278156a0d55baafbabb43f2b772: command returned error: 128
> >>
> >>Some amount of searching failed to turn up any hints on what this
> >>error means or how I can work around it. I'd appreciate any
> >>advice ...
> >
> >Is there a tag actually named "project-92~9"?
>
> unfortunately, there's more than one tag with this naming convention :(
>
> > If so, it's
> >an invalid branch name for git. I started working on a way
> >around it by mapping new names to it, but haven't gotten around to
> >finishing it....
>
> Aha! I don't really need 90% of the tags - is there a way I can tell
> the "git-svn fetch" not to bother with particular tags?
You can manually add fetch = foo:bar lines in the [svn-remote "svn"]
section of your .git/config file. See the one for trunk. Then
remove the tags= wildcard config line.
--
Eric Wong
^ permalink raw reply
* Re: git-svn dcommit failure
From: Eric Wong @ 2007-06-09 20:05 UTC (permalink / raw)
To: Florian Weimer; +Cc: git
In-Reply-To: <87abvb508z.fsf@mid.deneb.enyo.de>
Florian Weimer <fw@deneb.enyo.de> wrote:
> * Eric Wong:
>
> >> After manually committing the offending changes via Subversion, I
> >> git-svn has begun to work again, too (but I haven't yet tried to
> >> change the same files).
> >
> > Would upgrading to 1.5.2 final have helped?
>
> I've upgraded to this version, but since it's not exactly trivial for
> me to test this on the Subversion side, it'll take some time until I
> can be confident that the bug was indeed fixed in the
> 6442754d6cc0056cf5b69b43d218f8b6d317e7f5 revision.
>
> Is there some kind of magic environment variable I could set to get
> tracing information?
Nope. I usually strace protocol or put print statements all over the
code :x
> If such bugs turn up, I'd try to debug them on
> my own and submit a fix, or at least a reproduction recipe. (The
> repository itself may contain personally identifiable information in
> test cases and hence, cannot be shared.)
Was the commit modifying a file that was just one directory deep from
the project you were working on?
--
Eric Wong
^ permalink raw reply
* Re: Attributes for commits
From: Robin Rosenberg @ 2007-06-09 18:48 UTC (permalink / raw)
To: Paul Franz; +Cc: git
In-Reply-To: <466AA714.4050500@comcast.net>
lördag 09 juni 2007 skrev Paul Franz:
> I am a ClearCase administrator and one of the things that I love
> about it is the ability to assign attributes to versions of files. Is
> there anybody thinking of adding the ability to assign attributes to a
> commit? I ask because I was thinking about how I would move from
> ClearCase to git and part of our process is to assign the BugNum
> attribute to every checkin so that we can see what bugs have been fixed.
> We also track the files checked in the bug tracking software to. Thus
> giving us a two way linkage between repository and the bug tracking
> system. This is VERY useful. And I was wondering if there are any
> thought to this for commits.
>
> Now, I will be honest it is possible that this has already been done (I
> have not read all the documentation yet) and I am justing wasting
> bandwidth. In which, please excuse my ignorance and tell me to just RTFM.
>
> Paul Franz
>
We use the convention of putting the bug number as the first work on the first line of
the checkin, e.g. "3399 Add attributes for commits". The advantage over other methods
is that it is always there, without the need to further investigations of assigned attributes.
This convention has nothing to do with Git though. It works with any type of repo.
-- robin
^ permalink raw reply
* [PATCH] Silence error messages unless 'thorough_verify' is set
From: Johan Herland @ 2007-06-09 19:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <200706092028.54459.johan@herland.net>
We don't want to print error message on regular parsing of tag objects.
With this patch error messages are only printed when 'thorough_verify'
is set, i.e. when creating new or fscking tag objects.
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
On Saturday 09 June 2007, Johan Herland wrote:
> On Saturday 09 June 2007, Junio C Hamano wrote:
> > While I would agree that it makes sense to have the same
> > definition of what is and is not a 100% well formatted tag
> > object for producer side and consumer side, I do not necessarily
> > think it is a good idea to make parse_tag_buffer() chattier on
> > errors. mktag.c:verify_tag() can afford to be verbose in its
> > diagnosis, because it is used when the user is _creating_ the
> > tag, and it is generally a good idea to be strict when we
> > create.
> >
> > On the other hand, parse_tag_buffer() is on the side of users
> > who use existing tag objects that were produced by somebody
> > else. It is generally a good practice to be more lenient when
> > you are consuming.
> >
> > Also, callers of parse_tag_buffer() know the function is silent
> > on errors (unless there is something seriously wrong with the
> > repository); they do their own error messages when they get an
> > error return.
>
> Ok. I can make the error messages conditional on 'thorough_verify'.
Is this one ok?
(It goes on top of the patch series as a whole)
...Johan
tag.c | 34 +++++++++++++++++++---------------
1 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/tag.c b/tag.c
index af4356e..c3a2855 100644
--- a/tag.c
+++ b/tag.c
@@ -67,6 +67,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
#define PD_FMT "%td"
#endif
+#define FAIL(...) ( thorough_verify ? error(__VA_ARGS__) : -1 )
+
unsigned char sha1[20];
char type[20];
const char *type_line, *tag_line, *keywords_line, *tagger_line;
@@ -80,26 +82,26 @@ int parse_and_verify_tag_buffer(struct tag *item,
}
if (size < 65)
- return error("Tag object failed preliminary size check");
+ return FAIL("Tag object failed preliminary size check");
/* Verify mandatory object line */
if (prefixcmp(data, "object "))
- return error("Tag object (@ char 0): "
+ return FAIL("Tag object (@ char 0): "
"Does not start with \"object \"");
if (get_sha1_hex(data + 7, sha1))
- return error("Tag object (@ char 7): Could not get SHA1 hash");
+ return FAIL("Tag object (@ char 7): Could not get SHA1 hash");
/* Verify mandatory type line */
type_line = data + 48;
if (prefixcmp(type_line - 1, "\ntype "))
- return error("Tag object (@ char 47): "
+ return FAIL("Tag object (@ char 47): "
"Could not find \"\\ntype \"");
/* Verify optional tag line */
tag_line = strchr(type_line, '\n');
if (!tag_line++)
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not find \"\\n\" after \"type\"",
type_line - data);
if (prefixcmp(tag_line, "tag ")) /* no tag name given */
@@ -107,7 +109,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
else { /* tag name given */
keywords_line = strchr(tag_line, '\n');
if (!keywords_line++)
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not find \"\\n\" after \"tag\"",
tag_line - data);
}
@@ -118,7 +120,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
else { /* keywords given */
tagger_line = strchr(keywords_line, '\n');
if (!tagger_line++)
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not find \"\\n\" after \"keywords\"",
keywords_line - data);
}
@@ -132,7 +134,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
*/
if (prefixcmp(tagger_line, "tagger ")) { /* no tagger given */
if (thorough_verify)
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not find \"tagger \"",
tagger_line - data);
header_end = tagger_line;
@@ -140,7 +142,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
else { /* tagger given */
header_end = strchr(tagger_line, '\n');
if (!header_end++)
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not find \"\\n\" after \"tagger\"",
tagger_line - data);
}
@@ -164,7 +166,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
/* Get the actual type */
if (type_len >= sizeof(type))
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Type too long", type_line - data);
memcpy(type, type_line, type_len);
type[type_len] = '\0';
@@ -174,7 +176,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
/* Verify that the object matches */
if (verify_object(sha1, type))
- return error("Tag object (@ char 7): "
+ return FAIL("Tag object (@ char 7): "
"Could not verify tagged object %s",
sha1_to_hex(sha1));
@@ -184,7 +186,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
unsigned char c = tag_line[i];
if (c > ' ' && c != 0x7f)
continue;
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not verify tag name",
tag_line + i - data);
}
@@ -199,12 +201,12 @@ int parse_and_verify_tag_buffer(struct tag *item,
unsigned char c = keywords_line[i];
if (c == ',' && keywords_line[i + 1] == ',')
/* consecutive commas */
- return error("Tag object (@ char "
+ return FAIL("Tag object (@ char "
PD_FMT "): Found empty keyword",
keywords_line + i - data);
if (c > ' ' && c != 0x7f)
continue;
- return error("Tag object (@ char " PD_FMT "): "
+ return FAIL("Tag object (@ char " PD_FMT "): "
"Could not verify keywords",
keywords_line + i - data);
}
@@ -248,7 +250,7 @@ int parse_and_verify_tag_buffer(struct tag *item,
} else if (!strcmp(type, tag_type)) {
item->tagged = &lookup_tag(sha1)->object;
} else {
- error("Tag object (@ char " PD_FMT "): "
+ FAIL("Tag object (@ char " PD_FMT "): "
"Unknown type '%s'", type_line - data, type);
item->tagged = NULL;
}
@@ -262,6 +264,8 @@ int parse_and_verify_tag_buffer(struct tag *item,
return 0;
+#undef FAIL
+
#undef PD_FMT
}
--
1.5.2.1.144.gabc40
^ permalink raw reply related
* Re: Is this an acceptable workflow in git-svn, or a user error?
From: Eric Wong @ 2007-06-09 19:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7itdjv6.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <gitster@pobox.com> wrote:
> Suppose you are interacting with svn://some.where/ and you want
> to have a public git://my.svn/mirror.git maintained with your
> effort for others to consume. I am wondering if the following
> workflow, especially the push part, is kosher.
>
> ----------------------------------------------------------------
> 0. Priming the process
>
> $ (mkdir mirror.git && cd mirror.git && git init)
>
> $ mkdir private.git && cd private.git && git init
> $ git svn init svn://some.where/
> $ git push ../mirror.git/ refs/heads/master:refs/heads/master
>
> After this step, mirror.git would be in-sync with the SVN; we
> haven't done any of our own development yet.
>
> 1. Hack away
>
> $ edit/git-add/etc.
> $ git commit
>
> 2. Publish the result
>
> git push ../mirror.git/
>
> 3. Re-sync with SVN
>
> $ git svn dcommit
> ----------------------------------------------------------------
>
> I suspect that the above sequence is a user error, in that
> re-syncing with SVN using git-svn (either dcommit or rebase)
> would need to redo the commits to embed SVN metadata, but the
> public mirror now has commits made with git without such
> rewriting. In other words, I suspect that "git push" should
> never be done if you made changes on the git side since you
> sync'ed with the SVN.
Yes. This is an unfortunate sore point for git-svn users unfamiliar
with what rebase does (inside dcommit). Users using the the old
'set-tree' workflow are immune to this, but they have their own set of
complications...
Using "git push --force" would work after dcommit, however that
makes extra work for people tracking the git mirror
> The reason I am asking is that somebody on #git got in a very
> confusing situation. After the Re-sync, "git push" to the
> mirror would refuse because it is not a fast forward. "git
> fetch" from the mirror reveals that what after 3. is done, we
> see two identically-looking commits, one that was pushed (before
> re-sync with SVN) and the other (replaced because of re-sync)
> are different.
>
> If that is the case, then I presume that the correct workflow
> would be:
>
> 1. Hack away
> 2. Re-sync with SVN
> 3. Publish the result; do not do anything between 2. and 3.
>
> Is my understanding correct?
Yes. Since dcommit uses rebase, it'll rewrite history.
The "git svn dcommit" workflow is analogous to me making commits and
mailing patches to you to apply to the mainline git.git. When you apply
them to git.git, they'll be different than the commits I've made even
though they're only metadata changes.
--
Eric Wong
^ permalink raw reply
* Is this an acceptable workflow in git-svn, or a user error?
From: Junio C Hamano @ 2007-06-09 19:10 UTC (permalink / raw)
To: Eric Wong; +Cc: git
Suppose you are interacting with svn://some.where/ and you want
to have a public git://my.svn/mirror.git maintained with your
effort for others to consume. I am wondering if the following
workflow, especially the push part, is kosher.
----------------------------------------------------------------
0. Priming the process
$ (mkdir mirror.git && cd mirror.git && git init)
$ mkdir private.git && cd private.git && git init
$ git svn init svn://some.where/
$ git push ../mirror.git/ refs/heads/master:refs/heads/master
After this step, mirror.git would be in-sync with the SVN; we
haven't done any of our own development yet.
1. Hack away
$ edit/git-add/etc.
$ git commit
2. Publish the result
git push ../mirror.git/
3. Re-sync with SVN
$ git svn dcommit
----------------------------------------------------------------
I suspect that the above sequence is a user error, in that
re-syncing with SVN using git-svn (either dcommit or rebase)
would need to redo the commits to embed SVN metadata, but the
public mirror now has commits made with git without such
rewriting. In other words, I suspect that "git push" should
never be done if you made changes on the git side since you
sync'ed with the SVN.
The reason I am asking is that somebody on #git got in a very
confusing situation. After the Re-sync, "git push" to the
mirror would refuse because it is not a fast forward. "git
fetch" from the mirror reveals that what after 3. is done, we
see two identically-looking commits, one that was pushed (before
re-sync with SVN) and the other (replaced because of re-sync)
are different.
If that is the case, then I presume that the correct workflow
would be:
1. Hack away
2. Re-sync with SVN
3. Publish the result; do not do anything between 2. and 3.
Is my understanding correct?
^ permalink raw reply
* Re: [StGIT PATCH 0/2] Fix issues with series deletion
From: Yann Dirson @ 2007-06-09 18:43 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0706071450u1587d109x1e592bfa453c0c0@mail.gmail.com>
On Thu, Jun 07, 2007 at 10:50:00PM +0100, Catalin Marinas wrote:
> On 06/06/07, Yann Dirson <ydirson@altern.org> wrote:
> >I am however not happy at all with the way we delete patches and
> >series, starting with an existence check and then deleting. If any
> >error occurs midway, then we are left with an inconsistent state that
> >the user has to cleanup by hand. IMHO, we should have those methods
> >be as robust as possible, maybe starting by removing the formatversion
> >item, and printing a "cleaning up zombie stack" if does not find it.
> >So at least after fixing a "delete" bug, we could rerun the same
> >command and get to a sane state again.
>
> This sounds OK for a quick fix. Longer term, I think we should support
> some kind of transactions. One idea is to put the StGIT metadata in a
> single file (or maybe two if we include the config) that gets checked
> in after every operation.
Speaking of transactions, did you have a chance to read the proposal I
posted some time ago ? As stated in another mail, I fear that
approach does generalize easily to core git - but for lack of a better
solution, we may want to go this way anyway...
Best regards,
--
Yann
^ permalink raw reply
* Re: That improved git-gui blame viewer..
From: Junio C Hamano @ 2007-06-09 18:29 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Shawn O. Pearce, Git Mailing List
In-Reply-To: <alpine.LFD.0.98.0706091117510.20321@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Shawn, could you please push it out somewhere else than just "pu", and
> convince Junio to merge it? Right now, "git gui blame" is almost useless
I do not need convincing but "Hey, pull it" needs to be given,
especially as I do not use git-gui myself. The last time I
looked at it, I think I said I liked it much better than the
previous one.
^ permalink raw reply
* Re: [PATCH 02/21] Return error messages when parsing fails.
From: Johan Herland @ 2007-06-09 18:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <7vwsydf1m8.fsf@assigned-by-dhcp.cox.net>
On Saturday 09 June 2007, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
>
> > This patch brings the already similar tag.c:parse_tag_buffer() and
> > mktag.c:verify_tag() a little bit closer to eachother.
>
> While I would agree that it makes sense to have the same
> definition of what is and is not a 100% well formatted tag
> object for producer side and consumer side, I do not necessarily
> think it is a good idea to make parse_tag_buffer() chattier on
> errors. mktag.c:verify_tag() can afford to be verbose in its
> diagnosis, because it is used when the user is _creating_ the
> tag, and it is generally a good idea to be strict when we
> create.
>
> On the other hand, parse_tag_buffer() is on the side of users
> who use existing tag objects that were produced by somebody
> else. It is generally a good practice to be more lenient when
> you are consuming.
>
> Also, callers of parse_tag_buffer() know the function is silent
> on errors (unless there is something seriously wrong with the
> repository); they do their own error messages when they get an
> error return.
>
>
Ok. I can make the error messages conditional on 'thorough_verify'.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* That improved git-gui blame viewer..
From: Linus Torvalds @ 2007-06-09 18:26 UTC (permalink / raw)
To: Shawn O. Pearce, Junio C Hamano, Git Mailing List
Shawn, could you please push it out somewhere else than just "pu", and
convince Junio to merge it? Right now, "git gui blame" is almost useless
in the standard git tree, and maybe the new one has some bugs, but it's at
least _useful_ (and about a million times prettier than it used to be), so
I don't think you can possibly regress in this area.
That said, I do have one comment about the state of git-gui "pu".. I think
it's fairly pretty, and definitely useful, but one feature I end up really
wishing for is a "search" button (or Ctrl-F). I might not know what
line-number I'm looking for, I'm more likely to know which function I want
to look at, and the most natural way to find it is with a simple ctrl-F.
Maybe it exists, but if so, I didn't find it.
But the old git-gui blame viewer didn't have that either, so this
shouldn't hold up getting a useable git-gui into the standard git distro..
Linus
^ permalink raw reply
* Re: [PATCH] Port git-tag.sh to C.
From: Junio C Hamano @ 2007-06-09 18:26 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11813427591137-git-send-email-krh@redhat.com>
Kristian Høgsberg <krh@redhat.com> writes:
> Content-Type: TEXT/PLAIN; charset=ISO-8859-1
>
> From: Kristian Høgsberg <krh@redhat.com>
>
> A more or less straight-forward port of git-tag.sh to C.
>
> Signed-off-by: Kristian Høgsberg <krh@redhat.com>
> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>
I think your name in your commit message is in UTF-8 but munged your
mail was mismarked as iso-8859-1.
> +static int launch_editor(const char *path, const char *template,
> + char *buffer, size_t size)
> +{
It would have been nicer to have this in editor.c or somesuch,
as other commands will be redone in C in the future.
We could do the moving later, but the problem is that later is
conditional: "if we are lucky enough to remember that we already
have this function in builtin-tag when doing so".
> + fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0644);
I would understand an argument to use 0666 (honor umask) or 0600
(this is a temporary file and others have no business looking at
it while an edit is in progress), but I cannot justify 0644.
> + fd = open(path, O_RDONLY, 0644);
Open for reading with mode ;-)?
> + if (fd == -1)
> + die("could not read %s.", path);
> + len = read_in_full(fd, buffer, size);
> + if (len < 0)
> + die("failed to read '%s', %m", path);
> + close(fd);
> +
> + blank_lines = 1;
> + for (i = 0, j = 0; i < len; i++) {
> ...
> + }
> +
> + if (buffer[j - 1] != '\n')
> + buffer[j++] = '\n';
> +
> + unlink(path);
> +
> + return j;
> +}
I really think this function needs to be refactored into three.
* A generic "spawn an editor with this initial seed template,
return the result of editing in memory and also give exit
status of the editor" function that does not take path
parameter (instead perhaps mkstemp a temporary file on your
own);
* A function that does what git-stripspace does in core;
* A function for builtin-tag to use, that calls the above two
and uses the result (e.g. "did the user kill the editor?
does the resulting buffer have any nonempty line?") to decide
what it does.
> +static void create_tag(const unsigned char *object, const char *tag,
> + const char *message, int sign, unsigned char *result)
> +{
> + enum object_type type;
> + char buffer[4096];
> + int header, body, total;
> +
> + type = sha1_object_info(object, NULL);
> + if (type <= 0)
> + die("bad object type.");
> +
> + header = snprintf(buffer, sizeof buffer,
> + "object %s\n"
> + "type %s\n"
> + "tag %s\n"
> + "tagger %s\n\n",
> + sha1_to_hex(object),
> + typename(type),
> + tag,
> + git_committer_info(1));
> +
> + if (message == NULL)
> + body = launch_editor(git_path("TAGMSG"), tag_template,
> + buffer + header, sizeof buffer - header);
> + else
> + body = snprintf(buffer + header, sizeof buffer - header,
> + "%s\n", message);
> +
> + if (body == 0)
> + die("no tag message?");
> +
> + if (header + body > sizeof buffer)
> + die("tag message too big.");
Two issues:
* It used to be a tag had limit of 8kB which was lifted some
time ago; now it is limited to 4kB. Fixing this implies that
the "launch editor and get results in core" function I
mentioned above may need to realloc, and probably the buffer
is better passed as (char *, ulong) pair as done everywhere
else (although we know this is text so you can pass only a
pointer and have the user run strlen() when needed).
* I do not see any validation on the value of "tag". Do we want
to allow passing "" to it? What about "my\ntag"?
^ permalink raw reply
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