* [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)
From: Jonathan Nieder @ 2011-12-10 12:46 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <4ECCC935.7010407@viscovery.net>
(-cc: Phil)
Johannes Sixt wrote:
> IMO, it doesn't make sense that git-reset aborts a cherry-pick sequence:
> When I messed up a difficult conflict in the middle of a cherry-pick
> sequence, it might be useful to be able to 'git reset --hard && git
> cherry-pick that-one-commit' to restart the conflict resolution.
>
> (But does a single-commit cherry-pick during a multi-commit cherry-pick
> work to begin with?)
It should, I think.
Here are patches to address some UI warts (such as that one) in
current cherry-pick code.
Patch 1 cleans up to prepare for patch 2, which in turn makes "git
cherry-pick --continue" act like "git rebase --continue" by commiting
a conflict resolution if it has not already been committed. This
brings us closer to a less confusing world in which all commands that
can exit and ask the user to help to get closer to their goal provide
a --continue option as a standard interface to resume (I guess "git
merge --continue" is all that's left to do afterwards).
Patch 3 is from Ram's rr/revert-cherry-pick series. It doesn't have
much to do with this series, but I'd rather work on a codebase with
this particular patch applied, so I applied it before working on
patch 4.
Patch 4 uses Junio's cmdline_info API to distinguish single-picks
from multi-picks. This is the title feature. For a while I thought
something like this was the only sane thing to do, but laziness won
out.
Patches 5-7 remove hacks that patch 4 makes superfluous.
Patch 7 has the downside that if anyone had a .git/sequencer-old
directory lying around, then git will not care after applying the
patch and it will sit just taking up its few bytes and distracting
people that run "ls .git". I'm not sure whether that's worth fixing,
and if so how.
Anyway, I hope you enjoy the series. Thoughts and bug reports
appreciated as usual.
Jonathan Nieder (7):
revert: give --continue handling its own function
revert: allow cherry-pick --continue to commit before resuming
revert: pass around rev-list args in already-parsed form
revert: allow single-pick in the middle of cherry-pick sequence
revert: do not remove state until sequence is finished
Revert "reset: Make reset remove the sequencer state"
revert: stop creating or removing sequencer-old directory
branch.c | 2 -
builtin/revert.c | 138 ++++++++++++++++++++++-----------
sequencer.c | 10 +--
sequencer.h | 12 +---
t/t3510-cherry-pick-sequence.sh | 162 +++++++++++++++++++++++++++++++++++++--
t/t7106-reset-sequence.sh | 52 -------------
6 files changed, 251 insertions(+), 125 deletions(-)
delete mode 100755 t/t7106-reset-sequence.sh
^ permalink raw reply
* [PATCH 1/7] revert: give --continue handling its own function
From: Jonathan Nieder @ 2011-12-10 12:47 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
This makes pick_revisions() a little shorter and easier to read
straight through.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 28 +++++++++++++++++-----------
1 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 1ea525c1..9f6c85c1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1038,6 +1038,21 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
return 0;
}
+static int sequencer_continue(struct replay_opts *opts)
+{
+ struct commit_list *todo_list = NULL;
+
+ if (!file_exists(git_path(SEQ_TODO_FILE)))
+ return error(_("No %s in progress"), action_name(opts));
+ read_populate_opts(&opts);
+ read_populate_todo(&todo_list, opts);
+
+ /* Verify that the conflict has been resolved */
+ if (!index_differs_from("HEAD", 0))
+ todo_list = todo_list->next;
+ return pick_commits(todo_list, opts);
+}
+
static int pick_revisions(struct replay_opts *opts)
{
struct commit_list *todo_list = NULL;
@@ -1056,17 +1071,8 @@ static int pick_revisions(struct replay_opts *opts)
}
if (opts->subcommand == REPLAY_ROLLBACK)
return sequencer_rollback(opts);
- if (opts->subcommand == REPLAY_CONTINUE) {
- if (!file_exists(git_path(SEQ_TODO_FILE)))
- return error(_("No %s in progress"), action_name(opts));
- read_populate_opts(&opts);
- read_populate_todo(&todo_list, opts);
-
- /* Verify that the conflict has been resolved */
- if (!index_differs_from("HEAD", 0))
- todo_list = todo_list->next;
- return pick_commits(todo_list, opts);
- }
+ if (opts->subcommand == REPLAY_CONTINUE)
+ return sequencer_continue(opts);
/*
* Start a new cherry-pick/ revert sequence; but
--
1.7.8.rc3
^ permalink raw reply related
* [PATCH 2/7] revert: allow cherry-pick --continue to commit before resuming
From: Jonathan Nieder @ 2011-12-10 12:49 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
When "git cherry-pick ..bar" encounters conflicts, permit the operator
to use cherry-pick --continue after resolving them as a shortcut for
"git commit && git cherry-pick --continue" to record the resolution
and carry on with the rest of the sequence.
This improves the analogy with "git rebase" (in olden days --continue
was the way to preserve authorship when a rebase encountered
conflicts) and fits well with a general UI goal of making "git cmd
--continue" save humans the trouble of deciding what to do next.
Example: after encountering a conflict from running "git cherry-pick
foo bar baz":
CONFLICT (content): Merge conflict in main.c
error: could not apply f78a8d98c... bar!
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
We edit main.c to resolve the conflict, mark it acceptable with "git
add main.c", and can run "cherry-pick --continue" to resume the
sequence.
$ git cherry-pick --continue
[editor opens to confirm commit message]
[master 78c8a8c98] bar!
1 files changed, 1 insertions(+), 1 deletions(-)
[master 87ca8798c] baz!
1 files changed, 1 insertions(+), 1 deletions(-)
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 23 ++++++-
t/t3510-cherry-pick-sequence.sh | 139 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 156 insertions(+), 6 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 9f6c85c1..a43b4d85 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1038,18 +1038,35 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
return 0;
}
+static int continue_single_pick(void)
+{
+ const char *argv[] = { "commit", NULL };
+
+ if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
+ !file_exists(git_path("REVERT_HEAD")))
+ return error(_("no cherry-pick or revert in progress"));
+ return run_command_v_opt(argv, RUN_GIT_CMD);
+}
+
static int sequencer_continue(struct replay_opts *opts)
{
struct commit_list *todo_list = NULL;
if (!file_exists(git_path(SEQ_TODO_FILE)))
- return error(_("No %s in progress"), action_name(opts));
+ return continue_single_pick();
read_populate_opts(&opts);
read_populate_todo(&todo_list, opts);
/* Verify that the conflict has been resolved */
- if (!index_differs_from("HEAD", 0))
- todo_list = todo_list->next;
+ if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
+ file_exists(git_path("REVERT_HEAD"))) {
+ int ret = continue_single_pick();
+ if (ret)
+ return ret;
+ }
+ if (index_differs_from("HEAD", 0))
+ return error_dirty_index(opts);
+ todo_list = todo_list->next;
return pick_commits(todo_list, opts);
}
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2c4c1c85..4d1883b7 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -2,6 +2,7 @@
test_description='Test cherry-pick continuation features
+ + conflicting: rewrites unrelated to conflicting
+ yetanotherpick: rewrites foo to e
+ anotherpick: rewrites foo to d
+ picked: rewrites foo to c
@@ -27,6 +28,7 @@ test_cmp_rev () {
}
test_expect_success setup '
+ git config advice.detachedhead false
echo unrelated >unrelated &&
git add unrelated &&
test_commit initial foo a &&
@@ -35,8 +37,8 @@ test_expect_success setup '
test_commit picked foo c &&
test_commit anotherpick foo d &&
test_commit yetanotherpick foo e &&
- git config advice.detachedhead false
-
+ pristine_detach initial &&
+ test_commit conflicting unrelated
'
test_expect_success 'cherry-pick persists data on failure' '
@@ -243,7 +245,66 @@ test_expect_success '--continue complains when there are unresolved conflicts' '
test_must_fail git cherry-pick --continue
'
-test_expect_success '--continue continues after conflicts are resolved' '
+test_expect_success '--continue of single cherry-pick' '
+ pristine_detach initial &&
+ echo c >expect &&
+ test_must_fail git cherry-pick picked &&
+ echo c >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+
+ test_cmp expect foo &&
+ test_cmp_rev initial HEAD^ &&
+ git diff --exit-code HEAD &&
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+'
+
+test_expect_success '--continue of single revert' '
+ pristine_detach initial &&
+ echo resolved >expect &&
+ echo "Revert \"picked\"" >expect.msg &&
+ test_must_fail git revert picked &&
+ echo resolved >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+
+ git diff --exit-code HEAD &&
+ test_cmp expect foo &&
+ test_cmp_rev initial HEAD^ &&
+ git diff-tree -s --pretty=tformat:%s HEAD >msg &&
+ test_cmp expect.msg msg &&
+ test_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&
+ test_must_fail git rev-parse --verify REVERT_HEAD
+'
+
+test_expect_success '--continue after resolving conflicts' '
+ pristine_detach initial &&
+ echo d >expect &&
+ cat >expect.log <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M foo
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ OBJID
+ :000000 100644 OBJID OBJID A foo
+ :000000 100644 OBJID OBJID A unrelated
+ EOF
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo c >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual.log &&
+ test_cmp expect foo &&
+ test_cmp expect.log actual.log
+'
+
+test_expect_success '--continue after resolving conflicts and committing' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
echo "c" >foo &&
@@ -270,6 +331,29 @@ test_expect_success '--continue continues after conflicts are resolved' '
test_cmp expect actual
'
+test_expect_success '--continue asks for help after resolving patch to nil' '
+ pristine_detach conflicting &&
+ test_must_fail git cherry-pick initial..picked &&
+
+ test_cmp_rev unrelatedpick CHERRY_PICK_HEAD &&
+ git checkout HEAD -- unrelated &&
+ test_must_fail git cherry-pick --continue 2>msg &&
+ test_i18ngrep "The previous cherry-pick is now empty" msg
+'
+
+test_expect_failure 'follow advice and skip nil patch' '
+ pristine_detach conflicting &&
+ test_must_fail git cherry-pick initial..picked &&
+
+ git checkout HEAD -- unrelated &&
+ test_must_fail git cherry-pick --continue &&
+ git reset &&
+ git cherry-pick --continue &&
+
+ git rev-list initial..HEAD >commits &&
+ test_line_count = 3 commits
+'
+
test_expect_success '--continue respects opts' '
pristine_detach initial &&
test_must_fail git cherry-pick -x base..anotherpick &&
@@ -288,6 +372,29 @@ test_expect_success '--continue respects opts' '
grep "cherry picked from" anotherpick_msg
'
+test_expect_success '--continue of single-pick respects -x' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -x picked &&
+ echo c >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD >msg &&
+ grep "cherry picked from" msg
+'
+
+test_expect_success '--continue respects -x in first commit in multi-pick' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -x picked anotherpick &&
+ echo c >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git cat-file commit HEAD^ >msg &&
+ picked=$(git rev-parse --verify picked) &&
+ grep "cherry picked from.*$picked" msg
+'
+
test_expect_success '--signoff is not automatically propagated to resolved conflict' '
pristine_detach initial &&
test_must_fail git cherry-pick --signoff base..anotherpick &&
@@ -306,6 +413,32 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
grep "Signed-off-by:" anotherpick_msg
'
+test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -s picked anotherpick &&
+ echo c >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+
+ git diff --exit-code HEAD &&
+ test_cmp_rev initial HEAD^^ &&
+ git cat-file commit HEAD^ >msg &&
+ ! grep Signed-off-by: msg
+'
+
+test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick -s picked &&
+ echo c >foo &&
+ git add foo &&
+ git cherry-pick --continue &&
+
+ git diff --exit-code HEAD &&
+ test_cmp_rev initial HEAD^ &&
+ git cat-file commit HEAD >msg &&
+ ! grep Signed-off-by: msg
+'
+
test_expect_success 'malformed instruction sheet 1' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
--
1.7.8.rc3
^ permalink raw reply related
* [PATCH 1/3] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Nguyễn Thái Ngọc Duy @ 2011-12-10 12:53 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jonathan Nieder, Tony Wang,
Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/branch.c | 11 ++++-------
builtin/checkout.c | 15 +++++++--------
builtin/fmt-merge-msg.c | 6 +++---
builtin/for-each-ref.c | 7 ++-----
builtin/merge.c | 12 +++++-------
builtin/notes.c | 6 +++---
builtin/receive-pack.c | 8 +++-----
builtin/revert.c | 2 +-
builtin/show-branch.c | 4 +---
cache.h | 1 +
reflog-walk.c | 13 ++++++-------
refs.c | 6 ++++++
wt-status.c | 4 +---
13 files changed, 43 insertions(+), 52 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index e1e486e..72c4c31 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -103,7 +103,7 @@ static int branch_merged(int kind, const char *name,
* safely to HEAD (or the other branch).
*/
struct commit *reference_rev = NULL;
- const char *reference_name = NULL;
+ char *reference_name = NULL;
int merged;
if (kind == REF_LOCAL_BRANCH) {
@@ -115,10 +115,8 @@ static int branch_merged(int kind, const char *name,
branch->merge[0] &&
branch->merge[0]->dst &&
(reference_name =
- resolve_ref(branch->merge[0]->dst, sha1, 1, NULL)) != NULL) {
- reference_name = xstrdup(reference_name);
+ resolve_refdup(branch->merge[0]->dst, sha1, 1, NULL)) != NULL)
reference_rev = lookup_commit_reference(sha1);
- }
}
if (!reference_rev)
reference_rev = head_rev;
@@ -143,7 +141,7 @@ static int branch_merged(int kind, const char *name,
" '%s', even though it is merged to HEAD."),
name, reference_name);
}
- free((char *)reference_name);
+ free(reference_name);
return merged;
}
@@ -731,10 +729,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
track = git_branch_track;
- head = resolve_ref("HEAD", head_sha1, 0, NULL);
+ head = resolve_refdup("HEAD", head_sha1, 0, NULL);
if (!head)
die(_("Failed to resolve HEAD as a valid ref."));
- head = xstrdup(head);
if (!strcmp(head, "HEAD")) {
detached = 1;
} else {
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b7c6302..a66d3eb 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -696,17 +696,14 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
{
int ret = 0;
struct branch_info old;
+ char *path;
unsigned char rev[20];
int flag;
memset(&old, 0, sizeof(old));
- old.path = resolve_ref("HEAD", rev, 0, &flag);
- if (old.path)
- old.path = xstrdup(old.path);
+ old.path = path = resolve_refdup("HEAD", rev, 0, &flag);
old.commit = lookup_commit_reference_gently(rev, 1);
- if (!(flag & REF_ISSYMREF)) {
- free((char *)old.path);
+ if (!(flag & REF_ISSYMREF))
old.path = NULL;
- }
if (old.path && !prefixcmp(old.path, "refs/heads/"))
old.name = old.path + strlen("refs/heads/");
@@ -720,8 +717,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
}
ret = merge_working_tree(opts, &old, new);
- if (ret)
+ if (ret) {
+ free(path);
return ret;
+ }
if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
orphaned_commit_warning(old.commit);
@@ -729,7 +728,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
update_refs_for_switch(opts, &old, new);
ret = post_checkout_hook(old.commit, new->commit, 1);
- free((char *)old.path);
+ free(path);
return ret || opts->writeout_error;
}
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index bdfa0ea..a27efcd 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -372,14 +372,14 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
int i = 0, pos = 0;
unsigned char head_sha1[20];
const char *current_branch;
+ char *ref;
/* get current branch */
- current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
+ current_branch = ref = resolve_refdup("HEAD", head_sha1, 1, NULL);
if (!current_branch)
die("No current branch");
if (!prefixcmp(current_branch, "refs/heads/"))
current_branch += 11;
- current_branch = xstrdup(current_branch);
/* get a line */
while (pos < in->len) {
@@ -421,7 +421,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
}
strbuf_complete_line(out);
- free((char *)current_branch);
+ free(ref);
return 0;
}
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index d90e5d2..b01d76a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -628,11 +628,8 @@ static void populate_value(struct refinfo *ref)
if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
unsigned char unused1[20];
- const char *symref;
- symref = resolve_ref(ref->refname, unused1, 1, NULL);
- if (symref)
- ref->symref = xstrdup(symref);
- else
+ ref->symref = resolve_refdup(ref->refname, unused1, 1, NULL);
+ if (!ref->symref)
ref->symref = "";
}
diff --git a/builtin/merge.c b/builtin/merge.c
index a1c8534..6437db2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1096,6 +1096,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
struct commit_list *common = NULL;
const char *best_strategy = NULL, *wt_strategy = NULL;
struct commit_list **remotes = &remoteheads;
+ char *branch_ref;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage_with_options(builtin_merge_usage, builtin_merge_options);
@@ -1104,12 +1105,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* Check if we are _not_ on a detached HEAD, i.e. if there is a
* current branch.
*/
- branch = resolve_ref("HEAD", head_sha1, 0, &flag);
- if (branch) {
- if (!prefixcmp(branch, "refs/heads/"))
- branch += 11;
- branch = xstrdup(branch);
- }
+ branch = branch_ref = resolve_refdup("HEAD", head_sha1, 0, &flag);
+ if (branch && !prefixcmp(branch, "refs/heads/"))
+ branch += 11;
if (!branch || is_null_sha1(head_sha1))
head_commit = NULL;
else
@@ -1520,6 +1518,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
ret = suggest_conflicts(option_renormalize);
done:
- free((char *)branch);
+ free(branch_ref);
return ret;
}
diff --git a/builtin/notes.c b/builtin/notes.c
index 10b8bc7..816c998 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -804,6 +804,7 @@ static int merge_commit(struct notes_merge_options *o)
struct notes_tree *t;
struct commit *partial;
struct pretty_print_context pretty_ctx;
+ char *ref;
int ret;
/*
@@ -826,10 +827,9 @@ static int merge_commit(struct notes_merge_options *o)
t = xcalloc(1, sizeof(struct notes_tree));
init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
- o->local_ref = resolve_ref("NOTES_MERGE_REF", sha1, 0, NULL);
+ o->local_ref = ref = resolve_refdup("NOTES_MERGE_REF", sha1, 0, NULL);
if (!o->local_ref)
die("Failed to resolve NOTES_MERGE_REF");
- o->local_ref = xstrdup(o->local_ref);
if (notes_merge_commit(o, t, partial, sha1))
die("Failed to finalize notes merge");
@@ -846,7 +846,7 @@ static int merge_commit(struct notes_merge_options *o)
free_notes(t);
strbuf_release(&msg);
ret = merge_abort(o);
- free((char *)o->local_ref);
+ free(ref);
return ret;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b6d957c..5cd6fc7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -36,7 +36,7 @@ static int use_sideband;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
static int auto_gc = 1;
-static const char *head_name;
+static char *head_name;
static int sent_capabilities;
static enum deny_action parse_deny_action(const char *var, const char *value)
@@ -695,10 +695,8 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
check_aliased_updates(commands);
- free((char *)head_name);
- head_name = resolve_ref("HEAD", sha1, 0, NULL);
- if (head_name)
- head_name = xstrdup(head_name);
+ free(head_name);
+ head_name = resolve_refdup("HEAD", sha1, 0, NULL);
for (cmd = commands; cmd; cmd = cmd->next)
if (!cmd->skip_update)
diff --git a/builtin/revert.c b/builtin/revert.c
index 1ea525c..0c52a83 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -901,7 +901,7 @@ static int rollback_single_pick(void)
if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
!file_exists(git_path("REVERT_HEAD")))
return error(_("no cherry-pick or revert in progress"));
- if (!resolve_ref("HEAD", head_sha1, 0, NULL))
+ if (read_ref_full("HEAD", head_sha1, 0, NULL))
return error(_("cannot resolve HEAD"));
if (is_null_sha1(head_sha1))
return error(_("cannot abort from a branch yet to be born"));
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 4b480d7..a1f148e 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -726,10 +726,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
if (ac == 0) {
static const char *fake_av[2];
- const char *refname;
- refname = resolve_ref("HEAD", sha1, 1, NULL);
- fake_av[0] = xstrdup(refname);
+ fake_av[0] = resolve_refdup("HEAD", sha1, 1, NULL);
fake_av[1] = NULL;
av = fake_av;
ac = 1;
diff --git a/cache.h b/cache.h
index 8c98d05..4887a3e 100644
--- a/cache.h
+++ b/cache.h
@@ -866,6 +866,7 @@ extern int read_ref(const char *filename, unsigned char *sha1);
* errno is sometimes set on errors, but not always.
*/
extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
extern int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/reflog-walk.c b/reflog-walk.c
index da71a85..5572b42 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -50,11 +50,10 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
for_each_reflog_ent(ref, read_one_reflog, reflogs);
if (reflogs->nr == 0) {
unsigned char sha1[20];
- const char *name = resolve_ref(ref, sha1, 1, NULL);
+ char *name = resolve_refdup(ref, sha1, 1, NULL);
if (name) {
- name = xstrdup(name);
for_each_reflog_ent(name, read_one_reflog, reflogs);
- free((char *)name);
+ free(name);
}
}
if (reflogs->nr == 0) {
@@ -171,11 +170,11 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
else {
if (*branch == '\0') {
unsigned char sha1[20];
- const char *head = resolve_ref("HEAD", sha1, 0, NULL);
- if (!head)
- die ("No current branch");
free(branch);
- branch = xstrdup(head);
+ branch = resolve_refdup("HEAD", sha1, 0, NULL);
+ if (!branch)
+ die ("No current branch");
+
}
reflogs = read_complete_reflog(branch);
if (!reflogs || reflogs->nr == 0) {
diff --git a/refs.c b/refs.c
index f5cb297..8ffb32f 100644
--- a/refs.c
+++ b/refs.c
@@ -605,6 +605,12 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
return ref;
}
+char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag)
+{
+ const char *ret = resolve_ref(ref, sha1, reading, flag);
+ return ret ? xstrdup(ret) : NULL;
+}
+
/* The argument to filter_refs */
struct ref_filter {
const char *pattern;
diff --git a/wt-status.c b/wt-status.c
index 70fdb76..9ffc535 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -111,7 +111,6 @@ void status_printf_more(struct wt_status *s, const char *color,
void wt_status_prepare(struct wt_status *s)
{
unsigned char sha1[20];
- const char *head;
memset(s, 0, sizeof(*s));
memcpy(s->color_palette, default_wt_status_colors,
@@ -119,8 +118,7 @@ void wt_status_prepare(struct wt_status *s)
s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
s->use_color = -1;
s->relative_paths = 1;
- head = resolve_ref("HEAD", sha1, 0, NULL);
- s->branch = head ? xstrdup(head) : NULL;
+ s->branch = resolve_refdup("HEAD", sha1, 0, NULL);
s->reference = "HEAD";
s->fp = stdout;
s->index_file = get_index_file();
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 2/3] Guard memory overwriting in resolve_ref's static buffer
From: Nguyễn Thái Ngọc Duy @ 2011-12-10 12:53 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jonathan Nieder, Tony Wang,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1323521631-24320-1-git-send-email-pclouds@gmail.com>
There is a potential problem with resolve_ref() and some other
functions in git. The return value returned by resolve_ref() may
be changed when the function is called again. Callers must make sure the
next call won't happen as long as the value is still being used.
It's usually hard to track down this kind of problem. Michael Haggerty
has an idea [1] that, instead of passing the same static buffer to
caller every time the function is called, we free the old buffer and
allocate the new one. This way access to the old (now invalid) buffer
may be caught.
This patch applies the same principle for resolve_ref() with a
few modifications:
- This behavior is enabled when GIT_DEBUG_MEMCHECK is set. The ability
is always available. We may be able to ask users to rerun with this
flag on in suspicious cases.
- Rely on mmap/mprotect to catch illegal access. We need valgrind or
some other memory tracking tool to reliably catch this in Michael's
approach.
- Because mprotect is used instead of munmap, we definitely leak
memory. Hopefully callers will not put resolve_ref() in a
loop that runs 1 million times.
- Save caller location in the allocated buffer so we know who made this
call in the core dump.
Also introduce a new target, "make memcheck", that runs tests with this
flag on.
[1] http://comments.gmane.org/gmane.comp.version-control.git/182209
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
.gitignore | 1 +
Makefile | 4 ++++
cache.h | 3 ++-
git-compat-util.h | 9 +++++++++
refs.c | 13 +++++++++++--
t/t0071-memcheck.sh | 12 ++++++++++++
test-resolve-ref.c | 12 ++++++++++++
wrapper.c | 22 ++++++++++++++++++++++
8 files changed, 73 insertions(+), 3 deletions(-)
create mode 100755 t/t0071-memcheck.sh
create mode 100644 test-resolve-ref.c
diff --git a/.gitignore b/.gitignore
index 8572c8c..470e452 100644
--- a/.gitignore
+++ b/.gitignore
@@ -179,6 +179,7 @@
/test-obj-pool
/test-parse-options
/test-path-utils
+/test-resolve-ref
/test-run-command
/test-sha1
/test-sigchain
diff --git a/Makefile b/Makefile
index ed82320..d71cf04 100644
--- a/Makefile
+++ b/Makefile
@@ -444,6 +444,7 @@ TEST_PROGRAMS_NEED_X += test-obj-pool
TEST_PROGRAMS_NEED_X += test-parse-options
TEST_PROGRAMS_NEED_X += test-path-utils
TEST_PROGRAMS_NEED_X += test-run-command
+TEST_PROGRAMS_NEED_X += test-resolve-ref
TEST_PROGRAMS_NEED_X += test-sha1
TEST_PROGRAMS_NEED_X += test-sigchain
TEST_PROGRAMS_NEED_X += test-string-pool
@@ -2241,6 +2242,9 @@ export NO_SVN_TESTS
test: all
$(MAKE) -C t/ all
+memcheck: all
+ GIT_DEBUG_MEMCHECK=1 $(MAKE) -C t/ all
+
test-ctype$X: ctype.o
test-date$X: date.o ctype.o
diff --git a/cache.h b/cache.h
index 4887a3e..a63d890 100644
--- a/cache.h
+++ b/cache.h
@@ -865,7 +865,8 @@ extern int read_ref(const char *filename, unsigned char *sha1);
*
* errno is sometimes set on errors, but not always.
*/
-extern const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag);
+#define resolve_ref(ref, sha1, reading, flag) resolve_ref_real(ref, sha1, reading, flag, __FUNCTION__, __LINE__)
+extern const char *resolve_ref_real(const char *ref, unsigned char *sha1, int reading, int *flag, const char *file, int line);
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);
extern int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref);
diff --git a/git-compat-util.h b/git-compat-util.h
index 77062ed..9249fc2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -433,6 +433,15 @@ extern char *xstrndup(const char *str, size_t len);
extern void *xrealloc(void *ptr, size_t size);
extern void *xcalloc(size_t nmemb, size_t size);
extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+
+/*
+ * These functions are used to allocate new memory. When the memory
+ * area is no longer used, ban all access to it so any illegal access
+ * can be caught. xfree_mmap() does not really free memory.
+ */
+extern void *xmalloc_mmap(size_t, const char *file, int line);
+extern void xfree_mmap(void *);
+
extern ssize_t xread(int fd, void *buf, size_t len);
extern ssize_t xwrite(int fd, const void *buf, size_t len);
extern int xdup(int fd);
diff --git a/refs.c b/refs.c
index 8ffb32f..cf8dfcc 100644
--- a/refs.c
+++ b/refs.c
@@ -497,12 +497,21 @@ static int get_packed_ref(const char *ref, unsigned char *sha1)
return -1;
}
-const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *flag)
+const char *resolve_ref_real(const char *ref, unsigned char *sha1,
+ int reading, int *flag, const char *file, int line)
{
int depth = MAXDEPTH;
ssize_t len;
char buffer[256];
- static char ref_buffer[256];
+ static char real_ref_buffer[256];
+ static char *ref_buffer;
+
+ if (!ref_buffer && !getenv("GIT_DEBUG_MEMCHECK"))
+ ref_buffer = real_ref_buffer;
+ if (ref_buffer != real_ref_buffer) {
+ xfree_mmap(ref_buffer);
+ ref_buffer = xmalloc_mmap(256, file, line);
+ }
if (flag)
*flag = 0;
diff --git a/t/t0071-memcheck.sh b/t/t0071-memcheck.sh
new file mode 100755
index 0000000..b594732
--- /dev/null
+++ b/t/t0071-memcheck.sh
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+test_description='test that GIT_DEBUG_MEMCHECK works correctly'
+. ./test-lib.sh
+
+test_expect_success 'test-resolve-ref must crash' '
+ GIT_DEBUG_MEMCHECK=1 test-resolve-ref
+ exit_code=$? &&
+ test $exit_code -eq 139
+'
+
+test_done
diff --git a/test-resolve-ref.c b/test-resolve-ref.c
new file mode 100644
index 0000000..934d764
--- /dev/null
+++ b/test-resolve-ref.c
@@ -0,0 +1,12 @@
+#include "cache.h"
+
+int main(int argc, char **argv)
+{
+ unsigned char sha1[20];
+ const char *ref1, *ref2;
+ setup_git_directory();
+ ref1 = resolve_ref("HEAD", sha1, 0, NULL);
+ ref2 = resolve_ref("HEAD", sha1, 0, NULL);
+ printf("ref1 = %s\nref2 = %s\n", ref1, ref2);
+ return 0;
+}
diff --git a/wrapper.c b/wrapper.c
index 85f09df..407443a 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -60,6 +60,28 @@ void *xmallocz(size_t size)
return ret;
}
+void *xmalloc_mmap(size_t size, const char *file, int line)
+{
+ int *ret;
+ size += sizeof(int*) * 3;
+ ret = mmap(NULL, size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (ret == (int*)-1)
+ die_errno("unable to mmap %lu bytes anonymously",
+ (unsigned long)size);
+
+ ret[0] = (int)file;
+ ret[1] = line;
+ ret[2] = size;
+ return ret + 3;
+}
+
+void xfree_mmap(void *p)
+{
+ if (p && mprotect(((int*)p) - 3, ((int*)p)[-1], PROT_NONE) == -1)
+ die_errno("unable to remove memory access");
+}
+
/*
* xmemdupz() allocates (len + 1) bytes of memory, duplicates "len" bytes of
* "data" to the allocated memory, zero terminates the allocated memory,
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 3/3] Rename resolve_ref() to resolve_ref_unsafe()
From: Nguyễn Thái Ngọc Duy @ 2011-12-10 12:53 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jonathan Nieder, Tony Wang,
Nguyễn Thái Ngọc Duy
In-Reply-To: <1323521631-24320-1-git-send-email-pclouds@gmail.com>
resolve_ref() may return a pointer to a shared buffer and can be
overwritten by the next resolve_ref() calls. Callers need to
pay attention, not to keep the pointer when the next call happens.
Rename with "_unsafe" suffix to warn developers (or reviewers) before
introducing new call sites.
This patch is generated using this command
git grep -l 'resolve_ref(' -- '*.[ch]'|xargs sed -i 's/resolve_ref(/resolve_ref_unsafe(/g'
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
branch.c | 2 +-
builtin/branch.c | 2 +-
builtin/commit.c | 2 +-
builtin/fsck.c | 2 +-
builtin/log.c | 4 ++--
builtin/receive-pack.c | 2 +-
builtin/remote.c | 2 +-
builtin/show-branch.c | 2 +-
builtin/symbolic-ref.c | 2 +-
cache.h | 2 +-
refs.c | 20 ++++++++++----------
remote.c | 6 +++---
test-resolve-ref.c | 4 ++--
transport.c | 2 +-
14 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/branch.c b/branch.c
index d91a099..772a4c2 100644
--- a/branch.c
+++ b/branch.c
@@ -182,7 +182,7 @@ int validate_new_branchname(const char *name, struct strbuf *ref,
const char *head;
unsigned char sha1[20];
- head = resolve_ref("HEAD", sha1, 0, NULL);
+ head = resolve_ref_unsafe("HEAD", sha1, 0, NULL);
if (!is_bare_repository() && head && !strcmp(head, ref->buf))
die("Cannot force update the current branch.");
}
diff --git a/builtin/branch.c b/builtin/branch.c
index 72c4c31..641bee1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -251,7 +251,7 @@ static char *resolve_symref(const char *src, const char *prefix)
int flag;
const char *dst, *cp;
- dst = resolve_ref(src, sha1, 0, &flag);
+ dst = resolve_ref_unsafe(src, sha1, 0, &flag);
if (!(dst && (flag & REF_ISSYMREF)))
return NULL;
if (prefix && (cp = skip_prefix(dst, prefix)))
diff --git a/builtin/commit.c b/builtin/commit.c
index e36e9ad..4d39d25 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1304,7 +1304,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
rev.diffopt.break_opt = 0;
diff_setup_done(&rev.diffopt);
- head = resolve_ref("HEAD", junk_sha1, 0, NULL);
+ head = resolve_ref_unsafe("HEAD", junk_sha1, 0, NULL);
printf("[%s%s ",
!prefixcmp(head, "refs/heads/") ?
head + 11 :
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 30d0dc8..8c479a7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -563,7 +563,7 @@ static int fsck_head_link(void)
if (verbose)
fprintf(stderr, "Checking HEAD link\n");
- head_points_at = resolve_ref("HEAD", head_sha1, 0, &flag);
+ head_points_at = resolve_ref_unsafe("HEAD", head_sha1, 0, &flag);
if (!head_points_at)
return error("Invalid HEAD");
if (!strcmp(head_points_at, "HEAD"))
diff --git a/builtin/log.c b/builtin/log.c
index 4395f3e..89d0cc0 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1040,7 +1040,7 @@ static char *find_branch_name(struct rev_info *rev)
if (positive < 0)
return NULL;
strbuf_addf(&buf, "refs/heads/%s", rev->cmdline.rev[positive].name);
- branch = resolve_ref(buf.buf, branch_sha1, 1, NULL);
+ branch = resolve_ref_unsafe(buf.buf, branch_sha1, 1, NULL);
if (!branch ||
prefixcmp(branch, "refs/heads/") ||
hashcmp(rev->cmdline.rev[positive].item->sha1, branch_sha1))
@@ -1268,7 +1268,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
rev.pending.objects[0].item->flags |= UNINTERESTING;
add_head_to_pending(&rev);
- ref = resolve_ref("HEAD", sha1, 1, NULL);
+ ref = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (ref && !prefixcmp(ref, "refs/heads/"))
branch_name = xstrdup(ref + strlen("refs/heads/"));
else
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 5cd6fc7..a1a4b09 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -571,7 +571,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
int flag;
strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name);
- dst_name = resolve_ref(buf.buf, sha1, 0, &flag);
+ dst_name = resolve_ref_unsafe(buf.buf, sha1, 0, &flag);
strbuf_release(&buf);
if (!(flag & REF_ISSYMREF))
diff --git a/builtin/remote.c b/builtin/remote.c
index 407abfb..583eec9 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -573,7 +573,7 @@ static int read_remote_branches(const char *refname,
strbuf_addf(&buf, "refs/remotes/%s/", rename->old);
if (!prefixcmp(refname, buf.buf)) {
item = string_list_append(rename->remote_branches, xstrdup(refname));
- symref = resolve_ref(refname, orig_sha1, 1, &flag);
+ symref = resolve_ref_unsafe(refname, orig_sha1, 1, &flag);
if (flag & REF_ISSYMREF)
item->util = xstrdup(symref);
else
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index a1f148e..a59e088 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -789,7 +789,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
}
}
- head_p = resolve_ref("HEAD", head_sha1, 1, NULL);
+ head_p = resolve_ref_unsafe("HEAD", head_sha1, 1, NULL);
if (head_p) {
head_len = strlen(head_p);
memcpy(head, head_p, head_len + 1);
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index dea849c..2ef5962 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -12,7 +12,7 @@ static void check_symref(const char *HEAD, int quiet)
{
unsigned char sha1[20];
int flag;
- const char *refs_heads_master = resolve_ref(HEAD, sha1, 0, &flag);
+ const char *refs_heads_master = resolve_ref_unsafe(HEAD, sha1, 0, &flag);
if (!refs_heads_master)
die("No such ref: %s", HEAD);
diff --git a/cache.h b/cache.h
index a63d890..3ce8bda 100644
--- a/cache.h
+++ b/cache.h
@@ -865,7 +865,7 @@ extern int read_ref(const char *filename, unsigned char *sha1);
*
* errno is sometimes set on errors, but not always.
*/
-#define resolve_ref(ref, sha1, reading, flag) resolve_ref_real(ref, sha1, reading, flag, __FUNCTION__, __LINE__)
+#define resolve_ref_unsafe(ref, sha1, reading, flag) resolve_ref_real(ref, sha1, reading, flag, __FUNCTION__, __LINE__)
extern const char *resolve_ref_real(const char *ref, unsigned char *sha1, int reading, int *flag, const char *file, int line);
extern char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag);
diff --git a/refs.c b/refs.c
index cf8dfcc..010bb72 100644
--- a/refs.c
+++ b/refs.c
@@ -361,7 +361,7 @@ static int warn_if_dangling_symref(const char *refname, const unsigned char *sha
if (!(flags & REF_ISSYMREF))
return 0;
- resolves_to = resolve_ref(refname, junk, 0, NULL);
+ resolves_to = resolve_ref_unsafe(refname, junk, 0, NULL);
if (!resolves_to || strcmp(resolves_to, d->refname))
return 0;
@@ -616,7 +616,7 @@ const char *resolve_ref_real(const char *ref, unsigned char *sha1,
char *resolve_refdup(const char *ref, unsigned char *sha1, int reading, int *flag)
{
- const char *ret = resolve_ref(ref, sha1, reading, flag);
+ const char *ret = resolve_ref_unsafe(ref, sha1, reading, flag);
return ret ? xstrdup(ret) : NULL;
}
@@ -629,7 +629,7 @@ struct ref_filter {
int read_ref_full(const char *ref, unsigned char *sha1, int reading, int *flags)
{
- if (resolve_ref(ref, sha1, reading, flags))
+ if (resolve_ref_unsafe(ref, sha1, reading, flags))
return 0;
return -1;
}
@@ -1126,7 +1126,7 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
this_result = refs_found ? sha1_from_ref : sha1;
mksnpath(fullref, sizeof(fullref), *p, len, str);
- r = resolve_ref(fullref, this_result, 1, &flag);
+ r = resolve_ref_unsafe(fullref, this_result, 1, &flag);
if (r) {
if (!refs_found++)
*ref = xstrdup(r);
@@ -1156,7 +1156,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
const char *ref, *it;
mksnpath(path, sizeof(path), *p, len, str);
- ref = resolve_ref(path, hash, 1, NULL);
+ ref = resolve_ref_unsafe(path, hash, 1, NULL);
if (!ref)
continue;
if (!stat(git_path("logs/%s", path), &st) &&
@@ -1192,7 +1192,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
lock = xcalloc(1, sizeof(struct ref_lock));
lock->lock_fd = -1;
- ref = resolve_ref(ref, lock->old_sha1, mustexist, &type);
+ ref = resolve_ref_unsafe(ref, lock->old_sha1, mustexist, &type);
if (!ref && errno == EISDIR) {
/* we are trying to lock foo but we used to
* have foo/bar which now does not exist;
@@ -1205,7 +1205,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
error("there are still refs under '%s'", orig_ref);
goto error_return;
}
- ref = resolve_ref(orig_ref, lock->old_sha1, mustexist, &type);
+ ref = resolve_ref_unsafe(orig_ref, lock->old_sha1, mustexist, &type);
}
if (type_p)
*type_p = type;
@@ -1368,7 +1368,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
if (log && S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldref);
- symref = resolve_ref(oldref, orig_sha1, 1, &flag);
+ symref = resolve_ref_unsafe(oldref, orig_sha1, 1, &flag);
if (flag & REF_ISSYMREF)
return error("refname %s is a symbolic ref, renaming it is not supported",
oldref);
@@ -1657,7 +1657,7 @@ int write_ref_sha1(struct ref_lock *lock,
unsigned char head_sha1[20];
int head_flag;
const char *head_ref;
- head_ref = resolve_ref("HEAD", head_sha1, 1, &head_flag);
+ head_ref = resolve_ref_unsafe("HEAD", head_sha1, 1, &head_flag);
if (head_ref && (head_flag & REF_ISSYMREF) &&
!strcmp(head_ref, lock->ref_name))
log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
@@ -1994,7 +1994,7 @@ int update_ref(const char *action, const char *refname,
int ref_exists(const char *refname)
{
unsigned char sha1[20];
- return !!resolve_ref(refname, sha1, 1, NULL);
+ return !!resolve_ref_unsafe(refname, sha1, 1, NULL);
}
struct ref *find_ref_by_name(const struct ref *list, const char *name)
diff --git a/remote.c b/remote.c
index 6655bb0..73a3809 100644
--- a/remote.c
+++ b/remote.c
@@ -482,7 +482,7 @@ static void read_config(void)
return;
default_remote_name = xstrdup("origin");
current_branch = NULL;
- head_ref = resolve_ref("HEAD", sha1, 0, &flag);
+ head_ref = resolve_ref_unsafe("HEAD", sha1, 0, &flag);
if (head_ref && (flag & REF_ISSYMREF) &&
!prefixcmp(head_ref, "refs/heads/")) {
current_branch =
@@ -1007,7 +1007,7 @@ static char *guess_ref(const char *name, struct ref *peer)
struct strbuf buf = STRBUF_INIT;
unsigned char sha1[20];
- const char *r = resolve_ref(peer->name, sha1, 1, NULL);
+ const char *r = resolve_ref_unsafe(peer->name, sha1, 1, NULL);
if (!r)
return NULL;
@@ -1058,7 +1058,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
unsigned char sha1[20];
int flag;
- dst_value = resolve_ref(matched_src->name, sha1, 1, &flag);
+ dst_value = resolve_ref_unsafe(matched_src->name, sha1, 1, &flag);
if (!dst_value ||
((flag & REF_ISSYMREF) &&
prefixcmp(dst_value, "refs/heads/")))
diff --git a/test-resolve-ref.c b/test-resolve-ref.c
index 934d764..1847cb4 100644
--- a/test-resolve-ref.c
+++ b/test-resolve-ref.c
@@ -5,8 +5,8 @@ int main(int argc, char **argv)
unsigned char sha1[20];
const char *ref1, *ref2;
setup_git_directory();
- ref1 = resolve_ref("HEAD", sha1, 0, NULL);
- ref2 = resolve_ref("HEAD", sha1, 0, NULL);
+ ref1 = resolve_ref_unsafe("HEAD", sha1, 0, NULL);
+ ref2 = resolve_ref_unsafe("HEAD", sha1, 0, NULL);
printf("ref1 = %s\nref2 = %s\n", ref1, ref2);
return 0;
}
diff --git a/transport.c b/transport.c
index 51814b5..e9797c0 100644
--- a/transport.c
+++ b/transport.c
@@ -163,7 +163,7 @@ static void set_upstreams(struct transport *transport, struct ref *refs,
/* Follow symbolic refs (mainly for HEAD). */
localname = ref->peer_ref->name;
remotename = ref->name;
- tmp = resolve_ref(localname, sha, 1, &flag);
+ tmp = resolve_ref_unsafe(localname, sha, 1, &flag);
if (tmp && flag & REF_ISSYMREF &&
!prefixcmp(tmp, "refs/heads/"))
localname = tmp;
--
1.7.8.36.g69ee2
^ permalink raw reply related
* [PATCH 3/7] revert: pass around rev-list args in already-parsed form
From: Jonathan Nieder @ 2011-12-10 12:58 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
Date: Sat, 13 Aug 2011 12:06:23 -0500
Since 7e2bfd3f (revert: allow cherry-picking more than one commit,
2010-07-02), the pick/revert machinery has kept track of the set of
commits to be cherry-picked or reverted using commit_argc and
commit_argv variables, storing the corresponding command-line
parameters.
Future callers as other commands are built in (am, rebase, sequencer)
may find it easier to pass rev-list options to this machinery in
already-parsed form. Teach cmd_cherry_pick and cmd_revert to parse
the rev-list arguments in advance and pass the commit set to
pick_revisions() as a rev_info structure.
Original patch by Jonathan, tweaks and test from Ram.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Improved-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 53 +++++++++++++++++++++-----------------
t/t3510-cherry-pick-sequence.sh | 5 +++
2 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index a43b4d85..71570357 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -60,13 +60,14 @@ struct replay_opts {
int allow_rerere_auto;
int mainline;
- int commit_argc;
- const char **commit_argv;
/* Merge strategy */
const char *strategy;
const char **xopts;
size_t xopts_nr, xopts_alloc;
+
+ /* Only used by REPLAY_NONE */
+ struct rev_info *revs;
};
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@ -169,9 +170,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
die(_("program error"));
}
- opts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,
- PARSE_OPT_KEEP_ARGV0 |
- PARSE_OPT_KEEP_UNKNOWN);
+ argc = parse_options(argc, argv, NULL, options, usage_str,
+ PARSE_OPT_KEEP_ARGV0 |
+ PARSE_OPT_KEEP_UNKNOWN);
/* Check for incompatible subcommands */
verify_opt_mutually_compatible(me,
@@ -213,9 +214,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
NULL);
}
- else if (opts->commit_argc < 2)
- usage_with_options(usage_str, options);
-
if (opts->allow_ff)
verify_opt_compatible(me, "--ff",
"--signoff", opts->signoff,
@@ -223,7 +221,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
"-x", opts->record_origin,
"--edit", opts->edit,
NULL);
- opts->commit_argv = argv;
+
+ if (opts->subcommand != REPLAY_NONE) {
+ opts->revs = NULL;
+ } else {
+ opts->revs = xmalloc(sizeof(*opts->revs));
+ init_revisions(opts->revs, NULL);
+ opts->revs->no_walk = 1;
+ if (argc < 2)
+ usage_with_options(usage_str, options);
+ argc = setup_revisions(argc, argv, opts->revs, NULL);
+ }
+
+ if (argc > 1)
+ usage_with_options(usage_str, options);
}
struct commit_message {
@@ -631,23 +642,15 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
return res;
}
-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
+static void prepare_revs(struct replay_opts *opts)
{
- int argc;
-
- init_revisions(revs, NULL);
- revs->no_walk = 1;
if (opts->action != REVERT)
- revs->reverse = 1;
+ opts->revs->reverse ^= 1;
- argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
- if (argc > 1)
- usage(*revert_or_cherry_pick_usage(opts));
-
- if (prepare_revision_walk(revs))
+ if (prepare_revision_walk(opts->revs))
die(_("revision walk setup failed"));
- if (!revs->commits)
+ if (!opts->revs->commits)
die(_("empty commit set passed"));
}
@@ -844,14 +847,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
static void walk_revs_populate_todo(struct commit_list **todo_list,
struct replay_opts *opts)
{
- struct rev_info revs;
struct commit *commit;
struct commit_list **next;
- prepare_revs(&revs, opts);
+ prepare_revs(opts);
next = todo_list;
- while ((commit = get_revision(&revs)))
+ while ((commit = get_revision(opts->revs)))
next = commit_list_append(commit, next);
}
@@ -1075,6 +1077,9 @@ static int pick_revisions(struct replay_opts *opts)
struct commit_list *todo_list = NULL;
unsigned char sha1[20];
+ if (opts->subcommand == REPLAY_NONE)
+ assert(opts->revs);
+
read_and_refresh_cache(opts);
/*
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 4d1883b7..56c95ec1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -461,4 +461,9 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'empty commit set' '
+ pristine_detach initial &&
+ test_expect_code 128 git cherry-pick base..base
+'
+
test_done
--
1.7.8.rc3
^ permalink raw reply related
* [PATCH 4/7] revert: allow single-pick in the middle of cherry-pick sequence
From: Jonathan Nieder @ 2011-12-10 12:59 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
When I messed up a difficult conflict in the middle of a cherry-pick
sequence, it can be useful to be able to 'git checkout HEAD . && git
cherry-pick that-one-commit' to restart the conflict resolution.
Suggested-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Maybe this should come after patch 6/7, so the use case could use
"git reset --hard".
builtin/revert.c | 26 ++++++++++++++++++++++++++
t/t3510-cherry-pick-sequence.sh | 12 ++++++++++++
2 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 71570357..dcb69904 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1072,6 +1072,12 @@ static int sequencer_continue(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
+static int single_pick(struct commit *cmit, struct replay_opts *opts)
+{
+ setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ return do_pick_commit(cmit, opts);
+}
+
static int pick_revisions(struct replay_opts *opts)
{
struct commit_list *todo_list = NULL;
@@ -1097,6 +1103,26 @@ static int pick_revisions(struct replay_opts *opts)
return sequencer_continue(opts);
/*
+ * If we were called as "git cherry-pick <commit>", just
+ * cherry-pick/revert it, set CHERRY_PICK_HEAD /
+ * REVERT_HEAD, and don't touch the sequencer state.
+ * This means it is possible to cherry-pick in the middle
+ * of a cherry-pick sequence.
+ */
+ if (opts->revs->cmdline.nr == 1 &&
+ opts->revs->cmdline.rev->whence == REV_CMD_REV &&
+ opts->revs->no_walk &&
+ !opts->revs->cmdline.rev->flags) {
+ struct commit *cmit;
+ if (prepare_revision_walk(opts->revs))
+ die(_("revision walk setup failed"));
+ cmit = get_revision(opts->revs);
+ if (!cmit || get_revision(opts->revs))
+ die("BUG: expected exactly one commit from walk");
+ return single_pick(cmit, opts);
+ }
+
+ /*
* Start a new cherry-pick/ revert sequence; but
* first, make sure that an existing one isn't in
* progress
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 56c95ec1..98a27a23 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -50,6 +50,18 @@ test_expect_success 'cherry-pick persists data on failure' '
test_path_is_file .git/sequencer/opts
'
+test_expect_success 'cherry-pick mid-cherry-pick-sequence' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ test_cmp_rev picked CHERRY_PICK_HEAD &&
+ # "oops, I forgot that these patches rely on the change from base"
+ git checkout HEAD foo &&
+ git cherry-pick base &&
+ git cherry-pick picked &&
+ git cherry-pick --continue &&
+ git diff --exit-code anotherpick
+'
+
test_expect_success 'cherry-pick persists opts correctly' '
pristine_detach initial &&
test_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&
--
1.7.8.rc3
^ permalink raw reply related
* [PATCH 5/7] revert: do not remove state until sequence is finished
From: Jonathan Nieder @ 2011-12-10 13:02 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
As v1.7.8-rc0~141^2~4 (2011-08-04) explains, git cherry-pick removes
the sequencer state just before applying the final patch. In the
single-pick case, that was a good thing, since --abort and --continue
work fine without access to such state and removing it provides a
signal that git should not complain about the need to clobber it ("a
cherry-pick or revert is already in progress") in sequences like the
following:
git cherry-pick foo
git read-tree -m -u HEAD; # forget that; let's try a different one
git cherry-pick bar
After the recent patch "allow single-pick in the middle of cherry-pick
sequence" we don't need that hack any more. In the new regime, a
traditional "git cherry-pick <commit>" command never looks at
.git/sequencer, so we do not need to cripple "git cherry-pick
<commit>..<commit>" for it any more.
So now you can run "git cherry-pick --abort" near the end of a
multi-pick sequence and it will abort the entire sequence, instead of
misbehaving and aborting just the final commit.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 12 +-----------
t/t3510-cherry-pick-sequence.sh | 6 +++---
2 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index dcb69904..28deb85b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1018,18 +1018,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
for (cur = todo_list; cur; cur = cur->next) {
save_todo(cur, opts);
res = do_pick_commit(cur->item, opts);
- if (res) {
- if (!cur->next)
- /*
- * An error was encountered while
- * picking the last commit; the
- * sequencer state is useless now --
- * the user simply needs to resolve
- * the conflict and commit
- */
- remove_sequencer_state(0);
+ if (res)
return res;
- }
}
/*
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 98a27a23..851b147f 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -203,10 +203,10 @@ test_expect_success '--abort refuses to clobber unrelated change, harder case' '
test_cmp_rev initial HEAD
'
-test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '
+test_expect_success 'cherry-pick still writes sequencer state when one commit is left' '
pristine_detach initial &&
test_must_fail git cherry-pick base..picked &&
- test_path_is_missing .git/sequencer &&
+ test_path_is_dir .git/sequencer &&
echo "resolved" >foo &&
git add foo &&
git commit &&
@@ -227,7 +227,7 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le
test_cmp expect actual
'
-test_expect_failure '--abort after last commit in sequence' '
+test_expect_success '--abort after last commit in sequence' '
pristine_detach initial &&
test_must_fail git cherry-pick base..picked &&
git cherry-pick --abort &&
--
1.7.8.rc3
^ permalink raw reply related
* [PATCH 6/7] Revert "reset: Make reset remove the sequencer state"
From: Jonathan Nieder @ 2011-12-10 13:03 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
This reverts commit 95eb88d8ee588d89b4f06d2753ed4d16ab13b39f, which
was a UI experiment that did not reflect how "git reset" actually gets
used. The reversion also fixes a test, indicated in the patch.
Encouraged-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
branch.c | 2 -
t/t3510-cherry-pick-sequence.sh | 2 +-
t/t7106-reset-sequence.sh | 52 ---------------------------------------
3 files changed, 1 insertions(+), 55 deletions(-)
delete mode 100755 t/t7106-reset-sequence.sh
diff --git a/branch.c b/branch.c
index 025a97be..a6b6722e 100644
--- a/branch.c
+++ b/branch.c
@@ -3,7 +3,6 @@
#include "refs.h"
#include "remote.h"
#include "commit.h"
-#include "sequencer.h"
struct tracking {
struct refspec spec;
@@ -247,5 +246,4 @@ void remove_branch_state(void)
unlink(git_path("MERGE_MSG"));
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
- remove_sequencer_state(0);
}
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 851b147f..e80050e1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -353,7 +353,7 @@ test_expect_success '--continue asks for help after resolving patch to nil' '
test_i18ngrep "The previous cherry-pick is now empty" msg
'
-test_expect_failure 'follow advice and skip nil patch' '
+test_expect_success 'follow advice and skip nil patch' '
pristine_detach conflicting &&
test_must_fail git cherry-pick initial..picked &&
diff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh
deleted file mode 100755
index 83f7ea59..00000000
--- a/t/t7106-reset-sequence.sh
+++ /dev/null
@@ -1,52 +0,0 @@
-#!/bin/sh
-
-test_description='Test interaction of reset --hard with sequencer
-
- + anotherpick: rewrites foo to d
- + picked: rewrites foo to c
- + unrelatedpick: rewrites unrelated to reallyunrelated
- + base: rewrites foo to b
- + initial: writes foo as a, unrelated as unrelated
-'
-
-. ./test-lib.sh
-
-pristine_detach () {
- git cherry-pick --quit &&
- git checkout -f "$1^0" &&
- git read-tree -u --reset HEAD &&
- git clean -d -f -f -q -x
-}
-
-test_expect_success setup '
- echo unrelated >unrelated &&
- git add unrelated &&
- test_commit initial foo a &&
- test_commit base foo b &&
- test_commit unrelatedpick unrelated reallyunrelated &&
- test_commit picked foo c &&
- test_commit anotherpick foo d &&
- git config advice.detachedhead false
-
-'
-
-test_expect_success 'reset --hard cleans up sequencer state, providing one-level undo' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- test_path_is_dir .git/sequencer &&
- git reset --hard &&
- test_path_is_missing .git/sequencer &&
- test_path_is_dir .git/sequencer-old &&
- git reset --hard &&
- test_path_is_missing .git/sequencer-old
-'
-
-test_expect_success 'cherry-pick --abort does not leave sequencer-old dir' '
- pristine_detach initial &&
- test_must_fail git cherry-pick base..anotherpick &&
- git cherry-pick --abort &&
- test_path_is_missing .git/sequencer &&
- test_path_is_missing .git/sequencer-old
-'
-
-test_done
--
1.7.8.rc3
^ permalink raw reply related
* [PATCH 7/7] revert: stop creating and removing sequencer-old directory
From: Jonathan Nieder @ 2011-12-10 13:06 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Ramkumar Ramachandra, git, Christian Couder,
Martin von Zweigbergk, Jay Soffian
In-Reply-To: <20111210124644.GA22035@elie.hsd1.il.comcast.net>
Now that "git reset" no longer implicitly removes .git/sequencer that
the operator may or may not have wanted to keep, the logic to write a
backup copy of .git/sequencer and remove it when stale is not needed
any more. Simplify the sequencer API and repository layout by
dropping it.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That's the end. I hope the patches provided some amusement, and
advice towards making them more useful would be welcome.
Good night,
Jonathan
builtin/revert.c | 6 +++---
sequencer.c | 10 ++--------
sequencer.h | 12 ++----------
3 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 28deb85b..444df413 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -944,7 +944,7 @@ static int sequencer_rollback(struct replay_opts *opts)
}
if (reset_for_rollback(sha1))
goto fail;
- remove_sequencer_state(1);
+ remove_sequencer_state();
strbuf_release(&buf);
return 0;
fail:
@@ -1026,7 +1026,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
*/
- remove_sequencer_state(1);
+ remove_sequencer_state();
return 0;
}
@@ -1084,7 +1084,7 @@ static int pick_revisions(struct replay_opts *opts)
* one that is being continued
*/
if (opts->subcommand == REPLAY_REMOVE_STATE) {
- remove_sequencer_state(1);
+ remove_sequencer_state();
return 0;
}
if (opts->subcommand == REPLAY_ROLLBACK)
diff --git a/sequencer.c b/sequencer.c
index bc2c046a..d1f28a69 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3,17 +3,11 @@
#include "strbuf.h"
#include "dir.h"
-void remove_sequencer_state(int aggressive)
+void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
- struct strbuf seq_old_dir = STRBUF_INIT;
strbuf_addf(&seq_dir, "%s", git_path(SEQ_DIR));
- strbuf_addf(&seq_old_dir, "%s", git_path(SEQ_OLD_DIR));
- remove_dir_recursively(&seq_old_dir, 0);
- rename(git_path(SEQ_DIR), git_path(SEQ_OLD_DIR));
- if (aggressive)
- remove_dir_recursively(&seq_old_dir, 0);
+ remove_dir_recursively(&seq_dir, 0);
strbuf_release(&seq_dir);
- strbuf_release(&seq_old_dir);
}
diff --git a/sequencer.h b/sequencer.h
index f435fdb4..2d4528f2 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -2,19 +2,11 @@
#define SEQUENCER_H
#define SEQ_DIR "sequencer"
-#define SEQ_OLD_DIR "sequencer-old"
#define SEQ_HEAD_FILE "sequencer/head"
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
-/*
- * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
- * any errors. Intended to be used by 'git reset'.
- *
- * With the aggressive flag, it additionally removes SEQ_OLD_DIR,
- * ignoring any errors. Inteded to be used by the sequencer's
- * '--quit' subcommand.
- */
-void remove_sequencer_state(int aggressive);
+/* Removes SEQ_DIR. */
+extern void remove_sequencer_state(void);
#endif
--
1.7.8.rc3
^ permalink raw reply related
* Re: [PATCH 4/2] grep: turn off threading for non-worktree
From: Pete Wyckoff @ 2011-12-10 13:13 UTC (permalink / raw)
To: René Scharfe; +Cc: Thomas Rast, git, Eric Herman, Junio C Hamano
In-Reply-To: <4EDF9BA0.2080204@lsrfire.ath.cx>
rene.scharfe@lsrfire.ath.cx wrote on Wed, 07 Dec 2011 18:00 +0100:
> Am 07.12.2011 09:12, schrieb Thomas Rast:
> > René Scharfe wrote:
> >> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> >> index 47ac188..e981a9b 100644
> >> --- a/Documentation/git-grep.txt
> >> +++ b/Documentation/git-grep.txt
> >> @@ -228,8 +228,9 @@ OPTIONS
> >> there is a match and with non-zero status when there isn't.
> >>
> >> --threads <n>::
> >> + Run <n> search threads in parallel. Default is 8 when searching
> >> + the worktree and 0 otherwise. This option is ignored if git was
> >> + built without support for POSIX threads.
> > [...]
> >> - nr_threads = (online_cpus() > 1) ? THREADS : 0;
> >> + nr_threads = (online_cpus() > 1 && !list.nr) ? THREADS : 0;
> >
> > It would be more consistent to stick to the pack.threads convention
> > where 0 means "all of my cores", so to disable threading the user
> > would set the number of threads to 1. Or were you trying to measure
> > the contention between the worker thread and the add_work() thread?
>
> Yes, indeed, the cost for the threading overhead does interest me. The
> documentation should perhaps mention --no-threads explicitly to avoid
> confusion.
>
> Currently there is no way to specify "as many threads as there are
> cores" here. Previous measurements indicated that it wasn't too useful,
> however, because I/O parallelism was beneficial even for machines with
> less than eight cores and more threads didn't pay off.
Right. Even for single CPU machines this is true, so the
nr_threads calculation above should still use all 8 THREADS
regardless of the number of online_cpus().
-- Pete
^ permalink raw reply
* Re: [PATCH 1/3] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Jonathan Nieder @ 2011-12-10 13:15 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Tony Wang
In-Reply-To: <1323521631-24320-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy wrote:
> [Subject: Convert resolve_ref+xstrdup to new resolve_refdup function]
I like it.
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -901,7 +901,7 @@ static int rollback_single_pick(void)
> if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
> !file_exists(git_path("REVERT_HEAD")))
> return error(_("no cherry-pick or revert in progress"));
> - if (!resolve_ref("HEAD", head_sha1, 0, NULL))
> + if (read_ref_full("HEAD", head_sha1, 0, NULL))
> return error(_("cannot resolve HEAD"));
> if (is_null_sha1(head_sha1))
> return error(_("cannot abort from a branch yet to be born"));
Unrelated change that snuck in, I assume?
The rest of the patch looks very sensible and no-op-like. :)
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Gioele Barabucci @ 2011-12-10 14:16 UTC (permalink / raw)
To: git; +Cc: Heiko Voigt, git
In-Reply-To: <7vr51htbsy.fsf@alter.siamese.dyndns.org>
On 09/11/2011 18:01, Junio C Hamano wrote:
>> This is almost ready but I would like to know what users of the
>> "floating submodule" think about this.
>
> I do like to hear from potential users as well, because the general
> impression we got was that floating submodules is not a real need of
> anybody,
Floating modules are something much sought after by those who use Git
for non-development purposes, like those who have most of their $HOME
versioned with Git [1]. For example, part of what Joey Hess's `mr` tool
[2] does is to simulate floating submodules for Git-versioned $HOMEs.
In the context of versioned $HOMEs, or with backups in general, precise
tracking of submodules updates is not that important. To quote [3]:
«Last, change tracking is a bit more lenient with home directories. I
may shuffle some stuff around, and I don't need to explain the changes
to anyone else.». In my case, I want my ~/Documents dir (that is in a
different repo from $HOME) to be always updated; I would prefer not to
deal with submodule updates, merges and detached HEADs.
Bye,
[1] http://vcs-home.branchable.com/
[2] http://kitenet.net/~joey/code/mr/
[3] http://joshcarter.com/productivity/svn_hg_git_for_home_directory
--
Gioele Barabucci <gioele@svario.it>
^ permalink raw reply
* [PATCH] gitk: make "git describe" output clickable, too
From: Jim Meyering @ 2011-12-10 15:08 UTC (permalink / raw)
To: git list
I noticed that automake's contribution guidelines suggest using
"git describe" output in commit logs to reference previous commits.
By contrast, in coreutils, I had acquired the habit of using a bare SHA1
prefix (8 hex digits), since gitk creates clickable links for that, and
not for "git describe" output.
I prefer the readability of the full "git describe" output, yet want to
retain the gitk links, so wrote the following that renders as clickable
not just SHA1-like strings, but also an SHA1-like string that is
prefixed by "-g".
Signed-off-by: Jim Meyering <meyering@redhat.com>
---
This is relative to master.
Think of this as mere proof-of-concept:
Ideally, the string preceding the -g would be used to disambiguate
the SHA1 prefix, but that would require more code.
I confess that I haven't looked to see if documentation needs
to be updated or if this would merit test suite additions.
gitk-git/gitk | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..f8eb613 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -6688,7 +6688,7 @@ proc appendwithlinks {text tags} {
set start [$ctext index "end - 1c"]
$ctext insert end $text $tags
- set links [regexp -indices -all -inline {\m[0-9a-f]{6,40}\M} $text]
+ set links [regexp -indices -all -inline {(?:\m|-g)[0-9a-f]{6,40}\M} $text]
foreach l $links {
set s [lindex $l 0]
set e [lindex $l 1]
@@ -6704,6 +6704,10 @@ proc appendwithlinks {text tags} {
proc setlink {id lk} {
global curview ctext pendinglinks
+ if {[string range $id 0 1] eq "-g"} {
+ set id [string range $id 2 end]
+ }
+
set known 0
if {[string length $id] < 40} {
set matches [longid $id]
--
1.7.8.163.g9859a
^ permalink raw reply related
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Leif Gruenwoldt @ 2011-12-10 15:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vborhaqgw.fsf@alter.siamese.dyndns.org>
On Sat, Dec 10, 2011 at 1:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
> So that use case does not sound like a good rationale to require addition
> of floating submodules.
Ok I will try another scenario :)
Imagine again products A, B and C and a common library. The products are in
a stable state of development and track a stable branch of the common lib.
Then imagine an important security fix gets made to the common library. On
the next pull of products A, B, and C they get this fix for free
because they were
floating. They didn't need to communicate with the maintainer of the common
repo to know this. In fact they don't really care. They just want the
latest stable
code for that release branch.
This is how package management on many linux systems works. Dependencies
get updated and all products reap the benefit (or catastrophe) automatically.
^ permalink raw reply
* Re: Access to git repository through squid proxy: The remote end hung up unexpectedly
From: Konstantin Khomoutov @ 2011-12-10 15:37 UTC (permalink / raw)
To: Mathieu Peltier; +Cc: git
In-Reply-To: <CACjeFCA4h_w2UmYywMBV_P+YZcWAE=zRUz-z5eTfAO+oxWKPjw@mail.gmail.com>
On Sat, 10 Dec 2011 09:56:07 +0100
Mathieu Peltier <mathieu.peltier@gmail.com> wrote:
> Hi,
> I am trying to access a git repository (git:// URL) through a squid
> proxy.
>
> squid allows CONNECT for port 9418:
[...]
> 2011/12/09 12:22:44 socat[21428] D shutdown() -> 0
> fatal: The remote end hung up unexpectedly
>
> I tried to use also nc but I get the same error.
> Any advice?
I think you have to verify the git-daemon (you did not say you're using
git-daemon, but it can be presupposed based on the port number) works
by itself before starting to wrap it into layers of complexity.
What happens if you try to clone a git repo directly, without any
tunneling? If this is not possible, try to clone on the host running
git-daemon (use an URL like git://localhost/path/to/repo.git).
If it fails (I suppose it will), try increasing the daemon verbosity
(see git-daemon) manpage.
After all, may be it's as simple as forgetting to `touch` git-export-ok
file in the repository you're trying to clone.
P.S.
As a side note: why are you trying to implement such a strange setup?
Why not just use plain old SSH which just works and provides good level
of security (contrary to Basic HTTP authentication you might be using).
If you need a level of control about who can do what with the repository
you could look at https://github.com/sitaramc/gitolite
^ permalink raw reply
* Re: [PATCH 1/3] Convert resolve_ref+xstrdup to new resolve_refdup function
From: Nguyen Thai Ngoc Duy @ 2011-12-10 15:40 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Junio C Hamano, Tony Wang
In-Reply-To: <20111210131503.GI22035@elie.hsd1.il.comcast.net>
2011/12/10 Jonathan Nieder <jrnieder@gmail.com>:
>> --- a/builtin/revert.c
>> +++ b/builtin/revert.c
>> @@ -901,7 +901,7 @@ static int rollback_single_pick(void)
>> if (!file_exists(git_path("CHERRY_PICK_HEAD")) &&
>> !file_exists(git_path("REVERT_HEAD")))
>> return error(_("no cherry-pick or revert in progress"));
>> - if (!resolve_ref("HEAD", head_sha1, 0, NULL))
>> + if (read_ref_full("HEAD", head_sha1, 0, NULL))
>> return error(_("cannot resolve HEAD"));
>> if (is_null_sha1(head_sha1))
>> return error(_("cannot abort from a branch yet to be born"));
>
> Unrelated change that snuck in, I assume?
Yeah that slipped in. It should be part of c689332 (Convert many
resolve_ref() calls to read_ref*() and ref_exists() - 2011-11-13). I
guess either I missed it or it was a new call site after that patch.
Split it out as a separate patch?
--
Duy
^ permalink raw reply
* Re: [POC PATCH 0/5] Threaded loose object and pack access
From: Nguyen Thai Ngoc Duy @ 2011-12-10 15:51 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, René Scharfe, Junio C Hamano, Eric Herman
In-Reply-To: <cover.1323419666.git.trast@student.ethz.ch>
On Fri, Dec 9, 2011 at 3:39 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> Well, just to make sure we're all left in a confused mess of partly
> conflicting patches, here's another angle on the same thing:
>
> Jeff King wrote:
>> Wow, that's horrible. Leaving aside the parallelism, it's just terrible
>> that reading from the cache is 20 times slower than the worktree. I get
>> similar results on my quad-core machine.
>
> By poking around in sha1_file.c I got that down to about 10. It's not
> great yet, but it seems a start.
>
> The goal would be to improve it to the point where a patch lookup that
> already has all relevant packs open and windows mapped can proceed
> without locking. I'm not sure that's doable short of duplicating the
> whole pack state (including fds and windows) across threads, but I'll
> give it some more thought before going that route.
Another potential user for parallel pack access is fsck. Although fsck
access pattern may be different from grep, fsck would open and read
through all packs.
--
Duy
^ permalink raw reply
* Re: git-work, git-base: an example of how to use it.
From: Adam Spiers @ 2011-12-10 15:54 UTC (permalink / raw)
To: Jon Seymour; +Cc: Git Mailing List
In-Reply-To: <BANLkTim07-a5VwSAt7_vLMzOES_JZad9DA@mail.gmail.com>
Hi Jon,
I've only just discovered git-work, and it looks extremely
interesting. From what I can tell so far, it is exactly what I was
looking for and could have recently saved me considerable pain!
Here's some initial feedback.
On Mon, Apr 25, 2011 at 11:43 AM, Jon Seymour <jon.seymour@gmail.com> wrote:
> I haven't had much feedback about git-work, to this point. Peter
> Baumann mentioned it was a little hard to grok.
I suspect that there is a strong correlation between these two points.
I also found it hard to grok, although I think this could be easily
fixed with a few tweaks to your existing docs. Hopefully this would
result in more feedback.
First some comments about Documentation/git-work.txt:
The existing DESCRIPTION really isn't a description, but merely a
legend for the COMMANDS section, which is where it really should
belong IMHO. Also, the first sentence refers to the "base" of
{branch} without explaining what that means. This is currently the
first full sentence a potential user is likely to read about git-work,
but for me at least, unfortunately it triggered a "huh?" reaction
which sapped my will to continue reading. Subsequently I discovered
that git-base.txt gives a definition of the "base" of a working
branch, but it's such a key ingredient in understanding the whole
workflow that it needs to be covered in the primary document, not in
the manual page for the git-base helper command which, if I understand
correctly, is rarely meant to be invoked directly by the user anyway.
Furthermore, the "base" is defined in terms of the user's "current
work", but that is not clearly defined.
In contrast, the contents of the DISCUSSION section are very helpful;
my initial reaction was "yes! this is exactly what I need!" So I
think *this* should be the DESCRIPTION section, so that it's the first
thing a potential user encounters, other than the SYNOPSIS section
which by necessity of convention has to be at the top. BTW, there's
an "integraton" typo which needs to be fixed.
The EXAMPLES section which immediately follows the DISCUSSION is just
sequence of one-liners and it's not clear how they are related to each
other (if at all), and why/when you would want to use each one.
It would be better if the EXAMPLES section showed a use case "story"
which starts with examples of simple usage and then builds up to more
sophisticated workflows. You seem to have already started to address
this with your README.md example, but the first step in that is a 'git
add' without even explaining which branch you are on at the time, what
commits are already on that branch, or how it relates to other
branches. So currently (for my small brain, at least) there is too
much of a "WTF" factor for it to be useful at first sight.
Perhaps part of the confusion arises from a clash between your concept
of a private working branch which is regularly rebased by 'git work',
and many people's concept of master, which is often a public branch
which as such is expected to be safe to regularly merge from. Maybe
you could avoid this by recommending that a user adopting a workflow
based on 'git work' should use it to control an obviously private
branch, e.g. one named 'private' / 'working' / 'unstable' rather than
'master'.
In another email to this list (Subject: [PATCH 00/23] RFC: Introducing
git-test, git-atomic, git-base and git-work) you give some more
workflow examples, which are useful and could be incorporated into a
use case story.
A well-constructed story would answer questions implicitly raised
within the (current) DISCUSSION section, such as:
- How are dependencies tracked?
- Can you have chains of dependencies? If so, what does the
dependency graph look like? Can a branch have multiple (direct)
dependencies?
- In what order are dependencies included in the base of the working
branch?
By the way, one of the EXAMPLES one-liners is described as "start
gitk, showing only the current work", but looks like it might be a
typo; shouldn't it read:
$ gitk $(git work)
?
Another suggestion to encourage people to try your work out: provide a
quick "how to try this out" guide, preferably one which doesn't
involve building the whole of git.
Finally, please wrap all lines in git-work.txt etc. to less than 80
columns to conform to existing style. Currently these files are
unreadable in certain contexts, e.g.
https://github.com/jonseymour/git/blob/master/Documentation/git-work.txt
due to the unreadably long lines.
I hope that's useful feedback. I will continue experimenting with it ...
Adam
^ permalink raw reply
* Re: [RFD] Handling of non-UTF8 data in gitweb
From: Jakub Narebski @ 2011-12-10 16:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jürgen Kreileder, John Hawley
In-Reply-To: <7vehwhcj3q.fsf@alter.siamese.dyndns.org>
On Wed, 7 Dec 2011, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > But doing this would change gitweb behavior. Currently when
> > encountering something (usually line of output) that is not valid
> > UTF-8, we decode it (to UTF-8) using $fallback_encoding, by default
> > 'latin1'. Note however that this value is per gitweb installation,
> > not per repository.
>
> I think we added and you acked 00f429a (gitweb: Handle non UTF-8 text
> better, 2007-06-03) for a good reason, and I think the above argues that
> whatever issue the commit tried to address is a non-issue. Is it really
> true?
I think that UTF-8 is much more prevalent character encoding in operating
systems, programming languages, APIs, and software applications than it
was 4 years ago.
Also the solution implemented in said commit was a good start, but it
remains incomplete: $fallback_encoding is per-installation which is too
big granularity (there is `gui.encoding` per-repository config... but it
is about main not fallback encoding; best would be to use gitattribute
but currently there is no way to check attribute value at given revision).
The proposed
use open qw(:std :utf8);
and removal of to_utf8 and $fallback_encoding would be regression compared
to post-00f429a... but the tradeoff of more robust UTF-8 handling might be
worth it.
Note that to_utf8 handles git command output part by part, not as a whole;
for UTF-8 vs latin1 (i.e. iso-8859-1) it does not matter though because
latin1 is very unlikely to be recognized as valid utf-8[1], and ASCII
characters pass-through for UTF-8.
[1]: http://en.wikipedia.org/wiki/UTF-8#Advantages
> > ... I guess
> > it could be emulated by defining our own 'utf-8-with-fallback'
> > encoding, or by defining our own PerlIO layer with PerlIO::via.
> > But it no longer be simple solution (though still automatic).
>
> Between the current "everybody who read from the input must remember to
> call to_utf8" and "everybody gets to read utf8 automatically for internal
> processing", even though the latter may involve one-time pain to set up
> the framework to do so, the pros-and-cons feels obvious to me.
There is also a matter of performance (':utf8' and ':encoding(UTF-8)'
are AFAIK implemented in C, both the Encode part and PerlIO part).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Access to git repository through squid proxy: The remote end hung up unexpectedly
From: Mathieu Peltier @ 2011-12-10 17:56 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git
In-Reply-To: <20111210193753.994055f2.kostix@domain007.com>
> What happens if you try to clone a git repo directly, without any
> tunneling? If this is not possible, try to clone on the host running
[...]
> As a side note: why are you trying to implement such a strange setup?
It works from another machine without proxy. In fact I am trying to
allow use of git in a corporate environment (for example to access
read-only git repositories of sf.net only accessible through git
protocol). See for example:
http://www.emilsit.net/blog/archives/how-to-use-the-git-protocol-through-a-http-connect-proxy/
Thanks.Mathieu
^ permalink raw reply
* Re: Question about commit message wrapping
From: Jakub Narebski @ 2011-12-10 19:30 UTC (permalink / raw)
To: Sidney San Martín; +Cc: git
In-Reply-To: <E085218D-9287-4F82-B34C-8379742F818A@sidneysm.com>
On Fri, 9 Dec 2011, Sidney San Martín wrote:
> On Dec 9, 2011, at 8:41 AM, Jakub Narebski wrote:
>> Sidney San Martín <s@sidneysm.com> writes:
>>
>>> *Nothing else* in my everyday life works this way anymore. Line
>>> wrapping gets done on the display end in my email client, my web
>>> browser, my ebook reader entirely automatically, and it adapts to
>>> the size of the window.
>>
>> The problem with automatic wrapping on the display is that there could
>> be parts of commit message that *shouldn't* be wrapped, like code
>> sample, or URL... and in plain text you don't have a way to separate
>> flowed from non-flowed part.
Additional and the more serious problem with wrapping on output is
related to backward compatibility. If you have commit message that is
wrapped e.g. to 80 characters, and you wrap on output to 72 characters,
you would get ugly and nigh unreadable ragged output:
original:
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
commodo consequat. Duis aute irure dolor in reprehenderit in voluptate
velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint
occaecat cupidatat non proident, sunt in culpa qui officia deserunt
mollit anim id est laborum.
wrapped to 70th column:
Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad
minim
veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip
ex ea
commodo consequat. Duis aute irure dolor in reprehenderit in
voluptate
velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint
occaecat cupidatat non proident, sunt in culpa qui officia deserunt
mollit anim id est laborum.
You can re-wrap, but this is more code, and additional problems, like
with ASCII-art like this or pseudo-Markdown headers like this
^^^^^^^^^
Header
======
> I usually lead code, URLs, and other preformatted lines like this with
> a few spaces. Markdown uses the same convention, and it looks like
> commits in this repo do too.
>
> The patch in my last email prints lines which begin with whitespace
> verbatim. How does that work?
What about lists done using hanging indent, e.g.:
* Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua.
or
- Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua.
or
1. Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do
eiusmod tempor incididunt ut labore et dolore magna aliqua.
>> Also with long non-breakable identifiers you sometimes need to wrap by
>> hand (or use linebreaking algorithm from TeX) or go bit over the limit
>> to make it look nice.
>
> Could you elaborate on this? The patch uses strbuf_add_wrapped_text,
> which was already in Git. If an identifier is longer than the wrap
> width, it leaves it alone.
The line breaking algorithm of TeX typesetting engine takes into account
look of whole paragraph (well, mathematical representation of "good look")
before breaking lines and hyphenating words.
What I meant here that if you have long identifier, naive line-breaking
(word-wrapping) algorithm could leave the paragraph unbalanced, with one
or more lines much shorter than the rest, while allowing one line to be
overly long over 1 character leads to nicely wrapped result.
BTW. In Polish (and Czech) typography there is rule that you don't leave
single-letter prepositions like 'i' ('and' in English) hanging at the end
of the line... automatic wrapper cannot ensure that.
>> BTW. proper editor used to create commit message can wrap it for you
>> ;-).
>
> Only if everybody else in the world does it too!
Educate.
> And only if I never care about seeing my commits at any width besides
> 80 columns.
Overly long lines are hard to read, and sometimes fit-to-width would give
us too long lines to read comfortably.
Anyway I am not against adding support for wrapping commit message and
tag payload, but IMHO it must be *optional*: --no-wrap, --wrap=auto
(or just --wrap), --wrap=80, and log.wrap or core.wrap.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCHv3 03/13] introduce credentials API
From: Jeff King @ 2011-12-10 19:48 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git
In-Reply-To: <m3r50chcvm.fsf@localhost.localdomain>
On Sat, Dec 10, 2011 at 03:43:01AM -0800, Jakub Narebski wrote:
> Jeff King <peff@peff.net> writes:
>
> > There are a few places in git that need to get a username
> > and password credential from the user; the most notable one
> > is HTTP authentication for smart-http pushing.
>
> A question: does it work also for access via SSH, either without
> public key set up (i.e. 'keyboard-interactive'), or with encrypted
> private key without ssh-agent set up?
No. ssh handles its own password querying, and contacts the user
directly via the terminal. And there's not much point in using a
password helper with ssh; if you don't want to type your password, set
up a key and use ssh-agent.
> It would probably require URL i.e. ssh://git.example.com/srv/scm/repo.git
> or git+ssh://git.example.com/srv/scm/repo.git and not scp-like
> address i.e. user@git.example.com:/srv/scm/repo.git, isn't it?
It's not a matter of recognizing the URL; it's that we hand off the
authentication problem to ssh, which takes care of it entirely itself.
-Peff
^ permalink raw reply
* Re: [PATCHv3 08/13] credential: make relevance of http path configurable
From: Jeff King @ 2011-12-10 19:50 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, git
In-Reply-To: <m3mxb0hcjs.fsf@localhost.localdomain>
On Sat, Dec 10, 2011 at 03:50:11AM -0800, Jakub Narebski wrote:
> > This is nothing you couldn't do with a clever credential
> > helper at the start of your stack, like:
> >
> > [credential "http://"]
> > helper = "!f() { grep -v ^path= ; }; f"
> > helper = your_real_helper
> >
> > But doing this:
> >
> > [credential]
> > useHttpPath = false
>
> Shouldn't this be 'usePath' or 'usePathComponent' or 'useRepositoryPath',
> etc.? Because if^W when remote helper for Subversion is complete, you
> could have svn://svnserve.example.com/path/to/repo as an URL... which
> would be not HTTP(S).
It must be per-protocol, because paths will be relevant for some
protocols (e.g., unlocking certificates in the local filesystem).
So if anything, it would be "useNetworkPath" or something, if we wanted
to unify svn and http. But it would also be OK to have a separate flag
for http and svn, which is more flexible (and most users aren't going to
set it at all, so they won't notice).
-Peff
^ 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