* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Sébastien Cevey @ 2008-12-05 2:32 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <200812050308.52891.jnareb@gmail.com>
At Fri, 5 Dec 2008 03:08:52 +0100, Jakub Narebski wrote:
> Nice... but perhaps it would be better to simply pass $from / $to to
> build_projlist_by_category function, and have in %categories only
> projects which are shown...
Ah you're right, we could do that, I hadn't thought of it. Sounds
cleaner than the $from/$to dance I did in this patch.
> well, unless filtered out in print_project_rows() by $show_ctags; so
> I think that there is nonzero probability of empty (no project
> shown) categories.
Mh indeed, in fact this could happen any time we reach one of the
'next' statements in the loop in print_project_rows(), when performing
search, tag filtering, etc...
Actually, assuming the project list is split into page, this can also
lead to empty pages (if no project on the page matches the filter).
To avoid empty categories, it's a bit tricky since the header is
printed before we determine whether there are matching projects..
I need to read the code more carefully, but it seems that one solution
would be to add a function that determines whether a project should be
displayed or not (according to tags and search and forks); then we can
map this function on the list of projects.
I could do it if it sounds sane to you.
> I don't think that the games we play with $from / $to would be enough.
> Check what happens (I think that it wouldn't work correctly) if we have
> something like that:
>
> project | category | shown
> --------------------------
> 1 | A |
> 2 | A |
> 3 | B | Y
> 4 | C | Y
> 5 | B | Y
> 6 | C |
> 7 | C |
AFAIK this cannot happen with the current code, since projects are
grouped by category (according to the foreach on categories).
> I'll try to examine the code in more detail later... currently I don't
> know why but I can't git-am the second patch (this patch) correctly...
This is the third patch, are you sure you applied 1 and 2 before?
Thanks for your careful and supportive comments!
--
Sébastien Cevey / inso.cc
^ permalink raw reply
* Re: summaries in git add --patch[PATCH 1/2]
From: Junio C Hamano @ 2008-12-05 2:23 UTC (permalink / raw)
To: William Pursell; +Cc: git
In-Reply-To: <4937B456.7080604@gmail.com>
William Pursell <bill.pursell@gmail.com> writes:
> Thanks for pointing that out. Settings changed. I do appreciate
> you taking the time to essentially hold my hand through this
> process, and hope that I'm not causing you too much extra work.
Heh, I'll be saving extra work I have to do in the future by training you
how to produce patches line the ones I may write myself. By doing so,
eventually I wouldn't have to code anything myself ;-)
> +# Generate a one line summary of a hunk.
> +sub summarize_hunk {
> + my $rhunk = shift;
> + my $summary = $rhunk->{TEXT}[0];
> +
> + # Keep the line numbers, discard extra context.
> + $summary =~ s/@@(.*?)@@.*/$1 /s;
> + $summary .= " " x (20 - length $summary);
> +
> + # Add some user context.
> + for my $line (@{$rhunk->{TEXT}}) {
> + if ($line =~ m/^[+-].*\w/) {
> + $summary .= $line;
> + last;
> + }
> + }
> +
> + chomp $summary;
> + return substr($summary, 0, 80) . "\n";
> +}
I'll queue the patches in this round as-is in 'pu' and merge to 'next', as
we should stop slushing around at some point and start polishing on a
solid ground. But as you mentioned, these hardcoded 20 and 80 do not look
very nice.
I think the division of labor between the data producer (summarize_hunk)
and presenter (display_hunks) should be shifted somewhat, so that
* summarize_hunk returns a two-tuple:
[ $line_number_hint, $first_change ]
* display_hunks runs summarize_hunk for all 20 hunks and gathers the
return values before producing a single line of output, and then
computes the maximum $line_number_hint to decide how many extra SP to
use to pad it to uniform length (instead of " " x (20 - length)).
After doing so, it loops over the hunks, using the collected return
values and formats.
In later round of polishing, you might find out that some callers of
summarize_hunk may want to read the full line, not just the first 80
(perhaps they feed their output to "less -S"). By splitting the
responsibility between these functions in the way outlined above, you do
not have to modify summarize_hunk when that day comes.
> +
> +
> +# Print a one-line summary of each hunk in the array ref in
> +# the first argument, starting wih the index in the 2nd.
> +sub display_hunks {
> + my ($hunks, $i) = @_;
> + my $ctr = 0;
> + $i ||= 0;
> + for (; $i < @$hunks && $ctr < 20; $i++, $ctr++) {
> + my $status = " ";
> + if (defined $hunks->[$i]{USE}) {
> + $status = $hunks->[$i]{USE} ? "+" : "-";
> + }
> + printf "%s%2d: %s",
> + $status,
> + $i + 1,
> + summarize_hunk($hunks->[$i]);
> + }
By the way, I do not think this will align if you have more than 100
hunks. That is also a reason why I would suggest not to format/substr
inside the summarize_hunk function.
^ permalink raw reply
* Re: [PATCH] git-p4: Fix bug in p4Where method.
From: Junio C Hamano @ 2008-12-05 2:23 UTC (permalink / raw)
To: Tor Arvid Lund; +Cc: Simon Hausmann, git
In-Reply-To: <1228397853-15921-1-git-send-email-torarvid@gmail.com>
Thanks. Will apply to 'master' and will be in 1.6.1 final, unless some p4
users object (I do not use p4 myself, so that is the best I could do).
^ permalink raw reply
* [PATCH 4/4] Test that git-am does not lose -C/-p/--whitespace options
From: Junio C Hamano @ 2008-12-05 2:23 UTC (permalink / raw)
To: git
In-Reply-To: <1228443780-3386-4-git-send-email-gitster@pobox.com>
These tests make sure that "git am" does not lose command line options
specified when it was started, after it is interrupted by a patch that
does not apply earlier in the series.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t4252-am-options.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
t/t4252/am-test-1-1 | 19 +++++++++++++++++
t/t4252/am-test-1-2 | 21 +++++++++++++++++++
t/t4252/am-test-2-1 | 19 +++++++++++++++++
t/t4252/am-test-2-2 | 21 +++++++++++++++++++
t/t4252/am-test-3-1 | 19 +++++++++++++++++
t/t4252/am-test-3-2 | 21 +++++++++++++++++++
t/t4252/am-test-4-1 | 19 +++++++++++++++++
t/t4252/am-test-4-2 | 22 ++++++++++++++++++++
t/t4252/file-1-0 | 7 ++++++
t/t4252/file-2-0 | 7 ++++++
11 files changed, 229 insertions(+), 0 deletions(-)
create mode 100755 t/t4252-am-options.sh
create mode 100644 t/t4252/am-test-1-1
create mode 100644 t/t4252/am-test-1-2
create mode 100644 t/t4252/am-test-2-1
create mode 100644 t/t4252/am-test-2-2
create mode 100644 t/t4252/am-test-3-1
create mode 100644 t/t4252/am-test-3-2
create mode 100644 t/t4252/am-test-4-1
create mode 100644 t/t4252/am-test-4-2
create mode 100644 t/t4252/file-1-0
create mode 100644 t/t4252/file-2-0
diff --git a/t/t4252-am-options.sh b/t/t4252-am-options.sh
new file mode 100755
index 0000000..1a1946d
--- /dev/null
+++ b/t/t4252-am-options.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='git am not losing options'
+. ./test-lib.sh
+
+tm="$TEST_DIRECTORY/t4252"
+
+test_expect_success setup '
+ cp "$tm/file-1-0" file-1 &&
+ cp "$tm/file-2-0" file-2 &&
+ git add file-1 file-2 &&
+ test_tick &&
+ git commit -m initial &&
+ git tag initial
+'
+
+test_expect_success 'interrupted am --whitespace=fix' '
+ rm -rf .git/rebase-apply &&
+ git reset --hard initial &&
+ test_must_fail git am --whitespace=fix "$tm"/am-test-1-? &&
+ git am --skip &&
+ grep 3 file-1 &&
+ grep "^Six$" file-2
+'
+
+test_expect_success 'interrupted am -C1' '
+ rm -rf .git/rebase-apply &&
+ git reset --hard initial &&
+ test_must_fail git am -C1 "$tm"/am-test-2-? &&
+ git am --skip &&
+ grep 3 file-1 &&
+ grep "^Three$" file-2
+'
+
+test_expect_success 'interrupted am -p2' '
+ rm -rf .git/rebase-apply &&
+ git reset --hard initial &&
+ test_must_fail git am -p2 "$tm"/am-test-3-? &&
+ git am --skip &&
+ grep 3 file-1 &&
+ grep "^Three$" file-2
+'
+
+test_expect_success 'interrupted am -C1 -p2' '
+ rm -rf .git/rebase-apply &&
+ git reset --hard initial &&
+ test_must_fail git am -p2 -C1 "$tm"/am-test-4-? &&
+ cat .git/rebase-apply/apply_opt_extra &&
+ git am --skip &&
+ grep 3 file-1 &&
+ grep "^Three$" file-2
+'
+
+test_done
diff --git a/t/t4252/am-test-1-1 b/t/t4252/am-test-1-1
new file mode 100644
index 0000000..b0c09dc
--- /dev/null
+++ b/t/t4252/am-test-1-1
@@ -0,0 +1,19 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Three
+
+Application of this should be rejected because the first line in the
+context does not match.
+
+diff --git i/file-1 w/file-1
+index 06e567b..10f8342 100644
+--- i/file-1
++++ w/file-1
+@@ -1,6 +1,6 @@
+ One
+ 2
+-3
++Three
+ 4
+ 5
+ 6
diff --git a/t/t4252/am-test-1-2 b/t/t4252/am-test-1-2
new file mode 100644
index 0000000..1b874ae
--- /dev/null
+++ b/t/t4252/am-test-1-2
@@ -0,0 +1,21 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Six
+
+Applying this patch with --whitespace=fix should lose
+the trailing whitespace after "Six".
+
+diff --git i/file-2 w/file-2
+index 06e567b..b6f3a16 100644
+--- i/file-2
++++ w/file-2
+@@ -1,7 +1,7 @@
+ 1
+ 2
+-3
++Three
+ 4
+ 5
+-6
++Six
+ 7
diff --git a/t/t4252/am-test-2-1 b/t/t4252/am-test-2-1
new file mode 100644
index 0000000..feda94a
--- /dev/null
+++ b/t/t4252/am-test-2-1
@@ -0,0 +1,19 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Three
+
+Application of this should be rejected even with -C1 because the
+preimage line in the context does not match.
+
+diff --git i/file-1 w/file-1
+index 06e567b..10f8342 100644
+--- i/file-1
++++ w/file-1
+@@ -1,6 +1,6 @@
+ 1
+ 2
+-Tres
++Three
+ 4
+ 5
+ 6
diff --git a/t/t4252/am-test-2-2 b/t/t4252/am-test-2-2
new file mode 100644
index 0000000..2ac6600
--- /dev/null
+++ b/t/t4252/am-test-2-2
@@ -0,0 +1,21 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Six
+
+Applying this patch with -C1 should be successful even though
+the first line in the context does not match.
+
+diff --git i/file-2 w/file-2
+index 06e567b..b6f3a16 100644
+--- i/file-2
++++ w/file-2
+@@ -1,7 +1,7 @@
+ One
+ 2
+-3
++Three
+ 4
+ 5
+-6
++Six
+ 7
diff --git a/t/t4252/am-test-3-1 b/t/t4252/am-test-3-1
new file mode 100644
index 0000000..608e5ab
--- /dev/null
+++ b/t/t4252/am-test-3-1
@@ -0,0 +1,19 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Three
+
+Application of this should be rejected even with -p2 because the
+preimage line in the context does not match.
+
+diff --git i/junk/file-1 w/junk/file-1
+index 06e567b..10f8342 100644
+--- i/junk/file-1
++++ w/junk/file-1
+@@ -1,6 +1,6 @@
+ 1
+ 2
+-Tres
++Three
+ 4
+ 5
+ 6
diff --git a/t/t4252/am-test-3-2 b/t/t4252/am-test-3-2
new file mode 100644
index 0000000..0081b96
--- /dev/null
+++ b/t/t4252/am-test-3-2
@@ -0,0 +1,21 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Six
+
+Applying this patch with -p2 should be successful even though
+the patch is against a wrong level.
+
+diff --git i/junk/file-2 w/junk/file-2
+index 06e567b..b6f3a16 100644
+--- i/junk/file-2
++++ w/junk/file-2
+@@ -1,7 +1,7 @@
+ 1
+ 2
+-3
++Three
+ 4
+ 5
+-6
++Six
+ 7
diff --git a/t/t4252/am-test-4-1 b/t/t4252/am-test-4-1
new file mode 100644
index 0000000..e48cd6c
--- /dev/null
+++ b/t/t4252/am-test-4-1
@@ -0,0 +1,19 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Three
+
+Application of this should be rejected even with -C1 -p2 because
+the preimage line in the context does not match.
+
+diff --git i/junk/file-1 w/junk/file-1
+index 06e567b..10f8342 100644
+--- i/junk/file-1
++++ w/junk/file-1
+@@ -1,6 +1,6 @@
+ 1
+ 2
+-Tres
++Three
+ 4
+ 5
+ 6
diff --git a/t/t4252/am-test-4-2 b/t/t4252/am-test-4-2
new file mode 100644
index 0000000..0e69bfa
--- /dev/null
+++ b/t/t4252/am-test-4-2
@@ -0,0 +1,22 @@
+From: A U Thor <au.thor@example.com>
+Date: Thu Dec 4 16:00:00 2008 -0800
+Subject: Six
+
+Applying this patch with -C1 -p2 should be successful even though
+the patch is against a wrong level and the first context line does
+not match.
+
+diff --git i/junk/file-2 w/junk/file-2
+index 06e567b..b6f3a16 100644
+--- i/junk/file-2
++++ w/junk/file-2
+@@ -1,7 +1,7 @@
+ One
+ 2
+-3
++Three
+ 4
+ 5
+-6
++Six
+ 7
diff --git a/t/t4252/file-1-0 b/t/t4252/file-1-0
new file mode 100644
index 0000000..06e567b
--- /dev/null
+++ b/t/t4252/file-1-0
@@ -0,0 +1,7 @@
+1
+2
+3
+4
+5
+6
+7
diff --git a/t/t4252/file-2-0 b/t/t4252/file-2-0
new file mode 100644
index 0000000..06e567b
--- /dev/null
+++ b/t/t4252/file-2-0
@@ -0,0 +1,7 @@
+1
+2
+3
+4
+5
+6
+7
--
1.6.1.rc1.60.g1d1d7
^ permalink raw reply related
* [PATCH 3/4] git-am: propagate --3way options as well
From: Junio C Hamano @ 2008-12-05 2:22 UTC (permalink / raw)
To: git
In-Reply-To: <1228443780-3386-3-git-send-email-gitster@pobox.com>
The reasoning is the same as the previous patch, where we made -C<n>
and -p<n> propagate across a failure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-am.sh | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index ed54e71..13c02d6 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -247,10 +247,11 @@ else
exit 1
}
- # -s, -u, -k, --whitespace, -C and -p flags are kept
+ # -s, -u, -k, --whitespace, -3, -C and -p flags are kept
# for the resuming session after a patch failure.
- # -3 and -i can and must be given when resuming.
+ # -i can and must be given when resuming.
echo " $git_apply_opt" >"$dotest/apply_opt_extra"
+ echo "$threeway" >"$dotest/threeway"
echo "$sign" >"$dotest/sign"
echo "$utf8" >"$dotest/utf8"
echo "$keep" >"$dotest/keep"
@@ -283,6 +284,10 @@ if test "$(cat "$dotest/keep")" = t
then
keep=-k
fi
+if test "$(cat "$dotest/threeway")" = t
+then
+ threeway=t
+fi
git_apply_opt=$(cat "$dotest/apply_opt_extra")
if test "$(cat "$dotest/sign")" = t
then
--
1.6.1.rc1.60.g1d1d7
^ permalink raw reply related
* [PATCH 2/4] git-am: propagate -C<n>, -p<n> options as well
From: Junio C Hamano @ 2008-12-05 2:22 UTC (permalink / raw)
To: git
In-Reply-To: <1228443780-3386-2-git-send-email-gitster@pobox.com>
These options are meant to deal with patches that do not apply cleanly
due to the differences between the version the patch was based on and
the version "git am" is working on.
Because a series of patches applied in the same "git am" run tends to
come from the same source, it is more useful to propagate these options
after the application stops.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-am.sh | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index 1bf70d4..ed54e71 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -121,7 +121,7 @@ It does not apply to blobs recorded in its index."
prec=4
dotest="$GIT_DIR/rebase-apply"
-sign= utf8=t keep= skip= interactive= resolved= rebasing= abort= ws=
+sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
resolvemsg= resume=
git_apply_opt=
@@ -156,7 +156,7 @@ do
--resolvemsg)
shift; resolvemsg=$1 ;;
--whitespace)
- ws="--whitespace=$2"; shift ;;
+ git_apply_opt="$git_apply_opt $1=$2"; shift ;;
-C|-p)
git_apply_opt="$git_apply_opt $1$2"; shift ;;
--)
@@ -247,10 +247,10 @@ else
exit 1
}
- # -s, -u, -k and --whitespace flags are kept for the
- # resuming session after a patch failure.
+ # -s, -u, -k, --whitespace, -C and -p flags are kept
+ # for the resuming session after a patch failure.
# -3 and -i can and must be given when resuming.
- echo " $ws" >"$dotest/whitespace"
+ echo " $git_apply_opt" >"$dotest/apply_opt_extra"
echo "$sign" >"$dotest/sign"
echo "$utf8" >"$dotest/utf8"
echo "$keep" >"$dotest/keep"
@@ -283,7 +283,7 @@ if test "$(cat "$dotest/keep")" = t
then
keep=-k
fi
-ws=$(cat "$dotest/whitespace")
+git_apply_opt=$(cat "$dotest/apply_opt_extra")
if test "$(cat "$dotest/sign")" = t
then
SIGNOFF=`git var GIT_COMMITTER_IDENT | sed -e '
@@ -454,7 +454,7 @@ do
case "$resolved" in
'')
- git apply $git_apply_opt $ws --index "$dotest/patch"
+ git apply $git_apply_opt --index "$dotest/patch"
apply_status=$?
;;
t)
--
1.6.1.rc1.60.g1d1d7
^ permalink raw reply related
* [PATCH 1/4] git-am --whitespace: do not lose the command line option
From: Junio C Hamano @ 2008-12-05 2:22 UTC (permalink / raw)
To: git
In-Reply-To: <1228443780-3386-1-git-send-email-gitster@pobox.com>
When you start "git am --whitespace=fix" and the patch application process
is interrupted by an unapplicable patch early in the series, after
fixing the offending patch, the remainder of the patch should be processed
still with --whitespace=fix when restarted with "git am --resolved" (or
dropping the offending patch with "git am --skip").
The breakage was introduced by the commit 67dad68 (add -C[NUM] to git-am,
2007-02-08); this should fix it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-am.sh | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index aa60261..1bf70d4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -121,7 +121,7 @@ It does not apply to blobs recorded in its index."
prec=4
dotest="$GIT_DIR/rebase-apply"
-sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
+sign= utf8=t keep= skip= interactive= resolved= rebasing= abort= ws=
resolvemsg= resume=
git_apply_opt=
@@ -156,7 +156,7 @@ do
--resolvemsg)
shift; resolvemsg=$1 ;;
--whitespace)
- git_apply_opt="$git_apply_opt $1=$2"; shift ;;
+ ws="--whitespace=$2"; shift ;;
-C|-p)
git_apply_opt="$git_apply_opt $1$2"; shift ;;
--)
@@ -283,7 +283,7 @@ if test "$(cat "$dotest/keep")" = t
then
keep=-k
fi
-ws=`cat "$dotest/whitespace"`
+ws=$(cat "$dotest/whitespace")
if test "$(cat "$dotest/sign")" = t
then
SIGNOFF=`git var GIT_COMMITTER_IDENT | sed -e '
@@ -454,7 +454,7 @@ do
case "$resolved" in
'')
- git apply $git_apply_opt --index "$dotest/patch"
+ git apply $git_apply_opt $ws --index "$dotest/patch"
apply_status=$?
;;
t)
--
1.6.1.rc1.60.g1d1d7
^ permalink raw reply related
* [PATCH 0/4] Fix longstanding "git am" bug
From: Junio C Hamano @ 2008-12-05 2:22 UTC (permalink / raw)
To: git
Simon Schubert rerolled a patch to add "git am --directory=<dir>" from
three months ago, which is a good complement to existing "git am -p<n>",
but it had the same issue of duplicating an existing bug to the new feature.
Let's fix the existing bug first, before accepting any new feature, which
will happen after 1.6.1 goes final.
Junio C Hamano (4):
git-am --whitespace: do not lose the command line option
git-am: propagate -C<n>, -p<n> options as well
git-am: propagate --3way options as well
Test that git-am does not lose -C/-p/--whitespace options
git-am.sh | 15 +++++++++----
t/t4252-am-options.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
t/t4252/am-test-1-1 | 19 +++++++++++++++++
t/t4252/am-test-1-2 | 21 +++++++++++++++++++
t/t4252/am-test-2-1 | 19 +++++++++++++++++
t/t4252/am-test-2-2 | 21 +++++++++++++++++++
t/t4252/am-test-3-1 | 19 +++++++++++++++++
t/t4252/am-test-3-2 | 21 +++++++++++++++++++
t/t4252/am-test-4-1 | 19 +++++++++++++++++
t/t4252/am-test-4-2 | 22 ++++++++++++++++++++
t/t4252/file-1-0 | 7 ++++++
t/t4252/file-2-0 | 7 ++++++
12 files changed, 239 insertions(+), 5 deletions(-)
create mode 100755 t/t4252-am-options.sh
create mode 100644 t/t4252/am-test-1-1
create mode 100644 t/t4252/am-test-1-2
create mode 100644 t/t4252/am-test-2-1
create mode 100644 t/t4252/am-test-2-2
create mode 100644 t/t4252/am-test-3-1
create mode 100644 t/t4252/am-test-3-2
create mode 100644 t/t4252/am-test-4-1
create mode 100644 t/t4252/am-test-4-2
create mode 100644 t/t4252/file-1-0
create mode 100644 t/t4252/file-2-0
^ permalink raw reply
* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
From: Jakub Narebski @ 2008-12-05 2:08 UTC (permalink / raw)
To: Sébastien Cevey
Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <87hc5k22dr.wl%seb@cine7.net>
On Thu, 4 Dec 2008, Sébastien Cevey wrote:
> This adds the $projects_list_group_categories option which, if enabled,
> will result in grouping projects by category on the project list page.
> The category is specified for each project by the $GIT_DIR/category file
> or the 'category' variable in its configuration file. By default, projects
> are put in the $project_list_default_category category.
>
> Note:
> - Categories are always sorted alphabetically, with projects in
> each category sorted according to the globally selected $order.
> - When displaying a subset of all the projects (page limiting), the
> category headers are only displayed for projects present on the page.
>
> The feature is inspired from Sham Chukoury's patch for the XMMS2
> gitweb, but has been rewritten for the current gitweb development
> HEAD. The CSS for categories is inspired from Gustavo Sverzut Barbieri's
> patch to group projects by path.
>
> Thanks to Florian Ragwitz for Perl tips.
Very nice, and nicely done and thought out, idea.
>
> Signed-off-by: Sebastien Cevey <seb@cine7.net>
> ---
>
> Cleaner patch this time indeed. Still no fancy sorting of categories,
> only alphabetical.
>
> gitweb/README | 16 ++++++++++++
> gitweb/gitweb.css | 7 +++++
> gitweb/gitweb.perl | 67 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/README b/gitweb/README
> index 825162a..f8f8872 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -188,6 +188,15 @@ not include variables usually directly set during build):
> full description is available as 'title' attribute (usually shown on
> mouseover). By default set to 25, which might be too small if you
> use long project descriptions.
> + * $projects_list_group_categories
> + Enables the grouping of projects by category on the project list page.
> + The category of a project is determined by the $GIT_DIR/category
> + file or the 'gitweb.category' variable in its repository configuration.
> + Disabled by default.
> + * $project_list_default_category
> + Default category for projects for which none is specified. If set
> + to the empty string, such projects will remain uncategorized and
> + listed at the top, above categorized projects.
> * @git_base_url_list
> List of git base URLs used for URL to where fetch project from, shown
> in project summary page. Full URL is "$git_base_url/$project".
Good.
> @@ -269,6 +278,13 @@ You can use the following files in repository:
> from the template during repository creation. You can use the
> gitweb.description repo configuration variable, but the file takes
> precedence.
> + * category (or gitweb.category)
> + Singe line category of a project, used to group projects if
> + $projects_list_group_categories is enabled. By default (file and
> + configuration variable absent), uncategorized projects are put in
> + the $project_list_default_category category. You can use the
> + gitweb.category repo configuration variable, but the file takes
> + precedence.
> * cloneurl (or multiple-valued gitweb.url)
> File with repository URL (used for clone and fetch), one per line.
> Displayed in the project summary page. You can use multiple-valued
Good.
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index a01eac8..64f2a41 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -264,6 +264,13 @@ td.current_head {
> text-decoration: underline;
> }
>
> +td.category {
> + background-color: #d9d8d1;
> + border-top: 1px solid #000000;
> + border-left: 1px solid #000000;
> + font-weight: bold;
> +}
> +
> table.diff_tree span.file_status.new {
> color: #008000;
> }
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a6bb702..97a9b73 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -87,6 +87,14 @@ our $projects_list = "++GITWEB_LIST++";
> # the width (in characters) of the projects list "Description" column
> our $projects_list_description_width = 25;
>
> +# group projects by category on the projects list
> +# (enabled if this variable evaluates to true)
> +our $projects_list_group_categories = 0;
> +
> +# default category if none specified
> +# (leave the empty string for no category)
> +our $project_list_default_category = "";
> +
> # default order of projects list
> # valid values are none, project, descr, owner, and age
> our $default_projects_order = "project";
Nice.
> @@ -2023,6 +2031,11 @@ sub git_get_project_description {
> return git_get_file_or_project_config('description', $path);
> }
>
> +sub git_get_project_category {
> + my $path = shift;
> + return git_get_file_or_project_config('category', $path);
> +}
> +
Good. Nicely uses earlier patch, which adds this infrastructure.
> sub git_get_project_ctags {
> my $path = shift;
> my $ctags = {};
> @@ -3915,8 +3928,9 @@ sub git_patchset_body {
>
> # . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
>
> -# fills project list info (age, description, owner, forks) for each
> -# project in the list, removing invalid projects from returned list
> +# fills project list info (age, description, owner, category, forks)
> +# for each project in the list, removing invalid projects from
> +# returned list
> # NOTE: modifies $projlist, but does not remove entries from it
> sub fill_project_list_info {
> my ($projlist, $check_forks) = @_;
> @@ -3939,6 +3953,11 @@ sub fill_project_list_info {
> if (!defined $pr->{'owner'}) {
> $pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
> }
> + if ($projects_list_group_categories && !defined $pr->{'category'}) {
> + my $cat = git_get_project_category($pr->{'path'}) ||
> + $project_list_default_category;
> + $pr->{'category'} = to_utf8($cat);
> + }
> if ($check_forks) {
> my $pname = $pr->{'path'};
> if (($pname =~ s/\.git$//) &&
Nice. I see that you choose to go with $pr->{'category'} like existing
$pr->{'owner'}, rather than $pr->{'cat'} like existing $pr->{'descr'}.
> @@ -3956,6 +3975,19 @@ sub fill_project_list_info {
> return @projects;
> }
>
> +# returns a hash of categories, containing the list of project
> +# belonging to each category
> +sub build_projlist_by_category {
> + my $projlist = shift;
> + my %categories;
> +
> + for my $pr (@$projlist) {
> + push @{$categories{ $pr->{'category'} }}, $pr;
> + }
> +
> + return %categories;
> +}
This is very nice and simple way to group by categories, and it works
quite well with sorting (assuming that @$projlist is already sorted),
but I wonder how it works with $from / $to, i.e. with selecting
projects.
> +
> # print 'sort by' <th> element, generating 'sort by $name' replay link
> # if that order is not selected
> sub print_sort_th {
> @@ -4077,7 +4109,36 @@ sub git_project_list_body {
> "</tr>\n";
> }
>
> - print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> + if ($projects_list_group_categories) {
> + # only display categories with projects in the $from-$to window
> + my %categories = build_projlist_by_category(\@projects);
Nice... but perhaps it would be better to simply pass $from / $to to
build_projlist_by_category function, and have in %categories only
projects which are shown... well, unless filtered out in
print_project_rows() by $show_ctags; so I think that there is nonzero
probability of empty (no project shown) categories.
> + foreach my $cat (sort keys %categories) {
> + my $num_cats = @{$categories{$cat}};
> +
> + # out of the window to display, done
> + last if defined $to and $to < 0;
> +
> + # in the window to display
> + if (!defined $from or $from < $num_cats) {
> + unless ($cat eq "") {
> + print "<tr>\n";
> + if ($check_forks) {
> + print "<td></td>\n";
> + }
> + print "<td class=\"category\" colspan=\"5\">$cat</td>\n";
> + print "</tr>\n";
> + }
> +
> + print_project_rows($categories{$cat}, $from, $to, $check_forks, $show_ctags);
> + }
> +
> + # adjust $from/$to offset, keep $from positive
> + $from = ($from > $num_cats) ? $from - $num_cats : 0 if defined $from;
> + $to -= $num_cats if defined $to;
I don't think that the games we play with $from / $to would be enough.
Check what happens (I think that it wouldn't work correctly) if we have
something like that:
project | category | shown
--------------------------
1 | A |
2 | A |
3 | B | Y
4 | C | Y
5 | B | Y
6 | C |
7 | C |
It means that we display for example second page in projects list.
> + }
> + } else {
> + print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> + }
Nice.
>
> if (defined $extra) {
> print "<tr>\n";
> --
> 1.5.6.5
>
>
I'll try to examine the code in more detail later... currently I don't
know why but I can't git-am the second patch (this patch) correctly...
--
Jakub Narębski
Poland
^ permalink raw reply
* Re: [PATCH 1/3] Make some of fwrite/fclose/write/close failures visible
From: Junio C Hamano @ 2008-12-05 2:04 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <20081205003546.GA7294@blimp.localdomain>
All of them obviously look correct. Will apply to the tip of master.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions
From: Jakub Narebski @ 2008-12-05 1:52 UTC (permalink / raw)
To: Sébastien Cevey
Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <87iqq022gz.wl%seb@cine7.net>
On Tue, 4 Dec 2008 at 01:44, Sébastien Cevey wrote:
> Extract the printing of project rows on the project page into a
> separate print_project_rows function. This makes it easier to reuse
> the code to print different subsets of the whole project list.
Err... it was not obvious for me that 'project rows' are rows of
projects list table, i.e. currently the body of projects list table.
But I think it is a good description.
>
> The row printing code is merely moved into a separate function, but
> note that $projects is passed as a reference now.
>
> Signed-off-by: Sebastien Cevey <seb@cine7.net>
Nicely done.
For what it is worth:
Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
>
> Boo, the diff is still quite scarier than it really should be...
Well, nothing short of blame -C would make it work; the issue is that
we split subroutine into two, extracting from the middle, but with
some common/similar header
2 1
1 1
2 1
2
2 -> 2
1 2
1 2
2 2
2 2
2
And diff is forward preferring, i.e. it finds match then second part
of split as add, rather than first first part of split as add then
match.
>
> gitweb/gitweb.perl | 103 +++++++++++++++++++++++++++++-----------------------
> 1 files changed, 57 insertions(+), 46 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index b31274c..a6bb702 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3972,59 +3972,19 @@ sub print_sort_th {
> }
> }
>
> -sub git_project_list_body {
> +# print a row for each project in the given list, using the given
> +# range and extra display options
> +sub print_project_rows {
> # actually uses global variable $project
> - my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> -
> - my ($check_forks) = gitweb_check_feature('forks');
> - my @projects = fill_project_list_info($projlist, $check_forks);
> + my ($projects, $from, $to, $check_forks, $show_ctags) = @_;
I see that print_project_rows gets @$projects list already sorted, and
it passes explicitly $check_forks and $show_ctags from caller.
>
> - $order ||= $default_projects_order;
> $from = 0 unless defined $from;
> - $to = $#projects if (!defined $to || $#projects < $to);
> -
> - my %order_info = (
> - project => { key => 'path', type => 'str' },
> - descr => { key => 'descr_long', type => 'str' },
> - owner => { key => 'owner', type => 'str' },
> - age => { key => 'age', type => 'num' }
> - );
> - my $oi = $order_info{$order};
> - if ($oi->{'type'} eq 'str') {
> - @projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> - } else {
> - @projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> - }
> + $to = $#$projects if (!defined $to || $#$projects < $to);
>
> - my $show_ctags = gitweb_check_feature('ctags');
> - if ($show_ctags) {
> - my %ctags;
> - foreach my $p (@projects) {
> - foreach my $ct (keys %{$p->{'ctags'}}) {
> - $ctags{$ct} += $p->{'ctags'}->{$ct};
> - }
> - }
> - my $cloud = git_populate_project_tagcloud(\%ctags);
> - print git_show_project_tagcloud($cloud, 64);
> - }
> -
> - print "<table class=\"project_list\">\n";
> - unless ($no_header) {
> - print "<tr>\n";
> - if ($check_forks) {
> - print "<th></th>\n";
> - }
> - print_sort_th('project', $order, 'Project');
> - print_sort_th('descr', $order, 'Description');
> - print_sort_th('owner', $order, 'Owner');
> - print_sort_th('age', $order, 'Last Change');
> - print "<th></th>\n" . # for links
> - "</tr>\n";
> - }
> my $alternate = 1;
> my $tagfilter = $cgi->param('by_tag');
> for (my $i = $from; $i <= $to; $i++) {
> - my $pr = $projects[$i];
> + my $pr = $projects->[$i];
>
> next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
> next if $searchtext and not $pr->{'path'} =~ /$searchtext/
> @@ -4068,6 +4028,57 @@ sub git_project_list_body {
> "</td>\n" .
> "</tr>\n";
> }
> +}
> +
> +sub git_project_list_body {
> + my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> +
> + my ($check_forks) = gitweb_check_feature('forks');
> + my @projects = fill_project_list_info($projlist, $check_forks);
> +
> + $order ||= $default_projects_order;
> +
> + my %order_info = (
> + project => { key => 'path', type => 'str' },
> + descr => { key => 'descr_long', type => 'str' },
> + owner => { key => 'owner', type => 'str' },
> + age => { key => 'age', type => 'num' }
> + );
> + my $oi = $order_info{$order};
> + if ($oi->{'type'} eq 'str') {
> + @projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> + } else {
> + @projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> + }
> +
> + my $show_ctags = gitweb_check_feature('ctags');
> + if ($show_ctags) {
> + my %ctags;
> + foreach my $p (@projects) {
> + foreach my $ct (keys %{$p->{'ctags'}}) {
> + $ctags{$ct} += $p->{'ctags'}->{$ct};
> + }
> + }
> + my $cloud = git_populate_project_tagcloud(\%ctags);
> + print git_show_project_tagcloud($cloud, 64);
> + }
> +
> + print "<table class=\"project_list\">\n";
> + unless ($no_header) {
> + print "<tr>\n";
> + if ($check_forks) {
> + print "<th></th>\n";
> + }
> + print_sort_th('project', $order, 'Project');
> + print_sort_th('descr', $order, 'Description');
> + print_sort_th('owner', $order, 'Owner');
> + print_sort_th('age', $order, 'Last Change');
> + print "<th></th>\n" . # for links
> + "</tr>\n";
> + }
> +
> + print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> +
> if (defined $extra) {
> print "<tr>\n";
> if ($check_forks) {
> --
> 1.5.6.5
>
>
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic
From: Jakub Narebski @ 2008-12-05 1:38 UTC (permalink / raw)
To: Sébastien Cevey
Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri
In-Reply-To: <87k5ag22ke.wl%seb@cine7.net>
On Thu, 4 Dec 2008, at 01:42, Sébastien Cevey wrote:
> Introduce a git_get_file_or_project_config utility function to
> retrieve a repository variable either from a plain text file in the
> $GIT_DIR
I would say that we try $GIT_DIR/$variable file.
> or else from 'gitweb.$variable' in the repository config
> (e.g. 'description').
It _might_ also be added (just in case) that currently the only user
of this new subroutine is git_get_project_description, but this is
to change, and that is why this split was introduced.
>
> Signed-off-by: Sebastien Cevey <seb@cine7.net>
But those are minor issues. So, FWIW
Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 24 ++++++++++++++++--------
> 1 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 933e137..b31274c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2001,18 +2001,26 @@ sub git_get_path_by_hash {
> ## ......................................................................
> ## git utility functions, directly accessing git repository
>
> -sub git_get_project_description {
> - my $path = shift;
> +# get the value of a config variable either from a file with the same
> +# name in the repository, or the gitweb.$name value in the repository
> +# config file.
It would probably be better to explicitly say that we use $git_dir/$name
file, or if it doesn't exist, gitweb.$name configuration variable.
> +sub git_get_file_or_project_config {
> + my ($name, $path) = @_;
I think that $project, or $projectpath _might_ be better name for the
second argument to this subroutine.
>
> $git_dir = "$projectroot/$path";
> - open my $fd, "$git_dir/description"
> - or return git_get_project_config('description');
> - my $descr = <$fd>;
> + open my $fd, "$git_dir/$name"
> + or return git_get_project_config($name);
> + my $conf = <$fd>;
> close $fd;
> - if (defined $descr) {
> - chomp $descr;
> + if (defined $conf) {
> + chomp $conf;
> }
> - return $descr;
> + return $conf;
> +}
> +
> +sub git_get_project_description {
> + my $path = shift;
> + return git_get_file_or_project_config('description', $path);
> }
Nicely done.
>
> sub git_get_project_ctags {
> --
> 1.5.6.5
>
>
--
Jakub Narębski
Poland
^ permalink raw reply
* Git weekly news: 2008-49
From: Felipe Contreras @ 2008-12-05 0:43 UTC (permalink / raw)
To: git list
Hi there,
I've been following the git tag at delicious.com[1] and there's quite
many interesting links, so I thought on gathering them so the git
community can enjoy them in one pack :)
The blog post is here:
http://gitlog.wordpress.com/2008/12/05/git-weekely-news-2008-49/
But here are the links anyway. The order is rather random.
Why Git is Better than X
http://whygitisbetterthanx.com/
GitTorrent, The Movie
http://www.advogato.org/article/994.html
Peer-to-peer Protocol for Synchronizing of Git Repositories
http://code.google.com/p/gittorrent/
Codebase - Git repository hosting with source browser, changesets,
ticketing & deployment tracking.
http://atechmedia.com/codebase
Codebase Launches Git-based Project Management Service
http://www.sitepoint.com/blogs/2008/12/04/codebase-launches-git-based-project-management-service/
The Git Community Book
http://book.git-scm.com/index.html
GitX: A git GUI specifically for Mac OS X
http://gitx.frim.nl/index.html
Pushing and pulling with Git, part 1
http://www.gnome.org/~federico/news-2008-11.html#pushing-and-pulling-with-git-1
$ cheat git
http://cheat.errtheblog.com/s/git/
gh > hg
http://github.com/blog/218-gh-hg
Easy Git External Dependency Management with Giternal
http://www.rubyinside.com/giternal-easy-git-external-dependency-management-1322.html
Work with Git from Emacs
http://xtalk.msk.su/~ott/en/writings/emacs-vcs/EmacsGit.html
It's Magit! A Emacs mode for Git.
http://zagadka.vm.bytemark.co.uk/magit/
Improving my git workflow
http://hoth.entp.com/2008/11/10/improving-my-git-workflow
git-bz: Bugzilla subcommand for Git
http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/
android.git.kernel.org Git
http://android.git.kernel.org/
GitSvnComparsion
http://git.or.cz/gitwiki/GitSvnComparsion
Guides: Developing with Submodules
http://github.com/guides/developing-with-submodules
A web-focused Git workflow
http://joemaller.com/2008/11/25/a-web-focused-git-workflow/
Why we chose Git, a rebuttal
http://www.unethicalblogger.com/posts/2008/11/why_we_chose_git_a_rebuttal
Git'n Your Shared Host On
http://railstips.org/2008/11/24/gitn-your-shared-host-on
Git integration with Hudson and Trac
http://www.unethicalblogger.com/posts/2008/11/git_integration_with_hudson_and_trac
Everyday GIT With 20 Commands Or So
http://www.kernel.org/pub/software/scm/git/docs/everyday.html
Hosting Git repositories, The Easy (and Secure) Way
http://scie.nti.st/2007/11/14/hosting-git-repositories-the-easy-and-secure-way
10 Reasons to Use Git for Research
http://mendicantbug.com/2008/11/30/10-reasons-to-use-git-for-research/
[1] http://delicious.com/tag/git
--
Felipe Contreras
^ permalink raw reply
* [PATCH 3/3] Report symlink failures in merge-recursive
From: Alex Riesen @ 2008-12-05 0:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20081205003546.GA7294@blimp.localdomain>
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
I don't check for unlink success in the line above, because symlink
will fail if unlink failed to cleanup the new path.
merge-recursive.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 0e988f2..a0c804c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -525,7 +525,8 @@ static void update_file_flags(struct merge_options *o,
char *lnk = xmemdupz(buf, size);
safe_create_leading_directories_const(path);
unlink(path);
- symlink(lnk, path);
+ if (symlink(lnk, path))
+ die("failed to symlink %s: %s", path, strerror(errno));
free(lnk);
} else
die("do not know what to do with %06o %s '%s'",
--
1.6.1.rc1.29.gb140
^ permalink raw reply related
* [PATCH 2/3] Make chdir failures visible
From: Alex Riesen @ 2008-12-05 0:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20081205003546.GA7294@blimp.localdomain>
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
git.c | 4 ++--
setup.c | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/git.c b/git.c
index 9e5813c..940a498 100644
--- a/git.c
+++ b/git.c
@@ -195,8 +195,8 @@ static int handle_alias(int *argcp, const char ***argv)
ret = 1;
}
- if (subdir)
- chdir(subdir);
+ if (subdir && chdir(subdir))
+ die("Cannot change to %s: %s", subdir, strerror(errno));
errno = saved_errno;
diff --git a/setup.c b/setup.c
index 78a8041..833ced2 100644
--- a/setup.c
+++ b/setup.c
@@ -470,7 +470,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
}
die("Not a git repository");
}
- chdir("..");
+ if (chdir(".."))
+ die("Cannot change to %s/..: %s", cwd, strerror(errno));
}
inside_git_dir = 0;
--
1.6.1.rc1.29.gb140
^ permalink raw reply related
* [PATCH 1/3] Make some of fwrite/fclose/write/close failures visible
From: Alex Riesen @ 2008-12-05 0:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
So that full filesystem conditions or permissions problems wont go
unnoticed.
Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
This and the follow-up patches is fallout of Windows debugging
sessions. I implemented random error handling code just to figure out
where it might be going wrong. None of that code actually helped to
fix something (and lots was just thrown away), but some, I think,
still makes sense.
This patch adds error handling only to fwrite/fputs where the return
value was ignored and writing was definitely into a file. BTW, libc
headers in Ubuntu 8.10 have warn_unused_result attribute added to many
functions (fwrite(3) and write(2) amongst them). This helps finding
the problem places.
builtin-fsck.c | 8 ++++++--
builtin-merge.c | 6 ++++--
rerere.c | 46 +++++++++++++++++++++++++++++++++++-----------
3 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/builtin-fsck.c b/builtin-fsck.c
index d3f3de9..afded5e 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -201,12 +201,16 @@ static void check_unreachable_object(struct object *obj)
char *buf = read_sha1_file(obj->sha1,
&type, &size);
if (buf) {
- fwrite(buf, size, 1, f);
+ if (fwrite(buf, size, 1, f) != 1)
+ die("Could not write %s: %s",
+ filename, strerror(errno));
free(buf);
}
} else
fprintf(f, "%s\n", sha1_to_hex(obj->sha1));
- fclose(f);
+ if (fclose(f))
+ die("Could not finish %s: %s",
+ filename, strerror(errno));
}
return;
}
diff --git a/builtin-merge.c b/builtin-merge.c
index 7c2b90c..cf86975 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -293,8 +293,10 @@ static void squash_message(void)
pretty_print_commit(rev.commit_format, commit, &out, rev.abbrev,
NULL, NULL, rev.date_mode, 0);
}
- write(fd, out.buf, out.len);
- close(fd);
+ if (write(fd, out.buf, out.len) < 0)
+ die("Writing SQUASH_MSG: %s", strerror(errno));
+ if (close(fd))
+ die("Finishing SQUASH_MSG: %s", strerror(errno));
strbuf_release(&out);
}
diff --git a/rerere.c b/rerere.c
index 02931a1..718fb52 100644
--- a/rerere.c
+++ b/rerere.c
@@ -70,6 +70,19 @@ static int write_rr(struct string_list *rr, int out_fd)
return 0;
}
+static void ferr_write(const void *p, size_t count, FILE *fp, int *err)
+{
+ if (!count || *err)
+ return;
+ if (fwrite(p, count, 1, fp) != 1)
+ *err = errno;
+}
+
+static inline void ferr_puts(const char *s, FILE *fp, int *err)
+{
+ ferr_write(s, strlen(s), fp, err);
+}
+
static int handle_file(const char *path,
unsigned char *sha1, const char *output)
{
@@ -82,6 +95,7 @@ static int handle_file(const char *path,
struct strbuf one = STRBUF_INIT, two = STRBUF_INIT;
FILE *f = fopen(path, "r");
FILE *out = NULL;
+ int wrerror = 0;
if (!f)
return error("Could not open %s", path);
@@ -118,11 +132,11 @@ static int handle_file(const char *path,
hunk_no++;
hunk = RR_CONTEXT;
if (out) {
- fputs("<<<<<<<\n", out);
- fwrite(one.buf, one.len, 1, out);
- fputs("=======\n", out);
- fwrite(two.buf, two.len, 1, out);
- fputs(">>>>>>>\n", out);
+ ferr_puts("<<<<<<<\n", out, &wrerror);
+ ferr_write(one.buf, one.len, out, &wrerror);
+ ferr_puts("=======\n", out, &wrerror);
+ ferr_write(two.buf, two.len, out, &wrerror);
+ ferr_puts(">>>>>>>\n", out, &wrerror);
}
if (sha1) {
git_SHA1_Update(&ctx, one.buf ? one.buf : "",
@@ -139,7 +153,7 @@ static int handle_file(const char *path,
else if (hunk == RR_SIDE_2)
strbuf_addstr(&two, buf);
else if (out)
- fputs(buf, out);
+ ferr_puts(buf, out, &wrerror);
continue;
bad:
hunk = 99; /* force error exit */
@@ -149,8 +163,12 @@ static int handle_file(const char *path,
strbuf_release(&two);
fclose(f);
- if (out)
- fclose(out);
+ if (wrerror)
+ error("There were errors while writing %s (%s)",
+ path, strerror(wrerror));
+ if (out && fclose(out))
+ wrerror = error("Failed to flush %s: %s",
+ path, strerror(errno));
if (sha1)
git_SHA1_Final(sha1, &ctx);
if (hunk != RR_CONTEXT) {
@@ -158,6 +176,8 @@ static int handle_file(const char *path,
unlink(output);
return error("Could not parse conflict hunks in %s", path);
}
+ if (wrerror)
+ return -1;
return hunk_no;
}
@@ -200,9 +220,13 @@ static int merge(const char *name, const char *path)
if (!ret) {
FILE *f = fopen(path, "w");
if (!f)
- return error("Could not write to %s", path);
- fwrite(result.ptr, result.size, 1, f);
- fclose(f);
+ return error("Could not open %s: %s", path,
+ strerror(errno));
+ if (fwrite(result.ptr, result.size, 1, f) != 1)
+ error("Could not write %s: %s", path, strerror(errno));
+ if (fclose(f))
+ return error("Writing %s failed: %s", path,
+ strerror(errno));
}
free(cur.ptr);
--
1.6.1.rc1.29.gb140
^ permalink raw reply related
* Re: [PATCH] Allow passing of --directory to git-am.
From: Simon 'corecode' Schubert @ 2008-12-05 0:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7viqpzbhvr.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> If that is the case, and assuming that propagating -C/-p would be a good
> idea (which I am not sure yet), the patch I sent out earlier (which was
> flawed somewhat; it should use "$git_apply_opt_extra" where it invokes the
> "git apply" command) with necessary fix would serve as the basis to
> implement --directory=<dir>?
certainly. I'll be travelling, so don't expect anything real soon, will
resubmit unless I forget.
--
<3 the future +++ RENT this banner advert +++ ASCII Ribbon /"\
rock the past +++ space for low €€€ NOW!1 +++ Campaign \ /
Party Enjoy Relax | http://dragonflybsd.org Against HTML \
Dude 2c 2 the max ! http://golden-apple.biz Mail + News / \
^ permalink raw reply
* Re: [PATCH] Allow passing of --directory to git-am.
From: Junio C Hamano @ 2008-12-05 0:11 UTC (permalink / raw)
To: Simon 'corecode' Schubert; +Cc: git
In-Reply-To: <49386ABE.2050404@fs.ei.tum.de>
Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
> Junio C Hamano wrote:
>> Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
>>
>>> You mean not storing/restoring the flags across an invocation? No,
>>> that's a different thing. My patch only adds the --directory option,
>>> it does not fix the previously existing bug.
>>
>> The question is if it _introduces_ a bug that the directory given in the
>> initial invocation of "git am --directory=foo" is lost if an patch does
>> not apply and you need to manually resolve and continue.
>>
>> If it does not introduce such a bug, you do not have the same issue as the
>> old patch. Otherwise you have the same issue as the old patch. The
>> question was if you have the same issue or you don't. Yes? No?
>
> Yes, that's the issue. In this regard it behaves bug-compatible with
> the -p and -C options.
If that is the case, and assuming that propagating -C/-p would be a good
idea (which I am not sure yet), the patch I sent out earlier (which was
flawed somewhat; it should use "$git_apply_opt_extra" where it invokes the
"git apply" command) with necessary fix would serve as the basis to
implement --directory=<dir>?
^ permalink raw reply
* Re: git-gui: Warn when username and e-mail address is unconfigured?
From: Jeremy Ramer @ 2008-12-04 23:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: spearce, sverre, Peter Krefting, Git Mailing List
In-Reply-To: <7vskp3d3q9.fsf@gitster.siamese.dyndns.org>
On Thu, Dec 4, 2008 at 2:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> "Jeremy Ramer" <jdramer@gmail.com> writes:
>
>> On Thu, Dec 4, 2008 at 12:04 PM, Sverre Rabbelier <alturin@gmail.com> wrote:
>>> On Thu, Dec 4, 2008 at 17:05, Jeremy Ramer <jdramer@gmail.com> wrote:
>>>> That's strange. I am using git 1.6.0.4 on cygwin and I get a warning
>>>> message every time I start git gui. I actually find this really
>>>> annoying and would like a way to turn this warning message off.
>>>
>>> git config --global user.name "Your Name"
>>> git config --global user.email "you@example.com"
>>>
>>
>> I have done that. I still get the warning message every time I start git gui.
>
> I do not use Windows, and I do not run git-gui, so I am guessing only from
> the source. Are you talking about the message composed by this part?
>
> # -- Warn the user about environmental problems. Cygwin's Tcl
> # does *not* pass its env array onto any processes it spawns.
> # This means that git processes get none of our environment.
> #
> if {[is_Cygwin]} {
> set ignored_env 0
> set suggest_user {}
> set msg [mc "Possible environment issues exist.
>
> The following environment variables are probably
> going to be ignored by any Git subprocess run
> by %s:
>
> " [appname]]
Yes, that does appear to be the message I get, with the following
environment variables:
- GIT_AUTHOR_EMAIL
- GIT_COMMITTER_NAME
- GIT_COMMITER_EMAIL
- GIT_AUTHOR_NAME
Now that I look closer I see that I am setting these in my .bashrc
file. When I first started using git a year ago I was given the
impression that these were needed. But I see that that is no longer
the case since I use the config:
git config --global user.name "Your Name"
git config --global user.email "you@example.com"
Removing them from my .bashrc removes the warning. In hindsight the
warning should have clued me in, but I've been seeing that message
since I first started using git on Cygwin so I figured it was a cygwin
issue that I couldn't do anything about.
>
> The logic to produce the error message does look somewhat screwy.
>
> It checks a selected set of variables whose name begin with GIT_ in the
> environment, and if it finds any, it gives the above message. In
> addition, if GIT_{AUTHOR,COMMITTER}_{EMAIL,NAME} are among them, it also
> adds this to the message:
>
> if {$suggest_user ne {}} {
> append msg [mc "
> A good replacement for %s
> is placing values for the user.name and
> user.email settings into your personal
> ~/.gitconfig file.
> " $suggest_user]
>
> There are two and half issues about this code.
>
> (1) When it prepares additional message about user.{email,name},
> it does not check if the user already has them defined. IOW, there
> is no way other than unsetenv before running git-gui to squelch this
> part of the message.
>
> (2) For other environment variables, such as GIT_PAGER, it does not offer
> alternatives, such as core.pager. Again, there is no way other than
> unsetenv to squelch the warning.
>
> An excuse to both of the above could be that the warning is not about the
> user having environment variables that can be discarded, but about
> brokenness of Cygwin Tcl envirnonment that discards them. But if that is
> the case, there is this other half issue:
>
> (3) The warning does not trigger if the environment is not set when this
> check is made. Now I do not know if git-gui tries to spawn
> subprocesses with its own (customized) environment settings (e.g. you
> would need to be able to run git-commit-tree with modified
> GIT_AUTHOR_NAME if you want to use the lowlevel plumbing to create a
> new commit and lie about the author identity), but if it does, the
> warning does not trigger.
>
I agree that the logic could using improvement.
^ permalink raw reply
* Re: [PATCH] Allow passing of --directory to git-am.
From: Simon 'corecode' Schubert @ 2008-12-04 23:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7i6fd0zt.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
>
>> You mean not storing/restoring the flags across an invocation? No,
>> that's a different thing. My patch only adds the --directory option,
>> it does not fix the previously existing bug.
>
> The question is if it _introduces_ a bug that the directory given in the
> initial invocation of "git am --directory=foo" is lost if an patch does
> not apply and you need to manually resolve and continue.
>
> If it does not introduce such a bug, you do not have the same issue as the
> old patch. Otherwise you have the same issue as the old patch. The
> question was if you have the same issue or you don't. Yes? No?
Yes, that's the issue. In this regard it behaves bug-compatible with the
-p and -C options.
--
<3 the future +++ RENT this banner advert +++ ASCII Ribbon /"\
rock the past +++ space for low €€€ NOW!1 +++ Campaign \ /
Party Enjoy Relax | http://dragonflybsd.org Against HTML \
Dude 2c 2 the max ! http://golden-apple.biz Mail + News / \
^ permalink raw reply
* [PATCH - DONTUSE] git-am: propagate -C/-p as well
From: Junio C Hamano @ 2008-12-04 23:36 UTC (permalink / raw)
To: Simon 'corecode' Schubert; +Cc: git
In-Reply-To: <7vy6yvbki6.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I think this fixes the --whitespace=* one, although I obviously haven't
> tried to use it myself extensively.
This one comes on top of it *if* you want to propagate -C/-p as well, but
I think it might be a wrong idea to propagate these to begin with.
Just like --3way is a one-shot option to deal with a single unapplicable
patch (because it was based on an old version) in the whole series, and is
designed not to get propagated, I suspect that people use -C<n> to fix a
single broken patch and they may expect it not to apply to the whole
series.
The breakage --whitespace deals with is an attribute of the submitter (use
of a broken editor and lack of diligence). You most often feed a single
series from the same submitter in the same mbox to "git am", preserving
the --whitespace=fix option during the same "am" run makes sense, and
somewhat more importantly, even though the option indeed modifies what you
received, the change the option causes and the risk of breaking the
semantics of the patch is minimum. I am not sure the breakage --3way
deals with falls into the exactly the same category, but it is similar (if
the first patch in the series was based on an old version, it is very
likely that the subsequent ones are also based on the same old version).
So after all it might be better to propagate --3way as well (which this
patch does not do).
If we decide that propagating --3way is a good thing, then it would be
equally good to propagate -C, -p and --directory options.
I dunno.
git-am.sh | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git c/git-am.sh w/git-am.sh
index 1bf70d4..a35e07a 100755
--- c/git-am.sh
+++ w/git-am.sh
@@ -121,7 +121,7 @@ It does not apply to blobs recorded in its index."
prec=4
dotest="$GIT_DIR/rebase-apply"
-sign= utf8=t keep= skip= interactive= resolved= rebasing= abort= ws=
+sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
resolvemsg= resume=
git_apply_opt=
@@ -155,9 +155,7 @@ do
;;
--resolvemsg)
shift; resolvemsg=$1 ;;
- --whitespace)
- ws="--whitespace=$2"; shift ;;
- -C|-p)
+ -C|-p|--whitespace)
git_apply_opt="$git_apply_opt $1$2"; shift ;;
--)
shift; break ;;
@@ -247,10 +245,10 @@ else
exit 1
}
- # -s, -u, -k and --whitespace flags are kept for the
- # resuming session after a patch failure.
+ # -s, -u, -k, --whitespace, -C and -p flags are kept
+ # for the resuming session after a patch failure.
# -3 and -i can and must be given when resuming.
- echo " $ws" >"$dotest/whitespace"
+ echo " $git_apply_opt" >"$dotest/apply_opt_extra"
echo "$sign" >"$dotest/sign"
echo "$utf8" >"$dotest/utf8"
echo "$keep" >"$dotest/keep"
@@ -283,7 +281,7 @@ if test "$(cat "$dotest/keep")" = t
then
keep=-k
fi
-ws=$(cat "$dotest/whitespace")
+apply_opt_extra=$(cat "$dotest/apply_opt_extra")
if test "$(cat "$dotest/sign")" = t
then
SIGNOFF=`git var GIT_COMMITTER_IDENT | sed -e '
^ permalink raw reply related
* Re: [PATCH] Allow passing of --directory to git-am.
From: Junio C Hamano @ 2008-12-04 23:14 UTC (permalink / raw)
To: Simon 'corecode' Schubert; +Cc: git
In-Reply-To: <7v7i6fd0zt.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
>
>> You mean not storing/restoring the flags across an invocation? No,
>> that's a different thing. My patch only adds the --directory option,
>> it does not fix the previously existing bug.
>
> The question is if it _introduces_ a bug that the directory given in the
> initial invocation of "git am --directory=foo" is lost if an patch does
> not apply and you need to manually resolve and continue.
>
> If it does not introduce such a bug, you do not have the same issue as the
> old patch. Otherwise you have the same issue as the old patch. The
> question was if you have the same issue or you don't. Yes? No?
I think this fixes the --whitespace=* one, although I obviously haven't
tried to use it myself extensively.
The third hunk is just a style fix. "am" is written in a quite old
fashioned way.
-- >8 --
Subject: [PATCH] git-am --whitespace: do not lose the command line option
When you start "git am --whitespace=fix" and the patch application process
is interrupted by an unapplicable patch early in the series, after
fixing the offending patch, the remainder of the patch should be processed
still with --whitespace=fix when restarted with "git am --resolved".
The commit 67dad68 (add -C[NUM] to git-am, 2007-02-08) broke this long
time ago. This should fix it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-am.sh | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index aa60261..1bf70d4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -121,7 +121,7 @@ It does not apply to blobs recorded in its index."
prec=4
dotest="$GIT_DIR/rebase-apply"
-sign= utf8=t keep= skip= interactive= resolved= rebasing= abort=
+sign= utf8=t keep= skip= interactive= resolved= rebasing= abort= ws=
resolvemsg= resume=
git_apply_opt=
@@ -156,7 +156,7 @@ do
--resolvemsg)
shift; resolvemsg=$1 ;;
--whitespace)
- git_apply_opt="$git_apply_opt $1=$2"; shift ;;
+ ws="--whitespace=$2"; shift ;;
-C|-p)
git_apply_opt="$git_apply_opt $1$2"; shift ;;
--)
@@ -283,7 +283,7 @@ if test "$(cat "$dotest/keep")" = t
then
keep=-k
fi
-ws=`cat "$dotest/whitespace"`
+ws=$(cat "$dotest/whitespace")
if test "$(cat "$dotest/sign")" = t
then
SIGNOFF=`git var GIT_COMMITTER_IDENT | sed -e '
@@ -454,7 +454,7 @@ do
case "$resolved" in
'')
- git apply $git_apply_opt --index "$dotest/patch"
+ git apply $git_apply_opt $ws --index "$dotest/patch"
apply_status=$?
;;
t)
--
1.6.1.rc1.60.g1d1d7
^ permalink raw reply related
* Re: [PATCH] Allow passing of --directory to git-am.
From: Jakub Narebski @ 2008-12-04 22:46 UTC (permalink / raw)
To: Simon 'corecode' Schubert; +Cc: git
In-Reply-To: <493858CE.1030601@fs.ei.tum.de>
Simon 'corecode' Schubert wrote:
> Jakub Narebski wrote:
>> Simon 'corecode' Schubert wrote:
>>> @@ -155,8 +156,9 @@ do
>>> ;;
>>> --resolvemsg)
>>> shift; resolvemsg=$1 ;;
>>> - --whitespace)
>>> - git_apply_opt="$git_apply_opt $1=$2"; shift ;;
>>> + --whitespace|--directory)
>>> + quot=$(echo "$2" | sed -e "s/'/'\\\''/g")
>>
>> Why not simply use "git rev-parse --sq"?
>
> What I need is to convert $2 into a form suitable for quoting, does git
> rev-parse --sq do that?
$ git rev-parse --sq -- "don't do that"
'--' 'don'\''t do that'
Without terminating newline. The '--' is needed because otherwise
git-rev-parse expects revisions... and doesn't find any.
By the way you could both simplify option parsing _and_ take care of
proper quoting by using --parseopt, i.e. use git-rev-parse in PARSEOPT
mode. But that is more involved change.
>>> @@ -454,7 +456,7 @@ do
>>>
>>> case "$resolved" in
>>> '')
>>> - git apply $git_apply_opt --index "$dotest/patch"
>>> + eval git apply $git_apply_opt --index '"$dotest/patch"'
>>
>> Why eval?
>
> I quoted the above variable, so I now need to unquote it, that's done by eval.
Ah.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] Allow passing of --directory to git-am.
From: Junio C Hamano @ 2008-12-04 22:33 UTC (permalink / raw)
To: Simon 'corecode' Schubert; +Cc: git
In-Reply-To: <49385908.5020202@fs.ei.tum.de>
Simon 'corecode' Schubert <corecode@fs.ei.tum.de> writes:
> You mean not storing/restoring the flags across an invocation? No,
> that's a different thing. My patch only adds the --directory option,
> it does not fix the previously existing bug.
The question is if it _introduces_ a bug that the directory given in the
initial invocation of "git am --directory=foo" is lost if an patch does
not apply and you need to manually resolve and continue.
If it does not introduce such a bug, you do not have the same issue as the
old patch. Otherwise you have the same issue as the old patch. The
question was if you have the same issue or you don't. Yes? No?
^ permalink raw reply
* Re: pre-rebase safety hook
From: Junio C Hamano @ 2008-12-04 22:29 UTC (permalink / raw)
To: Tim Harper; +Cc: Git Mailing List
In-Reply-To: <7vbpvrens3.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> If you want to prevent a branch whose tip commit is on more than one
> branches from being rebased, I think something like this would suffice.
>
> #!/bin/sh
> LF='
> '
> in_branches=$(git branch -a --with "${2-HEAD}")
> case "$in_branches" in
> *"$LF"*)
> : this commit is on more than two branches
> exit 1
> ;;
> esac
> exit 0
>
> But I didn't test it.
Actually, the above cannot possibly be right. To decide whether to allow
rebasing of a branch or not, you need to also give it from which commit
the rebase will rewrite.
For example, suppose you have a branch "topic", that was forked from
"master" and built two commits, then another branch "side" was forked from
that, and you have three more commits on "topic" since then:
o "side"
/
A---B---C---D---E "topic"
/
---o---o---o---o "master"
Now, can I allow you to rebase "topic"? It depends. These should be
allowed:
git rebase B "topic"
git rebase C "topic"
git rebase D "topic"
but rebasing "topic" on top of "master", or anything that changes the fact
that "topic" contains commits A and B, should be prohibited, because it
will interfere with "side". For example,
git rebase A "topic"
would make this history:
B---o "side"
/
A---B'--C'--D'--E' "topic"
/
---o---o---o---o "master"
where B' and B are different commits.
So you need to check all the commits that will be affected by the rebase
to see if any of them is on a branch other than the one that is being
rebased. The set of commits that needs to be checked are:
git rev-list "$1..${2-HEAD}"
so a naive implementation that is based on brnach --with would probably
look like:
#!/bin/sh
: allow rebasing a detached HEAD
git symbolic-ref -q HEAD || exit 0
LF='
'
git rev-list "$1..${2-HEAD}" |
while read commit
do
case "$(git branch -a --with $commit)" in
*"$LF"*)
: this is on two or more branches
exit 1
;;
esac
done
^ 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