* [PATCH 7/9] revert: allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-12-09 15:42 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1323445326-24637-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 instructions use
the same action. Now you can do:
pick fdc0b12 picked
revert 965fed4 anotherpick
For cherry-pick and revert, this means that a 'git cherry-pick
--continue' can continue an ongoing revert operation and viceversa.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 133 +++++++++++++++++++--------------------
t/t3510-cherry-pick-sequence.sh | 58 +++++++++++++++++
2 files changed, 124 insertions(+), 67 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 86af516..cc55823 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,7 @@ 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,
@@ -47,6 +47,12 @@ enum replay_subcommand {
REPLAY_ROLLBACK
};
+struct replay_insn_list {
+ enum replay_action action;
+ struct commit *operand;
+ struct replay_insn_list *next;
+};
+
struct replay_opts {
enum replay_action action;
enum replay_subcommand subcommand;
@@ -73,14 +79,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,
@@ -159,7 +165,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"),
@@ -363,7 +369,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."));
@@ -467,7 +473,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;
@@ -542,7 +549,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
defmsg = git_pathdup("MERGE_MSG");
- if (opts->action == REVERT) {
+ if (action == REPLAY_REVERT) {
base = commit;
base_label = msg.label;
next = parent;
@@ -583,7 +590,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") || action == REPLAY_REVERT) {
res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
write_message(&msgbuf, defmsg);
@@ -607,13 +614,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(action == REPLAY_REVERT
? _("could not revert %s... %s")
: _("could not apply %s... %s"),
find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
@@ -637,7 +644,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)
init_revisions(revs, NULL);
revs->no_walk = 1;
- if (opts->action != REVERT)
+ if (opts->action != REPLAY_REVERT)
revs->reverse = 1;
argc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);
@@ -683,49 +690,49 @@ static void read_and_refresh_cache(struct replay_opts *opts)
* assert(commit_list_count(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(enum replay_action action,
+ struct commit *operand,
+ 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->action == 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;
if (!prefixcmp(bol, "pick ")) {
- action = CHERRY_PICK;
+ item->action = REPLAY_PICK;
bol += strlen("pick ");
} else if (!prefixcmp(bol, "revert ")) {
- action = REVERT;
+ item->action = REPLAY_REVERT;
bol += strlen("revert ");
} else
- return NULL;
+ return -1;
/* Eat up extra spaces/ tabs before object name */
while (*bol == ' ' || *bol == '\t')
@@ -737,37 +744,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 != opts->action) {
- const char *action_str;
- action_str = action == REVERT ? "revert" : "cherry-pick";
- error(_("Cannot %s during a %s"), action_str, action_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 = { 0, NULL, 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) < 0)
return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
@@ -775,8 +774,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;
@@ -792,7 +790,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);
@@ -841,18 +839,18 @@ 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 rev_info revs;
struct commit *commit;
- struct commit_list **next;
+ struct replay_insn_list **next;
prepare_revs(&revs, opts);
next = todo_list;
while ((commit = get_revision(&revs)))
- next = commit_list_append(commit, next);
+ next = replay_insn_list_append(opts->action, commit, next);
}
static int create_seq_dir(void)
@@ -950,7 +948,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;
@@ -958,7 +956,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);
@@ -1002,9 +1000,10 @@ 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, action_name(opts), 0);
@@ -1014,8 +1013,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) {
if (!cur->next)
/*
@@ -1040,7 +1039,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *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];
read_and_refresh_cache(opts);
@@ -1060,7 +1059,7 @@ static int pick_revisions(struct replay_opts *opts)
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);
+ read_populate_todo(&todo_list);
/* Verify that the conflict has been resolved */
if (!index_differs_from("HEAD", 0))
@@ -1078,7 +1077,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"));
}
@@ -1095,7 +1094,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);
@@ -1110,7 +1109,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);
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 70fd54b..aee869d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -356,4 +356,62 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'
+test_expect_success 'revert --continue continues after cherry-pick' '
+ pristine_detach initial &&
+ test_expect_code 1 git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ 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 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_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 cherry-pick --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.7.3
^ permalink raw reply related
* [PATCH 6/9] t3510 (cherry-pick-sequencer): remove malformed sheet 2
From: Ramkumar Ramachandra @ 2011-12-09 15:42 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1323445326-24637-1-git-send-email-artagnon@gmail.com>
The next patch allows mixing "pick" and "revert" instruction in the
same instruction sheet. By removing the "malformed instruction sheet
2" test in advance, it'll be easier to see the changes made by the
next patch.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3510-cherry-pick-sequence.sh | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 9ffc085..70fd54b 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -309,7 +309,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl
grep "Signed-off-by:" anotherpick_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 &&
@@ -320,18 +320,7 @@ 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 '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 &&
--
1.7.7.3
^ permalink raw reply related
* [PATCH 8/9] revert: report fine-grained error messages from insn parser
From: Ramkumar Ramachandra @ 2011-12-09 15:42 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1323445326-24637-1-git-send-email-artagnon@gmail.com>
Three kinds of errors can arise from parsing '.git/sequencer/todo':
1. Unrecognized action
2. Malformed object name
3. Object name does not refer to a valid commit
Since we would like to make the instruction sheet user-editable in the
future (much like the 'rebase -i' sheet), report more fine-grained
parse errors prefixed with the filename and line number.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/revert.c | 23 ++++++++++++++++-------
1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index cc55823..e2355d8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -719,8 +719,10 @@ static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)
return 0;
}
-static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
+static int parse_insn_line(char *bol, char *eol,
+ struct replay_insn_list *item, int lineno)
{
+ const char *todo_file = git_path(SEQ_TODO_FILE);
unsigned char commit_sha1[20];
char *end_of_object_name;
int saved, status;
@@ -731,8 +733,13 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
} else if (!prefixcmp(bol, "revert ")) {
item->action = REPLAY_REVERT;
bol += strlen("revert ");
- } else
- return -1;
+ } else {
+ size_t len = eol - bol;
+ if (len > 255)
+ len = 255;
+ return error(_("%s:%d: Unrecognized action: %.*s"),
+ todo_file, lineno, (int)len, bol);
+ }
/* Eat up extra spaces/ tabs before object name */
while (*bol == ' ' || *bol == '\t')
@@ -745,11 +752,13 @@ static int parse_insn_line(char *bol, char *eol, struct replay_insn_list *item)
*end_of_object_name = saved;
if (status < 0)
- return -1;
+ return error(_("%s:%d: Malformed object name: %s"),
+ todo_file, lineno, bol);
item->operand = lookup_commit_reference(commit_sha1);
if (!item->operand)
- return -1;
+ return error(_("%s:%d: Not a valid commit: %s"),
+ todo_file, lineno, bol);
item->next = NULL;
return 0;
@@ -764,8 +773,8 @@ static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)
for (i = 1; *p; i++) {
char *eol = strchrnul(p, '\n');
- if (parse_insn_line(p, eol, &item) < 0)
- return error(_("Could not parse line %d."), i);
+ if (parse_insn_line(p, eol, &item, i) < 0)
+ return -1;
next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
--
1.7.7.3
^ permalink raw reply related
* [PATCH 9/9] revert: simplify communicating command-line arguments
From: Ramkumar Ramachandra @ 2011-12-09 15:42 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder
In-Reply-To: <1323445326-24637-1-git-send-email-artagnon@gmail.com>
From: Jonathan Nieder <jrnieder@gmail.com>
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. So, 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 "struct rev_info".
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 53 +++++++++++++++++++++-----------------
t/t3510-cherry-pick-sequence.sh | 11 ++++++++
2 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index e2355d8..8d86bfd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -66,13 +66,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"
@@ -175,9 +176,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,
@@ -219,9 +220,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,
@@ -229,7 +227,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 = 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);
+ } else
+ opts->revs = NULL;
+
+ /* Forbid stray command-line arguments */
+ if (argc > 1)
+ usage_with_options(usage_str, options);
}
struct commit_message {
@@ -638,23 +649,15 @@ static int do_pick_commit(struct commit *commit, enum replay_action action,
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 != REPLAY_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"));
}
@@ -851,14 +854,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)
static void walk_revs_populate_todo(struct replay_insn_list **todo_list,
struct replay_opts *opts)
{
- struct rev_info revs;
struct commit *commit;
struct replay_insn_list **next;
- prepare_revs(&revs, opts);
+ prepare_revs(opts);
next = todo_list;
- while ((commit = get_revision(&revs)))
+ while ((commit = get_revision(opts->revs)))
next = replay_insn_list_append(opts->action, commit, next);
}
@@ -1051,6 +1053,9 @@ static int pick_revisions(struct replay_opts *opts)
struct replay_insn_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 aee869d..1f4685a 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -414,4 +414,15 @@ test_expect_success 'mixed pick and revert instructions' '
test_cmp expect actual
'
+test_expect_success 'empty commit set' '
+ pristine_detach initial &&
+ test_expect_code 128 git cherry-pick base..base
+'
+
+test_expect_success 'commit set passed through --all' '
+ pristine_detach initial &&
+ test_expect_code 1 git cherry-pick --all &&
+ git cherry-pick --continue
+'
+
test_done
--
1.7.7.3
^ permalink raw reply related
* Re: git-svn clone repositotory without files from codeplex
From: Konstantin Khomoutov @ 2011-12-09 15:46 UTC (permalink / raw)
To: Arkadiy Shapkin; +Cc: git
In-Reply-To: <4EE1FEEF.7070402@gmail.com>
On Fri, 09 Dec 2011 16:28:31 +0400
Arkadiy Shapkin <dragon.artec3d@gmail.com> wrote:
> I can't clone svn repository from codeplex
> https://vld.svn.codeplex.com/svn/vld . Repository generated by
> git-svn doesn't contain files, only log messages.
I suspect that that might be a problem with the fact codeplex seems
to not really provide Subversion repos but rather runs some sort of a
shim to interface Subversion clients with their TFS instance which is
what really hosts non-Mercurial repos.
What makes me think that way is that even svnrdump (a tool for remote
dumping of Subversion repos appeared in v1.7) fails to dump that repo
(my idea was to dump the vld repo then load it into a local repo and
then try cloning the resulting repo using git-svn). I tried svnrdump on
another two randomly picked Codeplex Subversion repos, and it failed on
all of them in the same way it failed for vld.
Hence maybe you should bring this issue with the Codeplex support team.
P.S.
Please next time you cross-post (posting the same message to more
than one mailing list), include a reference to the original discussion
even if it currently consists just of your message solely.
^ permalink raw reply
* Re: Question about commit message wrapping
From: Frans Klaver @ 2011-12-09 16:49 UTC (permalink / raw)
To: Sidney San Martín; +Cc: git
In-Reply-To: <06819C5A-C6D3-4A14-9930-73F66707CE3E@sidneysm.com>
I'm adding git@vger... again, cause there didn't seem to be a reason not to.
On Fri, Dec 9, 2011 at 3:10 PM, Sidney San Martín <s@sidneysm.com> wrote:
> On Dec 9, 2011, at 2:05 AM, Frans Klaver wrote:
>
>> On Fri, 09 Dec 2011 02:59:06 +0100, Sidney San Martín <s@sidneysm.com> wrote:
>>
>>> Hey, I want to ask about the practice of wrapping commit messages to 70-something charaters.
>>>
>>> The webpage most cited about it, which I otherwise really like, is
>>>
>>> http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
>>>
>>> *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.
>>
>> Actually, opera-mail autowraps at 72 characters but sets the text format to flowed. It also wraps the quoted text when you reply. But there's a reasonable chance that you don't use opera in your daily life. On the other hand I would not be surprised if most decent e-mail clients worked that way.
>>
>
> Interesting… either way, the end result is that the receiving mail client can wrap the lines to whatever length it (or you, as its operator) desires, which I think we can agree is a good thing, right?
>
Yes.
>> Hm. Saying "that's how the tool works" is not a good reason in my opinion. There might be tons of other reasons for wrapping at 80 characters. Readability is one that comes to mind for me.
>>
>
> That's my basic point. I hope it didn't seem like I was arguing against reading commit messages wrapped to 80 columns, by default. I only wanted to discuss whether it makes more sense to handle it on the display end instead of asking committers to do it in advance.
>
It somewhat looked like that. I think it might make sense for clients
to ignore the line wrapping when they can only show less than 80
characters on a line, but that would probably break the code part of a
patch mail.
> - My phone shows text most comfortably at about 40 characters per line. I do look at terminals at 80 columns most of the time, but not always, and I sometimes browse projects in GUI tools that use a proportional font in a window may be narrower or wider than that.
>
> - Right now, when I *am* in an 80-col terminal I have to trust everyone else to wrap their commit messages. Not everyone does. I feel like it would be more effective to give git the ability to wrap them automatically when I read them.
>
It could be a useful option to wrap when the lines extend the window
width, but I'd actually think it's better to leave that up to the
pager than to git.
>>>
>>> Second:
>>>
>>>> git format-patch --stdout converts a series of commits to a series of emails, using the messages for the message body. Good email netiquette dictates we wrap our plain text emails such that there’s room for a few levels of nested reply indicators without overflow in an 80 column terminal. (The current rails.git workflow doesn’t include email, but who knows what the future will bring.)
>>>
>>> There's been a standard for flowed plain text emails (which don't have to wrap at 80 columns) for well over ten years, RFC-2646 and is widely supported. Besides, code in diffs is often longer than 7x characters, and wrapping, like `git log`, could be done inside git. FWIW, there are a bunch of merge commits with lines longer than 80 characters in the git repo itself.
>>
>> Yes, that standard allows e-mail clients to display the text more fluidly, even if the source text is word-wrapped. While git uses e-mail format, it isn't an e-mail client. I always interpreted this whole thing as git basically creating plain-text e-mails. You're actually writing the source of the e-mail in your commit message. If you care about actual use in e-mail (like we do here on the list) you might want to add the relevant header to the mails. That said, Apple Mail (the client you used to send your mail) doesn't even use the RFC you quote in the sent message. That mail is going to be a pain in the butt to read in mutt from work ;).
>>
>
> Sorry, I'm not sure what you mean by, “If you care about actual use in e-mail (like we do here on the list) you might want to add the relevant header to the mails”.
I thought you might want to have wrapped text in the git commit
messages, but actually put a format flowed tag into the mail header.
I'm not sure what that would do to the code though.
> Interesting, I didn't realize that Mail didn't use it. It does, however, use quoted-printable which, as far as I can tell, has a similar effect on line wrapping. What happens when you view this email in mutt?
>
I had no idea quoted printable had any effect on line wrapping. As far
as I know it's just a way to encode non-ascii characters in 7bit, no
more no less. Your current e-mail happens to end lines with =<nl>,
which probably handles the wrapping. Your original message didn't have
that.
>>> - - -
>>>
>>> From a93b390d1506652d4ad41d1cbd987ba98a8deca0 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Sidney=20San=20Marti=CC=81n?= <s@sidneysm.com>
>>> Date: Thu, 8 Dec 2011 20:26:23 -0500
>>> Subject: [PATCH] Wrap commit messages on display
>>>
>>> - Wrap to 80 characters minus the indent
>>> - Use a hanging indent for lines which begin with "- "
>>> - Do not wrap lines which begin with whitespace
>>> ---
>>> pretty.c | 10 ++++++++--
>>> 1 files changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/pretty.c b/pretty.c
>>> index 230fe1c..15804ce 100644
>>> --- a/pretty.c
>>> +++ b/pretty.c
>>> @@ -1243,8 +1243,14 @@ void pp_remainder(const struct pretty_print_context *pp,
>>> memset(sb->buf + sb->len, ' ', indent);
>>> strbuf_setlen(sb, sb->len + indent);
>>> }
>>> - strbuf_add(sb, line, linelen);
>>> - strbuf_addch(sb, '\n');
>>> + if (line[0] == ' ' || line[0] == '\t') {
>>> + strbuf_add(sb, line, linelen);
>>> + } else {
>>> + struct strbuf wrapped = STRBUF_INIT;
>>> + strbuf_add(&wrapped, line, linelen);
>>> + strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + (line[0] == '-' && line[1] == ' ' ? 2 : 0), 80 - indent);
>>
>> While on the subject, In my mail view, the new line started with the [1] from line[1], in the quote the line looks entirely different. Now this is code we're talking about, so it makes slightly more sense to have a proper wrapping hard-coded. Compare the above with the following:
>>
>> + int hanging_indent = ((line[0] == '-' && line[1] == ' ') ? 2 : 0);
>> [...]
>> + strbuf_add_wrapped_text(sb, wrapped.buf, 0,
>> + indent + hanging_indent,
>> + 80 - indent);
>>
>> Much clearer, no? I personally usually have two or three terminals tucked next to each other, so I can look at two or three things at the same time. 80 characters limit is a nice feature then.
>
> Good point, that makes it clearer either way. I put an updated patch at the bottom of this email (also fixed forgetting the newline after lines with leading whitespace). I hope it's OK to include patches this way, I understand that they're supposed to represent whole emails but want to include them with this discussion.
>
You can include them in the discussion. While it is probably OK to put
some code into your mail to propose something (I've seen it happen
more than once), the end result is supposed to be submitted with a
git-format-patch'd commit. You can read more about contributing in
Documentation/SubmittingPatches.
>>
>>
>>> + strbuf_addch(sb, '\n');
>>> + }
>>> }
>>> }
>>>
>>
>> Cheers,
>> Frans
>
>
> From 53fd7deedaf5ac522c9d752e79cf71561cc57f07 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Sidney=20San=20Marti=CC=81n?= <s@sidneysm.com>
> Date: Thu, 8 Dec 2011 20:26:23 -0500
> Subject: [PATCH] Wrap commit messages on display
>
> - Wrap to 80 characters, minus the indent
> - Use a hanging indent for lines which begin with "- "
> - Do not wrap lines which begin with whitespace
> ---
> pretty.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index 230fe1c..841ccd1 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1243,7 +1243,14 @@ void pp_remainder(const struct pretty_print_context *pp,
> memset(sb->buf + sb->len, ' ', indent);
> strbuf_setlen(sb, sb->len + indent);
> }
> - strbuf_add(sb, line, linelen);
> + if (line[0] == ' ' || line[0] == '\t') {
> + strbuf_add(sb, line, linelen);
> + } else {
> + struct strbuf wrapped = STRBUF_INIT;
> + strbuf_add(&wrapped, line, linelen);
> + int hanging_indent = ((line[0] == '-' && line[1] == ' ') ? 2 : 0);
> + strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + hanging_indent, 80 - indent);
It's common in C (and in certain flavors even required) to have your
variable declaration at the beginning of the scope:
+ } else {
+ int hanging_indent;
+ struct strbuf wrapped = STRBUF_INIT;
+ strbuf_add(&wrapped, line, linelen);
+ hanging_indent = ((line[0] == '-' && line[1]
== ' ') ? 2 : 0);
+ strbuf_add_wrapped_text(sb, wrapped.buf, 0,
indent + hanging_indent, 80 - indent);
Gmail webclient mucks up the whitespace. Don't copy & paste ;)
As I said earlier in the mail, I'm not sure if this is something that
should be done by git. Maybe someone else can shed some light on that.
> + }
> strbuf_addch(sb, '\n');
> }
> }
> --
> 1.7.8
>
>
^ permalink raw reply
* git submodules and custom work tree / git dir
From: Michael Gorbach @ 2011-12-09 16:48 UTC (permalink / raw)
To: git
I am working on a setup for managing my personal config files
(shell settings and vim settings and such) using git.
I have a git repository which ignores all files by default, so they don't
cloud status. I have the following aliases:
alias config='git --git-dir=$HOME/.config.git/ --work-tree=$HOME'
alias config-add='git --git-dir=$HOME/.config.git --work-tree=$HOME add -f'
This works pretty well. It means that the config repo is separate from other
git repos, and I won't accidentally touch it by just typing "git."
What I'm trying to do now is to track vim (bundle) modules inside this git
repo, which requires the use of git submodules.
It appears that submodules don't work with this weird configuration.
By calling git as in the "config" alias above, except removing the --work-tree
argument, I was able to add submodules successfully with git submodule add. The
submodule info is written into .gitmodules and .config.git/config as expected.
However, the submodule doesn't seem to "stick." It displays when I type
"config status" as "untracked content" no matter what I do, even though
there is no untracked content and no change in the submodule.
Is there a way to make submodules work reasonably with git when using the
--work-tree and --git-dir arguments?
^ permalink raw reply
* [PATCH 1/2] t3401: modernize style
From: Martin von Zweigbergk @ 2011-12-09 16:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
Put the opening quote starting each test on the same line as the
test_expect_* invocation. Also make sure to use tabs for indentation.
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
t/t3401-rebase-partial.sh | 67 ++++++++++++++++++++++-----------------------
1 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index aea6685..d7c874c 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -11,51 +11,50 @@ local branch.
'
. ./test-lib.sh
-test_expect_success \
- 'prepare repository with topic branch' \
- 'echo First > A &&
- git update-index --add A &&
- git commit -m "Add A." &&
+test_expect_success 'prepare repository with topic branch' '
+ echo First > A &&
+ git update-index --add A &&
+ git commit -m "Add A." &&
- git checkout -b my-topic-branch &&
+ git checkout -b my-topic-branch &&
- echo Second > B &&
- git update-index --add B &&
- git commit -m "Add B." &&
+ echo Second > B &&
+ git update-index --add B &&
+ git commit -m "Add B." &&
- echo AnotherSecond > C &&
- git update-index --add C &&
- git commit -m "Add C." &&
+ echo AnotherSecond > C &&
+ git update-index --add C &&
+ git commit -m "Add C." &&
- git checkout -f master &&
+ git checkout -f master &&
- echo Third >> A &&
- git update-index A &&
- git commit -m "Modify A."
+ echo Third >> A &&
+ git update-index A &&
+ git commit -m "Modify A."
'
-test_expect_success \
- 'pick top patch from topic branch into master' \
- 'git cherry-pick my-topic-branch^0 &&
- git checkout -f my-topic-branch &&
- git branch master-merge master &&
- git branch my-topic-branch-merge my-topic-branch
+test_expect_success 'pick top patch from topic branch into master' '
+ git cherry-pick my-topic-branch^0 &&
+ git checkout -f my-topic-branch &&
+ git branch master-merge master &&
+ git branch my-topic-branch-merge my-topic-branch
'
-test_debug \
- 'git cherry master &&
- git format-patch -k --stdout --full-index master >/dev/null &&
- gitk --all & sleep 1
+test_debug '
+ git cherry master &&
+ git format-patch -k --stdout --full-index master >/dev/null &&
+ gitk --all & sleep 1
'
-test_expect_success \
- 'rebase topic branch against new master and check git am did not get halted' \
- 'git rebase master && test ! -d .git/rebase-apply'
+test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
+ git rebase master &&
+ test ! -d .git/rebase-apply
+'
-test_expect_success \
- 'rebase --merge topic branch that was partially merged upstream' \
- 'git checkout -f my-topic-branch-merge &&
- git rebase --merge master-merge &&
- test ! -d .git/rebase-merge'
+test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
+ git checkout -f my-topic-branch-merge &&
+ git rebase --merge master-merge &&
+ test ! -d .git/rebase-merge
+'
test_done
--
1.7.8.237.gcc4e3
^ permalink raw reply related
* [PATCH 2/2] t3401: use test_commit in setup
From: Martin von Zweigbergk @ 2011-12-09 16:59 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1323449952-14161-1-git-send-email-martin.von.zweigbergk@gmail.com>
Simplify t3401 by using test_commit in the setup. This lets us refer
to commits using their tags and there is no longer a need to create
the branch my-topic-branch-merge. Also, the branch master-merge points
to the same commit as master (even before this change), so that branch
does not need to be created either.
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
t/t3401-rebase-partial.sh | 31 ++++++++-----------------------
1 files changed, 8 insertions(+), 23 deletions(-)
diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index d7c874c..1aac22c 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -12,32 +12,17 @@ local branch.
. ./test-lib.sh
test_expect_success 'prepare repository with topic branch' '
- echo First > A &&
- git update-index --add A &&
- git commit -m "Add A." &&
-
+ test_commit A &&
git checkout -b my-topic-branch &&
-
- echo Second > B &&
- git update-index --add B &&
- git commit -m "Add B." &&
-
- echo AnotherSecond > C &&
- git update-index --add C &&
- git commit -m "Add C." &&
-
+ test_commit B &&
+ test_commit C &&
git checkout -f master &&
-
- echo Third >> A &&
- git update-index A &&
- git commit -m "Modify A."
+ test_commit A2 A.t
'
test_expect_success 'pick top patch from topic branch into master' '
- git cherry-pick my-topic-branch^0 &&
- git checkout -f my-topic-branch &&
- git branch master-merge master &&
- git branch my-topic-branch-merge my-topic-branch
+ git cherry-pick C &&
+ git checkout -f my-topic-branch
'
test_debug '
@@ -52,8 +37,8 @@ test_expect_success 'rebase topic branch against new master and check git am did
'
test_expect_success 'rebase --merge topic branch that was partially merged upstream' '
- git checkout -f my-topic-branch-merge &&
- git rebase --merge master-merge &&
+ git reset --hard C &&
+ git rebase --merge master &&
test ! -d .git/rebase-merge
'
--
1.7.8.237.gcc4e3
^ permalink raw reply related
* Re: [PATCH 1/2] run-command: Add checks after execvp fails with EACCES
From: Junio C Hamano @ 2011-12-09 17:23 UTC (permalink / raw)
To: Frans Klaver; +Cc: git
In-Reply-To: <op.v56xbxqs0aolir@keputer>
"Frans Klaver" <fransklaver@gmail.com> writes:
>> Wouldn't access(2) with R_OK|X_OK give you exactly what you want without
>> this much trouble?
>
> I just had a good look through the man page of access(2), and I think
> it depends. access works for the real uid, which is what I attempted
> to implement in the above check as well. However, do we actually need
> to use the real uid or do we need the set uid (geteuid(2))?
Does it matter? We do not use seteuid or setegid ourselves and we do not
expect to be installed as owned by root with u+s bit set.
access(2) checks with real uid exactly because it would not make a
difference to normal user level programs _and_ it makes it easier for a
suid programs to check with the real identity, and our use case falls into
the former, no?
^ permalink raw reply
* Re: git auto-repack is broken...
From: Junio C Hamano @ 2011-12-09 17:35 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Brandon Casey, Jeff King, Linus Torvalds,
Ævar Arnfjörð, Git Mailing List
In-Reply-To: <alpine.LFD.2.02.1112071709250.2907@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> writes:
> On Sat, 3 Dec 2011, Brandon Casey wrote:
>
>> Linus's scenario of fetching a lot of stuff that never actually makes
>> it into the reflogs is still a valid problem. I'm not sure that
>> people who don't know what they are doing are going to run into this
>> problem though. Since he fetches a lot of stuff without ever checking
>> it out or creating a branch from it, potentially many objects become
>> unreferenced every time FETCH_HEAD changes.
>
> Maybe FETCH_HEAD should have a reflog too?
It is a feature that the objects that were fetched for a quick peek become
immediately unreferenced and eligible for early removal unless they are
kept somewhere, e.g. remote tracking refs. What problem are we trying to
solve?
I thought everybody agreed that the current expire window for unreachable
objects is way too conservative, especially given that the only purpose of
that window is to protect live objects from concurrent gcs. Perhaps the
only thing we need to do is to trim that window down to say 2 days or even
8 hours?
^ permalink raw reply
* Re: Question about commit message wrapping
From: Sidney San Martín @ 2011-12-09 17:49 UTC (permalink / raw)
To: Frans Klaver; +Cc: git
In-Reply-To: <CAH6sp9OCbR_NG-sPn7Eq2d4LYbJAbaPX6rCdZR6rbet_nyaGCA@mail.gmail.com>
Ah, thank you!
On Dec 9, 2011, at 11:49 AM, Frans Klaver wrote:
> I'm adding git@vger... again, cause there didn't seem to be a reason not to.
>
> On Fri, Dec 9, 2011 at 3:10 PM, Sidney San Martín <s@sidneysm.com> wrote:
>> On Dec 9, 2011, at 2:05 AM, Frans Klaver wrote:
>>
>>> On Fri, 09 Dec 2011 02:59:06 +0100, Sidney San Martín <s@sidneysm.com> wrote:
>>>
>>>> Hey, I want to ask about the practice of wrapping commit messages to 70-something charaters.
>>>>
>>>> The webpage most cited about it, which I otherwise really like, is
>>>>
>>>> http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
>>>>
>>>> *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.
>>>
>>> Actually, opera-mail autowraps at 72 characters but sets the text format to flowed. It also wraps the quoted text when you reply. But there's a reasonable chance that you don't use opera in your daily life. On the other hand I would not be surprised if most decent e-mail clients worked that way.
>>>
>>
>> Interesting… either way, the end result is that the receiving mail client can wrap the lines to whatever length it (or you, as its operator) desires, which I think we can agree is a good thing, right?
>>
>
> Yes.
>
>>> Hm. Saying "that's how the tool works" is not a good reason in my opinion. There might be tons of other reasons for wrapping at 80 characters. Readability is one that comes to mind for me.
>>>
>>
>> That's my basic point. I hope it didn't seem like I was arguing against reading commit messages wrapped to 80 columns, by default. I only wanted to discuss whether it makes more sense to handle it on the display end instead of asking committers to do it in advance.
>>
>
> It somewhat looked like that. I think it might make sense for clients
> to ignore the line wrapping when they can only show less than 80
> characters on a line, but that would probably break the code part of a
> patch mail.
>
>
>> - My phone shows text most comfortably at about 40 characters per line. I do look at terminals at 80 columns most of the time, but not always, and I sometimes browse projects in GUI tools that use a proportional font in a window may be narrower or wider than that.
>>
>> - Right now, when I *am* in an 80-col terminal I have to trust everyone else to wrap their commit messages. Not everyone does. I feel like it would be more effective to give git the ability to wrap them automatically when I read them.
>>
>
> It could be a useful option to wrap when the lines extend the window
> width, but I'd actually think it's better to leave that up to the
> pager than to git.
>
>>>>
>>>> Second:
>>>>
>>>>> git format-patch --stdout converts a series of commits to a series of emails, using the messages for the message body. Good email netiquette dictates we wrap our plain text emails such that there’s room for a few levels of nested reply indicators without overflow in an 80 column terminal. (The current rails.git workflow doesn’t include email, but who knows what the future will bring.)
>>>>
>>>> There's been a standard for flowed plain text emails (which don't have to wrap at 80 columns) for well over ten years, RFC-2646 and is widely supported. Besides, code in diffs is often longer than 7x characters, and wrapping, like `git log`, could be done inside git. FWIW, there are a bunch of merge commits with lines longer than 80 characters in the git repo itself.
>>>
>>> Yes, that standard allows e-mail clients to display the text more fluidly, even if the source text is word-wrapped. While git uses e-mail format, it isn't an e-mail client. I always interpreted this whole thing as git basically creating plain-text e-mails. You're actually writing the source of the e-mail in your commit message. If you care about actual use in e-mail (like we do here on the list) you might want to add the relevant header to the mails. That said, Apple Mail (the client you used to send your mail) doesn't even use the RFC you quote in the sent message. That mail is going to be a pain in the butt to read in mutt from work ;).
>>>
>>
>> Sorry, I'm not sure what you mean by, “If you care about actual use in e-mail (like we do here on the list) you might want to add the relevant header to the mails”.
>
> I thought you might want to have wrapped text in the git commit
> messages, but actually put a format flowed tag into the mail header.
> I'm not sure what that would do to the code though.
>
>
>> Interesting, I didn't realize that Mail didn't use it. It does, however, use quoted-printable which, as far as I can tell, has a similar effect on line wrapping. What happens when you view this email in mutt?
>>
>
> I had no idea quoted printable had any effect on line wrapping. As far
> as I know it's just a way to encode non-ascii characters in 7bit, no
> more no less. Your current e-mail happens to end lines with =<nl>,
> which probably handles the wrapping. Your original message didn't have
> that.
>
>
>>>> - - -
>>>>
>>>> From a93b390d1506652d4ad41d1cbd987ba98a8deca0 Mon Sep 17 00:00:00 2001
>>>> From: =?UTF-8?q?Sidney=20San=20Marti=CC=81n?= <s@sidneysm.com>
>>>> Date: Thu, 8 Dec 2011 20:26:23 -0500
>>>> Subject: [PATCH] Wrap commit messages on display
>>>>
>>>> - Wrap to 80 characters minus the indent
>>>> - Use a hanging indent for lines which begin with "- "
>>>> - Do not wrap lines which begin with whitespace
>>>> ---
>>>> pretty.c | 10 ++++++++--
>>>> 1 files changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/pretty.c b/pretty.c
>>>> index 230fe1c..15804ce 100644
>>>> --- a/pretty.c
>>>> +++ b/pretty.c
>>>> @@ -1243,8 +1243,14 @@ void pp_remainder(const struct pretty_print_context *pp,
>>>> memset(sb->buf + sb->len, ' ', indent);
>>>> strbuf_setlen(sb, sb->len + indent);
>>>> }
>>>> - strbuf_add(sb, line, linelen);
>>>> - strbuf_addch(sb, '\n');
>>>> + if (line[0] == ' ' || line[0] == '\t') {
>>>> + strbuf_add(sb, line, linelen);
>>>> + } else {
>>>> + struct strbuf wrapped = STRBUF_INIT;
>>>> + strbuf_add(&wrapped, line, linelen);
>>>> + strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + (line[0] == '-' && line[1] == ' ' ? 2 : 0), 80 - indent);
>>>
>>> While on the subject, In my mail view, the new line started with the [1] from line[1], in the quote the line looks entirely different. Now this is code we're talking about, so it makes slightly more sense to have a proper wrapping hard-coded. Compare the above with the following:
>>>
>>> + int hanging_indent = ((line[0] == '-' && line[1] == ' ') ? 2 : 0);
>>> [...]
>>> + strbuf_add_wrapped_text(sb, wrapped.buf, 0,
>>> + indent + hanging_indent,
>>> + 80 - indent);
>>>
>>> Much clearer, no? I personally usually have two or three terminals tucked next to each other, so I can look at two or three things at the same time. 80 characters limit is a nice feature then.
>>
>> Good point, that makes it clearer either way. I put an updated patch at the bottom of this email (also fixed forgetting the newline after lines with leading whitespace). I hope it's OK to include patches this way, I understand that they're supposed to represent whole emails but want to include them with this discussion.
>>
>
> You can include them in the discussion. While it is probably OK to put
> some code into your mail to propose something (I've seen it happen
> more than once), the end result is supposed to be submitted with a
> git-format-patch'd commit. You can read more about contributing in
> Documentation/SubmittingPatches.
>
>
>>>
>>>
>>>> + strbuf_addch(sb, '\n');
>>>> + }
>>>> }
>>>> }
>>>>
>>>
>>> Cheers,
>>> Frans
>>
>>
>> From 53fd7deedaf5ac522c9d752e79cf71561cc57f07 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Sidney=20San=20Marti=CC=81n?= <s@sidneysm.com>
>> Date: Thu, 8 Dec 2011 20:26:23 -0500
>> Subject: [PATCH] Wrap commit messages on display
>>
>> - Wrap to 80 characters, minus the indent
>> - Use a hanging indent for lines which begin with "- "
>> - Do not wrap lines which begin with whitespace
>> ---
>> pretty.c | 9 ++++++++-
>> 1 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/pretty.c b/pretty.c
>> index 230fe1c..841ccd1 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -1243,7 +1243,14 @@ void pp_remainder(const struct pretty_print_context *pp,
>> memset(sb->buf + sb->len, ' ', indent);
>> strbuf_setlen(sb, sb->len + indent);
>> }
>> - strbuf_add(sb, line, linelen);
>> + if (line[0] == ' ' || line[0] == '\t') {
>> + strbuf_add(sb, line, linelen);
>> + } else {
>> + struct strbuf wrapped = STRBUF_INIT;
>> + strbuf_add(&wrapped, line, linelen);
>> + int hanging_indent = ((line[0] == '-' && line[1] == ' ') ? 2 : 0);
>> + strbuf_add_wrapped_text(sb, wrapped.buf, 0, indent + hanging_indent, 80 - indent);
>
> It's common in C (and in certain flavors even required) to have your
> variable declaration at the beginning of the scope:
>
> + } else {
> + int hanging_indent;
> + struct strbuf wrapped = STRBUF_INIT;
> + strbuf_add(&wrapped, line, linelen);
> + hanging_indent = ((line[0] == '-' && line[1]
> == ' ') ? 2 : 0);
> + strbuf_add_wrapped_text(sb, wrapped.buf, 0,
> indent + hanging_indent, 80 - indent);
>
>
> Gmail webclient mucks up the whitespace. Don't copy & paste ;)
>
>
> As I said earlier in the mail, I'm not sure if this is something that
> should be done by git. Maybe someone else can shed some light on that.
>
>
>> + }
>> strbuf_addch(sb, '\n');
>> }
>> }
>> --
>> 1.7.8
>>
>>
^ permalink raw reply
* Re: Question about commit message wrapping
From: Sidney San Martín @ 2011-12-09 17:50 UTC (permalink / raw)
To: Jakub Narebski, git
In-Reply-To: <m3zkf1hnh9.fsf@localhost.localdomain>
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.
>
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?
> 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.
> BTW. proper editor used to create commit message can wrap it for you
> ;-).
Only if everybody else in the world does it too! And only if I never care about seeing my commits at any width besides 80 columns.
> --
> Jakub Narębski
>
^ permalink raw reply
* Re: [PATCHv2 0/13] credential helpers
From: Junio C Hamano @ 2011-12-09 18:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111209022913.GA2600@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It works, and it detects truncated output both ways properly (I know
> because I had to update every test, since the old output was missing the
> end-of-credential marker).
>
> It makes me a little sad, because the original format (relying on EOF)
> was so Unix-y.
It saddens me, too. A reasonable middle ground would be to stop treating
an empty input as "no restriction" but "never matches".
I suspect that it is far more likely for a helper to fail (due to
configuration errors, for example) before it produces any output than
after it gives some but not all output lines.
^ permalink raw reply
* Re: [PATCH 1/6] t3040 (subprojects-basic): modernize style
From: Junio C Hamano @ 2011-12-09 18:26 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Jonathan Nieder, Matthieu Moy, Git List
In-Reply-To: <CALkWK0nbp465915ysrBXi46mur1dutBDtPNjwW=RdyPV03crzg@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> I suspect the above description, while it does describe your patch,
>> does not describe the _reason_ that the patch exists or that someone
>> would want to apply it. Isn't it something more like:
>> [...]
>
> Right, fixed.
Does the "fixed" mean "I fixed it locally; please expect to see it in the
next re-roll?"
Just a quick question, not asking you to always spell it that way.
^ permalink raw reply
* Re: [PATCH 1/6] t3040 (subprojects-basic): modernize style
From: Ramkumar Ramachandra @ 2011-12-09 18:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Matthieu Moy, Git List
In-Reply-To: <7vd3bxfvob.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>
>> Right, fixed.
>
> Does the "fixed" mean "I fixed it locally; please expect to see it in the
> next re-roll?"
Yes, it means exactly that :)
I respond/ archive the review emails as soon as I fix the
corresponding issue locally -- it's the only way to make sure that I
don't miss anything (although I miss a few comments despite that).
-- Ram
^ permalink raw reply
* Re: git auto-repack is broken...
From: Nicolas Pitre @ 2011-12-09 18:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Casey, Jeff King, Linus Torvalds,
Ævar Arnfjörð, Git Mailing List
In-Reply-To: <7v4nx9hcmv.fsf@alter.siamese.dyndns.org>
On Fri, 9 Dec 2011, Junio C Hamano wrote:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
> > On Sat, 3 Dec 2011, Brandon Casey wrote:
> >
> >> Linus's scenario of fetching a lot of stuff that never actually makes
> >> it into the reflogs is still a valid problem. I'm not sure that
> >> people who don't know what they are doing are going to run into this
> >> problem though. Since he fetches a lot of stuff without ever checking
> >> it out or creating a branch from it, potentially many objects become
> >> unreferenced every time FETCH_HEAD changes.
> >
> > Maybe FETCH_HEAD should have a reflog too?
>
> It is a feature that the objects that were fetched for a quick peek become
> immediately unreferenced and eligible for early removal unless they are
> kept somewhere, e.g. remote tracking refs. What problem are we trying to
> solve?
This is indeed a tangential observation to the expiration delay. I was
just suggesting that having a reflog for FETCH_HEAD in the case when you
fetch a branch with an explicit URL might be handy.
Nicolas
^ permalink raw reply
* Re: [PATCH 3/6] t1006 (cat-file): use test_cmp
From: Junio C Hamano @ 2011-12-09 18:43 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Jonathan Nieder, Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <1323430158-14885-4-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> In testing, a common paradigm involves checking the expected output
> with the actual output: test-lib provides a test_cmp to show the diff
> between the two outputs. So, use this function in preference to
> calling a human over to compare command outputs by eye.
>
> Helped-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> t/t1006-cat-file.sh | 119 ++++++++++++++++++++++++---------------------------
> 1 files changed, 56 insertions(+), 63 deletions(-)
>
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index d8b7f2f..5be3ce9 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -144,10 +119,13 @@ tag_size=$(strlen "$tag_content")
>
> run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_pretty_content" 1
>
> -test_expect_success \
> - "Reach a blob from a tag pointing to it" \
> - "test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\""
> +test_expect_success "Reach a blob from a tag pointing to it" '
> + echo_without_newline "$hello_content" >expect &&
> + git cat-file blob "$tag_sha1" >actual &&
> + test_cmp expect actual
> +'
>
> +test_done
> for batch in batch batch-check
> do
> for opt in t s e p
What is that test_done about?
^ permalink raw reply
* Re: [PATCH 3/6] t1006 (cat-file): use test_cmp
From: Ramkumar Ramachandra @ 2011-12-09 18:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Matthieu Moy, Git List
In-Reply-To: <7v8vmlfuw1.fsf@alter.siamese.dyndns.org>
Hi,
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> In testing, a common paradigm involves checking the expected output
>> with the actual output: test-lib provides a test_cmp to show the diff
>> between the two outputs. So, use this function in preference to
>> calling a human over to compare command outputs by eye.
>>
>> Helped-by: Jonathan Nieder <jrnieder@gmail.com>
>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>> ---
>> t/t1006-cat-file.sh | 119 ++++++++++++++++++++++++---------------------------
>> 1 files changed, 56 insertions(+), 63 deletions(-)
>>
>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
>> index d8b7f2f..5be3ce9 100755
>> --- a/t/t1006-cat-file.sh
>> +++ b/t/t1006-cat-file.sh
>> @@ -144,10 +119,13 @@ tag_size=$(strlen "$tag_content")
>>
>> run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_pretty_content" 1
>>
>> -test_expect_success \
>> - "Reach a blob from a tag pointing to it" \
>> - "test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\""
>> +test_expect_success "Reach a blob from a tag pointing to it" '
>> + echo_without_newline "$hello_content" >expect &&
>> + git cat-file blob "$tag_sha1" >actual &&
>> + test_cmp expect actual
>> +'
>>
>> +test_done
>> for batch in batch batch-check
>> do
>> for opt in t s e p
>
> What is that test_done about?
:facepalm: crept in while testing -- I often (temporarily) use a
test_done after the test I want to inspect so the others aren't
unnecessarily executed hiding the error.
Thanks for catching this.
-- Ram
^ permalink raw reply
* Re: [PATCH 1/2] t3401: modernize style
From: Ramkumar Ramachandra @ 2011-12-09 18:51 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1323449952-14161-1-git-send-email-martin.von.zweigbergk@gmail.com>
Hi Martin,
No cover letter, so I'm assuming these are just two random one-off
patches. The motivation is unclear: lazy afternoon? :P
Martin von Zweigbergk wrote:
> [...]
> diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
> index aea6685..d7c874c 100755
> --- a/t/t3401-rebase-partial.sh
> +++ b/t/t3401-rebase-partial.sh
> @@ -11,51 +11,50 @@ local branch.
> '
> . ./test-lib.sh
>
> -test_expect_success \
> - 'prepare repository with topic branch' \
> - 'echo First > A &&
> - git update-index --add A &&
> - git commit -m "Add A." &&
> +test_expect_success 'prepare repository with topic branch' '
> + echo First > A &&
> + git update-index --add A &&
> + git commit -m "Add A." &&
Style nit: >[^ ] is prevalent FWIW.
$ git grep '> [^ ]' t/ | wc -l
3091
$ git grep '>[^ ]' t/ | wc -l
9271
Sure, the regular expressions aren't tailored to make sure that only
redirections are caught, but I suppose it's safe to assume that one
number is significantly larger than the other.
> [...]
> -test_expect_success \
> - 'rebase topic branch against new master and check git am did not get halted' \
> - 'git rebase master && test ! -d .git/rebase-apply'
> +test_expect_success 'rebase topic branch against new master and check git am did not get halted' '
> + git rebase master &&
> + test ! -d .git/rebase-apply
> +'
While at it, why not change this "test ! -d" to
"test_path_is_missing"? My rationale is that you're touching the file
anyway to do a generic cleanup; might as well finish it.
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH 2/2] t3401: use test_commit in setup
From: Ramkumar Ramachandra @ 2011-12-09 19:02 UTC (permalink / raw)
To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1323449952-14161-2-git-send-email-martin.von.zweigbergk@gmail.com>
Hi again,
Martin von Zweigbergk wrote:
> Simplify t3401 by using test_commit in the setup. This lets us refer
> to commits using their tags and there is no longer a need to create
> the branch my-topic-branch-merge. Also, the branch master-merge points
> to the same commit as master (even before this change), so that branch
> does not need to be created either.
The terms "tag" and "branch" here have no significance, so
de-emphasizing them to "ref" is probably a good idea. Isn't the truth
more like: instead of creating commits and creating refs to track
those commits by hand, use test_commit to achieve the same result in a
single step?
Cheers.
-- Ram
^ permalink raw reply
* Re: [PATCH 9/9] revert: simplify communicating command-line arguments
From: Jonathan Nieder @ 2011-12-09 19:02 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <1323445326-24637-10-git-send-email-artagnon@gmail.com>
Hey,
Ramkumar Ramachandra wrote:
> From: Jonathan Nieder <jrnieder@gmail.com>
[...]
> 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. So, 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 "struct rev_info".
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Acked-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This sign-off chain suggests that the life of the patch happened in
three stages:
- first, you wrote the patch or some component of it and passed it to
me.
- then, I accepted it, tweaked it so much that I felt the need to claim
authorship and save you from blame (hence the "From:" field), and
passed it on to Junio.
- finally, Junio applied it to some tree.
and that you are sending out a copy of the patch Junio applied for
additional comments.
But in fact, this started with a patch from me (I don't rememember
whether it was signed off, but that doesn't matter --- I happily
retroactively sign off on it) and now you are sending it to the list
and Junio for comments. So I would think it should simply say
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
and Junio can add his sign-off below that when applying some version
to his tree. Bonus points if you mention what tweaks you made when
you are not just passing it on.
[...]
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -414,4 +414,15 @@ test_expect_success 'mixed pick and revert instructions' '
> test_cmp expect actual
> '
>
> +test_expect_success 'empty commit set' '
> + pristine_detach initial &&
> + test_expect_code 128 git cherry-pick base..base
> +'
> +
> +test_expect_success 'commit set passed through --all' '
> + pristine_detach initial &&
> + test_expect_code 1 git cherry-pick --all &&
> + git cherry-pick --continue
> +'
What does this test mean? "git cherry-pick --all" should report a
spurious conflict, and then --continue will clean it up automagically
and all will be well?
^ permalink raw reply
* Re: [PATCH 9/9] revert: simplify communicating command-line arguments
From: Ramkumar Ramachandra @ 2011-12-09 19:11 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano
In-Reply-To: <20111209190236.GA20913@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> [...]
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>
> and Junio can add his sign-off below that when applying some version
> to his tree. Bonus points if you mention what tweaks you made when
> you are not just passing it on.
True, the signoff-chain does look like quite a mess now :P
[rr: minor improvements, tests] should be fine.
>> --- a/t/t3510-cherry-pick-sequence.sh
>> +++ b/t/t3510-cherry-pick-sequence.sh
>> @@ -414,4 +414,15 @@ test_expect_success 'mixed pick and revert instructions' '
>> +test_expect_success 'commit set passed through --all' '
>> + pristine_detach initial &&
>> + test_expect_code 1 git cherry-pick --all &&
>> + git cherry-pick --continue
>> +'
>
> What does this test mean? "git cherry-pick --all" should report a
> spurious conflict, and then --continue will clean it up automagically
> and all will be well?
This one's actually quite interesting. "git cherry-pick --all" first
tries to apply everything from "intial" to "yetanotherpatch" (both
inclusive) -- its first "git commit" invocation returns 1, refusing to
create an empty commit. Then when we say "--continue", it notices
that the worktree and index are clean, removes "initial" from the
instruction sheet and executes everything else as usual. This is
something we should attempt to fix in the future: I think the Right
Way starts with creating an API for "git commit".
Thanks.
-- Ram
^ permalink raw reply
* Re: [PATCH] tag deletions not rejected with receive.denyDeletes= true
From: Junio C Hamano @ 2011-12-09 19:15 UTC (permalink / raw)
To: Jerome DE VIVIE; +Cc: git
In-Reply-To: <18683796.591323420674000.JavaMail.root@promailix.prometil.com>
Jerome DE VIVIE <j.devivie@prometil.com> writes:
> Hello,
>
> I have try to deny tag deletion over push using denyDeletes parameter:
>
> git config --system receive.denyDeletes true
> git daemon --reuseaddr --base-path=.. --export-all --verbose --enable=receive-pack
>
> I can push tag deletions despite what the internet says (http://progit.org/book/ch7-1.html#receivedenydeletes). I don't know if it is a bug. Could you have a look, pls ? Thank you
The code seems to be written in such a way that it _explicitly_ wants to
limit the effect of the configuration only to branches. The change was
introduced by a240de1 (Introduce receive.denyDeletes, 2008-11-01) and the
motivation was explained as:
Introduce receive.denyDeletes
Occasionally, it may be useful to prevent branches from getting deleted from
a centralized repository, particularly when no administrative access to the
server is available to undo it via reflog. It also makes
receive.denyNonFastForwards more useful if it is used for access control
since it prevents force-updating by deleting and re-creating a ref.
So I would have to say your "the internet" is wrong.
Our documentation can also use some updates, as it dates to the days back
when we more liberally used "refs" and "branches" interchangeably.
^ permalink raw reply
* Re: [PATCH 9/9] revert: simplify communicating command-line arguments
From: Jonathan Nieder @ 2011-12-09 19:29 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano
In-Reply-To: <CALkWK0m_6yGuyLt-VqvRZkAiDoSxES8KeNzeXaejYRCpW=HAOg@mail.gmail.com>
Ramkumar Ramachandra wrote:
>> Ramkumar Ramachandra wrote:
>>> +++ b/t/t3510-cherry-pick-sequence.sh
>>> @@ -414,4 +414,15 @@ test_expect_success 'mixed pick and revert instructions' '
>>> +test_expect_success 'commit set passed through --all' '
>>> + pristine_detach initial &&
>>> + test_expect_code 1 git cherry-pick --all &&
>>> + git cherry-pick --continue
>>> +'
[...]
> This one's actually quite interesting. "git cherry-pick --all" first
> tries to apply everything from "intial" to "yetanotherpatch" (both
> inclusive) -- its first "git commit" invocation returns 1, refusing to
> create an empty commit. Then when we say "--continue", it notices
> that the worktree and index are clean, removes "initial" from the
> instruction sheet and executes everything else as usual. This is
> something we should attempt to fix
So, is it a bad test? Was the initial command crazy and ill-defined
enough that no one would actually do that? Is the response to the
command incorrect, meaning that the new test should be instead
checking for some different result with test_expect_failure?
I only mentioned --all in the first place because it is a "revision
pseudo-option" (i.e., option starting with "--" whose position on the
rev-list command line matters) and gets handled by a slightly
different revision parsing code path than foo..bar. There are other
revision pseudo-options that are easier to control and might make for
a better test if it's wanted, like --remotes=foo.
> Way starts with creating an API for "git commit".
Not sure what this means.
^ 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