* [PATCH v3 05/25] sequencer: eventually release memory allocated for the option values
From: Johannes Schindelin @ 2016-10-10 17:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1476120229.git.johannes.schindelin@gmx.de>
The sequencer is our attempt to lib-ify cherry-pick. Yet it behaves
like a one-shot command when it reads its configuration: memory is
allocated and released only when the command exits.
This is kind of okay for git-cherry-pick, which *is* a one-shot
command. All the work to make the sequencer its work horse was
done to allow using the functionality as a library function, though,
including proper clean-up after use.
To remedy that, we now take custody of the option values in question,
requiring those values to be malloc()ed or strdup()ed (an alternative
approach, to add a list of pointers to malloc()ed data and to ask the
sequencer to release all of them in the end, was proposed earlier but
rejected during review).
Sadly, the current approach makes the code uglier, as we now have to
take care to strdup() the values passed via the command-line.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/revert.c | 6 ++++++
sequencer.c | 32 ++++++++++++++++++++++++++------
sequencer.h | 6 +++---
3 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 7365559..fce9c75 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -174,6 +174,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
if (argc > 1)
usage_with_options(usage_str, options);
+
+ /* These option values will be free()d */
+ if (opts->gpg_sign)
+ opts->gpg_sign = xstrdup(opts->gpg_sign);
+ if (opts->strategy)
+ opts->strategy = xstrdup(opts->strategy);
}
int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/sequencer.c b/sequencer.c
index 8d272fb..22c31c8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -117,6 +117,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
static void remove_sequencer_state(const struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
+ int i;
+
+ free(opts->gpg_sign);
+ free(opts->strategy);
+ for (i = 0; i < opts->xopts_nr; i++)
+ free(opts->xopts[i]);
+ free(opts->xopts);
strbuf_addf(&dir, "%s", get_dir(opts));
remove_dir_recursively(&dir, 0);
@@ -280,7 +287,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
struct merge_options o;
struct tree *result, *next_tree, *base_tree, *head_tree;
int clean;
- const char **xopt;
+ char **xopt;
static struct lock_file index_lock;
hold_locked_index(&index_lock, 1);
@@ -583,7 +590,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ res |= try_merge_command(opts->strategy,
+ opts->xopts_nr, (const char **)opts->xopts,
common, sha1_to_hex(head), remotes);
free_commit_list(common);
free_commit_list(remotes);
@@ -802,10 +810,22 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
opts->allow_ff = git_config_bool_or_int(key, value, &error_flag);
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
- else if (!strcmp(key, "options.strategy"))
- git_config_string(&opts->strategy, key, value);
- else if (!strcmp(key, "options.gpg-sign"))
- git_config_string(&opts->gpg_sign, key, value);
+ else if (!strcmp(key, "options.strategy")) {
+ if (!value)
+ config_error_nonbool(key);
+ else {
+ free(opts->strategy);
+ opts->strategy = xstrdup(value);
+ }
+ }
+ else if (!strcmp(key, "options.gpg-sign")) {
+ if (!value)
+ config_error_nonbool(key);
+ else {
+ free(opts->gpg_sign);
+ opts->gpg_sign = xstrdup(value);
+ }
+ }
else if (!strcmp(key, "options.strategy-option")) {
ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
opts->xopts[opts->xopts_nr++] = xstrdup(value);
diff --git a/sequencer.h b/sequencer.h
index dd4d33a..8453669 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -34,11 +34,11 @@ struct replay_opts {
int mainline;
- const char *gpg_sign;
+ char *gpg_sign;
/* Merge strategy */
- const char *strategy;
- const char **xopts;
+ char *strategy;
+ char **xopts;
size_t xopts_nr, xopts_alloc;
/* Only used by REPLAY_NONE */
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 03/25] sequencer: avoid unnecessary indirection
From: Johannes Schindelin @ 2016-10-10 17:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1476120229.git.johannes.schindelin@gmx.de>
We really do not need the *pointer to a* pointer to the options in
the read_populate_opts() function.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index cb16cbd..c2fbf6f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -813,7 +813,7 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
return 0;
}
-static int read_populate_opts(struct replay_opts **opts)
+static int read_populate_opts(struct replay_opts *opts)
{
if (!file_exists(git_path_opts_file()))
return 0;
@@ -823,7 +823,7 @@ static int read_populate_opts(struct replay_opts **opts)
* about this case, though, because we wrote that file ourselves, so we
* are pretty certain that it is syntactically correct.
*/
- if (git_config_from_file(populate_opts_cb, git_path_opts_file(), *opts) < 0)
+ if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0)
return error(_("Malformed options sheet: %s"),
git_path_opts_file());
return 0;
@@ -1054,7 +1054,7 @@ static int sequencer_continue(struct replay_opts *opts)
if (!file_exists(git_path_todo_file()))
return continue_single_pick();
- if (read_populate_opts(&opts) ||
+ if (read_populate_opts(opts) ||
read_populate_todo(&todo_list, opts))
return -1;
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 04/25] sequencer: future-proof remove_sequencer_state()
From: Johannes Schindelin @ 2016-10-10 17:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1476120229.git.johannes.schindelin@gmx.de>
In a couple of commits, we will teach the sequencer to handle the
nitty gritty of the interactive rebase, which keeps its state in a
different directory.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
sequencer.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index c2fbf6f..8d272fb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,11 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
+static const char *get_dir(const struct replay_opts *opts)
+{
+ return git_path_seq_dir();
+}
+
static int is_rfc2822_line(const char *buf, int len)
{
int i;
@@ -109,13 +114,13 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
return 1;
}
-static void remove_sequencer_state(void)
+static void remove_sequencer_state(const struct replay_opts *opts)
{
- struct strbuf seq_dir = STRBUF_INIT;
+ struct strbuf dir = STRBUF_INIT;
- strbuf_addstr(&seq_dir, git_path_seq_dir());
- remove_dir_recursively(&seq_dir, 0);
- strbuf_release(&seq_dir);
+ strbuf_addf(&dir, "%s", get_dir(opts));
+ remove_dir_recursively(&dir, 0);
+ strbuf_release(&dir);
}
static const char *action_name(const struct replay_opts *opts)
@@ -940,7 +945,7 @@ static int sequencer_rollback(struct replay_opts *opts)
}
if (reset_for_rollback(sha1))
goto fail;
- remove_sequencer_state();
+ remove_sequencer_state(opts);
strbuf_release(&buf);
return 0;
fail:
@@ -1034,7 +1039,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
*/
- remove_sequencer_state();
+ remove_sequencer_state(opts);
return 0;
}
@@ -1095,7 +1100,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
* one that is being continued
*/
if (opts->subcommand == REPLAY_REMOVE_STATE) {
- remove_sequencer_state();
+ remove_sequencer_state(opts);
return 0;
}
if (opts->subcommand == REPLAY_ROLLBACK)
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 02/25] sequencer: use memoized sequencer directory path
From: Johannes Schindelin @ 2016-10-10 17:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1476120229.git.johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/commit.c | 2 +-
sequencer.c | 11 ++++++-----
sequencer.h | 5 +----
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 1cba3b7..9fddb19 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -183,7 +183,7 @@ static void determine_whence(struct wt_status *s)
whence = FROM_MERGE;
else if (file_exists(git_path_cherry_pick_head())) {
whence = FROM_CHERRY_PICK;
- if (file_exists(git_path(SEQ_DIR)))
+ if (file_exists(git_path_seq_dir()))
sequencer_in_use = 1;
}
else
diff --git a/sequencer.c b/sequencer.c
index eec8a60..cb16cbd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -21,10 +21,11 @@
const char sign_off_header[] = "Signed-off-by: ";
static const char cherry_picked_prefix[] = "(cherry picked from commit ";
-static GIT_PATH_FUNC(git_path_todo_file, SEQ_TODO_FILE)
-static GIT_PATH_FUNC(git_path_opts_file, SEQ_OPTS_FILE)
-static GIT_PATH_FUNC(git_path_seq_dir, SEQ_DIR)
-static GIT_PATH_FUNC(git_path_head_file, SEQ_HEAD_FILE)
+GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
+
+static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
+static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
+static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
static int is_rfc2822_line(const char *buf, int len)
{
@@ -112,7 +113,7 @@ static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
- strbuf_addstr(&seq_dir, git_path(SEQ_DIR));
+ strbuf_addstr(&seq_dir, git_path_seq_dir());
remove_dir_recursively(&seq_dir, 0);
strbuf_release(&seq_dir);
}
diff --git a/sequencer.h b/sequencer.h
index db425ad..dd4d33a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,10 +1,7 @@
#ifndef SEQUENCER_H
#define SEQUENCER_H
-#define SEQ_DIR "sequencer"
-#define SEQ_HEAD_FILE "sequencer/head"
-#define SEQ_TODO_FILE "sequencer/todo"
-#define SEQ_OPTS_FILE "sequencer/opts"
+const char *git_path_seq_dir(void);
#define APPEND_SIGNOFF_DEDUP (1u << 0)
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 01/25] sequencer: use static initializers for replay_opts
From: Johannes Schindelin @ 2016-10-10 17:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1476120229.git.johannes.schindelin@gmx.de>
This change is not completely faithful: instead of initializing all fields
to 0, we choose to initialize command and subcommand to -1 (instead of
defaulting to REPLAY_REVERT and REPLAY_NONE, respectively). Practically,
it makes no difference at all, but future-proofs the code to require
explicit assignments for both fields.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/revert.c | 6 ++----
sequencer.h | 1 +
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 4e69380..7365559 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -178,10 +178,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
int cmd_revert(int argc, const char **argv, const char *prefix)
{
- struct replay_opts opts;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
- memset(&opts, 0, sizeof(opts));
if (isatty(0))
opts.edit = 1;
opts.action = REPLAY_REVERT;
@@ -195,10 +194,9 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
{
- struct replay_opts opts;
+ struct replay_opts opts = REPLAY_OPTS_INIT;
int res;
- memset(&opts, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
parse_args(argc, argv, &opts);
diff --git a/sequencer.h b/sequencer.h
index 5ed5cb1..db425ad 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -47,6 +47,7 @@ struct replay_opts {
/* Only used by REPLAY_NONE */
struct rev_info *revs;
};
+#define REPLAY_OPTS_INIT { -1, -1 }
int sequencer_pick_revisions(struct replay_opts *opts);
--
2.10.0.windows.1.325.ge6089c1
^ permalink raw reply related
* [PATCH v3 00/25] Prepare the sequencer for the upcoming rebase -i patches
From: Johannes Schindelin @ 2016-10-10 17:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narębski, Johannes Sixt
In-Reply-To: <cover.1473590966.git.johannes.schindelin@gmx.de>
This patch series marks the '4' in the countdown to speed up rebase -i
by implementing large parts in C. It is based on the `libify-sequencer`
patch series that I submitted last week.
The patches in this series merely prepare the sequencer code for the
next patch series that actually teaches the sequencer to run an
interactive rebase.
The reason to split these two patch series is simple: to keep them at a
sensible size.
The two patch series after that are much smaller: a two-patch "series"
that switches rebase -i to use the sequencer (except with --root or
--preserve-merges), and a couple of patches to move several pretty
expensive script processing steps to C (think: autosquash).
The end game of this patch series is a git-rebase--helper that makes
rebase -i 5x faster on Windows (according to t/perf/p3404). Travis says
that even MacOSX and Linux benefit (4x and 3x, respectively).
I have been working on this since early February, whenever time allowed,
and it is time to put it into the users' hands. To that end, I will most
likely submit the remaining three patch series in the next two days, and
integrate the whole shebang into Git for Windows 2.10.0.
Therefore I would be most grateful for every in-depth review.
Changes vs v2:
- dramatically simplified the logic to retain do_recursive_merge()'s
return value, even when positive (indicating merge conflicts).
- ensured that write_message() rolls back locked files in case of
errors.
- added a patch to downcase all first letters of sequencer's error
messages, as suggested by Junio.
- replaced 25/25 ("sequencer: remove bogus hint for translators") by a
fix to 8/25 ("completely revamp todo parsing"): the first %s is no
longer "revert" or "cherry-pick", but a "todo" command.
- backed out the sequencer_entrust() function and uglified the code to
always duplicate the option values for the sake of being able to
properly releasing them afterwards.
- CR/LFs are now handled like LFs in todo scripts: they are stripped
(thanks to Hannes Sixt for pointing that out).
- unexported sequencer_commit() (it can safely remain a static
function). While at it, renamed it back to run_git_commit() (the
unspecific name does not hurt if it remains private to sequencer.c).
- clarified in a code comment what read_author_script() does.
- marked for translation the last remaining error message in sequencer.c
that was not yet marked for translation.
Johannes Schindelin (25):
sequencer: use static initializers for replay_opts
sequencer: use memoized sequencer directory path
sequencer: avoid unnecessary indirection
sequencer: future-proof remove_sequencer_state()
sequencer: eventually release memory allocated for the option values
sequencer: future-proof read_populate_todo()
sequencer: completely revamp the "todo" script parsing
sequencer: strip CR from the todo script
sequencer: avoid completely different messages for different actions
sequencer: get rid of the subcommand field
sequencer: refactor the code to obtain a short commit name
sequencer: remember the onelines when parsing the todo file
sequencer: prepare for rebase -i's commit functionality
sequencer: introduce a helper to read files written by scripts
sequencer: allow editing the commit message on a case-by-case basis
sequencer: support amending commits
sequencer: support cleaning up commit messages
sequencer: do not try to commit when there were merge conflicts
sequencer: left-trim lines read from the script
sequencer: refactor write_message()
sequencer: remove overzealous assumption in rebase -i mode
sequencer: mark action_name() for translation
sequencer: quote filenames in error messages
sequencer: start error messages consistently with lower case
sequencer: mark all error messages for translation
builtin/commit.c | 2 +-
builtin/revert.c | 48 +--
sequencer.c | 690 ++++++++++++++++++++++++++++--------------
sequencer.h | 23 +-
t/t3501-revert-cherry-pick.sh | 2 +-
5 files changed, 503 insertions(+), 262 deletions(-)
Published-As: https://github.com/dscho/git/releases/tag/prepare-sequencer-v3
Fetch-It-Via: git fetch https://github.com/dscho/git prepare-sequencer-v3
Interdiff vs v2:
diff --git a/builtin/revert.c b/builtin/revert.c
index c9ae4dc..0a7b5f4 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -165,6 +165,12 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
if (argc > 1)
usage_with_options(usage_str, options);
+ /* These option values will be free()d */
+ if (opts->gpg_sign)
+ opts->gpg_sign = xstrdup(opts->gpg_sign);
+ if (opts->strategy)
+ opts->strategy = xstrdup(opts->strategy);
+
if (cmd == 'q')
return sequencer_remove_state(opts);
if (cmd == 'c')
diff --git a/sequencer.c b/sequencer.c
index cdff0f1..86d86ce 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -148,23 +148,15 @@ static const char *gpg_sign_opt_quoted(struct replay_opts *opts)
return buf.buf;
}
-void *sequencer_entrust(struct replay_opts *opts, void *to_free)
-{
- ALLOC_GROW(opts->owned, opts->owned_nr + 1, opts->owned_alloc);
- opts->owned[opts->owned_nr++] = to_free;
-
- return to_free;
-}
-
int sequencer_remove_state(struct replay_opts *opts)
{
struct strbuf dir = STRBUF_INIT;
int i;
- for (i = 0; i < opts->owned_nr; i++)
- free(opts->owned[i]);
- free(opts->owned);
-
+ free(opts->gpg_sign);
+ free(opts->strategy);
+ for (i = 0; i < opts->xopts_nr; i++)
+ free(opts->xopts[i]);
free(opts->xopts);
strbuf_addf(&dir, "%s", get_dir(opts));
@@ -249,13 +241,19 @@ static int write_with_lock_file(const char *filename,
int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
if (msg_fd < 0)
- return error_errno(_("Could not lock '%s'"), filename);
- if (write_in_full(msg_fd, buf, len) < 0)
- return error_errno(_("Could not write to '%s'"), filename);
- if (append_eol && write(msg_fd, "\n", 1) < 0)
- return error_errno(_("Could not write eol to '%s"), filename);
- if (commit_lock_file(&msg_file) < 0)
- return error(_("Error wrapping up '%s'."), filename);
+ return error_errno(_("could not lock '%s'"), filename);
+ if (write_in_full(msg_fd, buf, len) < 0) {
+ rollback_lock_file(&msg_file);
+ return error_errno(_("could not write to '%s'"), filename);
+ }
+ if (append_eol && write(msg_fd, "\n", 1) < 0) {
+ rollback_lock_file(&msg_file);
+ return error_errno(_("could not write eol to '%s"), filename);
+ }
+ if (commit_lock_file(&msg_file) < 0) {
+ rollback_lock_file(&msg_file);
+ return error(_("failed to finalize '%s'."), filename);
+ }
return 0;
}
@@ -288,7 +286,7 @@ static int read_oneliner(struct strbuf *buf,
return 0;
if (strbuf_read_file(buf, path, 0) < 0) {
- warning_errno("could not read '%s'", path);
+ warning_errno(_("could not read '%s'"), path);
return 0;
}
@@ -314,11 +312,11 @@ static int error_dirty_index(struct replay_opts *opts)
if (read_cache_unmerged())
return error_resolve_conflict(_(action_name(opts)));
- error(_("Your local changes would be overwritten by %s."),
+ error(_("your local changes would be overwritten by %s."),
_(action_name(opts)));
if (advice_commit_before_merge)
- advise(_("Commit your changes or stash them to proceed."));
+ advise(_("commit your changes or stash them to proceed."));
return -1;
}
@@ -379,7 +377,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
struct merge_options o;
struct tree *result, *next_tree, *base_tree, *head_tree;
int clean;
- const char **xopt;
+ char **xopt;
static struct lock_file index_lock;
hold_locked_index(&index_lock, 1);
@@ -427,7 +425,7 @@ static int is_index_unchanged(void)
struct commit *head_commit;
if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
- return error(_("Could not resolve HEAD commit\n"));
+ return error(_("could not resolve HEAD commit\n"));
head_commit = lookup_commit(head_sha1);
@@ -447,11 +445,15 @@ static int is_index_unchanged(void)
if (!cache_tree_fully_valid(active_cache_tree))
if (cache_tree_update(&the_index, 0))
- return error(_("Unable to update cache tree\n"));
+ return error(_("unable to update cache tree\n"));
return !hashcmp(active_cache_tree->sha1, head_commit->tree->object.oid.hash);
}
+/*
+ * Read the author-script file into an environment block, ready for use in
+ * run_command(), that can be free()d afterwards.
+ */
static char **read_author_script(void)
{
struct strbuf script = STRBUF_INIT;
@@ -495,11 +497,11 @@ static char **read_author_script(void)
* If we are revert, or if our cherry-pick results in a hand merge,
* we had better say that the current user is responsible for that.
*
- * An exception is when sequencer_commit() is called during an
+ * An exception is when run_git_commit() is called during an
* interactive rebase: in that case, we will want to retain the
* author metadata.
*/
-int sequencer_commit(const char *defmsg, struct replay_opts *opts,
+static int run_git_commit(const char *defmsg, struct replay_opts *opts,
int allow_empty, int edit, int amend,
int cleanup_commit_message)
{
@@ -513,16 +515,19 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
if (!env) {
const char *gpg_opt = gpg_sign_opt_quoted(opts);
- return error("You have staged changes in your working "
- "tree. If these changes are meant to be\n"
- "squashed into the previous commit, run:\n\n"
- " git commit --amend %s\n\n"
- "If they are meant to go into a new commit, "
- "run:\n\n"
- " git commit %s\n\n"
- "In both cases, once you're done, continue "
- "with:\n\n"
- " git rebase --continue\n", gpg_opt, gpg_opt);
+ return error(_("you have staged changes in your "
+ "working tree. If these changes are "
+ "meant to be\n"
+ "squashed into the previous commit, "
+ "run:\n\n"
+ " git commit --amend %s\n\n"
+ "If they are meant to go into a new "
+ "commit, run:\n\n"
+ " git commit %s\n\n"
+ "In both cases, once you're done, "
+ "continue with:\n\n"
+ " git rebase --continue\n"),
+ gpg_opt, gpg_opt);
}
}
@@ -566,12 +571,12 @@ static int is_original_commit_empty(struct commit *commit)
const unsigned char *ptree_sha1;
if (parse_commit(commit))
- return error(_("Could not parse commit %s\n"),
+ return error(_("could not parse commit %s\n"),
oid_to_hex(&commit->object.oid));
if (commit->parents) {
struct commit *parent = commit->parents->item;
if (parse_commit(parent))
- return error(_("Could not parse parent commit %s\n"),
+ return error(_("could not parse parent commit %s\n"),
oid_to_hex(&parent->object.oid));
ptree_sha1 = parent->tree->object.oid.hash;
} else {
@@ -645,7 +650,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
const char *base_label, *next_label;
struct commit_message msg = { NULL, NULL, NULL, NULL };
struct strbuf msgbuf = STRBUF_INIT;
- int res = 0, unborn = 0, allow;
+ int res, unborn = 0, allow;
if (opts->no_commit) {
/*
@@ -655,7 +660,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
* to work on.
*/
if (write_cache_as_tree(head, 0, NULL))
- return error(_("Your index file is unmerged."));
+ return error(_("your index file is unmerged."));
} else {
unborn = get_sha1("HEAD", head);
if (unborn)
@@ -674,7 +679,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
struct commit_list *p;
if (!opts->mainline)
- return error(_("Commit %s is a merge but no -m option was given."),
+ return error(_("commit %s is a merge but no -m option was given."),
oid_to_hex(&commit->object.oid));
for (cnt = 1, p = commit->parents;
@@ -682,11 +687,11 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
cnt++)
p = p->next;
if (cnt != opts->mainline || !p)
- return error(_("Commit %s does not have parent %d"),
+ return error(_("commit %s does not have parent %d"),
oid_to_hex(&commit->object.oid), opts->mainline);
parent = p->item;
} else if (0 < opts->mainline)
- return error(_("Mainline was specified but commit %s is not a merge."),
+ return error(_("mainline was specified but commit %s is not a merge."),
oid_to_hex(&commit->object.oid));
else
parent = commit->parents->item;
@@ -697,12 +702,16 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
return fast_forward_to(commit->object.oid.hash, head, unborn, opts);
if (parent && parse_commit(parent) < 0)
+ /*
+ * TRANSLATORS: The first %s will be a "todo" command like
+ * "revert" or "pick", the second %s a SHA1.
+ */
return error(_("%s: cannot parse parent commit %s"),
command_to_string(command),
oid_to_hex(&parent->object.oid));
if (get_message(commit, &msg) != 0)
- return error(_("Cannot get commit message for %s"),
+ return error(_("cannot get commit message for %s"),
oid_to_hex(&commit->object.oid));
/*
@@ -754,7 +763,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
}
if (!opts->strategy || !strcmp(opts->strategy, "recursive") || command == TODO_REVERT) {
- res |= do_recursive_merge(base, next, base_label, next_label,
+ res = do_recursive_merge(base, next, base_label, next_label,
head, &msgbuf, opts);
if (res < 0)
return res;
@@ -763,11 +772,12 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
- res |= write_message(&msgbuf, git_path_merge_msg());
+ res = write_message(&msgbuf, git_path_merge_msg());
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,
+ res |= try_merge_command(opts->strategy,
+ opts->xopts_nr, (const char **)opts->xopts,
common, sha1_to_hex(head), remotes);
free_commit_list(common);
free_commit_list(remotes);
@@ -800,13 +810,12 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
allow = allow_empty(opts, commit);
if (allow < 0) {
- res |= allow;
+ res = allow;
goto leave;
}
- if (!opts->no_commit)
- res |= sequencer_commit(opts->edit ?
- NULL : git_path_merge_msg(),
- opts, allow, opts->edit, 0, 0);
+ if (!res && !opts->no_commit)
+ res = run_git_commit(opts->edit ? NULL : git_path_merge_msg(),
+ opts, allow, opts->edit, 0, 0);
leave:
free_message(commit, &msg);
@@ -924,23 +933,27 @@ static int parse_insn_line(struct todo_item *item, const char *bol, char *eol)
static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
{
struct todo_item *item;
- char *p = buf;
+ char *p = buf, *next_p;
int i, res = 0;
- for (i = 1; *p; i++) {
+ for (i = 1; *p; i++, p = next_p) {
char *eol = strchrnul(p, '\n');
+ next_p = *eol ? eol + 1 /* skip LF */ : eol;
+
+ if (p != eol && eol[-1] == '\r')
+ eol--; /* skip Carriage Return */
+
item = append_new_todo(todo_list);
item->offset_in_buf = p - todo_list->buf.buf;
if (parse_insn_line(item, p, eol)) {
- res |= error(_("Invalid line %d: %.*s"),
+ res = error(_("invalid line %d: %.*s"),
i, (int)(eol - p), p);
item->command = -1;
}
- p = *eol ? eol + 1 : eol;
}
if (!todo_list->nr)
- return error(_("No commits parsed."));
+ return error(_("no commits parsed."));
return res;
}
@@ -953,16 +966,16 @@ static int read_populate_todo(struct todo_list *todo_list,
strbuf_reset(&todo_list->buf);
fd = open(todo_file, O_RDONLY);
if (fd < 0)
- return error_errno(_("Could not open '%s'"), todo_file);
+ return error_errno(_("could not open '%s'"), todo_file);
if (strbuf_read(&todo_list->buf, fd, 0) < 0) {
close(fd);
- return error(_("Could not read '%s'."), todo_file);
+ return error(_("could not read '%s'."), todo_file);
}
close(fd);
res = parse_insn_buffer(todo_list->buf.buf, todo_list);
if (res)
- return error(_("Unusable instruction sheet: '%s'"), todo_file);
+ return error(_("unusable instruction sheet: '%s'"), todo_file);
if (!is_rebase_i(opts)) {
enum todo_command valid =
@@ -973,9 +986,9 @@ static int read_populate_todo(struct todo_list *todo_list,
if (valid == todo_list->items[i].command)
continue;
else if (valid == TODO_PICK)
- return error(_("Cannot cherry-pick during a revert."));
+ return error(_("cannot cherry-pick during a revert."));
else
- return error(_("Cannot revert during a cherry-pick."));
+ return error(_("cannot revert during a cherry-pick."));
}
return 0;
@@ -1001,22 +1014,29 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
else if (!strcmp(key, "options.mainline"))
opts->mainline = git_config_int(key, value);
else if (!strcmp(key, "options.strategy")) {
- git_config_string(&opts->strategy, key, value);
- sequencer_entrust(opts, (char *) opts->strategy);
+ if (!value)
+ config_error_nonbool(key);
+ else {
+ free(opts->strategy);
+ opts->strategy = xstrdup(value);
+ }
}
else if (!strcmp(key, "options.gpg-sign")) {
- git_config_string(&opts->gpg_sign, key, value);
- sequencer_entrust(opts, (char *) opts->gpg_sign);
+ if (!value)
+ config_error_nonbool(key);
+ else {
+ free(opts->gpg_sign);
+ opts->gpg_sign = xstrdup(value);
+ }
}
else if (!strcmp(key, "options.strategy-option")) {
ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
- opts->xopts[opts->xopts_nr++] =
- sequencer_entrust(opts, xstrdup(value));
+ opts->xopts[opts->xopts_nr++] = xstrdup(value);
} else
- return error(_("Invalid key: %s"), key);
+ return error(_("invalid key: %s"), key);
if (!error_flag)
- return error(_("Invalid value for %s: %s"), key, value);
+ return error(_("invalid value for %s: %s"), key, value);
return 0;
}
@@ -1030,11 +1050,11 @@ static int read_populate_opts(struct replay_opts *opts)
if (!starts_with(buf.buf, "-S"))
strbuf_reset(&buf);
else {
- opts->gpg_sign = buf.buf + 2;
- sequencer_entrust(opts,
- strbuf_detach(&buf, NULL));
+ free(opts->gpg_sign);
+ opts->gpg_sign = xstrdup(buf.buf + 2);
}
}
+ strbuf_release(&buf);
return 0;
}
@@ -1048,7 +1068,7 @@ static int read_populate_opts(struct replay_opts *opts)
* are pretty certain that it is syntactically correct.
*/
if (git_config_from_file(populate_opts_cb, git_path_opts_file(), opts) < 0)
- return error(_("Malformed options sheet: '%s'"),
+ return error(_("malformed options sheet: '%s'"),
git_path_opts_file());
return 0;
}
@@ -1091,7 +1111,7 @@ static int create_seq_dir(void)
return -1;
}
else if (mkdir(git_path_seq_dir(), 0777) < 0)
- return error_errno(_("Could not create sequencer directory '%s'"),
+ return error_errno(_("could not create sequencer directory '%s'"),
git_path_seq_dir());
return 0;
}
@@ -1105,17 +1125,17 @@ static int save_head(const char *head)
fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
if (fd < 0) {
rollback_lock_file(&head_lock);
- return error_errno(_("Could not lock HEAD"));
+ return error_errno(_("could not lock HEAD"));
}
strbuf_addf(&buf, "%s\n", head);
if (write_in_full(fd, buf.buf, buf.len) < 0) {
rollback_lock_file(&head_lock);
- return error_errno(_("Could not write to '%s'"),
+ return error_errno(_("could not write to '%s'"),
git_path_head_file());
}
if (commit_lock_file(&head_lock) < 0) {
rollback_lock_file(&head_lock);
- return error(_("Error wrapping up '%s'."), git_path_head_file());
+ return error(_("failed to finalize '%s'."), git_path_head_file());
}
return 0;
}
@@ -1194,14 +1214,14 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
if (fd < 0)
- return error_errno(_("Could not lock '%s'"), todo_path);
+ return error_errno(_("could not lock '%s'"), todo_path);
offset = next < todo_list->nr ?
todo_list->items[next].offset_in_buf : todo_list->buf.len;
if (write_in_full(fd, todo_list->buf.buf + offset,
todo_list->buf.len - offset) < 0)
- return error_errno(_("Could not write to '%s'"), todo_path);
+ return error_errno(_("could not write to '%s'"), todo_path);
if (commit_lock_file(&todo_lock) < 0)
- return error(_("Error wrapping up '%s'."), todo_path);
+ return error(_("failed to finalize '%s'."), todo_path);
return 0;
}
@@ -1376,7 +1396,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
create_seq_dir() < 0)
return -1;
if (get_sha1("HEAD", sha1) && (opts->action == REPLAY_REVERT))
- return error(_("Can't revert as initial commit"));
+ return error(_("can't revert as initial commit"));
if (save_head(sha1_to_hex(sha1)))
return -1;
if (save_opts(opts))
diff --git a/sequencer.h b/sequencer.h
index 688fff1..7a513c5 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -26,37 +26,23 @@ struct replay_opts {
int mainline;
- const char *gpg_sign;
+ char *gpg_sign;
/* Merge strategy */
- const char *strategy;
- const char **xopts;
+ char *strategy;
+ char **xopts;
size_t xopts_nr, xopts_alloc;
/* Only used by REPLAY_NONE */
struct rev_info *revs;
-
- /* malloc()ed data entrusted to the sequencer */
- void **owned;
- int owned_nr, owned_alloc;
};
#define REPLAY_OPTS_INIT { -1 }
-/*
- * Make it the duty of sequencer_remove_state() to release the memory;
- * For ease of use, return the same pointer.
- */
-void *sequencer_entrust(struct replay_opts *opts, void *to_free);
-
int sequencer_pick_revisions(struct replay_opts *opts);
int sequencer_continue(struct replay_opts *opts);
int sequencer_rollback(struct replay_opts *opts);
int sequencer_remove_state(struct replay_opts *opts);
-int sequencer_commit(const char *defmsg, struct replay_opts *opts,
- int allow_empty, int edit, int amend,
- int cleanup_commit_message);
-
extern const char sign_off_header[];
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 51f3bbb..394f000 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -96,7 +96,7 @@ test_expect_success 'revert forbidden on dirty working tree' '
echo content >extra_file &&
git add extra_file &&
test_must_fail git revert HEAD 2>errors &&
- test_i18ngrep "Your local changes would be overwritten by " errors
+ test_i18ngrep "your local changes would be overwritten by " errors
'
--
2.10.0.windows.1.325.ge6089c1
base-commit: a23ca1b8dc42ffd4de2ef30d67ce1e21ded29886
^ permalink raw reply
* Re: [PATCH] pretty: respect color settings for %C placeholders
From: René Scharfe @ 2016-10-10 17:09 UTC (permalink / raw)
To: Jeff King, Duy Nguyen; +Cc: Tom Hale, git, Junio C Hamano
In-Reply-To: <20161010151517.6wszhuyp57yfncaj@sigill.intra.peff.net>
Am 10.10.2016 um 17:15 schrieb Jeff King:
> On Mon, Oct 10, 2016 at 10:28:18AM -0400, Jeff King wrote:
>
>>> We could add some new tag to change the behavior of all following %C
>>> tags. Something like %C(tty) maybe (probably a bad name), then
>>> discourage the use if "%C(auto" for terminal detection?
>>
>> Yeah, adding a "%C(enable-auto-color)" or something would be backwards
>> compatible and less painful than using "%C(auto)" everywhere. I do
>> wonder if anybody actually _wants_ the "always show color, even if
>> --no-color" behavior. I'm having trouble thinking of a good use for it.
>>
>> IOW, I'm wondering if anyone would disagree that the current behavior is
>> simply buggy. Reading the thread at:
>>
>> http://public-inbox.org/git/7v4njkmq07.fsf@alter.siamese.dyndns.org/
>>
>> I don't really see any compelling reason against it (there was some
>> question of which config to use, but we already answered that with
>> "%C(auto)", and use the value from the pretty_ctx).
>
> So here's what a patch to do that would look like. I admit that "I can't
> think of a good use" does not mean there _isn't_ one, but perhaps by
> posting this, it might provoke other people to think on it, too. And if
> nobody can come up with, maybe it's a good idea.
Color tags that respect the config and the --color option make the most
sense to me as well.
Nevertheless a possible counterpoint: Coloring is used in commands that
are intended for human consumption. Most of the time there is no need
to to conserve the output in a file. But even then, and of course with
pipes, once you look at it using less -R or grep you still get nice
colored lines and not a monochrome wall of text.
> -- >8 --
> Subject: pretty: respect color settings for %C placeholders
>
> The color placeholders have traditionally been
> unconditional, showing colors even when git is not otherwise
> configured to do so. This was not so bad for their original
> use, which was on the command-line (and the user could
> decide at that moment whether to add colors or not). But
> these days we have configured formats via pretty.*, and
> those should operate in multiple contexts.
>
> In 3082517 (log --format: teach %C(auto,black) to respect
> color config, 2012-12-17), we gave an extended placeholder
> that could be used to accomplish this. But it's rather
> clunky to use, because you have to specify it individually
> for each color (and their matching resets) in the format.
> We shied away from just switching the default to auto,
> because it is technically breaking backwards compatibility.
>
> However, there's not really a use case for unconditional
> colors. The most plausible reason you would want them
> unconditional is to redirect "git log" output to a file. But
> there, the right answer is --color=always, as it does the
> right thing both with custom user-format colors and
> git-generated colors.
>
> So let's switch to the more useful default. In the
> off-chance that somebody really does find a use for
> unconditional colors without wanting to enable the rest of
> git's colors, we can provide %C(always,...) to enable the
> old behavior.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> The tests unsurprisingly needed updating, as we're breaking the old
> behavior. The diff is easier to read read with "-w".
>
> Documentation/pretty-formats.txt | 13 +++---
> pretty.c | 19 +++++---
> t/t6006-rev-list-format.sh | 94 ++++++++++++++++++++--------------------
> 3 files changed, 70 insertions(+), 56 deletions(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index a942d57..7aa1a8b 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -167,11 +167,14 @@ endif::git-rev-list[]
> - '%Cblue': switch color to blue
> - '%Creset': reset color
> - '%C(...)': color specification, as described in color.branch.* config option;
> - adding `auto,` at the beginning will emit color only when colors are
> - enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
> - respecting the `auto` settings of the former if we are going to a
> - terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
> - on the next placeholders until the color is switched again.
> + By default, colors are shown only when enabled for log output (by
> + `color.diff`, `color.ui`, or `--color`, and respecting the `auto`
> + settings of the former if we are going to a terminal). `%C(auto,...)`
> + is accepted as a historical synonym for the default. Specifying
> + `%C(always,...) will show the colors always, even when colors are not
> + otherwise enabled (to enable this behavior for the whole format, use
> + `--color=always`). `auto` alone (i.e. `%C(auto)`) will turn on auto
> + coloring on the next placeholders until the color is switched again.
> - '%m': left (`<`), right (`>`) or boundary (`-`) mark
> - '%n': newline
> - '%%': a raw '%'
> diff --git a/pretty.c b/pretty.c
> index 25efbca..73e58b5 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
>
> if (!end)
> return 0;
> - if (skip_prefix(begin, "auto,", &begin)) {
> +
> + if (!skip_prefix(begin, "always,", &begin)) {
> if (!want_color(c->pretty_ctx->color))
> return end - placeholder + 1;
> }
Shouldn't we have an "else" here?
> +
> + /* this is a historical noop */
> + skip_prefix(begin, "auto,", &begin);
> +
> if (color_parse_mem(begin, end - begin, color) < 0)
> die(_("unable to parse --pretty format"));
> strbuf_addstr(sb, color);
> return end - placeholder + 1;
> }
> - if (skip_prefix(placeholder + 1, "red", &rest))
> + if (skip_prefix(placeholder + 1, "red", &rest) &&
> + want_color(c->pretty_ctx->color))
> strbuf_addstr(sb, GIT_COLOR_RED);
> - else if (skip_prefix(placeholder + 1, "green", &rest))
> + else if (skip_prefix(placeholder + 1, "green", &rest) &&
> + want_color(c->pretty_ctx->color))
> strbuf_addstr(sb, GIT_COLOR_GREEN);
> - else if (skip_prefix(placeholder + 1, "blue", &rest))
> + else if (skip_prefix(placeholder + 1, "blue", &rest) &&
> + want_color(c->pretty_ctx->color))
> strbuf_addstr(sb, GIT_COLOR_BLUE);
> - else if (skip_prefix(placeholder + 1, "reset", &rest))
> + else if (skip_prefix(placeholder + 1, "reset", &rest) &&
> + want_color(c->pretty_ctx->color))
> strbuf_addstr(sb, GIT_COLOR_RESET);
> return rest - placeholder;
> }
Perhaps it's a funtion like add_color(sb, ctx, color) or similar would
be nice?
René
^ permalink raw reply
* Re: [PATCH] t3700-add.sh: Avoid filename collission between tests which could lead to test failure
From: Jeremy Huddleston Sequoia @ 2016-10-10 17:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Thomas Gummerer
In-Reply-To: <xmqqlgxwgnmu.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]
> On Oct 10, 2016, at 09:44, Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
>
>> Regressed-in: 610d55af0f082f6b866dc858e144c03d8ed4424c
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>> CC: Thomas Gummerer <t.gummerer@gmail.com>
>> CC: Junio C Hamano <gitster@pobox.com>
>
> This is also under-explained. Was the test broken for everybody and
> you are the first to report, or was your environment somehow different
> that the existing problem was revealed there first?
There is nothing obviously different about my environment. I'd expect this test to fail for everyone that runs the entire test because xfoo3 is not cleaned up after this earlier test:
test_expect_success \
'git update-index --add: Test that executable bit is not used...' \
'git config core.filemode 0 &&
test_ln_s_add xfoo2 xfoo3 && # runs git update-index --add
test_mode_in_index 120000 xfoo3'
If anyone ran the test by itself or otherwise skipped the 'git update-index --add: Test that executable bit is not used...' test, they would not encounter the issue. However, the normal case is to run all of the tests, so I would expect everyone to be tripping over this issue. If they're not, then that itself might be indicative of another bug because clearly that previous test just added xfoo3 as a symlink.
--Jeremy
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* Re: [PATCH] remote.c: free previous results when looking for a ref match
From: Stefan Beller @ 2016-10-10 17:03 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <e9f58e04-a534-6794-feac-f6a47ca8bb65@web.de>
On Sat, Oct 8, 2016 at 6:38 AM, René Scharfe <l.s.r@web.de> wrote:
>
> Again, this check is not necessary. If I read the code correctly the
> pointer could be uninitialized at that point, though, causing free(3) to
> crash.
yep, this patch is bogus.
^ permalink raw reply
* Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Jeremy Huddleston Sequoia @ 2016-10-10 17:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Josh Triplett
In-Reply-To: <xmqqpon8gnp3.fsf@gitster.mtv.corp.google.com>
[-- Attachment #1: Type: text/plain, Size: 3355 bytes --]
> On Oct 10, 2016, at 09:42, Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
>
>> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>
> Please be considerate to future readers of "git log" to help them
> avoid mistakes earlier commits made that caused you troubles.
>
> This line by itself without any explanation of what was broken is
> quite useless as a commit message. What can the readers do? They'd
> go back and say "git show 480871e09" and then what? The test added
> or modified by the commit has been working quite well for others
> since it was made, so it is very likely the reader wouldn't be able
> to tell if anything is wrong in it.
>
> You would help if you said what is different in _your_ environment
> from others who have happily been running and passing this test for
> others to understand and learn from your fix. What is it?
>
> The "Adjust ... distro changes" in the title offers some hint but it
> could be more explicit. Please write something along this line
> instead.
>
> Subject: t4014: git --version can have SP in it
>
> 480871e09e ("format-patch: show base info before email
> signature", 2016-09-07) added a helper function to recreate the
> signature at the end of the e-mail, i.e. "-- " line followed by
> the version string of Git, using output from "git --version" and
> stripping everything before the last SP.
>
> Because the default Git version string looks like "git version
> 2.10.0-1-g480871e09e", this was mostly OK, but people can change
> this version string to arbitrary thing while compiling, which
> can break the assumption if they had SP in it. Notably, Apple
> ships modified Git with " (Apple Git-xx)" appended to its
> version number.
>
> Instead, come up with the version string by stripping the "git
> version " from the beginning.
Ok, I'll add that explanation to the commit message, thanks.
>
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>
> Good.
>
>> CC: Josh Triplett <josh@joshtriplett.org>
>> CC: Junio C Hamano <gitster@pobox.com>
>
> Please don't do this in your log message. These belong to your
> e-mail headers, not here.
Sorry, we do that consistently on other projects during the review process, so git send-email picks them up and adds them automatically for each patch being sent out. It's quite useful, actually.
>
>> ---
>> t/t4014-format-patch.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index 8d90a6e..33f6940 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -754,7 +754,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
>> git format-patch --ignore-if-in-upstream HEAD
>> '
>>
>> -git_version="$(git --version | sed "s/.* //")"
>> +git_version="$(git --version | sed "s/git version //")"
>
> Anchor the fixed prefix to the beginning, so that we can protect
> ourselves from another distro that would add "git version" in the
> middle of their arbitrary versioning scheme ;-). I.e.
>
> sed "s/^git version //"
Ok.
>
>>
>> signature() {
>> printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* RE: git merge deletes my changes
From: Eduard Egorov @ 2016-10-10 10:19 UTC (permalink / raw)
To: 'git@vger.kernel.org'
In-Reply-To: <AM4PR03MB1636BE3423E2BC4F0E998159DBDB0@AM4PR03MB1636.eurprd03.prod.outlook.com>
Hello again,
I've noticed that my git is quite old (1.8.3) and built it from source tarball (2.10.1). Now the output is:
# ~/gitbuild/git-2.10.1/git merge -s subtree --squash ceph_ansible
fatal: refusing to merge unrelated histories
I've checked my command history again:
738 git rm -rf ceph-ansible/
739 ll
740 ll ceph-ansible/
741 rm ceph-ansible/purge_ceph_cluster.yml
742 git read-tree --prefix=ceph-ansible/ -u ceph_ansible
743 gs
744 gc "ceph-ansible: reset repo state"
A quick googling showed (http://stackoverflow.com/questions/37937984/git-refusing-to-merge-unrelated-histories ) that the default behavior is changed. Adding ' --allow-unrelated-histories' clears the error message but the merge itself is still wrong (my changes are lost).
This error message might explain why git can't merge it correctly (since these repos doesn't has any relations, right). Can somebody confirm this please? Doesn't "merge -s subtree" really merges branches?
With best regards
Eduard Egorov
-----Original Message-----
From: Eduard Egorov
Sent: Monday, October 10, 2016 12:39 PM
To: 'git@vger.kernel.org' <git@vger.kernel.org>
Subject: git merge deletes my changes
Hello Git experts,
A week ago, I've reset a state of 'ceph-ansible' folder in %current% branch with code from corresponding branch (that tracks an upstream from github):
# git read-tree --prefix=ceph-ansible/ -u ceph_ansible
Then I've committed several changes, including:
1. Renamed file and commited:
# git mv site.yml.sample site.yml
2. Made some changes and committed
3. Pulled updates from original branch by:
# git merge -s subtree --squash ceph_ansible
It said:
Auto-merging ceph-ansible/site.yml.sample
blablabla
Squash commit -- not updating HEAD
Automatic merge went well; stopped before committing as requested
I can see that "my" ceph-ansible/site.yml is deleted (as well as few new files added by me), ceph-ansible/site.yml.sample is restored and doesn't contain my changes (it's just restored to the state before my changes).
`git log ceph_ansible site.yml.sample` shows the latest change on ceph_ansible branch done from 26th of August, 2016, my changes in %current% branch was this October, so it shouldn't be any conflict either (even if it was).
I believe there is some obvious explanation for this behavior. Could you help me please? What information can I provide in order to troubleshoot this issue?
A post on SO: http://stackoverflow.com/questions/39954265/git-merge-deletes-my-changes
With best regards
Eduard Egorov
^ permalink raw reply
* Re: [PATCH] t3700-add.sh: Avoid filename collission between tests which could lead to test failure
From: Junio C Hamano @ 2016-10-10 16:44 UTC (permalink / raw)
To: Jeremy Huddleston Sequoia; +Cc: git, Thomas Gummerer
In-Reply-To: <20161010035756.38408-1-jeremyhu@apple.com>
Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
> Regressed-in: 610d55af0f082f6b866dc858e144c03d8ed4424c
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> CC: Thomas Gummerer <t.gummerer@gmail.com>
> CC: Junio C Hamano <gitster@pobox.com>
This is also under-explained. Was the test broken for everybody and
you are the first to report, or was your environment somehow different
that the existing problem was revealed there first?
^ permalink raw reply
* Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Junio C Hamano @ 2016-10-10 16:46 UTC (permalink / raw)
To: Jeff King; +Cc: Jeremy Huddleston Sequoia, git, Josh Triplett
In-Reply-To: <20161010131041.lpdh4a7nol24hsz2@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Oct 09, 2016 at 07:53:23PM -0700, Jeremy Huddleston Sequoia wrote:
>
>> Subject: Re: [PATCH] t4014-format-patch: Adjust git_version regex to better
>> handle distro changes to DEF_VER
>>
>> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>
> I see there was a discussion elsewhere on the list about exactly what
> you are putting into DEF_VAR that causes the problem. Perhaps the commit
> message here would be a good place to mention that, why the current
> regex breaks it, and why your new version fixes not only it, but other
> possible values of DEF_VAR.
Ah, should have known that you'd already be helping with this.
Thanks.
^ permalink raw reply
* Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Junio C Hamano @ 2016-10-10 16:42 UTC (permalink / raw)
To: Jeremy Huddleston Sequoia; +Cc: git, Josh Triplett
In-Reply-To: <20161010025323.9415-1-jeremyhu@apple.com>
Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
Please be considerate to future readers of "git log" to help them
avoid mistakes earlier commits made that caused you troubles.
This line by itself without any explanation of what was broken is
quite useless as a commit message. What can the readers do? They'd
go back and say "git show 480871e09" and then what? The test added
or modified by the commit has been working quite well for others
since it was made, so it is very likely the reader wouldn't be able
to tell if anything is wrong in it.
You would help if you said what is different in _your_ environment
from others who have happily been running and passing this test for
others to understand and learn from your fix. What is it?
The "Adjust ... distro changes" in the title offers some hint but it
could be more explicit. Please write something along this line
instead.
Subject: t4014: git --version can have SP in it
480871e09e ("format-patch: show base info before email
signature", 2016-09-07) added a helper function to recreate the
signature at the end of the e-mail, i.e. "-- " line followed by
the version string of Git, using output from "git --version" and
stripping everything before the last SP.
Because the default Git version string looks like "git version
2.10.0-1-g480871e09e", this was mostly OK, but people can change
this version string to arbitrary thing while compiling, which
can break the assumption if they had SP in it. Notably, Apple
ships modified Git with " (Apple Git-xx)" appended to its
version number.
Instead, come up with the version string by stripping the "git
version " from the beginning.
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Good.
> CC: Josh Triplett <josh@joshtriplett.org>
> CC: Junio C Hamano <gitster@pobox.com>
Please don't do this in your log message. These belong to your
e-mail headers, not here.
> ---
> t/t4014-format-patch.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 8d90a6e..33f6940 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -754,7 +754,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream HEAD' '
> git format-patch --ignore-if-in-upstream HEAD
> '
>
> -git_version="$(git --version | sed "s/.* //")"
> +git_version="$(git --version | sed "s/git version //")"
Anchor the fixed prefix to the beginning, so that we can protect
ourselves from another distro that would add "git version" in the
middle of their arbitrary versioning scheme ;-). I.e.
sed "s/^git version //"
>
> signature() {
> printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
^ permalink raw reply
* Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Jeremy Huddleston Sequoia @ 2016-10-10 16:37 UTC (permalink / raw)
To: Jeff King; +Cc: git, Josh Triplett, Junio C Hamano
In-Reply-To: <20161010131041.lpdh4a7nol24hsz2@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1056 bytes --]
> On Oct 10, 2016, at 06:10, Jeff King <peff@peff.net> wrote:
>
> On Sun, Oct 09, 2016 at 07:53:23PM -0700, Jeremy Huddleston Sequoia wrote:
>
>> Subject: Re: [PATCH] t4014-format-patch: Adjust git_version regex to better
>> handle distro changes to DEF_VER
>>
>> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>
> I see there was a discussion elsewhere on the list about exactly what
> you are putting into DEF_VAR that causes the problem. Perhaps the commit
> message here would be a good place to mention that, why the current
> regex breaks it, and why your new version fixes not only it, but other
> possible values of DEF_VAR.
Thanks, I've added this blurb:
For example, git distributed with Apple's Xcode reports a version like:
git version 2.9.3 (Apple Git-75)
Apple started doing this to help customers distinguish between different
versions of their packaged git which have the same base version (eg: with
different patches applied).
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4465 bytes --]
^ permalink raw reply
* Re: git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
From: Seaders Oloinsigh @ 2016-10-10 16:12 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161010153032.v2x773cbs4ifvzec@sigill.intra.peff.net>
Thanks for the reply, Jeff.
Clearing the backups of the branches, those starting with
"refs/original" has gotten me closer, but I also needed to do that
with "refs/tags" as well, or change my filter-branch command to,
git filter-branch -f --prune-empty --tag-name-filter cat
--subdirectory-filter android -- --all
I still have remnants of that other history, though.
Due to the structure of this repo, it looks like there are some
branches that never had anything to do with the android/ subdirectory,
so they're not getting wiped out. My branch is in a better state to
how I want it, but still, if I run your suggestion,
git log --all --source -- unity/
I get output like
> commit 4853c... refs/heads/unity-sdk-3_1_3
> Author: serg... <serg...@...ve.com>
> Date: Thu Sep 11 16:30:01 2014 +0100
>
> Started 3.1.3
Which is basically logs of branches which contain only edits within
the unity/ subdirectory of the original root. There are other
branches like that for the other platforms / subdirectories of the
original root, which if that is the case, I would consider
filter-branch with the subdirectory-filter isn't acting as expected,
and doesn't get rid of all the history you want it to get rid of.
On Mon, Oct 10, 2016 at 4:30 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Oct 10, 2016 at 02:42:36PM +0100, Seaders Oloinsigh wrote:
>
>> We have a git repository that looks like
>>
>> sdk/
>> android/
>> ios/
>> unity/
>> windows/
>>
>> Which we'd like to split into 4 repositories, 1 for each platform. To
>> start this process (for splitting android out), I ran,
>>
>> git filter-branch -f --prune-empty --subdirectory-filter android -- --all
>
> OK, so that should rewrite each ref to have only the contents of the
> "android" directory at the top-level.
>
> Note that filter-branch saves a copy of the old refs in refs/original.
>
>> Which rewrote a ton of history and commits, and looked like it worked, but
>> on closer inspection had left a ton of history behind.
>>
>> If I run
>>
>> git log --all -- unity/
>>
>> It returns a list of commits that happened in the unity/ subfolder of the
>> original root.
>
> Here you asked for "--all", which includes refs/original. So you are
> seeing the original, unwritten commits (and none of your new ones, of
> course, because they do not have a unity/ directory!).
>
> Try:
>
> git log --all --source -- unity
>
> to see which ref each commit is coming from.
>
> Or try:
>
> git log --branches --tags -- unity
>
> to confirm that your branches and tags do not include that path.
>
> Or just:
>
> git for-each-ref --format='delete %(refname)' refs/original |
> git update-ref --stdin
>
> to get rid of the backup refs entirely.
>
> -Peff
^ permalink raw reply
* Re: git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
From: Jeff King @ 2016-10-10 15:30 UTC (permalink / raw)
To: Seaders Oloinsigh; +Cc: git
In-Reply-To: <CAN40BKqPdPP2+K4FBzgDDfYiGkzk1gYcOeP==_t+pr5w0rj0EQ@mail.gmail.com>
On Mon, Oct 10, 2016 at 02:42:36PM +0100, Seaders Oloinsigh wrote:
> We have a git repository that looks like
>
> sdk/
> android/
> ios/
> unity/
> windows/
>
> Which we'd like to split into 4 repositories, 1 for each platform. To
> start this process (for splitting android out), I ran,
>
> git filter-branch -f --prune-empty --subdirectory-filter android -- --all
OK, so that should rewrite each ref to have only the contents of the
"android" directory at the top-level.
Note that filter-branch saves a copy of the old refs in refs/original.
> Which rewrote a ton of history and commits, and looked like it worked, but
> on closer inspection had left a ton of history behind.
>
> If I run
>
> git log --all -- unity/
>
> It returns a list of commits that happened in the unity/ subfolder of the
> original root.
Here you asked for "--all", which includes refs/original. So you are
seeing the original, unwritten commits (and none of your new ones, of
course, because they do not have a unity/ directory!).
Try:
git log --all --source -- unity
to see which ref each commit is coming from.
Or try:
git log --branches --tags -- unity
to confirm that your branches and tags do not include that path.
Or just:
git for-each-ref --format='delete %(refname)' refs/original |
git update-ref --stdin
to get rid of the backup refs entirely.
-Peff
^ permalink raw reply
* Re: git merge deletes my changes
From: Jeff King @ 2016-10-10 15:26 UTC (permalink / raw)
To: Eduard Egorov; +Cc: 'git@vger.kernel.org'
In-Reply-To: <AM4PR03MB1636D18D727968F332F16021DBDB0@AM4PR03MB1636.eurprd03.prod.outlook.com>
On Mon, Oct 10, 2016 at 09:39:13AM +0000, Eduard Egorov wrote:
> A week ago, I've reset a state of 'ceph-ansible' folder in %current%
> branch with code from corresponding branch (that tracks an upstream
> from github):
>
> # git read-tree --prefix=ceph-ansible/ -u ceph_ansible
This pulls in the subtree files, but there's no actual relationship with
the commit history in ceph_ansible.
So later...
> Then I've committed several changes, including:
>
> 1. Renamed file and commited:
> # git mv site.yml.sample site.yml
>
> 2. Made some changes and committed
>
> 3. Pulled updates from original branch by:
> # git merge -s subtree --squash ceph_ansible
>
> It said:
> Auto-merging ceph-ansible/site.yml.sample
> blablabla
> Squash commit -- not updating HEAD
> Automatic merge went well; stopped before committing as requested
When you merge from ceph_ansible, there is no shared history, and git
uses the empty tree as a common ancestor. It looks like the other side
added site.yml.sample, for instance, because that is a change from the
empty tree.
> A post on SO: http://stackoverflow.com/questions/39954265/git-merge-deletes-my-changes
As you noted on SO, modern git disallows merges of unrelated history by
default, because it's usually a mistake to do so.
If you are doing repeated merges into the subtree, you need to somehow
tell git how the histories are related. The obvious answer is to do a
"git merge -s ours ceph_ansible" after your initial read-tree, so that
git knows you've pulled in the changes up to that point. But I'd guess
from your use of "--squash" that you don't want to carry the
ceph_ansible history in your project.
So you need to record the original upstream commit somewhere (probably
in the commit message when you commit the read-tree result), and then
ask git to use that as the merge-base during subsequent merges (which
will require using plumbing codes, as git-merge wants to compute the
merge base itself). I believe the git-subtree command (in
contrib/subtree of git.git) handles this use case, but I haven't used it
myself.
-Peff
^ permalink raw reply
* [PATCH] pretty: respect color settings for %C placeholders
From: Jeff King @ 2016-10-10 15:15 UTC (permalink / raw)
To: Duy Nguyen; +Cc: René Scharfe, Tom Hale, git, Junio C Hamano
In-Reply-To: <20161010142818.lglwrxpks6l6aqrm@sigill.intra.peff.net>
On Mon, Oct 10, 2016 at 10:28:18AM -0400, Jeff King wrote:
> > We could add some new tag to change the behavior of all following %C
> > tags. Something like %C(tty) maybe (probably a bad name), then
> > discourage the use if "%C(auto" for terminal detection?
>
> Yeah, adding a "%C(enable-auto-color)" or something would be backwards
> compatible and less painful than using "%C(auto)" everywhere. I do
> wonder if anybody actually _wants_ the "always show color, even if
> --no-color" behavior. I'm having trouble thinking of a good use for it.
>
> IOW, I'm wondering if anyone would disagree that the current behavior is
> simply buggy. Reading the thread at:
>
> http://public-inbox.org/git/7v4njkmq07.fsf@alter.siamese.dyndns.org/
>
> I don't really see any compelling reason against it (there was some
> question of which config to use, but we already answered that with
> "%C(auto)", and use the value from the pretty_ctx).
So here's what a patch to do that would look like. I admit that "I can't
think of a good use" does not mean there _isn't_ one, but perhaps by
posting this, it might provoke other people to think on it, too. And if
nobody can come up with, maybe it's a good idea.
-- >8 --
Subject: pretty: respect color settings for %C placeholders
The color placeholders have traditionally been
unconditional, showing colors even when git is not otherwise
configured to do so. This was not so bad for their original
use, which was on the command-line (and the user could
decide at that moment whether to add colors or not). But
these days we have configured formats via pretty.*, and
those should operate in multiple contexts.
In 3082517 (log --format: teach %C(auto,black) to respect
color config, 2012-12-17), we gave an extended placeholder
that could be used to accomplish this. But it's rather
clunky to use, because you have to specify it individually
for each color (and their matching resets) in the format.
We shied away from just switching the default to auto,
because it is technically breaking backwards compatibility.
However, there's not really a use case for unconditional
colors. The most plausible reason you would want them
unconditional is to redirect "git log" output to a file. But
there, the right answer is --color=always, as it does the
right thing both with custom user-format colors and
git-generated colors.
So let's switch to the more useful default. In the
off-chance that somebody really does find a use for
unconditional colors without wanting to enable the rest of
git's colors, we can provide %C(always,...) to enable the
old behavior.
Signed-off-by: Jeff King <peff@peff.net>
---
The tests unsurprisingly needed updating, as we're breaking the old
behavior. The diff is easier to read read with "-w".
Documentation/pretty-formats.txt | 13 +++---
pretty.c | 19 +++++---
t/t6006-rev-list-format.sh | 94 ++++++++++++++++++++--------------------
3 files changed, 70 insertions(+), 56 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a942d57..7aa1a8b 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -167,11 +167,14 @@ endif::git-rev-list[]
- '%Cblue': switch color to blue
- '%Creset': reset color
- '%C(...)': color specification, as described in color.branch.* config option;
- adding `auto,` at the beginning will emit color only when colors are
- enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
- respecting the `auto` settings of the former if we are going to a
- terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
- on the next placeholders until the color is switched again.
+ By default, colors are shown only when enabled for log output (by
+ `color.diff`, `color.ui`, or `--color`, and respecting the `auto`
+ settings of the former if we are going to a terminal). `%C(auto,...)`
+ is accepted as a historical synonym for the default. Specifying
+ `%C(always,...) will show the colors always, even when colors are not
+ otherwise enabled (to enable this behavior for the whole format, use
+ `--color=always`). `auto` alone (i.e. `%C(auto)`) will turn on auto
+ coloring on the next placeholders until the color is switched again.
- '%m': left (`<`), right (`>`) or boundary (`-`) mark
- '%n': newline
- '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index 25efbca..73e58b5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -965,22 +965,31 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
if (!end)
return 0;
- if (skip_prefix(begin, "auto,", &begin)) {
+
+ if (!skip_prefix(begin, "always,", &begin)) {
if (!want_color(c->pretty_ctx->color))
return end - placeholder + 1;
}
+
+ /* this is a historical noop */
+ skip_prefix(begin, "auto,", &begin);
+
if (color_parse_mem(begin, end - begin, color) < 0)
die(_("unable to parse --pretty format"));
strbuf_addstr(sb, color);
return end - placeholder + 1;
}
- if (skip_prefix(placeholder + 1, "red", &rest))
+ if (skip_prefix(placeholder + 1, "red", &rest) &&
+ want_color(c->pretty_ctx->color))
strbuf_addstr(sb, GIT_COLOR_RED);
- else if (skip_prefix(placeholder + 1, "green", &rest))
+ else if (skip_prefix(placeholder + 1, "green", &rest) &&
+ want_color(c->pretty_ctx->color))
strbuf_addstr(sb, GIT_COLOR_GREEN);
- else if (skip_prefix(placeholder + 1, "blue", &rest))
+ else if (skip_prefix(placeholder + 1, "blue", &rest) &&
+ want_color(c->pretty_ctx->color))
strbuf_addstr(sb, GIT_COLOR_BLUE);
- else if (skip_prefix(placeholder + 1, "reset", &rest))
+ else if (skip_prefix(placeholder + 1, "reset", &rest) &&
+ want_color(c->pretty_ctx->color))
strbuf_addstr(sb, GIT_COLOR_RESET);
return rest - placeholder;
}
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index a1dcdb8..29b1a1a 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -59,7 +59,10 @@ test_format () {
}
# Feed to --format to provide predictable colored sequences.
+BASIC_COLOR='%Credfoo%Creset'
+COLOR='%C(red)foo%C(reset)'
AUTO_COLOR='%C(auto,red)foo%C(auto,reset)'
+ALWAYS_COLOR='%C(always,red)foo%C(always,reset)'
has_color () {
printf '\033[31mfoo\033[m\n' >expect &&
test_cmp expect "$1"
@@ -170,57 +173,56 @@ $added
EOF
-test_format colors %Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy <<EOF
-commit $head2
-^[[31mfoo^[[32mbar^[[34mbaz^[[mxyzzy
-commit $head1
-^[[31mfoo^[[32mbar^[[34mbaz^[[mxyzzy
-EOF
-
-test_format advanced-colors '%C(red yellow bold)foo%C(reset)' <<EOF
-commit $head2
-^[[1;31;43mfoo^[[m
-commit $head1
-^[[1;31;43mfoo^[[m
-EOF
-
-test_expect_success '%C(auto,...) does not enable color by default' '
- git log --format=$AUTO_COLOR -1 >actual &&
- has_no_color actual
-'
-
-test_expect_success '%C(auto,...) enables colors for color.diff' '
- git -c color.diff=always log --format=$AUTO_COLOR -1 >actual &&
- has_color actual
-'
+for spec in \
+ "%Cred:$BASIC_COLOR" \
+ "%C(...):$COLOR" \
+ "%C(auto,...):$AUTO_COLOR"
+do
+ desc=${spec%%:*}
+ color=${spec#*:}
+ test_expect_success "$desc does not enable color by default" '
+ git log --format=$color -1 >actual &&
+ has_no_color actual
+ '
-test_expect_success '%C(auto,...) enables colors for color.ui' '
- git -c color.ui=always log --format=$AUTO_COLOR -1 >actual &&
- has_color actual
-'
+ test_expect_success "$desc enables colors for color.diff" '
+ git -c color.diff=always log --format=$color -1 >actual &&
+ has_color actual
+ '
-test_expect_success '%C(auto,...) respects --color' '
- git log --format=$AUTO_COLOR -1 --color >actual &&
- has_color actual
-'
+ test_expect_success "$desc enables colors for color.ui" '
+ git -c color.ui=always log --format=$color -1 >actual &&
+ has_color actual
+ '
-test_expect_success '%C(auto,...) respects --no-color' '
- git -c color.ui=always log --format=$AUTO_COLOR -1 --no-color >actual &&
- has_no_color actual
-'
+ test_expect_success "$desc respects --color" '
+ git log --format=$color -1 --color >actual &&
+ has_color actual
+ '
-test_expect_success TTY '%C(auto,...) respects --color=auto (stdout is tty)' '
- test_terminal env TERM=vt100 \
- git log --format=$AUTO_COLOR -1 --color=auto >actual &&
- has_color actual
-'
-
-test_expect_success '%C(auto,...) respects --color=auto (stdout not tty)' '
- (
- TERM=vt100 && export TERM &&
- git log --format=$AUTO_COLOR -1 --color=auto >actual &&
+ test_expect_success "$desc respects --no-color" '
+ git -c color.ui=always log --format=$color -1 --no-color >actual &&
has_no_color actual
- )
+ '
+
+ test_expect_success TTY "$desc respects --color=auto (stdout is tty)" '
+ test_terminal env TERM=vt100 \
+ git log --format=$color -1 --color=auto >actual &&
+ has_color actual
+ '
+
+ test_expect_success "$desc respects --color=auto (stdout not tty)" '
+ (
+ TERM=vt100 && export TERM &&
+ git log --format=$color -1 --color=auto >actual &&
+ has_no_color actual
+ )
+ '
+done
+
+test_expect_success '%C(always,...) enables color even without tty' '
+ git log --format=$ALWAYS_COLOR -1 >actual &&
+ has_color actual
'
test_expect_success '%C(auto) respects --color' '
--
2.10.1.527.g93d4615
^ permalink raw reply related
* Re: How to watch a mailing list & repo for patches which affect a certain area of code?
From: Eric Wong @ 2016-10-10 15:11 UTC (permalink / raw)
To: Ian Kelling; +Cc: git
In-Reply-To: <1476039798.3060702.750483225.1DE6C48B@webmail.messagingengine.com>
Ian Kelling <ian@iankelling.org> wrote:
> I've got patches in various projects, and I don't have time to keep up
> with the mailing list, but I'd like to help out with maintenance of that
> code, or the functions/files it touches. People don't cc me. I figure I
> could filter the list, test patches submitted, commits made, mentions of
> files/functions, build filters based on the code I have in the repo even
> if it's been moved or changed subsequently. I'm wondering what other
> people have implemented already for automation around this, or general
> thoughts. Web search is not showing me much.
For the mailing list, you can try following an Atom feed for
any specific search query.
https://public-inbox.org/git/?q=FILE_OR_FUNCTION&x=A
(the "x=A" makes it an Atom feed)
It's all still a work-in-progress but there'll be better
filename and diff handling in public-inbox soon.
It's all AGPL and the data is only sourced from this mailing
list, so 100% reproducible as I'm incapable of running a
reliable server :> Clone instructions at the bottom of
https://public-inbox.org/git/
^ permalink raw reply
* Re: %C(auto) not working as expected
From: Jeff King @ 2016-10-10 14:28 UTC (permalink / raw)
To: Duy Nguyen; +Cc: René Scharfe, Tom Hale, git, Junio C Hamano
In-Reply-To: <CACsJy8CroyynVMctbPhuVr2VVQB7YyfcxDaMT25BikQ4R4We0Q@mail.gmail.com>
On Mon, Oct 10, 2016 at 04:26:18PM +0700, Duy Nguyen wrote:
> > If we do a revamp of the pretty-formats to bring them more in line with
> > ref-filter (e.g., something like "%(color:red)") maybe that would be an
> > opportunity to make minor adjustments. Though, hmm, it looks like
> > for-each-ref already knows "%(color:red)", and it's unconditional.
> > <sigh> So perhaps we would need to go through some deprecation period or
> > other transition.
>
> We could add some new tag to change the behavior of all following %C
> tags. Something like %C(tty) maybe (probably a bad name), then
> discourage the use if "%C(auto" for terminal detection?
Yeah, adding a "%C(enable-auto-color)" or something would be backwards
compatible and less painful than using "%C(auto)" everywhere. I do
wonder if anybody actually _wants_ the "always show color, even if
--no-color" behavior. I'm having trouble thinking of a good use for it.
IOW, I'm wondering if anyone would disagree that the current behavior is
simply buggy. Reading the thread at:
http://public-inbox.org/git/7v4njkmq07.fsf@alter.siamese.dyndns.org/
I don't really see any compelling reason against it (there was some
question of which config to use, but we already answered that with
"%C(auto)", and use the value from the pretty_ctx).
-Peff
^ permalink raw reply
* git filter-branch --subdirectory-filter not working as expected, history of other folders is preserved
From: Seaders Oloinsigh @ 2016-10-10 13:42 UTC (permalink / raw)
To: git
From
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
Now your new project root is what was in the trunk subdirectory each time.
> Git will also automatically remove commits that did not affect the
> subdirectory.
>
We have a git repository that looks like
sdk/
android/
ios/
unity/
windows/
Which we'd like to split into 4 repositories, 1 for each platform. To
start this process (for splitting android out), I ran,
git filter-branch -f --prune-empty --subdirectory-filter android -- --all
Which rewrote a ton of history and commits, and looked like it worked, but
on closer inspection had left a ton of history behind.
If I run
git log --all -- unity/
It returns a list of commits that happened in the unity/ subfolder of the
original root.
commit c4ea2797...
> Author: tom... <tom@...ve.com>
> Date: Thu Feb 25 14:20:59 2016 +0000
>
> kick off build
>
> ...
>
Which only contains an edit to a file with path "unity/tom" relative to the
root *before* the filter-branch, doesn't exist any more, and from my
understanding of the docs, shouldn't have been taken across.
It's also not an isolated instance, if I run the same checks against
"ios/", "windows/", any file that existed in a folder other than "android"
to the old root, that history is also preserved.
I've just about resorted to running multiple other, explicit filters to
remove all references to those other folders, but it seems like this would
be redoing the job that I understood git filter-branch should have been
doing.
Any help in this regard is greatly appreciated.
seaders.
^ permalink raw reply
* Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER
From: Jeff King @ 2016-10-10 13:10 UTC (permalink / raw)
To: Jeremy Huddleston Sequoia; +Cc: git, Josh Triplett, Junio C Hamano
In-Reply-To: <20161010025323.9415-1-jeremyhu@apple.com>
On Sun, Oct 09, 2016 at 07:53:23PM -0700, Jeremy Huddleston Sequoia wrote:
> Subject: Re: [PATCH] t4014-format-patch: Adjust git_version regex to better
> handle distro changes to DEF_VER
>
> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
I see there was a discussion elsewhere on the list about exactly what
you are putting into DEF_VAR that causes the problem. Perhaps the commit
message here would be a good place to mention that, why the current
regex breaks it, and why your new version fixes not only it, but other
possible values of DEF_VAR.
-Peff
^ permalink raw reply
* Re: Bug? git worktree fails with master on bare repo
From: Michael Tutty @ 2016-10-10 13:06 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Dennis Kaarsemaker, Git Mailing List, Michael Rappazzo
In-Reply-To: <CACsJy8DMKWeZ+DuQ0uoY6rdPfusq8D1SfBCkPyn+6X9S589ncg@mail.gmail.com>
> If I create a bare _clone_, then "HEAD" could be detached, or point to some branch, depending on where "HEAD" is in the source repo
I didn't mean a clone, I meant a brand-new (bare) repo. Then I would
clone it somewhere, add commits and branches, and push them to the
bare repo.
> If source repo's HEAD is "master", I got the same behavior (worktree add fails)
So if it's possible for a bare repo to have HEAD pointing at master,
is there a safe way for me to change this (e.g., as a cleanup step
before doing my actual merge process)?
On Mon, Oct 10, 2016 at 4:45 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Oct 9, 2016 at 8:42 PM, Michael Tutty <mtutty@gforgegroup.com> wrote:
>> Dennis,
>> Thanks for the great response, and for spending time on my issue.
>> I'll try that first patch and see what happens.
>>
>> In the meantime, it got weirder...
>>
>> I created a brand-new (bare) repo
>
> Elaboration needed here. If I create a bare _clone_, then "HEAD" could
> be detached, or point to some branch, depending on where "HEAD" is in
> the source repo. If source repo's HEAD is "master", I got the same
> behavior (worktree add fails). If it's detached or points to some
> other branch, it's ok. If this is "git init --bare" then I got "fatal:
> invalid reference: master".
>
>> and was able to git add worktree
>> /path master. I was able to do this repeatedly, even using the
>> worktree to merge other branches to master. I didn't find any
>> condition or step that caused some kind of orphan master work tree,
>> which was what I thought the underlying problem might be.
> --
> Duy
--
Michael Tutty, CTO
e: mtutty@gforgegroup.com
t: @mtutty, @gforgegroup
v: 515-789-0772
w: http://gforgegroup.com, http://gforge.com
^ permalink raw reply
* Re: [PATCH v2] gpg-interface: use more status letters
From: Michael J Gruber @ 2016-10-10 12:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ramsay Jones, git, Alex
In-Reply-To: <xmqqk2dli25w.fsf@gitster.mtv.corp.google.com>
Junio C Hamano venit, vidit, dixit 06.10.2016 23:43:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>> Also, I'm open to using another letter for EXPKEYSIG but couldn't decide
>>> between 'Y', 'Z', 'K'. 'K' could be confused with REVKEYSIG, I'm afraid.
>>> 'Y' is next to 'X' and contained in 'KEY', it would be my first choice.
>>
>> Sounds good enough to me. Thanks.
>
> I really do not want to leave too many topics listed in the "What's
> cooking" report to be in undecided / waiting state. How about
> squashing this in, with a fully updated log message to replace?
Sorry, this got "lost in vacation". Before that, I was looking for an
easy way to test expired signatures, but gpg1 and gpg2 behave somewhat
differently in that respect (2 does not allow to create already expired
signatures).
Is there anything I should or could do now?
Michael
> -- >8 --
> From: Michael J Gruber <git@drmicha.warpmail.net>
> Date: Wed, 28 Sep 2016 16:24:13 +0200
> Subject: [PATCH] SQUASH: gpg-interface: use more status letters
>
> According to gpg2's doc/DETAILS:
>
> For each signature only one of the codes GOODSIG, BADSIG,
> EXPSIG, EXPKEYSIG, REVKEYSIG or ERRSIG will be emitted.
>
> gpg1 ("classic") behaves the same (although doc/DETAILS differs).
>
> Currently, we parse gpg's status output for GOODSIG, BADSIG and
> trust information and translate that into status codes G, B, U, N
> for the %G? format specifier.
>
> git-verify-* returns success in the GOODSIG case only. This is
> somewhat in disagreement with gpg, which considers the first 5 of
> the 6 above as VALIDSIG, but we err on the very safe side.
>
> Introduce additional status codes E, X, Y, R for ERRSIG, EXPSIG,
> EXPKEYSIG, and REVKEYSIG so that a user of %G? gets more information
> about the absence of a 'G' on first glance.
>
> Requested-by: Alex <agrambot@gmail.com>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/pretty-formats.txt | 3 ++-
> gpg-interface.c | 2 +-
> pretty.c | 1 +
> 3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
> index c28ff2b919..179c9389aa 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -146,7 +146,8 @@ endif::git-rev-list[]
> - '%G?': show "G" for a good (valid) signature,
> "B" for a bad signature,
> "U" for a good signature with unknown validity,
> - "X" for a good expired signature, or good signature made by an expired key,
> + "X" for a good signature that has expired,
> + "Y" for a good signature made by an expired key,
> "R" for a good signature made by a revoked key,
> "E" if the signature cannot be checked (e.g. missing key)
> and "N" for no signature
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 6999e7b469..e44cc27da1 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -35,7 +35,7 @@ static struct {
> { 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
> { 'E', "\n[GNUPG:] ERRSIG "},
> { 'X', "\n[GNUPG:] EXPSIG "},
> - { 'X', "\n[GNUPG:] EXPKEYSIG "},
> + { 'Y', "\n[GNUPG:] EXPKEYSIG "},
> { 'R', "\n[GNUPG:] REVKEYSIG "},
> };
>
> diff --git a/pretty.c b/pretty.c
> index 39a36cd825..f98b271069 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1236,6 +1236,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
> case 'U':
> case 'N':
> case 'X':
> + case 'Y':
> case 'R':
> strbuf_addch(sb, c->signature_check.result);
> }
>
^ 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