* Re: [PATCH] Prevent users from adding the file that has all-zero SHA-1
From: Nguyen Thai Ngoc Duy @ 2011-09-20 10:13 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git
In-Reply-To: <CALkWK0n9kJtUPrpxTmtfH7kVd7CksULjt7bo3cFhOrS_NBVbMw@mail.gmail.com>
2011/9/20 Ramkumar Ramachandra <artagnon@gmail.com>:
> Hi Nguyễn,
>
> Nguyễn Thái Ngọc Duy writes:
>> This particular SHA-1 has special meaning to git, very much like NULL
>> in C. If a user adds a file that has this SHA-1, unexpected things can
>> happen.
>> [...]
>
> Interesting patch. Is it possible to write some sort of testcase?
Naah. I tested it by explicitly clear the result hash, just before it
gets to my changes, then do "git add <blah>".
--
Duy
^ permalink raw reply
* Re: Git 1.7.6: Sparse checkouts do not work with directory exclusions
From: Nguyen Thai Ngoc Duy @ 2011-09-20 10:09 UTC (permalink / raw)
To: Joshua Jensen; +Cc: git@vger.kernel.org
In-Reply-To: <4E77BC36.7060005@workspacewhiz.com>
On Tue, Sep 20, 2011 at 8:03 AM, Joshua Jensen
<jjensen@workspacewhiz.com> wrote:
> Sometime after Git 1.7.3.2, sparse checkouts stopped working for me. My
> sparse-checkout file looks something like:
>
> *
> !DirA/
> !DirB/
> DirC/
>
> I have restored some lines of code that were removed in November 2010. This
> resolves the sparse checkout issue for me, but my guess is the solution is
> not implemented properly.
>
> Can anyone confirm the issue
Confirmed. It got me wonder why the negated pattern tests did not
catch this. Turns out this works:
/*
!DirA/
!DirB/
DirC
This is my theory why yours does not work: negated patterns !DirA and
!DirB excludes both directories, but git still descends in them
because you may have other patterns that re-include parts of
DirA/DirB, for example:
DirA/DirD
!DirA
When it's in DirA/DirB, "*" tells git to match everything (equivalent
"DirA/*" and "DirB/*"), so it matches all entries in DirA/DirB again,
essentially reverting "!DirA" and "!DirB" effects.
By using "/*" instead of "*", we tell git to just match entries at top
level, not all levels.
I think it makes sense, but it's a bit tricky.
> and describe why those lines were removed?
Quotes from 9e08273: "The commit provided a workaround for matching
directories in index. But it is no longer needed."
--
Duy
^ permalink raw reply
* Re: [PATCH v2] post-receive-email: explicitly set Content-Type header
From: Johannes Sixt @ 2011-09-20 9:32 UTC (permalink / raw)
To: Alexey Shumkin; +Cc: git, Junio C Hamano
In-Reply-To: <1316506023-5098-1-git-send-email-zapped@mail.ru>
Am 9/20/2011 10:07, schrieb Alexey Shumkin:
> Some email clients (e.g. claws-mail) incorrectly display
> message body when there is no Content-Type header and charset
> explicitly defined.
> So, set explicitly Content-Type header and charset
> can be defined with hooks.emailcharset config variable.
Please write full sentences with complete punctuation and capitalization.
Perhaps you meant to say:
Some email clients (e.g. claws-mail) display the message body
incorrectly when the charset is not defined explicitly in a
Content-Type header. Insert a Content-Type header in the message.
The charset can be defined with the config variable
hooks.emailcharset.
> +# hooks.emailcharset
> +# The charset used in Content-Type header. UTF-8, if not specified.
How can you be sure that the output produced by git log etc. that are used
in the script are in the encoding specified by this variable? IOW,
wouldn't log.outputencoding be the better choice to use in the
Content-Type header?
-- Hannes
^ permalink raw reply
* Re: [RFC/PATCHv2] git-web--browse: avoid the use of eval
From: Chris Packham @ 2011-09-20 9:04 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git, chriscool, jepler
In-Reply-To: <20110919183408.GB26115@sigill.intra.peff.net>
On 20/09/11 06:34, Jeff King wrote:
> On Mon, Sep 19, 2011 at 09:26:55PM +1200, Chris Packham wrote:
>
>> Using eval causes problems when the URL contains an appropriately
>> escaped ampersand (\&). Dropping eval from the built-in browser
>> invocation avoids the problem.
>>
>> Cc: peff@peff.net
>> Cc: chriscool@tuxfamily.org
>> Cc: jepler@unpythonic.net
>
> Although other projects do use "cc" in the commit message, I think we
> don't usually bother adding this noise in the git project. The cc
> headers in your email are enough.
That's more for git send-email's benefit than anything else. I'm working
on a laptop with a touchpad (and a cat) so the less switching between
editor and MUA the better. Any better suggestions for tracking Cc's for
git send-email?
>> I've replaced my tests With the test suggested by Peff (should I be
>> giving him credit in the copyright line or something?).
>
> For a minor bit of help, usually mentioning the person in the commit
> message (with a "Helped-by", or indicating which parts they contributed
> to) is plenty. Personally, I don't even care much about that. My
> contributions to git are thoroughly documented in the commit history and
> the mailing list at this point. :)
>
> I also find the "Copyright ..." lines in the files to be overkill, too.
> They end up becoming out-of-date as other people work on the file. The
> commit history is the best way to get the right answer, and a comment in
> the file is at best redundant with what's there. But that is just my
> opinion; I don't know that we have a particular policy for such
> things[1].
>
> -Peff
>
> [1] Once upon a time, I think I saw the advice that every file should
> have a copyright notice and mention the license at the top of the file,
> but I don't know that it has ever been tested in court. I suppose the
> distributed tarballs of a particular version would lack the copyright
> attribution, but in that case, my solution would be to generate it from
> the commit history at packaging time.
The example in t/README has has a copyright notice which is why I put
one in but I don't consider the test (or the fix itself) to actually be
copyrightable. If I wasn't creating a new file I wouldn't have bothered
putting anything in (other than the testcase).
^ permalink raw reply
* Re: [PATCH] t4014: remove Message-Id/timestamp before comparing patches
From: Thomas Rast @ 2011-09-20 8:43 UTC (permalink / raw)
To: Junio C Hamano, Jeff King; +Cc: Pang Yan Han, git
In-Reply-To: <7vr53c9tlt.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Thanks, your fix makes a lot more sense.
Agreed. Thanks!
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] send-email: Honor multi-part email messages
From: Alexey Shumkin @ 2011-09-20 8:29 UTC (permalink / raw)
To: git; +Cc: Alexey Shumkin, Junio C Hamano
"git format-patch --attach/--inline" generates multi-part messages.
Every part of such messages can contain non-ASCII characters with its own
"Content-Type" and "Content-Transfer-Encoding" headers.
But git-send-mail script interprets a patch-file as one-part message
and does not recognize multi-part messages.
So already quoted printable email subject may be encoded as quoted printable
again. Due to this bug email subject looks corrupted in email clients.
Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
git-send-email.perl | 5 +++
t/t9001-send-email.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 98ab33a..1abf4a4 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1403,12 +1403,17 @@ sub file_has_nonascii {
sub body_or_subject_has_nonascii {
my $fn = shift;
+ my $multipart = 0;
open(my $fh, '<', $fn)
or die "unable to open $fn: $!\n";
while (my $line = <$fh>) {
last if $line =~ /^$/;
+ if ($line =~ /^Content-Type:\s*multipart\/mixed.*$/) {
+ $multipart = 1;
+ }
return 1 if $line =~ /^Subject.*[^[:ascii:]]/;
}
+ return 0 if $multipart;
while (my $line = <$fh>) {
return 1 if $line =~ /[^[:ascii:]]/;
}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 579ddb7..151ad35 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1168,4 +1168,70 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' '
test -n "$(ls msgtxt*)"
'
+test_expect_success $PREREQ 'setup multi-part message' '
+cat >multi-part-email-using-8bit <<EOF
+From fe6ecc66ece37198fe5db91fa2fc41d9f4fe5cc4 Mon Sep 17 00:00:00 2001
+Message-Id: <bogus-message-id@example.com>
+From: author@example.com
+Date: Sat, 12 Jun 2010 15:53:58 +0200
+Subject: [PATCH] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=20?=
+ =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB?=
+MIME-Version: 1.0
+Content-Type: multipart/mixed; boundary="------------123"
+
+This is a multi-part message in MIME format.
+--------------1.7.6.3.4.gf71f
+Content-Type: text/plain; charset=UTF-8; format=fixed
+Content-Transfer-Encoding: 8bit
+
+This is a message created with "git format-patch --attach=123"
+---
+ master | 1 +
+ файл | 1 +
+ 2 files changed, 2 insertions(+), 0 deletions(-)
+ create mode 100644 master
+ create mode 100644 файл
+
+
+--------------123
+Content-Type: text/x-patch; name="0001-.patch"
+Content-Transfer-Encoding: 8bit
+Content-Disposition: attachment; filename="0001-.patch"
+
+diff --git a/master b/master
+new file mode 100644
+index 0000000..1f7391f
+--- /dev/null
++++ b/master
+@@ -0,0 +1 @@
++master
+diff --git a/файл b/файл
+new file mode 100644
+index 0000000..44e5cfe
+--- /dev/null
++++ b/файл
+@@ -0,0 +1 @@
++содержимое файла
+
+--------------123--
+EOF
+'
+
+test_expect_success $PREREQ 'setup expect' '
+cat >expected <<EOF
+Subject: [PATCH] =?UTF-8?q?=D0=94=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB=D0=B5=D0=BD=20?= =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB?=
+EOF
+'
+
+test_expect_success $PREREQ '--attach/--inline also treats subject' '
+ clean_fake_sendmail &&
+ echo bogus |
+ git send-email --from=author@example.com --to=nobody@example.com \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ --8bit-encoding=UTF-8 \
+ multi-part-email-using-8bit >stdout &&
+ grep "Subject" msgtxt1 >actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.7.6.3.4.gf71f
^ permalink raw reply related
* [PATCH] format-patch: cover letter does not respect i18n.commitencoding
From: Alexey Shumkin @ 2011-09-20 8:26 UTC (permalink / raw)
To: git; +Cc: Alexey Shumkin, Junio C Hamano
When we write a cover letter it is naturally enough to suppose
that we use the same encoding as we do commits in. So make
"format-patch --cover-letter" to use i18n.commitencoding setting
in "Content-Type" header of a cover letter instead of hard-coded "UTF-8".
Especially as cover letter contains shortlog and diffstat of patches
which can contain author names and file names in i18n.commitencoding.
Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
builtin/log.c | 4 ++--
log-tree.c | 10 ++++++----
log-tree.h | 3 ++-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 5c2af59..6a4050c 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -769,7 +769,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
struct shortlog log;
struct strbuf sb = STRBUF_INIT;
int i;
- const char *encoding = "UTF-8";
+ const char *encoding = get_commit_output_encoding();
struct diff_options opts;
int need_8bit_cte = 0;
struct commit *commit = NULL;
@@ -806,7 +806,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
}
log_write_email_headers(rev, head, &pp.subject, &pp.after_subject,
- &need_8bit_cte);
+ &need_8bit_cte, get_commit_output_encoding());
for (i = 0; !need_8bit_cte && i < nr; i++)
if (has_non_ascii(list[i]->buffer))
diff --git a/log-tree.c b/log-tree.c
index 39913d7..403aede 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -284,7 +284,8 @@ void get_patch_filename(struct commit *commit, int nr, const char *suffix,
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **subject_p,
const char **extra_headers_p,
- int *need_8bit_cte_p)
+ int *need_8bit_cte_p,
+ const char *encoding)
{
const char *subject = NULL;
const char *extra_headers = opt->extra_headers;
@@ -340,11 +341,12 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
"format.\n"
"--%s%s\n"
"Content-Type: text/plain; "
- "charset=UTF-8; format=fixed\n"
+ "charset=%s; format=fixed\n"
"Content-Transfer-Encoding: 8bit\n\n",
extra_headers ? extra_headers : "",
mime_boundary_leader, opt->mime_boundary,
- mime_boundary_leader, opt->mime_boundary);
+ mime_boundary_leader, opt->mime_boundary,
+ encoding);
extra_headers = subject_buffer;
get_patch_filename(opt->numbered_files ? NULL : commit, opt->nr,
@@ -433,7 +435,7 @@ void show_log(struct rev_info *opt)
if (opt->commit_format == CMIT_FMT_EMAIL) {
log_write_email_headers(opt, commit, &ctx.subject, &extra_headers,
- &ctx.need_8bit_cte);
+ &ctx.need_8bit_cte, get_commit_output_encoding());
} else if (opt->commit_format != CMIT_FMT_USERFORMAT) {
fputs(diff_get_color_opt(&opt->diffopt, DIFF_COMMIT), stdout);
if (opt->commit_format != CMIT_FMT_ONELINE)
diff --git a/log-tree.h b/log-tree.h
index 5c4cf7c..ce0acf7 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -17,7 +17,8 @@ void show_decorations(struct rev_info *opt, struct commit *commit);
void log_write_email_headers(struct rev_info *opt, struct commit *commit,
const char **subject_p,
const char **extra_headers_p,
- int *need_8bit_cte_p);
+ int *need_8bit_cte_p,
+ const char *encoding);
void load_ref_decorations(int flags);
#define FORMAT_PATCH_NAME_MAX 64
--
1.7.6.3.4.gf71f
^ permalink raw reply related
* [PATCH v3 1/2] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
From: Alexey Shumkin @ 2011-09-20 8:21 UTC (permalink / raw)
To: git; +Cc: Alexey Shumkin, Junio C Hamano
In-Reply-To: <7v1uvptjhr.fsf@alter.siamese.dyndns.org>
The following two commands are expected to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it format "%s".
Log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted with the latter even
when i18n.logOutputEncoding and terminal encoding are the same.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
t/t4041-diff-submodule-option.sh | 56 ++++++------
t/t4205-log-pretty-formats.sh | 22 ++++-
t/t6006-rev-list-format.sh | 192 +++++++++++++++++++++++---------------
3 files changed, 164 insertions(+), 106 deletions(-)
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index bf9a752..b0cec5e 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -1,6 +1,7 @@
#!/bin/sh
#
# Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin
+# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding tests)
#
test_description='Support for verbose submodule differences in git diff
@@ -10,6 +11,7 @@ This test tries to verify the sanity of the --submodule option of git diff.
. ./test-lib.sh
+added=$(printf "\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275")
add_file () {
sm=$1
shift
@@ -19,8 +21,11 @@ add_file () {
echo "$name" > "$name" &&
git add "$name" &&
test_tick &&
- git commit -m "Add $name"
+ msg_added_cp1251=$(echo "Add $name ($added $name)" | iconv -f utf-8 -t cp1251) &&
+ git config i18n.commitEncoding cp1251 &&
+ git commit -m "$msg_added_cp1251"
done >/dev/null
+ git config --unset i18n.commitEncoding
git rev-parse --verify HEAD | cut -c1-7
cd "$owd"
}
@@ -48,28 +53,25 @@ head2=$(add_file sm1 foo3)
test_expect_success 'modified submodule(forward)' "
git diff-index -p --submodule=log HEAD >actual &&
- cat >expected <<-EOF &&
-Submodule sm1 $head1..$head2:
- > Add foo3
-EOF
+ printf \"Submodule sm1 $head1..$head2:\n\
+ > Add foo3 ($added foo3)\n\
+\" > expected &&
test_cmp expected actual
"
test_expect_success 'modified submodule(forward)' "
git diff --submodule=log >actual &&
- cat >expected <<-EOF &&
-Submodule sm1 $head1..$head2:
- > Add foo3
-EOF
+ printf \"Submodule sm1 $head1..$head2:\n\
+ > Add foo3 ($added foo3)\n\
+\" > expected &&
test_cmp expected actual
"
test_expect_success 'modified submodule(forward) --submodule' "
git diff --submodule >actual &&
- cat >expected <<-EOF &&
-Submodule sm1 $head1..$head2:
- > Add foo3
-EOF
+ printf \"Submodule sm1 $head1..$head2:\n\
+ > Add foo3 ($added foo3)\n\
+\" > expected &&
test_cmp expected actual
"
@@ -98,11 +100,10 @@ head3=$(
test_expect_success 'modified submodule(backward)' "
git diff-index -p --submodule=log HEAD >actual &&
- cat >expected <<-EOF &&
-Submodule sm1 $head2..$head3 (rewind):
- < Add foo3
- < Add foo2
-EOF
+ printf \"Submodule sm1 $head2..$head3 (rewind):\n\
+ < Add foo3 ($added foo3)\n\
+ < Add foo2 ($added foo2)\n\
+\" > expected &&
test_cmp expected actual
"
@@ -110,13 +111,12 @@ head4=$(add_file sm1 foo4 foo5) &&
head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD)
test_expect_success 'modified submodule(backward and forward)' "
git diff-index -p --submodule=log HEAD >actual &&
- cat >expected <<-EOF &&
-Submodule sm1 $head2...$head4:
- > Add foo5
- > Add foo4
- < Add foo3
- < Add foo2
-EOF
+ printf \"Submodule sm1 $head2...$head4:\n\
+ > Add foo5 ($added foo5)\n\
+ > Add foo4 ($added foo4)\n\
+ < Add foo3 ($added foo3)\n\
+ < Add foo2 ($added foo2)\n\
+\" > expected &&
test_cmp expected actual
"
@@ -131,10 +131,10 @@ mv sm1-bak sm1
test_expect_success 'typechanged submodule(submodule->blob), --cached' "
git diff --submodule=log --cached >actual &&
cat >expected <<-EOF &&
-Submodule sm1 41fbea9...0000000 (submodule deleted)
+Submodule sm1 $head4...0000000 (submodule deleted)
diff --git a/sm1 b/sm1
new file mode 100644
-index 0000000..9da5fb8
+index 0000000..$head5
--- /dev/null
+++ b/sm1
@@ -0,0 +1 @@
@@ -148,7 +148,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' "
cat >expected <<-EOF &&
diff --git a/sm1 b/sm1
deleted file mode 100644
-index 9da5fb8..0000000
+index $head5..0000000
--- a/sm1
+++ /dev/null
@@ -1 +0,0 @@
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 2ae9faa..9da6835 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -1,6 +1,7 @@
#!/bin/sh
#
# Copyright (c) 2010, Will Palmer
+# Copyright (c) 2011, Alexey Shumkin (+ non-UTF-8 commit encoding tests)
#
test_description='Test pretty formats'
@@ -11,10 +12,13 @@ test_expect_success 'set up basic repos' '
>bar &&
git add foo &&
test_tick &&
- git commit -m initial &&
+ git config i18n.commitEncoding cp1251 &&
+ msg_commit_cp1251=$(printf "initial \320\272\320\276\320\274\320\274\320\270\321\202" | iconv -f utf-8 -t cp1251) &&
+ git commit -m "$msg_commit_cp1251" &&
git add bar &&
test_tick &&
- git commit -m "add bar"
+ git commit -m "add bar" &&
+ git config --unset i18n.commitEncoding
'
test_expect_success 'alias builtin format' '
@@ -38,6 +42,20 @@ test_expect_success 'alias user-defined format' '
test_cmp expected actual
'
+test_expect_success 'alias user-defined tformat with %s (cp1251 encoding)' '
+ git config i18n.logOutputEncoding cp1251 &&
+ git log --oneline >expected-s &&
+ git log --pretty="tformat:%h %s" >actual-s &&
+ git config --unset i18n.logOutputEncoding &&
+ test_cmp expected-s actual-s
+'
+
+test_expect_success 'alias user-defined tformat with %s (utf-8 encoding)' '
+ git log --oneline >expected-s &&
+ git log --pretty="tformat:%h %s" >actual-s &&
+ test_cmp expected-s actual-s
+'
+
test_expect_success 'alias user-defined tformat' '
git log --pretty="tformat:%h" >expected &&
git config pretty.test-alias "tformat:%h" &&
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index d918cc0..8d99635 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -1,13 +1,34 @@
#!/bin/sh
+# Copyright (c) 2009 Jens Lehmann
+# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding tests)
+
test_description='git rev-list --pretty=format test'
. ./test-lib.sh
test_tick
+added=$(printf "added (\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275) foo")
+added_cp1251=$(echo "$added" | iconv -f utf-8 -t cp1251)
+changed=$(printf "changed (\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275) foo")
+changed_cp1251=$(echo "$changed" | iconv -f utf-8 -t cp1251)
+
test_expect_success 'setup' '
-touch foo && git add foo && git commit -m "added foo" &&
- echo changed >foo && git commit -a -m "changed foo"
+ touch foo &&
+ git add foo &&
+ git config i18n.commitEncoding cp1251 &&
+ git commit -m "$added_cp1251" &&
+ head1=$(git rev-parse --verify HEAD) &&
+ head1_7=$(echo $head1 | cut -c1-7) &&
+ echo "$changed" > foo &&
+ git commit -a -m "$changed_cp1251" &&
+ head2=$(git rev-parse --verify HEAD) &&
+ head2_7=$(echo $head2 | cut -c1-7) &&
+ tree2=$(git cat-file -p HEAD | grep tree | cut -f 2 -d" ") &&
+ tree2_7=$(echo $tree2 | cut -c1-7) &&
+ head2_parent=$(git cat-file -p HEAD | grep parent | cut -f 2 -d" ") &&
+ head2_parent_7=$(echo $head2_parent | cut -c1-7) &&
+ git config --unset i18n.commitEncoding
'
# usage: test_format name format_string <expected_output
@@ -19,49 +40,49 @@ test_cmp expect.$1 output.$1
"
}
-test_format percent %%h <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format percent %%h <<EOF
+commit $head2
%h
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
%h
EOF
-test_format hash %H%n%h <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-131a310eb913d107dd3c09a65d1651175898735d
-131a310
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cf
+test_format hash %H%n%h <<EOF
+commit $head2
+$head2
+$head2_7
+commit $head1
+$head1
+$head1_7
EOF
-test_format tree %T%n%t <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-fe722612f26da5064c32ca3843aa154bdb0b08a0
-fe72261
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format tree %T%n%t <<EOF
+commit $head2
+$tree2
+$tree2_7
+commit $head1
4d5fcadc293a348e88f777dc0920f11e7d71441c
4d5fcad
EOF
-test_format parents %P%n%p <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-86c75cf
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format parents %P%n%p <<EOF
+commit $head2
+$head1
+$head2_parent_7
+commit $head1
EOF
# we don't test relative here
-test_format author %an%n%ae%n%ad%n%aD%n%at <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format author %an%n%ae%n%ad%n%aD%n%at <<EOF
+commit $head2
A U Thor
author@example.com
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
A U Thor
author@example.com
Thu Apr 7 15:13:13 2005 -0700
@@ -69,14 +90,14 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format committer %cn%n%ce%n%cd%n%cD%n%ct <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format committer %cn%n%ce%n%cd%n%cD%n%ct <<EOF
+commit $head2
C O Mitter
committer@example.com
Thu Apr 7 15:13:13 2005 -0700
Thu, 7 Apr 2005 15:13:13 -0700
1112911993
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
C O Mitter
committer@example.com
Thu Apr 7 15:13:13 2005 -0700
@@ -84,86 +105,105 @@ Thu, 7 Apr 2005 15:13:13 -0700
1112911993
EOF
-test_format encoding %e <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format encoding %e <<EOF
+commit $head2
+commit $head1
EOF
-test_format subject %s <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-changed foo
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-added foo
+expected=$(printf "commit $head2\n\
+$changed\n\
+commit $head1\n\
+$added
+")
+
+test_format subject %s <<EOF
+$expected
EOF
-test_format body %b <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format body %b <<EOF
+commit $head2
+commit $head1
EOF
-test_format raw-body %B <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
-changed foo
+expected=$(printf "commit $head2\n\
+$changed\n\
+\n\
+commit $head1\n\
+$added
+")
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-added foo
+test_format raw-body %B <<EOF
+$expected
EOF
-test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy <<EOF
+commit $head2
^[[31mfoo^[[32mbar^[[34mbaz^[[mxyzzy
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
^[[31mfoo^[[32mbar^[[34mbaz^[[mxyzzy
EOF
-test_format advanced-colors '%C(red yellow bold)foo%C(reset)' <<'EOF'
-commit 131a310eb913d107dd3c09a65d1651175898735d
+test_format advanced-colors '%C(red yellow bold)foo%C(reset)' <<EOF
+commit $head2
^[[1;31;43mfoo^[[m
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head1
^[[1;31;43mfoo^[[m
EOF
-cat >commit-msg <<'EOF'
+iconv -f utf-8 -t cp1251 > commit-msg <<EOF
Test printing of complex bodies
This commit message is much longer than the others,
-and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+and it will be encoded in cp1251. We should therefore
+include an cp1251 character: так вот!
EOF
+
test_expect_success 'setup complex body' '
-git config i18n.commitencoding iso8859-1 &&
- echo change2 >foo && git commit -a -F commit-msg
+ git config i18n.commitencoding cp1251 &&
+ echo change2 >foo && git commit -a -F commit-msg &&
+ head3=$(git rev-parse --verify HEAD) &&
+ head3_7=$(echo $head3 | cut -c1-7)
'
-test_format complex-encoding %e <<'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
-iso8859-1
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+test_format complex-encoding %e <<EOF
+commit $head3
+cp1251
+commit $head2
+cp1251
+commit $head1
+cp1251
EOF
-test_format complex-subject %s <<'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
-Test printing of complex bodies
-commit 131a310eb913d107dd3c09a65d1651175898735d
-changed foo
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
-added foo
+# unset commit encoding config
+# otherwise %e does not print encoding value
+# and following test fails
+git config --unset i18n.commitencoding
+
+expected=$(printf "commit $head3\n\
+Test printing of complex bodies\n\
+commit $head2\n\
+$changed\n\
+commit $head1\n\
+$added
+")
+
+test_format complex-subject %s <<EOF
+$expected
EOF
-test_format complex-body %b <<'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+test_format complex-body %b <<EOF
+commit $head3
This commit message is much longer than the others,
-and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+and it will be encoded in cp1251. We should therefore
+include an cp1251 character: так вот!
-commit 131a310eb913d107dd3c09a65d1651175898735d
-commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
+commit $head2
+commit $head1
EOF
test_expect_success '%x00 shows NUL' '
- echo >expect commit f58db70b055c5718631e5c61528b28b12090cdea &&
+ echo >expect commit $head3 &&
echo >>expect fooQbar &&
git rev-list -1 --format=foo%x00bar HEAD >actual.nul &&
nul_to_q <actual.nul >actual &&
@@ -210,12 +250,12 @@ test_expect_success 'add LF before non-empty (2)' '
test_expect_success 'add SP before non-empty (1)' '
git show -s --pretty=format:"%s% bThanks" HEAD^^ >actual &&
- test $(wc -w <actual) = 2
+ test $(wc -w <actual) = 3
'
test_expect_success 'add SP before non-empty (2)' '
git show -s --pretty=format:"%s% sThanks" HEAD^^ >actual &&
- test $(wc -w <actual) = 4
+ test $(wc -w <actual) = 6
'
test_expect_success '--abbrev' '
--
1.7.6.3.4.gf71f
^ permalink raw reply related
* [PATCH v3 2/2] pretty: user format ignores i18n.logOutputEncoding setting
From: Alexey Shumkin @ 2011-09-20 8:21 UTC (permalink / raw)
To: git; +Cc: Alexey Shumkin, Junio C Hamano
In-Reply-To: <7v1uvptjhr.fsf@alter.siamese.dyndns.org>
The following two commands are expected to give the same output to a terminal:
$ git log --oneline --no-color
$ git log --pretty=format:'%h %s'
However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it format "%s".
Log messages written in an encoding i18n.commitEncoding which differs
from terminal encoding are shown corrupted with the latter even
when i18n.logOutputEncoding and terminal encoding are the same.
The same corruption is true for
$ git diff --submodule=log
and
$ git rev-list --pretty=format:%s HEAD
Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
builtin/rev-list.c | 1 +
builtin/shortlog.c | 1 +
log-tree.c | 1 +
submodule.c | 3 +++
4 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 56727e8..831077d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -105,6 +105,7 @@ static void show_commit(struct commit *commit, void *data)
ctx.abbrev = revs->abbrev;
ctx.date_mode = revs->date_mode;
ctx.fmt = revs->commit_format;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(&ctx, commit, &buf);
if (revs->graph) {
if (buf.len) {
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 37f3193..eba4086 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -163,6 +163,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
ctx.subject = "";
ctx.after_subject = "";
ctx.date_mode = DATE_NORMAL;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(&ctx, commit, &ufbuf);
buffer = ufbuf.buf;
} else if (*buffer) {
diff --git a/log-tree.c b/log-tree.c
index e945701..39913d7 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -488,6 +488,7 @@ void show_log(struct rev_info *opt)
ctx.preserve_subject = opt->preserve_subject;
ctx.reflog_info = opt->reflog_info;
ctx.fmt = opt->commit_format;
+ ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(&ctx, commit, &msgbuf);
if (opt->add_signoff)
diff --git a/submodule.c b/submodule.c
index b648927..8c96de7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -213,10 +213,13 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,
static const char format[] = " %m %s";
struct strbuf sb = STRBUF_INIT;
struct commit *commit;
+ const char *log_output_encoding;
+ log_output_encoding = get_log_output_encoding();
while ((commit = get_revision(rev))) {
struct pretty_print_context ctx = {0};
ctx.date_mode = rev->date_mode;
+ ctx.output_encoding = log_output_encoding;
strbuf_setlen(&sb, 0);
if (commit->object.flags & SYMMETRIC_LEFT) {
if (del)
--
1.7.6.3.4.gf71f
^ permalink raw reply related
* [PATCH v3 0/2] pretty: user format ignores i18n.logOutputEncoding setting
From: Alexey Shumkin @ 2011-09-20 8:20 UTC (permalink / raw)
To: git; +Cc: Alexey Shumkin, Junio C Hamano
In-Reply-To: <7v1uvptjhr.fsf@alter.siamese.dyndns.org>
>>You seem to have inherited bad grammar from some example; I wonder
>>where... ;-)
I rephrased... I made an attempt to as least ;)
>>Format it this way instead if you want to use multi-line:
>> echo "..." |
>> iconv ... |
>> xargs ...
done
Alexey Shumkin (2):
pretty: Add failing tests: user format ignores i18n.logOutputEncoding
setting
pretty: user format ignores i18n.logOutputEncoding setting
builtin/rev-list.c | 1 +
builtin/shortlog.c | 1 +
log-tree.c | 1 +
submodule.c | 3 +
t/t4041-diff-submodule-option.sh | 56 ++++++------
t/t4205-log-pretty-formats.sh | 22 ++++-
t/t6006-rev-list-format.sh | 192 +++++++++++++++++++++++---------------
7 files changed, 170 insertions(+), 106 deletions(-)
--
1.7.6.3.4.gf71f
^ permalink raw reply
* [PATCH v2] post-receive-email: explicitly set Content-Type header
From: Alexey Shumkin @ 2011-09-20 8:07 UTC (permalink / raw)
To: git; +Cc: Alexey Shumkin, Junio C Hamano
In-Reply-To: <1316505154-3904-1-git-send-email-zapped@mail.ru>
Some email clients (e.g. claws-mail) incorrectly display
message body when there is no Content-Type header and charset
explicitly defined.
So, set explicitly Content-Type header and charset
can be defined with hooks.emailcharset config variable.
Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
[resent: Fixed typo in commit message]
contrib/hooks/post-receive-email | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 21989fc..64d7c2f 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -60,6 +60,8 @@
# email body. If not specified, there is no limit.
# Lines beyond the limit are suppressed and counted, and a final
# line is added indicating the number of suppressed lines.
+# hooks.emailcharset
+# The charset used in Content-Type header. UTF-8, if not specified.
#
# Notes
# -----
@@ -229,6 +231,7 @@ generate_email_header()
cat <<-EOF
To: $recipients
Subject: ${emailprefix}$projectdesc $refname_type $short_refname ${change_type}d. $describe
+ Content-Type: text/plain; charset=$emailcharset
X-Git-Refname: $refname
X-Git-Reftype: $refname_type
X-Git-Oldrev: $oldrev
@@ -723,6 +726,7 @@ envelopesender=$(git config hooks.envelopesender)
emailprefix=$(git config hooks.emailprefix || echo '[SCM] ')
custom_showrev=$(git config hooks.showrev)
maxlines=$(git config hooks.emailmaxlines)
+emailcharset=$(git config hooks.emailcharset || echo 'UTF-8')
# --- Main loop
# Allow dual mode: run from the command line just like the update hook, or
--
1.7.6.3.4.gf71f
^ permalink raw reply related
* [PATCH] post-receive-email: explicitly set Content-Type header
From: Alexey Shumkin @ 2011-09-20 7:52 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Alexey Shumkin
Some email clients (e.g. claws-mail) incorrectly display
message body when there is no Content-Type header and charset
explicitly defined.
So, set explicitly Content-Type headrer and charset
can be defined with hooks.emailcharset config variable.
Signed-off-by: Alexey Shumkin <zapped@mail.ru>
---
contrib/hooks/post-receive-email | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 21989fc..64d7c2f 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -60,6 +60,8 @@
# email body. If not specified, there is no limit.
# Lines beyond the limit are suppressed and counted, and a final
# line is added indicating the number of suppressed lines.
+# hooks.emailcharset
+# The charset used in Content-Type header. UTF-8, if not specified.
#
# Notes
# -----
@@ -229,6 +231,7 @@ generate_email_header()
cat <<-EOF
To: $recipients
Subject: ${emailprefix}$projectdesc $refname_type $short_refname ${change_type}d. $describe
+ Content-Type: text/plain; charset=$emailcharset
X-Git-Refname: $refname
X-Git-Reftype: $refname_type
X-Git-Oldrev: $oldrev
@@ -723,6 +726,7 @@ envelopesender=$(git config hooks.envelopesender)
emailprefix=$(git config hooks.emailprefix || echo '[SCM] ')
custom_showrev=$(git config hooks.showrev)
maxlines=$(git config hooks.emailmaxlines)
+emailcharset=$(git config hooks.emailcharset || echo 'UTF-8')
# --- Main loop
# Allow dual mode: run from the command line just like the update hook, or
--
1.7.6.3.4.gf71f
^ permalink raw reply related
* Worktree vs. working copy
From: Carlos Martín Nieto @ 2011-09-20 7:50 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
Hello,
I've noticed that in some places in git.git, the term 'working copy' is
used. Mostly it's in git-cvsserver which I guess it's fine, but the
git-config man page talks about the 'working copy' in three places.
$ git grep 'worktree' | wc -l
412
$ git grep 'working copy' | grep -v ^git-cvsserver | wc -l
32
Most of the references to a 'working copy' are in Git.pm, and the
git-gui glossary treats them as the same thing.
Is there some sort of official position, or rough agreement that we
should use woktree instead of working copy? The latter term IMHO implies
a centralised system, where an user gets a copy.
cmn
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 0/8] fast-import: cache oe more often
From: Dmitry Ivankov @ 2011-09-20 7:17 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Shawn O. Pearce, David Barr,
Sverre Rabbelier
In-Reply-To: <20110920042655.GH6343@elie>
On Tue, Sep 20, 2011 at 10:26 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Junio C Hamano wrote:
>
>> I gave the series a cursory look, and the patches all looked like a good
>> and straight forward rewrites. Provided if it is indeed a good idea
>> overall to stuff more objects in-core, that is.
>
> Right, that's exactly the question I had. When and why is it a good
> idea to stuff more objects in-core (or when might it be a bad idea,
> for that matter)?
The next step would be to replace sha1 with struct object_entry* in fast-import.
So it'll be in struct tree_entry (twice, for each of versions[2]),
branch, tag, hash_list (used to store merge from lists), last_object.
Then some fields will be deleted as they can be accessed from
object_entry:
last_object->depth
last_object->offset
tree_content->delta_depth
branch,tag->pack_id
And it all even slightly decreased memory consumption (checked some
time ago, but think it's still true). Probably because of tree nodes
having NULL instead of null_sha1 and then ptr instead of sha1; and
maybe because for huge imports each object is from our pack so for a
sha1 there always is object_entry anyway.
In short, if there is nothing bad with this patchset, it'll be
absolutely natural one after switch to oe instead of sha1, but it's
put before to split the big series. And of course this part may have a
small speedup of it's own. If it's not too good to be accepted on it's
own, I'll just include it into future series depending on it.
^ permalink raw reply
* Re: [PATCH] Prevent users from adding the file that has all-zero SHA-1
From: Ramkumar Ramachandra @ 2011-09-20 6:25 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1316259574-1291-1-git-send-email-pclouds@gmail.com>
Hi Nguyễn,
Nguyễn Thái Ngọc Duy writes:
> This particular SHA-1 has special meaning to git, very much like NULL
> in C. If a user adds a file that has this SHA-1, unexpected things can
> happen.
> [...]
Interesting patch. Is it possible to write some sort of testcase?
-- Ram
^ permalink raw reply
* Re: [PATCH/RFCv4 2/4] gitweb: Add manpage for /etc/gitweb.conf (for gitweb documentation)
From: Jonathan Nieder @ 2011-09-20 4:41 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Drew Northup, John 'Warthog9' Hawley, admin
In-Reply-To: <1316352884-26193-3-git-send-email-jnareb@gmail.com>
Jakub Narebski wrote:
> [jn: Improved, extended, removed duplicate info from README]
Hoorah! I have very little to add to what's already been done ---
mostly I can just lend my ear as a native English speaker.
[...]
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -3,7 +3,7 @@ MAN1_TXT= \
> $(wildcard git-*.txt)) \
> gitk.txt git.txt
> MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt githooks.txt \
> - gitrepository-layout.txt
> + gitrepository-layout.txt gitweb.conf.txt
I take back what I wrote about patch 1. Maybe its commit message could
be clarified to mention that the new targets build a subset of the pages
built by the ordinary "make doc" et al targets.
[...]
> +++ b/Documentation/gitweb.conf.txt
> @@ -0,0 +1,781 @@
[...]
> +The configuration file is used to override the default settings that
> +were built into gitweb at the time 'gitweb.cgi' script was generated.
> +
> +While one could just alter the configuration settings in the gitweb
> +CGI itself, those changes would be lost upon upgrade. Configuration
> +settings might also be placed into a file in the same directory as the
> +CGI script with the default name 'gitweb_config.perl' -- allowing
> +one to have multiple gitweb instances with different configurations by
> +the use of symlinks.
This reads nicely now. Language nit: missing "the" before
"'gitweb.cgi' script".
[...]
> +DISCUSSION
> +----------
> +Gitweb obtains configuration data from the following sources in the
> +following order:
> +
> + * built-in values (some set during build stage),
> +
> + * common system-wide configuration file (defaults to
> + '/etc/gitweb-common.conf'),
> +
> + * either per-instance configuration file (defaults to 'gitweb_config.perl'
> + in the same directory as the installed gitweb), or if it does not exists
> + then system-wide configuration file (defaults to '/etc/gitweb.conf').
Usage nit: in the first sentence, "reads" would be more natural than
"obtains".
In the third item, I'd suggest the phrase "the fallback system-wide
configuration file" to emphasize how it differs from
gitweb-common.conf.
> +
> +Values obtained in later configuration files override values obtained earlier
> +in above sequence.
Language nit: missing "the" before "above sequence".
> +
> +The location of common system-wide configuration file, of fallback system-wide
> +configuration file and of per-instance configuration file is defined at compile
> +time using the build-time Makefile configuration variables, respectively
> +`GITWEB_CONFIG_COMMON`, `GITWEB_CONFIG_SYSTEM` and `GITWEB_CONFIG`.
Language nits: missing "the" before "common system-wide configuration
file", "fallback system-wide configuration file", and "per-instance
configuration files". The subject ("The location") and verb should be
plural because we are talking about multiple locations. Would sound a
bit more natural in English (unlike French) if the preposition "of"
were used once instead of repeated.
Another language nit: it sounds a little better without the "the"
before "build-time Makefile configuration variables", like so:
... are defined at compile time using build-time Makefile
variables, namely `GITWEB_CONFIG_COMMON`, `GITWEB_CONFIG_SYSTEM`,
and `GITWEB_CONFIG`.
> +
> +You can override location of gitweb configuration files during runtime
> +by setting respective environment variables: `GITWEB_CONFIG_COMMON`,
> +`GITWEB_CONFIG_SYSTEM` and `GITWEB_CONFIG` to non-empty value.
Language nit: missing "the" before "location" and "gitweb
configuration files". "location" should probably be plural. In place
of "respective", "the following" sounds a little more natural. Might
be nice to add an "also" after 'can' to emphasize the relationship to
the previous paragraph --- in addition to being able to override the
default locations at compile time, you can override them at run time
by setting environment variables.
> +
> +
> +The syntax of the configuration files is that of Perl, as these files are
> +indeed handled as fragments of Perl code (the language that gitweb itself is
> +written in). Variables may be set using "`our $variable = value`"; text from
> +"#" character until the end of a line is ignored. See the *perlsyn*(1) man
> +page for more information.
> +
> +Actually using `our` qualifier in `our $variable = <value>;` is a safety
> +check: if newer gitweb does no longer use given variable, by using `our` we
> +won't get syntax errors.
Language nit: for "indeed", something like "internally" might be more
natural.
The syntax and the typical idiom were already described in the
DESCRIPTION section, so the emphasis should be somehow different here
to avoid boring the reader. One way might be to plunge directly into
what the second paragraph above says:
The syntax of the configuration files is that of Perl, since these files are
handled by sourcing them as fragments of Perl code (the language that
gitweb itself is written in). Variables are typically set using the
`our` qualifier (as in "`our $variable = <value>;`") to avoid syntax
errors if a new version of gitweb no longer uses a variable and therefore
stops declaring it.
> +
> +You can include other configuration file using read_config_file()
> +subroutine. For example, you can read defaults in fallback
> +system-wide GITWEB_CONFIG_SYSTEM from GITWEB_CONFIG by adding
> +
> + read_config_file($GITWEB_CONFIG_SYSTEM);
> +
> +at very beginning of per-instance GITWEB_CONFIG file. In this case
> +settings in said per-instance file will override settings from
> +system-wide configuration file. Note that read_config_file checks
> +itself that the file it reads exists.
Language nits: missing "the" before "fallback systemwide
$GITWEB_CONFIG_SYSTEM file" and "very beginning"; missing "a" before
"per-instance $GITWEB_CONFIG file"; "case" should perhaps be "example"
and followed by a comma.
The reader wonders: After checking that the file exists, what does
read_config_file do (does it silently skip it or error out)? And why
would I use this trick instead of just writing a gitweb-common.conf
file? (Was read_config_file introduced before gitweb-common.conf,
making this useful when creating a configuration that should be usable
with older versions of gitweb?)
[...]
> +CONFIGURATION VARIABLES
> +-----------------------
> +Some of configuration variables have their default values (embedded in CGI
> +file) set during building gitweb -- if that is the case, it is put in their
> +description. See gitweb's 'INSTALL' file for instructions on building and
> +installing gitweb.
Language nit: "Some of configuration variables" should be "Some
configuration variables"; missing "the" before "CGI script"; "it is"
should be "they are" to agree with 'variables'.
> +
> +
> +Location of repositories
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +Configuration variables described below control finding git repositories by
> +gitweb, and control display and access to repositories.
Language nits: missing "The" before "Configuration variables described
below"; "finding git repositories by gitweb" sounds more natural when
paraphrased as "how gitweb finds git repositories" and "display and
access to repositories" paraphrased as "how repositories are displayed
and accessed"; it seems that the sentence scans a little better when
not repeating the verb "control".
[...]
> +For example if `$projectroot` is set to "/srv/git" by putting the following
> +in gitweb config file:
> ++
> +----------------------------------------------------------------------------
> +our $projectroot = "/srv/git";
> +----------------------------------------------------------------------------
> ++
> +then
> ++
> +------------------------------------------------
> +http://git.example.com/gitweb.cgi?p=foo/bar.git
> +------------------------------------------------
> ++
> +or its path_info based equivalent
> ++
> +------------------------------------------------
> +http://git.example.com/gitweb.cgi/foo/bar.git
> +------------------------------------------------
> ++
> +will map to path '/srv/git/foo/bar.git' on filesystem.
Language nits: missing comma after "For example"; "or" should be
"and"; missing "the" before "path '/srv/git/foo/bar.git'" and
"filesystem".
> +
> +$project_maxdepth::
> + Filesystem traversing depth limit for recursively scanning for git
> + repositories, used if `$projects_list` (below) is unset. The default
> + value of this variable is determined by build-time configuration
> + variable `GITWEB_PROJECT_MAXDEPTH`, which defaults to 2007.
"Filesystem traversing depth limit" is quite a long stack of nouns :),
and unfortunately I don't know what it means. Is this the depth of
nested subdirectories below $project_root that gitweb will look at
when discovering repositories? If I have no subdirectories in the
projectroot except the repositories themselves, should I set this to 1
or 0? What happens with forks? Is this just a convenience feature or
can it be used for security or to create performance gaurantees?
By the way, what happens if projectroot contains a symlink to some
directory elsewhere in the filesystem containing repositories --- will
gitweb traverse it?
What if I want this to be infinite (i.e., to disable the feature) ---
would I be crazy?
Language nit: missing "the" before "`GITWEB_PROJECT_MAXDEPTH` Makefile
variable used at build time, which defaults to 2007".
> +
> +$projects_list::
> + Plain text file listing projects or name of directory
> + to be scanned for projects.
> ++
> +Project list files should list one project per line, with each line
> +having the following format
> ++
> +-----------------------------------------------------------------------------
> +<URI-encoded filesystem path to repository> SP <URI-encoded repository owner>
> +-----------------------------------------------------------------------------
> ++
> +The default value of this variable is determined by the `GITWEB_LIST`
> +makefile variable at installation time. If this variable is empty, gitweb
> +will fall back to scanning the `$projectroot` directory for repositories.
Language nit: missing "Path to a" or "Name of a" before "Plain text
file".
When this is a relative path, what is it taken relative to?
> +
> +$export_ok::
> + Show repository only if this file exists (in repository). Only
> + effective if this variable evaluates to true. Can be set during
> + building gitweb via `GITWEB_EXPORT_OK`. [No default / Not set]
This is always a relative path, right? What is it relative to ---
$GIT_DIR, I guess?
Usage nits: the phrase starting with "during" reads more naturally if
"during" is replaced with "when" and "via" replaced with "by setting".
If there were no default, that would mean that gitweb errors out when
gitweb.conf does not set this variable; instead, the default seems to
be "undef" (or 'false') if I understand correctly.
> +
> +$export_auth_hook::
> + Show repository only if this subroutine returns true, when given as
> + the only parameter the full path to the project. Example:
> ++
> +----------------------------------------------------------------------------
> +our $export_auth_hook = sub { return -e "$_[0]/git-daemon-export-ok"; };
> +----------------------------------------------------------------------------
> ++
> +though the above might be done by using `$export_ok` instead
> ++
> +----------------------------------------------------------------------------
> +our $export_ok = "git-daemon-export-ok";
> +----------------------------------------------------------------------------
Style nit: commands in this kind of documentation should be directed to
the reader, not gitweb, so it would be nicer to explain this in terms of
what the $export_auth_hook is. That is, something like the following:
$export_auth_hook::
Function used to determine which repositories should be shown.
This subroutine should take one parameter, the full path to
a project, and if it returns true, that project will be included
in the projects list and can be accessed through gitweb as long
as it fulfills the other requirements described by $export_ok,
$projects_list, and $projects_maxdepth. Example:
Is "our $export_auth_hook = undef;" a valid configuration? What is the
default?
> +
> +$strict_export::
> + Only allow viewing of repositories also shown on the overview page.
> + This for example makes `$gitweb_export_ok` file decide if repository is
> + available and not only if it is shown. If `$gitweb_list` points to
> + file with list of project, only those repositories listed would be
> + available for gitweb. Can be set during building gitweb via
> + `GITWEB_STRICT_EXPORT`. [No default / Not set]
Is the default behavior as though this were true or false?
> +Finding files
> +~~~~~~~~~~~~~
> +Those configuration variables tell gitweb where to find files. Values of
> +those variables are paths on filesystem. Of those only `$GIT` is required
> +to be (correctly) set for gitweb to be able to work; all the rest are
> +required only for extra configuration or extra features.
Language nit: "Those" should be "The following" in the first sentence and
"these" in remaining sentences; missing "The" before "Values of those
variables" and "filesystem"; missing comma after "Of these".
When you say the remainder are only required in special cases, does
that mean that they are ignored unless some other enabling variable is
set or that they can be set to "undef" to disable the relevant
feature?
> +
> +$GIT::
> + Core git executable to use. By default set to `$GIT_BINDIR/git`, which
> + in turn is by default set to `$(bindir)/git`. If you use git from binary
> + package, set this to "/usr/bin/git". This can just be "git" if your
> + webserver has a sensible PATH. If you have multiple git versions installed
> + it can be used to choose which one to use.
Language nit: missing "a" or "your operating system distributor's" before
"binary package".
> +
> +$mimetypes_file::
> + File to use for (filename extension based) guessing of MIME types before
> + trying '/etc/mime.types'. Path, if relative, is taken currently as
> + relative to the current git repository. [Unset by default]
Language nit: missing "The" before "Path".
The use of "currently" sounds like an apology. Does that mean that
gitweb ought to be rejecting relative paths for this variable?
> +
> +$highlight_bin::
> + Path to the highlight executable to use (must be the one from
> + http://www.andre-simon.de due to assumptions about parameters and output).
> + Useful if 'highlight' is not installed on your webserver's PATH.
> + [Default: 'highlight']
> ++
> +*NOTE*: if you want to add support for new syntax (supported by
> +"highlight" but not used by gitweb), you need to modify `%highlight_ext`
> +or `%highlight_basename`, depending on whether you detect type of file
> +based on extension (for example "*.sh") or on its basename (for example
> +"Makefile"). Keys of those hashes are extension or basename,
> +respectively, and value for given key is name of syntax to be passed via
> +`--syntax <syntax>` to highlighter.
Language nits: missing "it" before "must be the one"; "new syntax"
should probably be "a new file type" or something like that; missing
"The" before "Keys of those hashes"; "those" should be "these"; "or"
should be "and".
Is "*.sh" actually an example of an extension? I.e., do I write the
following?
our %highlight_ext;
$highlight_ext{"*.sh"} = "sh";
> +
> +
> +Links and their targets
> +~~~~~~~~~~~~~~~~~~~~~~~
> +Configuration variables described below configure some of gitweb links:
> +their target and their look (text or image), and where to find page
> +prerequisites (stylesheet, favicon, images, scripts). Usually they are left
> +at their default values, with the possible exception of `@stylesheets`
> +variable.
Language nits: missing "The" before "Configuration variables described
below"; missing "some" before "supporting files".
> +
> +@stylesheets::
> + List of URIs of stylesheets (relative to base URI of a page). You
> + might specify more than one stylesheet, for example use gitweb.css
> + as base, with site specific modifications in separate stylesheet
> + to make it easier to upgrade gitweb. You can add a `site` stylesheet
> + for example by putting
> ++
> +----------------------------------------------------------------------------
> +push @stylesheets, "gitweb-site.css";
> +----------------------------------------------------------------------------
> ++
> +in the gitweb config file. Those values that are relative paths, are
> +relative to base URI of gitweb.
Language nits: missing "the" before "base URI". What is a base URI ---
is it set by another variable? Missing "to" before "use gitweb.css as
a base"; there should not be a comma before "with site-specific
modifications" or before "are relative"; missing "a" before "separate
stylesheet"; missing commas surrounding "for example" (or even better,
"For example," could move to the start of the sentence).
> ++
> +This list should contain URI of gitweb's stylesheet. The URI of gitweb
> +stylesheet is set during build time via `GITWEB_CSS` variable. It's default
> +value is 'static/gitweb.css' (or 'static/gitweb.min.css' if the `CSSMIN`
> +variable is defined, i.e. if CSS minifier is used during build).
> ++
> +*Note*: there is also legacy `$stylesheet` configuration variable, which was
> +used by older gitweb.
Language nits: missing "the" before "URI of gitweb's standard stylesheet";
missing "default" before "URI of the gitweb stylesheet"; "is set
during" should be "can be set at"; "via `GITWEB_CSS` variable" should be
"using the `GITWEB_CSS` makefile variable"; extra apostrophe in "It's";
missing "a" before "legacy `$stylesheet` configuration variable".
If I am new on the job and find the $stylesheet variable set, what
should I interpret it to mean? How can it be translated to the newer
@stylesheets style? What happens if both variables are set --- does
one override the other, or are they combined somehow?
> +
> +$logo::
> + Points to the location where you put 'git-logo.png' on your web
> + server, or to be more generic URI of logo, 72x27 size). This image
> + is displayed in top right corner of each gitweb page, and used as
> + logo for Atom feed. Relative to base URI of gitweb (as a path).
> + Can be adjusted during building gitweb using `GITWEB_LOGO` variable
> + [Default: 'static/git-logo.png']
Language nits: missing "the" before "generic URI", "top-right corner",
"Atom feed", and "base URI of gitweb"; missing "a" before "logo";
there should not be a comma before "and used"; "during" sounds more
natural when it is replaced with "when".
> +
> +$favicon::
> + Points to the location where you put 'git-favicon.png' on your web
> + server, or to be more generic URI of favicon, assumed to be
> + "image/png" type; web browsers that support favicons (website icons)
> + may display them in the browser's URL bar and next to site name in
> + bookmarks. Relative to base URI of gitweb. Can be adjusted during
> + build time using `GITWEB_FAVICON` variable.
> + [Default: 'static/git-favicon.png']
Language nits: missing "the" before "generic URI", "site name", and
"base URI"; missing "your chosen" before "favicon"; "assumed to be"
should be "which will be served as" or something like that; would be
easier to read with the semicolon before "web browsers" replaced by a
full stop; "may" should be "will"; "during" sounds more natural when
it is replaced with "at".
> +
> +$javascript::
> + Points to the location where you put 'gitweb.js' on your web server,
> + or to be more generic URI of JavaScript code used by gitweb.
> + Relative to base URI of gitweb. Set during build time using
> + `GITWEB_JS` build-time configuration variable.
> ++
> +Default value is either 'static/gitweb.js', or 'static/gitweb.min.js' if
> +`JSMIN` build variable is defined, i.e. if JavaScript minifier is used
> +during build. *Note* that this single file is build (composed) of
> +individual JavaScript "modules".
Language nits: missing "the" before "URI", "Javascript library", "base
URI", "`GITWEB_JS`" makefile variable", "Default value", and "`JSMIN`
makefile variable"; "Set during" sounds more natural when it is
replaced by "Can be set at"; "is defined" and "is used" should be "was
defined" and "was used" to reflect installation probably already
having occured; "during build" sounds more natural when it is replaced
by "at build time"; "build of" should be "built from" or "generated
from multiple".
> +
> +$home_link::
> + Target of the home link on top of all pages (the first part of view
> + "breadcrumbs"). By default set to absolute URI of a page ($my_uri).
Language nit: missing "the" before "top" and "absolute URI".
Which page does "a page" refer to here (doesn't $my_uri change from
request to request)?
> +
> +$home_link_str::
> + Description of the home link on top of all pages, leading to $home_link
> + (usually main gitweb page, which means projects list). Used as first
> + part of gitweb view "breadcrumb trail": `<home> / <project> / <view>`.
> + Can be set during build time using `GITWEB_HOME_LINK_STR` variable.
> + [Default: "projects"]
Language nits: "Description of" means "Label for", I suppose; "home
link" should be "home page link" (?); "on top" is "at the top";
missing "the" before "main gitweb page"; "which means projects list"
is more friendly if paraphrased as "which contains the projects list";
"part of gitweb view" could be paraphrased as "component of gitweb's";
"during" sounds more natural if replaced by "at"; missing "the" before
"`GITWEB_HOME_LINK_STR` makefile variable".
> +
> +$logo_url::
> +$logo_label::
> + URI and label (title) of GIT logo link (or your site logo, if you choose
> + to use different logo image). By default they point to git homepage;
> + in the past they pointed to git documentation at www.kernel.org.
Language nits: "of GIT logo link" should probably be "for the Git logo
link"; "choose" should be "chose" since that's in the past when the
admin is setting this variable; missing comma after "By default" and
"in the past"; the first "they" should be "these both".
> +
> +
> +Changing gitweb look
> +~~~~~~~~~~~~~~~~~~~~
> +You can adjust how pages generated by gitweb look using variables described
> +below. You can change site name, add common headers and footers for all
> +pages, and add description of gitweb installation on its main page (which is
> +projects list page), etc.
Language nits: missing "'s" in "Changing gitweb's look"; missing "the"
before "variables described below", "site name", and "project list
page"; missing "a" before "description"; missing "this" before "gitweb
installation".
> +
> +$site_name::
> + Name of your site or organization to appear in page titles. Set it
> + to something descriptive for clearer bookmarks etc. If not set (if
> + empty) then gitweb uses value of `SERVER_NAME` CGI environment
> + variable setting prefix of each page title to "$SERVER_NAME Git", or
> + "Untitled Git" if this variable is not set (e.g. if running gitweb
> + as standalone script).
> ++
> +Can be set using `GITWEB_SITENAME` during building. [No default]
Language nits: missing comma after "organization" and before "for use
in page titles"; missing "the" before "the value", "`SERVER_NAME`
environment variable", and "`GITWEB_SITENAME` makefile variable";
"If not set" is missing a subject --- "If this variable is unset or
empty, then gitweb will ..."; "during building" sounds more natural
when replaced by "at build time".
> +
> +$site_header::
> + Filename of html text to include at top of each page. Relative to
> + 'gitweb.cgi' script. Can be set using `GITWEB_SITE_HEADER` during
> + building. [No default]
Language nits: "Filename of html text to include" should be "Name of a
file with HTML to be included" (or "Path to ..."); "'gitweb.cgi'
script" should be "the directory containing the 'gitweb.cgi' script";
"during building" sounds more natural when replaced by "at build
time".
> +
> +$site_footer::
> + Filename of html text to include at bottom of each page. Relative to
> + gitweb.cgi script. Can be set using `GITWEB_SITE_FOOTER` during
> + building. [No default]
Likewise.
> +
> +$home_text::
> + Points to an HTML file which, if it exists, is included on the
> + gitweb projects overview page ("projects_list" view). Relative to
> + gitweb.cgi script. Default value can be adjusted using during build
> + via `GITWEB_HOMETEXT` variable.
> + [Default: 'indextext.html']
Likewise.
> +
> +$projects_list_description_width::
> + The width (in characters) of the projects list "Description" column.
> + Longer descriptions will be cut (trying to cut at word boundary);
> + 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.
Language nit: 'project list "Description" column' sounds more natural when
rearranged to 'the "Description" column of the project list'; "cut" should
be "truncated"; missing "the" before "full description"; "as 'title'
attribute" sounds better as "in the 'title' attribute"; instead of "By
default set to", it is clearer to say "The default is".
> +
> +$default_projects_order::
> + Default value of ordering of projects on projects list page. Valid
> + values are "none" (unsorted), "project" (by project name, i.e. path
> + to repository relative to `$projectroot`), "descr" (project
> + description), "owner", and "age" (by date of most current commit).
> ++
> +Default value is "project". Unknown value means unsorted.
What does "Default" mean here (i.e., what overrides it)?
> +
> +
> +Changing gitweb behavior
> +~~~~~~~~~~~~~~~~~~~~~~~~
> +Those configuration variables control _internal_ gitweb behavior.
Language nits: missing "'s" in "gitweb's behavior"; "Those" should be
"These". Does internal behavior mean "functionality as opposed to
appearance and input location" or something like that?
> +
> +$default_blob_plain_mimetype::
> + Default mimetype for blob_plain (raw) view, if mimetype checking
> + doesn't result in some other type; by default "text/plain".
Language nit: missing "the" before "blob_plain (raw) view".
Where can I look to read more about mimetype autodetection?
> +
> +$default_text_plain_charset::
> + Default charset for text files. If not set, web server configuration
> + would be used.
Language nits: to make clear that it is not the web server
configuration that is not set, "If not set" could be paraphrased to
"If this is unset"; "would" should be "will" (yes, conditionals in
English are weird); missing "the" before "web server's usual
configuration".
> +
> +$fallback_encoding::
> + Gitweb assumes this charset if line contains non-UTF-8 characters.
> + Fallback decoding is used without error checking, so it can be even
> + "utf-8". Value must be valid encoding; see *Encoding::Supported*(3pm)
> + man page for a list. By default "latin1", aka. "iso-8859-1".
Language nits: "if" should be "when"; missing "a" before "line" and
"valid encoding"; missing "The" before "Fallback encoding", "Value",
and "Encoding::Supported(3pm) manpage"; "By default" sounds a little
better as "The default is" or "Default:".
> +
> +@diff_opts::
> + Rename detection options for git-diff and git-diff-tree. By default
> + (\'-M'); set it to (\'-C') or (\'-C', \'-C') to also detect copies,
> + or set it to () i.e. empty list if you don't want to have renames
> + detection.
Language nit: "By default" sounds a little better as "The default is".
Probably worth mentioning that GNU patch still has problems with some
rename patches, especially when they involve file copies ['-C'] or
criss-cross renames ['-B'] (see [1] and [2], for example).
[1] http://savannah.gnu.org/bugs/?31058
[2] http://thread.gmane.org/gmane.linux.ports.sh.devel/8697/focus=8773
> +
> +Extra features, administrative
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Most of features are configured via `%feature` hash; however some of extra
> +gitweb features can be turned on and configured using variables described
> +below. This list beside configuration variables that control how gitweb
> +looks does contain variables configuring administrative side of gitweb
> +(e.g. cross-site scripting prevention; admittedly this as side effect
> +affects how "summary" pages look like, or load limiting).
Language nit: the title sounds like these are going to be extra
administrative features --- might be best to spell it out ("Some
optional features and policies" or something like that).
> +
> +@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`". You
> + can setup multiple base URLs (for example one for `git://` protocol
> + access, and one for `http://` "dumb" protocol access). Note that per
> + repository configuration can be set in '$GIT_DIR/cloneurl' file, or as
> + values of multi-value `gitweb.url` configuration variable in project
> + config.
> ++
> +You can setup one single value (single entry/item in this list) during
> +building by using `GITWEB_BASE_URL` built-time configuration variable.
> +[No default]
Language nits: "git base URLs used for URL to where fetch project from"
means something like "Git URLs relative to which projects can be
fetched"; missing "The" before and 'for the `"$projectroot/$project"`
repository' after "Full URL"; "setup" as a verb is spelled "set up" or
"set" (2x); "http://" access is not necessarily passive; "during
building" sounds better as "at build time"; "using" should be "setting
the"; "built-time configuration variable" is spelled as "build-time
configu..." or even better as "makefile variable"; "No default" does
not seem quite right --- I think the default is "Default: ()" (i.e.,
it defaults to the empty list).
Is the per repository cloneurl added to this list, or does it override
it?
> +
> +$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].
Language nits: "its repository configuration" probably means "each
repository's configuration"; "Enables" can be rephrased more clearly
as "Whether to enable" (assuming this is boolean).
> +
> +$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. Used only if project cateories are
> + enabled, which means if `$projects_list_group_categories` is true.
> + [Default: "" (empty string)]
Language nit: "If set to the empty string" is clearer as "If this is
set to the empty string" since it is not the projects but this
variable that is being set to the empty string.
> +
> +$prevent_xss::
> + If true, some gitweb features are disabled to prevent content in
> + repositories from launching cross-site scripting (XSS) attacks. Set this
> + to true if you don't trust the content of your repositories.
> + [Default: false].
Nice.
> +
> +$maxload::
> + Used to set the maximum load that we will still respond to gitweb queries.
> + If server load exceed this value then return "503 Service Unavailable"
> + error. Server load is taken to be 0 if gitweb cannot determine its value.
> + Set it to undefined value to turn it off. [Default: 300]
Probably worth spelling out that this is a "load average" (as shown by uptime(1)).
Language nits: missing "the" before "server load" (2x); instead of
"exceed", use "exceeds" (singular); missing subject "gitweb" before
"will return the"; clearer to say it is "this feature" rather than
"it" that can be turned off by setting this var to undef.
> +
> +$per_request_config::
> + If set to code reference, it would be run once per each request. You can
> + set parts of configuration that change per session, e.g. by adding
> + the following code to gitweb configuration file
> ++
> +--------------------------------------------------------------------------------
> +our $per_request_config = sub {
> + $ENV{GL_USER} = $cgi->remote_user || "gitweb";
> +};
> +--------------------------------------------------------------------------------
Language nits: missing "this is" before "set to a code reference";
"would" should be "will" (those crazy English conditionals again); the
appropriate preposition before "each request" is "for", not "per";
missing "this way" after "change per session"; would be clearer with a
full stop before the example, and with the example made into a full
sentence ("For example, one might use the following code in a gitweb
configuration file:").
> ++
> +Otherwise it is treated as boolean value: if true gitweb would process
> +config file once per request, if false it would process config file only
> +once. [Default: true]
> ++
> +*NOTE*: `$my_url`, `$my_uri`, and `$base_url` are overwritten with their default
> +values before every request, so if you want to change them, be sure to set
> +this variable to true or a code reference effecting the desired changes.
At this point, I've forgotten what the "Otherwise" is contrasting with;
probably best to repeat it ("If $per_request_config is not a code
reference, it is interpreted as a boolean value."); "would" should be
"will" again (2x); missing "the" or "each" before "config file" (2x);
missing "and" before "if it is false"; "it" could be "gitweb" to
avoid confusion in this pronoun-heavy sentence; missing "each time it
is executed" after "process the config file only once".
Probably worth mentioning that this variable starts to shine when one
gitweb instance is used to serve multiple requests, with CGI
replacements like mod_perl, fastcgi, plackup, and so on.
> +
> +
> +Other variables
> +~~~~~~~~~~~~~~~
> +Usually you should not need to change (adjust) any of configuration
> +variables described below; they should be automatically set by gitweb to
> +correct value.
> +
> +
> +$version::
> + Gitweb version, set automatically when creating gitweb.cgi from
> + gitweb.perl. You might want to modify it if you are running modified
> + gitweb, for example
> ++
> +---------------------------------------------------
> +our $version .= " with caching";
> +---------------------------------------------------
Might be worth mentioning this is only used in HTML comments and the
"generator" metadata field, nothing more special than that.
> +
> +$my_url::
> +$my_uri::
> + Full URL and absolute URL of gitweb script;
> + in earlier versions of gitweb you might have need to set those
> + variables, now there should be no need to do it. See
> + `$per_request_config` if you need to set them still.
Language nits: missing "the" before "gitweb script" and "but" before
"now".
What is the difference between a full URL and an absolute URL?
> +
> +$base_url::
> + Base URL for relative URLs in pages generated by gitweb,
> + (e.g. `$logo`, `$favicon`, `@stylesheets` if they are relative URLs),
> + needed and used only for URLs with nonempty PATH_INFO via
> + '<base href="$base_url">'. Usually gitweb sets its value correctly,
> + and there is no need to set this variable, e.g. to $my_uri or "/".
> + See `$per_request_config` if you need to set it anyway.
Language nits: clearer with "via '<base ...>'" moved before "only for
URLs with nonempty PATH_INFO"; "no need to set" could be "no need to
override".
> +
> +
> +CONFIGURING GITWEB FEATURES
> +---------------------------
> +Many gitweb features can be enabled (or disabled) and configured using the
> +`%feature` hash. Names of gitweb features are keys of this hash.
> +
> +Each `%feature` hash element is a hash reference and has the following
> +structure:
> +----------------------------------------------------------------------
> +"<feature_name>" => {
> + "sub" => <feature-sub (subroutine)>,
> + "override" => <allow-override (boolean)>,
> + "default" => [ <options>... ]
> +},
> +----------------------------------------------------------------------
Nice.
> +Some of features does not support project specific override. For those
> +features the structure of appropriate `%feature` hash element has a simpler
> +form:
> +----------------------------------------------------------------------
> +"<feature_name>" => {
> + "override" => 0,
> + "default" => [ <options>... ]
> +},
> +----------------------------------------------------------------------
> +As one can see it lacks \'sub' element.
Language nits: the "of" in "Some of features" is extraneous; "does" should
be "do" (plural) because we are talking about multiple features,
or, better, be replaced by "cannot be overriden per project" (so:
"Some features cannot be overridden per project"); missing "the"
before "'sub' field".
> +
> +The meaning of respective parts of feature configuration are described
> +below:
Language nit: "respective parts" should be "each part", "each field",
or something like that; "are" needs to be "is" to agree with the
subject ('meaning').
> +
> +default::
> + List (array reference) of feature parameters (if there are any),
> + used also to toggle (enable or disable) given feature.
> ++
> +Note that it is currently *always* an array reference, even if
> +feature doesn't accept any configuration parameters, and \'default'
> +is used only to turn it on or off. In such case you turn feature on
> +by setting this element to `[1]`, and torn it off by setting it to
> +`[0]`. See also part about "blame" feature in the "Examples"
> +section.
> ++
> +To disable feature that accepts parameters (is configurable), you
> +need to set this element to empty list i.e. `[]`.
Nits: "part" means "passage" here; missing "the" before "passage" and
'"blame" feature'; "feature" should be "features", "accepts" should
be "accept", and "is configurable" should be "are configurable".
> +
> +override::
> + If it has a true value then given feature is overridable, which
> + means that it can be configured (or enabled/disabled) on
> + per-repository basis.
> ++
> +Usually given "<feature>" is configurable via `gitweb.<feature>`
> +config variable in per-repository git configuration file.
> ++
> +*Note* that no feature is overridable by default.
Nits: better to spell out what "it" is ("this field", I guess);
missing "the" before "given feature", "`gitweb.<feature>`
configuration variable", and "per-repository Git configuration file".
> +
> +sub::
> + Subroutine that will be called with default options as parameters to
> + examine per-repository configuration, but only if feature is
> + overridable (\'override' is set to true). If not present then
> + per-repository override for given feature is not supported.
> ++
> +You wouldn't need to ever change it in gitweb config file.
Language nits: "default options as parameters" means "the array
referred to by the 'default' field as parameter list", I guess.
Missing "the" before "feature is overridable" and "gitweb
configuration files". "given feature" -> "this feature". "not
supported" -> "not enabled", maybe.
Actually, I'm not sure I understand this one. Are users supposed to
set this field? Where can they look to find out what features have it
set by default? Who calls this function, and what does that person or
code path do with the return value? If it is not part of the gitweb
configuration that an administrator might modify (the subject of this
manpage), why not just say that ("subroutine used internally; present
if and only if this feature can be made overridable with the
"override" field; you should not change it") and leave it at that?
> +
> +
> +Features in `%feature`
> +~~~~~~~~~~~~~~~~~~~~~~
> +Below there are described gitweb features that are configurable via
> +`%feature` hash. For a complete list please consult gitweb sources.
Language nit: "Below there are described gitweb features ..." -> "The
gitweb features ... are listed below". The second sentence could
probably be more apologetic, something like "Currently the only
authoritative and complete list is in the comments in the gitweb.cgi
source code."
> +
> +blame::
> + Enable the "blame" and "blame_incremental" blob views, showing for
> + each line the last commit that modified it; see linkgit:git-blame[1].
> + This can be very CPU-intensive and is therefore disabled by default.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.blame` configuration variable (boolean).
Nice.
> +
> +snapshot::
> + Enable and configure "snapshot" action, providing a compressed
> + archive of any tree or commit; see also linkgit:git-archive[1].
> + This can potentally generate high traffic if you have large project.
> ++
> +The value (of \'default') is a list of names of snapshot formats
> +defined in `%known_snapshot_formats` hash, that you wish to offer.
> +Among supported formats are "tgz", "tbz2", "txz" (gzip/bzip2/xz
> +compressed tar archive) and "zip"; please consult gitweb sources for
> +a definitive list. By default only "tgz" is offered.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.blame` configuration variable, which contains
> +a comma separated list of formats, or "none" to disable snapshots.
> +Unknown values will be ignored.
Language nits: missing "the" before "snapshot action"; "providing"
would be clearer if replaced by "which allows users to download" or
"which serves" or something like that; instead of "see also" it might
be nice to say "as produced by" because more precise; would be clearer
without parentheses around "of 'default'"; missing comma before
"defined in the %known_snapshot_formats hash"; "Among supported
formats are" is clearer as "Supported formats include"; comma before
"or 'none' to disable snapshots" is extraneous; "will be ignored"
should be "are ignored".
> +
> +grep::
> + Enable grep search, which will list the files in currently selected
> + tree (directory) containing the given string; see linkgit:git-grep[1].
> + This can be potentially CPU-intensive, of course. Enabled by default.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.grep` configuration variable (boolean).
Language nit: "will list" should be "lists". Nice.
> +
> +pickaxe::
> + Enable the so called pickaxe search, which will list the commits
> + that modified a given string in a file. This can be practical and
> + quite faster alternative to "blame" action, but still potentially
> + CPU-intensive. Enabled by default.
> ++
> +The pickaxe search is described in linkgit:git-log[1] (the
> +description of `-S<string>` option, which refers to pickaxe entry in
> +linkgit:gitdiffcore[7] for more details).
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.pickaxe` configuration variable (boolean).
I think "modified" means "introduced or removed" (as an approximation to
"changes the number of occurences of"). Missing "it is" before "still
potentially CPU-intensive". "via" seems to mean "using the".
(By the way, is there any way for the the very paranoid to limit the
length of regexes used or CPU time used per request, to get some
reasonable cap on this sort of thing? I guess that's more in the
realm of web server configuration than something gitweb should be
responsible for --- but if there's some common practice or well known
reference on the topic, it could be worth mentioning at the top of
this GITWEB FEATURES section some day.)
> +
> +show-sizes::
> + Enable showing size of blobs (ordinary files) in a "tree" view, in a
> + separate column, similar to what `ls -l` does; see description of
> + `-l` option in linkgit:git-ls-tree[1] manpage. This cost a bit of
> + I/O. Enabled by default.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.showsizes` configuration variable (boolean).
Nit: "cost" -> "costs". Interesting (I guess packv4 will help with
the CPU [decompression] cost of this when it comes).
> +
> +patches::
> + Configure "patches" view, which displays list of commits in email
> + (plain text) output format; see also linkgit:git-format-patch[1].
> + The value is the maximum number of patches in a patchset generated
> + in "patches" view. Set this to 0 or undef to disable patch view, or
> + to a negative number to remove any limit. Default value is 16.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.patches` configuration variable (integer).
"Configure" -> "Enable and configure" and "Set this to ..." -> "Set
the 'default' field to a list containing the single item ...", I
guess.
> +
> +avatar::
> + Avatar support. When this feature is enabled, views such as
> + "shortlog" or "commit" will display an avatar associated with
> + the email of the committer(s) and/or author(s).
> ++
> +Currently available providers are *"gravatar"* and *"picon"*.
> +If an unknown provider is specified, the feature is disabled.
> +*Note* that some provides might require extra Perl packages to be
> +installed; see 'gitweb/INSTALL' for more details.
"commiter(s) and/or author(s)" -> "each committer and author".
If I set 'default' to ["gravator", "picon", "unknown"], will that
really disable the feature?
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.avatar` configuration variable.
> ++
> +See also `%avatar_size` with pixel sizes for icons and avatars
> +("default" is used for one-line like "log" and "shortlog", "double"
> +is used for two-line like "commit", "commitdiff" or "tag"). If the
> +default font sizes or lineheights are changed (e.g. via adding extra
> +CSS stylesheet in `@stylesheets`), it may be appropriate to change
> +these values.
What is the syntax of the "[gitweb] avatar" value --- space delimited?
comma delimited?
> +
> +highlight::
> + Server-side syntax hightlight support in "blob" view. It requires
> + `$highlight_bin` program available (see description of this variable
> + in "Configuration variables" section above), and therefore is
> + disabled by default.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.highlight` configuration variable (boolean).
Language nit: missing "to be" before "available", "the" before
"description" and '"Configuration variables" section'; the comma
before "and" is extraneous.
> +
> +remote_heads::
> + Enable displaying remote heads (remote-tracking branches) in the "heads"
> + list. In most cases it is unnecessary internal private detail, and it
> + is therefore disabled by default. linkgit:git-instaweb[1], which is
> + usually used to browse local repositories, enables and uses this
> + feature.
> ++
> +This feature can be configured on a per-repository basis via
> +repository's `gitweb.remote_heads` configuration variable (boolean).
Could clarify that "it" means "the list of remote-tracking branches"
first and then "this feature". Missing "an" before "irrelevant,
private detail".
> +
> +
> +The list below presents features that do not allow project specific
> +overrides.
If we consider it one list: "The remaining features cannot be
overridden on a per project basis".
> +
> +search::
> + Enable text search, which will list the commits which match author,
> + committer or commit text to a given string; see description of
> + `--author`, `--committer` and `--grep` options in linkgit:git-log[1]
> + manpage. Enabled by default.
> ++
> +Project specific override is not supported.
Nice. I prefer "Enables" over "Enable" so commands can be directed at
the reader only, but the current text is clear enough. Missing "the"
before "descriptions".
> +
> +forks::
> + Make gitweb consider projects in subdirectories of project root
> + (basename) to be forks of existing projects. Given project
> + `$projname.git`, projects matching `$projname/*.git` will not be
> + shown in the main projects list, instead a \'+' mark will be added
> + to `$projname` there and a "forks" view will be enabled for the
> + project, listing all the forks. If project list is taken from a
> + file, forks have to be listed after the main project.
> ++
> +Project specific override is not supported.
To avoid questions of who made it happen: "If this feature is enabled,
gitweb considers projects in subdirectories ...".
If I understand correctly, "Given project `$projname.git`, projects
matching `$projname/*.git`" can be written as "If the project
`$projname.git` exists, projects in the `$projname/` directory". Is
it just projects in that directory, or are subdirectories included, as
in the following example?
project.git
project/foo/bar/baz/qux.git
Replacing the comma before "instead" with a full stop, adding a
comma after "instead", and putting the sentence in the present tense
("Instead, a '+' mark is shown next to `$projname` and links to a
"forks" view that lists projects in the $projname/ subdirectory)
seems to make it clearer.
Missing "the" before "project list"; missing "in that file" after
"after the main project"; "have to be" probably means "are only
recognized if".
> +
> +actions::
> + Insert custom links to the action bar of all project pages. This
> + enables you to link to third-party scripts integrating into gitweb.
> ++
> +The "default" value consists of a list of triplets in the form
> +`("<label>", "<link>", "<position>")` where "position" is the label
> +after which to insert the link, "link" is a format string where `%n`
> +expands to the project name, `%f` to the project path within the
> +filesystem, `%h` to the current hash (\'h' gitweb parameter) and
> +`%b` to the current hash base (\'hb' gitweb parameter); `%%` expands
> +to \'%'.
Interesting. "enables" should be "allows" here.
Is %f an absolute path (i.e., starting with '/')?
> ++
> +For example http://repo.or.cz git hosting site sets it to the
> +following to enable graphical log (via third party tool
> +*git-browser*):
> ++
> +----------------------------------------------------------------------
> +$feature{'actions'}{'default'} =
> + [ ('graphiclog', '/git-browser/by-commit.html?r=%n', 'summary')];
> +----------------------------------------------------------------------
> ++
> +This adds link titled "graphiclog" after "summary" link, leading to
> +`git-browser` script, passing `r=<project>` as a query parameter.
Missing ", at the time this page was written, the" after "For
example" (with "sets" replaced by "set" to match); "via" sounds
more natural when replaced by "using the"; missing "a" before "link";
missing "the" before "summary link on the project listing and project
pages".
> ++
> +Project specific override is not supported.
> +
> +timed::
> + Enable displaying how much time and how many git commands it took to
> + generate and display page in the page footer (at the bottom of
> + page). For example it might be: "This page took 6.53325 seconds and
> + 13 git commands to generate." Disabled by default.
> ++
> +Project specific override is not supported.
Missing "each" between "display" and "page". "it might be" -> "the
footer might contain".
> +
> +javascript-timezone::
> + Enable and configure ability to change common timezone for dates in
> + gitweb output via JavaScript. Dates in gitweb output include
> + authordate and comitterdate in "commit", "commitdiff" and "log"
> + views, and taggerdate in "tag" view. Enabled by default.
> ++
> +Value is list of three values: default timezone (if client didn't
> +select other timezone and saved it in a cookie), name of cookie
> +where to store selected timezone, and CSS class used to mark up
> +dates for manipulation. If you want to turn it off, set "default"
> +to empty list: `[]`.
> ++
Language nits: missing "the" before "ability", "client", and "Value";
missing "a" before "common timezone", "list of three values", "default
timezone", "cookie name", and "HTML class name" (why is this last one
configurable?); "change" should probably be "choose"; missing "for
users" before "to choose"; "didn't select other timezone" -> "hasn't
selected some other timezone"; missing "for" before "if the client
hasn't selected"; "turn it off" would be clearer as "turn this feature
off".
> +In most case you would want to change only default timezone:
> ++
> +---------------------------------------------------------------------------
> +$feature{'javascript-timezone'}{'default'}[0] = "utc";
> +---------------------------------------------------------------------------
Nice. "In most case you would want to change only" -> "Typical gitweb
config files will only change" can take the desires of an admin that
is implementing someone else's will out of the equation.
> ++
> +Timezone value can be "local" (for local timezone that browser uses), "utc"
> +(what gitweb uses when JavaScript or this feature is disabled), or numerical
> +timezone in the form of "+/-HHMM" for example "+0200". This way is
> +guaranteed to be backward compatibile.
> ++
> +Project specific override is not supported.
Language nits: when we are talking about supported values in general,
it sounds better for some reason with "value" -> values"; "numerical
timezone" -> "numerical timezones"; "for example" -> "such as".
What is "This way", and what other way should people be avoiding to
prevent forward compatibility gotchas?
> +
> +
> +EXAMPLES
> +--------
Hoorah!
[...]
> +ENVIRONMENT
> +-----------
> +The location of per-instance and system-wide configuration files can be set
> +using the following environment variables:
Clearer to say "overridden" to emphasize that these replace rather
than supplementing the locations described under FILES.
[...]
> +GITWEB_CONFIG::
> + Sets location of per-instance configuration file.
[...]
> +FILES
> +-----
> +gitweb_config.perl::
> + This is default value for per-instance configuration file. The
> + format of this file is described above.
> +/etc/gitweb.conf::
> + This is default value for fallback system-wide configuration
> + file. This file is used only if per-instance configuration
> + variable is not found.
> +/etc/gitweb-common.conf::
> + This is default value for common system-wide configuration
> + file.
"default value" sounds strange here --- I guess I would have expected
something more like
ENVIRONMENT
-----------
...
GITWEB_CONFIG::
Path to use to find the per-instance configuration file,
instead of gitweb_config.perl. If relative, this is relative
to $GITWEBDIR.
FILES
-----
/etc/gitweb-common.conf::
Options to be shared by all gitweb instances. The format is
described above.
$GITWEBDIR/gitweb_config.perl::
Additional settings for a particular gitweb instance (in
the same format).
/etc/gitweb.conf::
Fallback configuration file with settings for gitweb instances
that do not contain a gitweb_config.perl file.
[...]
> gitweb/README | 133 +-------
[...]
> +++ b/gitweb/README
> @@ -41,139 +41,8 @@ Ultimate description on how to reconfigure the default features setting
> in your `GITWEB_CONFIG` or per-project in `project.git/config` can be found
> as comments inside 'gitweb.cgi'.
>
> -See also the "Gitweb config file" (with an example of config file), and
[...]
> +See also gitweb.conf(5) manpage.
> +
Thanks very much for this.
^ permalink raw reply
* Re: [PATCH 0/8] fast-import: cache oe more often
From: Jonathan Nieder @ 2011-09-20 4:26 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dmitry Ivankov, git, Shawn O. Pearce, David Barr,
Sverre Rabbelier
In-Reply-To: <7vy5xj7tf5.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> I gave the series a cursory look, and the patches all looked like a good
> and straight forward rewrites. Provided if it is indeed a good idea
> overall to stuff more objects in-core, that is.
Right, that's exactly the question I had. When and why is it a good
idea to stuff more objects in-core (or when might it be a bad idea,
for that matter)?
^ permalink raw reply
* Re: [PATCH 0/8] fast-import: cache oe more often
From: Junio C Hamano @ 2011-09-20 4:02 UTC (permalink / raw)
To: Dmitry Ivankov
Cc: git, Jonathan Nieder, Shawn O. Pearce, David Barr,
Sverre Rabbelier
In-Reply-To: <1316395657-6991-1-git-send-email-divanorama@gmail.com>
Dmitry Ivankov <divanorama@gmail.com> writes:
> fast-import keeps a struct object_entry for each object written to
> it's pack. This is to keep type, pack-coordinates and delta_depth.
> struct object_entry is also used to cache this metadata for objects
> that exist outside fast-import's pack ('old' objects).
> struct object_entry has a small fixed size and thus it should be
> reasonable to cache any 'old' object metadata retrieval to save the
> disk i/o.
>
> Also it is a step toward making fast-import identify objects via
> struct object_entry rather than sha1. One pointer takes less than
> 20 bytes, it'll be later possible to have references to objects
> that don't yet have sha1 computed (fast-import with threads future).
I gave the series a cursory look, and the patches all looked like a good
and straight forward rewrites. Provided if it is indeed a good idea
overall to stuff more objects in-core, that is.
Hopefully people more involved in fast-import can review and ack after the
pre-release feature freeze.
^ permalink raw reply
* Re: [PATCH] git-mergetool: check return value from read
From: Andrew Ardill @ 2011-09-20 1:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jay Soffian, git, David Aguilar
In-Reply-To: <7vboug82qk.fsf@alter.siamese.dyndns.org>
On 20 September 2011 10:41, Junio C Hamano <gitster@pobox.com> wrote:
> Jay Soffian <jaysoffian@gmail.com> writes:
>
>>> I suspect that it would be more consistent with 6b44577 (mergetool: check
>>> return value from read, 2011-07-01), which this patch is a follow-up to,
>>> to do:
>>>
>>> read answer || return 1
>>>
>>> here.
>>
>> Thanks, sorry I missed that.
>
> Thank _you_ for spotting these unchecked "read"s. Will queue.
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
I got hit by the ctrl+c bug while using mergetool just the other day.
Thanks for the fix :)
^ permalink raw reply
* Re: how to remove unreachable objects?
From: Jeff King @ 2011-09-20 0:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: dieter, git
In-Reply-To: <7vfwjs82sc.fsf@alter.siamese.dyndns.org>
On Mon, Sep 19, 2011 at 05:40:03PM -0700, Junio C Hamano wrote:
> > Does that work? I had the impression from the documentation that the
> > arguments are purely about the reachability analysis, and that the
> > actual corruption/correctness checks actually look through the object db
> > directly, making sure each object is well-formed. Skimming cmd_fsck
> > seems to confirm that.
>
> You are right that you may see "corrupt object" for unreachable from the
> tips in the object store, but I was talking more about verifying
> everything that is needed for reachability analysis from the given tips
> can be read, iow, "missing object" errors, lack of which would mean you
> can salvage everything reachable from the refs involved in the traversal.
True. Though one could also do that with "git log", and it would be much
cheaper (since each trial you run with git-fsck is going to actually
fsck the object db, which is expensive).
I can't help but think the right solution there is something like:
1. If the corrupted or missing object is a blob or tree, figure out
which commits reference it with something like:
a. Create a set B of bad objects (blobs or trees).
b. For each tree in the object db, open and see if it contains
any elements of B. If so, add the tree to another set, B'.
c. If B' is empty, done. Otherwise, add elements from B' to B and
goto step (b).
d. For each commit in the object db, open and check the tree
pointer. If it points to an element of B, then the commit is
bad.
2. If the object is a commit, or if you arrived at a set of bad
commits through step (1), then use "branch --contains" on the
bad commits.
which is algorithmically efficient (though probably slow if you had to
cat-file each tree). It might be a handy special command, though (I have
seen people ask for "which part of history references this blob" on
occasion). I've never bothered writing it because I've never had a
corrupt object. :)
Anyway, that is perhaps not relevant to your point. But I do think that
fsck with arguments is more likely to confuse someone than to actually
be part of a productive use-case. I have no problem with deprecating or
removing it.
-Peff
^ permalink raw reply
* Re: [PATCH] git-mergetool: check return value from read
From: Junio C Hamano @ 2011-09-20 0:41 UTC (permalink / raw)
To: Jay Soffian; +Cc: git, David Aguilar
In-Reply-To: <1316475652-35188-1-git-send-email-jaysoffian@gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
>> I suspect that it would be more consistent with 6b44577 (mergetool: check
>> return value from read, 2011-07-01), which this patch is a follow-up to,
>> to do:
>>
>> read answer || return 1
>>
>> here.
>
> Thanks, sorry I missed that.
Thank _you_ for spotting these unchecked "read"s. Will queue.
^ permalink raw reply
* Re: how to remove unreachable objects?
From: Junio C Hamano @ 2011-09-20 0:40 UTC (permalink / raw)
To: Jeff King; +Cc: dieter, git
In-Reply-To: <20110919225219.GD4056@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> One possible use case might be when your repository is corrupt, and does
>> not pass "git fsck" (without any argument). In such a case, if you are
>> lucky and your disk corrupted objects only reachable from a recent topic
>> branch, you might find that this command:
>>
>> $ git fsck master next ...list other topics here...
>>
>> still succeeds, so that you can figure out which topic makes such a
>> limited fsck fail when it is listed on the command line, judge its
>> importance and resurrect what you can from there, before nuking it to
>> bring the repository back in health so that you can recreate the topic.
>
> Does that work? I had the impression from the documentation that the
> arguments are purely about the reachability analysis, and that the
> actual corruption/correctness checks actually look through the object db
> directly, making sure each object is well-formed. Skimming cmd_fsck
> seems to confirm that.
You are right that you may see "corrupt object" for unreachable from the
tips in the object store, but I was talking more about verifying
everything that is needed for reachability analysis from the given tips
can be read, iow, "missing object" errors, lack of which would mean you
can salvage everything reachable from the refs involved in the traversal.
^ permalink raw reply
* Re: Branching strategies
From: Andrew Ardill @ 2011-09-20 0:12 UTC (permalink / raw)
To: gitlist; +Cc: git
In-Reply-To: <4E778F15.9050705@myword.co.uk>
On 20 September 2011 04:51, gitlist <gitlist@myword.co.uk> wrote:
>> A very interesting read is
>> http://nvie.com/posts/a-successful-git-branching-model/
>>
>> It may not be perfect for you, however it does discuss some very
>> interesting issues, particularly how workflow is just as important as
>> the branching model.
>
>
> I came across the nvie post some time and it was very useful, but it doesn't
> address handling of feature branches, especially where there is overlap.
>
> I have a website where people can register. They can also buy things. If
> they haven't registered when they come to checkout, the checkout process
> includes registration. Users can also create "sponsorship" pages where they
> ask friends to sponsor them in a marathon etc. If someone setting up a
> sponsorship page is not already registered, it's included in the process.
>
> So there are three strands (to avoid using the word "branch") -
> registration, buying, and sponsorship - which end up affecting the same bits
> of code. Registration was done a year ago but recently needed updating;
> buying was started some months ago but got held up; sponsorship started
> recently, has been completed, and has "overtaken" buying.
>
> How should I use branches in this scenario? Or if I've got the concept
> wrong, how should I change my workflow?
>
> Thanks
>
> Roddie Grant
The concept of 'feature branches' apply primarily to development.They
allow you to build individual features for your system independently
of one another, and concurrently. Further, they provide a history of
commits that detail the effort that went in to creating that feature.
Once the feature is completed however, typically it will get merged
with a given release of the system. If further work is to be done on
that feature, either a new feature branch is created for the
'improvement', or a similar hot-fix/bug-fix branch is created. When
that work is done it gets merged either back on to the same release or
into a future release.
These feature branches do not host the separate functionality of the
system, but instead reveal the history of their development. I have
not heard of any intelligent way to use just an SCM to partition and
keep separate the different functional aspects of a system; developing
modules is generally achieved by writing modular code.
If you do have a number of modules you can track each of them
individually. Write version-dependencies into the modules, and you can
develop on them individually, updating them to depend on the latest
version as appropriate.
If you are not using well separated modules, and the development of
each 'thread' affects the same 'core' files, you need to migrate
changes to these core files between threads when appropriate. For
example, if you are working on the sponsorship thread, and develop a
paypal integration that you now want to include in the buying thread
(and can't continue developing it until you do), you should identify
the commits that entail that change and merge them into the buying
thread branch. Better than retrospectively trying to dig out features
and bug-fixes, however, is ensuring a clean separation of thread code
and core code. Thankfully, since git has such good merging ability,
you can often merge a feature branch which is miles behind the latest
release and it will slot in flawlessly.
There are many different aspects to this sort of work, but the best
thing to do is to keep the future in mind, and try to do things that
will make life easier down the track. The great thing about git is
that it offers many tools that make things easier later on even when
you didn't plan for them - but that is no excuse for poor planning!
Hope that helps somewhat,
Andrew
^ permalink raw reply
* Re: [PATCH/RFCv4 1/4] Documentation: Preparation for gitweb manpages
From: Jakub Narebski @ 2011-09-20 0:10 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Drew Northup, John 'Warthog9' Hawley, admin
In-Reply-To: <20110919233703.GF6343@elie>
Thanks a lot for all language fixes.
Nb. I'll respond to other comments separately.
On Tue, 20 Sep 2011, Jonathan Nieder wrote:
> Jakub Narebski wrote:
> > This way to build gitweb documentation one can use
> >
> > make -C gitweb doc
> >
> > or
> >
> > cd gitweb; make doc
Actually it should read "to build only gitweb documentation"...
> Does "make -CDocumentation man html" build the gitweb documentation,
> too (and "make install-doc" install it)?
>
> [...]
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -170,6 +170,9 @@ info: git.info gitman.info
> >
> > pdf: user-manual.pdf
> >
> > +GITWEB_DOC = $(filter gitweb.%,$(DOC_HTML) $(DOC_MAN1) $(DOC_MAN5) $(DOC_MAN7))
> > +gitweb-doc: $(GITWEB_DOC)
>
> Looks like no, alas.
... because of the fact that gitweb.txt is in DOC_MAN1, and gitweb.conf.txt
is in DOC_MAN5, then 'make doc' would among others build gitweb
documentation. 'make -C Documentation gitweb-doc' / 'make -C gitweb doc'
just builds a _subset_ of said documentation.
>
> Except for that detail, this looks good.
Nb. what is also missing is having links to gitweb(1) and gitweb.conf(5)
in git(1) manpages together with the rest.
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] git-mergetool: check return value from read
From: Jay Soffian @ 2011-09-19 23:40 UTC (permalink / raw)
To: git; +Cc: Jay Soffian, David Aguilar, Junio C Hamano
In-Reply-To: <7vaaa09skn.fsf@alter.siamese.dyndns.org>
Mostly fixed already by 6b44577 (mergetool: check return value
from read, 2011-07-01). Catch two uses it missed.
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
On Mon, Sep 19, 2011 at 4:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> We still seem to miss one "read" unchecked in resolve_symlink_merge(),
> even with this patch.
> [...]
> I suspect that it would be more consistent with 6b44577 (mergetool: check
> return value from read, 2011-07-01), which this patch is a follow-up to,
> to do:
>
> read answer || return 1
>
> here.
Thanks, sorry I missed that.
git-mergetool--lib.sh | 2 +-
git-mergetool.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 8fc65d0400..ed630b208a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -21,7 +21,7 @@ check_unchanged () {
do
echo "$MERGED seems unchanged."
printf "Was the merge successful? [y/n] "
- read answer
+ read answer || return 1
case "$answer" in
y*|Y*) status=0; break ;;
n*|N*) status=1; break ;;
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 3c157bcd26..b6d463f0d0 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -72,7 +72,7 @@ describe_file () {
resolve_symlink_merge () {
while true; do
printf "Use (l)ocal or (r)emote, or (a)bort? "
- read ans
+ read ans || return 1
case "$ans" in
[lL]*)
git checkout-index -f --stage=2 -- "$MERGED"
--
1.7.7.rc2.2.gdf97720
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox