* Re: [PATCH bg/format-patch-p-noop] log-tree: always add --- marker when options are patch and a stat
From: Junio C Hamano @ 2009-11-04 5:49 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Björn Gustavsson
In-Reply-To: <1257283456-7007-1-git-send-email-bebarino@gmail.com>
Stephen Boyd <bebarino@gmail.com> writes:
> Previously, the three dash marker (---) would only be added if the diff
> output format was a patch and diffstat (usually -p and --stat). Now that
> patches are always generated by format-patch regardless of the stat
> format being used (--stat, --raw, --numstat, etc.), always add the three
> dash marker when a patch is being generated and a stat option is used.
>
> This allows users to choose the stat format they want and unifies the
> format of patches with stats. It also make patches easier to apply when
> generated by format-patch with non-standard stat options as the stat is
> no longer considered part of the commit message.
>
> Signed-off-by: Stephen Boyd <bebarino@gmail.com>
> ---
I actually am more worried about 68daa64 from 14 months ago, as I vaguely
recall seeing an explicit user request that in some community the diffstat
information is unwanted on their mailing list, and I am reasonably sure
that "-p suppresses --stat" was done deliberately to satisfy them (even
though it may have been a suboptimal UI and --no-stat might have been a
lot more straightforward).
Even though I personally find the stat information very useful, I would be
happier if somebody reverts the bg/format-patch-p-noop series and instead
fixes the regression caused by 68daa64, and does so without touching any
output from the low-level plumbing like diff-tree that may be used by
scripts.
With older (say 1.6.0) git, format-patch with the -p option does not give
these three-dash lines, and it does look funny. Even though the same
funniness appears only when you use --raw or --numstat with the current
code, if we fix "-p" to suppress the default "--stat", this will become an
issue again.
> I'm not sure this is wanted though and I guess this could break people's
> scripts. Are people actually using --numstat or --raw to put the stat into
> the commit message?
I am not worried so much about "format-patch --any-option" output; I think
it is sane to have three-dash line in it and that is what people expect to
see.
I however think people used "diff-tree --pretty --raw" as a mechanism to
obtain statistics. A script can easily see where the header is and where
messages are (they are four-space indented), and what remains after
stripping them give you the list of paths each commit touches. --numstat
was invented to help this kind of application gather line-level statistics
more easily, and I am a bit reluctant to suddenly start giving three-dash
in their output. It will upset what is reading from the pipe downstream.
In an ideal world, I would probably say:
* format-patch should have three-dash after the commit message, no matter
what format the patch is asked for, and it always will give patch text.
* format-patch -p should be reinstated as a way to ask for "just patch
text, no diffstat". Introducing a new option --no-stat _in addition_
to improve the UI is Ok.
* format-patch -U<n> should not be mistaken as a request to suppress
diffstat; what 68daa64 _tried_ to do was worthy.
* Other commands of "log" family that understand -p/-U<n> to produce
patch text should also give three-dash after the log message, and no
three-dash when they don't produce patch text.
^ permalink raw reply
* Re: [PATCH] Require a struct remote in transport_get()
From: Junio C Hamano @ 2009-11-04 5:42 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.2.00.0911032133540.14365@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> cmd_ls_remote() was calling transport_get() with a NULL remote and a
> non-NULL url in the case where it was run outside a git
> repository. This involved a bunch of ill-tested special
> cases. Instead, simply get the struct remote for the URL with
> remote_get(), which works fine outside a git repository, and can also
> take global options into account.
>
> This fixes a tiny and obscure bug where "git ls-remote" without a repo
> didn't support global url.*.insteadOf, even though "git clone" and
> "git ls-remote" in any repo did.
>
> Also, enforce that all callers provide a struct remote to transport_get().
>
> Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> ---
> This is sufficient to stop the segfault when tring "git ls-remote
> http://..." outside of a repo, but not to make it work, which requires
> either something simple but not ideal or something complex.
Thanks; I think this and your other patch are important fixes, and should
go directly on 'maint'. Do you prefer to queue them on 'next' to cook for
a week instead?
^ permalink raw reply
* Re: [PATCH] Allow curl helper to work without a local repository
From: Sverre Rabbelier @ 2009-11-04 5:32 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0911032149390.14365@iabervon.org>
Heya,
On Wed, Nov 4, 2009 at 03:52, Daniel Barkalow <barkalow@iabervon.org> wrote:
> This is the simple change to let remote-curl work without a local
> repository for git ls-remote; it leave the transport-helper code assuming
> that all helpers can list without a local repo, which happens to be true
> of this helper, the only one in current git.
Add a capability for it? :P
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [RFC/PATCH 2/3] revision: change '--bisect' rev machinery argument to 'bisect-refs'
From: Christian Couder @ 2009-11-04 4:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <20091104034312.4545.2176.chriscool@tuxfamily.org>
Using '--bisect' as a revision machinery argument is bogus because
it conflicts with the special '--bisect' argument for "rev-list".
And as shown by a test case added by a previous commit, this cannot
be fixed by adding a special flag to "struct rev_info".
So this commit just renames the '--bisect' flag to the revision
machinery into '--bisect-refs', and reverts the changes that added
the special flag to "struct rev_info".
This makes the test case added previously pass.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 2 --
builtin-rev-parse.c | 4 ++--
revision.c | 5 ++---
revision.h | 1 -
t/t6030-bisect-porcelain.sh | 2 +-
5 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 9e736b4..2ccbfbb 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -320,8 +320,6 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
memset(&info, 0, sizeof(info));
info.revs = &revs;
- if (revs.bisect)
- bisect_list = 1;
quiet = DIFF_OPT_TST(&revs.diffopt, QUICK);
for (i = 1 ; i < argc; i++) {
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 9526aaf..201a458 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -39,7 +39,7 @@ static int is_rev_argument(const char *arg)
{
static const char *rev_args[] = {
"--all",
- "--bisect",
+ "--bisect-refs",
"--dense",
"--branches",
"--header",
@@ -554,7 +554,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
for_each_ref(show_reference, NULL);
continue;
}
- if (!strcmp(arg, "--bisect")) {
+ if (!strcmp(arg, "--bisect-refs")) {
for_each_ref_in("refs/bisect/bad", show_reference, NULL);
for_each_ref_in("refs/bisect/good", anti_reference, NULL);
continue;
diff --git a/revision.c b/revision.c
index 5cedd15..d1a1edc 100644
--- a/revision.c
+++ b/revision.c
@@ -995,7 +995,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
!strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
!strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
!strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk") ||
- !strcmp(arg, "--bisect"))
+ !strcmp(arg, "--bisect-refs"))
{
unkv[(*unkc)++] = arg;
return 1;
@@ -1270,10 +1270,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
handle_refs(revs, flags, for_each_branch_ref);
continue;
}
- if (!strcmp(arg, "--bisect")) {
+ if (!strcmp(arg, "--bisect-refs")) {
handle_refs(revs, flags, for_each_bad_bisect_ref);
handle_refs(revs, flags ^ UNINTERESTING, for_each_good_bisect_ref);
- revs->bisect = 1;
continue;
}
if (!strcmp(arg, "--tags")) {
diff --git a/revision.h b/revision.h
index 921656a..b6421a6 100644
--- a/revision.h
+++ b/revision.h
@@ -63,7 +63,6 @@ struct rev_info {
reverse:1,
reverse_output_stage:1,
cherry_pick:1,
- bisect:1,
first_parent_only:1;
/* Diff flags */
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 88a2877..4e4160e 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -392,7 +392,7 @@ test_expect_success 'bisect does not create a "bisect" branch' '
git branch -D bisect
'
-test_expect_failure 'bisect and "rev-list --bisect"' '
+test_expect_success 'bisect and "rev-list --bisect"' '
rev_list2=$(git rev-list --bisect $HASH3 --not $HASH1) &&
test "$rev_list2" = "$HASH2" &&
rev_list4=$(git rev-list --bisect $HASH7 --not $HASH1) &&
--
1.6.5.1.gaf97d
^ permalink raw reply related
* [RFC/PATCH 1/3] t6030: show "rev-list --bisect" breakage when bisecting
From: Christian Couder @ 2009-11-04 4:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <20091104034312.4545.2176.chriscool@tuxfamily.org>
In the following commits:
ad3f9a7 Add '--bisect' revision machinery argument
81ee52b Merge branch 'lt/revision-bisect' into next
the '--bisect' argument was added to the revision machinery to easily
use the "bad" and "good" refs from the current bisection in any
command that expect some refs.
The problem was that '--bisect' already had a special meaning for
"git rev-list" outside the revision machinery and now it was eaten by
the revision machinery. So a flag named "bisect" was added to
"struct rev_info" in these commits, so that "git rev-list" outside the
revision machinery could see that "--bisect" had been used and operate
as if "--bisect" had been passed to it.
But the above does not fix everything, and this commit adds a test
case to show that.
Now "git rev-list --bisect BAD --not GOOD" behaves differently
depending on whether we are currently bisecting or not. If we are not
currently bisecting, it uses "BAD --not GOOD" as the bisect refs and
if we are bisecting it uses the bisect refs of the current bisection
as the bisect refs. This means that we don't behave like we used to
when we are bisecting and the reafs passed on the command line to
"git rev-list --bisect" are different from the bisect refs of the
current bisection.
The following commit will fix this regression.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t6030-bisect-porcelain.sh | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index def397c..88a2877 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -392,6 +392,19 @@ test_expect_success 'bisect does not create a "bisect" branch' '
git branch -D bisect
'
+test_expect_failure 'bisect and "rev-list --bisect"' '
+ rev_list2=$(git rev-list --bisect $HASH3 --not $HASH1) &&
+ test "$rev_list2" = "$HASH2" &&
+ rev_list4=$(git rev-list --bisect $HASH7 --not $HASH1) &&
+ test "$rev_list4" = "$HASH4" &&
+ git bisect start $HASH7 $HASH1 &&
+ rev_hash4=$(git rev-parse --verify HEAD) &&
+ test "$rev_hash4" = "$HASH4" &&
+ rev_list2=$(git rev-list --bisect $HASH3 --not $HASH1) &&
+ test "$rev_list2" = "$HASH2" &&
+ git bisect reset
+'
+
# This creates a "side" branch to test "siblings" cases.
#
# H1-H2-H3-H4-H5-H6-H7 <--other
--
1.6.5.1.gaf97d
^ permalink raw reply related
* [RFC/PATCH 0/3] use '--bisect-refs' as bisect rev machinery option
From: Christian Couder @ 2009-11-04 4:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
So I suggest to use '--bisect-refs' instead of '--bisect' as the new
bisect revision machinery option, because otherwise I think we get a
regression when we call "git rev-list --bisect BAD --not GOOD" and we
are already bisecting with bisect refs different than BAD and GOOD.
This also simplifies the code a little bit.
I had a look at using '--bisect-refs' in the git bisect helper instead
of collecting the good and bad refs in bisect.c::read_bisect_refs(),
but I gave up because I think we need the good and bad refs anyway for
other purposes like checking that all good refs are ancestor of the bad
ref. So I think we would not gain much if anything there.
If this is ok then the next steps I can do is add some documentation
and tests for the new '--bisect-refs' option.
Christian Couder (3):
t6030: show "rev-list --bisect" breakage when bisecting
revision: change '--bisect' rev machinery argument to 'bisect-refs'
bisect: simplify calling visualizer using '--bisect-refs'
builtin-rev-list.c | 2 --
builtin-rev-parse.c | 4 ++--
git-bisect.sh | 3 +--
revision.c | 5 ++---
revision.h | 1 -
t/t6030-bisect-porcelain.sh | 13 +++++++++++++
6 files changed, 18 insertions(+), 10 deletions(-)
^ permalink raw reply
* [RFC/PATCH 3/3] bisect: simplify calling visualizer using '--bisect-refs'
From: Christian Couder @ 2009-11-04 4:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <20091104034312.4545.2176.chriscool@tuxfamily.org>
It is now shorter and safer to use the new '--bisect-refs' revision
machinery option, than to compute the refs that we must pass.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
git-bisect.sh | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 8b3c585..41b9118 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -300,8 +300,7 @@ bisect_visualize() {
esac
fi
- not=$(git for-each-ref --format='%(refname)' "refs/bisect/good-*")
- eval '"$@"' refs/bisect/bad --not $not -- $(cat "$GIT_DIR/BISECT_NAMES")
+ eval '"$@"' --bisect-refs -- $(cat "$GIT_DIR/BISECT_NAMES")
}
bisect_reset() {
--
1.6.5.1.gaf97d
^ permalink raw reply related
* Re: How to ensure a word has been removed from repository?
From: Sitaram Chamarty @ 2009-11-04 3:35 UTC (permalink / raw)
To: Patrick Higgins; +Cc: git
In-Reply-To: <6fb3af8e0911031812j54a9b698xca9f5301ac07442a@mail.gmail.com>
On Wed, Nov 4, 2009 at 7:42 AM, Patrick Higgins
<patrick.allen.higgins@gmail.com> wrote:
> Given that much of the repository is stored in compressed packs, I
> can't just use grep to look for the words. To get around this, I've
> unpacked the objects, use a Perl script (filtinf example script) to
> decompress them and then use grep (this has proven to be quite slow).
> Is that going to find every possible occurrence if all the relevant
> files are plain text?
> Is there an easier way to search the repository? The way I'm doing it
> has required some awfully deep knowledge to expire and prune
> everything. I feel like I must be missing something.
Instead of expire and prune, I'd clone the repo to some other
location, and search in that clone. You'd still need the commands
that Nicolas gave in his reply but you wouldn't get any false
positivies from missing some reflog or something kept a ref alive.
^ permalink raw reply
* [PATCH v2] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Erick Mattos @ 2009-11-04 3:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erick Mattos
When we use -c, -C, or --amend, we are trying one of two things: using
the source as a template or modifying a commit with corrections.
When these options are used, the authorship and timestamp recorded in
the newly created commit are always taken from the original commit.
This is inconvenient when we just want to borrow the commit log message
or when our change to the code is so significant that we should take
over the authorship (with the blame for bugs we introduce, of course).
The new --reset-author option is meant to solve this need by
regenerating the timestamp and setting as the new author the committer
or the one specified by --author option.
Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---
I have remade the testing script to let it easier for people to understand and
to make it do all reasonable tests.
I have made minor message log changes and as you can see by the third paragraph
I am showing a different approach to option --author. Please read the following
text:
--author text says: "override author for commit".
As I see, something that OVERRIDES supersedes everything else.
IMHO --author shouldn't be blocked by the new option.
Cutting --author away would make impossible for someone to force a new author
with a new timestamp in case he is templating. As an example he can be using
the --author because he is doing a change in a computer not his own or
something alike.
So I would not wipe "author" out from the new option.
Please don't forget that I am just being a small contributor. I am just
suggesting things. You have the final word and if you want we can add your
small test to block it:
if (force_author && renew_authorship)
die("Using both --reset-author and --author does not make sense");
Documentation/git-commit.txt | 7 ++-
builtin-commit.c | 9 ++-
t/t7509-commit.sh | 123 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 135 insertions(+), 4 deletions(-)
create mode 100755 t/t7509-commit.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..f89db9a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C) <commit>] [-F <file> | -m <msg>]
+ [(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
[--cleanup=<mode>] [--] [[-i | -o ]<file>...]
@@ -69,6 +69,11 @@ OPTIONS
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the commit message.
+--reset-author::
+ When used with -C/-c/--amend options, declare that the
+ authorship of the resulting commit now belongs of the committer.
+ This also renews the author timestamp.
+
-F <file>::
--file=<file>::
Take the commit message from the given file. Use '-' to
diff --git a/builtin-commit.c b/builtin-commit.c
index beddf01..6b51a1b 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -51,7 +51,7 @@ static const char *template_file;
static char *edit_message, *use_message;
static char *author_name, *author_email, *author_date;
static int all, edit_flag, also, interactive, only, amend, signoff;
-static int quiet, verbose, no_verify, allow_empty, dry_run;
+static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static char *untracked_files_arg;
/*
* The default commit message cleanup mode will remove the lines
@@ -91,8 +91,9 @@ static struct option builtin_commit_options[] = {
OPT_FILENAME('F', "file", &logfile, "read log from file"),
OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
- OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
+ OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+ OPT_BOOLEAN(0, "reset-author", &renew_authorship, "reset timestamp and authorship to committer"),
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
@@ -381,7 +382,7 @@ static void determine_author_info(void)
email = getenv("GIT_AUTHOR_EMAIL");
date = getenv("GIT_AUTHOR_DATE");
- if (use_message) {
+ if (use_message && !renew_authorship) {
const char *a, *lb, *rb, *eol;
a = strstr(use_message_buffer, "\nauthor ");
@@ -780,6 +781,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message = edit_message;
if (amend && !use_message)
use_message = "HEAD";
+ if (!use_message && renew_authorship)
+ die("Option --reset-author is used only with -C/-c/--amend.");
if (use_message) {
unsigned char sha1[20];
static char utf8[] = "UTF-8";
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
new file mode 100755
index 0000000..1c27de7
--- /dev/null
+++ b/t/t7509-commit.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Erick Mattos
+#
+
+test_description='git commit
+
+Tests for --reset-author option on a commit.'
+
+. ./test-lib.sh
+
+TEST_FILE=foo
+
+author_id () {
+ git cat-file -p "$1" | \
+ grep "^author" | \
+ sed -e "s/author //" -e "s/>.*/>/"
+}
+
+author_timestamp () {
+ git cat-file -p "$1" | \
+ grep "^author" | \
+ sed "s/.*> //"
+}
+
+message_body () {
+ git cat-file commit "$1" | \
+ sed -e '1,/^$/d'
+}
+
+initiate_test () {
+ test_tick
+ echo "initial" >> "$TEST_FILE"
+ git add "$TEST_FILE"
+ git commit -m "Initial Commit" --author "Frigate <flying@over.world>"
+ test_tick
+}
+
+make_files () {
+ author_id "$1" > "aid$2"
+ author_timestamp "$1" > "atime$2"
+ message_body "$1" > "message$2"
+}
+
+get_committer_id () {
+ echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" > aid1
+}
+
+test_expect_success '-C without --reset-author uses the author from the old commit' '
+ initiate_test &&
+ echo "Test 1" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD &&
+ make_files HEAD^ 1 &&
+ make_files HEAD 2 &&
+ test_cmp aid1 aid2 &&
+ test_cmp atime1 atime2 &&
+ test_cmp message1 message2
+'
+
+test_expect_success '-C with --reset-author makes me the author' '
+ test_tick &&
+ echo "Test 2" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD^ --reset-author &&
+ make_files HEAD^ 1 &&
+ make_files HEAD 2 &&
+ get_committer_id &&
+ test_cmp aid1 aid2 &&
+ test_must_fail cmp atime1 atime2 &&
+ test_cmp message1 message2
+'
+
+test_expect_success '-c without --reset-author uses the author from the old commit' '
+ initiate_test &&
+ echo "Test 3" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ EDITOR=: VISUAL=: git commit -c HEAD &&
+ make_files HEAD^ 1 &&
+ make_files HEAD 2 &&
+ test_cmp aid1 aid2 &&
+ test_cmp atime1 atime2 &&
+ test_cmp message1 message2
+'
+
+test_expect_success '-c with --reset-author makes me the author' '
+ test_tick &&
+ echo "Test 4" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ EDITOR=: VISUAL=: git commit -c HEAD^ --reset-author &&
+ make_files HEAD^ 1 &&
+ make_files HEAD 2 &&
+ get_committer_id &&
+ test_cmp aid1 aid2 &&
+ test_must_fail cmp atime1 atime2 &&
+ test_cmp message1 message2
+'
+
+test_expect_success '--amend without --reset-author uses the author from the old commit' '
+ initiate_test &&
+ make_files HEAD 2 &&
+ echo "Test 5" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "--amend test" --amend &&
+ make_files HEAD 1 &&
+ test_cmp aid1 aid2 &&
+ test_cmp atime1 atime2 &&
+ test_must_fail cmp message1 message2
+'
+
+test_expect_success '--amend with --reset-author makes me the author' '
+ test_tick &&
+ echo "Test 6" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Changed" --amend --reset-author &&
+ make_files HEAD 2 &&
+ get_committer_id &&
+ test_cmp aid1 aid2 &&
+ test_must_fail cmp atime1 atime2 &&
+ test_must_fail cmp message1 message2
+'
+
+test_done
--
1.6.5.2.144.g27f8d.dirty
^ permalink raw reply related
* [PATCH] commit: fix too generous RFC-2822 footer handling
From: SZEDER Gábor @ 2009-11-04 3:09 UTC (permalink / raw)
To: Junio C Hamano, David Brown; +Cc: git, SZEDER Gábor
In-Reply-To: <20091103165951.GA2241@neumann>
Since commit c1e01b0c (commit: More generous accepting of RFC-2822
footer lines, 2009-10-28) RFC-2822-looking lines at the end of the
message are considered part of the footer and 'git commit -s -m'
doesn't add a newline between that footer and the new S-O-B line.
This new behaviour causes problems with subject-only commit messages
which happens to look like an RFC-2822 header (e.g. 'git commit -s -m
"subsystem: coolest feature ever"'). In such cases there won't be any
newline between the subject and the S-O-B line, and the S-O-B line
will show up at places where it should not (e.g. in the output of 'git
shortlog').
With this patch the newline will be always added if a commit message
has only a single line, even if it looks like an RFC-2822 header.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
Maybe something like this? Be careful when reviewing, it's 4AM
here...
builtin-commit.c | 8 ++++++++
t/t7501-commit.sh | 4 ++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index beddf01..4971156 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -429,6 +429,14 @@ static int ends_rfc2822_footer(struct strbuf *sb)
hit = (buf[i] == '\n');
}
+ for (j = i-1; j > 0; j--)
+ if (buf[j] == '\n') {
+ hit = 1;
+ break;
+ }
+ if (!hit) /* one-line message */
+ return 0;
+
while (i < len - 1 && buf[i] == '\n')
i++;
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d2de576..aaeedda 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -215,10 +215,10 @@ test_expect_success 'sign off (1)' '
echo 1 >positive &&
git add positive &&
- git commit -s -m "thank you" &&
+ git commit -s -m "subsystem: coolest feature ever" &&
git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
(
- echo thank you
+ echo subsystem: coolest feature ever
echo
git var GIT_COMMITTER_IDENT |
sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
--
1.6.5.2.201.g0f47
^ permalink raw reply related
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Erick Mattos @ 2009-11-04 2:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7v639rnkvt.fsf@alter.siamese.dyndns.org>
2009/11/3 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> ... I had already sent another patch with the
>> suggestions he made in a previous email.
>
> That happens in real life with people working in different timezones.
6 hours between you and me!
>> The new option only touches on getting new author or copying the
>> original so that is why I made the first check in whole and the others
>> only by author. If people think that this operation is so uncertain,
>> then everything should be compared: parent, author and message on all
>> tests.
>
> You probably have misunderstood why we write tests; it is not about making
> sure _your_ implementation is Ok. If that were the case, using knowledge
> of implementation details to short-circuit the tests would perfectly be
> acceptable.
>
> We write tests so that long after you get bored and stop visiting the git
> project mailing-list, if somebody _else_ changes the program and its
> behaviour gets changed in a way _you_ did not expect, such a mistake can
> be caught, even if you are not monitoring the mailing list to actively
> catch such a bad change to go into the system. So we prefer to test both
> sides of the coin without saying "this option only affects this codepath
> (currently) so it never can break this part, it is not worth checking this
> and that (right now)" when it is not too much trouble. It is a win in the
> long run.
I really did not get the reason before the other guy argued... :-S
> In any case, I like --reset-author better than --mine. I didn't think of
> diamond-mine, though ;-)
>
So that's it! Diamond... me neither! :-D
I am going to send you another patch in a few minutes. I hope this
time will be almost there.
Regards
^ permalink raw reply
* [PATCH] Allow curl helper to work without a local repository
From: Daniel Barkalow @ 2009-11-04 2:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
It's okay to use the curl helper without a local repository, so long
as you don't use "fetch". There aren't any git programs that would try
to use it, and it doesn't make sense to try it (since there's nowhere
to write the results), but we may as well be clear.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
This is the simple change to let remote-curl work without a local
repository for git ls-remote; it leave the transport-helper code assuming
that all helpers can list without a local repo, which happens to be true
of this helper, the only one in current git.
remote-curl.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 2faf1c6..ebdab36 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -82,9 +82,10 @@ int main(int argc, const char **argv)
struct strbuf buf = STRBUF_INIT;
const char *url;
struct walker *walker = NULL;
+ int nongit;
git_extract_argv0_path(argv[0]);
- setup_git_directory();
+ setup_git_directory_gently(&nongit);
if (argc < 2) {
fprintf(stderr, "Remote needed\n");
return 1;
@@ -103,6 +104,8 @@ int main(int argc, const char **argv)
break;
if (!prefixcmp(buf.buf, "fetch ")) {
char *obj = buf.buf + strlen("fetch ");
+ if (nongit)
+ die("Fetch attempted without a local repo");
if (!walker)
walker = get_http_walker(url, remote);
walker->get_all = 1;
--
1.6.5.2.142.g063c5.dirty
^ permalink raw reply related
* Re: How to ensure a word has been removed from repository?
From: Nicolas Pitre @ 2009-11-04 2:49 UTC (permalink / raw)
To: Patrick Higgins; +Cc: git
In-Reply-To: <6fb3af8e0911031812j54a9b698xca9f5301ac07442a@mail.gmail.com>
On Tue, 3 Nov 2009, Patrick Higgins wrote:
> Hi all,
>
> I just completed a series of filter-branch commands to remove a couple
> of sensitive words from a repository before I publish it. The words
> were found in commit messages, directory names, file contents, and
> various other places (kind of weird, I know). I believe I have removed
> them all, but I would like to double check but don't know how.
>
> Given that much of the repository is stored in compressed packs, I
> can't just use grep to look for the words. To get around this, I've
> unpacked the objects, use a Perl script (filtinf example script) to
> decompress them and then use grep (this has proven to be quite slow).
>
> Is that going to find every possible occurrence if all the relevant
> files are plain text?
>
> Is there an easier way to search the repository? The way I'm doing it
> has required some awfully deep knowledge to expire and prune
> everything. I feel like I must be missing something.
An easy way to look for the presence of a particular string in all the
repository data is:
git rev-list --all --objects | cut -c -40 | \
git cat-file --batch | grep <string>
Alternatively you can use:
git fast-export --all --signed-tag=verbatim | grep <string>
Nicolas
^ permalink raw reply
* Re: [PATCH 1/1] MSVC: port pthread code to native Windows threads
From: Joshua Jensen @ 2009-11-04 2:34 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.DEB.1.00.0911040031210.4985@pacific.mpi-cbg.de>
----- Original Message -----
From: Johannes Schindelin
Date: 11/3/2009 4:38 PM
>> #ifdef THREADED_DELTA_SEARCH
>> -#include "thread-utils.h"
>> -#include<pthread.h>
>> +# include "thread-utils.h"
>> +# ifndef _WIN32
>> +# include<pthread.h>
>> +# else
>> +# include<winthread.h>
>> +# endif
>> #endif
>>
>>
> It is unlikely that an #ifdef "contamination" of this extent will go
> through easily, but I have a suggestion that may make your patch both
> easier to read and more likely to be accepted into git.git: Try to wrap
> the win32 calls into pthread-compatible function signatures. Then you can
> add a compat/win32/pthread.h and not even touch core files of git.git at
> all.
>
Pardon my ignorance, but is there a reason to not use Pthreads for
Win32? http://sourceware.org/pthreads-win32/
Josh
^ permalink raw reply
* [PATCH] Require a struct remote in transport_get()
From: Daniel Barkalow @ 2009-11-04 2:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
cmd_ls_remote() was calling transport_get() with a NULL remote and a
non-NULL url in the case where it was run outside a git
repository. This involved a bunch of ill-tested special
cases. Instead, simply get the struct remote for the URL with
remote_get(), which works fine outside a git repository, and can also
take global options into account.
This fixes a tiny and obscure bug where "git ls-remote" without a repo
didn't support global url.*.insteadOf, even though "git clone" and
"git ls-remote" in any repo did.
Also, enforce that all callers provide a struct remote to transport_get().
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
This is sufficient to stop the segfault when tring "git ls-remote
http://..." outside of a repo, but not to make it work, which requires
either something simple but not ideal or something complex.
builtin-ls-remote.c | 6 +++---
transport.c | 7 +++++--
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
index 78a88f7..b5bad0c 100644
--- a/builtin-ls-remote.c
+++ b/builtin-ls-remote.c
@@ -86,10 +86,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
pattern[j - i] = p;
}
}
- remote = nongit ? NULL : remote_get(dest);
- if (remote && !remote->url_nr)
+ remote = remote_get(dest);
+ if (!remote->url_nr)
die("remote %s has no configured URL", dest);
- transport = transport_get(remote, remote ? remote->url[0] : dest);
+ transport = transport_get(remote, remote->url[0]);
if (uploadpack != NULL)
transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
diff --git a/transport.c b/transport.c
index 644a30a..298dc46 100644
--- a/transport.c
+++ b/transport.c
@@ -812,6 +812,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
{
struct transport *ret = xcalloc(1, sizeof(*ret));
+ if (!remote)
+ die("No remote provided to transport_get()");
+
ret->remote = remote;
ret->url = url;
@@ -849,10 +852,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
data->thin = 1;
data->conn = NULL;
data->uploadpack = "git-upload-pack";
- if (remote && remote->uploadpack)
+ if (remote->uploadpack)
data->uploadpack = remote->uploadpack;
data->receivepack = "git-receive-pack";
- if (remote && remote->receivepack)
+ if (remote->receivepack)
data->receivepack = remote->receivepack;
}
--
1.6.5.2.142.g063c5.dirty
^ permalink raw reply related
* How to ensure a word has been removed from repository?
From: Patrick Higgins @ 2009-11-04 2:12 UTC (permalink / raw)
To: git
Hi all,
I just completed a series of filter-branch commands to remove a couple
of sensitive words from a repository before I publish it. The words
were found in commit messages, directory names, file contents, and
various other places (kind of weird, I know). I believe I have removed
them all, but I would like to double check but don't know how.
Given that much of the repository is stored in compressed packs, I
can't just use grep to look for the words. To get around this, I've
unpacked the objects, use a Perl script (filtinf example script) to
decompress them and then use grep (this has proven to be quite slow).
Is that going to find every possible occurrence if all the relevant
files are plain text?
Is there an easier way to search the repository? The way I'm doing it
has required some awfully deep knowledge to expire and prune
everything. I feel like I must be missing something.
Thanks,
Patrick
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Sitaram Chamarty @ 2009-11-04 1:58 UTC (permalink / raw)
To: Tim Mazid; +Cc: git
In-Reply-To: <1257295737457-3942366.post@n2.nabble.com>
On Wed, Nov 4, 2009 at 6:18 AM, Tim Mazid <timmazid@hotmail.com> wrote:
> Might be better to include a configuration option to allow this, for those
> that know what they're doing. Most of the people that know what they're
> doing will use the command line, anyway, but it may irritate some people.
I considered that but found my tcl fu was seriously lacking. These
are literally the first 3 lines of tcl I ever wrote in my life, and
this program is one huge 11,000+ line monolith, so I'm naturally
scared to make more than very, very, small changes :)
In any case, as you said, most people who know what they're doing can
use the CLI to get there anyway...
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-04 1:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, Scott Chacon, git list
In-Reply-To: <20091104011802.GE10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> > I don't think we ever send an empty packet. If we have no data to
>> > send, why the hell did we create the packet header?
>>
>> Oh, I do not disagree that it is pointless, but the example that followed
>> the part we are discussing also had "0004". I think it is Ok to allow it.
>
> If its pointless, why encourage it? Why not discourage it with SHOULD NOT?
Oh, no, I didn't mean to _encourage_ it.
I just thought that it being pointless at the semantic level would already
be an enough discouragement for people who are intelligent enough.
As I said, this was not an objection to start with.
> Sure, but can't packet_write just return early without write()
> if format_packet returned 4 (aka vsnprintf returned 0)?
Ah, that's right.
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Junio C Hamano @ 2009-11-04 1:23 UTC (permalink / raw)
To: Erick Mattos; +Cc: Nanako Shiraishi, git
In-Reply-To: <55bacdd30911031551k1bfd3151t940864e4793f5a37@mail.gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
> ... I had already sent another patch with the
> suggestions he made in a previous email.
That happens in real life with people working in different timezones.
> The new option only touches on getting new author or copying the
> original so that is why I made the first check in whole and the others
> only by author. If people think that this operation is so uncertain,
> then everything should be compared: parent, author and message on all
> tests.
You probably have misunderstood why we write tests; it is not about making
sure _your_ implementation is Ok. If that were the case, using knowledge
of implementation details to short-circuit the tests would perfectly be
acceptable.
We write tests so that long after you get bored and stop visiting the git
project mailing-list, if somebody _else_ changes the program and its
behaviour gets changed in a way _you_ did not expect, such a mistake can
be caught, even if you are not monitoring the mailing list to actively
catch such a bad change to go into the system. So we prefer to test both
sides of the coin without saying "this option only affects this codepath
(currently) so it never can break this part, it is not worth checking this
and that (right now)" when it is not too much trouble. It is a win in the
long run.
In any case, I like --reset-author better than --mine. I didn't think of
diamond-mine, though ;-)
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Shawn O. Pearce @ 2009-11-04 1:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Chacon, git list
In-Reply-To: <7vljinnllj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > I don't think we ever send an empty packet. If we have no data to
> > send, why the hell did we create the packet header?
>
> Oh, I do not disagree that it is pointless, but the example that followed
> the part we are discussing also had "0004". I think it is Ok to allow it.
If its pointless, why encourage it? Why not discourage it with SHOULD NOT?
> The original intent of packet_flush() was that everything else could be
> kept buffered in-core without going to write() until packet_flush() is
> called. The protocol was defined in a way that we won't wait for
> listening a response from the other end to an earlier message we "sent"
> with packet_write() but haven't called packet_flush() yet hence could
> still be in our buffer. We still have this comment:
>
> /*
> * If we buffered things up above (we don't, but we should),
> * we'd flush it here
> */
> void packet_flush(int fd)
The smart-http series causes fetch-pack to buffer. :-)
> And once we start buffering, allowing "0004" packet_write() wouldn't even
> be a problem; it can be optimized out in the buffering layer.
Sure, but can't packet_write just return early without write()
if format_packet returned 4 (aka vsnprintf returned 0)?
--
Shawn.
^ permalink raw reply
* Re: Unhappy git in a jailshell?
From: Alex MDC @ 2009-11-04 1:17 UTC (permalink / raw)
To: Michael J Gruber, Dmitry Potapov; +Cc: git
In-Reply-To: <4AEEE3D2.9040905@drmicha.warpmail.net>
Hi guys,
I requested the admins allow access to the `git --exec-path` directory
from the jailshell and incredibly, they granted it! Now everything is
working and "git --help --all" lists all the commands as expected, so
that was indeed the problem.
It was just a bit deceiving at first that some git commands were
working but others weren't, but Dmitry explained that one.
Thanks for your help,
Alex
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author
From: Junio C Hamano @ 2009-11-04 1:12 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Erick Mattos, git
In-Reply-To: <20091104073822.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> I think it is much better to replace "--mine" in gmane/131893
> with "--reset-author" and make no other change to the test.
Heh, I would not claim my tests were perfect, even though I agree with
most of the suggestions and comments, including the parts I did not quote
that are not about the test script.
My first test did not check the message contents, but all the other ones
(except the last one that we expect the command to fail) check output from
both author_header and message_body, so that not just "--mine affects the
authorship" but also "--mine does not mangle the message contents" are
checked.
One thing you did not mention was that I made sure that the command failed
when given both --mine and --author options; I think Erick's last round
fails to test this condition.
Side note. When writing tests for their shiny new toy, people often
forget to test "the other side".
It is just human nature. It is more fun to demonstrate what your new
feature does, than making sure that the new feature does not kick in
when it is not supposed to, nor it does not change what it is not
supposed to change.
Negative tests are not particularly hard to write. The harder part is
to force the habit of writing them on yourself. Right after designing
and implementing a new feature, the list of things it is supposed to
do and when it is supposed to kick in are pretty clear in your mind,
and that is what makes writing positive tests psychologically a lot
easier.
On the other hand, it takes concious effort to list what it is _not_
supposed to do or when it is _not_ supposed to kick in. That is why
people often fail to write negative tests.
I think in addition to the obvious "s/--mine/--reset-author/g"
replacements, we would need this patch on top of mine.
t/t7509-commit.sh | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
index ec13cea..1dad228 100755
--- a/t/t7509-commit.sh
+++ b/t/t7509-commit.sh
@@ -28,6 +28,10 @@ test_expect_success '-C option copies authorship and message' '
git commit -a -C Initial &&
author_header Initial >expect &&
author_header HEAD >actual &&
+ test_cmp expect actual &&
+
+ message_body Initial >expect &&
+ message_body HEAD >actual &&
test_cmp expect actual
'
^ permalink raw reply related
* Re: [PATCH] Update packfile transfer protocol documentation
From: Junio C Hamano @ 2009-11-04 1:07 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Scott Chacon, git list
In-Reply-To: <20091104005614.GD10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
>> > +A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
>> > +is a special case and MUST be handled differently than an empty
>> > +pkt-line ("0004").
>>
>> ...especially that this sentence makes it sound as if it is perfectly
>> normal to send "0004" for "an empty line" (and I've always thought that is
>> Ok), I am quite puzzled by that "SHOULD NOT".
>
> I don't think we ever send an empty packet. If we have no data to
> send, why the hell did we create the packet header?
Oh, I do not disagree that it is pointless, but the example that followed
the part we are discussing also had "0004". I think it is Ok to allow it.
The original intent of packet_flush() was that everything else could be
kept buffered in-core without going to write() until packet_flush() is
called. The protocol was defined in a way that we won't wait for
listening a response from the other end to an earlier message we "sent"
with packet_write() but haven't called packet_flush() yet hence could
still be in our buffer. We still have this comment:
/*
* If we buffered things up above (we don't, but we should),
* we'd flush it here
*/
void packet_flush(int fd)
And once we start buffering, allowing "0004" packet_write() wouldn't even
be a problem; it can be optimized out in the buffering layer.
^ permalink raw reply
* Re: [PATCH] Update packfile transfer protocol documentation
From: Shawn O. Pearce @ 2009-11-04 0:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Scott Chacon, git list
In-Reply-To: <7v4opbp1fa.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> Scott Chacon <schacon@gmail.com> writes:
>
> > diff --git a/Documentation/technical/protocol-capabilities.txt
> > +
> > +The client MUST send only maximum of one of "side-band" and "side-
> > +band-64k". Server MUST favor side-band-64k if client requests both.
>
> Again I think sending both is an error and should be diagnosed as such.
> This is not a three-way handshake where I say "I can handle both", you say
> "I can too", and then I finally pick "then we'll use this one". There is
> no way for the requesting side to tell which one was chosen, and the
> requester who sent both assumed that the other end chose "side-band" and
> allocated only a 1000-byte buffer like older implementation did, the limit
> of buffer will be busted.
I think Scott borrowed the above from me. The last sentance with that
server MUST is my error.
> Fix the last line "Server MUST favor" to "Server MUST diagnose it as an
> error". Also drop "A client should ask for only one of them," near the
> beginning of this section, as it is redundant. I think it is fine to keep
> "A modern client always favors".
Ack, I agree.
> > +include-tag
> > +-----------
> > +
> > +The 'include-tag' capability is about sending tags if we are sending
> > +objects they point to. If we pack an object to the client, and a tag
> > +points exactly at that object, we pack the tag too. In general this
> > +allows a client to get all new tags when it fetches a branch, in a
> > +single network connection.
> > +
> > +Clients MAY always send include-tag, hardcoding it into a request.
>
> "... when the server advertises this capability", no?
This is also my fault. Its rooted in the way the C implementation
of upload-pack parses the want line, it doesn't care if there are
unrecognized capabilities requested by the client. This is a bug
in the C implementation.
I agree with you, and disagree with the original text I wrote.
> > +Clients SHOULD NOT send include-tag if remote.name.tagopt was set to
> > +--no-tags, as the client doesn't want tag data.
> > +
> > +Servers MUST accept include-tag without error or warning, even if the
> > +server does not understand or support the option.
>
> Why is this special case here?
Ack, I agree with you, this should be removd.
> > +Servers SHOULD pack the tags if their referrant is packed and the
> > +client has requested include-tag.
>
> Sorry, I do not understand the motivation to make make this so weak? If
> the server claims to support this capability, and when a referrant is
> going to the client, the server MUST do so---if it cannot guarantee, why
> claim to support that capability?
>
> Or am I missing something?
IIRC at one time the C implementation didn't fully ensure the tag is
packed when the referrant is packed. This SHOULD exists because it
may have been possible for a tag to be omitted. But I don't think
I've seen this happen under any condition, and it probably is now
a bug if it occurs, which means we likely can convert this to a MUST.
> > diff --git a/Documentation/technical/protocol-common.txt
> > +...
> > +pkt-line Format
> > +---------------
> > +
> > +Implementations SHOULD NOT send an empty pkt-line ("0004").
>
> Not an objection, but where is this coming from?
Me. I think sending "0004" is stupid.
"0004" must immediately be followed by another pkt-len because there
is no data payload behind it. "0004" is the same as having called
write(fd, buf, 0), which is equally pointless. Such a kernel call
will be a no-op, my point here is that "0004" as a packet is also
stupid and shouldn't be sent.
> > +A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
> > +is a special case and MUST be handled differently than an empty
> > +pkt-line ("0004").
>
> ...especially that this sentence makes it sound as if it is perfectly
> normal to send "0004" for "an empty line" (and I've always thought that is
> Ok), I am quite puzzled by that "SHOULD NOT".
I don't think we ever send an empty packet. If we have no data to
send, why the hell did we create the packet header?
--
Shawn.
^ permalink raw reply
* Re: [PATCH] gitk: disable checkout of remote branch
From: Tim Mazid @ 2009-11-04 0:48 UTC (permalink / raw)
To: git
In-Reply-To: <fabb9a1e0911030807h6b76b661pef75628a1255356@mail.gmail.com>
Sverre Rabbelier-2 wrote:
>
> On Tue, Nov 3, 2009 at 17:00, Sitaram Chamarty <sitaramc@gmail.com> wrote:
>> At the command line, this gives you a detailed warning message, but the
>> GUI currently allows it without any fuss.
>
> This is even better than an annoying popup dialog, as we all know
> those are just ignored anyway :).
>
Might be better to include a configuration option to allow this, for those
that know what they're doing. Most of the people that know what they're
doing will use the command line, anyway, but it may irritate some people.
--
View this message in context: http://n2.nabble.com/PATCH-gitk-disable-checkout-of-remote-branch-tp3939363p3942366.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox