* [PATCH v3 03/10] clone: factor out checkout code
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
Read HEAD from disk instead of relying on local variable
our_head_points_at. This reduces complexity of cmd_clone() a little
bit.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/clone.c | 101 +++++++++++++++++++++++++++++++-----------------------
1 files changed, 58 insertions(+), 43 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 8dde1ea..100056d 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -448,6 +448,63 @@ static void write_remote_refs(const struct ref *local_refs)
clear_extra_refs();
}
+static int checkout(void)
+{
+ unsigned char sha1[20];
+ char *head;
+ struct lock_file *lock_file;
+ struct unpack_trees_options opts;
+ struct tree *tree;
+ struct tree_desc t;
+ int err = 0, fd;
+
+ if (option_no_checkout)
+ return 0;
+
+ head = resolve_refdup("HEAD", sha1, 1, NULL);
+ if (!head) {
+ warning(_("remote HEAD refers to nonexistent ref, "
+ "unable to checkout.\n"));
+ return 0;
+ }
+ if (strcmp(head, "HEAD")) {
+ if (prefixcmp(head, "refs/heads/"))
+ die(_("HEAD not found below refs/heads!"));
+ }
+ free(head);
+
+ /* We need to be in the new work tree for the checkout */
+ setup_work_tree();
+
+ lock_file = xcalloc(1, sizeof(struct lock_file));
+ fd = hold_locked_index(lock_file, 1);
+
+ memset(&opts, 0, sizeof opts);
+ opts.update = 1;
+ opts.merge = 1;
+ opts.fn = oneway_merge;
+ opts.verbose_update = (option_verbosity > 0);
+ opts.src_index = &the_index;
+ opts.dst_index = &the_index;
+
+ tree = parse_tree_indirect(sha1);
+ parse_tree(tree);
+ init_tree_desc(&t, tree->buffer, tree->size);
+ unpack_trees(1, &t, &opts);
+
+ if (write_cache(fd, active_cache, active_nr) ||
+ commit_locked_index(lock_file))
+ die(_("unable to write new index file"));
+
+ err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
+ sha1_to_hex(sha1), "1", NULL);
+
+ if (!err && option_recursive)
+ err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+
+ return err;
+}
+
static int write_one_config(const char *key, const char *value, void *data)
{
return git_config_set_multivar(key, value ? value : "true", "^$", 0);
@@ -722,13 +779,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
/* Source had detached HEAD pointing somewhere. */
update_ref(reflog_msg.buf, "HEAD", remote_head->old_sha1,
NULL, REF_NODEREF, DIE_ON_ERR);
- our_head_points_at = remote_head;
- } else {
- /* Nothing to checkout out */
- if (!option_no_checkout)
- warning(_("remote HEAD refers to nonexistent ref, "
- "unable to checkout.\n"));
- option_no_checkout = 1;
}
if (transport) {
@@ -736,42 +786,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
transport_disconnect(transport);
}
- if (!option_no_checkout) {
- struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
- struct unpack_trees_options opts;
- struct tree *tree;
- struct tree_desc t;
- int fd;
-
- /* We need to be in the new work tree for the checkout */
- setup_work_tree();
-
- fd = hold_locked_index(lock_file, 1);
-
- memset(&opts, 0, sizeof opts);
- opts.update = 1;
- opts.merge = 1;
- opts.fn = oneway_merge;
- opts.verbose_update = (option_verbosity > 0);
- opts.src_index = &the_index;
- opts.dst_index = &the_index;
-
- tree = parse_tree_indirect(our_head_points_at->old_sha1);
- parse_tree(tree);
- init_tree_desc(&t, tree->buffer, tree->size);
- unpack_trees(1, &t, &opts);
-
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(lock_file))
- die(_("unable to write new index file"));
-
- err |= run_hook(NULL, "post-checkout", sha1_to_hex(null_sha1),
- sha1_to_hex(our_head_points_at->old_sha1), "1",
- NULL);
-
- if (!err && option_recursive)
- err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
- }
+ err = checkout();
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 04/10] clone: factor out HEAD update code
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/clone.c | 36 +++++++++++++++++++-----------------
1 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 100056d..26a037c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -448,6 +448,24 @@ static void write_remote_refs(const struct ref *local_refs)
clear_extra_refs();
}
+static void update_head(const struct ref *our, const struct ref *remote,
+ const char *msg)
+{
+ if (our) {
+ /* Local default branch link */
+ create_symref("HEAD", our->name, NULL);
+ if (!option_bare) {
+ const char *head = skip_prefix(our->name, "refs/heads/");
+ update_ref(msg, "HEAD", our->old_sha1, NULL, 0, DIE_ON_ERR);
+ install_branch_config(0, head, option_origin, our->name);
+ }
+ } else if (remote) {
+ /* Source had detached HEAD pointing somewhere. */
+ update_ref(msg, "HEAD", remote->old_sha1,
+ NULL, REF_NODEREF, DIE_ON_ERR);
+ }
+}
+
static int checkout(void)
{
unsigned char sha1[20];
@@ -763,23 +781,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
reflog_msg.buf);
}
- if (our_head_points_at) {
- /* Local default branch link */
- create_symref("HEAD", our_head_points_at->name, NULL);
- if (!option_bare) {
- const char *head = skip_prefix(our_head_points_at->name,
- "refs/heads/");
- update_ref(reflog_msg.buf, "HEAD",
- our_head_points_at->old_sha1,
- NULL, 0, DIE_ON_ERR);
- install_branch_config(0, head, option_origin,
- our_head_points_at->name);
- }
- } else if (remote_head) {
- /* Source had detached HEAD pointing somewhere. */
- update_ref(reflog_msg.buf, "HEAD", remote_head->old_sha1,
- NULL, REF_NODEREF, DIE_ON_ERR);
- }
+ update_head(our_head_points_at, remote_head, reflog_msg.buf);
if (transport) {
transport_unlock_pack(transport);
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 05/10] clone: factor out remote ref writing
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/clone.c | 36 ++++++++++++++++++++++++------------
1 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 26a037c..73d07ed 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -448,6 +448,28 @@ static void write_remote_refs(const struct ref *local_refs)
clear_extra_refs();
}
+static void update_remote_refs(const struct ref *refs,
+ const struct ref *mapped_refs,
+ const struct ref *remote_head_points_at,
+ const char *branch_top,
+ const char *msg)
+{
+ if (refs) {
+ clear_extra_refs();
+ write_remote_refs(mapped_refs);
+ }
+
+ if (remote_head_points_at && !option_bare) {
+ struct strbuf head_ref = STRBUF_INIT;
+ strbuf_addstr(&head_ref, branch_top);
+ strbuf_addstr(&head_ref, "HEAD");
+ create_symref(head_ref.buf,
+ remote_head_points_at->peer_ref->name,
+ msg);
+ }
+
+}
+
static void update_head(const struct ref *our, const struct ref *remote,
const char *msg)
{
@@ -735,10 +757,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
}
if (refs) {
- clear_extra_refs();
-
- write_remote_refs(mapped_refs);
-
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
guess_remote_head(remote_head, mapped_refs, 0);
@@ -772,14 +790,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
"refs/heads/master");
}
- if (remote_head_points_at && !option_bare) {
- struct strbuf head_ref = STRBUF_INIT;
- strbuf_addstr(&head_ref, branch_top.buf);
- strbuf_addstr(&head_ref, "HEAD");
- create_symref(head_ref.buf,
- remote_head_points_at->peer_ref->name,
- reflog_msg.buf);
- }
+ update_remote_refs(refs, mapped_refs, remote_head_points_at,
+ branch_top.buf, reflog_msg.buf);
update_head(our_head_points_at, remote_head, reflog_msg.buf);
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 06/10] clone: delay cloning until after remote HEAD checking
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
This gives us an opportunity to abort the command during remote HEAD
check without wasting much bandwidth.
Cloning with remote-helper remains before the check because the remote
helper updates mapped_refs, which is necessary for remote ref checks.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
I'm not familiar with remote-helper to see if there's any better way
to do this..
builtin/clone.c | 54 +++++++++++++++++++++++++++---------------------------
transport.c | 5 ++++-
2 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 73d07ed..05224d7 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -361,13 +361,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
closedir(dir);
}
-static const struct ref *clone_local(const char *src_repo,
- const char *dest_repo)
+static void clone_local(const char *src_repo, const char *dest_repo)
{
- const struct ref *ret;
- struct remote *remote;
- struct transport *transport;
-
if (option_shared) {
struct strbuf alt = STRBUF_INIT;
strbuf_addf(&alt, "%s/objects", src_repo);
@@ -383,13 +378,8 @@ static const struct ref *clone_local(const char *src_repo,
strbuf_release(&dest);
}
- remote = remote_get(src_repo);
- transport = transport_get(remote, src_repo);
- ret = transport_get_remote_refs(transport);
- transport_disconnect(transport);
if (0 <= option_verbosity)
printf(_("done.\n"));
- return ret;
}
static const char *junk_work_tree;
@@ -576,6 +566,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
struct strbuf branch_top = STRBUF_INIT, reflog_msg = STRBUF_INIT;
struct transport *transport = NULL;
char *src_ref_prefix = "refs/heads/";
+ struct remote *remote;
int err = 0;
struct refspec *refspec;
@@ -727,13 +718,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
strbuf_reset(&value);
- if (is_local) {
- refs = clone_local(path, git_dir);
- mapped_refs = wanted_peer_refs(refs, refspec);
- } else {
- struct remote *remote = remote_get(option_origin);
- transport = transport_get(remote, remote->url[0]);
+ remote = remote_get(option_origin);
+ transport = transport_get(remote, remote->url[0]);
+ if (!is_local) {
if (!transport->get_refs_list || !transport->fetch)
die(_("Don't know how to clone %s"), transport->url);
@@ -748,14 +736,23 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (option_upload_pack)
transport_set_option(transport, TRANS_OPT_UPLOADPACK,
option_upload_pack);
-
- refs = transport_get_remote_refs(transport);
- if (refs) {
- mapped_refs = wanted_peer_refs(refs, refspec);
- transport_fetch_refs(transport, mapped_refs);
- }
}
+ refs = transport_get_remote_refs(transport);
+ mapped_refs = refs ? wanted_peer_refs(refs, refspec) : NULL;
+
+ /*
+ * mapped_refs may be updated if transport-helper is used so
+ * we need fetch it early because remote_head code below
+ * relies on it.
+ *
+ * for normal clones, transport_get_remote_refs() should
+ * return reliable ref set, we can delay cloning until after
+ * remote HEAD check.
+ */
+ if (!is_local && remote->foreign_vcs && refs)
+ transport_fetch_refs(transport, mapped_refs);
+
if (refs) {
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -790,15 +787,18 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
"refs/heads/master");
}
+ if (is_local)
+ clone_local(path, git_dir);
+ else if (refs && !remote->foreign_vcs)
+ transport_fetch_refs(transport, mapped_refs);
+
update_remote_refs(refs, mapped_refs, remote_head_points_at,
branch_top.buf, reflog_msg.buf);
update_head(our_head_points_at, remote_head, reflog_msg.buf);
- if (transport) {
- transport_unlock_pack(transport);
- transport_disconnect(transport);
- }
+ transport_unlock_pack(transport);
+ transport_disconnect(transport);
err = checkout();
diff --git a/transport.c b/transport.c
index a99b7c9..aae9889 100644
--- a/transport.c
+++ b/transport.c
@@ -895,8 +895,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
while (is_urlschemechar(p == url, *p))
p++;
- if (!prefixcmp(p, "::"))
+ if (!prefixcmp(p, "::")) {
helper = xstrndup(url, p - url);
+ remote->foreign_vcs = helper;
+ }
}
if (helper) {
@@ -938,6 +940,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
char *handler = xmalloc(len + 1);
handler[len] = 0;
strncpy(handler, url, len);
+ remote->foreign_vcs = helper;
transport_helper_init(ret, handler);
}
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 07/10] clone: --branch=<branch> always means refs/heads/<branch>
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
It does not make sense to look outside refs/heads for HEAD's target
(src_ref_prefix can be set to "refs/" if --mirror is used) because ref
code only allows symref in form refs/heads/...
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/clone.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 05224d7..960242f 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -463,7 +463,7 @@ static void update_remote_refs(const struct ref *refs,
static void update_head(const struct ref *our, const struct ref *remote,
const char *msg)
{
- if (our) {
+ if (our && !prefixcmp(our->name, "refs/heads/")) {
/* Local default branch link */
create_symref("HEAD", our->name, NULL);
if (!option_bare) {
@@ -760,7 +760,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (option_branch) {
struct strbuf head = STRBUF_INIT;
- strbuf_addstr(&head, src_ref_prefix);
+ strbuf_addstr(&head, "refs/heads/");
strbuf_addstr(&head, option_branch);
our_head_points_at =
find_ref_by_name(mapped_refs, head.buf);
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 08/10] clone: refuse to clone if --branch points to bogus ref
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
It's possible that users make a typo in the branch name. Stop and let
users recheck. Falling back to remote's HEAD is not documented any
way.
Except when using remote helper, the pack has not been transferred at
this stage yet so we don't waste much bandwidth.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/clone.c | 10 ++++------
t/t5706-clone-branch.sh | 8 ++------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 960242f..f751997 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -766,12 +766,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
find_ref_by_name(mapped_refs, head.buf);
strbuf_release(&head);
- if (!our_head_points_at) {
- warning(_("Remote branch %s not found in "
- "upstream %s, using HEAD instead"),
- option_branch, option_origin);
- our_head_points_at = remote_head_points_at;
- }
+ if (!our_head_points_at)
+ die(_("Remote branch %s not found in "
+ "upstream %s, using HEAD instead"),
+ option_branch, option_origin);
}
else
our_head_points_at = remote_head_points_at;
diff --git a/t/t5706-clone-branch.sh b/t/t5706-clone-branch.sh
index f3f9a76..56be67e 100755
--- a/t/t5706-clone-branch.sh
+++ b/t/t5706-clone-branch.sh
@@ -57,12 +57,8 @@ test_expect_success 'clone -b does not munge remotes/origin/HEAD' '
)
'
-test_expect_success 'clone -b with bogus branch chooses HEAD' '
- git clone -b bogus parent clone-bogus &&
- (cd clone-bogus &&
- check_HEAD master &&
- check_file one
- )
+test_expect_success 'clone -b with bogus branch' '
+ test_must_fail git clone -b bogus parent clone-bogus
'
test_done
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 09/10] clone: allow --branch to take a tag
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
Because a tag ref cannot be put to HEAD, HEAD will become detached.
This is consistent with "git checkout <tag>".
This is mostly useful in shallow clone, where it allows you to clone a
tag in addtion to branches.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-clone.txt | 5 +++--
builtin/clone.c | 13 +++++++++++++
t/t5601-clone.sh | 9 +++++++++
3 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 4b8b26b..db2b29c 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -146,8 +146,9 @@ objects from the source repository into a pack in the cloned repository.
-b <name>::
Instead of pointing the newly created HEAD to the branch pointed
to by the cloned repository's HEAD, point to `<name>` branch
- instead. In a non-bare repository, this is the branch that will
- be checked out.
+ instead. `--branch` can also take tags and treat them like
+ detached HEAD. In a non-bare repository, this is the branch
+ that will be checked out.
--upload-pack <upload-pack>::
-u <upload-pack>::
diff --git a/builtin/clone.c b/builtin/clone.c
index f751997..ed18c1a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -471,6 +471,11 @@ static void update_head(const struct ref *our, const struct ref *remote,
update_ref(msg, "HEAD", our->old_sha1, NULL, 0, DIE_ON_ERR);
install_branch_config(0, head, option_origin, our->name);
}
+ } else if (our) {
+ struct commit *c = lookup_commit_reference(our->old_sha1);
+ /* Source had detached HEAD pointing somewhere. */
+ update_ref(msg, "HEAD", c->object.sha1,
+ NULL, REF_NODEREF, DIE_ON_ERR);
} else if (remote) {
/* Source had detached HEAD pointing somewhere. */
update_ref(msg, "HEAD", remote->old_sha1,
@@ -766,6 +771,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
find_ref_by_name(mapped_refs, head.buf);
strbuf_release(&head);
+ if (!our_head_points_at) {
+ strbuf_addstr(&head, "refs/tags/");
+ strbuf_addstr(&head, option_branch);
+ our_head_points_at =
+ find_ref_by_name(mapped_refs, head.buf);
+ strbuf_release(&head);
+ }
+
if (!our_head_points_at)
die(_("Remote branch %s not found in "
"upstream %s, using HEAD instead"),
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index e0b8db6..67869b4 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -271,4 +271,13 @@ test_expect_success 'clone from original with relative alternate' '
grep /src/\\.git/objects target-10/objects/info/alternates
'
+test_expect_success 'clone checking out a tag' '
+ git clone --branch=some-tag src dst.tag &&
+ GIT_DIR=src/.git git rev-parse some-tag >expected &&
+ test_cmp expected dst.tag/.git/HEAD &&
+ GIT_DIR=dst.tag/.git git config remote.origin.fetch >fetch.actual &&
+ echo "+refs/heads/*:refs/remotes/origin/*" >fetch.expected &&
+ test_cmp fetch.expected fetch.actual
+'
+
test_done
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* [PATCH v3 10/10] clone: print advice on checking out detached HEAD
From: Nguyễn Thái Ngọc Duy @ 2012-01-10 9:57 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <1326023188-15559-1-git-send-email-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
advice.c | 14 ++++++++++++++
advice.h | 1 +
builtin/checkout.c | 16 +---------------
builtin/clone.c | 5 ++++-
4 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/advice.c b/advice.c
index e02e632..3e1a145 100644
--- a/advice.c
+++ b/advice.c
@@ -64,3 +64,17 @@ void NORETURN die_resolve_conflict(const char *me)
error_resolve_conflict(me);
die("Exiting because of an unresolved conflict.");
}
+
+void detach_advice(const char *new_name)
+{
+ const char fmt[] =
+ "Note: checking out '%s'.\n\n"
+ "You are in 'detached HEAD' state. You can look around, make experimental\n"
+ "changes and commit them, and you can discard any commits you make in this\n"
+ "state without impacting any branches by performing another checkout.\n\n"
+ "If you want to create a new branch to retain commits you create, you may\n"
+ "do so (now or later) by using -b with the checkout command again. Example:\n\n"
+ " git checkout -b new_branch_name\n\n";
+
+ fprintf(stderr, fmt, new_name);
+}
diff --git a/advice.h b/advice.h
index e5d0af7..7bda45b 100644
--- a/advice.h
+++ b/advice.h
@@ -14,5 +14,6 @@ int git_default_advice_config(const char *var, const char *value);
void advise(const char *advice, ...);
int error_resolve_conflict(const char *me);
extern void NORETURN die_resolve_conflict(const char *me);
+void detach_advice(const char *new_name);
#endif /* ADVICE_H */
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f1984d9..5bf96ba 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -514,20 +514,6 @@ static void report_tracking(struct branch_info *new)
strbuf_release(&sb);
}
-static void detach_advice(const char *old_path, const char *new_name)
-{
- const char fmt[] =
- "Note: checking out '%s'.\n\n"
- "You are in 'detached HEAD' state. You can look around, make experimental\n"
- "changes and commit them, and you can discard any commits you make in this\n"
- "state without impacting any branches by performing another checkout.\n\n"
- "If you want to create a new branch to retain commits you create, you may\n"
- "do so (now or later) by using -b with the checkout command again. Example:\n\n"
- " git checkout -b new_branch_name\n\n";
-
- fprintf(stderr, fmt, new_name);
-}
-
static void update_refs_for_switch(struct checkout_opts *opts,
struct branch_info *old,
struct branch_info *new)
@@ -575,7 +561,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,
REF_NODEREF, DIE_ON_ERR);
if (!opts->quiet) {
if (old->path && advice_detached_head)
- detach_advice(old->path, new->name);
+ detach_advice(new->name);
describe_detached_head(_("HEAD is now at"), new->commit);
}
} else if (new->path) { /* Switch branches. */
diff --git a/builtin/clone.c b/builtin/clone.c
index ed18c1a..70f0280 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -502,7 +502,10 @@ static int checkout(void)
"unable to checkout.\n"));
return 0;
}
- if (strcmp(head, "HEAD")) {
+ if (!strcmp(head, "HEAD")) {
+ if (advice_detached_head)
+ detach_advice(sha1_to_hex(sha1));
+ } else {
if (prefixcmp(head, "refs/heads/"))
die(_("HEAD not found below refs/heads!"));
}
--
1.7.3.1.256.g2539c.dirty
^ permalink raw reply related
* Re: git grep doesn't follow symbolic link
From: Thomas Rast @ 2012-01-10 10:00 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Bertrand BENOIT, git
In-Reply-To: <CALkWK0=-LZH4MYhX50v-RWpGA2r+6q50YxsKaOxc0mJ__yuK7g@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Hi Bertrand,
>
> Bertrand BENOIT wrote:
>> When using git grep, symbolic links are not followed.
>> Is it a wanted behavior ?
>
> I'd imagine so: symbolic links are not portable across different file
> systems; Git's internal representation of a symbolic link is a file
> containing the path of the file to be linked to.
I'd actually welcome a fix to this general area, for an entirely
different reason. With bash and ordinary diff I can do things like
diff -u <(ls) <(cd elsewhere && ls) | less
But I lose all the cute features of git-diff. I *could* say
git diff --no-index <(ls) <(cd elsewhere && ls)
and it helpfully tells me
diff --git 1/dev/fd/63 2/dev/fd/62
index 55ccbe5..d796c45 120000
--- 1/dev/fd/63
+++ 2/dev/fd/62
@@ -1 +1 @@
-pipe:[607341]
\ No newline at end of file
+pipe:[607343]
\ No newline at end of file
Of course that's diff and not grep, but I think they suffer from the
same flaw: they share the file-kind handling logic of the rest of git in
a case where it's not very helpful.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [bug] submodule --update insufficiently verbose
From: Thomas Rast @ 2012-01-10 10:43 UTC (permalink / raw)
To: Dave Abrahams; +Cc: Git Mailing List
In-Reply-To: <m239bqnctf.fsf@pluto.luannocracy.com>
Dave Abrahams <dave@boostpro.com> writes:
> When a "git submodule --update" fails, e.g., due to a dirty working
> directory in one of the submodules, nothing is printed out indicating
> the submodule or directory in which the failure occurred. This seems
> like a usability bug to me.
I assume you mean 'git submodule update'. What git version is this, and
how do you reproduce? I get a message like
error: Your local changes to the following files would be overwritten by checkout:
foo
Please, commit your changes or stash them before you can switch branches.
Aborting
Unable to checkout '1fac3675a8c65b14e0a98e0136b5c43db97706fe' in submodule path 'sub'
in a simple test. I am using a development version, but a brief glance
at the history seems to indictate that the 'Unable to ... in submodule
...' has always been there.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH v2 2/3] index-pack: eliminate recursion in find_unresolved_deltas
From: Nguyen Thai Ngoc Duy @ 2012-01-10 12:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vzkdwcys4.fsf@alter.siamese.dyndns.org>
2012/1/10 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> I find both the original and the updated code rather dense to read without
> annotation, but from a cursory look all changes look good.
Maybe I stared at it for too long it seems obvious to me (hence no
further description in commit message). Let me describe it (and put in
commit message later if it makes sense)
Current code already links all bases together in a form of tree, using
struct base_data, with prev_base pointer to point to parent node. The
only problem is that struct base_data is all allocated on stack. So we
need to put all on heap (parse_pack_objects and
fix_unresolved_deltas). After that, it's simple depth-first traversal
where each node also maintains its own state (ofs and ref indices to
iterate over all children nodes).
So we process one node:
- if it returns a new (child) node (a parent base), we link it to our
tree, then process the new node.
- if it returns nothing, the node is done, free it. We go back to
parent node and resume whatever it's doing.
and do it until we have no nodes to process.
--
Duy
^ permalink raw reply
* Re: [PATCH v2 3/3] index-pack: eliminate unlimited recursion in get_delta_base()
From: Nguyen Thai Ngoc Duy @ 2012-01-10 13:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <7vvcokcwvt.fsf@alter.siamese.dyndns.org>
2012/1/10 Junio C Hamano <gitster@pobox.com>:
> In parse_pack_objects(), we have two passes. The first pass scans to
> enumerate the objects that appear in the pack and sift them into base and
> delta objects, and the second one starts from a base object, resolves its
> immediate children with find_unresolved_daltas(), but that function recurses
> many times, bound only by the number of objects in the pack, which is the
> issue you are trying to address with this series.
>
> I wonder if a cleaner approach is to change the loop in the second pass in
> such a way that (1) the function it calls resolves _only_ the immediate
> children of the object we know its final shape (either because the object
> was recorded in the deflated form in the pack, or we have already resolved
> it in earlier iteration), and (2) the loop goes over the objects[] array
> not just once, but until we stopped making progress.
So basically:
- remove the recursion code from get_base_data()
- update find_unresolved_deltas() to mark resolved objects handled
- put find_unresolve_deltas() call into a loop to go through all
unhandled objects
correct? Yeah I think it's simpler than current code.
> It would require us to be able to tell, by looking at objects[i], if the
> object itself has already been handled (perhaps you can look at its
> idx.sha1 field for this purpose) and if we have already handled its
> immediate delta children (you may need to add a bit to struct object_entry
> for this).
--
Duy
^ permalink raw reply
* Re: [PATCH v2 3/3] index-pack: eliminate unlimited recursion in get_delta_base()
From: Nguyen Thai Ngoc Duy @ 2012-01-10 13:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Shawn O. Pearce
In-Reply-To: <CACsJy8A-FOpjDeTpxMze7jouceWMJHND_V2fyV5pLNKF8xp8kQ@mail.gmail.com>
On Tue, Jan 10, 2012 at 8:03 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> correct? Yeah I think it's simpler than current code.
I wrote about core.deltabasecachelimit, then deleted it, thinking it
should be ok, but how do handle the case when you use up cache limit?
Some handled objects may be freed when we're short on cache and need
to be resolved again before we could resolve an unhandled object.
Assume we're at the limit, have resolved A, which has two children B
and C. When we resolve B we need to free A to keep it fit in cache. By
the time we look at C, A needs to be resolved again. Without tracing
back (i.e. what get_base_data() does), we don't know what we need to
re-resolve among all handled objects.
--
Duy
^ permalink raw reply
* Re: Please support add -p with a new file, to add only part of the file
From: Thomas Rast @ 2012-01-10 14:07 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Josh Triplett, git, Wincent Colaiuta, Jeff King
In-Reply-To: <20120109204721.GC23825@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Josh Triplett wrote:
>
>> I recently found myself with a new file that I needed to check in part
>> of with several commits. I wanted to use "git add -p newfile" and use
>> 'e' to add and commit several times (along with corresponding bits in
>> existing files). However, "git add -p" does not work on a new file,
>> only an existing file.
>
> Yep. A workaround is to use "git add -N newfile" before running
> "git add -p newfile".
>
> I imagine "git add -p '*.c'" should also offer to add hunks from
> source files that git doesn't know about yet, too.
>
> Here's a quick demo (untested) that might _almost_ do the right thing.
> Unfortunately it leaves intent-to-add entries around even for files
> the operator rejects. Anyway, maybe it can be a good starting point
> for playing around.
I think a proper solution needs a way to generate a diff that includes
all of the named files (even untracked). AFAICS there is no such
feature in the diff-* commands right now.
A not-so-proper solution might of course start by looking at which files
are untracked, and only run the 'git add -N' immediately before patch
application.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Another casualty
From: Chris Hatton @ 2012-01-10 14:08 UTC (permalink / raw)
To: git
In-Reply-To: <m3zkhe16bu.fsf@localhost.localdomain>
3 hours-worth of work just got whacked by this bug. Unimpressed :-(
Could have been worse I suppose. Thanks for the confirmation.
<Goes to update Git>
^ permalink raw reply
* Re: git-send-email: bug with sendemail.multiedit
From: Jean-Francois Dagenais @ 2012-01-10 14:23 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Pierre Habouzit, pierre.habouzit, git
In-Reply-To: <20120109225542.GB9902@sigill.intra.peff.net>
Tested with 0/1/true/false, all works as expected, i.e. fix is backward compatible.
git version 1.7.8.2
On Jan 9, 2012, at 17:55, Jeff King wrote:
> On Mon, Jan 09, 2012 at 02:09:30PM -0500, Jean-Francois Dagenais wrote:
>
>> I think there is a bug with git-send-email.perl's evaluation of the
>> sendemail.multiedit config variable.
>>
>> I was only able to make the "do_edit()" function detect it as false by
>> setting the variable to "0" instead of "false", like so:
>
> I think it's this:
>
> -- >8 --
> Subject: [PATCH] send-email: multiedit is a boolean config option
>
> The sendemail.multiedit variable is meant to be a boolean.
> However, it is not marked as such in the code, which means
> we store its value literally. Thus in the do_edit function,
> perl ends up coercing it to a boolean value according to
> perl rules, not git rules. This works for "0", but "false",
> "no", or "off" will erroneously be interpreted as true.
>
> Signed-off-by: Jeff King <peff@peff.net>
Tested-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
> ---
> git-send-email.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d491db9..ef30c55 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -210,6 +210,7 @@ my %config_bool_settings = (
> "signedoffbycc" => [\$signed_off_by_cc, undef],
> "signedoffcc" => [\$signed_off_by_cc, undef], # Deprecated
> "validate" => [\$validate, 1],
> + "multiedit" => [\$multiedit, undef]
> );
>
> my %config_settings = (
> @@ -227,7 +228,6 @@ my %config_settings = (
> "bcc" => \@bcclist,
> "suppresscc" => \@suppress_cc,
> "envelopesender" => \$envelope_sender,
> - "multiedit" => \$multiedit,
> "confirm" => \$confirm,
> "from" => \$sender,
> "assume8bitencoding" => \$auto_8bit_encoding,
> --
> 1.7.8
>
^ permalink raw reply
* Re: [PATCH] Documentation: rerere.enabled overrides [ -d rr-cache ]
From: Thomas Rast @ 2012-01-10 14:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk454gkh3.fsf@alter.siamese.dyndns.org>
Sorry I'm late, but...
On Fri, 6 Jan 2012 21:17:12 -0800, Junio C Hamano <gitster@pobox.com> wrote:
>
> Running "unset" is how to return the _variable_ to the previous state, but
> that is _not_ how to return to the previous state of the _repository_.
>
> Perhaps something like this in addition?
[...]
> encountered again. By default, linkgit:git-rerere[1] is
> enabled if there is an `rr-cache` directory under the
> - `$GIT_DIR`.
> + `$GIT_DIR`, e.g. if "rerere" was previously used in the
> + repository.
I don't feel strongly about it, but this is a really neat and concise
way to put it, so why not. Here's hoping I can save you some work:
---- 8< ---- 8< ----
From: Junio C Hamano <gitster@pobox.com>
Subject: Documentation: rerere's rr-cache auto-creation and rerere.enabled
The description of rerere.enabled left the user in the dark as to who
might create an rr-cache directory. Add a note that simply invoking
rerere does this.
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 04f5e19..c523c67 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1786,7 +1786,8 @@ rerere.enabled::
conflict hunks can be resolved automatically, should they be
encountered again. By default, linkgit:git-rerere[1] is
enabled if there is an `rr-cache` directory under the
- `$GIT_DIR`.
+ `$GIT_DIR`, e.g. if "rerere" was previously used in the
+ repository.
sendemail.identity::
A configuration identity. When given, causes values in the
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply related
* Re: [PATCH 6/6] sequencer: factor code out of revert builtin
From: Ramkumar Ramachandra @ 2012-01-10 15:21 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120108203800.GM1942@burratino>
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>> memset(&opts, 0, sizeof(opts));
>> opts.command = REPLAY_CMD_FOO;
>> opts.revisions = xmalloc(sizeof(*opts.revs));
>> parse_args_populate_opts(argc, argv, &opts);
>> init_revisions(opts.revs);
>> sequencer_pick_revisions(&opts);
>
> Hm, I wonder if opts.command should be a string so each new caller
> does not have to add to the enum and switch statements...
The new caller would have to add enum and switch statements to define
a new action anyway, so I think this should be an enum too.
-- Ram
^ permalink raw reply
* Re: [PATCH 5/6] revert: report fine-grained error messages from insn parser
From: Ramkumar Ramachandra @ 2012-01-10 15:24 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20120108213318.GQ1942@burratino>
Jonathan Nieder wrote:
> My "alternatively" was bogus --- lookup_commit_reference takes a (raw)
> full commit name as its argument.
>
> I dunno. The question was not actually rhetorical --- I just meant
> that it's worth thinking about these cases. There are a few cases:
>
> - missing object
> - object is present but corrupt
> - object is a blob, not a commit
>
> In the second case, there's an error message printed describing the
> problem, but in the other two there isn't. The other callers tend to
> say "not a valid <foo>" or "could not lookup commit <foo>, so I guess
>
> error: .git/sequencer/todo:5: not a valid commit: 78a89f493
>
> would be fine.
>
> Except that this focusses on the .git/sequencer/todo filename which
> would leave the person debugging astray. It is not that
> .git/sequencer/todo contains a typo (that would have been caught by
> get_sha1), but that it referred to a bad object or non-commit. Maybe
> something in the direction of
>
> error: cannot pick 78a89f493 because it is not a valid commit
>
> would be more helpful.
>
> Is this the right moment to report that error? Will the operator be
> happy that we errored out right away before cherry-picking anything
> and wasting the human's time in assisting with that process, or will
> she be unhappy that inability to do something later that she might
> have been planning on skipping anyway prevented making progress right
> away? (I'm not sure what the best thing to do is --- I guess some
> advice like
>
> hint: to abort, use cherry-pick --abort
> hint: to skip or replace that commit, use cherry-pick --edit
>
> would help.)
Ignoring this in the re-roll: I'd be inclined to put these changes in
a separate series that begins with bolting on some advice
configuration to the sequencer.
Thanks for thinking about these things.
-- Ram
^ permalink raw reply
* Re: Auto-refresh git-gui
From: Uri Okrent @ 2012-01-10 16:14 UTC (permalink / raw)
To: victor.engmark; +Cc: git
In-Reply-To: <20120105080322.GD3484@victor>
Not to muddy the waters, but if you're open to alternatives, if you
have inotify installed, git cola will automatically update it's status
whenever files in your repository change.
--
Uri
Please consider the environment before printing this message.
http://wwf.panda.org/savepaper/
^ permalink raw reply
* [PATCH v2 0/8] The move to sequencer.c
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326025653-11922-1-git-send-email-artagnon@gmail.com>
Hi,
The big changes in this round are:
1. Dropped "revert: don't let revert continue a cherry-pick" from the
last round after a quick discussion with Jonathan.
2. Separated out "revert: separate out parse errors logically" from
"revert: report fine-grained errors from insn parser". Definitely
looks clearer.
3. Improved "revert: report fine-grained errors from insn parser" by
eliminating repetition. 20 is a bit arbitrary, but it looks pretty
enough on my terminal.
4. Added "sha1_name: introduce getn_sha1() to take length" and
"revert: use getn_sha1() to simplify insn parsing". I'm happy with
them. Name is inspired from the strncmp() variant of strcmp().
5. Included minimal API documentation with "sequencer: factor code out
of revert builtin".
Thanks for reading. I think I'll work on fixing the memory leaks now.
Ramkumar Ramachandra (8):
revert: prepare to move replay_action to header
revert: decouple sequencer actions from builtin commands
revert: allow mixing "pick" and "revert" actions
revert: separate out parse errors logically
revert: report fine-grained errors from insn parser
sha1_name: introduce getn_sha1() to take length
revert: use getn_sha1() to simplify insn parsing
sequencer: factor code out of revert builtin
Documentation/technical/api-sequencer.txt | 22 +
builtin/revert.c | 958 +----------------------------
cache.h | 1 +
sequencer.c | 925 ++++++++++++++++++++++++++++-
sequencer.h | 48 ++
sha1_name.c | 23 +-
t/t3510-cherry-pick-sequence.sh | 46 +-
7 files changed, 1056 insertions(+), 967 deletions(-)
create mode 100644 Documentation/technical/api-sequencer.txt
--
1.7.8.2
^ permalink raw reply
* [PATCH 1/8] revert: prepare to move replay_action to header
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Later in the series, we will build a generalized sequencer by
factoring code out of the revert builtin, and leaving it with just
argument parsing work. This involves moving "replay_action" to
sequencer.h, so that both sequencer.c and builtin/revert.c can use it.
Unfortunately, "REVERT" and "CHERRY_PICK" are unsuitable variable
names, as their purpose is unclear in the global context; in
anticipation of the move, rename them to "REPLAY_REVERT" and
"REPLAY_PICK" respectively.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 40 ++++++++++++++++++++++------------------
1 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0d8020c..2739405 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,11 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action { REVERT, CHERRY_PICK };
+enum replay_action {
+ REPLAY_REVERT,
+ REPLAY_PICK
+};
+
enum replay_subcommand {
REPLAY_NONE,
REPLAY_REMOVE_STATE,
@@ -74,14 +78,14 @@ struct replay_opts {
static const char *action_name(const struct replay_opts *opts)
{
- return opts->action == REVERT ? "revert" : "cherry-pick";
+ return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
}
static char *get_encoding(const char *message);
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
- return opts->action == REVERT ? revert_usage : cherry_pick_usage;
+ return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
}
static int option_parse_x(const struct option *opt,
@@ -160,7 +164,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
OPT_END(),
};
- if (opts->action == CHERRY_PICK) {
+ if (opts->action == REPLAY_PICK) {
struct option cp_extra[] = {
OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -374,7 +378,7 @@ static int error_dirty_index(struct replay_opts *opts)
return error_resolve_conflict(action_name(opts));
/* Different translation strings for cherry-pick and revert */
- if (opts->action == CHERRY_PICK)
+ if (opts->action == REPLAY_PICK)
error(_("Your local changes would be overwritten by cherry-pick."));
else
error(_("Your local changes would be overwritten by revert."));
@@ -553,7 +557,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REVERT) {
+ if (opts->action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -594,7 +598,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -618,13 +622,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->action == CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (opts->action == REVERT && ((opts->no_commit && res == 0) || res == 1))
+ if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->action == REVERT
+ error(opts->action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -644,7 +648,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
static void prepare_revs(struct replay_opts *opts)
{
- if (opts->action != REVERT)
+ if (opts->action != REPLAY_REVERT)
opts->revs->reverse ^= 1;
if (prepare_revision_walk(opts->revs))
@@ -701,7 +705,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
{
struct commit_list *cur = NULL;
const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REVERT ? "revert" : "pick";
+ const char *action_str = opts->action == REPLAY_REVERT ? "revert" : "pick";
const char *subject;
int subject_len;
@@ -722,10 +726,10 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
int saved, status, padding;
if (!prefixcmp(bol, "pick")) {
- action = CHERRY_PICK;
+ action = REPLAY_PICK;
bol += strlen("pick");
} else if (!prefixcmp(bol, "revert")) {
- action = REVERT;
+ action = REPLAY_REVERT;
bol += strlen("revert");
} else
return NULL;
@@ -748,7 +752,7 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
*/
if (action != opts->action) {
const char *action_str;
- action_str = action == REVERT ? "revert" : "cherry-pick";
+ action_str = action == REPLAY_REVERT ? "revert" : "cherry-pick";
error(_("Cannot %s during a %s"), action_str, action_name(opts));
return NULL;
}
@@ -1124,7 +1128,7 @@ static int pick_revisions(struct replay_opts *opts)
if (create_seq_dir() < 0)
return -1;
if (get_sha1("HEAD", sha1)) {
- if (opts->action == REVERT)
+ if (opts->action == REPLAY_REVERT)
return error(_("Can't revert as initial commit"));
return error(_("Can't cherry-pick into empty head"));
}
@@ -1141,7 +1145,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
- opts.action = REVERT;
+ opts.action = REPLAY_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
@@ -1156,7 +1160,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
int res;
memset(&opts, 0, sizeof(opts));
- opts.action = CHERRY_PICK;
+ opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
--
1.7.8.2
^ permalink raw reply related
* [PATCH 2/8] revert: decouple sequencer actions from builtin commands
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Currently, 'git cherry-pick' fills up the '.git/sequencer/todo'
instruction sheet with "pick" actions, while 'git revert' fills it up
with "revert" actions. Inspired by the way 'rebase -i' works, we
would like to permit mixing arbitrary actions in the same instruction
sheet. To do this, we first have to decouple the notion of an action
in the instruction sheet from builtin commands. While a future
instruction sheet would look like:
pickle next~4
action3 b74fea
revert rr/moo^2~34
the actions "pickle", "action3" and "revert" need not necessarily
correspond to the specific builtins. Introduce a "replay_command",
and replace instances of "replay_action" with "replay_command"
everywhere except in parse_insn_line(), the function that parses the
instruction sheet.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 63 ++++++++++++++++++++++++++++++------------------------
1 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 2739405..9bca9c7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -44,6 +44,11 @@ enum replay_action {
REPLAY_PICK
};
+enum replay_command {
+ REPLAY_CMD_REVERT,
+ REPLAY_CMD_CHERRY_PICK
+};
+
enum replay_subcommand {
REPLAY_NONE,
REPLAY_REMOVE_STATE,
@@ -52,7 +57,7 @@ enum replay_subcommand {
};
struct replay_opts {
- enum replay_action action;
+ enum replay_command command;
enum replay_subcommand subcommand;
/* Boolean options */
@@ -76,16 +81,16 @@ struct replay_opts {
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
-static const char *action_name(const struct replay_opts *opts)
+static const char *command_name(struct replay_opts *opts)
{
- return opts->action == REPLAY_REVERT ? "revert" : "cherry-pick";
+ return opts->command == REPLAY_CMD_REVERT ? "revert" : "cherry-pick";
}
static char *get_encoding(const char *message);
static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)
{
- return opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;
+ return opts->command == REPLAY_CMD_REVERT ? revert_usage : cherry_pick_usage;
}
static int option_parse_x(const struct option *opt,
@@ -142,7 +147,7 @@ static void verify_opt_mutually_compatible(const char *me, ...)
static void parse_args(int argc, const char **argv, struct replay_opts *opts)
{
const char * const * usage_str = revert_or_cherry_pick_usage(opts);
- const char *me = action_name(opts);
+ const char *me = command_name(opts);
int remove_state = 0;
int contin = 0;
int rollback = 0;
@@ -164,7 +169,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
OPT_END(),
};
- if (opts->action == REPLAY_PICK) {
+ if (opts->command == REPLAY_CMD_CHERRY_PICK) {
struct option cp_extra[] = {
OPT_BOOLEAN('x', NULL, &opts->record_origin, "append commit name"),
OPT_BOOLEAN(0, "ff", &opts->allow_ff, "allow fast-forward"),
@@ -375,10 +380,10 @@ static struct tree *empty_tree(void)
static int error_dirty_index(struct replay_opts *opts)
{
if (read_cache_unmerged())
- return error_resolve_conflict(action_name(opts));
+ return error_resolve_conflict(command_name(opts));
/* Different translation strings for cherry-pick and revert */
- if (opts->action == REPLAY_PICK)
+ if (opts->command == REPLAY_CMD_CHERRY_PICK)
error(_("Your local changes would be overwritten by cherry-pick."));
else
error(_("Your local changes would be overwritten by revert."));
@@ -434,7 +439,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
(write_cache(index_fd, active_cache, active_nr) ||
commit_locked_index(&index_lock)))
/* TRANSLATORS: %s will be "revert" or "cherry-pick" */
- die(_("%s: Unable to write new index file"), action_name(opts));
+ die(_("%s: Unable to write new index file"), command_name(opts));
rollback_lock_file(&index_lock);
if (!clean) {
@@ -542,7 +547,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
/* TRANSLATORS: The first %s will be "revert" or
"cherry-pick", the second %s a SHA1 */
return error(_("%s: cannot parse parent commit %s"),
- action_name(opts), sha1_to_hex(parent->object.sha1));
+ command_name(opts), sha1_to_hex(parent->object.sha1));
if (get_message(commit, &msg) != 0)
return error(_("Cannot get commit message for %s"),
@@ -557,7 +562,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REPLAY_REVERT) {
+ if (opts->command == REPLAY_CMD_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -598,7 +603,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->action == REPLAY_REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->command == REPLAY_CMD_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -622,13 +627,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ if (opts->command == REPLAY_CMD_CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (opts->action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
+ if (opts->command == REPLAY_CMD_REVERT && ((opts->no_commit && res == 0) || res == 1))
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->action == REPLAY_REVERT
+ error(opts->command == REPLAY_CMD_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -648,7 +653,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
static void prepare_revs(struct replay_opts *opts)
{
- if (opts->action != REPLAY_REVERT)
+ if (opts->command != REPLAY_CMD_REVERT)
opts->revs->reverse ^= 1;
if (prepare_revision_walk(opts->revs))
@@ -663,12 +668,13 @@ static void read_and_refresh_cache(struct replay_opts *opts)
static struct lock_file index_lock;
int index_fd = hold_locked_index(&index_lock, 0);
if (read_index_preload(&the_index, NULL) < 0)
- die(_("git %s: failed to read the index"), action_name(opts));
+ die(_("git %s: failed to read the index"), command_name(opts));
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
if (the_index.cache_changed) {
if (write_index(&the_index, index_fd) ||
commit_locked_index(&index_lock))
- die(_("git %s: failed to refresh the index"), action_name(opts));
+ die(_("git %s: failed to refresh the index"),
+ command_name(opts));
}
rollback_lock_file(&index_lock);
}
@@ -705,7 +711,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
{
struct commit_list *cur = NULL;
const char *sha1_abbrev = NULL;
- const char *action_str = opts->action == REPLAY_REVERT ? "revert" : "pick";
+ const char *action_str = opts->command == REPLAY_CMD_REVERT ? "revert" : "pick";
const char *subject;
int subject_len;
@@ -750,10 +756,11 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
* Verify that the action matches up with the one in
* opts; we don't support arbitrary instructions
*/
- if (action != opts->action) {
- const char *action_str;
- action_str = action == REPLAY_REVERT ? "revert" : "cherry-pick";
- error(_("Cannot %s during a %s"), action_str, action_name(opts));
+ if ((action == REPLAY_PICK && opts->command == REPLAY_CMD_REVERT) ||
+ (action == REPLAY_REVERT && opts->command == REPLAY_CMD_CHERRY_PICK)) {
+ error(_("Cannot %s during a %s"),
+ action == REPLAY_REVERT ? "revert" : "pick",
+ command_name(opts));
return NULL;
}
@@ -1015,7 +1022,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
struct commit_list *cur;
int res;
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || opts->edit));
@@ -1070,7 +1077,7 @@ static int sequencer_continue(struct replay_opts *opts)
static int single_pick(struct commit *cmit, struct replay_opts *opts)
{
- setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
+ setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
return do_pick_commit(cmit, opts);
}
@@ -1128,7 +1135,7 @@ static int pick_revisions(struct replay_opts *opts)
if (create_seq_dir() < 0)
return -1;
if (get_sha1("HEAD", sha1)) {
- if (opts->action == REPLAY_REVERT)
+ if (opts->command == REPLAY_CMD_REVERT)
return error(_("Can't revert as initial commit"));
return error(_("Can't cherry-pick into empty head"));
}
@@ -1145,7 +1152,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
- opts.action = REPLAY_REVERT;
+ opts.command = REPLAY_CMD_REVERT;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
@@ -1160,7 +1167,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
int res;
memset(&opts, 0, sizeof(opts));
- opts.action = REPLAY_PICK;
+ opts.command = REPLAY_CMD_CHERRY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
res = pick_revisions(&opts);
--
1.7.8.2
^ permalink raw reply related
* [PATCH 3/8] revert: allow mixing "pick" and "revert" actions
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Parse the instruction sheet in '.git/sequencer/todo' as a list of
(action, operand) pairs, instead of assuming that all lines have the
same action. Now, an instruction sheet like the following is
perfectly valid:
pick fdc0b12 picked
revert 965fed4 anotherpick
The operator can use this feature by hand-editing the instruction
sheet and using '--continue' as appropriate:
$ git cherry-pick foo..bar
[conflict occurs]
$ edit problematicfile
$ git add problematicfile
$ edit .git/sequencer/todo
$ git revert --continue
[finishes successfully]
Consequently, this means that a 'git cherry-pick --continue' can
continue an ongoing 'git revert' operation, and viceversa.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 142 ++++++++++++++++++++-------------------
t/t3510-cherry-pick-sequence.sh | 46 +++++++++----
2 files changed, 105 insertions(+), 83 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 9bca9c7..1841ffa 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -56,6 +56,12 @@ enum replay_subcommand {
REPLAY_ROLLBACK
};
+struct replay_insn_list {
+ struct commit *operand;
+ enum replay_action action;
+ struct replay_insn_list *next;
+};
+
struct replay_opts {
enum replay_command command;
enum replay_subcommand subcommand;
@@ -487,7 +493,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)
return run_command_v_opt(args, RUN_GIT_CMD);
}
-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
+static int do_pick_commit(struct commit *commit, enum replay_action action,
+ struct replay_opts *opts)
{
unsigned char head[20];
struct commit *base, *next, *parent;
@@ -562,7 +569,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->command == REPLAY_CMD_REVERT) {
+ if (action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -603,7 +610,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
}
- if (!opts->strategy || !strcmp(opts->strategy, "recursive") || opts->command == REPLAY_CMD_REVERT) {
+ if (!opts->strategy || !strcmp(opts->strategy, "recursive") || action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -627,13 +634,13 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
* However, if the merge did not even start, then we don't want to
* write it at all.
*/
- if (opts->command == REPLAY_CMD_CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))
+ if (action == REPLAY_PICK && !opts->no_commit && (res == 0 || res == 1))
write_cherry_pick_head(commit, "CHERRY_PICK_HEAD");
- if (opts->command == REPLAY_CMD_REVERT && ((opts->no_commit && res == 0) || res == 1))
+ if (action == REPLAY_REVERT && ((opts->no_commit && res == 0) || res == 1))
write_cherry_pick_head(commit, "REVERT_HEAD");
if (res) {
- error(opts->command == REPLAY_CMD_REVERT
+ error(action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -680,70 +687,70 @@ static void read_and_refresh_cache(struct replay_opts *opts)
}
/*
- * Append a commit to the end of the commit_list.
+ * Append a (commit, action) to the end of the replay_insn_list.
*
* next starts by pointing to the variable that holds the head of an
- * empty commit_list, and is updated to point to the "next" field of
- * the last item on the list as new commits are appended.
+ * empty replay_insn_list, and is updated to point to the "next" field of
+ * the last item on the list as new (commit, action) pairs are appended.
*
* Usage example:
*
- * struct commit_list *list;
- * struct commit_list **next = &list;
+ * struct replay_insn_list *list;
+ * struct replay_insn_list **next = &list;
*
- * next = commit_list_append(c1, next);
- * next = commit_list_append(c2, next);
- * assert(commit_list_count(list) == 2);
+ * next = replay_insn_list_append(c1, a1, next);
+ * next = replay_insn_list_append(c2, a2, next);
+ * assert(len(list) == 2);
* return list;
*/
-static struct commit_list **commit_list_append(struct commit *commit,
- struct commit_list **next)
+static struct replay_insn_list **replay_insn_list_append(struct commit *operand,
+ enum replay_action action,
+ struct replay_insn_list **next)
{
- struct commit_list *new = xmalloc(sizeof(struct commit_list));
- new->item = commit;
+ struct replay_insn_list *new = xmalloc(sizeof(*new));
+ new->action = action;
+ new->operand = operand;
*next = new;
new->next = NULL;
return &new->next;
}
-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
- struct replay_opts *opts)
+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
{
- struct commit_list *cur = NULL;
- const char *sha1_abbrev = NULL;
- const char *action_str = opts->command == REPLAY_CMD_REVERT ? "revert" : "pick";
- const char *subject;
- int subject_len;
+ struct replay_insn_list *cur;
for (cur = todo_list; cur; cur = cur->next) {
- sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- subject_len = find_commit_subject(cur->item->buffer, &subject);
+ const char *sha1_abbrev, *action_str, *subject;
+ int subject_len;
+
+ action_str = cur->action == REPLAY_REVERT ? "revert" : "pick";
+ sha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);
+ subject_len = find_commit_subject(cur->operand->buffer, &subject);
strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
subject_len, subject);
}
return 0;
}
-static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
+static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
{
unsigned char commit_sha1[20];
- enum replay_action action;
char *end_of_object_name;
int saved, status, padding;
if (!prefixcmp(bol, "pick")) {
- action = REPLAY_PICK;
+ item->action = REPLAY_PICK;
bol += strlen("pick");
} else if (!prefixcmp(bol, "revert")) {
- action = REPLAY_REVERT;
+ item->action = REPLAY_REVERT;
bol += strlen("revert");
} else
- return NULL;
+ return -1;
/* Eat up extra spaces/ tabs before object name */
padding = strspn(bol, " \t");
if (!padding)
- return NULL;
+ return -1;
bol += padding;
end_of_object_name = bol + strcspn(bol, " \t\n");
@@ -752,38 +759,29 @@ static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *
status = get_sha1(bol, commit_sha1);
*end_of_object_name = saved;
- /*
- * Verify that the action matches up with the one in
- * opts; we don't support arbitrary instructions
- */
- if ((action == REPLAY_PICK && opts->command == REPLAY_CMD_REVERT) ||
- (action == REPLAY_REVERT && opts->command == REPLAY_CMD_CHERRY_PICK)) {
- error(_("Cannot %s during a %s"),
- action == REPLAY_REVERT ? "revert" : "pick",
- command_name(opts));
- return NULL;
- }
-
if (status < 0)
- return NULL;
+ return -1;
+
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return -1;
- return lookup_commit_reference(commit_sha1);
+ item->next = NULL;
+ return 0;
}
-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
- struct replay_opts *opts)
+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
{
- struct commit_list **next = todo_list;
- struct commit *commit;
+ struct replay_insn_list **next = todo_list;
+ struct replay_insn_list item = { NULL, 0, NULL };
char *p = buf;
int i;
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- commit = parse_insn_line(p, eol, opts);
- if (!commit)
+ if (parse_insn_line(p, eol, &item))
return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(item.operand, item.action, next);
p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
@@ -791,8 +789,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
return 0;
}
-static void read_populate_todo(struct commit_list **todo_list,
- struct replay_opts *opts)
+static void read_populate_todo(struct replay_insn_list **todo_list)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
struct strbuf buf = STRBUF_INIT;
@@ -808,7 +805,7 @@ static void read_populate_todo(struct commit_list **todo_list,
}
close(fd);
- res = parse_insn_buffer(buf.buf, todo_list, opts);
+ res = parse_insn_buffer(buf.buf, todo_list);
strbuf_release(&buf);
if (res)
die(_("Unusable instruction sheet: %s"), todo_file);
@@ -857,17 +854,19 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
die(_("Malformed options sheet: %s"), opts_file);
}
-static void walk_revs_populate_todo(struct commit_list **todo_list,
+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
struct replay_opts *opts)
{
struct commit *commit;
- struct commit_list **next;
+ struct replay_insn_list **next;
+ enum replay_action action;
prepare_revs(opts);
next = todo_list;
+ action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
while ((commit = get_revision(opts->revs)))
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(commit, action, next);
}
static int create_seq_dir(void)
@@ -965,7 +964,7 @@ fail:
return -1;
}
-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
+static void save_todo(struct replay_insn_list *todo_list)
{
const char *todo_file = git_path(SEQ_TODO_FILE);
static struct lock_file todo_lock;
@@ -973,7 +972,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
int fd;
fd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);
- if (format_todo(&buf, todo_list, opts) < 0)
+ if (format_todo(&buf, todo_list) < 0)
die(_("Could not format %s."), todo_file);
if (write_in_full(fd, buf.buf, buf.len) < 0) {
strbuf_release(&buf);
@@ -1017,9 +1016,9 @@ static void save_opts(struct replay_opts *opts)
}
}
-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
+static int pick_commits(struct replay_insn_list *todo_list, struct replay_opts *opts)
{
- struct commit_list *cur;
+ struct replay_insn_list *cur;
int res;
setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
@@ -1029,8 +1028,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
read_and_refresh_cache(opts);
for (cur = todo_list; cur; cur = cur->next) {
- save_todo(cur, opts);
- res = do_pick_commit(cur->item, opts);
+ save_todo(cur);
+ res = do_pick_commit(cur->operand, cur->action, opts);
if (res)
return res;
}
@@ -1055,12 +1054,12 @@ static int continue_single_pick(void)
static int sequencer_continue(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct replay_insn_list *todo_list = NULL;
if (!file_exists(git_path(SEQ_TODO_FILE)))
return continue_single_pick();
read_populate_opts(&opts);
- read_populate_todo(&todo_list, opts);
+ read_populate_todo(&todo_list);
/* Verify that the conflict has been resolved */
if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
@@ -1077,13 +1076,16 @@ static int sequencer_continue(struct replay_opts *opts)
static int single_pick(struct commit *cmit, struct replay_opts *opts)
{
+ enum replay_action action;
+ action = opts->command == REPLAY_CMD_REVERT ? REPLAY_REVERT : REPLAY_PICK;
+
setenv(GIT_REFLOG_ACTION, command_name(opts), 0);
- return do_pick_commit(cmit, opts);
+ return do_pick_commit(cmit, action, opts);
}
static int pick_revisions(struct replay_opts *opts)
{
- struct commit_list *todo_list = NULL;
+ struct replay_insn_list *todo_list = NULL;
unsigned char sha1[20];
if (opts->subcommand == REPLAY_NONE)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 97f3710..af747c8 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -454,7 +454,7 @@ test_expect_success 'sign-off needs to be reaffirmed after conflict resolution,
! grep Signed-off-by: msg
'
-test_expect_success 'malformed instruction sheet 1' '
+test_expect_success 'malformed instruction sheet, action' '
pristine_detach initial &&
test_expect_code 1 git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
@@ -465,23 +465,12 @@ test_expect_success 'malformed instruction sheet 1' '
test_expect_code 128 git cherry-pick --continue
'
-test_expect_success 'malformed instruction sheet 2' '
- pristine_detach initial &&
- test_expect_code 1 git cherry-pick base..anotherpick &&
- echo "resolved" >foo &&
- git add foo &&
- git commit &&
- sed "s/pick/revert/" .git/sequencer/todo >new_sheet &&
- cp new_sheet .git/sequencer/todo &&
- test_expect_code 128 git cherry-pick --continue
-'
-
test_expect_success 'empty commit set' '
pristine_detach initial &&
test_expect_code 128 git cherry-pick base..base
'
-test_expect_success 'malformed instruction sheet 3' '
+test_expect_success 'malformed instruction sheet, object name' '
pristine_detach initial &&
test_expect_code 1 git cherry-pick base..anotherpick &&
echo "resolved" >foo &&
@@ -517,4 +506,35 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'
+test_expect_success 'mixed pick and revert instructions' '
+ pristine_detach initial &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ oldsha=`git rev-parse --short HEAD~1` &&
+ echo "revert $oldsha unrelatedpick" >>.git/sequencer/todo &&
+ git revert --continue &&
+ test_path_is_missing .git/sequencer &&
+ {
+ git rev-list HEAD |
+ git diff-tree --root --stdin |
+ sed "s/$_x40/OBJID/g"
+ } >actual &&
+ cat >expect <<-\EOF &&
+ OBJID
+ :100644 100644 OBJID OBJID M unrelated
+ 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_cmp expect actual
+'
+
test_done
--
1.7.8.2
^ permalink raw reply related
* [PATCH 4/8] revert: separate out parse errors logically
From: Ramkumar Ramachandra @ 2012-01-10 16:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1326212039-13806-1-git-send-email-artagnon@gmail.com>
Three kinds of errors can arise from parsing the instruction sheet:
1. Unrecognized action
2. Malformed object name
3. Object name does not refer to a valid commit
The next patch makes an attempt to make the parser report meaningful
errors by replacing the "return -1" with "return error(...)"
appropriately. For the first kind of error, it is insufficient to
check if the buffer beings with a "pick" or "revert", otherwise the
following insn sheet would be interpreted as having a malformed object
name:
pickle a1fe57~2
In reality, the issue is that "pickle" is an unrecognized instruction.
So, check that the buffer starts with ("pick " or "pick\t") and
("revert " or "revert\t").
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 15 ++++++---------
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 1841ffa..9a09471 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -736,22 +736,19 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
{
unsigned char commit_sha1[20];
char *end_of_object_name;
- int saved, status, padding;
+ int saved, status;
- if (!prefixcmp(bol, "pick")) {
+ if (!prefixcmp(bol, "pick ") || !prefixcmp(bol, "pick\t")) {
item->action = REPLAY_PICK;
- bol += strlen("pick");
- } else if (!prefixcmp(bol, "revert")) {
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ") || !prefixcmp(bol, "revert\t")) {
item->action = REPLAY_REVERT;
- bol += strlen("revert");
+ bol += strlen("revert ");
} else
return -1;
/* Eat up extra spaces/ tabs before object name */
- padding = strspn(bol, " \t");
- if (!padding)
- return -1;
- bol += padding;
+ bol += strspn(bol, " \t");
end_of_object_name = bol + strcspn(bol, " \t\n");
saved = *end_of_object_name;
--
1.7.8.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox