* [PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
By extracting the code for updating the HEAD and ORIG_HEAD symbolic
references to a separate function, we declutter cmd_reset() a bit and
we make it clear that e.g. the four variables {,sha1_}{,old_}orig are
only used by this code.
---
builtin/reset.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 68be05c..4d556e7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -239,16 +239,35 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c
return *argv ? get_pathspec(prefix, argv) : NULL;
}
+static int update_refs(const char *rev, const unsigned char *sha1) {
+ int update_ref_status;
+ struct strbuf msg = STRBUF_INIT;
+ unsigned char *orig = NULL, sha1_orig[20],
+ *old_orig = NULL, sha1_old_orig[20];
+
+ if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+ old_orig = sha1_old_orig;
+ if (!get_sha1("HEAD", sha1_orig)) {
+ orig = sha1_orig;
+ set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+ update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+ }
+ else if (old_orig)
+ delete_ref("ORIG_HEAD", old_orig, 0);
+ set_reflog_message(&msg, "updating HEAD", rev);
+ update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+ strbuf_release(&msg);
+ return update_ref_status;
+}
+
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
int patch_mode = 0;
const char *rev;
- unsigned char sha1[20], *orig = NULL, sha1_orig[20],
- *old_orig = NULL, sha1_old_orig[20];
+ unsigned char sha1[20];
const char **pathspec = NULL;
struct commit *commit;
- struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
OPT__QUIET(&quiet, N_("be quiet, only report errors")),
OPT_SET_INT(0, "mixed", &reset_type,
@@ -332,17 +351,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
/* Any resets update HEAD to the head being switched to,
* saving the previous head in ORIG_HEAD before. */
- if (!get_sha1("ORIG_HEAD", sha1_old_orig))
- old_orig = sha1_old_orig;
- if (!get_sha1("HEAD", sha1_orig)) {
- orig = sha1_orig;
- set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
- update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
- }
- else if (old_orig)
- delete_ref("ORIG_HEAD", old_orig, 0);
- set_reflog_message(&msg, "updating HEAD", rev);
- update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
+ update_ref_status = update_refs(rev, sha1);
switch (reset_type) {
case HARD:
@@ -359,7 +368,5 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
remove_branch_state();
- strbuf_release(&msg);
-
return update_ref_status;
}
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* [PATCH 06/19] reset.c: remove unnecessary variable 'i'
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
Throughout most of parse_args(), the variable 'i' remains at 0. In the
remaining few cases, we can do pointer arithmentic on argv itself
instead.
---
This is clearly mostly a matter of taste. The remainder of the series
does not depend on it in any way.
builtin/reset.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 9473725..68be05c 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -199,7 +199,6 @@ static void die_if_unmerged_cache(int reset_type)
}
const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) {
- int i = 0;
const char *rev = "HEAD";
unsigned char unused[20];
/*
@@ -210,34 +209,34 @@ const char **parse_args(int argc, const char **argv, const char *prefix, const c
* git reset [-opts] -- <paths>...
* git reset [-opts] <paths>...
*
- * At this point, argv[i] points immediately after [-opts].
+ * At this point, argv points immediately after [-opts].
*/
- if (i < argc) {
- if (!strcmp(argv[i], "--")) {
- i++; /* reset to HEAD, possibly with paths */
- } else if (i + 1 < argc && !strcmp(argv[i+1], "--")) {
- rev = argv[i];
- i += 2;
+ if (argc) {
+ if (!strcmp(argv[0], "--")) {
+ argv++; /* reset to HEAD, possibly with paths */
+ } else if (argc > 1 && !strcmp(argv[1], "--")) {
+ rev = argv[0];
+ argv += 2;
}
/*
- * Otherwise, argv[i] could be either <rev> or <paths> and
+ * Otherwise, argv[0] could be either <rev> or <paths> and
* has to be unambiguous.
*/
- else if (!get_sha1_committish(argv[i], unused)) {
+ else if (!get_sha1_committish(argv[0], unused)) {
/*
- * Ok, argv[i] looks like a rev; it should not
+ * Ok, argv[0] looks like a rev; it should not
* be a filename.
*/
- verify_non_filename(prefix, argv[i]);
- rev = argv[i++];
+ verify_non_filename(prefix, argv[0]);
+ rev = *argv++;
} else {
/* Otherwise we treat this as a filename */
- verify_filename(prefix, argv[i], 1);
+ verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
- return i < argc ? get_pathspec(prefix, argv + i) : NULL;
+ return *argv ? get_pathspec(prefix, argv) : NULL;
}
int cmd_reset(int argc, const char **argv, const char *prefix)
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
We use the path arguments in two places in reset.c: in
interactive_reset() and read_from_tree(). Both of these call
get_pathspec(), so we pass the (prefix, arv) pair to both
functions. Move the call to get_pathspec() out of these methods, for
two reasons: 1) One argument is simpler than two. 2) It lets us use
the (arguably clearer) "if (pathspec)" in place of "if (i < argc)".
---
If I understand correctly, this should be rebased on top of
nd/parse-pathspec. Please let me know.
builtin/reset.c | 27 ++++++++++-----------------
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 65413d0..045c960 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q,
}
}
-static int interactive_reset(const char *revision, const char **argv,
- const char *prefix)
-{
- const char **pathspec = NULL;
-
- if (*argv)
- pathspec = get_pathspec(prefix, argv);
-
- return run_add_interactive(revision, "--patch=reset", pathspec);
-}
-
-static int read_from_tree(const char *prefix, const char **argv,
- unsigned char *tree_sha1, int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
+ int refresh_flags)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
- diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
+ diff_tree_setup_paths(pathspec, &opt);
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
@@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
const char *rev = "HEAD";
unsigned char sha1[20], *orig = NULL, sha1_orig[20],
*old_orig = NULL, sha1_old_orig[20];
+ const char **pathspec = NULL;
struct commit *commit;
struct strbuf msg = STRBUF_INIT;
const struct option options[] = {
@@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not parse object '%s'."), rev);
hashcpy(sha1, commit->object.sha1);
+ if (i < argc)
+ pathspec = get_pathspec(prefix, argv + i);
+
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with --{hard,mixed,soft}"));
- return interactive_reset(rev, argv + i, prefix);
+ return run_add_interactive(rev, "--patch=reset", pathspec);
}
/* git reset tree [--] paths... can be used to
* load chosen paths from the tree into the index without
* affecting the working tree nor HEAD. */
- if (i < argc) {
+ if (pathspec) {
if (reset_type == MIXED)
warning(_("--mixed with paths is deprecated; use 'git reset -- <paths>' instead."));
else if (reset_type != NONE)
die(_("Cannot do %s reset with paths."),
_(reset_type_names[reset_type]));
- return read_from_tree(prefix, argv + i, sha1,
+ return read_from_tree(pathspec, sha1,
quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
}
if (reset_type == NONE)
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* [PATCH 11/19] reset: avoid redundant error message
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
If writing or committing the new index file fails, we print "Could not
write new index file." followed by "Could not reset index file to
revision $rev.". The first message seems to imply the second, so print
only the first message.
---
builtin/reset.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8e5d097..54e3c5b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -337,13 +337,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
int err = reset_index(sha1, reset_type, quiet);
if (reset_type == KEEP && !err)
err = reset_index(sha1, MIXED, quiet);
- if (!err &&
- (write_cache(newfd, active_cache, active_nr) ||
- commit_locked_index(lock))) {
- err = error(_("Could not write new index file."));
- }
if (err)
die(_("Could not reset index file to revision '%s'."), rev);
+ if (write_cache(newfd, active_cache, active_nr) ||
+ commit_locked_index(lock))
+ die(_("Could not write new index file."));
}
/* Any resets update HEAD to the head being switched to,
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* [PATCH 18/19] reset: allow reset on unborn branch
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
Some users seem to think, knowingly or not, that being on an unborn
branch is like having a commit with an empty tree checked out, but
when run on an unborn branch, "git reset" currently fails with:
fatal: Failed to resolve 'HEAD' as a valid ref.
Instead of making users figure out that they should run
git rm --cached -r .
, let's teach "git reset" without a revision argument, when on an
unborn branch, to behave as if the user asked to reset to an empty
tree. Don't take the analogy with an empty commit too far, though, but
still disallow explictly referring to HEAD in "git reset HEAD".
---
builtin/reset.c | 17 ++++++++------
t/t7106-reset-unborn-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 62 insertions(+), 7 deletions(-)
create mode 100755 t/t7106-reset-unborn-branch.sh
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c223bd..5cd48ac 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -239,7 +239,7 @@ static int update_refs(const char *rev, const unsigned char *sha1) {
int cmd_reset(int argc, const char **argv, const char *prefix)
{
int reset_type = NONE, update_ref_status = 0, quiet = 0;
- int patch_mode = 0;
+ int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
const char **pathspec = NULL;
@@ -264,8 +264,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, git_reset_usage,
PARSE_OPT_KEEP_DASHDASH);
pathspec = parse_args(argc, argv, prefix, &rev);
-
- if (!pathspec) {
+ unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1);
+ if (unborn) {
+ /* reset on unborn branch: treat as reset to empty tree */
+ hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+ } else if (!pathspec) {
if (get_sha1_committish(rev, sha1))
die(_("Failed to resolve '%s' as a valid revision."), rev);
commit = lookup_commit_reference(sha1);
@@ -285,7 +288,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (patch_mode) {
if (reset_type != NONE)
die(_("--patch is incompatible with --{hard,mixed,soft}"));
- return run_add_interactive(rev, "--patch=reset", pathspec);
+ return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", pathspec);
}
/* git reset tree [--] paths... can be used to
@@ -337,16 +340,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("Could not write new index file."));
}
- if (!pathspec) {
+ if (!pathspec && !unborn) {
/* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
update_ref_status = update_refs(rev, sha1);
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
-
- remove_branch_state();
}
+ if (!pathspec)
+ remove_branch_state();
return update_ref_status;
}
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 0000000..4ff6af4
--- /dev/null
+++ b/t/t7106-reset-unborn-branch.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git reset should work on unborn branch'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo a >a &&
+ echo b >b
+'
+
+test_expect_success 'reset' '
+ git add a b &&
+ git reset &&
+ test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset HEAD' '
+ rm .git/index &&
+ git add a b &&
+ test_must_fail git reset HEAD
+'
+
+test_expect_success 'reset $file' '
+ rm .git/index &&
+ git add a b &&
+ git reset a &&
+ test "$(git ls-files)" == "b"
+'
+
+test_expect_success 'reset -p' '
+ rm .git/index &&
+ git add a &&
+ echo y | git reset -p &&
+ test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset --soft is a no-op' '
+ rm .git/index &&
+ git add a &&
+ git reset --soft
+ test "$(git ls-files)" == "a"
+'
+
+test_expect_success 'reset --hard' '
+ rm .git/index &&
+ git add a &&
+ git reset --hard &&
+ test "$(git ls-files)" == "" &&
+ test_path_is_missing a
+'
+
+test_done
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* [PATCH 12/19] reset.c: move update_index_refresh() call out of read_from_tree()
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
The final part of cmd_reset() essentially looks like:
if (pathspec) {
...
read_from_tree(...);
} else {
...
reset_index(...);
update_index_refresh(...);
...
}
where read_from_tree() internally also calls
update_index_refresh(). Move the call to update_index_refresh() out of
read_from_tree for symmetry with the 'else' block, making
read_from_tree() and reset_index() closer in functionality.
---
builtin/reset.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 54e3c5b..a21ba31 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -145,11 +145,8 @@ static void update_index_from_diff(struct diff_queue_struct *q,
}
}
-static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
- int refresh_flags)
+static int read_from_tree(const char **pathspec, unsigned char *tree_sha1)
{
- struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int index_fd;
struct diff_options opt;
memset(&opt, 0, sizeof(opt));
@@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
opt.output_format = DIFF_FORMAT_CALLBACK;
opt.format_callback = update_index_from_diff;
- index_fd = hold_locked_index(lock, 1);
read_cache();
if (do_diff_cache(tree_sha1, &opt))
return 1;
@@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1,
diff_flush(&opt);
diff_tree_release_paths(&opt);
- return update_index_refresh(index_fd, lock, refresh_flags);
+ return 0;
}
static void set_reflog_message(struct strbuf *sb, const char *action,
@@ -321,9 +317,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
die(_("%s reset is not allowed in a bare repository"),
_(reset_type_names[reset_type]));
- if (pathspec)
- return read_from_tree(pathspec, sha1,
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ if (pathspec) {
+ struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+ int index_fd = hold_locked_index(lock, 1);
+ return read_from_tree(pathspec, sha1) ||
+ update_index_refresh(index_fd, lock,
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+ }
/* Soft reset does not touch the index file nor the working tree
* at all, but requires them in a good order. Other resets reset
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* Re: [PATCH] commit: make default of "cleanup" option configurable
From: Jonathan Nieder @ 2013-01-09 8:28 UTC (permalink / raw)
To: Ralf Thielow; +Cc: git, gitster
In-Reply-To: <CAN0XMO+t2gu9UKJFVXAxt91-hUUhMqqmMoop88KYp0vo3x6c_g@mail.gmail.com>
Ralf Thielow wrote:
> It's actually my own usecase :). The bugtracker I'm using is able
> to create relationships between issues and related commits. It
> expects that a part of the commit message contains the issue number
> in format "#<issueId>". So I need to use a cleanup mode different
> from "default" to keep the commentary. The mode I'd use is "whitespace",
> "verbatim" is also possible.
Hm, so "whitespace-when-editing" would be the ideal setting.
Would it be confusing if the '[commit] cleanup' setting only took
effect when launching an editor (and not with -F, -C, or -m)? My
first impression is that I'd like that behavior better, even though
it's harder to explain.
[...]
> When a user uses a script/importer which expects that the "default" option
> is used without setting it explicitly, and then the user changes the default,
> isn't it the users fault if that would break things?
Consider the following series of events.
1. My friend writes an importer that uses the "git commit" command.
I like it and start using it.
2. Another friend writes a blog post about the '[commit] cleanup'
setting. I like it and start using it.
3. I try to use the importer again.
4. Years later, I notice the commit messages are corrupted in the
imported history.
It's hard to assign blame. I guess it's my fault. ;)
[...]
> I'll add a sentence of my bugtracker example to it. I think many developers
> are using such a tool, so it'd makes sense.
Thanks, sounds good.
Regards,
Jonathan
^ permalink raw reply
* Re: Proposal for git stash rename
From: Michael Haggerty @ 2013-01-09 8:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Micheil Smith, git
In-Reply-To: <7vbod4tynt.fsf@alter.siamese.dyndns.org>
On 01/04/2013 10:40 PM, Junio C Hamano wrote:
> Micheil Smith <micheil@brandedcode.com> writes:
>
>>> This patch implements a "git stash rename" using a new
>>> "git reflog update" command that updates the message associated
>>> with a reflog entry.
>> ...
>> I note that this proposal is now two years old. A work in progress patch was
>> requested, however, after one was given this thread ended. I'm also finding
>> a need for this feature;
>
> The whole point of reflog is that it is a mechanism to let users to
> go safely back to the previous state, by using a file that is pretty
> much append-only. It feels that a mechanism to "rewrite" one goes
> completely against that principle, at least to me.
The implementation of "git stash" itself seems to violate your
principle, by storing its branches-that-are-not-branches within a
mutable reflog.
Just an observation...
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH 03/19] reset.c: pass pathspec around instead of (prefix, argv) pair
From: Matt Kraai @ 2013-01-09 11:42 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1357719376-16406-4-git-send-email-martinvonz@gmail.com>
On Wed, Jan 09, 2013 at 12:16:00AM -0800, Martin von Zweigbergk wrote:
> We use the path arguments in two places in reset.c: in
> interactive_reset() and read_from_tree(). Both of these call
> get_pathspec(), so we pass the (prefix, arv) pair to both
^^^
argv is misspelled.
--
Matt
^ permalink raw reply
* Re: [PATCH 07/19] reset.c: extract function for updating {ORIG,}HEAD
From: Matt Kraai @ 2013-01-09 11:54 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1357719376-16406-8-git-send-email-martinvonz@gmail.com>
In the summary, {ORIG,} should be {ORIG_,}.
--
Matt
^ permalink raw reply
* Failure and unhelpful error message from 'rebase --preserve-merges'
From: Phil Hord @ 2013-01-09 13:19 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Neil Horman, Martin von Zweigbergk
In-Reply-To: <50ED63CB.7060108@cisco.com>
Since 90e1818f9a (git-rebase: add keep_empty flag, 2012-04-20)
'git rebase --preserve-merges' fails in a case where it used to
succeed, and it does so with an unhelpful error message.
$ git rebase --preserve-merges master
error: Commit 452524... is a merge but no -m option was given.
fatal: cherry-pick failed
Could not pick 452524f925aecd0439ae5728fca3887292114dd7
I also tried rebase with '-m'
$ git rebase --preserve-merges -m master
but that also failed.
The same commands worked fine for these same commits in v1.7.9
>From 90e1818f9a I figured out that the rebase-interactive
machinery had dropped one of my merges. I normally would not
notice this when using 'git rebase -p' since it does not invoke $EDITOR
by default; but I can see it if I use this:
git -c sequence.editor=cat rebase -p master
With that I see my list of commits, including these:
...
pick 184ec4d WIP: DHCP datastore reporting
# pick 16ca56c Merge ptss into sock-threads
pick 06aea55 WIP: More work normalizing config handlers
...
pick 452524f Merge branch 'ptss' into sock-threads
...
#
# Note that empty commits are commented out
The failure points to the 2nd merge commit, but it is not the merge
commit which was commented out. It is a later merge between the same two
branches. I'm not sure how this is related, yet.
But I now know I can work around the problem with this:
git rebase --keep-empty -p master
I see three problems here, but I don't have any time to go fix them
myself right now.
1. 'rebase -p' should default to --keep-empty since the user will not
be given the opportunity to edit the list to uncomment the missing
commits.
2. 'rebase --interactive -p' should not drop empty merge commits.
3. rebase should not die with a cryptic cherry-pick error message,
although I am not sure what useful thing it could say in this
particular case. Maybe there are other conditions which will cause
this same failure even if 1 and 2 are fixed.
Phil
^ permalink raw reply
* Re: On --depth=funny value
From: Duy Nguyen @ 2013-01-09 14:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, Jonathan Nieder, schlotter, Ralf.Wildenhues, git
In-Reply-To: <7vr4lv7x2u.fsf@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 12:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> * We would like to update "clone --depth=1" to end up with a tip
>>> only repository, but let's not to touch "git fetch" (and "git
>>> clone") and make them send 0 over the wire when the command line
>>> tells them to use "--depth=1" (i.e. let's not do the "off-by-one"
>>> thing).
>>
>> You can't anyway. Depth 0 on the wire is considered invalid by upload-pack.
>
> Yes, that is a good point that we say "if (0 < opt->depth) do the
> shallow thing" everywhere, so 0 is spcial in that sense.
>
> Which suggests that if we wanted to, we could update the fetch side
> to do the off-by-one thing against the current upload-pack when the
> given depth is two or more, and still send 1 when depth=1. When
> talking with an updated upload-pack that advertises exact-shallow
> protocol extension, it can disable that off-by-one for all values of
> depth. That way, the updated client gets history of wrong depth
> only for --depth=1 when talking with the current upload-pack; all
> other cases, it will get history of correct depth.
>
> Hmm?
I haven't checked because frankly I have never run JGit, but are we
sure this off-by-one thing applies to JGit server as well? So far I'm
only aware of three sever implementations: C Git, JGit and Dulwich.
The last one does not support shallow extension so it's out of
question.
--
Duy
^ permalink raw reply
* Re: On --depth=funny value
From: Duy Nguyen @ 2013-01-09 14:59 UTC (permalink / raw)
To: Stefan Beller
Cc: Jonathan Nieder, schlotter, Ralf.Wildenhues, git, Junio C Hamano
In-Reply-To: <7vy5g383sy.fsf_-_@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * Make "git fetch" and "git clone" die() when zero or negative
> number is given with --depth=$N, for the following reasons:
> ...
For Stefan when you update the patch. If "git fetch --depth=0" is
considered invalid too as Junio outlined, then the proper place for
the check is transport.c:set_git_option(), not clone.c. It already
catches --depth=random-string. Adding "depth < 1" check should be
trivial. You may want to update builtin/fetch-pack.c too because it
does not share this code.
--
Duy
^ permalink raw reply
* Re: Fwd: git-gui / Warning: "No newline at end of file”
From: Pat Thoyts @ 2013-01-09 14:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Tobias Preuss
In-Reply-To: <7vzk0qw8ts.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>Tobias Preuss <tobias.preuss@googlemail.com> writes:
>
>> Hello. I never got a response. Did my email pass the distribution
>> list? Best, Tobias
>
>Pat?
>
I did have a brief look at this but I don't have a solution at the
moment. The "\ No newline at end of file" is emitted by git at we don't
appear to handle it well in the lib/diff.tcl apply_range_or_line
function.
>> ---------- Forwarded message ----------
>> From: Tobias Preuss <tobias.preuss@googlemail.com>
>> Date: Tue, Nov 13, 2012 at 9:26 PM
>> Subject: git-gui / Warning: "No newline at end of file”
>> To: git <git@vger.kernel.org>
>>
>>
>> Hello.
>> I noticed a problem when working with git-gui which might be a bug.
>> The issue only affects you when you are visually trying to stage
>> changes line by line. Here are the steps to reproduce the problem:
>>
>> 1. Initialize a new repository.
>> 2. Create a file with three lines of content each with the word
>> "Hello". Do not put a new line at the end of the file.
>> 3. Add and commit the file.
>> 4. Edit the same file putting words inbetween the three lines.
>> 5. Open git-gui and try to stage the changes line by line.
>>
>> The editor will append the warning "No newline at end of file” to the
>> end of the diff. When you are trying to stage a line an error occurs.
>> The problem is also illustrated in a question on Stackoverflow [1].
>>
>> Please let me know if you need more information or if I should send
>> this problem to some other mailing list.
>> Thank you, Tobias
>>
>> ____________
>> [1] http://stackoverflow.com/questions/13223868/how-to-stage-line-by-line-in-git-gui-although-no-newline-at-end-of-file-warnin
>
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* Re: [PATCH] commit: make default of "cleanup" option configurable
From: Junio C Hamano @ 2013-01-09 15:40 UTC (permalink / raw)
To: Ralf Thielow; +Cc: Jonathan Nieder, git
In-Reply-To: <CAN0XMO+t2gu9UKJFVXAxt91-hUUhMqqmMoop88KYp0vo3x6c_g@mail.gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> When a user uses a script/importer which expects that the "default" option
> is used without setting it explicitly, and then the user changes the default,
> isn't it the users fault if that would break things?
Not necessarily. There are many people who use scripts written by
others, without knowing how they work. You could blame that the
script is broken, but not the poor user of that script.
^ permalink raw reply
* git branch case insensitivity (Possible bug)
From: Alexander Gallego @ 2013-01-09 15:46 UTC (permalink / raw)
To: git
Hello,
Here is a pastebin where I've reproduced the steps on a clean git repo.
http://pastebin.com/0vQZEat0
Brief description of the problem:
1.Basically one creates a local branch call it 'imp_fix' (branch off
master --> this doesn't matter)
2.One does work, commit, etc
3.One rebases imp_fix with master via: (inside imp_fix) git rebase master
4.One checks out master via: git checkout master
5.One merges an incorrect name "imp_Fix" (notice the capital F)
6.The expected output is that git would say, silly you --> that branch
does not exist.
7. Instead it merges (what I think is incorrectly) imp_fix.
Kindly let me know if I can provide more details.
For your convenience here is the paste:
agallego@agallego-macpro.local] /tmp
$ git clone git@bitbucket.org/agallego/gitbug
Cloning into 'gitbug'...
warning: You appear to have cloned an empty repository.
[agallego@agallego-macpro.local] /tmp
$ cd gitbug
[agallego@agallego-macpro.local] /tmp/gitbug
$ ls
[agallego@agallego-macpro.local] /tmp/gitbug
$ echo "Trying to reproduce a bug" > README
[agallego@agallego-macpro.local] /tmp/gitbug
$ ls
README
[agallego@agallego-macpro.local] /tmp/gitbug
$ git add .
[agallego@agallego-macpro.local] /tmp/gitbug
$ git commit -am "adding readme"
[master (root-commit) 0bfd62a] adding readme
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 README
[agallego@agallego-macpro.local] /tmp/gitbug
$ git push origin master
Counting objects: 3, done.
Writing objects: 100% (3/3), 230 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
remote: bb/acl: agallego is allowed. accepted payload.
To git@bitbucket.org:agallego/gitbug
* [new branch] master -> master
[agallego@agallego-macpro.local] /tmp/gitbug
$ git checkout imp_fix
error: pathspec 'imp_fix' did not match any file(s) known to git.
[agallego@agallego-macpro.local] /tmp/gitbug
$ git branch imp_fix
[agallego@agallego-macpro.local] /tmp/gitbug
$ git checkout imp_fix
Switched to branch 'imp_fix'
[agallego@agallego-macpro.local] /tmp/gitbug
$ echo "imp_fix" >> README
[agallego@agallego-macpro.local] /tmp/gitbug
$ git commit -am "step 2, create an imp_fix branch and then merge"
[imp_fix 178c8f3] step 2, create an imp_fix branch and then merge
1 files changed, 1 insertions(+), 0 deletions(-)
[agallego@agallego-macpro.local] /tmp/gitbug
$ ls
README
[agallego@agallego-macpro.local] /tmp/gitbug
$ git rebase master
Current branch imp_fix is up to date.
[agallego@agallego-macpro.local] /tmp/gitbug
$ git checkout master
Switched to branch 'master'
[agallego@agallego-macpro.local] /tmp/gitbug
$ git merge imp_Fix
Merge made by the 'recursive' strategy.
README | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
[agallego@agallego-macpro.local] /tmp/gitbug
$ git push origin master
Counting objects: 6, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (4/4), 392 bytes, done.
Total 4 (delta 1), reused 0 (delta 0)
remote: bb/acl: agallego is allowed. accepted payload.
To git@bitbucket.org:agallego/gitbug
0bfd62a..f99f8a1 master -> master
Sincerely,
Alexander Gallego
---*---
------*
* * *
^ permalink raw reply
* Re: git branch case insensitivity (Possible bug)
From: Andreas Ericsson @ 2013-01-09 15:52 UTC (permalink / raw)
To: Alexander Gallego; +Cc: git
In-Reply-To: <CAL+iW28LdnNiho4KksLX_S_-+bKX+77GPJ0zqQfkz4JpBJRskw@mail.gmail.com>
On 01/09/2013 04:46 PM, Alexander Gallego wrote:
> Hello,
>
> Here is a pastebin where I've reproduced the steps on a clean git repo.
>
> http://pastebin.com/0vQZEat0
>
>
>
> Brief description of the problem:
>
>
>
> 1.Basically one creates a local branch call it 'imp_fix' (branch off
> master --> this doesn't matter)
> 2.One does work, commit, etc
> 3.One rebases imp_fix with master via: (inside imp_fix) git rebase master
> 4.One checks out master via: git checkout master
> 5.One merges an incorrect name "imp_Fix" (notice the capital F)
> 6.The expected output is that git would say, silly you --> that branch
> does not exist.
> 7. Instead it merges (what I think is incorrectly) imp_fix.
>
>
> Kindly let me know if I can provide more details.
>
Are you using Mac OSX?
Are you using the HFS+ filesystem shipped with it?
Did you use the filesystem's default settings rather than reinstall your
system with sensible settings?
If you said "yes" to all of the above, this is a filesystem "feature",
courtesy of (cr)Apple, and you're screwed.
You can work around it by running "git pack-refs" every time you create
a branch or a tag though.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
^ permalink raw reply
* Re: [PATCH] commit: make default of "cleanup" option configurable
From: Junio C Hamano @ 2013-01-09 15:56 UTC (permalink / raw)
To: Ralf Thielow; +Cc: Jonathan Nieder, git
In-Reply-To: <CAN0XMO+t2gu9UKJFVXAxt91-hUUhMqqmMoop88KYp0vo3x6c_g@mail.gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> It's actually my own usecase :). The bugtracker I'm using is able
> to create relationships between issues and related commits. It
> expects that a part of the commit message contains the issue number
> in format "#<issueId>". So I need to use a cleanup mode different
> from "default" to keep the commentary.
This is orthogonal to the patch in question, but in your particular
case, I wonder if it is a workable solution to indent #<bugid> in
your message, which won't be stripped away.
I also wonder, as a longer term alternative (which would require a
lot of code auditing and some refactoring), if it is useful to have
an option and/or configuration that lets you configure the "comment
in log message editor" character from the default "#" to something
else. "git -c log.commentchar=% commit" may start the log message
editor with something like this in it:
% Please enter the commit message for your changes. Lines starting
% with '%' will be ignored, and an empty message aborts the commit.
Naturally, setting log.commentchar to "none" would disable stripping
of any commented lines if such a feature existed, and stop issuing
these helpful comments altogether, but still strip excess blank
lines and trailing whitespaces.
I wouldn't seriously suggest it as I am not sure if the usefulness
of such a feature would outweigh the cost of coding it, though; at
least not at this point yet.
^ permalink raw reply
* git interaction between push and auto-gc
From: Bob Lavey @ 2013-01-09 15:56 UTC (permalink / raw)
To: git
Greetings,
We are seeing some unexpected behavior with git that we'd like to better
understand. We are running git 1.7.11.1 on the remote repo server
(CentOS 6.3-x86) and the clients (mostly Windows 7), and we are running
gitolite 3.04 on the server.
At times, our repos can get many unreachable loose objects, and if we
don't clean them up, we will see instances where a push to the remote
repo does not work correctly. This almost always occurs when we try to
delete a remote branch with a "git push origin :branch" call, but this
week we saw two occurrences where a push failed to create a branch (as
well as about a dozen cases where a push failed to delete the branch).
This behavior does not occur for every push, however: I'd guess 10% of
pushes fail.
Here is output from a user's machine:
d:\git\jedi\jedifw>git push origin
configMgrTestBuilder:refs/23s/iq/qbar/user.name@hp.com/jit20073
Counting objects: 59, done.
Delta compression using up to 16 threads.
Compressing objects: 100% (29/29), done.
Writing objects: 100% (30/30), 129.69 KiB, done.
Total 30 (delta 24), reused 1 (delta 0)
Auto packing the repository for optimum performance.
warning: There are too many unreachable loose objects; run 'git prune'
to remove them.
To git@git.boi.hp.com:/jedi/jedifw.git
After a couple of hours, that user was concerned that their branch
hadn't started processing, so they sent an email to support and the
re-pushed. Here are the lines from the gitolite log showing both of
those pushes:
gitolite-2013-01-07.log:2013-01-07.15:57:58 23285 update
jedi/jedifw user.name@hp.com W
refs/23s/iq/qbar/user.name@hp.com/jit20073
0000000000000000000000000000000000000000
2ab0e6626c7a51799179993ea0fdbb829a1ea852
gitolite-2013-01-07.log:2013-01-07.19:57:16 30374 update
jedi/jedifw user.name@hp.com W
refs/23s/iq/qbar/user.name@hp.com/jit20073
0000000000000000000000000000000000000000
2ab0e6626c7a51799179993ea0fdbb829a1ea852
There are no log entries for that branch or that SHA in between those
two lines. My understanding is that both of those lines show a new
branch being created on the remote repo. I'm not sure what happened to
the first push, though: I expected that the branch would have been
created at that time, and subsequent fetches from the remote repo would
show the branch. However, fetches from the remote repo did not show the
branch until after the second push.
We've been running git for about 3 years, and this happened occasionally
when we first started with git, but we always found it was related to a
huge number of unreachable loose objects, which triggered an auto-gc on
the remote repo. When we performed manual gc --aggressive and prune
operations on the remote repo, the problem stopped. It happened this
week because we'd deleted over 4,000 refs/topics branches on
Friday/Saturday, which left a huge number of unreachable loose objects.
Our cleanup script was only pruning objects older than 2.weeks.ago,
and when I pruned objects older than 2.days.ago the problem again stopped.
Is this expected behavior for the interaction between pushes and auto-gc
on the remote repo?
Also, I went through the release notes for the latest versions of git,
and I found this for 1.7.11.6:
* When "git push" triggered the automatic gc on the receiving end, a
message from "git prune" that said it was removing cruft leaked to
the standard output, breaking the communication protocol.
Could that be related to what we're seeing?
Thanks,
Bob Lavey
^ permalink raw reply
* Re: Please pull l10n updates
From: Junio C Hamano @ 2013-01-09 16:31 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, Ralf Thielow, Christian Stimming, Michael J Gruber
In-Reply-To: <CANYiYbENRCEv93eL5h8AHZtg=gxpGiTLQX2J7R9SdFWdrYFvKA@mail.gmail.com>
Thanks.
^ permalink raw reply
* [PATCH] Makefile: track TCLTK_PATH as it used to be tracked
From: Junio C Hamano @ 2013-01-09 16:55 UTC (permalink / raw)
To: paulus; +Cc: git, Christian Couder
From: Christian Couder <chriscool@tuxfamily.org>
gitk, when bound into the git.git project tree, used to live at the
root level, but in 62ba514 (Move gitk to its own subdirectory,
2007-11-17) it was moved to a subdirectory. The code used to track
changes to TCLTK_PATH (which should cause gitk to be rebuilt to
point at the new interpreter) was left in the main Makefile instead
of being moved to the new Makefile that was created for the gitk
project.
Also add .gitignore file to list build artifacts for the gitk
project.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Paul, this is relative to the tip of your tree, 386befb (gitk:
Display important heads even when there are many, 2013-01-02).
Could you consider applying it?
Also I notice that you have many patches I still do not have
there, and I'd appreciate a "Go ahead and pull from me!".
Thanks.
.gitignore | 2 ++
Makefile | 16 ++++++++++++++--
2 files changed, 16 insertions(+), 2 deletions(-)
create mode 100644 .gitignore
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..d7ebcaf
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+/GIT-TCLTK-VARS
+/gitk-wish
diff --git a/Makefile b/Makefile
index e1b6045..5acdc90 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,16 @@ DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
bindir_SQ = $(subst ','\'',$(bindir))
TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
+### Detect Tck/Tk interpreter path changes
+TRACK_TCLTK = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
+
+GIT-TCLTK-VARS: FORCE
+ @VARS='$(TRACK_TCLTK)'; \
+ if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
+ echo 1>&2 " * new Tcl/Tk interpreter location"; \
+ echo "$$VARS" >$@; \
+ fi
+
## po-file creation rules
XGETTEXT ?= xgettext
ifdef NO_MSGFMT
@@ -49,9 +59,9 @@ uninstall::
$(RM) '$(DESTDIR_SQ)$(bindir_SQ)'/gitk
clean::
- $(RM) gitk-wish po/*.msg
+ $(RM) gitk-wish po/*.msg GIT-TCLTK-VARS
-gitk-wish: gitk
+gitk-wish: gitk GIT-TCLTK-VARS
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1,3s|^exec .* "$$0"|exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' <gitk >$@+ && \
chmod +x $@+ && \
@@ -65,3 +75,5 @@ $(ALL_MSGFILES): %.msg : %.po
@echo Generating catalog $@
$(MSGFMT) --statistics --tcl $< -l $(basename $(notdir $<)) -d $(dir $@)
+.PHONY: all install uninstall clean update-po
+.PHONY: FORCE
--
1.8.1.336.g866ceff
^ permalink raw reply related
* Re: [PATCH 16/19] reset [--mixed] --quiet: don't refresh index
From: Jeff King @ 2013-01-09 17:01 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1357719376-16406-17-git-send-email-martinvonz@gmail.com>
On Wed, Jan 09, 2013 at 12:16:13AM -0800, Martin von Zweigbergk wrote:
> "git reset [--mixed]" without --quiet refreshes the index in order to
> display the "Unstaged changes after reset". When --quiet is given,
> that output is suppressed, removing the need to refresh the index.
> Other porcelain commands that care about a refreshed index should
> already be refreshing it, so running e.g. "git reset -q && git diff"
> is still safe.
Hmm. But "git reset -q && git diff-files" would not be?
We have never been very clear about which commands refresh the index.
Since "reset" is about manipulating the index, I'd expect it to be
refreshed afterwards. On the other hand, since we have never guaranteed
anything, perhaps a careful script should always use "git update-index
--refresh". I would not be too surprised if some of our own scripts are
not that careful, though.
-Peff
^ permalink raw reply
* Re: git branch case insensitivity (Possible bug)
From: Alexander Gallego @ 2013-01-09 17:03 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: git
In-Reply-To: <50ED925B.2060402@op5.se>
On Wed, Jan 9, 2013 at 10:52 AM, Andreas Ericsson <ae@op5.se> wrote:
> On 01/09/2013 04:46 PM, Alexander Gallego wrote:
>> Hello,
>>
>> Here is a pastebin where I've reproduced the steps on a clean git repo.
>>
>> http://pastebin.com/0vQZEat0
>>
>>
>>
>> Brief description of the problem:
>>
>>
>>
>> 1.Basically one creates a local branch call it 'imp_fix' (branch off
>> master --> this doesn't matter)
>> 2.One does work, commit, etc
>> 3.One rebases imp_fix with master via: (inside imp_fix) git rebase master
>> 4.One checks out master via: git checkout master
>> 5.One merges an incorrect name "imp_Fix" (notice the capital F)
>> 6.The expected output is that git would say, silly you --> that branch
>> does not exist.
>> 7. Instead it merges (what I think is incorrectly) imp_fix.
>>
>>
>> Kindly let me know if I can provide more details.
>>
>
> Are you using Mac OSX?
Yes
> Are you using the HFS+ filesystem shipped with it?
Likely. I have not touched the fs settings.
> Did you use the filesystem's default settings rather than reinstall your
> system with sensible settings?
Yup. I use whatever was shipped with it.
> If you said "yes" to all of the above, this is a filesystem "feature",
> courtesy of (cr)Apple, and you're screwed.
>
> You can work around it by running "git pack-refs" every time you create
> a branch or a tag though.
Thanks for the tips. I'll be sure to use this.
> --
> Andreas Ericsson andreas.ericsson@op5.se
> OP5 AB www.op5.se
> Tel: +46 8-230225 Fax: +46 8-230231
>
> Considering the successes of the wars on alcohol, poverty, drugs and
> terror, I think we should give some serious thought to declaring war
> on peace.
^ permalink raw reply
* Re: Enabling scissors by default?
From: Jeff King @ 2013-01-09 17:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Phillip Susi, git
In-Reply-To: <7vr4lvb63a.fsf@alter.siamese.dyndns.org>
On Tue, Jan 08, 2013 at 03:36:09PM -0800, Junio C Hamano wrote:
> You could introduce a new configuration variable "am.scissors" and
> personally turn it on, though. Setting that variable *does* count
> as the user explicitly asking for it.
I think we have mailinfo.scissors already.
> > I often see patches being tweaked in response to feedback and
> > resubmitted, usually with a description of what has changed since the
> > previous version. Such descriptions don't need to be in the change
> > log when it is finally applied and seem a perfect use of scissors.
>
> Putting such small logs under "---" line is the accepted practice.
Maybe it is just me, but I find the scissors form more readable, because
the "cover letter" material often serves to introduce and give context
to the patch (e.g., "Thanks for your feedback. I've tried to do X, and
it came out well. Here's the patch." serves as an introduction, and
logically comes before the commit message itself).
That does not say anything one way or another about how dangerous or not
it might be to enable scissors by default. Just my two cents that I like
the scissors style for patches that come as part of a discussion (and I
prefer the "---" style when making comments on the contents of a patch;
i.e., when the comments make more sense to be read after reading the
commit message to understand what the patch does).
-Peff
^ permalink raw reply
* Re: [PATCH v2 03/10] mailmap: remove email copy and length limitation
From: Antoine Pelisse @ 2013-01-09 17:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1357603821-8647-4-git-send-email-gitster@pobox.com>
> +static struct string_list_item *lookup_prefix(struct string_list *map,
> + const char *string, size_t len)
> +{
> + int i = string_list_find_insert_index(map, string, 1);
> + if (i < 0) {
> + /* exact match */
> + i = -1 - i;
> + /* does it match exactly? */
> + if (!map->items[i].string[len])
> + return &map->items[i];
I'm not sure the condition above is necessary, as I don't see why an
exact match would not be an exact match.
We have to trust the cmp function (that mailmap sets itself) to not
return 0 when the lengths are different.
> + }
> +
> + /*
> + * i is at the exact match to an overlong key, or
> + * location the possibly overlong key would be inserted,
> + * which must be after the real location of the key.
> + */
> + while (0 <= --i && i < map->nr) {
> + int cmp = strncasecmp(map->items[i].string, string, len);
> + if (cmp < 0)
> + /*
> + * "i" points at a key definitely below the prefix;
> + * the map does not have string[0:len] in it.
> + */
> + break;
> + else if (!cmp && !map->items[i].string[len])
> + /* found it */
> + return &map->items[i];
> + /*
> + * otherwise, the string at "i" may be string[0:len]
> + * followed by a string that sorts later than string[len:];
> + * keep trying.
> + */
> + }
> + return NULL;
> +}
> +
I've tried to think about nasty use cases but everything seems fine.
^ 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