* [PATCH 1/7] diff: export diffstat interface
From: Daniel Ferreira via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Daniel Ferreira
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Daniel Ferreira <bnmvco@gmail.com>
Make the diffstat interface (namely, the diffstat_t struct and
compute_diffstat) no longer be internal to diff.c and allow it to be used
by other parts of git.
This is helpful for code that may want to easily extract information
from files using the diff machinery, while flushing it differently from
how the show_* functions used by diff_flush() do it. One example is the
builtin implementation of git-add--interactive's status.
Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
diff.c | 36 ++++++++++++++----------------------
diff.h | 18 ++++++++++++++++++
2 files changed, 32 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index dc9965e836..46a7d8cf29 100644
--- a/diff.c
+++ b/diff.c
@@ -2421,22 +2421,6 @@ static void pprint_rename(struct strbuf *name, const char *a, const char *b)
}
}
-struct diffstat_t {
- int nr;
- int alloc;
- struct diffstat_file {
- char *from_name;
- char *name;
- char *print_name;
- const char *comments;
- unsigned is_unmerged:1;
- unsigned is_binary:1;
- unsigned is_renamed:1;
- unsigned is_interesting:1;
- uintmax_t added, deleted;
- } **files;
-};
-
static struct diffstat_file *diffstat_add(struct diffstat_t *diffstat,
const char *name_a,
const char *name_b)
@@ -5922,12 +5906,7 @@ void diff_flush(struct diff_options *options)
dirstat_by_line) {
struct diffstat_t diffstat;
- memset(&diffstat, 0, sizeof(struct diffstat_t));
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
- if (check_pair_status(p))
- diff_flush_stat(p, options, &diffstat);
- }
+ compute_diffstat(options, &diffstat);
if (output_format & DIFF_FORMAT_NUMSTAT)
show_numstat(&diffstat, options);
if (output_format & DIFF_FORMAT_DIFFSTAT)
@@ -6227,6 +6206,19 @@ static int is_submodule_ignored(const char *path, struct diff_options *options)
return ignored;
}
+void compute_diffstat(struct diff_options *options, struct diffstat_t *diffstat)
+{
+ int i;
+ struct diff_queue_struct *q = &diff_queued_diff;
+
+ memset(diffstat, 0, sizeof(struct diffstat_t));
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+ if (check_pair_status(p))
+ diff_flush_stat(p, options, diffstat);
+ }
+}
+
void diff_addremove(struct diff_options *options,
int addremove, unsigned mode,
const struct object_id *oid,
diff --git a/diff.h b/diff.h
index ce5e8a8183..7809db3039 100644
--- a/diff.h
+++ b/diff.h
@@ -239,6 +239,22 @@ void diff_emit_submodule_error(struct diff_options *o, const char *err);
void diff_emit_submodule_pipethrough(struct diff_options *o,
const char *line, int len);
+struct diffstat_t {
+ int nr;
+ int alloc;
+ struct diffstat_file {
+ char *from_name;
+ char *name;
+ char *print_name;
+ const char *comments;
+ unsigned is_unmerged:1;
+ unsigned is_binary:1;
+ unsigned is_renamed:1;
+ unsigned is_interesting:1;
+ uintmax_t added, deleted;
+ } **files;
+};
+
enum color_diff {
DIFF_RESET = 0,
DIFF_CONTEXT = 1,
@@ -327,6 +343,8 @@ void diff_change(struct diff_options *,
struct diff_filepair *diff_unmerge(struct diff_options *, const char *path);
+void compute_diffstat(struct diff_options *options, struct diffstat_t *diffstat);
+
#define DIFF_SETUP_REVERSE 1
#define DIFF_SETUP_USE_SIZE_CACHE 4
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/7] add--helper: create builtin helper for interactive add
From: Daniel Ferreira via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Daniel Ferreira
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Daniel Ferreira <bnmvco@gmail.com>
Create a builtin helper for git-add--interactive, which right now is not
able to do anything.
This is the first step in an effort to convert git-add--interactive.perl
to a C builtin, in search for better portability, expressibility and
performance (specially on non-POSIX systems like Windows).
Additionally, an eventual complete port of git-add--interactive would
remove the last "big" Git script to have Perl as a dependency, allowing
most Git users to have a NOPERL build running without big losses.
Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
.gitignore | 1 +
Makefile | 1 +
builtin.h | 1 +
builtin/add--helper.c | 6 ++++++
git.c | 1 +
5 files changed, 10 insertions(+)
create mode 100644 builtin/add--helper.c
diff --git a/.gitignore b/.gitignore
index 0d77ea5894..2ee71ed217 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,6 +15,7 @@
/git
/git-add
/git-add--interactive
+/git-add--helper
/git-am
/git-annotate
/git-apply
diff --git a/Makefile b/Makefile
index 1a44c811aa..9c84b80739 100644
--- a/Makefile
+++ b/Makefile
@@ -1023,6 +1023,7 @@ LIB_OBJS += xdiff-interface.o
LIB_OBJS += zlib.o
BUILTIN_OBJS += builtin/add.o
+BUILTIN_OBJS += builtin/add--helper.o
BUILTIN_OBJS += builtin/am.o
BUILTIN_OBJS += builtin/annotate.o
BUILTIN_OBJS += builtin/apply.o
diff --git a/builtin.h b/builtin.h
index 6538932e99..dd811ef7d5 100644
--- a/builtin.h
+++ b/builtin.h
@@ -128,6 +128,7 @@ extern void setup_auto_pager(const char *cmd, int def);
extern int is_builtin(const char *s);
extern int cmd_add(int argc, const char **argv, const char *prefix);
+extern int cmd_add__helper(int argc, const char **argv, const char *prefix);
extern int cmd_am(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
diff --git a/builtin/add--helper.c b/builtin/add--helper.c
new file mode 100644
index 0000000000..6a97f0e191
--- /dev/null
+++ b/builtin/add--helper.c
@@ -0,0 +1,6 @@
+#include "builtin.h"
+
+int cmd_add__helper(int argc, const char **argv, const char *prefix)
+{
+ return 0;
+}
diff --git a/git.c b/git.c
index 2f604a41ea..5507591f2e 100644
--- a/git.c
+++ b/git.c
@@ -443,6 +443,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
static struct cmd_struct commands[] = {
{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+ { "add--helper", cmd_add__helper, RUN_SETUP | NEED_WORK_TREE },
{ "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
{ "annotate", cmd_annotate, RUN_SETUP | NO_PARSEOPT },
{ "apply", cmd_apply, RUN_SETUP_GENTLY },
--
gitgitgadget
^ permalink raw reply related
* [PATCH 4/7] add--interactive.perl: use add--helper --status for status_cmd
From: Daniel Ferreira via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Daniel Ferreira
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Daniel Ferreira <bnmvco@gmail.com>
Call the newly introduced add--helper builtin on
status_cmd() instead of relying on add--interactive's Perl
functions to build print the numstat.
Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
git-add--interactive.perl | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 20eb81cc92..a6536f9cf3 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -597,9 +597,7 @@ EOF
}
sub status_cmd {
- list_and_choose({ LIST_ONLY => 1, HEADER => $status_head },
- list_modified());
- print "\n";
+ system(qw(git add--helper --status));
}
sub say_n_paths {
--
gitgitgadget
^ permalink raw reply related
* [PATCH 3/7] add-interactive.c: implement status command
From: Daniel Ferreira via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Daniel Ferreira
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Daniel Ferreira <bnmvco@gmail.com>
Add new files: add-interactive.c and add-interactive.h, which
will be used for implementing "application logic" of git add -i,
whereas add--helper.c will be used mostly for parsing the command line.
We're a bit lax with the command-line parsing, as the command is
intended to be called only by one internal user: the add--interactive script.
Implement add --interactive's status command in add-interactive.c and
use it in builtin add--helper.c.
It prints a numstat comparing changed files between a) the worktree and
the index; b) the index and the HEAD.
To do so, we use run_diff_index() and run_diff_files() to get changed
files, use the diffstat API on them to get the numstat and use a
combination of a hashmap and qsort() to print the result in
O(n) + O(n lg n) complexity.
This is the first interactive add command implemented in C of those
anticipated by the previous commit, which introduced
the add--helper built-in.
Signed-off-by: Daniel Ferreira <bnmvco@gmail.com>
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
Makefile | 1 +
add-interactive.c | 246 ++++++++++++++++++++++++++++++++++++++++++
add-interactive.h | 8 ++
builtin/add--helper.c | 32 ++++++
4 files changed, 287 insertions(+)
create mode 100644 add-interactive.c
create mode 100644 add-interactive.h
diff --git a/Makefile b/Makefile
index 9c84b80739..2a4a5cc37b 100644
--- a/Makefile
+++ b/Makefile
@@ -827,6 +827,7 @@ LIB_H = $(shell $(FIND) . \
-name '*.h' -print)
LIB_OBJS += abspath.o
+LIB_OBJS += add-interactive.o
LIB_OBJS += advice.o
LIB_OBJS += alias.o
LIB_OBJS += alloc.o
diff --git a/add-interactive.c b/add-interactive.c
new file mode 100644
index 0000000000..c55d934186
--- /dev/null
+++ b/add-interactive.c
@@ -0,0 +1,246 @@
+#include "add-interactive.h"
+#include "cache.h"
+#include "commit.h"
+#include "color.h"
+#include "config.h"
+#include "diffcore.h"
+#include "revision.h"
+
+#define HEADER_INDENT " "
+
+enum collection_phase {
+ WORKTREE,
+ INDEX
+};
+
+struct file_stat {
+ struct hashmap_entry ent;
+ struct {
+ uintmax_t added, deleted;
+ } index, worktree;
+ char name[FLEX_ARRAY];
+};
+
+struct collection_status {
+ enum collection_phase phase;
+
+ const char *reference;
+ struct pathspec pathspec;
+
+ struct hashmap file_map;
+};
+
+static int use_color = -1;
+enum color_add_i {
+ COLOR_PROMPT,
+ COLOR_HEADER,
+ COLOR_HELP,
+ COLOR_ERROR
+};
+
+static char colors[][COLOR_MAXLEN] = {
+ GIT_COLOR_BOLD_BLUE, /* Prompt */
+ GIT_COLOR_BOLD, /* Header */
+ GIT_COLOR_BOLD_RED, /* Help */
+ GIT_COLOR_BOLD_RED /* Error */
+};
+
+static const char *get_color(enum color_add_i ix)
+{
+ if (want_color(use_color))
+ return colors[ix];
+ return "";
+}
+
+static int parse_color_slot(const char *slot)
+{
+ if (!strcasecmp(slot, "prompt"))
+ return COLOR_PROMPT;
+ if (!strcasecmp(slot, "header"))
+ return COLOR_HEADER;
+ if (!strcasecmp(slot, "help"))
+ return COLOR_HELP;
+ if (!strcasecmp(slot, "error"))
+ return COLOR_ERROR;
+
+ return -1;
+}
+
+int add_i_config(const char *var,
+ const char *value, void *cbdata)
+{
+ const char *name;
+
+ if (!strcmp(var, "color.interactive")) {
+ use_color = git_config_colorbool(var, value);
+ return 0;
+ }
+
+ if (skip_prefix(var, "color.interactive.", &name)) {
+ int slot = parse_color_slot(name);
+ if (slot < 0)
+ return 0;
+ if (!value)
+ return config_error_nonbool(var);
+ return color_parse(value, colors[slot]);
+ }
+
+ return git_default_config(var, value, cbdata);
+}
+
+static int hash_cmp(const void *unused_cmp_data, const void *entry,
+ const void *entry_or_key, const void *keydata)
+{
+ const struct file_stat *e1 = entry, *e2 = entry_or_key;
+ const char *name = keydata ? keydata : e2->name;
+
+ return strcmp(e1->name, name);
+}
+
+static int alphabetical_cmp(const void *a, const void *b)
+{
+ struct file_stat *f1 = *((struct file_stat **)a);
+ struct file_stat *f2 = *((struct file_stat **)b);
+
+ return strcmp(f1->name, f2->name);
+}
+
+static void collect_changes_cb(struct diff_queue_struct *q,
+ struct diff_options *options,
+ void *data)
+{
+ struct collection_status *s = data;
+ struct diffstat_t stat = { 0 };
+ int i;
+
+ if (!q->nr)
+ return;
+
+ compute_diffstat(options, &stat);
+
+ for (i = 0; i < stat.nr; i++) {
+ struct file_stat *entry;
+ const char *name = stat.files[i]->name;
+ unsigned int hash = strhash(name);
+
+ entry = hashmap_get_from_hash(&s->file_map, hash, name);
+ if (!entry) {
+ FLEX_ALLOC_STR(entry, name, name);
+ hashmap_entry_init(entry, hash);
+ hashmap_add(&s->file_map, entry);
+ }
+
+ if (s->phase == WORKTREE) {
+ entry->worktree.added = stat.files[i]->added;
+ entry->worktree.deleted = stat.files[i]->deleted;
+ } else if (s->phase == INDEX) {
+ entry->index.added = stat.files[i]->added;
+ entry->index.deleted = stat.files[i]->deleted;
+ }
+ }
+}
+
+static void collect_changes_worktree(struct collection_status *s)
+{
+ struct rev_info rev;
+
+ s->phase = WORKTREE;
+
+ init_revisions(&rev, NULL);
+ setup_revisions(0, NULL, &rev, NULL);
+
+ rev.max_count = 0;
+
+ rev.diffopt.flags.ignore_dirty_submodules = 1;
+ rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
+ rev.diffopt.format_callback = collect_changes_cb;
+ rev.diffopt.format_callback_data = s;
+
+ run_diff_files(&rev, 0);
+}
+
+static void collect_changes_index(struct collection_status *s)
+{
+ struct rev_info rev;
+ struct setup_revision_opt opt = { 0 };
+
+ s->phase = INDEX;
+
+ init_revisions(&rev, NULL);
+ opt.def = s->reference;
+ setup_revisions(0, NULL, &rev, &opt);
+
+ rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
+ rev.diffopt.format_callback = collect_changes_cb;
+ rev.diffopt.format_callback_data = s;
+
+ run_diff_index(&rev, 1);
+}
+
+void add_i_print_modified(void)
+{
+ int i = 0;
+ struct collection_status s;
+ /* TRANSLATORS: you can adjust this to align "git add -i" status menu */
+ const char *modified_fmt = _("%12s %12s %s");
+ const char *header_color = get_color(COLOR_HEADER);
+ struct object_id sha1;
+
+ struct hashmap_iter iter;
+ struct file_stat **files;
+ struct file_stat *entry;
+
+ if (read_cache() < 0)
+ return;
+
+ s.reference = !get_oid("HEAD", &sha1) ? "HEAD": empty_tree_oid_hex();
+ hashmap_init(&s.file_map, hash_cmp, NULL, 0);
+
+ collect_changes_worktree(&s);
+ collect_changes_index(&s);
+
+ if (hashmap_get_size(&s.file_map) < 1) {
+ printf("\n");
+ return;
+ }
+
+ printf(HEADER_INDENT);
+ color_fprintf(stdout, header_color, modified_fmt, _("staged"),
+ _("unstaged"), _("path"));
+ printf("\n");
+
+ hashmap_iter_init(&s.file_map, &iter);
+
+ files = xcalloc(hashmap_get_size(&s.file_map), sizeof(struct file_stat *));
+ while ((entry = hashmap_iter_next(&iter))) {
+ files[i++] = entry;
+ }
+ QSORT(files, hashmap_get_size(&s.file_map), alphabetical_cmp);
+
+ for (i = 0; i < hashmap_get_size(&s.file_map); i++) {
+ struct file_stat *f = files[i];
+
+ char worktree_changes[50];
+ char index_changes[50];
+
+ if (f->worktree.added || f->worktree.deleted)
+ snprintf(worktree_changes, 50, "+%"PRIuMAX"/-%"PRIuMAX, f->worktree.added,
+ f->worktree.deleted);
+ else
+ snprintf(worktree_changes, 50, "%s", _("nothing"));
+
+ if (f->index.added || f->index.deleted)
+ snprintf(index_changes, 50, "+%"PRIuMAX"/-%"PRIuMAX, f->index.added,
+ f->index.deleted);
+ else
+ snprintf(index_changes, 50, "%s", _("unchanged"));
+
+ printf(" %2d: ", i + 1);
+ printf(modified_fmt, index_changes, worktree_changes, f->name);
+ printf("\n");
+ }
+ printf("\n");
+
+ free(files);
+ hashmap_free(&s.file_map, 1);
+}
diff --git a/add-interactive.h b/add-interactive.h
new file mode 100644
index 0000000000..1f4747553c
--- /dev/null
+++ b/add-interactive.h
@@ -0,0 +1,8 @@
+#ifndef ADD_INTERACTIVE_H
+#define ADD_INTERACTIVE_H
+
+int add_i_config(const char *var, const char *value, void *cbdata);
+
+void add_i_print_modified(void);
+
+#endif
\ No newline at end of file
diff --git a/builtin/add--helper.c b/builtin/add--helper.c
index 6a97f0e191..43545d9af5 100644
--- a/builtin/add--helper.c
+++ b/builtin/add--helper.c
@@ -1,6 +1,38 @@
+#include "add-interactive.h"
#include "builtin.h"
+#include "config.h"
+#include "revision.h"
+
+static const char * const builtin_add_helper_usage[] = {
+ N_("git add-interactive--helper <command>"),
+ NULL
+};
+
+enum cmd_mode {
+ DEFAULT = 0,
+ STATUS
+};
int cmd_add__helper(int argc, const char **argv, const char *prefix)
{
+ enum cmd_mode mode = DEFAULT;
+
+ struct option options[] = {
+ OPT_CMDMODE(0, "status", &mode,
+ N_("print status information with diffstat"), STATUS),
+ OPT_END()
+ };
+
+ git_config(add_i_config, NULL);
+ argc = parse_options(argc, argv, NULL, options,
+ builtin_add_helper_usage,
+ PARSE_OPT_KEEP_ARGV0);
+
+ if (mode == STATUS)
+ add_i_print_modified();
+ else
+ usage_with_options(builtin_add_helper_usage,
+ options);
+
return 0;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH 5/7] add-interactive.c: implement show-help command
From: Slavica Djukic via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Slavica Djukic
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Slavica Djukic <slawica92@hotmail.com>
Implement show-help command in add-interactive.c and use it in
builtin add--helper.c.
Use command name "show-help" instead of "help": add--helper is
builtin, hence add--helper --help would be intercepted by
handle_builtin and re-routed to the help command, without ever
calling cmd_add__helper().
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
add-interactive.c | 19 +++++++++++++++++++
add-interactive.h | 2 ++
builtin/add--helper.c | 7 ++++++-
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/add-interactive.c b/add-interactive.c
index c55d934186..ff5bfbac49 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -8,6 +8,16 @@
#define HEADER_INDENT " "
+/* TRANSLATORS: please do not translate the command names
+ 'status', 'update', 'revert', etc. */
+static const char help_info[] =
+ N_("status - show paths with changes\n"
+ "update - add working tree state to the staged set of changes\n"
+ "revert - revert staged set of changes back to the HEAD version\n"
+ "patch - pick hunks and update selectively\n"
+ "diff - view diff between HEAD and index\n"
+ "add untracked - add contents of untracked files to the staged set of changes");
+
enum collection_phase {
WORKTREE,
INDEX
@@ -244,3 +254,12 @@ void add_i_print_modified(void)
free(files);
hashmap_free(&s.file_map, 1);
}
+
+void show_help(void)
+{
+ const char *help_color = get_color(COLOR_HELP);
+ const char *modified_fmt = _("%s");
+ printf("\n");
+ color_fprintf(stdout, help_color, modified_fmt, _(help_info));
+ printf("\n");
+}
diff --git a/add-interactive.h b/add-interactive.h
index 1f4747553c..a74c65b7e1 100644
--- a/add-interactive.h
+++ b/add-interactive.h
@@ -5,4 +5,6 @@ int add_i_config(const char *var, const char *value, void *cbdata);
void add_i_print_modified(void);
+void show_help(void);
+
#endif
\ No newline at end of file
diff --git a/builtin/add--helper.c b/builtin/add--helper.c
index 43545d9af5..e288412d56 100644
--- a/builtin/add--helper.c
+++ b/builtin/add--helper.c
@@ -10,7 +10,8 @@ static const char * const builtin_add_helper_usage[] = {
enum cmd_mode {
DEFAULT = 0,
- STATUS
+ STATUS,
+ HELP
};
int cmd_add__helper(int argc, const char **argv, const char *prefix)
@@ -20,6 +21,8 @@ int cmd_add__helper(int argc, const char **argv, const char *prefix)
struct option options[] = {
OPT_CMDMODE(0, "status", &mode,
N_("print status information with diffstat"), STATUS),
+ OPT_CMDMODE(0, "show-help", &mode,
+ N_("show help"), HELP),
OPT_END()
};
@@ -30,6 +33,8 @@ int cmd_add__helper(int argc, const char **argv, const char *prefix)
if (mode == STATUS)
add_i_print_modified();
+ else if (mode == HELP)
+ show_help();
else
usage_with_options(builtin_add_helper_usage,
options);
--
gitgitgadget
^ permalink raw reply related
* [PATCH 6/7] Git.pm: introduce environment variable GIT_TEST_PRETEND_TTY
From: Slavica Djukic via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Slavica Djukic
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Slavica Djukic <slawica92@hotmail.com>
To enable testing the colored output on Windows, enable TTY
by using environment variable GIT_TEST_PRETEND_TTY.
This is the original idea by Johannes Schindelin.
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
color.c | 4 ++++
perl/Git.pm | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/color.c b/color.c
index ebb222ec33..4aa6cc3442 100644
--- a/color.c
+++ b/color.c
@@ -323,6 +323,10 @@ static int check_auto_color(int fd)
{
static int color_stderr_is_tty = -1;
int *is_tty_p = fd == 1 ? &color_stdout_is_tty : &color_stderr_is_tty;
+
+ if (git_env_bool("GIT_TEST_PRETEND_TTY", 0))
+ return 1;
+
if (*is_tty_p < 0)
*is_tty_p = isatty(fd);
if (*is_tty_p || (fd == 1 && pager_in_use() && pager_use_color)) {
diff --git a/perl/Git.pm b/perl/Git.pm
index d856930b2e..51a1ce0617 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -757,7 +757,7 @@ and returns boolean (true for "use color", false for "do not use color").
sub get_colorbool {
my ($self, $var) = @_;
- my $stdout_to_tty = (-t STDOUT) ? "true" : "false";
+ my $stdout_to_tty = $ENV{GIT_TEST_PRETEND_TTY} || (-t STDOUT) ? "true" : "false";
my $use_color = $self->command_oneline('config', '--get-colorbool',
$var, $stdout_to_tty);
return ($use_color eq 'true');
--
gitgitgadget
^ permalink raw reply related
* [PATCH 7/7] add--interactive.perl: use add--helper --show-help for help_cmd
From: Slavica Djukic via GitGitGadget @ 2018-12-20 12:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Slavica Djukic
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
From: Slavica Djukic <slawica92@hotmail.com>
Change help_cmd sub in git-add--interactive.perl to use
show-help command from builtin add--helper.
Add test to t3701-add-interactive to verify that show-help
outputs expected content. Use GIT_PRETENT_TTY
introduced in earlier commit to be able to test output color
on Windows.
Signed-off-by: Slavica Djukic <slawica92@hotmail.com>
---
git-add--interactive.perl | 11 +----------
t/t3701-add-interactive.sh | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index a6536f9cf3..32ee729a58 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1717,16 +1717,7 @@ sub quit_cmd {
}
sub help_cmd {
-# TRANSLATORS: please do not translate the command names
-# 'status', 'update', 'revert', etc.
- print colored $help_color, __ <<'EOF' ;
-status - show paths with changes
-update - add working tree state to the staged set of changes
-revert - revert staged set of changes back to the HEAD version
-patch - pick hunks and update selectively
-diff - view diff between HEAD and index
-add untracked - add contents of untracked files to the staged set of changes
-EOF
+ system(qw(git add--helper --show-help));
}
sub process_args {
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 65dfbc033a..9c9d5bd935 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -639,4 +639,29 @@ test_expect_success 'add -p patch editing works with pathological context lines'
test_cmp expected-2 actual
'
+test_expect_success 'show help from add--helper' '
+ git reset --hard &&
+ cat >expect <<-\EOF &&
+
+ <BOLD>*** Commands ***<RESET>
+ 1: <BOLD;BLUE>s<RESET>tatus 2: <BOLD;BLUE>u<RESET>pdate 3: <BOLD;BLUE>r<RESET>evert 4: <BOLD;BLUE>a<RESET>dd untracked
+ 5: <BOLD;BLUE>p<RESET>atch 6: <BOLD;BLUE>d<RESET>iff 7: <BOLD;BLUE>q<RESET>uit 8: <BOLD;BLUE>h<RESET>elp
+ <BOLD;BLUE>What now<RESET>>
+ <BOLD;RED>status - show paths with changes
+ update - add working tree state to the staged set of changes
+ revert - revert staged set of changes back to the HEAD version
+ patch - pick hunks and update selectively
+ diff - view diff between HEAD and index
+ add untracked - add contents of untracked files to the staged set of changes<RESET>
+ <BOLD>*** Commands ***<RESET>
+ 1: <BOLD;BLUE>s<RESET>tatus 2: <BOLD;BLUE>u<RESET>pdate 3: <BOLD;BLUE>r<RESET>evert 4: <BOLD;BLUE>a<RESET>dd untracked
+ 5: <BOLD;BLUE>p<RESET>atch 6: <BOLD;BLUE>d<RESET>iff 7: <BOLD;BLUE>q<RESET>uit 8: <BOLD;BLUE>h<RESET>elp
+ <BOLD;BLUE>What now<RESET>>
+ Bye.
+ EOF
+ test_write_lines h | GIT_TEST_PRETEND_TTY=1 git add -i >actual.colored &&
+ test_decode_color <actual.colored >actual &&
+ test_i18ncmp expect actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/7] Turn git add-i into built-in
From: Johannes Schindelin @ 2018-12-20 12:34 UTC (permalink / raw)
To: git; +Cc: gitster, Slavica Đukić via GitGitGadget
From: "Slavica Đukić via GitGitGadget" <gitgitgadget@gmail.com>
This is the first version of a patch series to start porting
git-add--interactive from Perl to C. Daniel Ferreira's patch series used as
a head start:
https://public-inbox.org/git/1494907234-28903-1-git-send-email-bnmvco@gmail.com/t/#u
Daniel Ferreira (4):
diff: export diffstat interface
add--helper: create builtin helper for interactive add
add-interactive.c: implement status command
add--interactive.perl: use add--helper --status for status_cmd
Slavica Djukic (3):
add-interactive.c: implement show-help command
Git.pm: introduce environment variable GIT_TEST_PRETEND_TTY
add--interactive.perl: use add--helper --show-help for help_cmd
.gitignore | 1 +
Makefile | 2 +
add-interactive.c | 265 +++++++++++++++++++++++++++++++++++++
add-interactive.h | 10 ++
builtin.h | 1 +
builtin/add--helper.c | 43 ++++++
color.c | 4 +
diff.c | 36 ++---
diff.h | 18 +++
git-add--interactive.perl | 15 +--
git.c | 1 +
perl/Git.pm | 2 +-
t/t3701-add-interactive.sh | 25 ++++
13 files changed, 387 insertions(+), 36 deletions(-)
create mode 100644 add-interactive.c
create mode 100644 add-interactive.h
create mode 100644 builtin/add--helper.c
base-commit: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-103%2FslavicaDj%2Fadd-i-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-103/slavicaDj/add-i-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/103
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 0/7] Turn git add-i into built-in
From: Johannes Schindelin @ 2018-12-20 12:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <pull.103.git.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]
Hi,
just a note: this mail had not been sent by GitGitGadget (and hence has me
as sender, even if the email address is GitGitGadget's) because it still
has problems with From: and To: and Cc: headers that contain non-ASCII
characters. I hope to find the time soon to work on this.
Ciao,
Johannes
On Thu, 20 Dec 2018, Johannes Schindelin wrote:
> From: "Slavica Đukić via GitGitGadget" <gitgitgadget@gmail.com>
>
> This is the first version of a patch series to start porting
> git-add--interactive from Perl to C. Daniel Ferreira's patch series used as
> a head start:
> https://public-inbox.org/git/1494907234-28903-1-git-send-email-bnmvco@gmail.com/t/#u
>
> Daniel Ferreira (4):
> diff: export diffstat interface
> add--helper: create builtin helper for interactive add
> add-interactive.c: implement status command
> add--interactive.perl: use add--helper --status for status_cmd
>
> Slavica Djukic (3):
> add-interactive.c: implement show-help command
> Git.pm: introduce environment variable GIT_TEST_PRETEND_TTY
> add--interactive.perl: use add--helper --show-help for help_cmd
>
> .gitignore | 1 +
> Makefile | 2 +
> add-interactive.c | 265 +++++++++++++++++++++++++++++++++++++
> add-interactive.h | 10 ++
> builtin.h | 1 +
> builtin/add--helper.c | 43 ++++++
> color.c | 4 +
> diff.c | 36 ++---
> diff.h | 18 +++
> git-add--interactive.perl | 15 +--
> git.c | 1 +
> perl/Git.pm | 2 +-
> t/t3701-add-interactive.sh | 25 ++++
> 13 files changed, 387 insertions(+), 36 deletions(-)
> create mode 100644 add-interactive.c
> create mode 100644 add-interactive.h
> create mode 100644 builtin/add--helper.c
>
>
> base-commit: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-103%2FslavicaDj%2Fadd-i-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-103/slavicaDj/add-i-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/103
> --
> gitgitgadget
>
^ permalink raw reply
* Re: [PATCH 2/8] entry: factor out unlink_entry function
From: Thomas Gummerer @ 2018-12-20 13:36 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano, Elijah Newren
In-Reply-To: <CACsJy8AGerhjnT0O6vT264tND78N5cbgFREzYtdmriXERu0Jtw@mail.gmail.com>
On 12/10, Duy Nguyen wrote:
> On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote:
> >
> > Factor out the 'unlink_entry()' function from unpack-trees.c to
> > entry.c. It will be used in other places as well in subsequent
> > steps.
> >
> > As it's no longer a static function, also move the documentation to
> > the header file to make it more discoverable.
> >
> > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> > ---
> > cache.h | 5 +++++
> > entry.c | 15 +++++++++++++++
> > unpack-trees.c | 19 -------------------
> > 3 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/cache.h b/cache.h
> > index ca36b44ee0..c1c953e810 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1542,6 +1542,11 @@ struct checkout {
> > extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
> > extern void enable_delayed_checkout(struct checkout *state);
> > extern int finish_delayed_checkout(struct checkout *state);
> > +/*
> > + * Unlink the last component and schedule the leading directories for
> > + * removal, such that empty directories get removed.
> > + */
> > +extern void unlink_entry(const struct cache_entry *ce);
>
> I'm torn. We try to remove 'extern' but I can see you may want to add
> it here to be consistent with others. And removing extern even from
> functions from entry.c only would cause some conflicts.
Yeah I felt like favoring consistency here would be better. Once your
path counting series and my series land, this may get quieter and we
can remove the 'extern' then?
> I wonder if we should move the 'removal' variable in symlinks to
> 'struct checkout' to reduce another global variable. But I guess
> that's the problem for another day. It's not the focus of this
> series.
Yeah, I'd prefer leaving that for another day :)
> --
> Duy
^ permalink raw reply
* [PATCH v2 0/8] introduce no-overlay mode in git checkout
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181209200449.16342-1-t.gummerer@gmail.com>
Previous round is at <20181209200449.16342-1-t.gummerer@gmail.com>.
Thanks Junio, Duy and Elijah for your comments and suggestions on the
previous round.
This round drops the last three patches from the previous round,
namely introducing a "--cached" and a "--ignore-unmatched" option, and
using the new no-overlay mode in "git stash". The --ignore-unmatched
option may not be necessary, while using the new mode in 'git stash'
will be done once the stash-in-C topic landed.
Introducing a --cached and --worktree-only (as suggested by Elijah)
option can come in a future step, they are orthogonal to this topic.
Other changes from v1:
- Rebase onto the current master, so we can also move t2028 and t2029
to the t24xx range.
- Add a comment clarifying why using the CE_WT_REMOVE flag and topath
in checkout_entry is a bug.
- clarify a comment in checkout.c
- factor out the function to mark a cache entry as CE_MATCHED, and
have separate such functions for overlay mode and no-overlay mode.
This should hopefully make the logic a bit easier to follow.
- Adjust the commit message, justifying why we don't remove untracked
files even in the new no-overlay mode.
- add documentation for the new feature
- document that -p defaults to no overlay mode, and cannot be used
with overlay mode.
- add a config option checkout.overlayMode, so overlay mode can be
turned on by default.
Range-diff can be found after the diffstat.
Thomas Gummerer (8):
move worktree tests to t24*
entry: factor out unlink_entry function
entry: support CE_WT_REMOVE flag in checkout_entry
read-cache: add invalidate parameter to remove_marked_cache_entries
checkout: clarify comment
checkout: factor out mark_cache_entry_for_checkout function
checkout: introduce --{,no-}overlay option
checkout: introduce checkout.overlayMode config
Documentation/config/checkout.txt | 7 +
Documentation/git-checkout.txt | 10 ++
builtin/checkout.c | 133 +++++++++++++-----
cache.h | 7 +-
entry.c | 26 ++++
read-cache.c | 8 +-
split-index.c | 2 +-
t/t2025-checkout-no-overlay.sh | 57 ++++++++
...-worktree-add.sh => t2400-worktree-add.sh} | 0
...ktree-prune.sh => t2401-worktree-prune.sh} | 0
...orktree-list.sh => t2402-worktree-list.sh} | 0
...orktree-move.sh => t2403-worktree-move.sh} | 0
...ree-config.sh => t2404-worktree-config.sh} | 0
t/t9902-completion.sh | 1 +
unpack-trees.c | 21 +--
15 files changed, 213 insertions(+), 59 deletions(-)
create mode 100755 t/t2025-checkout-no-overlay.sh
rename t/{t2025-worktree-add.sh => t2400-worktree-add.sh} (100%)
rename t/{t2026-worktree-prune.sh => t2401-worktree-prune.sh} (100%)
rename t/{t2027-worktree-list.sh => t2402-worktree-list.sh} (100%)
rename t/{t2028-worktree-move.sh => t2403-worktree-move.sh} (100%)
rename t/{t2029-worktree-config.sh => t2404-worktree-config.sh} (100%)
1: 70bd75b202 ! 1: fa450cda7c move worktree tests to t24*
@@ -29,3 +29,13 @@
similarity index 100%
rename from t/t2027-worktree-list.sh
rename to t/t2402-worktree-list.sh
+
+ diff --git a/t/t2028-worktree-move.sh b/t/t2403-worktree-move.sh
+ similarity index 100%
+ rename from t/t2028-worktree-move.sh
+ rename to t/t2403-worktree-move.sh
+
+ diff --git a/t/t2029-worktree-config.sh b/t/t2404-worktree-config.sh
+ similarity index 100%
+ rename from t/t2029-worktree-config.sh
+ rename to t/t2404-worktree-config.sh
2: 0fd9be987d = 2: 9ada8d3484 entry: factor out unlink_entry function
3: 4d6112b112 ! 3: 41c0ea4047 entry: support CE_WT_REMOVE flag in checkout_entry
@@ -22,6 +22,10 @@
+ if (ce->ce_flags & CE_WT_REMOVE) {
+ if (topath)
++ /*
++ * No content and thus no path to create, so we have
++ * no pathname to return.
++ */
+ BUG("Can't remove entry to a path");
+ unlink_entry(ce);
+ return 0;
4: 6e9f68b8f1 ! 4: afccb0848d read-cache: add invalidate parameter to remove_marked_cache_entries
@@ -11,6 +11,10 @@
function will take care of invalidating the path in the cache tree and
in the untracked cache.
+ Note that the current callsites already do the invalidation properly
+ in other places, so we're just passing 0 from there to keep the status
+ quo.
+
This will be useful in a subsequent commit.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
-: ---------- > 5: 8a2b5efdad checkout: clarify comment
-: ---------- > 6: c405f20471 checkout: factor out mark_cache_entry_for_checkout function
5: 4a7670d34c ! 7: e5b18bcd02 checkout: introduce --{,no-}overlay option
@@ -17,8 +17,43 @@
'git checkout --overlay -p' to avoid confusing users who would expect
to be able to force overlay mode in 'git checkout -p' this way.
+ Untracked files are not affected by this change, so 'git checkout
+ --no-overlay HEAD -- untracked' will not remove untracked from the
+ working tree. This is so e.g. 'git checkout --no-overlay HEAD -- dir/'
+ doesn't delete all untracked files in dir/, but rather just resets the
+ state of files that are known to git.
+
+ Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
+ diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
+ --- a/Documentation/git-checkout.txt
+ +++ b/Documentation/git-checkout.txt
+@@
+ This means that you can use `git checkout -p` to selectively discard
+ edits from your current working tree. See the ``Interactive Mode''
+ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
+++
++Note that this option uses the no overlay mode by default (see also
++-`--[no-]overlay`), and currently doesn't support overlay mode.
+
+ --ignore-other-worktrees::
+ `git checkout` refuses when the wanted ref is already checked
+@@
+ Just like linkgit:git-submodule[1], this will detach the
+ submodules HEAD.
+
++--[no-]overlay::
++ In the default overlay mode files `git checkout` never
++ removes files from the index or the working tree. When
++ specifying --no-overlay, files that appear in the index and
++ working tree, but not in <tree-ish> are removed, to make them
++ match <tree-ish> exactly.
++
+ <branch>::
+ Branch to checkout; if it refers to a branch (i.e., a name that,
+ when prepended with "refs/heads/", is a valid ref), then that
+
diff --git a/builtin/checkout.c b/builtin/checkout.c
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -70,44 +105,60 @@
return error(_("path '%s' does not have our version"), ce->name);
else
@@
- ce->ce_flags &= ~CE_MATCHED;
- if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
- continue;
-- if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
-- /*
-- * "git checkout tree-ish -- path", but this entry
-- * is in the original index; it will not be checked
-- * out to the working tree and it does not matter
-- * if pathspec matched this entry. We will not do
-- * anything to this entry at all.
-- */
-- continue;
-+ if (opts->source_tree && !(ce->ce_flags & CE_UPDATE)) {
-+ if (!opts->overlay_mode &&
-+ ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) {
-+ /*
-+ * "git checkout --no-overlay <tree-ish> -- path",
-+ * and the path is not in tree-ish, but is in
-+ * the current index, which means that it should
-+ * be removed.
-+ */
-+ ce->ce_flags |= CE_MATCHED | CE_REMOVE | CE_WT_REMOVE;
-+ continue;
-+ } else {
-+ /*
-+ * "git checkout tree-ish -- path", but this
-+ * entry is in the original index; it will not
-+ * be checked out to the working tree and it
-+ * does not matter if pathspec matched this
-+ * entry. We will not do anything to this entry
-+ * at all.
-+ */
-+ continue;
-+ }
-+ }
- /*
- * Either this entry came from the tree-ish we are
- * checking the paths out of, or we are checking out
+ return status;
+ }
+
+-static void mark_ce_for_checkout(struct cache_entry *ce,
+- char *ps_matched,
+- const struct checkout_opts *opts)
++static void mark_ce_for_checkout_overlay(struct cache_entry *ce,
++ char *ps_matched,
++ const struct checkout_opts *opts)
+ {
+ ce->ce_flags &= ~CE_MATCHED;
+ if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
+@@
+ ce->ce_flags |= CE_MATCHED;
+ }
+
++static void mark_ce_for_checkout_no_overlay(struct cache_entry *ce,
++ char *ps_matched,
++ const struct checkout_opts *opts)
++{
++ ce->ce_flags &= ~CE_MATCHED;
++ if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
++ return;
++ if (ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) {
++ ce->ce_flags |= CE_MATCHED;
++ if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
++ /*
++ * In overlay mode, but the path is not in
++ * tree-ish, which means we should remove it
++ * from the index and the working tree.
++ */
++ ce->ce_flags |= CE_REMOVE | CE_WT_REMOVE;
++ }
++}
++
+ static int checkout_paths(const struct checkout_opts *opts,
+ const char *revision)
+ {
+@@
+ * to be checked out.
+ */
+ for (pos = 0; pos < active_nr; pos++)
+- mark_ce_for_checkout(active_cache[pos], ps_matched, opts);
++ if (opts->overlay_mode)
++ mark_ce_for_checkout_overlay(active_cache[pos],
++ ps_matched,
++ opts);
++ else
++ mark_ce_for_checkout_no_overlay(active_cache[pos],
++ ps_matched,
++ opts);
+
+ if (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) {
+ free(ps_matched);
@@
if (opts->force) {
warning(_("path '%s' is unmerged"), ce->name);
@@ -160,7 +211,7 @@
"checkout", "control recursive updating of submodules",
PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
-+ OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")),
++ OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
OPT_END(),
};
@@ -198,7 +249,7 @@
+ git commit --allow-empty -m "initial"
+'
+
-+test_expect_success 'checkout --no-overlay deletes files not in <tree>' '
++test_expect_success 'checkout --no-overlay deletes files not in <tree-ish>' '
+ >file &&
+ mkdir dir &&
+ >dir/file1 &&
@@ -218,7 +269,7 @@
+ test_i18ngrep "fatal: -p and --overlay are mutually exclusive" actual
+'
+
-+test_expect_success '--no-overlay --theirs with M/D conflict deletes file' '
++test_expect_success '--no-overlay --theirs with D/F conflict deletes file' '
+ test_commit file1 file1 &&
+ test_commit file2 file2 &&
+ git rm --cached file1 &&
6: 695b671675 < -: ---------- checkout: add --cached option
7: d0b5a356b2 < -: ---------- checkout: allow ignoring unmatched pathspec
8: 0a4565acc1 < -: ---------- stash: use git checkout --no-overlay
-: ---------- > 8: de24990d57 checkout: introduce checkout.overlayMode config
--
2.20.1.415.g653613c723
^ permalink raw reply
* [PATCH v2 1/8] move worktree tests to t24*
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
The 'git worktree' command used to be just another mode in 'git
checkout', namely 'git checkout --to'. When the tests for the latter
were retrofitted for the former, the test name was adjusted, but the
test number was kept, even though the test is testing a different
command now. t/README states: "Second digit tells the particular
command we are testing.", so 'git worktree' should have a separate
number just for itself.
Move the worktree tests to t24* to adhere to that guideline. We're
going to make use of the free'd up numbers in a subsequent commit.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
t/{t2025-worktree-add.sh => t2400-worktree-add.sh} | 0
t/{t2026-worktree-prune.sh => t2401-worktree-prune.sh} | 0
t/{t2027-worktree-list.sh => t2402-worktree-list.sh} | 0
t/{t2028-worktree-move.sh => t2403-worktree-move.sh} | 0
t/{t2029-worktree-config.sh => t2404-worktree-config.sh} | 0
5 files changed, 0 insertions(+), 0 deletions(-)
rename t/{t2025-worktree-add.sh => t2400-worktree-add.sh} (100%)
rename t/{t2026-worktree-prune.sh => t2401-worktree-prune.sh} (100%)
rename t/{t2027-worktree-list.sh => t2402-worktree-list.sh} (100%)
rename t/{t2028-worktree-move.sh => t2403-worktree-move.sh} (100%)
rename t/{t2029-worktree-config.sh => t2404-worktree-config.sh} (100%)
diff --git a/t/t2025-worktree-add.sh b/t/t2400-worktree-add.sh
similarity index 100%
rename from t/t2025-worktree-add.sh
rename to t/t2400-worktree-add.sh
diff --git a/t/t2026-worktree-prune.sh b/t/t2401-worktree-prune.sh
similarity index 100%
rename from t/t2026-worktree-prune.sh
rename to t/t2401-worktree-prune.sh
diff --git a/t/t2027-worktree-list.sh b/t/t2402-worktree-list.sh
similarity index 100%
rename from t/t2027-worktree-list.sh
rename to t/t2402-worktree-list.sh
diff --git a/t/t2028-worktree-move.sh b/t/t2403-worktree-move.sh
similarity index 100%
rename from t/t2028-worktree-move.sh
rename to t/t2403-worktree-move.sh
diff --git a/t/t2029-worktree-config.sh b/t/t2404-worktree-config.sh
similarity index 100%
rename from t/t2029-worktree-config.sh
rename to t/t2404-worktree-config.sh
--
2.20.1.415.g653613c723
^ permalink raw reply
* [PATCH v2 2/8] entry: factor out unlink_entry function
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
Factor out the 'unlink_entry()' function from unpack-trees.c to
entry.c. It will be used in other places as well in subsequent
steps.
As it's no longer a static function, also move the documentation to
the header file to make it more discoverable.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
cache.h | 5 +++++
entry.c | 15 +++++++++++++++
unpack-trees.c | 19 -------------------
3 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/cache.h b/cache.h
index ca36b44ee0..c1c953e810 100644
--- a/cache.h
+++ b/cache.h
@@ -1542,6 +1542,11 @@ struct checkout {
extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
extern void enable_delayed_checkout(struct checkout *state);
extern int finish_delayed_checkout(struct checkout *state);
+/*
+ * Unlink the last component and schedule the leading directories for
+ * removal, such that empty directories get removed.
+ */
+extern void unlink_entry(const struct cache_entry *ce);
struct cache_def {
struct strbuf path;
diff --git a/entry.c b/entry.c
index 0a3c451f5f..b9eef57117 100644
--- a/entry.c
+++ b/entry.c
@@ -508,3 +508,18 @@ int checkout_entry(struct cache_entry *ce,
create_directories(path.buf, path.len, state);
return write_entry(ce, path.buf, state, 0);
}
+
+void unlink_entry(const struct cache_entry *ce)
+{
+ const struct submodule *sub = submodule_from_ce(ce);
+ if (sub) {
+ /* state.force is set at the caller. */
+ submodule_move_head(ce->name, "HEAD", NULL,
+ SUBMODULE_MOVE_HEAD_FORCE);
+ }
+ if (!check_leading_path(ce->name, ce_namelen(ce)))
+ return;
+ if (remove_or_warn(ce->ce_mode, ce->name))
+ return;
+ schedule_dir_for_removal(ce->name, ce_namelen(ce));
+}
diff --git a/unpack-trees.c b/unpack-trees.c
index 7570df481b..e8d1a6ac50 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -300,25 +300,6 @@ static void load_gitmodules_file(struct index_state *index,
}
}
-/*
- * Unlink the last component and schedule the leading directories for
- * removal, such that empty directories get removed.
- */
-static void unlink_entry(const struct cache_entry *ce)
-{
- const struct submodule *sub = submodule_from_ce(ce);
- if (sub) {
- /* state.force is set at the caller. */
- submodule_move_head(ce->name, "HEAD", NULL,
- SUBMODULE_MOVE_HEAD_FORCE);
- }
- if (!check_leading_path(ce->name, ce_namelen(ce)))
- return;
- if (remove_or_warn(ce->ce_mode, ce->name))
- return;
- schedule_dir_for_removal(ce->name, ce_namelen(ce));
-}
-
static struct progress *get_progress(struct unpack_trees_options *o)
{
unsigned cnt = 0, total = 0;
--
2.20.1.415.g653613c723
^ permalink raw reply related
* [PATCH v2 3/8] entry: support CE_WT_REMOVE flag in checkout_entry
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
'checkout_entry()' currently only supports creating new entries in the
working tree, but not deleting them. Add the ability to remove
entries at the same time if the entry is marked with the CE_WT_REMOVE
flag.
Currently this doesn't have any effect, as the CE_WT_REMOVE flag is
only used in unpack-tree, however we will make use of this in a
subsequent step in the series.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
entry.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/entry.c b/entry.c
index b9eef57117..3d3701e7ae 100644
--- a/entry.c
+++ b/entry.c
@@ -441,6 +441,17 @@ int checkout_entry(struct cache_entry *ce,
static struct strbuf path = STRBUF_INIT;
struct stat st;
+ if (ce->ce_flags & CE_WT_REMOVE) {
+ if (topath)
+ /*
+ * No content and thus no path to create, so we have
+ * no pathname to return.
+ */
+ BUG("Can't remove entry to a path");
+ unlink_entry(ce);
+ return 0;
+ }
+
if (topath)
return write_entry(ce, topath, state, 1);
--
2.20.1.415.g653613c723
^ permalink raw reply related
* [PATCH v2 4/8] read-cache: add invalidate parameter to remove_marked_cache_entries
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
When marking cache entries for removal, and later removing them all at
once using 'remove_marked_cache_entries()', cache entries currently
have to be invalidated manually in the cache tree and in the untracked
cache.
Add an invalidate flag to the function. With the flag set, the
function will take care of invalidating the path in the cache tree and
in the untracked cache.
Note that the current callsites already do the invalidation properly
in other places, so we're just passing 0 from there to keep the status
quo.
This will be useful in a subsequent commit.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
cache.h | 2 +-
read-cache.c | 8 +++++++-
split-index.c | 2 +-
unpack-trees.c | 2 +-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index c1c953e810..1deee48f5b 100644
--- a/cache.h
+++ b/cache.h
@@ -751,7 +751,7 @@ extern void rename_index_entry_at(struct index_state *, int pos, const char *new
/* Remove entry, return true if there are more entries to go. */
extern int remove_index_entry_at(struct index_state *, int pos);
-extern void remove_marked_cache_entries(struct index_state *istate);
+extern void remove_marked_cache_entries(struct index_state *istate, int invalidate);
extern int remove_file_from_index(struct index_state *, const char *path);
#define ADD_CACHE_VERBOSE 1
#define ADD_CACHE_PRETEND 2
diff --git a/read-cache.c b/read-cache.c
index bd45dc3e24..978d43f676 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -590,13 +590,19 @@ int remove_index_entry_at(struct index_state *istate, int pos)
* CE_REMOVE is set in ce_flags. This is much more effective than
* calling remove_index_entry_at() for each entry to be removed.
*/
-void remove_marked_cache_entries(struct index_state *istate)
+void remove_marked_cache_entries(struct index_state *istate, int invalidate)
{
struct cache_entry **ce_array = istate->cache;
unsigned int i, j;
for (i = j = 0; i < istate->cache_nr; i++) {
if (ce_array[i]->ce_flags & CE_REMOVE) {
+ if (invalidate) {
+ cache_tree_invalidate_path(istate,
+ ce_array[i]->name);
+ untracked_cache_remove_from_index(istate,
+ ce_array[i]->name);
+ }
remove_name_hash(istate, ce_array[i]);
save_or_free_index_entry(istate, ce_array[i]);
}
diff --git a/split-index.c b/split-index.c
index 5820412dc5..8aebc3661b 100644
--- a/split-index.c
+++ b/split-index.c
@@ -162,7 +162,7 @@ void merge_base_index(struct index_state *istate)
ewah_each_bit(si->replace_bitmap, replace_entry, istate);
ewah_each_bit(si->delete_bitmap, mark_entry_for_delete, istate);
if (si->nr_deletions)
- remove_marked_cache_entries(istate);
+ remove_marked_cache_entries(istate, 0);
for (i = si->nr_replacements; i < si->saved_cache_nr; i++) {
if (!ce_namelen(si->saved_cache[i]))
diff --git a/unpack-trees.c b/unpack-trees.c
index e8d1a6ac50..8e6afa924d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -392,7 +392,7 @@ static int check_updates(struct unpack_trees_options *o)
unlink_entry(ce);
}
}
- remove_marked_cache_entries(index);
+ remove_marked_cache_entries(index, 0);
remove_scheduled_dirs();
if (should_update_submodules() && o->update && !o->dry_run)
--
2.20.1.415.g653613c723
^ permalink raw reply related
* [PATCH v2 5/8] checkout: clarify comment
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
The key point for the if statement is that read_tree_some did not
update the entry, because either it doesn't exist in tree-ish or
doesn't match the pathspec. Clarify that.
Suggested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
builtin/checkout.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acdafc6e4c..cb166b2e07 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -304,10 +304,10 @@ static int checkout_paths(const struct checkout_opts *opts,
continue;
if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
/*
- * "git checkout tree-ish -- path", but this entry
- * is in the original index; it will not be checked
- * out to the working tree and it does not matter
- * if pathspec matched this entry. We will not do
+ * "git checkout tree-ish -- path" and this entry
+ * is in the original index, but is not in tree-ish
+ * or does not match the pathspec; it will not be
+ * checked out to the working tree. We will not do
* anything to this entry at all.
*/
continue;
--
2.20.1.415.g653613c723
^ permalink raw reply related
* [PATCH v2 6/8] checkout: factor out mark_cache_entry_for_checkout function
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
Factor out the code that marks a cache entry as matched for checkout
into a separate function. We are going to introduce a new mode in
'git checkout' in a subsequent commit, that is going to have a
slightly different logic. This would make this code unnecessarily
complex.
Moving that complexity into separate functions will make the code in
the subsequent step easier to follow.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
builtin/checkout.c | 67 +++++++++++++++++++++++++---------------------
1 file changed, 36 insertions(+), 31 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index cb166b2e07..32c4b7f897 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -247,6 +247,40 @@ static int checkout_merged(int pos, const struct checkout *state)
return status;
}
+static void mark_ce_for_checkout(struct cache_entry *ce,
+ char *ps_matched,
+ const struct checkout_opts *opts)
+{
+ ce->ce_flags &= ~CE_MATCHED;
+ if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
+ return;
+ if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
+ /*
+ * "git checkout tree-ish -- path", but this entry
+ * is in the original index but is not in tree-ish
+ * or does not match the pathspec; it will not be
+ * checked out to the working tree. We will not do
+ * anything to this entry at all.
+ */
+ return;
+ /*
+ * Either this entry came from the tree-ish we are
+ * checking the paths out of, or we are checking out
+ * of the index.
+ *
+ * If it comes from the tree-ish, we already know it
+ * matches the pathspec and could just stamp
+ * CE_MATCHED to it from update_some(). But we still
+ * need ps_matched and read_tree_recursive (and
+ * eventually tree_entry_interesting) cannot fill
+ * ps_matched yet. Once it can, we can avoid calling
+ * match_pathspec() for _all_ entries when
+ * opts->source_tree != NULL.
+ */
+ if (ce_path_match(&the_index, ce, &opts->pathspec, ps_matched))
+ ce->ce_flags |= CE_MATCHED;
+}
+
static int checkout_paths(const struct checkout_opts *opts,
const char *revision)
{
@@ -297,37 +331,8 @@ static int checkout_paths(const struct checkout_opts *opts,
* Make sure all pathspecs participated in locating the paths
* to be checked out.
*/
- for (pos = 0; pos < active_nr; pos++) {
- struct cache_entry *ce = active_cache[pos];
- ce->ce_flags &= ~CE_MATCHED;
- if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
- continue;
- if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
- /*
- * "git checkout tree-ish -- path" and this entry
- * is in the original index, but is not in tree-ish
- * or does not match the pathspec; it will not be
- * checked out to the working tree. We will not do
- * anything to this entry at all.
- */
- continue;
- /*
- * Either this entry came from the tree-ish we are
- * checking the paths out of, or we are checking out
- * of the index.
- *
- * If it comes from the tree-ish, we already know it
- * matches the pathspec and could just stamp
- * CE_MATCHED to it from update_some(). But we still
- * need ps_matched and read_tree_recursive (and
- * eventually tree_entry_interesting) cannot fill
- * ps_matched yet. Once it can, we can avoid calling
- * match_pathspec() for _all_ entries when
- * opts->source_tree != NULL.
- */
- if (ce_path_match(&the_index, ce, &opts->pathspec, ps_matched))
- ce->ce_flags |= CE_MATCHED;
- }
+ for (pos = 0; pos < active_nr; pos++)
+ mark_ce_for_checkout(active_cache[pos], ps_matched, opts);
if (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) {
free(ps_matched);
--
2.20.1.415.g653613c723
^ permalink raw reply related
* [PATCH v2 7/8] checkout: introduce --{,no-}overlay option
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
Currently 'git checkout' is defined as an overlay operation, which
means that if in 'git checkout <tree-ish> -- [<pathspec>]' we have an
entry in the index that matches <pathspec>, but that doesn't exist in
<tree-ish>, that entry will not be removed from the index or the
working tree.
Introduce a new --{,no-}overlay option, which allows using 'git
checkout' in non-overlay mode, thus removing files from the working
tree if they do not exist in <tree-ish> but match <pathspec>.
Note that 'git checkout -p <tree-ish> -- [<pathspec>]' already works
this way, so no changes are needed for the patch mode. We disallow
'git checkout --overlay -p' to avoid confusing users who would expect
to be able to force overlay mode in 'git checkout -p' this way.
Untracked files are not affected by this change, so 'git checkout
--no-overlay HEAD -- untracked' will not remove untracked from the
working tree. This is so e.g. 'git checkout --no-overlay HEAD -- dir/'
doesn't delete all untracked files in dir/, but rather just resets the
state of files that are known to git.
Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/git-checkout.txt | 10 ++++++
builtin/checkout.c | 66 +++++++++++++++++++++++++++++-----
t/t2025-checkout-no-overlay.sh | 47 ++++++++++++++++++++++++
t/t9902-completion.sh | 1 +
4 files changed, 116 insertions(+), 8 deletions(-)
create mode 100755 t/t2025-checkout-no-overlay.sh
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 801de2f764..4ac8c55865 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -260,6 +260,9 @@ the conflicted merge in the specified paths.
This means that you can use `git checkout -p` to selectively discard
edits from your current working tree. See the ``Interactive Mode''
section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
++
+Note that this option uses the no overlay mode by default (see also
+-`--[no-]overlay`), and currently doesn't support overlay mode.
--ignore-other-worktrees::
`git checkout` refuses when the wanted ref is already checked
@@ -276,6 +279,13 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
Just like linkgit:git-submodule[1], this will detach the
submodules HEAD.
+--[no-]overlay::
+ In the default overlay mode files `git checkout` never
+ removes files from the index or the working tree. When
+ specifying --no-overlay, files that appear in the index and
+ working tree, but not in <tree-ish> are removed, to make them
+ match <tree-ish> exactly.
+
<branch>::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with "refs/heads/", is a valid ref), then that
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 32c4b7f897..0c5fe948ef 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -44,6 +44,7 @@ struct checkout_opts {
int ignore_skipworktree;
int ignore_other_worktrees;
int show_progress;
+ int overlay_mode;
/*
* If new checkout options are added, skip_merge_working_tree
* should be updated accordingly.
@@ -132,7 +133,8 @@ static int skip_same_name(const struct cache_entry *ce, int pos)
return pos;
}
-static int check_stage(int stage, const struct cache_entry *ce, int pos)
+static int check_stage(int stage, const struct cache_entry *ce, int pos,
+ int overlay_mode)
{
while (pos < active_nr &&
!strcmp(active_cache[pos]->name, ce->name)) {
@@ -140,6 +142,8 @@ static int check_stage(int stage, const struct cache_entry *ce, int pos)
return 0;
pos++;
}
+ if (!overlay_mode)
+ return 0;
if (stage == 2)
return error(_("path '%s' does not have our version"), ce->name);
else
@@ -165,7 +169,7 @@ static int check_stages(unsigned stages, const struct cache_entry *ce, int pos)
}
static int checkout_stage(int stage, const struct cache_entry *ce, int pos,
- const struct checkout *state)
+ const struct checkout *state, int overlay_mode)
{
while (pos < active_nr &&
!strcmp(active_cache[pos]->name, ce->name)) {
@@ -173,6 +177,10 @@ static int checkout_stage(int stage, const struct cache_entry *ce, int pos,
return checkout_entry(active_cache[pos], state, NULL);
pos++;
}
+ if (!overlay_mode) {
+ unlink_entry(ce);
+ return 0;
+ }
if (stage == 2)
return error(_("path '%s' does not have our version"), ce->name);
else
@@ -247,9 +255,9 @@ static int checkout_merged(int pos, const struct checkout *state)
return status;
}
-static void mark_ce_for_checkout(struct cache_entry *ce,
- char *ps_matched,
- const struct checkout_opts *opts)
+static void mark_ce_for_checkout_overlay(struct cache_entry *ce,
+ char *ps_matched,
+ const struct checkout_opts *opts)
{
ce->ce_flags &= ~CE_MATCHED;
if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
@@ -281,6 +289,25 @@ static void mark_ce_for_checkout(struct cache_entry *ce,
ce->ce_flags |= CE_MATCHED;
}
+static void mark_ce_for_checkout_no_overlay(struct cache_entry *ce,
+ char *ps_matched,
+ const struct checkout_opts *opts)
+{
+ ce->ce_flags &= ~CE_MATCHED;
+ if (!opts->ignore_skipworktree && ce_skip_worktree(ce))
+ return;
+ if (ce_path_match(&the_index, ce, &opts->pathspec, ps_matched)) {
+ ce->ce_flags |= CE_MATCHED;
+ if (opts->source_tree && !(ce->ce_flags & CE_UPDATE))
+ /*
+ * In overlay mode, but the path is not in
+ * tree-ish, which means we should remove it
+ * from the index and the working tree.
+ */
+ ce->ce_flags |= CE_REMOVE | CE_WT_REMOVE;
+ }
+}
+
static int checkout_paths(const struct checkout_opts *opts,
const char *revision)
{
@@ -332,7 +359,14 @@ static int checkout_paths(const struct checkout_opts *opts,
* to be checked out.
*/
for (pos = 0; pos < active_nr; pos++)
- mark_ce_for_checkout(active_cache[pos], ps_matched, opts);
+ if (opts->overlay_mode)
+ mark_ce_for_checkout_overlay(active_cache[pos],
+ ps_matched,
+ opts);
+ else
+ mark_ce_for_checkout_no_overlay(active_cache[pos],
+ ps_matched,
+ opts);
if (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) {
free(ps_matched);
@@ -353,7 +387,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (opts->force) {
warning(_("path '%s' is unmerged"), ce->name);
} else if (opts->writeout_stage) {
- errs |= check_stage(opts->writeout_stage, ce, pos);
+ errs |= check_stage(opts->writeout_stage, ce, pos, opts->overlay_mode);
} else if (opts->merge) {
errs |= check_stages((1<<2) | (1<<3), ce, pos);
} else {
@@ -380,12 +414,14 @@ static int checkout_paths(const struct checkout_opts *opts,
continue;
}
if (opts->writeout_stage)
- errs |= checkout_stage(opts->writeout_stage, ce, pos, &state);
+ errs |= checkout_stage(opts->writeout_stage, ce, pos, &state, opts->overlay_mode);
else if (opts->merge)
errs |= checkout_merged(pos, &state);
pos = skip_same_name(ce, pos) - 1;
}
}
+ remove_marked_cache_entries(&the_index, 1);
+ remove_scheduled_dirs();
errs |= finish_delayed_checkout(&state);
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
@@ -547,6 +583,11 @@ static int skip_merge_working_tree(const struct checkout_opts *opts,
* opts->show_progress only impacts output so doesn't require a merge
*/
+ /*
+ * opts->overlay_mode cannot be used with switching branches so is
+ * not tested here
+ */
+
/*
* If we aren't creating a new branch any changes or updates will
* happen in the existing branch. Since that could only be updating
@@ -1183,6 +1224,10 @@ static int checkout_branch(struct checkout_opts *opts,
die(_("'%s' cannot be used with switching branches"),
"--patch");
+ if (!opts->overlay_mode)
+ die(_("'%s' cannot be used with switching branches"),
+ "--no-overlay");
+
if (opts->writeout_stage)
die(_("'%s' cannot be used with switching branches"),
"--ours/--theirs");
@@ -1271,6 +1316,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
"checkout", "control recursive updating of submodules",
PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater },
OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
+ OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
OPT_END(),
};
@@ -1279,6 +1325,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
opts.overwrite_ignore = 1;
opts.prefix = prefix;
opts.show_progress = -1;
+ opts.overlay_mode = -1;
git_config(git_checkout_config, &opts);
@@ -1302,6 +1349,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
die(_("-b, -B and --orphan are mutually exclusive"));
+ if (opts.overlay_mode == 1 && opts.patch_mode)
+ die(_("-p and --overlay are mutually exclusive"));
+
/*
* From here on, new_branch will contain the branch to be checked out,
* and new_branch_force and new_orphan_branch will tell us which one of
diff --git a/t/t2025-checkout-no-overlay.sh b/t/t2025-checkout-no-overlay.sh
new file mode 100755
index 0000000000..76330cb5ab
--- /dev/null
+++ b/t/t2025-checkout-no-overlay.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+
+test_description='checkout --no-overlay <tree-ish> -- <pathspec>'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ git commit --allow-empty -m "initial"
+'
+
+test_expect_success 'checkout --no-overlay deletes files not in <tree-ish>' '
+ >file &&
+ mkdir dir &&
+ >dir/file1 &&
+ git add file dir/file1 &&
+ git checkout --no-overlay HEAD -- file &&
+ test_path_is_missing file &&
+ test_path_is_file dir/file1
+'
+
+test_expect_success 'checkout --no-overlay removing last file from directory' '
+ git checkout --no-overlay HEAD -- dir/file1 &&
+ test_path_is_missing dir
+'
+
+test_expect_success 'checkout -p --overlay is disallowed' '
+ test_must_fail git checkout -p --overlay HEAD 2>actual &&
+ test_i18ngrep "fatal: -p and --overlay are mutually exclusive" actual
+'
+
+test_expect_success '--no-overlay --theirs with D/F conflict deletes file' '
+ test_commit file1 file1 &&
+ test_commit file2 file2 &&
+ git rm --cached file1 &&
+ echo 1234 >file1 &&
+ F1=$(git rev-parse HEAD:file1) &&
+ F2=$(git rev-parse HEAD:file2) &&
+ {
+ echo "100644 $F1 1 file1" &&
+ echo "100644 $F2 2 file1"
+ } | git update-index --index-info &&
+ test_path_is_file file1 &&
+ git checkout --theirs --no-overlay -- file1 &&
+ test_path_is_missing file1
+'
+
+test_done
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index d01ad8eb25..5758fffa0d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1436,6 +1436,7 @@ test_expect_success 'double dash "git checkout"' '
--progress Z
--no-quiet Z
--no-... Z
+ --overlay Z
EOF
'
--
2.20.1.415.g653613c723
^ permalink raw reply related
* [PATCH v2 8/8] checkout: introduce checkout.overlayMode config
From: Thomas Gummerer @ 2018-12-20 13:48 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy,
Elijah Newren, Thomas Gummerer
In-Reply-To: <20181220134820.21810-1-t.gummerer@gmail.com>
In the previous patch we introduced a new no-overlay mode for git
checkout. Some users (such as the author of this commit) may want to
have this mode turned on by default as it matches their mental model
more closely. Make that possible by introducing a new config option
to that extend.
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
Documentation/config/checkout.txt | 7 +++++++
builtin/checkout.c | 8 +++++++-
t/t2025-checkout-no-overlay.sh | 10 ++++++++++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/checkout.txt b/Documentation/config/checkout.txt
index c4118fa196..53f917e15e 100644
--- a/Documentation/config/checkout.txt
+++ b/Documentation/config/checkout.txt
@@ -21,3 +21,10 @@ checkout.optimizeNewBranch::
will not update the skip-worktree bit in the index nor add/remove
files in the working directory to reflect the current sparse checkout
settings nor will it show the local changes.
+
+checkout.overlayMode::
+ In the default overlay mode files `git checkout` never
+ removes files from the index or the working tree. When
+ setting checkout.overlayMode to false, files that appear in
+ the index and working tree, but not in <tree-ish> are removed,
+ to make them match <tree-ish> exactly.
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 0c5fe948ef..b5dfc45736 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1019,13 +1019,19 @@ static int switch_branches(const struct checkout_opts *opts,
static int git_checkout_config(const char *var, const char *value, void *cb)
{
+ struct checkout_opts *opts = cb;
+
if (!strcmp(var, "checkout.optimizenewbranch")) {
checkout_optimize_new_branch = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "checkout.overlaymode")) {
+ opts->overlay_mode = git_config_bool(var, value);
+ return 0;
+ }
+
if (!strcmp(var, "diff.ignoresubmodules")) {
- struct checkout_opts *opts = cb;
handle_ignore_submodules_arg(&opts->diff_options, value);
return 0;
}
diff --git a/t/t2025-checkout-no-overlay.sh b/t/t2025-checkout-no-overlay.sh
index 76330cb5ab..a4912e35cb 100755
--- a/t/t2025-checkout-no-overlay.sh
+++ b/t/t2025-checkout-no-overlay.sh
@@ -44,4 +44,14 @@ test_expect_success '--no-overlay --theirs with D/F conflict deletes file' '
test_path_is_missing file1
'
+test_expect_success 'checkout with checkout.overlayMode=false deletes files not in <tree-ish>' '
+ >file &&
+ mkdir dir &&
+ >dir/file1 &&
+ git add file dir/file1 &&
+ git -c checkout.overlayMode=false checkout HEAD -- file &&
+ test_path_is_missing file &&
+ test_path_is_file dir/file1
+'
+
test_done
--
2.20.1.415.g653613c723
^ permalink raw reply related
* Re: [PATCH 1/3] setup: drop return value from `read_repository_format()`
From: Jeff King @ 2018-12-20 14:53 UTC (permalink / raw)
To: brian m. carlson, Martin Ågren, git
In-Reply-To: <20181220034555.GC35965@genre.crustytoothpaste.net>
On Thu, Dec 20, 2018 at 03:45:55AM +0000, brian m. carlson wrote:
> > > I will point out that with the SHA-256 work, reading the config file
> > > becomes essential for SHA-256 repositories, because we need to know the
> > > object format. Removing the config file leads to things blowing up in a
> > > bad way (what specific bad way I don't remember).
> > >
> > > That may influence the direction we want to take in this work, or not.
> >
> > Wouldn't we just treat that the same way we do now? I.e., assume the
> > default of sha1, just like we assume repositoryformatversion==0?
>
> Yeah, we'll default to SHA-1, but the repository will be broken. HEAD
> can't be read. Trying to run git status dies with "fatal: Unknown index
> entry format". And so on. We've written data with 64-character object
> IDs, which can't be read by Git in SHA-1 mode.
Oh, I see. Yes, if you have a SHA-256 repository and you don't tell
anybody (via a config entry), then everything will fail to work. That
seems like a perfectly reasonable outcome to me.
> My point is essentially that in an SHA-256 repository, the config file
> isn't optional anymore. We probably need to consider that and error out
> in more situations (e.g. unreadable file or I/O error) instead of
> silently falling back to the defaults, since failing loudly in a visible
> way is better than having the user try to figure out why the index is
> suddenly "corrupt".
Yes, I agree that ideally we'd produce a better error message. I'd just
be wary of breaking compatibility for the existing cases by making new
requirements when we don't yet suspect the repo is SHA-256.
When we see such a corruption, would it be possible to poke at the data
as if it were the old SHA-1 format, and if _that_ looks sane, suggest to
the user what the problem might be? That would help a number of cases
beyond this one (i.e., you're missing config, you have config but it has
the wrong repo format, you're missing the correct extensions field,
etc).
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/3] ref-filter: add worktreepath atom
From: Jeff King @ 2018-12-20 14:59 UTC (permalink / raw)
To: Nickolai Belakovski
Cc: git, Rafael Ascensão, Junio C Hamano,
Ævar Arnfjörð Bjarmason
In-Reply-To: <CAC05384H1LgsGMO=ggUyfFTXrXAFcjUXDSdcmev9ActPt5081A@mail.gmail.com>
On Wed, Dec 19, 2018 at 11:09:59PM -0800, Nickolai Belakovski wrote:
> > Also, why are we replacing it with a single space? Wouldn't the empty
> > string be more customary (and work with the other "if empty, then do
> > this" formatting options)?
>
> I was just following what was done for HEAD, but overall I agree that
> empty is preferable to single space, will change.
Ah, right, that makes more sense. I do still think for %(HEAD) it's a
little different because it is "+" or a single space, so always one
character. Here we have some value or not, and in the "not" case for
such things we usually give an empty string (e.g., for %(push),
%(upstream), etc).
> To sum up the hashmap comments:
> -I hadn't thought to re-use the head_ref of worktree as the key.
> That's clever. I like the readability of having separate entries for
> key and value, but I can see the benefit of not having to do an extra
> allocation. I can make up for the readability hit with a comment.
Thanks, that makes sense.
> -Actually, for any valid use case there will only be one instance of
> the map since the entries of used_atom are cached, but regardless it
> makes sense to keep per-atom info in used_atom and global context
> somewhere else, so I'll make that change to make it a static variable
> outside of used_atom.
Ah, right, I forgot there was some magic around used_atom. I do still
agree that the separate static global makes things a little simpler.
> -Will change the lookup logic to remove the extra allocation. Since
> I'm letting the hashmap use its internal comparison function on the
> hash, I don't need to provide a comparison function.
I don't think that works. The default function is always_equal(), which
will treat two entries equal if they have the same hash value. I.e., any
collisions would be considered a match.
> Thanks for all the feedback, will try to turn these around quickly.
Great, thanks! I'll be on vacation for the next two weeks, so I may be
very slow to look at the next iteration. :)
-Peff
^ permalink raw reply
* Re: Periodic hang during git index-pack
From: Jeff King @ 2018-12-20 15:10 UTC (permalink / raw)
To: Sitsofe Wheeler; +Cc: git
In-Reply-To: <CALjAwxg2E_48kQYt1GHkcXvVmaFyPY3PGG9rHZNMp+++UqKfow@mail.gmail.com>
On Thu, Dec 20, 2018 at 10:03:45AM +0000, Sitsofe Wheeler wrote:
> > Check the backtrace of git-clone to see why it isn't feeding more data
> > (but note that it will generally have two threads -- one processing the
> > data from the remote, and one wait()ing for index-pack to finish).
> >
> > My guess, though, is that you'll find that git-clone is simply waiting
> > on another pipe: the one from ssh.
>
> Ok here are backtraces from another run (with SSH multiplexing disabled):
OK, that's about what I expected. Here we have clone's sideband-demux
thread waiting to pull more packfile data from the remote:
> (gdb) thread apply all bt
>
> Thread 2 (Thread 0x7faafbf1c700 (LWP 36586)):
> #0 0x00007faafc805384 in __libc_read (fd=fd@entry=5,
> buf=buf@entry=0x7faafbf0ddec, nbytes=nbytes@entry=5)
> at ../sysdeps/unix/sysv/linux/read.c:27
> #1 0x000055c8ca2f5b23 in read (__nbytes=5, __buf=0x7faafbf0ddec, __fd=5)
> at /usr/include/x86_64-linux-gnu/bits/unistd.h:44
> [...coming from packet_read / recv_sideband / sideband_demux...]
I assume fd=5 there is a pipe connected to ssh. You could double check
with "lsof" or similar, but I don't think it would ever be reading from
anywhere else.
And then this thread:
> Thread 1 (Thread 0x7faafce3bb80 (LWP 36584)):
> #0 0x00007faafc805384 in __libc_read (fd=fd@entry=7,
> buf=buf@entry=0x7ffda489ef10, nbytes=nbytes@entry=46)
> at ../sysdeps/unix/sysv/linux/read.c:27
> #1 0x000055c8ca2f5b23 in read (__nbytes=46, __buf=0x7ffda489ef10, __fd=7)
> at /usr/include/x86_64-linux-gnu/bits/unistd.h:44
> #2 xread (fd=fd@entry=7, buf=buf@entry=0x7ffda489ef10, len=len@entry=46)
> at wrapper.c:260
> #3 0x000055c8ca2f5cdb in read_in_full (fd=7, buf=buf@entry=0x7ffda489ef10,
> count=count@entry=46) at wrapper.c:318
> #4 0x000055c8ca26a54f in index_pack_lockfile (ip_out=<optimised out>)
> at pack-write.c:297
...is us waiting on index-pack to single completion by printing out the
name of the pack's .keep file.
And then this is just index-pack:
> (gdb) thread apply all bt
>
> Thread 1 (Thread 0x7ff8a03dab80 (LWP 36587)):
> #0 0x00007ff89fda434e in __libc_read (fd=fd@entry=0,
> buf=buf@entry=0x5604bea43d40 <input_buffer>, nbytes=nbytes@entry=4096)
> at ../sysdeps/unix/sysv/linux/read.c:27
> #1 0x00005604be77bb23 in read (__nbytes=4096,
> __buf=0x5604bea43d40 <input_buffer>, __fd=0)
> at /usr/include/x86_64-linux-gnu/bits/unistd.h:44
> #2 xread (fd=0, buf=0x5604bea43d40 <input_buffer>, len=4096) at wrapper.c:260
> #3 0x00005604be5fb069 in fill (min=min@entry=1) at builtin/index-pack.c:255
...waiting for more data.
So though it goes back and forth between two processes, the sequence of
blocking data is linear due to the threads:
ssh -> git-clone -> index-pack -> git-clone
(sideband_demux) (index_pack_lockfile)
with each blocking on read() from its predecessor. So you need to find
out why "ssh" is blocking. Unfortunately, short of a bug in ssh, the
likely cause is either:
1. The git-upload-pack on the remote side stopped generating data for
some reason. You may or may not have access on the remotehost to
dig into that.
It's certainly possible there's a deadlock bug between the server
and client side of a Git conversation. But I'd find it extremely
unlikely to find such a deadlock bug at this point in the
conversation, because at this point the client side has nothing
left to say to the server. The server should just be streaming out
the packfile bytes and then closing the descriptor.
You mentioned "Phabricator sshd scripts" running on the server.
I don't know what Phabricator might be sticking in the middle of
the connection, but that could be the source of the stall.
2. It's possible the network connection dropped but ssh did not
notice. Maybe try turning on ssh keepalives and seeing if it
eventually times out?
-Peff
^ permalink raw reply
* Re: A bug in git-add with GIT_DIR?
From: Jeff King @ 2018-12-20 15:18 UTC (permalink / raw)
To: Orgad Shaneh; +Cc: git
In-Reply-To: <CAGHpTBKyBgPURYfuZgVwnskGSy9L1+3WMrYuPmziQ7VcGDkMcA@mail.gmail.com>
On Thu, Dec 20, 2018 at 10:37:21AM +0200, Orgad Shaneh wrote:
> I played around with t5403-post-checkout-hook, and noticed that its
> state is not exactly what I'd expect it to be.
>
> The test setup is:
> echo Data for commit0. >a &&
> echo Data for commit0. >b &&
> git update-index --add a &&
> git update-index --add b &&
> tree0=$(git write-tree) &&
> commit0=$(echo setup | git commit-tree $tree0) &&
> git update-ref refs/heads/master $commit0 &&
> git clone ./. clone1 &&
> git clone ./. clone2 &&
> GIT_DIR=clone2/.git git branch new2 &&
> echo Data for commit1. >clone2/b &&
> GIT_DIR=clone2/.git git add clone2/b &&
> GIT_DIR=clone2/.git git commit -m new2
>
> Now, the line before the last one executes git add clone2/b with GIT_DIR set.
When GIT_DIR is set but not GIT_WORK_TREE, the current directory is
taken as the working tree.
So that will find clone2/b (from the current directory, which is a real
file), and add an index entry with that path "clone2/b" and the sha1 of
that content.
But when commands are run from inside "clone2", they will naturally
treat "clone2" as the working tree. And since "clone2/b" does not exist
inside there, they will say "oops, it looks like this file has been
deleted".
> I'd expect that to add b inside clone2, but instead it adds an
> inexistent clone2/clone2/b, and if I stop at this line, then the
> status shows:
Sort of. It never sees the path "clone2/clone2/b", but the path in the
index coupled with the working tree being inside clone2 means that it
would look for such a file.
> On branch master
> Your branch is up to date with 'origin/master'.
>
> Changes to be committed:
> (use "git reset HEAD <file>..." to unstage)
>
> new file: clone2/b
>
> Changes not staged for commit:
> (use "git add/rm <file>..." to update what will be committed)
> (use "git checkout -- <file>..." to discard changes in working directory)
>
> modified: b
> deleted: clone2/b
>
> Is this the intended behavior? It looks like that's not what the test
> meant to do anyway...
This is the expected behavior if you did "cd clone2 && git status".
Looking at the test, I don't think it quite meant to do this. It looks
like it predates "git -C", but for some reason did not want to "cd" in a
subshell.
I think it would be better written as:
git -C clone2 add b &&
git -C clone2 commit -m new2
or:
(
cd clone2 &&
git add b &&
git commit -m new2
)
And ditto for all of the other uses of $GIT_DIR in that script. E.g.,
the ones that do:
GIT_DIR=clone1/.git git checkout master
are likely writing the contents of clone1's master branch to the
_current_ directory (not the working tree in clone1).
> And if I change it to (cd clone2 && git add b), then the commits look
> reasonable, but step 6 fails.
You probably just need to update the other calls, too, so they all
match.
-Peff
^ permalink raw reply
* Re: A bug in git-add with GIT_DIR?
From: Orgad Shaneh @ 2018-12-20 15:21 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20181220151830.GD27361@sigill.intra.peff.net>
On Thu, Dec 20, 2018 at 5:18 PM Jeff King <peff@peff.net> wrote:
>
> On Thu, Dec 20, 2018 at 10:37:21AM +0200, Orgad Shaneh wrote:
>
> > I played around with t5403-post-checkout-hook, and noticed that its
> > state is not exactly what I'd expect it to be.
> >
> > The test setup is:
> > echo Data for commit0. >a &&
> > echo Data for commit0. >b &&
> > git update-index --add a &&
> > git update-index --add b &&
> > tree0=$(git write-tree) &&
> > commit0=$(echo setup | git commit-tree $tree0) &&
> > git update-ref refs/heads/master $commit0 &&
> > git clone ./. clone1 &&
> > git clone ./. clone2 &&
> > GIT_DIR=clone2/.git git branch new2 &&
> > echo Data for commit1. >clone2/b &&
> > GIT_DIR=clone2/.git git add clone2/b &&
> > GIT_DIR=clone2/.git git commit -m new2
> >
> > Now, the line before the last one executes git add clone2/b with GIT_DIR set.
>
> When GIT_DIR is set but not GIT_WORK_TREE, the current directory is
> taken as the working tree.
>
> So that will find clone2/b (from the current directory, which is a real
> file), and add an index entry with that path "clone2/b" and the sha1 of
> that content.
>
> But when commands are run from inside "clone2", they will naturally
> treat "clone2" as the working tree. And since "clone2/b" does not exist
> inside there, they will say "oops, it looks like this file has been
> deleted".
>
> > I'd expect that to add b inside clone2, but instead it adds an
> > inexistent clone2/clone2/b, and if I stop at this line, then the
> > status shows:
>
> Sort of. It never sees the path "clone2/clone2/b", but the path in the
> index coupled with the working tree being inside clone2 means that it
> would look for such a file.
>
> > On branch master
> > Your branch is up to date with 'origin/master'.
> >
> > Changes to be committed:
> > (use "git reset HEAD <file>..." to unstage)
> >
> > new file: clone2/b
> >
> > Changes not staged for commit:
> > (use "git add/rm <file>..." to update what will be committed)
> > (use "git checkout -- <file>..." to discard changes in working directory)
> >
> > modified: b
> > deleted: clone2/b
> >
> > Is this the intended behavior? It looks like that's not what the test
> > meant to do anyway...
>
> This is the expected behavior if you did "cd clone2 && git status".
> Looking at the test, I don't think it quite meant to do this. It looks
> like it predates "git -C", but for some reason did not want to "cd" in a
> subshell.
>
> I think it would be better written as:
>
> git -C clone2 add b &&
> git -C clone2 commit -m new2
>
> or:
>
> (
> cd clone2 &&
> git add b &&
> git commit -m new2
> )
>
> And ditto for all of the other uses of $GIT_DIR in that script. E.g.,
> the ones that do:
>
> GIT_DIR=clone1/.git git checkout master
>
> are likely writing the contents of clone1's master branch to the
> _current_ directory (not the working tree in clone1).
>
> > And if I change it to (cd clone2 && git add b), then the commits look
> > reasonable, but step 6 fails.
>
> You probably just need to update the other calls, too, so they all
> match.
>
> -Peff
Thanks. I'll refactor the tests and post a patch later.
- Orgad
^ permalink raw reply
* Re: [PATCH] diff: add support for reading files literally with --no-index
From: Jeff King @ 2018-12-20 15:48 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Duy Nguyen, Dennis Kaarsemaker
In-Reply-To: <20181220002610.43832-1-sandals@crustytoothpaste.net>
On Thu, Dec 20, 2018 at 12:26:10AM +0000, brian m. carlson wrote:
> In some shells, such as bash and zsh, it's possible to use a command
> substitution to provide the output of a command as a file argument to
> another process, like so:
>
> diff -u <(printf "a\nb\n") <(printf "a\nc\n")
>
> However, this syntax does not produce useful results with git diff
> --no-index.
>
> On macOS, the arguments to the command are named pipes under /dev/fd,
> and git diff doesn't know how to handle a named pipe. On Linux, the
> arguments are symlinks to pipes, so git diff "helpfully" diffs these
> symlinks, comparing their targets like "pipe:[1234]" and "pipe:[5678]".
>
> Because this behavior is not very helpful, and because git diff has many
> features that people would like to use even on non-Git files, add an
> option to git diff --no-index to read files literally, dereferencing
> symlinks and reading them as a normal file.
>
> Note that this behavior requires that the files be read entirely into
> memory, just as we do when reading from standard input.
Yeah, I think this is a useful feature to have. It seemed familiar, and
indeed there were some patches a while back:
https://public-inbox.org/git/20170113102021.6054-1-dennis@kaarsemaker.net/
Compared to that series, the implementation here seems simpler and less
likely to have unexpected effects.
Reading the patch, one thing I _thought_ it did (but it does not) is to
impact only the actual arguments on the command line, not entries we
find from recursing trees.
I.e., if I said:
ln -s bar a/foo
ln -s baz b/foo
git diff --no-index --literally a/foo b/foo
git diff --no-index --literally a/ b/
should I still see a/foo as a symlink in the second one, or not?
The distinction is a bit subtle, but I think treating only the actual
top-level arguments as symlinks would solve your problem, but still
allow a more detailed diff for the recursive cases.
There's some more discussion on this in the earlier iteration of that
series:
https://public-inbox.org/git/20161111201958.2175-1-dennis@kaarsemaker.net/
I also suspect people might want a config option to make this the
default (which should be OK, as git-diff is, after all, porcelain). But
either way, a command line option is the first step.
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> This is a long-standing annoyance of mine, and it also makes some use
> cases possible (for example, diffing filtered and non-filtered objects).
>
> We don't include a test for the pipe scenario because I couldn't get
> that case to work in portable shell (although of course it works in
> bash). I have, however, tested it on both macOS and Linux. No clue how
> this works on Windows.
I think focusing on symlinks makes sense. You could probably test pipes
with mkfifo, but looking at your implementation, it's pretty clear that
it will operate on anything that works with read().
> diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
> index 030f162f30..4c4695c88d 100644
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -111,6 +111,11 @@ include::diff-options.txt[]
> "Unmerged". Can be used only when comparing the working tree
> with the index.
>
> +--literally::
> + Read the specified files literally, as `diff` would,
> + dereferencing any symlinks and reading data from pipes.
> + This option only works with `--no-index`.
> +
Looks like spaces for indent, whereas the context uses tabs.
I think "literal" is a good way to describe this concept. If we do grow
a config option to make this the default, then countermanding it would
be "--no-literally", which parses a bit funny as English.
If you agree with my "only the top-level" line of reasoning above, maybe
"--literal-arguments" and "--no-literal-arguments" might make sense.
We could also in theory offer several levels: no literals, top-level
literals, everything literal, at which point it becomes a tri-state.
> -static struct diff_filespec *noindex_filespec(const char *name, int mode)
> +static int populate_literally(struct diff_filespec *s)
> +{
> + struct strbuf buf = STRBUF_INIT;
> + size_t size = 0;
> + int fd = xopen(s->path, O_RDONLY);
> +
> + if (strbuf_read(&buf, fd, 0) < 0)
> + return error_errno("error while reading from '%s'", s->path);
> +
> + s->should_munmap = 0;
> + s->data = strbuf_detach(&buf, &size);
> + s->size = size;
> + s->should_free = 1;
> + s->read_literally = 1;
> + return 0;
> +}
> +
> +static struct diff_filespec *noindex_filespec(const char *name, int mode,
> + struct diff_options *o)
> [...]
> + else if (o->flags.read_literally)
> + populate_literally(s);
The implementation is pleasantly simple. If we want to do the "only
top-level" thing, we'd just have to pass in a depth flag here.
> diff --git a/diff.c b/diff.c
> index dc9965e836..740d0087b9 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4282,18 +4282,18 @@ static void run_diff_cmd(const char *pgm,
> fprintf(o->file, "* Unmerged path %s\n", name);
> }
>
> -static void diff_fill_oid_info(struct diff_filespec *one, struct index_state *istate)
> +static void diff_fill_oid_info(struct diff_filespec *one, struct diff_options *o)
It might be worth breaking these "pass the options around" hunks into a
separate preparatory patch.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox