* [PATCH 01/12] Rename another local variable name -> refname
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
I missed this one earlier. (There are probably others too. I hope
you don't feel that I'm morally obliged to find every single analogous
example...)
refs.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/refs.c b/refs.c
index 7e6cea5..f6752a4 100644
--- a/refs.c
+++ b/refs.c
@@ -317,11 +317,11 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
if (dir) {
struct dirent *de;
int baselen = strlen(base);
- char *ref = xmalloc(baselen + 257);
+ char *refname = xmalloc(baselen + 257);
- memcpy(ref, base, baselen);
+ memcpy(refname, base, baselen);
if (baselen && base[baselen-1] != '/')
- ref[baselen++] = '/';
+ refname[baselen++] = '/';
while ((de = readdir(dir)) != NULL) {
unsigned char sha1[20];
@@ -337,31 +337,31 @@ static void get_ref_dir(struct ref_cache *refs, const char *base,
continue;
if (has_extension(de->d_name, ".lock"))
continue;
- memcpy(ref + baselen, de->d_name, namelen+1);
+ memcpy(refname + baselen, de->d_name, namelen+1);
refdir = *refs->name
- ? git_path_submodule(refs->name, "%s", ref)
- : git_path("%s", ref);
+ ? git_path_submodule(refs->name, "%s", refname)
+ : git_path("%s", refname);
if (stat(refdir, &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
- get_ref_dir(refs, ref, array);
+ get_ref_dir(refs, refname, array);
continue;
}
if (*refs->name) {
hashclr(sha1);
flag = 0;
- if (resolve_gitlink_ref(refs->name, ref, sha1) < 0) {
+ if (resolve_gitlink_ref(refs->name, refname, sha1) < 0) {
hashclr(sha1);
flag |= REF_BROKEN;
}
} else
- if (!resolve_ref(ref, sha1, 1, &flag)) {
+ if (!resolve_ref(refname, sha1, 1, &flag)) {
hashclr(sha1);
flag |= REF_BROKEN;
}
- add_ref(ref, sha1, flag, array, NULL);
+ add_ref(refname, sha1, flag, array, NULL);
}
- free(ref);
+ free(refname);
closedir(dir);
}
}
--
1.7.7
^ permalink raw reply related
* [PATCH 02/12] repack_without_ref(): remove temporary
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1319060692-27216-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Trivial and boring.
refs.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/refs.c b/refs.c
index f6752a4..431d3c6 100644
--- a/refs.c
+++ b/refs.c
@@ -1190,12 +1190,10 @@ static struct lock_file packlock;
static int repack_without_ref(const char *refname)
{
struct ref_array *packed;
- struct ref_entry *ref;
int fd, i;
packed = get_packed_refs(get_ref_cache(NULL));
- ref = search_ref_array(packed, refname);
- if (ref == NULL)
+ if (search_ref_array(packed, refname) == NULL)
return 0;
fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
if (fd < 0) {
@@ -1206,8 +1204,7 @@ static int repack_without_ref(const char *refname)
for (i = 0; i < packed->nr; i++) {
char line[PATH_MAX + 100];
int len;
-
- ref = packed->refs[i];
+ struct ref_entry *ref = packed->refs[i];
if (!strcmp(refname, ref->name))
continue;
--
1.7.7
^ permalink raw reply related
* [PATCH 00/12] Use refs API more consistently
From: mhagger @ 2011-10-19 21:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
This patch series primarily has to do with using the refs API more
consistently within refs.c itself. We want to minimize the surface
area for accessing the ref_cache data structure so that when it is
changed into a tree (we're on the verge of that change now, I
promise!) we don't have to change callers more than necessary. There
is also a bit of an eat-your-own-dogfood thing going on here; if
for_each_ref() and its friends are good enough for "outsiders", they
should be good enough for most internal use.
The first two patches are trivial preparation cleanups.
The third verifies that refnames read from packed-ref files are
properly formatted. When the REFNAME_FULL changes becomes available,
this check can be tightened up.
The do_for_each_ref_in_{array,arrays}() functions provide an
iterator-like interface to iterating over a single ref_array and
iterating over two ref_arrays in parallel. These are useful
abstractions in and of themselves (the former is used to reimplement
repack_without_ref() and is_refname_available() in patches 9 and 12
respectively). And they will be even more useful when references are
stored hierarchically.
This series applies on top of mh/ref-api-2. It conflicts with the
"Checking full vs. partial refnames" series that I just submitted but
not in any fundamental way. I'll be happy to reconcile them for you
in a later round when it is clear if and when each series is accepted.
Michael Haggerty (12):
Rename another local variable name -> refname
repack_without_ref(): remove temporary
parse_ref_line(): add a check that the refname is properly formatted
create_ref_entry(): extract function from add_ref()
add_ref(): take a (struct ref_entry *) parameter
do_for_each_ref(): correctly terminate while processesing extra_refs
do_for_each_ref_in_array(): new function
do_for_each_ref_in_arrays(): new function
repack_without_ref(): reimplement using do_for_each_ref_in_array()
names_conflict(): new function, extracted from is_refname_available()
names_conflict(): simplify implementation
is_refname_available(): reimplement using do_for_each_ref_in_array()
refs.c | 259 ++++++++++++++++++++++++++++++++++++++++------------------------
1 files changed, 162 insertions(+), 97 deletions(-)
--
1.7.7
^ permalink raw reply
* [PATCH 6/6] revert: simplify communicating command-line arguments
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
From: Jonathan Nieder <jrnieder@gmail.com>
Currently, command-line arguments are communicated using (argc, argv)
until a prepare_revs() turns it into a terse structure. However,
since we plan to expose the cherry-picking machinery through a public
API in the future, we want callers to be able to call in with a
filled-in structure. For the revert builtin, this means that the
chief argument parser, parse_args(), should parse into such a
structure. Make this change.
[rr: minor improvements, commit message]
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 53 +++++++++++++++++++++++++++++------------------------
1 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 0201e98..576b9e7 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -54,13 +54,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"
@@ -161,9 +162,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,
@@ -198,9 +199,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,
@@ -208,7 +206,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 {
@@ -605,23 +616,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"));
}
@@ -809,14 +812,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);
}
@@ -940,6 +942,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);
/*
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 5/6] revert: allow mixed pick and revert instructions
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-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.
This patch lays the foundation for extending the parser to support
more actions so 'git rebase -i' can reuse this machinery in the
future. While at it, also improve the error messages reported by the
insn sheet parser.
Helped-by: Jonathan Nieder <jrnider@gmail.com>
Acked-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 | 130 +++++++++++++++++++--------------------
sequencer.h | 8 +++
t/t3510-cherry-pick-sequence.sh | 58 +++++++++++++++++
3 files changed, 129 insertions(+), 67 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 8e34dc0..0201e98 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {
NULL
};
-enum replay_action { REVERT, CHERRY_PICK };
enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };
struct replay_opts {
@@ -68,14 +67,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,
@@ -152,7 +151,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"),
@@ -346,7 +345,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."));
@@ -450,7 +449,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;
@@ -525,7 +525,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;
@@ -568,7 +568,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
write_cherry_pick_head(commit);
}
- 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);
@@ -587,7 +587,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
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),
@@ -611,7 +611,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);
@@ -657,49 +657,53 @@ 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;
+ } else {
+ size_t len = strchrnul(bol, '\n') - bol;
+ if (len > 255)
+ len = 255;
+ return error(_("Unrecognized action: %.*s"), (int)len, bol);
+ }
end_of_object_name = bol + strcspn(bol, " \n");
saved = *end_of_object_name;
@@ -707,37 +711,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 error(_("Malformed object name: %s"), bol);
- return lookup_commit_reference(commit_sha1);
+ item->operand = lookup_commit_reference(commit_sha1);
+ if (!item->operand)
+ return error(_("Not a valid commit: %s"), bol);
+
+ 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)
- return error(_("Could not parse line %d."), i);
- next = commit_list_append(commit, next);
+ if (parse_insn_line(p, eol, &item) < 0)
+ return error(_("on line %d."), i);
+ next = replay_insn_list_append(item.action, item.operand, next);
p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
@@ -745,8 +741,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;
@@ -762,7 +757,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);
@@ -811,18 +806,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)
@@ -851,7 +846,7 @@ static void save_head(const char *head)
die(_("Error wrapping up %s."), head_file);
}
-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;
@@ -859,7 +854,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);
@@ -903,9 +898,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);
@@ -915,8 +911,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)
/*
@@ -941,7 +937,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);
@@ -958,7 +954,7 @@ static int pick_revisions(struct replay_opts *opts)
if (!file_exists(git_path(SEQ_TODO_FILE)))
goto error;
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))
@@ -978,7 +974,7 @@ static int pick_revisions(struct replay_opts *opts)
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"));
}
@@ -998,7 +994,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);
@@ -1013,7 +1009,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/sequencer.h b/sequencer.h
index 905d295..f4db257 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -7,6 +7,14 @@
#define SEQ_TODO_FILE "sequencer/todo"
#define SEQ_OPTS_FILE "sequencer/opts"
+enum replay_action { REPLAY_REVERT, REPLAY_PICK };
+
+struct replay_insn_list {
+ enum replay_action action;
+ struct commit *operand;
+ struct replay_insn_list *next;
+};
+
/*
* Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring
* any errors. Intended to be used by 'git reset'.
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 39b55c1..4b12244 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -239,4 +239,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_must_fail 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_must_fail 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.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 4/6] revert: make commit subjects in insn sheet optional
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Change the instruction sheet format subtly so that the subject of the
commit message that follows the object name is optional. As a result,
an instruction sheet like this is now perfectly valid:
pick 35b0426
pick fbd5bbcbc2e
pick 7362160f
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 37 ++++++++++++++++---------------------
t/t3510-cherry-pick-sequence.sh | 14 ++++++++++++++
2 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 474dda1..8e34dc0 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -685,31 +685,27 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
return 0;
}
-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
+static struct commit *parse_insn_line(char *bol, char *eol, struct replay_opts *opts)
{
unsigned char commit_sha1[20];
- char sha1_abbrev[40];
enum replay_action action;
- int insn_len = 0;
- char *p, *q;
+ char *end_of_object_name;
+ int saved, status;
- if (!prefixcmp(start, "pick ")) {
+ if (!prefixcmp(bol, "pick ")) {
action = CHERRY_PICK;
- insn_len = strlen("pick");
- p = start + insn_len + 1;
- } else if (!prefixcmp(start, "revert ")) {
+ bol += strlen("pick ");
+ } else if (!prefixcmp(bol, "revert ")) {
action = REVERT;
- insn_len = strlen("revert");
- p = start + insn_len + 1;
+ bol += strlen("revert ");
} else
return NULL;
- q = strchr(p, ' ');
- if (!q || q - p + 1 > sizeof(sha1_abbrev))
- return NULL;
- q++;
-
- strlcpy(sha1_abbrev, p, q - p);
+ end_of_object_name = bol + strcspn(bol, " \n");
+ saved = *end_of_object_name;
+ *end_of_object_name = '\0';
+ status = get_sha1(bol, commit_sha1);
+ *end_of_object_name = saved;
/*
* Verify that the action matches up with the one in
@@ -722,7 +718,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
return NULL;
}
- if (get_sha1(sha1_abbrev, commit_sha1) < 0)
+ if (status < 0)
return NULL;
return lookup_commit_reference(commit_sha1);
@@ -737,13 +733,12 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,
int i;
for (i = 1; *p; i++) {
- commit = parse_insn_line(p, opts);
+ char *eol = strchrnul(p, '\n');
+ commit = parse_insn_line(p, eol, opts);
if (!commit)
return error(_("Could not parse line %d."), i);
next = commit_list_append(commit, next);
- p = strchrnul(p, '\n');
- if (*p)
- p++;
+ p = *eol ? eol + 1 : eol;
}
if (!*todo_list)
return error(_("No commits parsed."));
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 2113308..39b55c1 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -225,4 +225,18 @@ test_expect_success 'malformed instruction sheet 3' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'commit descriptions in insn sheet are optional' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "c" >foo &&
+ git add foo &&
+ git commit &&
+ cut -d" " -f1,2 .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ git cherry-pick --continue &&
+ test_path_is_missing .git/sequencer &&
+ git rev-list HEAD >commits
+ test_line_count = 4 commits
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 3/6] revert: fix buffer overflow in insn sheet parser
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Check that the commit name argument to a "pick" or "revert" action in
'.git/sequencer/todo' is not too long, to avoid overflowing an
on-stack buffer. This fixes a regression introduced by 5a5d80f4
(revert: Introduce --continue to continue the operation, 2011-08-04).
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 2 +-
t/t3510-cherry-pick-sequence.sh | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index acb357d..474dda1 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -705,7 +705,7 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)
return NULL;
q = strchr(p, ' ');
- if (!q)
+ if (!q || q - p + 1 > sizeof(sha1_abbrev))
return NULL;
q++;
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3bca2b3..2113308 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -12,6 +12,9 @@ test_description='Test cherry-pick continuation features
. ./test-lib.sh
+# Repeat first match 10 times
+_r10='\1\1\1\1\1\1\1\1\1\1'
+
pristine_detach () {
git cherry-pick --reset &&
git checkout -f "$1^0" &&
@@ -211,4 +214,15 @@ test_expect_success 'malformed instruction sheet 2' '
test_must_fail git cherry-pick --continue
'
+test_expect_success 'malformed instruction sheet 3' '
+ pristine_detach initial &&
+ test_must_fail git cherry-pick base..anotherpick &&
+ echo "resolved" >foo &&
+ git add foo &&
+ git commit &&
+ sed "s/pick \([0-9a-f]*\)/pick $_r10/" .git/sequencer/todo >new_sheet &&
+ cp new_sheet .git/sequencer/todo &&
+ test_must_fail git cherry-pick --continue
+'
+
test_done
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 2/6] revert: simplify getting commit subject in format_todo()
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
format_todo() calls get_message(), but uses only the subject line of
the commit message. Save work and unnecessary memory allocations by
using find_commit_subject() instead.
Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 94b7c50..acb357d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -671,16 +671,16 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
struct replay_opts *opts)
{
struct commit_list *cur = NULL;
- struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
const char *sha1_abbrev = NULL;
const char *action_str = opts->action == REVERT ? "revert" : "pick";
+ const char *subject;
+ int subject_len;
for (cur = todo_list; cur; cur = cur->next) {
sha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);
- if (get_message(cur->item, &msg))
- return error(_("Cannot get commit message for %s"), sha1_abbrev);
- strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
- free_message(&msg);
+ subject_len = find_commit_subject(cur->item->buffer, &subject);
+ strbuf_addf(buf, "%s %s %.*s\n", action_str, sha1_abbrev,
+ subject_len, subject);
}
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 1/6] revert: free msg in format_todo()
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
In-Reply-To: <1319058208-17923-1-git-send-email-artagnon@gmail.com>
Memory allocated to the fields of msg by get_message() isn't freed.
This is potentially a big leak, because fresh memory is allocated to
store the commit message for each commit. Fix this using
free_message().
Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/revert.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 010508d..94b7c50 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -680,6 +680,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
if (get_message(cur->item, &msg))
return error(_("Cannot get commit message for %s"), sha1_abbrev);
strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
+ free_message(&msg);
}
return 0;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH v2 0/6] Sequencer fixups mini-series
From: Ramkumar Ramachandra @ 2011-10-19 21:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jonathan Nieder, Christian Couder
Hi,
This is the second iteration of series I first posted in
$gmane/183157. The improvements can be summarized as follows:
1. Junio wrote some extra code to squash into the fourth part. It
took me some time to resolve conflicts correctly while rebasing.
2. Jonathan and Tay (off-list) have helped improve all the commit
messages.
Thanks.
Jonathan Nieder (1):
revert: simplify communicating command-line arguments
Ramkumar Ramachandra (5):
revert: free msg in format_todo()
revert: simplify getting commit subject in format_todo()
revert: fix buffer overflow in insn sheet parser
revert: make commit subjects in insn sheet optional
revert: allow mixed pick and revert instructions
builtin/revert.c | 219 +++++++++++++++++++--------------------
sequencer.h | 8 ++
t/t3510-cherry-pick-sequence.sh | 86 +++++++++++++++
3 files changed, 202 insertions(+), 111 deletions(-)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply
* [RFC 12/13] resolve_ref: use check_refname_format(..., REFNAME_FULL)
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
...instead of own inline code.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index a40dfa5..67df3a6 100644
--- a/refs.c
+++ b/refs.c
@@ -548,8 +548,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
if (len < 0)
return NULL;
buffer[len] = 0;
- if (!prefixcmp(buffer, "refs/") &&
- !check_refname_format(buffer, 0)) {
+ if (!check_refname_format(buffer, REFNAME_FULL)) {
strcpy(ref_buffer, buffer);
ref = ref_buffer;
if (flag)
--
1.7.7
^ permalink raw reply related
* [RFC 13/13] filter_refs(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/fetch-pack.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index c6bc8eb..d72aa44 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -546,7 +546,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
for (ref = *refs; ref; ref = next) {
next = ref->next;
if (!memcmp(ref->name, "refs/", 5) &&
- check_refname_format(ref->name + 5, 0))
+ check_refname_format(ref->name, REFNAME_FULL))
; /* trash */
else if (args.fetch_all &&
(!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
--
1.7.7
^ permalink raw reply related
* [RFC 11/13] replace_object(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/replace.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 517fa10..f5d4f77 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -94,7 +94,7 @@ static int replace_object(const char *object_ref, const char *replace_ref,
"refs/replace/%s",
sha1_to_hex(object)) > sizeof(ref) - 1)
die("replace ref name too long: %.*s...", 50, ref);
- if (check_refname_format(ref, 0))
+ if (check_refname_format(ref, REFNAME_FULL))
die("'%s' is not a valid ref name.", ref);
if (!resolve_ref(ref, prev, 1, NULL))
--
1.7.7
^ permalink raw reply related
* [RFC 10/13] strbuf_check_tag_ref(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/tag.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 9b6fd95..abf3cb0 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -412,7 +412,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
strbuf_reset(sb);
strbuf_addf(sb, "refs/tags/%s", name);
- return check_refname_format(sb->buf, 0);
+ return check_refname_format(sb->buf, REFNAME_FULL);
}
int cmd_tag(int argc, const char **argv, const char *prefix)
--
1.7.7
^ permalink raw reply related
* [RFC 01/13] check_refname_component(): iterate via index rather than via pointer
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
This will make later changes easier.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs.c b/refs.c
index 0d74d70..3b41252 100644
--- a/refs.c
+++ b/refs.c
@@ -912,11 +912,11 @@ static inline int bad_ref_char(int ch)
*/
static int check_refname_component(const char *ref, int flags)
{
- const char *cp;
+ int i, len = strlen(ref);
char last = '\0';
- for (cp = ref; ; cp++) {
- char ch = *cp;
+ for (i = 0; i < len; i++) {
+ char ch = ref[i];
if (ch == '\0' || ch == '/')
break;
if (bad_ref_char(ch))
@@ -927,7 +927,7 @@ static int check_refname_component(const char *ref, int flags)
return -1; /* Refname contains "@{". */
last = ch;
}
- if (cp == ref)
+ if (i == 0)
return -1; /* Component has zero length. */
if (ref[0] == '.') {
if (!(flags & REFNAME_DOT_COMPONENT))
@@ -936,12 +936,12 @@ static int check_refname_component(const char *ref, int flags)
* Even if leading dots are allowed, don't allow "."
* as a component (".." is prevented by a rule above).
*/
- if (ref[1] == '\0')
+ if (i == 1)
return -1; /* Component equals ".". */
}
- if (cp - ref >= 5 && !memcmp(cp - 5, ".lock", 5))
+ if (i >= 5 && !memcmp(&ref[i - 5], ".lock", 5))
return -1; /* Refname ends with ".lock". */
- return cp - ref;
+ return i;
}
int check_refname_format(const char *ref, int flags)
--
1.7.7
^ permalink raw reply related
* [RFC 03/13] Teach check_refname_format() to check full refnames
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
A full refname has to start with "refs/" or consist entirely of capital
letters and underscores. Add an option REFNAME_FULL that causes
check_refname_format() and parse_refname_prefix() to verify that the
refname is a valid full refname.
Add a "--full" option to "git check-ref-format" that can be used to
access the new option. Use this to add some tests involving "git
check-ref-format --full".
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/git-check-ref-format.txt | 14 +++++++++++
builtin/check-ref-format.c | 4 +++
refs.c | 39 ++++++++++++++++++-------------
refs.h | 12 ++++++---
t/t1402-check-ref-format.sh | 31 +++++++++++++++++++++++++
5 files changed, 80 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index 103e7b1..9a8aafb 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -27,6 +27,11 @@ if refs are packed by `git gc`).
git imposes the following rules on how references are named:
+. They must either start with `refs/` (e.g., `refs/heads/master`) or
+ consist entirely of capital letters and underscores (e.g., `HEAD` or
+ `MERGE_HEAD`). This rule is enforced if the `--full` option is
+ used.
+
. They can include slash `/` for hierarchical (directory)
grouping, but no slash-separated component can begin with a
dot `.` or end with the sequence `.lock`.
@@ -83,6 +88,15 @@ typed the branch name.
OPTIONS
-------
+--full::
+--no-full::
+ Controls whether the refname must represent a full refname
+ (i.e., starting with `refs/`). If `--full` and
+ `--allow-onelevel` are both specified, then special top-level
+ refnames consisting of capital letters and underscored (like
+ `HEAD` or `MERGE_HEAD`) are also allowed. The default is
+ `--no-full`.
+
--allow-onelevel::
--no-allow-onelevel::
Controls whether one-level refnames are accepted (i.e.,
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 28a7320..a73626d 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -64,6 +64,10 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
for (i = 1; i < argc && argv[i][0] == '-'; i++) {
if (!strcmp(argv[i], "--normalize") || !strcmp(argv[i], "--print"))
normalize = 1;
+ else if (!strcmp(argv[i], "--full"))
+ flags |= REFNAME_FULL;
+ else if (!strcmp(argv[i], "--no-full"))
+ flags &= ~REFNAME_FULL;
else if (!strcmp(argv[i], "--allow-onelevel"))
flags |= REFNAME_ALLOW_ONELEVEL;
else if (!strcmp(argv[i], "--no-allow-onelevel"))
diff --git a/refs.c b/refs.c
index 387da83..8299e51 100644
--- a/refs.c
+++ b/refs.c
@@ -980,6 +980,11 @@ static int parse_refname_prefix(const char *ref, int len, int flags)
component_count++;
if (component_len == len || p[component_len] != '/')
break;
+ if (component_count == 1 && (flags & REFNAME_FULL)) {
+ /* That was the first of multiple components */
+ if (component_len != 4 || strncmp(p, "refs", 4))
+ return -1;
+ }
/* Skip to next component. */
p += component_len + 1;
len -= component_len + 1;
@@ -989,8 +994,22 @@ static int parse_refname_prefix(const char *ref, int len, int flags)
if (ref[valid_len - 1] == '.')
return -1; /* Refname ends with '.'. */
- if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
- return -1; /* Refname has only one component. */
+
+ if (component_count == 1) {
+ if (!(flags & REFNAME_ALLOW_ONELEVEL))
+ return -1;
+
+ if (flags & REFNAME_FULL) {
+ /* Make sure that the refname is ALL_CAPS. */
+ int i;
+ for (i = 0; i < component_len; i++) {
+ char ch = p[i];
+ if (ch != '_' && (ch < 'A' || 'Z' < ch))
+ return -1;
+ }
+ }
+ }
+
return valid_len;
}
@@ -1028,20 +1047,8 @@ const char *ref_fetch_rules[] = {
static int refname_ok_at_root_level(const char *str, int len)
{
- if (len >= 5 && !memcmp(str, "refs/", 5))
- return 1;
-
- while (len--) {
- char ch = *str++;
-
- /*
- * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
- * the root level as a ref.
- */
- if (ch != '_' && (ch < 'A' || 'Z' < ch))
- return 0;
- }
- return 1;
+ return parse_refname_prefix(str, len,
+ REFNAME_ALLOW_ONELEVEL|REFNAME_FULL) == len;
}
int refname_match(const char *abbrev_name, const char *full_name, const char **rules)
diff --git a/refs.h b/refs.h
index 0229c57..7707cda 100644
--- a/refs.h
+++ b/refs.h
@@ -97,9 +97,10 @@ int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, voi
*/
extern int for_each_reflog(each_ref_fn, void *);
-#define REFNAME_ALLOW_ONELEVEL 1
-#define REFNAME_REFSPEC_PATTERN 2
-#define REFNAME_DOT_COMPONENT 4
+#define REFNAME_ALLOW_ONELEVEL 01
+#define REFNAME_REFSPEC_PATTERN 02
+#define REFNAME_DOT_COMPONENT 04
+#define REFNAME_FULL 010
/*
* Return 0 iff ref has the correct format for a refname according to
@@ -110,7 +111,10 @@ extern int for_each_reflog(each_ref_fn, void *);
* components. No leading or repeated slashes are accepted. If
* REFNAME_DOT_COMPONENT is set in flags, then allow refname
* components to start with "." (but not a whole component equal to
- * "." or "..").
+ * "." or ".."). If REFNAME_FULL is set in flags, then additionally
+ * verify that the refname is a valid full refname--that it either
+ * starts with "refs/" or that it consists of only capital letters and
+ * underscores (like "HEAD" or "MERGE_HEAD").
*/
extern int check_refname_format(const char *ref, int flags);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1ae4d87..2f8a48e 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -70,6 +70,7 @@ invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
invalid_ref "$ref" --normalize
valid_ref "$ref" '--allow-onelevel --normalize'
+invalid_ref "$ref" '--full'
ref='foo/bar'
valid_ref "$ref"
@@ -77,12 +78,19 @@ valid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
valid_ref "$ref" --normalize
+invalid_ref "$ref" '--full'
ref='foo/*'
invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--full'
+invalid_ref "$ref" '--refspec-pattern --full'
+
+ref='refs/*'
+invalid_ref "$ref" '--full'
+valid_ref "$ref" '--refspec-pattern --full'
ref='*/foo'
invalid_ref "$ref"
@@ -91,18 +99,23 @@ valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
invalid_ref "$ref" --normalize
valid_ref "$ref" '--refspec-pattern --normalize'
+invalid_ref "$ref" '--full'
+invalid_ref "$ref" '--refspec-pattern --full'
ref='foo/*/bar'
invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
valid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--full'
ref='*'
invalid_ref "$ref"
invalid_ref "$ref" --allow-onelevel
invalid_ref "$ref" --refspec-pattern
valid_ref "$ref" '--refspec-pattern --allow-onelevel'
+invalid_ref "$ref" '--full'
+invalid_ref "$ref" '--refspec-pattern --full'
ref='foo/*/*'
invalid_ref "$ref" --refspec-pattern
@@ -126,6 +139,24 @@ valid_ref NOT_MINGW "$ref" '--allow-onelevel --normalize'
invalid_ref NOT_MINGW "$ref" '--refspec-pattern --normalize'
valid_ref NOT_MINGW "$ref" '--refspec-pattern --allow-onelevel --normalize'
+ref='HEAD'
+invalid_ref "$ref"
+valid_ref "$ref" --allow-onelevel
+invalid_ref "$ref" --full
+valid_ref "$ref" '--allow-onelevel --full'
+
+valid_ref FETCH_HEAD '--allow-onelevel --full'
+valid_ref refs/foo --full
+invalid_ref HEAD/ '--allow-onelevel --full'
+valid_ref HEAD/ '--allow-onelevel --full --normalize'
+invalid_ref refs '--full'
+invalid_ref refs '--allow-onelevel --full'
+invalid_ref refs/ '--allow-onelevel --full'
+invalid_ref HEAD/foo '--full'
+invalid_ref head '--allow-onelevel --full'
+valid_ref 'refs/foo/*' '--refspec-pattern --full'
+valid_ref 'refs/*/foo' '--refspec-pattern --full'
+
test_expect_success "check-ref-format --branch @{-1}" '
T=$(git write-tree) &&
sha1=$(echo A | git commit-tree $T) &&
--
1.7.7
^ permalink raw reply related
* [RFC 08/13] expand_namespace(): the refname is full, so use REFNAME_FULL option
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
environment.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/environment.c b/environment.c
index 0bee6a7..a6d6f04 100644
--- a/environment.c
+++ b/environment.c
@@ -107,7 +107,7 @@ static char *expand_namespace(const char *raw_namespace)
if (strcmp((*c)->buf, "/") != 0)
strbuf_addf(&buf, "refs/namespaces/%s", (*c)->buf);
strbuf_list_free(components);
- if (check_refname_format(buf.buf, 0))
+ if (check_refname_format(buf.buf, REFNAME_FULL))
die("bad git namespace path \"%s\"", raw_namespace);
strbuf_addch(&buf, '/');
return strbuf_detach(&buf, NULL);
--
1.7.7
^ permalink raw reply related
* [RFC 07/13] one_local_ref(): use check_refname_format(..., REFNAME_FULL)
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Instead of manually skipping over "refs/", let check_refname_format()
handle the whole refname.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
remote.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/remote.c b/remote.c
index e52aa9b..30ee6d8 100644
--- a/remote.c
+++ b/remote.c
@@ -1595,7 +1595,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
int len;
/* we already know it starts with refs/ to get here */
- if (check_refname_format(refname + 5, 0))
+ if (check_refname_format(refname, REFNAME_FULL))
return 0;
len = strlen(refname) + 1;
--
1.7.7
^ permalink raw reply related
* [RFC 09/13] new_branch(): verify that new branch name is a valid full refname
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Is it possible to omit the REFNAME_ALLOW_ONELEVEL option from this
call?
fast-import.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c..51cf898 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -722,7 +722,7 @@ static struct branch *new_branch(const char *name)
if (b)
die("Invalid attempt to create duplicate branch: %s", name);
- if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL))
+ if (check_refname_format(name, REFNAME_FULL|REFNAME_ALLOW_ONELEVEL))
die("Branch name doesn't conform to GIT standards: %s", name);
b = pool_calloc(1, sizeof(struct branch));
--
1.7.7
^ permalink raw reply related
* [RFC 06/13] strbuf_check_branch_ref(): use check_refname_format(..., REFNAME_FULL)
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
The reference name should be a full one, so adjust the check
accordingly.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
sha1_name.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index 03ffc2c..335da37 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -883,7 +883,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
if (name[0] == '-')
return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
- return check_refname_format(sb->buf, 0);
+ return check_refname_format(sb->buf, REFNAME_FULL);
}
/*
--
1.7.7
^ permalink raw reply related
* [RFC 02/13] parse_refname_prefix(): new function
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Add a function parse_refname_prefix() that can read a possible refname
from the front of a string. This is like check_refname_format(),
except:
* It accepts (string, len) parameters and can therefore handle
non-NUL-terminated strings.
* It returns the length of the part of the string that was parsed,
allowing it to be used for refnames that are followed by additional
characters.
Re-implement check_refname_format() using the new function.
Rename function check_refname_component() to parse_refname_component()
for consistency.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
refs.c | 64 ++++++++++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/refs.c b/refs.c
index 3b41252..387da83 100644
--- a/refs.c
+++ b/refs.c
@@ -907,24 +907,30 @@ static inline int bad_ref_char(int ch)
/*
* Try to read one refname component from the front of ref. Return
- * the length of the component found, or -1 if the component is not
- * legal.
+ * the length of the component found, or -1 if the start of the string
+ * cannot be interpreted as a component of a legal refname.
*/
-static int check_refname_component(const char *ref, int flags)
+static int parse_refname_component(const char *ref, int len, int flags)
{
- int i, len = strlen(ref);
+ int i;
char last = '\0';
for (i = 0; i < len; i++) {
char ch = ref[i];
- if (ch == '\0' || ch == '/')
- break;
+ if (ch == '/')
+ break; /* Component terminated by "..". */
if (bad_ref_char(ch))
- return -1; /* Illegal character in refname. */
- if (last == '.' && ch == '.')
- return -1; /* Refname contains "..". */
- if (last == '@' && ch == '{')
- return -1; /* Refname contains "@{". */
+ break; /* Component terminated by illegal character. */
+ if (last == '.' && ch == '.') {
+ /* Component terminated by "..". */
+ i--;
+ break;
+ }
+ if (last == '@' && ch == '{') {
+ /* Component terminated by "@{". */
+ i--;
+ break;
+ }
last = ch;
}
if (i == 0)
@@ -944,17 +950,26 @@ static int check_refname_component(const char *ref, int flags)
return i;
}
-int check_refname_format(const char *ref, int flags)
+/*
+ * Try to interpret the beginning of a string as a refname. Return
+ * the length of the part of the string that could constitute a valid
+ * refname, or -1 if the start of the string cannot possibly be
+ * interpreted as a refname. flags has the same interpretation as for
+ * check_refname_format().
+ */
+static int parse_refname_prefix(const char *ref, int len, int flags)
{
int component_len, component_count = 0;
+ const char *p = ref;
+ int valid_len;
while (1) {
- /* We are at the start of a path component. */
- component_len = check_refname_component(ref, flags);
+ /* p is at the start of a path component. */
+ component_len = parse_refname_component(p, len, flags);
if (component_len < 0) {
if ((flags & REFNAME_REFSPEC_PATTERN) &&
- ref[0] == '*' &&
- (ref[1] == '\0' || ref[1] == '/')) {
+ len && p[0] == '*' &&
+ (len == 1 || p[1] == '/')) {
/* Accept one wildcard as a full refname component. */
flags &= ~REFNAME_REFSPEC_PATTERN;
component_len = 1;
@@ -963,17 +978,26 @@ int check_refname_format(const char *ref, int flags)
}
}
component_count++;
- if (ref[component_len] == '\0')
+ if (component_len == len || p[component_len] != '/')
break;
/* Skip to next component. */
- ref += component_len + 1;
+ p += component_len + 1;
+ len -= component_len + 1;
}
- if (ref[component_len - 1] == '.')
+ valid_len = p + component_len - ref;
+
+ if (ref[valid_len - 1] == '.')
return -1; /* Refname ends with '.'. */
if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
return -1; /* Refname has only one component. */
- return 0;
+ return valid_len;
+}
+
+int check_refname_format(const char *ref, int flags)
+{
+ int len = strlen(ref);
+ return parse_refname_prefix(ref, len, flags) == len ? 0 : -1;
}
const char *prettify_refname(const char *name)
--
1.7.7
^ permalink raw reply related
* [RFC 04/13] add_ref(): move the call of check_refname_format() to callers
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Do not call check_refname_format() in add_ref(); instead change its
callers to do the check. (In fact, don't do any checking in
add_extra_ref(), because that function handles bizarre things like
"refs/tags/3.1.1.1^{}".)
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
I'm still not clear on how extra_refs are used. Are they generated
from local refs or are they generated from remote refs? If the
latter, then it is probably irresponsible not to do *some* sanity
checking in add_extra_ref() to prevent any chance of refnames like
"../../../etc/passwd".
refs.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 8299e51..a40dfa5 100644
--- a/refs.c
+++ b/refs.c
@@ -60,8 +60,6 @@ static void add_ref(const char *name, const unsigned char *sha1,
entry = xmalloc(sizeof(struct ref_entry) + len);
hashcpy(entry->sha1, sha1);
hashclr(entry->peeled);
- if (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
- die("Reference has invalid format: '%s'", name);
memcpy(entry->name, name, len);
entry->flag = flag;
if (new_entry)
@@ -232,6 +230,8 @@ static void read_packed_refs(FILE *f, struct ref_array *array)
name = parse_ref_line(refline, sha1);
if (name) {
+ if (check_refname_format(name, REFNAME_FULL))
+ die("Reference has invalid format: '%s'", name);
add_ref(name, sha1, flag, array, &last);
continue;
}
@@ -336,6 +336,8 @@ static void get_ref_dir(const char *submodule, const char *base,
hashclr(sha1);
flag |= REF_BROKEN;
}
+ if (check_refname_format(ref, REFNAME_FULL))
+ die("Reference has invalid format: '%s'", ref);
add_ref(ref, sha1, flag, array, NULL);
}
free(ref);
--
1.7.7
^ permalink raw reply related
* [RFC 05/13] receive-pack::update(): use check_refname_format(..., REFNAME_FULL)
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
In-Reply-To: <1319057716-28094-1-git-send-email-mhagger@alum.mit.edu>
From: Michael Haggerty <mhagger@alum.mit.edu>
Replace local code with a use of check_refname_format()'s new
REFNAME_FULL option.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
builtin/receive-pack.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 261b610..508451b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -401,7 +401,7 @@ static const char *update(struct command *cmd)
struct ref_lock *lock;
/* only refs/... are allowed */
- if (prefixcmp(name, "refs/") || check_refname_format(name + 5, 0)) {
+ if (check_refname_format(name, REFNAME_FULL)) {
rp_error("refusing to create funny ref '%s' remotely", name);
return "funny refname";
}
--
1.7.7
^ permalink raw reply related
* [RFC 00/13] Checking full vs. partial refnames
From: mhagger @ 2011-10-19 20:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier, Michael Haggerty
From: Michael Haggerty <mhagger@alum.mit.edu>
There are many places where it is necessary to determine whether a
refname is a complete, valid, top-level reference name in the "refs/"
tree, one of the special refnames like "HEAD" or "FETCH_HEAD", or
whether it is potentially a valid fragment of a refname that can be
DWIMed into a true reference. Until now such checks have been
incomplete and/or scattered around.
The first three patches in this series beef up check_refname_format()
(and adds another static function, parse_refname_prefix()) with the
ability to make such checks when the REFNAME_FULL flag is used.
The fourth patch removes the checking of refnames passed to
add_extra_ref(), allowing the function to tolerate oddities like
"refs/tags/3.1.1.1^{}".
The rest of the patches consist of wild-assed guesses about some
callers of check_refname_format() that I suppose can use the
REFNAME_FULL option, thereby tightening up what they accept. About
all I can say is that the test suite passes with these patches
applied. But recent experience indicates that the test suite has a
lot of holes. Therefore, it would be great if experts would look over
these suggestions.
There are many other callers of check_refname_format() that I haven't
touched, because I'm not even brave enough to make wild-assed guesses
about them. (Since I left them without the REFNAME_FULL option, they
rather allow too many references through than too few.) It would be
great if a more experienced developer would look at the other callers
and decide which need to be changed.
BTW, this patch series does *not* fix the specific problem that Junio
mentioned (that "git update-ref tmp/junk HEAD" does not reject the
bogus refname), nor probably many others. The gruelling work is not
this patch series; it is the effort of tracking down all of the code
paths that might try to pass bogus refnames to the refs API.
This patch series applies on top of jc/check-ref-format-fixup, and
also rebases smoothly to the current "next".
Michael Haggerty (13):
check_refname_component(): iterate via index rather than via pointer
parse_refname_prefix(): new function
Teach check_refname_format() to check full refnames
add_ref(): move the call of check_refname_format() to callers
receive-pack::update(): use check_refname_format(..., REFNAME_FULL)
strbuf_check_branch_ref(): use check_refname_format(...,
REFNAME_FULL)
one_local_ref(): use check_refname_format(..., REFNAME_FULL)
expand_namespace(): the refname is full, so use REFNAME_FULL option
new_branch(): verify that new branch name is a valid full refname
strbuf_check_tag_ref(): the refname is full, so use REFNAME_FULL
option
replace_object(): the refname is full, so use REFNAME_FULL option
resolve_ref: use check_refname_format(..., REFNAME_FULL)
filter_refs(): the refname is full, so use REFNAME_FULL option
Documentation/git-check-ref-format.txt | 14 ++++
builtin/check-ref-format.c | 4 +
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/replace.c | 2 +-
builtin/tag.c | 2 +-
environment.c | 2 +-
fast-import.c | 2 +-
refs.c | 124 ++++++++++++++++++++------------
refs.h | 12 ++-
remote.c | 2 +-
sha1_name.c | 2 +-
t/t1402-check-ref-format.sh | 31 ++++++++
13 files changed, 143 insertions(+), 58 deletions(-)
--
1.7.7
^ permalink raw reply
* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-19 20:39 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, cmn, A Large Angry SCM, Daniel Barkalow,
Sverre Rabbelier
In-Reply-To: <4E9F33B9.4040803@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I would have preferred this change being applied on top of your first
> patch in jc/check-ref-format-fixup, because moving these functions to
> refs.c makes sense.
Thanks for a quick sanity check, and I agree.
^ 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