[PATCH] read-tree: replace bracket set with parentheses to clarify usage
-u and -i can only be given if -m, --reset, or --prefix is given.
Without parentheses, it looks like -u and -i can be used no matter
what, and the second pair of brackets is confusing.
Signed-off-by: Alex Henrie
---
builtin/read-tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 43b47f7..2379e11 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -34,7 +34,7 @@ static int list_tree(unsigned char *sha1)
}
static const char * const read_tree_usage[] = {
- N_("git read-tree [[-m [--trivial] [--aggressive] | --reset |
--prefix=] [-u [--exclude-per-directory=] | -i]]
[--no-sparse-checkout] [--index-output=] (--empty |
[ []])"),
+ N_("git read-tree [(-m [--trivial] [--aggressive] | --reset |
--prefix=) [-u [--exclude-per-directory=] | -i]]
[--no-sparse-checkout] [--index-output=] (--empty |
[ []])"),
NULL
};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] show-ref: place angle brackets around variable in usage string
Signed-off-by: Alex Henrie
---
builtin/show-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index dfbc314..d9c1633 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -8,7 +8,7 @@
static const char * const show_ref_usage[] = {
N_("git show-ref [-q | --quiet] [--verify] [--head] [-d |
--dereference] [-s | --hash[=]] [--abbrev[=]] [--tags] [--heads] [--]
[...]"),
- N_("git show-ref --exclude-existing[=pattern] < ref-list"),
+ N_("git show-ref --exclude-existing[=pattern] < "),
NULL
};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] pack-objects: place angle brackets around variables in usage strings
Signed-off-by: Alex Henrie
---
builtin/pack-objects.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 62cc16d..1c63f8f 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -25,8 +25,8 @@
#include "argv-array.h"
static const char *pack_usage[] = {
- N_("git pack-objects --stdout [options...] [< ref-list | <
object-list]"),
- N_("git pack-objects [options...] base-name [< ref-list | <
object-list]"),
+ N_("git pack-objects --stdout [...] [< | <
]"),
+ N_("git pack-objects [...] [< | <
]"),
NULL
};
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-submodule: remove extraneous space from error message
Signed-off-by: Alex Henrie
---
git-submodule.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 36797c3..25b1ddf 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -904,7 +904,7 @@ Maybe you want to use 'update --init'?")"
;;
!*)
command="${update_module#!}"
- die_msg="$(eval_gettext "Execution of
'\$command \$sha1' failed in submodule path '\$prefix\$sm_path'")"
+ die_msg="$(eval_gettext "Execution of
'\$command \$sha1' failed in submodule path '\$prefix\$sm_path'")"
say_msg="$(eval_gettext "Submodule path
'\$prefix\$sm_path': '\$command \$sha1'")"
must_die_on_failure=yes
;;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] reflog: add missing single quote to error message
The error message can be seen by running
`git config gc.reflogexpire foo` and then `git reflog expire`.
Signed-off-by: Alex Henrie
---
builtin/reflog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7ed0e85..f96ca2a 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -429,7 +429,7 @@ static int parse_expire_cfg_value(const char *var, const
char *value, unsigned l
if (!value)
return config_error_nonbool(var);
if (parse_expiry_date(value, expire))
- return error(_("%s' for '%s' is not a valid timestamp"),
+ return error(_("'%s' for '%s' is not a valid timestamp"),
value, var);
return 0;
}
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] thread-utils: add a threaded task queue
This adds functionality to do work in a parallel threaded
fashion while the boiler plate code for setting up threads
and tearing them down as well as queuing up tasks is hidden
behind the new API.
Signed-off-by: Stefan Beller
---
run-command.c | 29 +---
thread-utils.c | 227 +
thread-utils.h | 35 +
3 files changed, 279 insertions(+), 12 deletions(-)
diff --git a/run-command.c b/run-command.c
index 28e1d55..cb15cd9 100644
--- a/run-command.c
+++ b/run-command.c
@@ -668,6 +668,22 @@ int git_atexit(void (*handler)(void))
#endif
+void setup_main_thread(void)
+{
+ if (!main_thread_set) {
+ /*
+* We assume that the first time that start_async is called
+* it is from the main thread.
+*/
+ main_thread_set = 1;
+ main_thread = pthread_self();
+ pthread_key_create(&async_key, NULL);
+ pthread_key_create(&async_die_counter, NULL);
+ set_die_routine(die_async);
+ set_die_is_recursing_routine(async_die_is_recursing);
+ }
+}
+
int start_async(struct async *async)
{
int need_in, need_out;
@@ -740,18 +756,7 @@ int start_async(struct async *async)
else if (async->out)
close(async->out);
#else
- if (!main_thread_set) {
- /*
-* We assume that the first time that start_async is called
-* it is from the main thread.
-*/
- main_thread_set = 1;
- main_thread = pthread_self();
- pthread_key_create(&async_key, NULL);
- pthread_key_create(&async_die_counter, NULL);
- set_die_routine(die_async);
- set_die_is_recursing_routine(async_die_is_recursing);
- }
+ setup_main_thread();
if (proc_in >= 0)
set_cloexec(proc_in);
diff --git a/thread-utils.c b/thread-utils.c
index a2135e0..b45ab92 100644
--- a/thread-utils.c
+++ b/thread-utils.c
@@ -1,5 +1,7 @@
#include "cache.h"
#include "thread-utils.h"
+#include "run-command.h"
+#include "git-compat-util.h"
#if defined(hpux) || defined(__hpux) || defined(_hpux)
# include
@@ -75,3 +77,228 @@ int init_recursive_mutex(pthread_mutex_t *m)
}
return ret;
}
+
+#ifndef NO_PTHREADS
+struct job_list {
+ int (*fct)(struct task_queue *tq, void *task);
+ void *task;
+ struct job_list *next;
+};
+
+static pthread_t main_thread;
+static int main_thread_set;
+static pthread_key_t async_key;
+static pthread_key_t async_die_counter;
+
+static NORETURN void die_async(const char *err, va_list params)
+{
+ vreportf("fatal: ", err, params);
+
+ if (!pthread_equal(main_thread, pthread_self()))
+ pthread_exit((void *)128);
+
+ exit(128);
+}
+
+static int async_die_is_recursing(void)
+{
+ void *ret = pthread_getspecific(async_die_counter);
+ pthread_setspecific(async_die_counter, (void *)1);
+ return ret != NULL;
+}
+
+/* FIXME: deduplicate this code with run-command.c */
+static void setup_main_thread(void)
+{
+ if (!main_thread_set) {
+ main_thread_set = 1;
+ main_thread = pthread_self();
+ pthread_key_create(&async_key, NULL);
+ pthread_key_create(&async_die_counter, NULL);
+ set_die_routine(die_async);
+ set_die_is_recursing_routine(async_die_is_recursing);
+ }
+}
+
+struct task_queue {
+ pthread_mutex_t mutex;
+ pthread_cond_t cond_non_empty;
+
+ int queued_tasks;
+ struct job_list *first;
+ struct job_list *last;
+
+ pthread_t *threads;
+ unsigned max_threads;
+ unsigned max_tasks;
+
+ void (*finish_function)(struct task_queue *tq);
+ int early_return;
+};
+
+static void next_task(struct task_queue *tq,
+ int (**fct)(struct task_queue *tq, void *task),
+ void **task,
+ int *early_return)
+{
+ struct job_list *job = NULL;
+
+ pthread_mutex_lock(&tq->mutex);
+ while (tq->queued_tasks == 0)
+ pthread_cond_wait(&tq->cond_non_empty, &tq->mutex);
+
+ tq->early_return |= *early_return;
+
+ if (!tq->early_return) {
+ job = tq->first;
+ tq->first = job->next;
+ if (!tq->first)
+ tq->last = NULL;
+ tq->queued_tasks--;
+ }
+
+ pthread_mutex_unlock(&tq->mutex);
+
+ if (job) {
+ *fct = job->fct;
+ *task = job->task;
+ } else {
+ *fct = NULL;
+ *task = NULL;
+ }
+
+ free(job);
+}
+
+static void *dispatcher(void *args)
+{
+ void *task;
+ int (*fct)(struct task_queue *tq, void *task);
+ int early_return = 0;
+ struct task_queue *tq = a
[PATCH 3/5] submodule: helper to run foreach in parallel
Similar to `git submodule foreach` the new command
`git submodule foreach_parallel` will run a command
on each submodule.
The commands are run in parallel up to the number of
cores by default, or you can specify '-j 4' tun just
run with 4 threads for example.
One major difference to `git submodule foreach` is the
handling of input and output to the commands. Because
of the parallel nature of the execution it is not trivial
how to schedule the std{in,out,err} channel for submodule
the command is run in. So in this patch there is no
support for stdin.
The goal of the output for std{out, err} is to look like
the single threaded version as much as possible, so
stdout and stderr from one submodule operation are
buffered together in one single channel and output
together when the output is allowed.
To do that, we'll have a mutex for the output, which
each thread will try to acquire and directly pipe their
output to the standard output if they are lucky to
get the mutex.
If they do not have the mutex each thread will buffer
their output.
Example:
Let's assume we have 5 submodules A,B,C,D,E and the
operation on each submodule takes a different amount
of time (say `git fetch`), then the output of
`git submodule foreach` might look like this:
time -->
output: |---A---| |-B-| |C---| |-D-| |-E-|
When we schedule these threads into two threads, a schedule
and sample output over time may look like this:
thread 1: |---A---| |-D-| |-E-|
thread 2: |-B-| |C---|
output: |---A---| B |C---| E D
So A will be perceived as it would run normally in
the single threaded version of foreach. As B has finished
by the time the mutex becomes available, the whole buffer
will just be dumped into the standard output. This will be
perceived by the user as just a 'very fast' operation of B.
Once that is done, C takes the mutex, and flushes the piled
up buffer to standard output. In case the subcommand is a
git command, we have a progress display, which will just
look like the first half of C happend really quickly.
Notice how E and D are put out in a different order than the
original as the new parallel foreach doesn't care about the
order.
So this way of output is really good for human consumption
and not for machine consumption as you always see the progress,
but it is not easy to tell which output comes from which
command as there is no indication other than displaying
"Entering " for each beginning section of
output.
Maybe we want to integrate the unthreaded foreach eventually
into the new code base in C and have special cases for that,
such as accepting stdin again.
Signed-off-by: Stefan Beller
---
builtin/submodule--helper.c | 160 +++-
git-submodule.sh| 11 ++-
2 files changed, 168 insertions(+), 3 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f11fb9c..2c06f28 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -8,8 +8,11 @@
#include "submodule.h"
#include "submodule-config.h"
#include "string-list.h"
+#include "thread-utils.h"
#include "run-command.h"
-
+#ifndef NO_PTHREADS
+#include
+#endif
static const struct cache_entry **ce_entries;
static int ce_alloc, ce_used;
static const char *alternative_path;
@@ -279,6 +282,155 @@ static int module_clone(int argc, const char **argv,
const char *prefix)
return 0;
}
+struct submodule_args {
+ const char *name;
+ const char *path;
+ const char *sha1;
+ const char *toplevel;
+ const char *prefix;
+ const char **cmd;
+ pthread_mutex_t *mutex;
+};
+
+int run_cmd_submodule(struct task_queue *aq, void *task)
+{
+ int i, lock_acquired = 0;
+ struct submodule_args *args = task;
+ struct strbuf out = STRBUF_INIT;
+ struct strbuf sb = STRBUF_INIT;
+ struct child_process *cp = xmalloc(sizeof(*cp));
+ char buf[1024];
+
+ strbuf_addf(&out, N_("Entering %s\n"), relative_path(args->path,
args->prefix, &sb));
+
+ child_process_init(cp);
+ argv_array_pushv(&cp->args, args->cmd);
+
+ argv_array_pushf(&cp->env_array, "name=%s", args->name);
+ argv_array_pushf(&cp->env_array, "path=%s", args->path);
+ argv_array_pushf(&cp->env_array, "sha1=%s", args->sha1);
+ argv_array_pushf(&cp->env_array, "toplevel=%s", args->toplevel);
+
+ for (i = 0; local_repo_env[i]; i++)
+ argv_array_push(&cp->env_array, local_repo_env[i]);
+
+ cp->no_stdin = 1;
+ cp->out = 0;
+ cp->err = -1;
+ cp->dir = args->path;
+ cp->stdout_to_stderr = 1;
+ cp->use_shell = 1;
+
+ if (start_command(cp)) {
+ die("Could not start command");
+ for (i = 0; cp->args.argv; i++)
+ fprintf(stderr, "%s\n", cp->args.argv[i]);
+ }
+
+ while (1) {
+ ssize_t len = xread(cp->err, buf, sizeof(buf));
+
[PATCH 4/5] index-pack: Use the new worker pool
This demonstrates how the new threading API may be used.
There is no change in the workflow, just using the new
threading API instead of keeping track of the pthreads
ourselves.
Signed-off-by: Stefan Beller
---
builtin/index-pack.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3f10840..187b281 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1075,7 +1075,7 @@ static void resolve_base(struct object_entry *obj)
}
#ifndef NO_PTHREADS
-static void *threaded_second_pass(void *data)
+static int threaded_second_pass(struct task_queue *tq, void *data)
{
set_thread_data(data);
for (;;) {
@@ -1096,7 +1096,7 @@ static void *threaded_second_pass(void *data)
resolve_base(&objects[i]);
}
- return NULL;
+ return 0;
}
#endif
@@ -1195,18 +1195,19 @@ static void resolve_deltas(void)
nr_ref_deltas + nr_ofs_deltas);
#ifndef NO_PTHREADS
- nr_dispatched = 0;
+
if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
+ struct task_queue *tq;
+ nr_dispatched = 0;
init_thread();
- for (i = 0; i < nr_threads; i++) {
- int ret = pthread_create(&thread_data[i].thread, NULL,
-threaded_second_pass,
thread_data + i);
- if (ret)
- die(_("unable to create thread: %s"),
- strerror(ret));
- }
+
+ tq = create_task_queue(nr_threads);
for (i = 0; i < nr_threads; i++)
- pthread_join(thread_data[i].thread, NULL);
+ add_task(tq, threaded_second_pass, thread_data + i);
+
+ if (finish_task_queue(tq, NULL))
+ die("Not all threads have finished");
+
cleanup_thread();
return;
}
--
2.5.0.264.g784836d
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] pack-objects: Use new worker pool
Before we had threads doing the delta finding work, and the main thread
was load balancing the threads, i.e. moving work from a thread with a large
amount left to an idle thread whenever such a situation arose.
This moves the load balancing to the threads themselves. As soon as one
thread is done working it will look at its peer threads and will pickup
half the work load from the thread with the largest pending load.
By having the load balancing as part of the threads, the locking and
communication model becomes easier, such that we don't need so many
mutexes any more.
It also demonstrates the usage of the new threading pool being easily
applied in different situations.
Signed-off-by: Stefan Beller
---
builtin/pack-objects.c | 175 -
1 file changed, 57 insertions(+), 118 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 62cc16d..f46d2df 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -17,6 +17,7 @@
#include "pack-objects.h"
#include "progress.h"
#include "refs.h"
+#include "run-command.h"
#include "streaming.h"
#include "thread-utils.h"
#include "pack-bitmap.h"
@@ -1887,26 +1888,12 @@ static void try_to_free_from_threads(size_t size)
static try_to_free_t old_try_to_free_routine;
-/*
- * The main thread waits on the condition that (at least) one of the workers
- * has stopped working (which is indicated in the .working member of
- * struct thread_params).
- * When a work thread has completed its work, it sets .working to 0 and
- * signals the main thread and waits on the condition that .data_ready
- * becomes 1.
- */
-
struct thread_params {
- pthread_t thread;
struct object_entry **list;
unsigned list_size;
unsigned remaining;
int window;
int depth;
- int working;
- int data_ready;
- pthread_mutex_t mutex;
- pthread_cond_t cond;
unsigned *processed;
};
@@ -1933,7 +1920,52 @@ static void cleanup_threaded_search(void)
pthread_mutex_destroy(&progress_mutex);
}
-static void *threaded_find_deltas(void *arg)
+static struct thread_params *p;
+
+static void threaded_split_largest_workload(struct thread_params *target)
+{
+ int i;
+
+ struct object_entry **list;
+ struct thread_params *victim = NULL;
+ unsigned sub_size = 0;
+
+ /* Find a victim */
+ progress_lock();
+ for (i = 0; i < delta_search_threads; i++)
+ if (p[i].remaining > 2*window &&
+ (!victim || victim->remaining < p[i].remaining))
+ victim = &p[i];
+
+ if (victim) {
+ sub_size = victim->remaining / 2;
+ list = victim->list + victim->list_size - sub_size;
+ while (sub_size && list[0]->hash &&
+ list[0]->hash == list[-1]->hash) {
+ list++;
+ sub_size--;
+ }
+ if (!sub_size) {
+ /*
+* It is possible for some "paths" to have
+* so many objects that no hash boundary
+* might be found. Let's just steal the
+* exact half in that case.
+*/
+ sub_size = victim->remaining / 2;
+ list -= sub_size;
+ }
+ victim->list_size -= sub_size;
+ victim->remaining -= sub_size;
+
+ target->list = list;
+ target->list_size = sub_size;
+ target->remaining = sub_size;
+ }
+ progress_unlock();
+}
+
+static int threaded_find_deltas(struct task_queue *tq, void *arg)
{
struct thread_params *me = arg;
@@ -1941,34 +1973,17 @@ static void *threaded_find_deltas(void *arg)
find_deltas(me->list, &me->remaining,
me->window, me->depth, me->processed);
- progress_lock();
- me->working = 0;
- pthread_cond_signal(&progress_cond);
- progress_unlock();
-
- /*
-* We must not set ->data_ready before we wait on the
-* condition because the main thread may have set it to 1
-* before we get here. In order to be sure that new
-* work is available if we see 1 in ->data_ready, it
-* was initialized to 0 before this thread was spawned
-* and we reset it to 0 right away.
-*/
- pthread_mutex_lock(&me->mutex);
- while (!me->data_ready)
- pthread_cond_wait(&me->cond, &me->mutex);
- me->data_ready = 0;
- pthread_mutex_unlock(&me->mutex);
+ threaded_split_largest_workload(me);
}
- /* leave ->working 1 so that this doesn't get more work assigned */
-
[PATCH 1/5] submodule: implement `module_clone` as a builtin helper
`module_clone` is part of the update command, which I want to convert
to C next.
Signed-off-by: Stefan Beller
Signed-off-by: Junio C Hamano
---
builtin/submodule--helper.c | 160 +++-
git-submodule.sh| 80 +-
2 files changed, 162 insertions(+), 78 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4b32a3c..f11fb9c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -8,6 +8,7 @@
#include "submodule.h"
#include "submodule-config.h"
#include "string-list.h"
+#include "run-command.h"
static const struct cache_entry **ce_entries;
static int ce_alloc, ce_used;
@@ -124,6 +125,160 @@ static int module_name(int argc, const char **argv, const
char *prefix)
return 0;
}
+static int clone_submodule(const char *path, const char *gitdir, const char
*url,
+ const char *depth, const char *reference, int quiet)
+{
+ struct child_process cp;
+ child_process_init(&cp);
+
+ argv_array_push(&cp.args, "clone");
+ argv_array_push(&cp.args, "--no-checkout");
+ if (quiet)
+ argv_array_push(&cp.args, "--quiet");
+ if (depth && strcmp(depth, "")) {
+ argv_array_push(&cp.args, "--depth");
+ argv_array_push(&cp.args, depth);
+ }
+ if (reference && strcmp(reference, "")) {
+ argv_array_push(&cp.args, "--reference");
+ argv_array_push(&cp.args, reference);
+ }
+ if (gitdir) {
+ argv_array_push(&cp.args, "--separate-git-dir");
+ argv_array_push(&cp.args, gitdir);
+ }
+ argv_array_push(&cp.args, url);
+ argv_array_push(&cp.args, path);
+
+ cp.git_cmd = 1;
+ cp.env = local_repo_env;
+
+ cp.no_stdin = 1;
+ cp.no_stdout = 1;
+ cp.no_stderr = 1;
+
+ return run_command(&cp);
+}
+
+/*
+ * Clone a submodule
+ *
+ * $1 = submodule path
+ * $2 = submodule name
+ * $3 = URL to clone
+ * $4 = reference repository to reuse (empty for independent)
+ * $5 = depth argument for shallow clones (empty for deep)
+ *
+ * Prior to calling, cmd_update checks that a possibly existing
+ * path is not a git repository.
+ * Likewise, cmd_add checks that path does not exist at all,
+ * since it is the location of a new submodule.
+ */
+static int module_clone(int argc, const char **argv, const char *prefix)
+{
+ const char *path = NULL, *name = NULL, *url = NULL, *reference = NULL,
*depth = NULL;
+ int quiet = 0;
+ FILE *submodule_dot_git;
+ const char *sm_gitdir, *p;
+ struct strbuf rel_path = STRBUF_INIT;
+ struct strbuf sb = STRBUF_INIT;
+
+ struct option module_update_options[] = {
+ OPT_STRING(0, "prefix", &alternative_path,
+ N_("path"),
+ N_("alternative anchor for relative paths")),
+ OPT_STRING(0, "path", &path,
+ N_("path"),
+ N_("where the new submodule will be cloned to")),
+ OPT_STRING(0, "name", &name,
+ N_("string"),
+ N_("name of the new submodule")),
+ OPT_STRING(0, "url", &url,
+ N_("string"),
+ N_("url where to clone the submodule from")),
+ OPT_STRING(0, "reference", &reference,
+ N_("string"),
+ N_("reference repository")),
+ OPT_STRING(0, "depth", &depth,
+ N_("string"),
+ N_("depth for shallow clones")),
+ OPT_END()
+ };
+
+ static const char * const git_submodule_helper_usage[] = {
+ N_("git submodule--helper update [--prefix=] [--quiet]
[--remote] [-N|--no-fetch]"
+ "[-f|--force] [--rebase|--merge] [--reference ]"
+ "[--depth ] [--recursive] [--] [...]"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, module_update_options,
+git_submodule_helper_usage, 0);
+
+ if (getenv("GIT_QUIET"))
+ quiet = 1;
+
+ strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
+ sm_gitdir = strbuf_detach(&sb, NULL);
+
+ if (!file_exists(sm_gitdir)) {
+ safe_create_leading_directories_const(sm_gitdir);
+ if (clone_submodule(path, sm_gitdir, url, depth, reference,
quiet))
+ die(N_("Clone of '%s' into submodule path '%s' failed"),
+ url, path);
+ } else {
+ safe_create_leading_directories_const(path);
+ unlink(sm_gitdir);
+ }
+
+ /* Write a .git file in the submodule to redirect to the superproject.
*/
+ if (alternative_path && !strcmp(alternative_path,
[RFC PATCH 0/5] Progressing with `git submodule foreach_parallel`
This series applies on top of 10d0bef9981c8045b8c9aac1ca7b8761896bed0c in the branch origin/sb/submodule-helper The first patch is a resend of module_clone, which now contains all memory leak fixes. The second patch adding the new task queue is rewritten to not use semaphores any more, but only mutexes and condition variables as that's what we use at other places in the code base. So it is better to align to these places, it makes the threading pool also easier to understand internally. Originally there were 2 semaphores to control the queue length (0 < queue length < number of threads), such that we don't overallocate memory, but rather slow down the thread adding new tasks to the queue. This has been removed and the queue may grow infinitely large. The third patch adding `git submodule foreach_parallel` learned a totally different behavior how to interact with the output channels (way better for human consumption as you have early feedback and feedback all the time, ok-ish for machine consumption, but far from perfect). The fourth patch rewriting index-pack was made smaller and doesn't rewrite the workflow any more, but only plugs in the new threading API instead of constructing the pthreads themselves. I started benchmarking that patch, but I seem to get only bogus results, so I cannot tell if it still makes it 2 % slower as Jeff claimed. The fifth patch refactors pack-objects to be *way* more understandable IMHO! It also switches to the new threading pool, but the refactor may be suitable also if we decide to not use the threading API there. The reason for it being more readable is the movement of the rebalancing into the threads themselves, which then turns out to require a lot less of locking and communication variables. Any feedback is welcome! Thanks, Stefan Stefan Beller (5): submodule: implement `module_clone` as a builtin helper thread-utils: add a threaded task queue submodule: helper to run foreach in parallel index-pack: Use the new worker pool pack-objects: Use new worker pool builtin/index-pack.c| 23 ++-- builtin/pack-objects.c | 175 builtin/submodule--helper.c | 318 +++- git-submodule.sh| 91 ++--- run-command.c | 29 ++-- thread-utils.c | 227 +++ thread-utils.h | 35 + 7 files changed, 677 insertions(+), 221 deletions(-) -- 2.5.0.264.g784836d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git's inconsistent command line options
On Wed, Aug 26, 2015 at 4:02 PM, Jacob Keller wrote: > On Wed, Aug 26, 2015 at 3:52 PM, Philip Oakley wrote: >> From: "Jacob Keller" >>> >>> On Wed, Aug 26, 2015 at 10:56 AM, Junio C Hamano >>> wrote: But notice that I said "if you really want to". I personally think it is a road to madness. >>> >>> >>> Agreed. I don't believe in command line API here. I think we'd need a >>> better solution. >>> >>> My gut says: Live with the warts on old commands and try to make >>> people use command words for new commands. >>> -- >> >> Agreed. However Graeme's original question also said "I can supply a more >> extensive list if needed", and "I still have to reference the help to remind >> me of what parameter to use in certain situation", which suggests that one >> option is to capture that list within some part of the documentation, >> especially if Graeme already has an easy to read layout. >> >> If it could be part of the documenation, where should it go - gitcli or >> perhaps it's own guide (`git help -g` and friends)? >> > > I hope to spend some time investigating this in the near future. > Personally I feel that command names make more sense than options > since they are essentially non-options, and it helps separate > logically related but non-equivalent operations more easily. > > The confusion that results from git-checkout, and git-branch has > caught more than a few new people at $dayjob. > > Regards, > Jake That being said, use of "--create" or so forth really isn't that bothersome so maybe the gains outway the cons here. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git's inconsistent command line options
On Wed, Aug 26, 2015 at 3:52 PM, Philip Oakley wrote: > From: "Jacob Keller" >> >> On Wed, Aug 26, 2015 at 10:56 AM, Junio C Hamano >> wrote: >>> >>> But notice that I said "if you really want to". I personally think >>> it is a road to madness. >> >> >> Agreed. I don't believe in command line API here. I think we'd need a >> better solution. >> >> My gut says: Live with the warts on old commands and try to make >> people use command words for new commands. >> -- > > Agreed. However Graeme's original question also said "I can supply a more > extensive list if needed", and "I still have to reference the help to remind > me of what parameter to use in certain situation", which suggests that one > option is to capture that list within some part of the documentation, > especially if Graeme already has an easy to read layout. > > If it could be part of the documenation, where should it go - gitcli or > perhaps it's own guide (`git help -g` and friends)? > I hope to spend some time investigating this in the near future. Personally I feel that command names make more sense than options since they are essentially non-options, and it helps separate logically related but non-equivalent operations more easily. The confusion that results from git-checkout, and git-branch has caught more than a few new people at $dayjob. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git's inconsistent command line options
From: "Jacob Keller" On Wed, Aug 26, 2015 at 10:56 AM, Junio C Hamano wrote: But notice that I said "if you really want to". I personally think it is a road to madness. Agreed. I don't believe in command line API here. I think we'd need a better solution. My gut says: Live with the warts on old commands and try to make people use command words for new commands. -- Agreed. However Graeme's original question also said "I can supply a more extensive list if needed", and "I still have to reference the help to remind me of what parameter to use in certain situation", which suggests that one option is to capture that list within some part of the documentation, especially if Graeme already has an easy to read layout. If it could be part of the documenation, where should it go - gitcli or perhaps it's own guide (`git help -g` and friends)? -- Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-send-email: Delete additional From message body
brillian...@inbox.ru writes:
> Additional From added to message body if git-send-email run
> with --from parameters
>
> Signed-off-by: Brilliantov Kirill Vladimirovich
> ---
> git-send-email.perl | 25 +++--
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index b660cc2..92ec74a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1522,24 +1522,21 @@ foreach my $t (@files) {
> $subject = quote_subject($subject, $auto_8bit_encoding);
> }
>
> - if (defined $sauthor and $sauthor ne $sender) {
> - $message = "From: $author\n\n$message";
This condition attempts to say "Look at the author taken from the
From: line of the message, and if that is different from the sender
(which will be recorded in the header of the resulting e-mail), then
we do need to add an additional in-body From: line to record the
author of the commit, who may be different from the name that would
appear in the e-mail."
In other words, the "Additional From" is very much deliberate when
the "--from" option is used and is different from the patch author.
And I do not see this "add in-body header if the author and the
sender are different" you are removing from here appear anywhere in
the added text of your patch. So one consequence of this patch is
to _never_ use the in-body From: line.
That needs a serious justification.
Now, it is entirely possible that what you wanted to do (and what
you are doing) in this patch may be different from "unconditionally,
remove the in-body From: line". Perhaps the code mistakenly
declares that the sender given via "--from" and the author are
different, possibly due to a bug in rfc2047 unquoting code or
something, and you are only trying to make sure that the code
notices they are really the same. But I somehow have a feeling that
that is not what is going on, because in such a patch, there still
will be some code that adds "From: $author" at the very beginning of
outgoing message body; such a fix might update the "if ()" condition
used to guard it (e.g. $sauthor and $sender may have to get further
cleansed before comparing), but I do not see anything of that sort,
either. Also all the changes below do not have anything to do with
the additional in-body "From: " line.
So, I dunno. I cannot tell what you are trying to do, and the only
immediate effect of the change I can read is that in-body "From:"
line is unconditionally disabled, which is certainly not something
we would want to do.
Puzzled...
> - if (defined $author_encoding) {
> - if ($has_content_type) {
> - if ($body_encoding eq $author_encoding) {
> - # ok, we already have the right encoding
> - }
> - else {
> - # uh oh, we should re-encode
> - }
> + if (defined $author_encoding) {
> + if ($has_content_type) {
> + if ($body_encoding eq $author_encoding) {
> + # ok, we already have the right encoding
> }
> else {
> - $xfer_encoding = '8bit' if not defined
> $xfer_encoding;
> - $has_content_type = 1;
> - push @xh,
> - "Content-Type: text/plain;
> charset=$author_encoding";
> + # uh oh, we should re-encode
> }
> }
> + else {
> + $xfer_encoding = '8bit' if not defined $xfer_encoding;
> + $has_content_type = 1;
> + push @xh,
> + "Content-Type: text/plain; charset=$author_encoding";
> + }
> }
> if (defined $target_xfer_encoding) {
> $xfer_encoding = '8bit' if not defined $xfer_encoding;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] path: optimize common dir checking
On Wed, 2015-08-26 at 18:10 -0400, David Turner wrote: > On Wed, 2015-08-26 at 14:15 -0700, Junio C Hamano wrote: > > > + * For example, consider the following set of strings: > > > + * abc > > > + * def > > > + * definite > > > + * definition > > > + * > > > + * The trie would look look like: > > > + * root: len = 0, value = (something), children a and d non-NULL. > > > > "value = NULL", as there is no empty string registered in the trie? > > Indeed. > > > > + *a: len = 2, contents = bc > > > > "value = NULL" here, too (just showing I am following along, not > > just skimming)? > > Yep. No, wait. value should be non-NULL, since abc is in the string set. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] path: optimize common dir checking
David Turner writes: > On Wed, 2015-08-26 at 14:15 -0700, Junio C Hamano wrote: > >> Thanks for a pleasant read. > > Thank you! I'll re-roll with those last two fixes (re value=NULL) > tomorrow-ish. If these two "value = NULL" are the only things, I can locally fix them up. No need to resend. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] path: optimize common dir checking
On Wed, 2015-08-26 at 14:15 -0700, Junio C Hamano wrote: > > + * For example, consider the following set of strings: > > + * abc > > + * def > > + * definite > > + * definition > > + * > > + * The trie would look look like: > > + * root: len = 0, value = (something), children a and d non-NULL. > > "value = NULL", as there is no empty string registered in the trie? Indeed. > > + *a: len = 2, contents = bc > > "value = NULL" here, too (just showing I am following along, not > just skimming)? Yep. > > + /* Partial path normalization: skip consecutive slashes */ > > + while (key[0] == '/' && key[1] == '/') > > + key ++; > > These "skipping consecutive slashes" inside this function is cute. > I would have expected me to react "Eek, Ugly" to it, but somehow I > didn't find them ugly. It's right on the cute/ugly edge for me too. > I somehow wonder if we'd want to precomute the trie at the build > time, though ;-) LOL. > Thanks for a pleasant read. Thank you! I'll re-roll with those last two fixes (re value=NULL) tomorrow-ish. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] git-p4: Obey core.ignorecase when using P4 client specs.
Lars Schneider writes: > On 26 Aug 2015, at 22:05, Junio C Hamano wrote: > >> larsxschnei...@gmail.com writes: >> >>> From: Lars Schneider >>> >>> We run P4 servers on Linux and P4 clients on Windows. For an unknown >>> reason the file path for a number of files in P4 does not match the >>> directory path with respect to case sensitivity. >> >> Thanks, but is this still "For an unknown reason", or during the >> course of debugging you found the root cause, which is what led to >> this fix? > We are migrating away from P4 and therefore I haven’t debugged the > root cause. The source of this problem is 100% P4 related. No Git > involvement at all. Maybe I just remove this paragraph? Perhaps. If it is irrelevant how the P4 depot ended up recording the paths in mixed cases, then we can just say that as a prerequiste condition to trigger the problem. Perhaps rephrase the entire thing like this? Perforce depot may record paths in mixed cases, e.g. "p4 files" may show that there are these two paths: //depot/path/to/file1 //depot/pATH/to/file2 and with "p4" or "p4v", these end up in the same directory, e.g. //depot/path/to/file1 //depot/path/to/file2 which is the desired outcome on case insensitive systems. If git-p4 is used with client spec "//depot/path/...", however, then all files not matching the case in the client spec are ignored (in the example above "//depot/pATH/to/file2"). Fix this by downcasing the depot_path when core.ignorcase is set to true. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] git-p4: Obey core.ignorecase when using P4 client specs.
On 26 Aug 2015, at 22:05, Junio C Hamano wrote: > larsxschnei...@gmail.com writes: > >> From: Lars Schneider >> >> We run P4 servers on Linux and P4 clients on Windows. For an unknown >> reason the file path for a number of files in P4 does not match the >> directory path with respect to case sensitivity. > > Thanks, but is this still "For an unknown reason", or during the > course of debugging you found the root cause, which is what led to > this fix? We are migrating away from P4 and therefore I haven’t debugged the root cause. The source of this problem is 100% P4 related. No Git involvement at all. Maybe I just remove this paragraph? > >> >> E.g. "p4 files" might return >> //depot/path/to/file1 >> //depot/pATH/to/file2 >> >> If you use P4/P4V then these files end up in the same directory, e.g. >> //depot/path/to/file1 >> //depot/path/to/file2 >> >> If you use git-p4 and clone the code via client spec "//depot/path/..." >> then all files not matching the case in the client spec will be ignored >> (in the example above "file2"). This is correct if core.ignorecase=false >> but not otherwise. > > This sentence is hard to grok. What are you describing? Solution? > Current problematic behaviour? Desired behaviour that the patch > attempts to obtain? I tried to describe the situation that causes the strange behavior. > > If I paraphrase it like this, did I understood you correctly? > >The current code always ignores paths in wrong case that do not >match client spec. It is correct to ignore them when >core.ignorecase is not set; //depot/path/ and //depot/pATH/ are >different things in that case. > >But it is a wrong thing to do if core.ignorecase is set to true. >Let's make sure we avoid it by downcasing the depot_path when >core.ignorecase is set to true. How about this: - The current code always ignores files with paths that do not match the case of the paths defined in the client spec. This commit changes this behavior and obeys these files if “core.ignorecase” is set to "true”. Example: A P4 repository contains the following files: //depot/path/to/file1 //depot/pATH/to/file2 The P4 repository is cloned with git-p4 and the following client spec view: //depot/path/… //client/... The cloned Git repository will contain only the following file: to/file1 “file2” is not present in the cloned Git repository. If “core.ignorecase" is set to “true” then path case sensitivity is ignored and “file2” will be present. Thanks, Lars -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/4] path: optimize common dir checking
David Turner writes:
> Instead of a linear search over common_list to check whether
> a path is common, use a trie. The trie search operates on
> path prefixes, and handles excludes.
>
> Signed-off-by: David Turner
> ---
> path.c| 226
> ++
> t/t0060-path-utils.sh | 1 +
> 2 files changed, 213 insertions(+), 14 deletions(-)
>
> diff --git a/path.c b/path.c
> index d24bfa2..c7a4c40 100644
> --- a/path.c
> +++ b/path.c
> @@ -121,25 +121,223 @@ struct common_dir common_list[] = {
> { 0, 0, 0, NULL }
> };
>
> -static void update_common_dir(struct strbuf *buf, int git_dir_len)
> +/*
> + * A compressed trie. A trie node consists of zero or more characters that
> + * are common to all elements with this prefix, optionally followed by some
> + * children. If value is not NULL, the trie node is a terminal node.
> + *
> + * For example, consider the following set of strings:
> + * abc
> + * def
> + * definite
> + * definition
> + *
> + * The trie would look look like:
> + * root: len = 0, value = (something), children a and d non-NULL.
"value = NULL", as there is no empty string registered in the trie?
> + *a: len = 2, contents = bc
"value = NULL" here, too (just showing I am following along, not
just skimming)?
> + *d: len = 2, contents = ef, children i non-NULL, value = (something)
> + * i: len = 3, contents = nit, children e and i non-NULL, value = NULL
> + * e: len = 0, children all NULL, value = (something)
> + * i: len = 2, contents = on, children all NULL, value =
> (something)
> + */
> +struct trie {
> + struct trie *children[256];
> + int len;
> + char *contents;
> + void *value;
> +};
> +
> +static struct trie *make_trie_node(const char *key, void *value)
> {
> - char *base = buf->buf + git_dir_len;
> - const struct common_dir *p;
> + struct trie *new_node = xcalloc(1, sizeof(*new_node));
> + new_node->len = strlen(key);
> + if (new_node->len) {
> + new_node->contents = xmalloc(new_node->len);
> + memcpy(new_node->contents, key, new_node->len);
> + }
> + new_node->value = value;
> + return new_node;
> +}
>
> - if (is_dir_file(base, "logs", "HEAD") ||
> - is_dir_file(base, "info", "sparse-checkout"))
> - return; /* keep this in $GIT_DIR */
> - for (p = common_list; p->dirname; p++) {
> - const char *path = p->dirname;
> - if (p->is_dir && dir_prefix(base, path)) {
> - replace_dir(buf, git_dir_len, get_git_common_dir());
> - return;
> +/*
> + * Add a key/value pair to a trie. The key is assumed to be \0-terminated.
> + * If there was an existing value for this key, return it.
> + */
> +static void *add_to_trie(struct trie *root, const char *key, void *value)
> +{
> + struct trie *child;
> + void *old;
> + int i;
> +
> + if (!*key) {
> + /* we have reached the end of the key */
> + old = root->value;
> + root->value = value;
> + return old;
> + }
> +
> + for (i = 0; i < root->len; ++i) {
> + if (root->contents[i] == key[i])
> + continue;
> +
> + /*
> + * Split this node: child will contain this node's
> + * existing children.
> + */
> + child = malloc(sizeof(*child));
> + memcpy(child->children, root->children, sizeof(root->children));
> +
> + child->len = root->len - i - 1;
> + if (child->len) {
> + child->contents = strndup(root->contents + i + 1,
> +child->len);
> }
> - if (!p->is_dir && !strcmp(base, path)) {
> - replace_dir(buf, git_dir_len, get_git_common_dir());
> - return;
> + child->value = root->value;
> + root->value = NULL;
> + root->len = i;
> +
> + memset(root->children, 0, sizeof(root->children));
> + root->children[(unsigned char)root->contents[i]] = child;
> +
> + /* This is the newly-added child. */
> + root->children[(unsigned char)key[i]] =
> + make_trie_node(key + i + 1, value);
> + return NULL;
> + }
> +
> + /* We have matched the entire compressed section */
> + if (key[i]) {
> + child = root->children[(unsigned char)key[root->len]];
> + if (child) {
> + return add_to_trie(child, key + root->len + 1, value);
> + } else {
> + child = make_trie_node(key + root->len + 1, value);
> + root->children[(unsigned char)key[root->len]] = child;
> + return NULL;
> }
> }
> +
> + old = root->value;
> + root->value = va
[PATCH] tag: teach -n to override tag.sort configuration
From: Jacob Keller
git tag -n will always fail if tag.sort is set to a sort other than the
default "refname" setting. Teach git tag to have -n override the
configuration setting when -n is used, instead of failing with
fatal: --sort and -n incompatible
since the user probably sets tag.sort once as a default and would expect
-n to override this setting.
As a consequence, -n and --sort will always be incompatible now, even if
the --sort is "--sort=refname" which is essentially a no-op and
previously was acceptable. No sane user should be doing such.
Add a test to document that --sort and -n are incompatible. Add a
further test to show that tag.sort should be ignored if -n is
configured.
Signed-off-by: Jacob Keller
---
I wasn't sure who to Cc for this, since I wrote the tag.sort code.
Noticed this when I tried to use -n option, and it kept failing. Maybe
this isn't the best way to implement this. I could replace how we read
tag.sort and grab it using git_config_get_string or something after we
parse the command line options. But this seemed like the easiest way.
builtin/tag.c | 11 +--
t/t7004-tag.sh | 15 +++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 471d6b1ab884..83f94bd7f833 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -33,6 +33,7 @@ static const char * const git_tag_usage[] = {
#define REVERSE_SORT0x8000
static int tag_sort;
+static int cmd_line_sort = 0;
struct tag_filter {
const char **patterns;
@@ -559,6 +560,8 @@ static int parse_opt_sort(const struct option *opt, const
char *arg, int unset)
{
int *sort = opt->value;
+ cmd_line_sort = 1;
+
return parse_sort_string(NULL, arg, sort);
}
@@ -647,8 +650,12 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
copts.padding = 2;
run_column_filter(colopts, &copts);
}
- if (lines != -1 && tag_sort)
- die(_("--sort and -n are incompatible"));
+ if (lines != -1) {
+ if (cmd_line_sort)
+ die(_("--sort and -n are incompatible"));
+ else
+ tag_sort = 0;
+ }
ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit,
tag_sort);
if (column_active(colopts))
stop_column_filter();
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d31788cc6ce6..92a8f1b4670c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1392,6 +1392,21 @@ test_expect_success 'multiple --points-at are OR-ed
together' '
test_cmp expect actual
'
+test_expect_success 'listing tag with -n --sort= should fail' '
+ test_must_fail git tag --sort=refname -n
+'
+
+test_expect_success 'listing tag with -n and tag.sort must not fail' '
+ echo "tag-one-lineA msg" >expect &&
+ git -c tag.sort="v:refname" tag -n1 -l | grep "^tag-one-line" >actual &&
+ test_cmp expect actual &&
+ echo "tag-lines tag line one" >expect &&
+ echo "tag line two" >>expect &&
+ echo "tag line three" >>expect &&
+ git -c tag.sort="v:refname" tag -n3 -l | grep "^ *tag.line" >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'lexical sort' '
git tag foo1.3 &&
git tag foo1.6 &&
--
2.5.0.280.g4aaba03
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git's inconsistent command line options
Jacob Keller writes: > On Wed, Aug 26, 2015 at 10:56 AM, Junio C Hamano wrote: >> But notice that I said "if you really want to". I personally think >> it is a road to madness. > > Agreed. I don't believe in command line API here. I think we'd need a > better solution. > > My gut says: Live with the warts on old commands and try to make > people use command words for new commands. A transition to make everybody to use subsubcommands (thereby changing what "git tag delete master" means) is impossible in practice. On the other hand, a transition to make everybody use command mode options (thereby allowing "git worktree list" to be also spelled as "git worktree --list") _is_ possible. Has anybody created a handy catalog of Git commands with subcommands and command mode options? If we see such a list and replace the column of subcommands with command mode options, we might find that such a "command mode option only" world a pleasant future for us to live in, or an unpleasant one that we have to keep typing two extra dashes all the time. I cannot tell offhand. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git clone svn: authors from authors file are ignored, authors-prog works, but crashes on branch points
Hi, i am observing some weired "git svn clone" behavior during my try to migrate the Scaffold Hunter [1] SVN repository [2] to Git: if i just use the command $ git svn clone svn://svn.code.sf.net/p/scaffoldhunter/code --no-metadata -s code everything went smoothly. Now i wanted to replace the authors SVN-logins by useful names and created a mapping file with the following content: anjenson = Andrew Zhilka <> bernhard.dick = Bernhard Dick <> dominic.sacre = Dominic Sacré <> doxmoxbox = doxmoxbox <> falkn = Falk Nette <> henning.garus = Henning Garus <> kakl = Karsten Klein <> klein = Karsten Klein <> lappie00 = Jeroen Lappenschaar <> michael.hesse = Michael Hesse <> nlskrg = Nils Kriege philipp.kopp = Philipp Kopp <> philipp.lewe = Philipp Lewe <> schrins = Sven Schrinner <> shamshadnpti = Shamshad Alam <> srenner7 = Steffen Renner <> sturm89 = Werner Sturm <> thomas.schmitz = Thomas Schmitz <> thorsten.fluegel = Thorsten Flügel <> till.schaefer = Till Schäfer tillschaefer = Till Schäfer I executed: $ git svn clone svn://svn.code.sf.net/p/scaffoldhunter/code --authors-file=/home/till/temp/code-authors-transform.txt --no-metadata -s code and got the error message "Author: klein not defined in /home/till/temp/code-authors-transform.txt file". I tried a workaround by using a python script to return the entries i have already defined in the authors file: $ git svn clone svn://svn.code.sf.net/p/scaffoldhunter/code --authors-file=/home/till/temp/code-authors-transform.txt --authors-prog=/home/till/temp/authors.py --no-metadata -s code The import process now went over the "klein" commit. The weired thing is, that a few SVN usernames seem to be recognized in the authors file and a few are passed to my script (I logged the output). However, at the first revision, where a tag was added in the SVN repo the cloning process crashed with the error message: Found possible branch point: svn://svn.code.sf.net/p/scaffoldhunter/code/trunk => svn://svn.code.sf.net/p/scaffoldhunter/code/branches/subsearch, 17 Use of uninitialized value $u in substitution (s///) at /usr/lib64/perl5/vendor_perl/5.20.2/Git/SVN.pm line 101. Use of uninitialized value $u in concatenation (.) or string at /usr/lib64/perl5/vendor_perl/5.20.2/Git/SVN.pm line 101. refs/remotes/origin/trunk: 'svn://svn.code.sf.net/p/scaffoldhunter/code' not found in '' One problem I am seeing in the SVN repo is, that between the revisions 97 and 102 the trunk folder was absent. the old one was moved to some subfolder and a new one was created. Therefore, I started the cloning beginning with rev 102 using the command line option "-r102:HEAD". However, the same error occurred for some later tag (the first tag after rev 102). Found possible branch point: svn://svn.code.sf.net/p/scaffoldhunter/code/trunk => svn://svn.code.sf.net/p/scaffoldhunter/code/tags/release-2.0, 1565 Use of uninitialized value $u in substitution (s///) at /usr/lib64/perl5/vendor_perl/5.20.2/Git/SVN.pm line 101. Use of uninitialized value $u in concatenation (.) or string at /usr/lib64/perl5/vendor_perl/5.20.2/Git/SVN.pm line 101. refs/remotes/origin/trunk: 'svn://svn.code.sf.net/p/scaffoldhunter/code' not found in '' I would be very glad if someone has a hint about what is going wrong here. Is this a bug in git or is something wrong with the SVN repo? used software versions: - git 2.5.0 - subversion 1.8.14 Regards, Till [1] http://scaffoldhunter.sourceforge.net/ [2] https://svn.code.sf.net/p/scaffoldhunter/code/trunk -- Dipl.-Inf. Till Schäfer TU Dortmund University Chair 11 - Algorithm Engineering Otto-Hahn-Str. 14 / Room 237 44227 Dortmund, Germany e-mail: till.schae...@cs.tu-dortmund.de phone: +49(231)755-7706 fax: +49(231)755-7740 web: http://ls11-www.cs.uni-dortmund.de/staff/schaefer pgp: https://keyserver2.pgp.com/vkd/SubmitSearch.event?&&SearchCriteria=0xD84DED79 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak writes:
> On Wed, Aug 26, 2015 at 8:07 PM, Junio C Hamano wrote:
>>
>> ... You can give a new format_ref_array_item()
>> that does not print but fills a strbuf to this caller, make
>> show_ref_array_item() a thin wrapper that calls it and prints it
>> with the final LF for other callers.
>>
> You're saying remove show_ref_array_item() (even the wrapper you mentioned
> above) and just have something like format_ref_array_item() which
> would output to a strbuf. and let the caller worry about the printing?
Among the current callers, the one in builtin/tag.c that wants to
trigger show_tag_lines() hack embedded in show_ref_array_item()
function can stop calling show_ref_array_item() and instead can do
for (i = 0; i < array.nr; i++) {
struct strbuf out = STRBUF_INIT;
format_ref_array_item(&out, ...);
if (filter->lines) {
... append tag lines to out ...
}
printf("%s\n", out.buf);
strbuf_reset(&out);
}
The current and future callers of show_ref_array_item() that do not
want to trigger the show_tag_liens() hack embedded in there may
still want it to print the formatted string including the trailing
LF, so you can keep show_ref_array_item() as a thin wrapper around
format_ref_array_item() for them to call, e.g.
show_ref_array_item(...) {
struct strbuf out = STRBUF_INIT;
format_ref_array_item(&out, ...);
printf("%s\n", out.buf);
strbuf_release(&out);
}
But if it has only one caller each, you may not even want to have
show_ref_array_item(), if you are going to do the "output to strbuf"
variant.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6] git-p4: Obey core.ignorecase when using P4 client specs.
larsxschnei...@gmail.com writes:
> From: Lars Schneider
>
> We run P4 servers on Linux and P4 clients on Windows. For an unknown
> reason the file path for a number of files in P4 does not match the
> directory path with respect to case sensitivity.
Thanks, but is this still "For an unknown reason", or during the
course of debugging you found the root cause, which is what led to
this fix?
>
> E.g. "p4 files" might return
> //depot/path/to/file1
> //depot/pATH/to/file2
>
> If you use P4/P4V then these files end up in the same directory, e.g.
> //depot/path/to/file1
> //depot/path/to/file2
>
> If you use git-p4 and clone the code via client spec "//depot/path/..."
> then all files not matching the case in the client spec will be ignored
> (in the example above "file2"). This is correct if core.ignorecase=false
> but not otherwise.
This sentence is hard to grok. What are you describing? Solution?
Current problematic behaviour? Desired behaviour that the patch
attempts to obtain?
If I paraphrase it like this, did I understood you correctly?
The current code always ignores paths in wrong case that do not
match client spec. It is correct to ignore them when
core.ignorecase is not set; //depot/path/ and //depot/pATH/ are
different things in that case.
But it is a wrong thing to do if core.ignorecase is set to true.
Let's make sure we avoid it by downcasing the depot_path when
core.ignorecase is set to true.
>
> Signed-off-by: Lars Schneider
> Acked-by: Luke Diamand
> ---
> git-p4.py | 7 ++
> t/t9821-git-p4-path-variations.sh | 200
> ++
> 2 files changed, 207 insertions(+)
> create mode 100755 t/t9821-git-p4-path-variations.sh
>
> diff --git a/git-p4.py b/git-p4.py
> index 073f87b..0093fa3 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1950,10 +1950,14 @@ class View(object):
> if "unmap" in res:
> # it will list all of them, but only one not unmap-ped
> continue
> +if gitConfigBool("core.ignorecase"):
> +res['depotFile'] = res['depotFile'].lower()
> self.client_spec_path_cache[res['depotFile']] =
> self.convert_client_path(res["clientFile"])
>
> # not found files or unmap files set to ""
> for depotFile in fileArgs:
> +if gitConfigBool("core.ignorecase"):
> +depotFile = depotFile.lower()
> if depotFile not in self.client_spec_path_cache:
> self.client_spec_path_cache[depotFile] = ""
>
> @@ -1962,6 +1966,9 @@ class View(object):
> depot file should live. Returns "" if the file should
> not be mapped in the client."""
>
> +if gitConfigBool("core.ignorecase"):
> +depot_path = depot_path.lower()
> +
> if depot_path in self.client_spec_path_cache:
> return self.client_spec_path_cache[depot_path]
>
> diff --git a/t/t9821-git-p4-path-variations.sh
> b/t/t9821-git-p4-path-variations.sh
> new file mode 100755
> index 000..599d16c
> --- /dev/null
> +++ b/t/t9821-git-p4-path-variations.sh
> @@ -0,0 +1,200 @@
> +#!/bin/sh
> +
> +test_description='Clone repositories with path case variations'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d with case folding enabled' '
> + start_p4d -C1
> +'
> +
> +test_expect_success 'Create a repo with path case variations' '
> + client_view "//depot/... //client/..." &&
> + (
> + cd "$cli" &&
> +
> + mkdir -p One/two &&
> + >One/two/File2.txt &&
> + p4 add One/two/File2.txt &&
> + p4 submit -d "Add file2" &&
> + rm -rf One &&
> +
> + mkdir -p one/TWO &&
> + >one/TWO/file1.txt &&
> + p4 add one/TWO/file1.txt &&
> + p4 submit -d "Add file1" &&
> + rm -rf one &&
> +
> + mkdir -p one/two &&
> + >one/two/file3.txt &&
> + p4 add one/two/file3.txt &&
> + p4 submit -d "Add file3" &&
> + rm -rf one &&
> +
> + mkdir -p outside-spec &&
> + >outside-spec/file4.txt &&
> + p4 add outside-spec/file4.txt &&
> + p4 submit -d "Add file4" &&
> + rm -rf outside-spec
> + )
> +'
> +
> +test_expect_success 'Clone root' '
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config core.ignorecase false &&
> + git p4 clone --use-client-spec --destination="$git" //depot &&
> + # This method is used instead of "test -f" to ensure the case is
> + # checked even if the test is executed on case-insensitive file
> systems.
> + # All files are there as expected although the path cases look
> random.
> + cat
Re: [PATCH v2 2/2] trailer: support multiline title
Christian Couder writes:
> We currently ignore the first line passed to `git interpret-trailers`,
> when looking for the beginning of the trailers.
>
> Unfortunately this does not work well when a commit is created with a
> line break in the title, using for example the following command:
>
> git commit -m 'place of
> code: change we made'
>
> In this special case, it is best to look at the first line and if it
> does not contain only spaces, consider that the second line is not a
> trailer.
> ---
Missing sign-off, but more importantly, "let's special case the
first line" followed by "oh, that is not enough, we need to check
the found one is sensible and tweak it otherwise" smells like
incrementally papering over things.
I think the analysis behind the first patch is correct. It stops
the backward scan of the main loop to reach there by realizing that
the first line, which must be the first line of the patch title
paragraph, can never be a trailer.
To extend that correct realization to cover the case where the title
paragraph has more than one line, the right thing to do is to scan
forward from the beginning to find the first paragraph break, which
must be the end of the title paragraph, and exclude the whole thing,
wouldn't it?
That is, I am wondering why the patch is not more like this (there
may be off-by-one, but just to illustrate the approach; I didn't
even compile test this one so...)?
Puzzled...
static int find_trailer_start(struct strbuf **lines, int count)
{
- int start, only_spaces = 1;
+ int start, end_of_title, only_spaces = 1;
+
+ /* The first paragraph is the title and cannot be trailer */
+ for (start = 0; start < count; start++)
+ if (!lines[start]->len)
+ break; /* paragraph break */
+ end_of_title = start;
/*
* Get the start of the trailers by looking starting from the end
* for a line with only spaces before lines with one separator.
-* The first line must not be analyzed as the others as it
-* should be either the message title or a blank line.
*/
- for (start = count - 1; start >= 1; start--) {
+ for (start = count - 1; start >= end_of_title; start--) {
if (lines[start]->buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]->buf)) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] refs: clean up common_list
Instead of common_list having formatting like ! and /, use a struct to
hold common_list data in a structured form.
We don't use 'exclude' yet; instead, we keep the old codepath that
handles info/sparse-checkout and logs/HEAD. Later, we will use exclude.
Signed-off-by: David Turner
---
path.c | 58 +-
1 file changed, 37 insertions(+), 21 deletions(-)
diff --git a/path.c b/path.c
index 10f4cbf..d24bfa2 100644
--- a/path.c
+++ b/path.c
@@ -91,35 +91,51 @@ static void replace_dir(struct strbuf *buf, int len, const
char *newdir)
buf->buf[newlen] = '/';
}
-static const char *common_list[] = {
- "/branches", "/hooks", "/info", "!/logs", "/lost-found",
- "/objects", "/refs", "/remotes", "/worktrees", "/rr-cache", "/svn",
- "config", "!gc.pid", "packed-refs", "shallow",
- NULL
+struct common_dir {
+ /* Not considered garbage for report_linked_checkout_garbage */
+ unsigned ignore_garbage:1;
+ unsigned is_dir:1;
+ /* Not common even though its parent is */
+ unsigned exclude:1;
+ const char *dirname;
+};
+
+struct common_dir common_list[] = {
+ { 0, 1, 0, "branches" },
+ { 0, 1, 0, "hooks" },
+ { 0, 1, 0, "info" },
+ { 0, 0, 1, "info/sparse-checkout" },
+ { 1, 1, 0, "logs" },
+ { 1, 1, 1, "logs/HEAD" },
+ { 0, 1, 0, "lost-found" },
+ { 0, 1, 0, "objects" },
+ { 0, 1, 0, "refs" },
+ { 0, 1, 0, "remotes" },
+ { 0, 1, 0, "worktrees" },
+ { 0, 1, 0, "rr-cache" },
+ { 0, 1, 0, "svn" },
+ { 0, 0, 0, "config" },
+ { 1, 0, 0, "gc.pid" },
+ { 0, 0, 0, "packed-refs" },
+ { 0, 0, 0, "shallow" },
+ { 0, 0, 0, NULL }
};
static void update_common_dir(struct strbuf *buf, int git_dir_len)
{
char *base = buf->buf + git_dir_len;
- const char **p;
+ const struct common_dir *p;
if (is_dir_file(base, "logs", "HEAD") ||
is_dir_file(base, "info", "sparse-checkout"))
return; /* keep this in $GIT_DIR */
- for (p = common_list; *p; p++) {
- const char *path = *p;
- int is_dir = 0;
- if (*path == '!')
- path++;
- if (*path == '/') {
- path++;
- is_dir = 1;
- }
- if (is_dir && dir_prefix(base, path)) {
+ for (p = common_list; p->dirname; p++) {
+ const char *path = p->dirname;
+ if (p->is_dir && dir_prefix(base, path)) {
replace_dir(buf, git_dir_len, get_git_common_dir());
return;
}
- if (!is_dir && !strcmp(base, path)) {
+ if (!p->is_dir && !strcmp(base, path)) {
replace_dir(buf, git_dir_len, get_git_common_dir());
return;
}
@@ -129,16 +145,16 @@ static void update_common_dir(struct strbuf *buf, int
git_dir_len)
void report_linked_checkout_garbage(void)
{
struct strbuf sb = STRBUF_INIT;
- const char **p;
+ const struct common_dir *p;
int len;
if (!git_common_dir_env)
return;
strbuf_addf(&sb, "%s/", get_git_dir());
len = sb.len;
- for (p = common_list; *p; p++) {
- const char *path = *p;
- if (*path == '!')
+ for (p = common_list; p->dirname; p++) {
+ const char *path = p->dirname;
+ if (p->ignore_garbage)
continue;
strbuf_setlen(&sb, len);
strbuf_addstr(&sb, path);
--
2.4.2.622.gac67c30-twtrsrc
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/4] path: optimize common dir checking
Instead of a linear search over common_list to check whether
a path is common, use a trie. The trie search operates on
path prefixes, and handles excludes.
Signed-off-by: David Turner
---
path.c| 226 ++
t/t0060-path-utils.sh | 1 +
2 files changed, 213 insertions(+), 14 deletions(-)
diff --git a/path.c b/path.c
index d24bfa2..c7a4c40 100644
--- a/path.c
+++ b/path.c
@@ -121,25 +121,223 @@ struct common_dir common_list[] = {
{ 0, 0, 0, NULL }
};
-static void update_common_dir(struct strbuf *buf, int git_dir_len)
+/*
+ * A compressed trie. A trie node consists of zero or more characters that
+ * are common to all elements with this prefix, optionally followed by some
+ * children. If value is not NULL, the trie node is a terminal node.
+ *
+ * For example, consider the following set of strings:
+ * abc
+ * def
+ * definite
+ * definition
+ *
+ * The trie would look look like:
+ * root: len = 0, value = (something), children a and d non-NULL.
+ *a: len = 2, contents = bc
+ *d: len = 2, contents = ef, children i non-NULL, value = (something)
+ * i: len = 3, contents = nit, children e and i non-NULL, value = NULL
+ * e: len = 0, children all NULL, value = (something)
+ * i: len = 2, contents = on, children all NULL, value = (something)
+ */
+struct trie {
+ struct trie *children[256];
+ int len;
+ char *contents;
+ void *value;
+};
+
+static struct trie *make_trie_node(const char *key, void *value)
{
- char *base = buf->buf + git_dir_len;
- const struct common_dir *p;
+ struct trie *new_node = xcalloc(1, sizeof(*new_node));
+ new_node->len = strlen(key);
+ if (new_node->len) {
+ new_node->contents = xmalloc(new_node->len);
+ memcpy(new_node->contents, key, new_node->len);
+ }
+ new_node->value = value;
+ return new_node;
+}
- if (is_dir_file(base, "logs", "HEAD") ||
- is_dir_file(base, "info", "sparse-checkout"))
- return; /* keep this in $GIT_DIR */
- for (p = common_list; p->dirname; p++) {
- const char *path = p->dirname;
- if (p->is_dir && dir_prefix(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
- return;
+/*
+ * Add a key/value pair to a trie. The key is assumed to be \0-terminated.
+ * If there was an existing value for this key, return it.
+ */
+static void *add_to_trie(struct trie *root, const char *key, void *value)
+{
+ struct trie *child;
+ void *old;
+ int i;
+
+ if (!*key) {
+ /* we have reached the end of the key */
+ old = root->value;
+ root->value = value;
+ return old;
+ }
+
+ for (i = 0; i < root->len; ++i) {
+ if (root->contents[i] == key[i])
+ continue;
+
+ /*
+* Split this node: child will contain this node's
+* existing children.
+*/
+ child = malloc(sizeof(*child));
+ memcpy(child->children, root->children, sizeof(root->children));
+
+ child->len = root->len - i - 1;
+ if (child->len) {
+ child->contents = strndup(root->contents + i + 1,
+ child->len);
}
- if (!p->is_dir && !strcmp(base, path)) {
- replace_dir(buf, git_dir_len, get_git_common_dir());
- return;
+ child->value = root->value;
+ root->value = NULL;
+ root->len = i;
+
+ memset(root->children, 0, sizeof(root->children));
+ root->children[(unsigned char)root->contents[i]] = child;
+
+ /* This is the newly-added child. */
+ root->children[(unsigned char)key[i]] =
+ make_trie_node(key + i + 1, value);
+ return NULL;
+ }
+
+ /* We have matched the entire compressed section */
+ if (key[i]) {
+ child = root->children[(unsigned char)key[root->len]];
+ if (child) {
+ return add_to_trie(child, key + root->len + 1, value);
+ } else {
+ child = make_trie_node(key + root->len + 1, value);
+ root->children[(unsigned char)key[root->len]] = child;
+ return NULL;
}
}
+
+ old = root->value;
+ root->value = value;
+ return old;
+}
+
+typedef int (*match_fn)(const char *unmatched, void *data, void *baton);
+
+/*
+ * Search a trie for some key. Find the longest /-or-\0-terminated
+ * prefix of the key for which the trie contains a value. Call fn
+ * with the unmatched portion of the key and the
[PATCH v4 3/4] refs: make refs/worktree/* per-worktree
We need a place to stick refs for bisects in progress that is not
shared between worktrees. So we use the refs/worktree/ hierarchy.
The is_per_worktree_ref function and associated docs learn that
refs/worktree/ is per-worktree, as does the git_path code in path.c
The ref-packing functions learn that per-worktree refs should not be
packed (since packed-refs is common rather than per-worktree).
Since refs/worktree is per-worktree, logs/refs/worktree should be too.
Signed-off-by: David Turner
---
Documentation/glossary-content.txt | 5 +++--
path.c | 2 ++
refs.c | 32 +++-
t/t0060-path-utils.sh | 5 +
t/t1400-update-ref.sh | 19 +++
t/t3210-pack-refs.sh | 7 +++
6 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/Documentation/glossary-content.txt
b/Documentation/glossary-content.txt
index 8c6478b..5c707e6 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -413,8 +413,9 @@ exclude;;
[[def_per_worktree_ref]]per-worktree ref::
Refs that are per-<>, rather than
- global. This is presently only <>, but might
- later include other unusual refs.
+ global. This is presently only <> and any refs
+ that start with `refs/worktree/`, but might later include other
+ unusual refs.
[[def_pseudoref]]pseudoref::
Pseudorefs are a class of files under `$GIT_DIR` which behave
diff --git a/path.c b/path.c
index c7a4c40..acac7b1 100644
--- a/path.c
+++ b/path.c
@@ -107,9 +107,11 @@ struct common_dir common_list[] = {
{ 0, 0, 1, "info/sparse-checkout" },
{ 1, 1, 0, "logs" },
{ 1, 1, 1, "logs/HEAD" },
+ { 0, 1, 1, "logs/refs/worktree" },
{ 0, 1, 0, "lost-found" },
{ 0, 1, 0, "objects" },
{ 0, 1, 0, "refs" },
+ { 0, 1, 1, "refs/worktree", },
{ 0, 1, 0, "remotes" },
{ 0, 1, 0, "worktrees" },
{ 0, 1, 0, "rr-cache" },
diff --git a/refs.c b/refs.c
index e6fc3fe..30331bc 100644
--- a/refs.c
+++ b/refs.c
@@ -298,6 +298,11 @@ struct ref_entry {
};
static void read_loose_refs(const char *dirname, struct ref_dir *dir);
+static int search_ref_dir(struct ref_dir *dir, const char *refname, size_t
len);
+static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,
+ const char *dirname, size_t len,
+ int incomplete);
+static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
static struct ref_dir *get_ref_dir(struct ref_entry *entry)
{
@@ -306,6 +311,24 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
dir = &entry->u.subdir;
if (entry->flag & REF_INCOMPLETE) {
read_loose_refs(entry->name, dir);
+
+ /*
+* Manually add refs/worktree, which, being
+* per-worktree, might not appear in the directory
+* listing for refs/ in the main repo.
+*/
+ if (!strcmp(entry->name, "refs/")) {
+ int pos = search_ref_dir(dir, "refs/worktree/", 14);
+ if (pos < 0) {
+ struct ref_entry *child_entry;
+ child_entry = create_dir_entry(dir->ref_cache,
+ "refs/worktree/",
+ 14, 1);
+ add_entry_to_dir(dir, child_entry);
+ read_loose_refs("refs/worktree",
+ &child_entry->u.subdir);
+ }
+ }
entry->flag &= ~REF_INCOMPLETE;
}
return dir;
@@ -2643,6 +2666,8 @@ struct pack_refs_cb_data {
struct ref_to_prune *ref_to_prune;
};
+static int is_per_worktree_ref(const char *refname);
+
/*
* An each_ref_entry_fn that is run over loose references only. If
* the loose reference can be packed, add an entry in the packed ref
@@ -2656,6 +2681,10 @@ static int pack_if_possible_fn(struct ref_entry *entry,
void *cb_data)
struct ref_entry *packed_entry;
int is_tag_ref = starts_with(entry->name, "refs/tags/");
+ /* Do not pack per-worktree refs: */
+ if (is_per_worktree_ref(entry->name))
+ return 0;
+
/* ALWAYS pack tags */
if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref)
return 0;
@@ -2850,7 +2879,8 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
static int is_per_worktree_ref(const char *refname)
{
- return !strcmp(refname, "HEAD");
+ return !strcmp(refname, "HEAD") ||
+ starts_with(refname, "refs/worktree/");
}
static int is_pseudoref_syntax(
[PATCH v4 4/4] bisect: make bisection refs per-worktree
Using the new refs/worktree/ refs, make bisection per-worktree.
Signed-off-by: David Turner
---
Documentation/git-bisect.txt | 4 ++--
Documentation/rev-list-options.txt | 14 +++---
bisect.c | 2 +-
builtin/rev-parse.c| 6 --
git-bisect.sh | 14 +++---
revision.c | 2 +-
t/t6030-bisect-porcelain.sh| 20 ++--
7 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index e97f2de..f0c52d1 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -82,7 +82,7 @@ to ask for the next commit that needs testing.
Eventually there will be no more revisions left to inspect, and the
command will print out a description of the first bad commit. The
-reference `refs/bisect/bad` will be left pointing at that commit.
+reference `refs/worktree/bisect/bad` will be left pointing at that commit.
Bisect reset
@@ -373,7 +373,7 @@ on a single line.
$ git bisect start HEAD [ ... ]
--no-checkout
$ git bisect run sh -c '
- GOOD=$(git for-each-ref "--format=%(objectname)" refs/bisect/good-*) &&
+ GOOD=$(git for-each-ref "--format=%(objectname)"
refs/worktree/bisect/good-*) &&
git rev-list --objects BISECT_HEAD --not $GOOD >tmp.$$ &&
git pack-objects --stdout >/dev/null >"$GIT_DIR/BISECT_LOG"
test -n "$nolog" || echo "git bisect $state $rev"
>>"$GIT_DIR/BISECT_LOG"
}
@@ -282,8 +282,8 @@ bisect_state() {
bisect_next_check() {
missing_good= missing_bad=
- git show-ref -q --verify refs/bisect/$TERM_BAD || missing_bad=t
- test -n "$(git for-each-ref "refs/bisect/$TERM_GOOD-*")" ||
missing_good=t
+ git show-ref -q --verify refs/worktree/bisect/$TERM_BAD || missing_bad=t
+ test -n "$(git for-each-ref "refs/worktree/bisect/$TERM_GOOD-*")" ||
missing_good=t
case "$missing_good,$missing_bad,$1" in
,,*)
@@ -341,15 +341,15 @@ bisect_next() {
# Check if we should exit because bisection is finished
if test $res -eq 10
then
- bad_rev=$(git show-ref --hash --verify refs/bisect/$TERM_BAD)
+ bad_rev=$(git show-ref --hash --verify
refs/worktree/bisect/$TERM_BAD)
bad_commit=$(git show-branch $bad_rev)
echo "# first $TERM_BAD commit: $bad_commit"
>>"$GIT_DIR/BISECT_LOG"
exit 0
elif test $res -eq 2
then
echo "# only skipped commits left to test"
>>"$GIT_DIR/BISECT_LOG"
- good_revs=$(git for-each-ref --format="%(objectname)"
"refs/bisect/$TERM_GOOD-*")
- for skipped in $(git rev-list refs/bisect/$TERM_BAD --not
$good_revs)
+ good_revs=$(git for-each-ref --format="%(objectname)"
"refs/worktree/bisect/$TERM_GOOD-*")
+ for skipped in $(git rev-list refs/worktree/bisect/$TERM_BAD
--not $good_revs)
do
skipped_commit=$(git show-branch $skipped)
echo "# possible first $TERM_BAD commit:
$skipped_commit" >>"$GIT_DIR/BISECT_LOG"
@@ -412,7 +412,7 @@ Try 'git bisect reset '.")"
bisect_clean_state() {
# There may be some refs packed during bisection.
- git for-each-ref --format='%(refname) %(objectname)' refs/bisect/\* |
+ git for-each-ref --format='%(refname) %(objectname)'
refs/worktree/bisect/\* |
while read ref hash
do
git update-ref -d $ref $hash || exit
diff --git a/revision.c b/revision.c
index b6b2cf7..974a11f 100644
--- a/revision.c
+++ b/revision.c
@@ -2084,7 +2084,7 @@ extern void read_bisect_terms(const char **bad, const
char **good);
static int for_each_bisect_ref(const char *submodule, each_ref_fn fn, void
*cb_data, const char *term) {
struct strbuf bisect_refs = STRBUF_INIT;
int status;
- strbuf_addf(&bisect_refs, "refs/bisect/%s", term);
+ strbuf_addf(&bisect_refs, "refs/worktree/bisect/%s", term);
status = for_each_ref_in_submodule(submodule, bisect_refs.buf, fn,
cb_data);
strbuf_release(&bisect_refs);
return status;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9e2c203..bfd5463 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -68,7 +68,7 @@ test_expect_success 'bisect fails if given any junk instead
of revs' '
git bisect reset &&
test_must_fail git bisect start foo $HASH1 -- &&
test_must_fail git bisect start $HASH4 $HASH1 bar -- &&
- test -z "$(git for-each-ref "refs/bisect/*")" &&
+ test -z "$(git for-each-ref "refs/worktree/bisect/*")" &&
test -z "$(ls .git/BISECT_* 2>/dev/null)" &&
git bisect start &&
test_must_fail git bisect good foo $HASH1 &&
@@ -77,7 +77,7 @@ test_expect_success 'bisect fails if given any jun
[PATCH v4 0/4] per-worktree bisection refs
This reroll includes changes suggested by Duy Nguyen: A. Path normalization (partial). B. Rearrangement of common_list struct to make formatting prettier. It also includes a test style fix suggested by Eric Sunshine and others: a bogus test_must_fail on a non-git command has been replaced by a two-step check. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re* OS X Yosemite make all doc fails
On 2015-08-26 19.42, Junio C Hamano wrote: > Jeff S writes: > >> Brian thanks for responding! I'm finally able to build git completely. >> Would it be possible to add the OS X dependency to the git/INSTALL >> file? >> >> OSX Yosemite 10.10.5 >> Xcode 6.4 (6E35b) >> … >> $ brew install autoconf >> $ brew install asciidoc >> $ brew install xmlto >> $ brew install docbook >> $ export XML_CATALOG_FILES=/usr/local/etc/xml/catalog >> $ brew install docbook-xsl > > It may not be a bad idea to add a whole section at the end of the > document to list the prerequisite packages for various common > platforms, whose beginning perhaps would look like this? > > diff --git a/INSTALL b/INSTALL > index ffb071e..84fa5cf 100644 > --- a/INSTALL > +++ b/INSTALL > @@ -221,3 +221,23 @@ Issues of note: > http://www.oasis-open.org/docbook/xml/4.5/xsl/current \ > /usr/share/sgml/docbook/xml-dtd-4.5 \ > /etc/xml/catalog > + > + > +Platform specific hints: > + > +You would need to install prerequiste tools and libraries to compile > +and use Git from the source. > + > + - OSX needs the following packages installed with 'brew install' > + (in addition to the usual make and C compiler suite): > + > + autoconf, asciidoc, xmlto, docbook, docbook-xsl > + > + - Linux distributions derived from Debian need the following packages > + instaslled via 'apt-get install' or similar (in addition to the > + usual 'make' and C compiler suite that come as part of > + build-essential): > + > + autoconf, asciidoc, xmlto, docbook, libz-dev, livcurl4-openssl-dev, > + ... > + I like this. This is what I have been using for Debian (and RHEL 6.5 or so) (But it doesn't include "make doc") #!/bin/sh if type apt-get; then APTGET="sudo apt-get install" fi if type yum; then APTGET="sudo /usr/bin/yum install" fi export APTGET type gcc || $APTGET gcc type curl-config || $APTGET libcurl4-openssl-dev || $APTGET libcurl-devel test -r /usr/include/expat.h || $APTGET libexpat1-dev || $APTGET expat-devel test -r /usr/include/ssl.h || test -r /usr/include/openssl/ssl.h || $APTGET openssl-devel type msgfmt || $APTGET gettext type make || $APTGET make -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
On Wed, Aug 26, 2015 at 8:07 PM, Junio C Hamano wrote:
> Karthik Nayak writes:
>
>> On Wed, Aug 26, 2015 at 12:46 AM, Junio C Hamano wrote:
>>> Karthik Nayak writes:
>>>
>>> I didn't check how wide the original is supposed to be, but perhaps
>>> changing builtin/tag.c this way
>>>
>>> if (filter->lines)
>>> - format = "%(align:16,left)%(refname:short)%(end)";
>>> + format = "%(align:15,left)%(refname:short)%(end) ";
>>> else
>>> format = "%(refname:short)";
>>> }
>>>
>>> may be one way to fix it. Check the original "tag -l" output for
>>> tags whose names are 14, 15, 16 and 17 display-columns wide and try
>>> to match it.
>>
>> That should be the fix, since it's a space problem.
>>
>> The problem with doing this is, the lines need to be displayed
>> immediately after the refname,
>> followed by a newline, what you're suggesting breaks that flow.
>
> That is only because show_ref_array_item() does too much. The
> function is given a placeholder string and a set of data to fill the
> placeholder with for an item, and the reason why the caller
> repeatedly calls it, once per item it has, is because it wants to
> produce a one-line-per-item output. An alternative valid way to
> structure the API is to have it format into a string and leave the
> printing to the caller. You can give a new format_ref_array_item()
> that does not print but fills a strbuf to this caller, make
> show_ref_array_item() a thin wrapper that calls it and prints it
> with the final LF for other callers.
>
> Another alternate approach, if you want to give "tag -l" annotation
> available to for-each-ref, would be to do this:
>
>if (filter->lines)
>format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
> "%%(contents:lines=%s)", filter->lines);
>else
>format = "%(refname:short)";
>
> and teach a new %(contents:lines=1) atom. That way, you can lose
> the ugly special case call to show_tag_lines() that can only be at
> the end of the output. I somehow think this is a better approach.
>
This seems like a good approach, since contents is already an atom, this would
fit in easily.
> Of course you can (and probably would want to) do both, giving a
> bit lower level "emit to a strbuf" function to the callers _and_
> losing hardcoded call to show_tag_lines().
You're saying remove show_ref_array_item() (even the wrapper you mentioned
above) and just have something like format_ref_array_item() which
would output to a strbuf. and let the caller worry about the printing?
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
On Wed, Aug 26, 2015 at 9:18 PM, Matthieu Moy wrote: > Junio C Hamano writes: > >> Junio C Hamano writes: >> >>> Matthieu Moy writes: >>> > For the current code %(if:empty)%(align)%(end)%(then)Empty%(else)Not > Empty%(end) > would print non-empty, I guess the documentation holds in that case. > Not sure if we require it to print non-empty. You don't want the %(if) condition to depend on whether --shell/--python/... is used. Since %(if:empty)%(align)%(end)%(then) holds when you don't use --shell, you also want it to hold when you quote. IOW, you should check for emptyness before (or actually without) doing the quoting. I guess this is what you're doing, and if so, I think it's "The Right Thing". >>> >>> I agree that %(align)%(end) should expand to empty and %(if:empty)...%(then) >>> should look at that empty string without quoting. So >>> >>> %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) >>> >>> should give "Empty"; otherwise the code is buggy, I think. >> >> (I shouldn't be typing while eating...) >> >> It should give "Empty", but the --shell/--python/... may make the >> whole "Empty", as the result of %(if:...)...%(end), be quoted. So >> you may see "'Empty'" in the output. > > Agreed (with both points). > Thanks, will work on this. -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git's inconsistent command line options
On Wed, Aug 26, 2015 at 10:56 AM, Junio C Hamano wrote: > But notice that I said "if you really want to". I personally think > it is a road to madness. Agreed. I don't believe in command line API here. I think we'd need a better solution. My gut says: Live with the warts on old commands and try to make people use command words for new commands. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git's inconsistent command line options
Hilco Wijbenga writes: > On 25 August 2015 at 16:43, Junio C Hamano wrote: >> I do not see a good way to do such a safe transition with command >> words approach, *unless* we are going to introduce new commands, >> i.e. "git list-tag", "git create-tag", etc. > > Perhaps we could introduce a more explicit notion (in .git/config) of > a Git API version (or, perhaps more accurate, a Git CLI API version)? > ... > I think that from a user's point of view this could work quite well. > Obviously, (worst case scenario) Git commands might have to support up > to 3 APIs at the same time (previous [2], current [3], and future [4] > for Git 3.x) so from a code maintenance POV it would certainly > introduce complexity and probably some duplication of code. I'm > hopeful it would be limited to CL argument processing but I suspect > that when Git code calls other Git code (especially in the Bash based > commands) there might be some more complexity there. > > Would something like that be feasible? A bigger issue you need to think about is what to do to scripts people have. Your approach forces them to update a script to delete a tag and do something else to say something silly like this: #!/bin/sh # My Script case "$(git config core.apiVersion)" in "" | 2) git tag -d "$1" do something else on "$1" using v2 API ;; 3) git tag delete "$1" do something else on "$1" using v3 API ;; *) echo >&2 "sorry, I do not know what Git you are using" exit 1 ;; esac So, it may be feasible to implement, but I do not think it would be useful to do so. Instead, if you really want to invent the multi-version world, you would rather want to have something more like this: #!/bin/sh # My Script GIT_API_VERSION=2 export GIT_API_VERSION git tag -d "$1" do something else on "$1" using v2 API But notice that I said "if you really want to". I personally think it is a road to madness. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re* OS X Yosemite make all doc fails
Jeff S writes: > Brian thanks for responding! I'm finally able to build git completely. > Would it be possible to add the OS X dependency to the git/INSTALL > file? > > OSX Yosemite 10.10.5 > Xcode 6.4 (6E35b) > … > $ brew install autoconf > $ brew install asciidoc > $ brew install xmlto > $ brew install docbook > $ export XML_CATALOG_FILES=/usr/local/etc/xml/catalog > $ brew install docbook-xsl It may not be a bad idea to add a whole section at the end of the document to list the prerequisite packages for various common platforms, whose beginning perhaps would look like this? diff --git a/INSTALL b/INSTALL index ffb071e..84fa5cf 100644 --- a/INSTALL +++ b/INSTALL @@ -221,3 +221,23 @@ Issues of note: http://www.oasis-open.org/docbook/xml/4.5/xsl/current \ /usr/share/sgml/docbook/xml-dtd-4.5 \ /etc/xml/catalog + + +Platform specific hints: + +You would need to install prerequiste tools and libraries to compile +and use Git from the source. + + - OSX needs the following packages installed with 'brew install' + (in addition to the usual make and C compiler suite): + + autoconf, asciidoc, xmlto, docbook, docbook-xsl + + - Linux distributions derived from Debian need the following packages + instaslled via 'apt-get install' or similar (in addition to the + usual 'make' and C compiler suite that come as part of + build-essential): + + autoconf, asciidoc, xmlto, docbook, libz-dev, livcurl4-openssl-dev, + ... + -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] submodule: helper to run foreach in parallel
On Wed, Aug 26, 2015 at 10:06 AM, Jeff King wrote:
> On Tue, Aug 25, 2015 at 10:28:24AM -0700, Stefan Beller wrote:
>
>> +int module_foreach_parallel(int argc, const char **argv, const char *prefix)
>> +{
>> [...]
>> + for (i = 0; i < ce_used; i++) {
>> + const struct submodule *sub;
>> + const struct cache_entry *ce = ce_entries[i];
>> + struct submodule_args *args = malloc(sizeof(*args));
>> +
>> + if (ce_stage(ce))
>> + args->sha1 = xstrdup(sha1_to_hex(null_sha1));
>> + else
>> + args->sha1 = xstrdup(sha1_to_hex(ce->sha1));
>> +
>> + strbuf_reset(&sb);
>> + strbuf_addf(&sb, "%s/.git", ce->name);
>> + if (!file_exists(sb.buf))
>> + continue;
>
> "args" and "args->sha1" go out of scope and leak here.
>
>> + args->name = sub->name;
>> + args->toplevel = xstrdup(xgetcwd());
>
> Another xgetcwd leak. :) I think this one can just drop the xstrdup.
>
> (Both of these were spotted by Coverity. I know you have played with it
> a little, so if you are actually reading the emails it sends, I'll stop
> relaying them).
Fixing those memleaks was the first thing I did this morning. :)
And yeah I do pay attention to these emails. (I maintain the automatic testing
every other day, so there is that.)
>
> -Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/5] submodule: helper to run foreach in parallel
On Tue, Aug 25, 2015 at 10:28:24AM -0700, Stefan Beller wrote:
> +int module_foreach_parallel(int argc, const char **argv, const char *prefix)
> +{
> [...]
> + for (i = 0; i < ce_used; i++) {
> + const struct submodule *sub;
> + const struct cache_entry *ce = ce_entries[i];
> + struct submodule_args *args = malloc(sizeof(*args));
> +
> + if (ce_stage(ce))
> + args->sha1 = xstrdup(sha1_to_hex(null_sha1));
> + else
> + args->sha1 = xstrdup(sha1_to_hex(ce->sha1));
> +
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "%s/.git", ce->name);
> + if (!file_exists(sb.buf))
> + continue;
"args" and "args->sha1" go out of scope and leak here.
> + args->name = sub->name;
> + args->toplevel = xstrdup(xgetcwd());
Another xgetcwd leak. :) I think this one can just drop the xstrdup.
(Both of these were spotted by Coverity. I know you have played with it
a little, so if you are actually reading the emails it sends, I'll stop
relaying them).
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pack negotiation algorithm between 2 share-nothing repos
Junio C Hamano writes: > Duy Nguyen writes: > >> I know this is a corner case, but because it has a valid use case, >> maybe we should do something about it. Immediate reaction is to add an >> option to send no "have"s. But maybe you guys have better ideas. > > This and similar corner cases were discussed in very early days of > Git. > > One interesting idea floated back then but was not pursued was to > dig and send have's sparsely and then back up. Instead of digging > and sending _all_ commits in a contiguous history, after sending the > tip, you skip the commits from the history before sending the next > one, and progressively make the skipping larger (e.g. Fibonacci, or > exponential). You need to remember what you sent and for each of > what you sent its topologically-oldest descendant you sent earlier > that you heard the other side does not have. > > Then, when you get an Ack, you know a stretch of history between a > commit that is known to be common (i.e. the one you heard an Ack > just now) and its descendant that is known only to you (i.e. the > topologically-oldest one you remember that you did send and they > didn't say is common). At that point, you and the other end can > bisect that range. If anybody is interested, here is a good place to start: http://thread.gmane.org/gmane.comp.version-control.git/96149/focus=102485 [Cc'ed Stefan as I think he was collecting possible enhancement to put in the protocol v2]. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Aug 2015, #04; Tue, 25)
Jacob Keller writes: > On Tue, Aug 25, 2015 at 4:28 PM, Junio C Hamano wrote: >> * jk/notes-merge-config (2015-08-17) 6 commits >> - notes: teach git-notes about notes..mergeStrategy option >> - notes: add notes.mergeStrategy option to select default strategy >> - notes: add tests for --commit/--abort/--strategy exclusivity >> - notes: extract parse_notes_merge_strategy to notes-utils >> - notes: extract enum notes_merge_strategy to notes-utils.h >> - notes: document cat_sort_uniq rewriteMode >> >> "git notes merge" can be told with "--strategy=" option how to >> automatically handle conflicts; this can now be configured by >> setting notes.merge configuration variable. >> >> I think this is ready for 'next', but did I miss comments that need >> to be addressed before it happens? > > I believe I got all the comments fixed in the most recent series. I > was not aware of any other comments so if someone has them letting me > know would be helpful as I had no plans to re-roll anything at > present. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] trailer: support multiline title
Junio C Hamano writes: > While the reordering would certainly stop showing the comments and > patch, I am not sure if that is a move in the right direction. It > will rob from the hooks information that they have traditionally > been given--- The information given in the comments do not have a 100% stable format, and the hook is executed after letting the user possibly edit or delete it, so I'm tempted to say that a hook using the commit comment is broken. Using the diff in the hook _is_ really broken: it relies on the user calling "git commit" with -v, and there's nothing to garantee that. > it will break some hooks. It will also repair some hooks that were broken, but whose breakage was never noticed or never explained. > After all, interpret-trailers was invented exactly because we did not > want individual hooks to roll their own ways to detect the end of the > message proper, so the command should know where the message ends. Right, but you can't prevent people from writting command-that-shows-stuff >> "$1" in their commit-msg hook. And these people will keep wondering why their hook "sometimes doesn't work" (that's how I considered it for a while, it took me a few commits to notice the correlation between "-v" and lack of sign-off). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 05/12] ref-filter: add option to filter out tags, branches and remotes
Comments inline.
On 08/22/2015 05:39 AM, Karthik Nayak wrote:
> From: Karthik Nayak
>
> Add a function called 'for_each_reftype_fullpath()' to refs.{c,h}
> which iterates through each ref for the given path without trimming
> the path and also accounting for broken refs, if mentioned.
>
> Add 'filter_ref_kind()' in ref-filter.c to check the kind of ref being
> handled and return the kind to 'ref_filter_handler()', where we
> discard refs which we do not need and assign the kind to needed refs.
>
> Mentored-by: Christian Couder
> Mentored-by: Matthieu Moy
> Signed-off-by: Karthik Nayak
> ---
> ref-filter.c | 59 ++-
> ref-filter.h | 12 ++--
> refs.c | 9 +
> refs.h | 1 +
> 4 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index ffec10a..d5fae1a 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1123,6 +1123,36 @@ static struct ref_array_item *new_ref_array_item(const
> char *refname,
> return ref;
> }
>
> +static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> +{
> + unsigned int i;
> +
> + static struct {
> + const char *prefix;
> + unsigned int kind;
> + } ref_kind[] = {
> + { "refs/heads/" , FILTER_REFS_BRANCHES },
> + { "refs/remotes/" , FILTER_REFS_REMOTES },
> + { "refs/tags/", FILTER_REFS_TAGS}
> + };
> +
> + if (filter->kind == FILTER_REFS_BRANCHES)
> + return FILTER_REFS_BRANCHES;
> + else if (filter->kind == FILTER_REFS_REMOTES)
> + return FILTER_REFS_REMOTES;
> + else if (filter->kind == FILTER_REFS_TAGS)
> + return FILTER_REFS_TAGS;
> + else if (!strcmp(refname, "HEAD"))
> + return FILTER_REFS_DETACHED_HEAD;
I think this would be clearer written like so:
if (filter->kind == FILTER_REFS_BRANCHES ||
filter->kind == FILTER_REFS_REMOTES ||
filter->kind == FILTER_REFS_TAGS)
return filter->kind;
if (!strcmp(refname, "HEAD"))
return FILTER_REFS_DETACHED_HEAD;
Or, even better, if you can set filter->kind to zero if it is not one of
these constants, then you could simplify this to
if (filter->kind)
return filter->kind;
if (!strcmp(refname, "HEAD"))
return FILTER_REFS_DETACHED_HEAD;
> +
> + for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
> + if (starts_with(refname, ref_kind[i].prefix))
> + return ref_kind[i].kind;
> + }
> +
> + return FILTER_REFS_OTHERS;
> +}
> +
> /*
> * A call-back given to for_each_ref(). Filter refs and keep them for
> * later object processing.
> @@ -1133,6 +1163,7 @@ static int ref_filter_handler(const char *refname,
> const struct object_id *oid,
> struct ref_filter *filter = ref_cbdata->filter;
> struct ref_array_item *ref;
> struct commit *commit = NULL;
> + unsigned int kind;
>
> if (flag & REF_BAD_NAME) {
> warning("ignoring ref with broken name %s", refname);
> @@ -1144,6 +1175,10 @@ static int ref_filter_handler(const char *refname,
> const struct object_id *oid,
> return 0;
> }
>
> + kind = filter_ref_kind(filter, refname);
> + if (!(kind & filter->kind))
> + return 0;
> +
> if (*filter->name_patterns &&
> !match_name_as_path(filter->name_patterns, refname))
> return 0;
>
> @@ -1175,6 +1210,7 @@ static int ref_filter_handler(const char *refname,
> const struct object_id *oid,
>
> REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
> ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
> + ref->kind = kind;
> return 0;
> }
>
> @@ -1251,16 +1287,29 @@ int filter_refs(struct ref_array *array, struct
> ref_filter *filter, unsigned int
> {
> struct ref_filter_cbdata ref_cbdata;
> int ret = 0;
> + unsigned int broken = 0;
>
> ref_cbdata.array = array;
> ref_cbdata.filter = filter;
>
> /* Simple per-ref filtering */
> - if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
> - ret = for_each_rawref(ref_filter_handler, &ref_cbdata);
> - else if (type & FILTER_REFS_ALL)
> - ret = for_each_ref(ref_filter_handler, &ref_cbdata);
> - else if (type)
> + if (type & FILTER_REFS_INCLUDE_BROKEN) {
> + type &= ~FILTER_REFS_INCLUDE_BROKEN;
> + broken = 1;
> + }
I wouldn't mask out the FILTER_REFS_INCLUDE_BROKEN bit here. Instead I
would write
> +
> + filter->kind = type;
as
filter->kind = type & FILTER_REFS_KIND_MASK;
where FILTER_REFS_KIND_MASK is the OR of all of the kind bits. The
advantage is that this approach would remain correct if more bits are
added to type. Then in the following if statements, test filter->kind
instead of type.
> + if (type == FILTER
[PATCH v6] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider
We run P4 servers on Linux and P4 clients on Windows. For an unknown
reason the file path for a number of files in P4 does not match the
directory path with respect to case sensitivity.
E.g. "p4 files" might return
//depot/path/to/file1
//depot/pATH/to/file2
If you use P4/P4V then these files end up in the same directory, e.g.
//depot/path/to/file1
//depot/path/to/file2
If you use git-p4 and clone the code via client spec "//depot/path/..."
then all files not matching the case in the client spec will be ignored
(in the example above "file2"). This is correct if core.ignorecase=false
but not otherwise.
Signed-off-by: Lars Schneider
Acked-by: Luke Diamand
---
git-p4.py | 7 ++
t/t9821-git-p4-path-variations.sh | 200 ++
2 files changed, 207 insertions(+)
create mode 100755 t/t9821-git-p4-path-variations.sh
diff --git a/git-p4.py b/git-p4.py
index 073f87b..0093fa3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1950,10 +1950,14 @@ class View(object):
if "unmap" in res:
# it will list all of them, but only one not unmap-ped
continue
+if gitConfigBool("core.ignorecase"):
+res['depotFile'] = res['depotFile'].lower()
self.client_spec_path_cache[res['depotFile']] =
self.convert_client_path(res["clientFile"])
# not found files or unmap files set to ""
for depotFile in fileArgs:
+if gitConfigBool("core.ignorecase"):
+depotFile = depotFile.lower()
if depotFile not in self.client_spec_path_cache:
self.client_spec_path_cache[depotFile] = ""
@@ -1962,6 +1966,9 @@ class View(object):
depot file should live. Returns "" if the file should
not be mapped in the client."""
+if gitConfigBool("core.ignorecase"):
+depot_path = depot_path.lower()
+
if depot_path in self.client_spec_path_cache:
return self.client_spec_path_cache[depot_path]
diff --git a/t/t9821-git-p4-path-variations.sh
b/t/t9821-git-p4-path-variations.sh
new file mode 100755
index 000..599d16c
--- /dev/null
+++ b/t/t9821-git-p4-path-variations.sh
@@ -0,0 +1,200 @@
+#!/bin/sh
+
+test_description='Clone repositories with path case variations'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d with case folding enabled' '
+ start_p4d -C1
+'
+
+test_expect_success 'Create a repo with path case variations' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+
+ mkdir -p One/two &&
+ >One/two/File2.txt &&
+ p4 add One/two/File2.txt &&
+ p4 submit -d "Add file2" &&
+ rm -rf One &&
+
+ mkdir -p one/TWO &&
+ >one/TWO/file1.txt &&
+ p4 add one/TWO/file1.txt &&
+ p4 submit -d "Add file1" &&
+ rm -rf one &&
+
+ mkdir -p one/two &&
+ >one/two/file3.txt &&
+ p4 add one/two/file3.txt &&
+ p4 submit -d "Add file3" &&
+ rm -rf one &&
+
+ mkdir -p outside-spec &&
+ >outside-spec/file4.txt &&
+ p4 add outside-spec/file4.txt &&
+ p4 submit -d "Add file4" &&
+ rm -rf outside-spec
+ )
+'
+
+test_expect_success 'Clone root' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config core.ignorecase false &&
+ git p4 clone --use-client-spec --destination="$git" //depot &&
+ # This method is used instead of "test -f" to ensure the case is
+ # checked even if the test is executed on case-insensitive file
systems.
+ # All files are there as expected although the path cases look
random.
+ cat >expect <<-\EOF &&
+ One/two/File2.txt
+ one/TWO/file1.txt
+ one/two/file3.txt
+ outside-spec/file4.txt
+ EOF
+ git ls-files >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'Clone root (ignorecase)' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config core.ignorecase true &&
+ git p4 clone --use-client-spec --destination="$git" //depot &&
+ # This method is used instead of "test -f" to ensure the case is
+ # checked even if the test is executed on case-insensitive file
systems.
+ # All files are there as expected although the path cases look
random.
+ cat >expect <<-\EOF &&
+ one/TWO/File2.txt
+
Re: [PATCH v2 2/2] trailer: support multiline title
Christian Couder writes: > There is already code to detect a patch in interpret-trailers, but it > relies on the patch starting with a line with only three dashes. Hmm, then it can be taught to notice "everything below..." as another marker, right? > Maybe. I don't know if there is a reason why the commit-msg is called > before removing the patch. Is that "removing", or are you talking about changing the order from - prepare log template in-core - add comments and patch to that in-core copy - write in-core copy out - run hook - read the hook's result in-core - use the message to - prepare log template in-core - write in-core copy out - run hook - read the hook's result in-core - add comments and patch to that in-core copy - use the message While the reordering would certainly stop showing the comments and patch, I am not sure if that is a move in the right direction. It will rob from the hooks information that they have traditionally been given---it will break some hooks. But if interpret-trailers is almost there to reliably know where the log message ends, teaching it the one last step would be the right thing to do anyway. After all, interpret-trailers was invented exactly because we did not want individual hooks to roll their own ways to detect the end of the message proper, so the command should know where the message ends. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] i18n: am: fix typo in description of -b option
Signed-off-by: Jiang Xin
---
builtin/am.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/am.c b/builtin/am.c
index b9c62e3..699a924 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2168,7 +2168,7 @@ int cmd_am(int argc, const char **argv, const char
*prefix)
OPT_BOOL('i', "interactive", &state.interactive,
N_("run interactively")),
OPT_HIDDEN_BOOL('b', "binary", &binary,
- N_("(historical option -- no-op")),
+ N_("historical option -- no-op")),
OPT_BOOL('3', "3way", &state.threeway,
N_("allow fall back on 3way merging if needed")),
OPT__QUIET(&state.quiet, N_("be quiet")),
--
2.5.0.416.gf01ac32
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Junio C Hamano writes: > Junio C Hamano writes: > >> Matthieu Moy writes: >> For the current code %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) would print non-empty, I guess the documentation holds in that case. Not sure if we require it to print non-empty. >>> >>> You don't want the %(if) condition to depend on whether >>> --shell/--python/... is used. Since %(if:empty)%(align)%(end)%(then) >>> holds when you don't use --shell, you also want it to hold when you >>> quote. IOW, you should check for emptyness before (or actually without) >>> doing the quoting. I guess this is what you're doing, and if so, I think >>> it's "The Right Thing". >> >> I agree that %(align)%(end) should expand to empty and %(if:empty)...%(then) >> should look at that empty string without quoting. So >> >> %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) >> >> should give "Empty"; otherwise the code is buggy, I think. > > (I shouldn't be typing while eating...) > > It should give "Empty", but the --shell/--python/... may make the > whole "Empty", as the result of %(if:...)...%(end), be quoted. So > you may see "'Empty'" in the output. Agreed (with both points). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Junio C Hamano writes: > Matthieu Moy writes: > >>> For the current code %(if:empty)%(align)%(end)%(then)Empty%(else)Not >>> Empty%(end) >>> would print non-empty, I guess the documentation holds in that case. >>> Not sure if we require it to print non-empty. >> >> You don't want the %(if) condition to depend on whether >> --shell/--python/... is used. Since %(if:empty)%(align)%(end)%(then) >> holds when you don't use --shell, you also want it to hold when you >> quote. IOW, you should check for emptyness before (or actually without) >> doing the quoting. I guess this is what you're doing, and if so, I think >> it's "The Right Thing". > > I agree that %(align)%(end) should expand to empty and %(if:empty)...%(then) > should look at that empty string without quoting. So > > %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) > > should give "Empty"; otherwise the code is buggy, I think. (I shouldn't be typing while eating...) It should give "Empty", but the --shell/--python/... may make the whole "Empty", as the result of %(if:...)...%(end), be quoted. So you may see "'Empty'" in the output. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Matthieu Moy writes: >> For the current code %(if:empty)%(align)%(end)%(then)Empty%(else)Not >> Empty%(end) >> would print non-empty, I guess the documentation holds in that case. >> Not sure if we require it to print non-empty. > > You don't want the %(if) condition to depend on whether > --shell/--python/... is used. Since %(if:empty)%(align)%(end)%(then) > holds when you don't use --shell, you also want it to hold when you > quote. IOW, you should check for emptyness before (or actually without) > doing the quoting. I guess this is what you're doing, and if so, I think > it's "The Right Thing". I agree that %(align)%(end) should expand to empty and %(if:empty)...%(then) should look at that empty string without quoting. So %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) should give "Empty"; otherwise the code is buggy, I think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] trailer: support multiline title
Sorry I sent the part below privately by mistake: On Tue, Aug 25, 2015 at 11:07 PM, Matthieu Moy wrote: > > Now, I found another issue: I still have this "interpret-trailers" in my > hooks/commit-msg, and it behaves badly when I use "git commit -v". With > -v, I get a diff in COMMIT_EDITMSG, and interpret-trailers tries to > insert my Sign-off within the diff, like this: > > # Do not touch the line above. > # Everything below will be removed. > diff --git a/git-multimail/README b/git-multimail/README > index f41906b..93d4751 100644 > > Signed-off-by: Matthieu Moy > --- a/git-multimail/README > +++ b/git-multimail/README This might be a bug. I will have a look. > Either commit-msg should be called after stripping the diff from > COMMIT_MSG, or interpret-trailers should learn to stop reading when the > patch starts. There is already code to detect a patch in interpret-trailers, but it relies on the patch starting with a line with only three dashes. So another option would be to make "commit -v" emit a line with three dashes just under the "# Everything below will be removed." line. > I think the first option is better, since it means that > any commit-msg hook does not have to deal with the patch stuff (my guess > is that there are many broken commit-msg hooks out there, but people > didn't notice because they don't use "commit -v"). Maybe. I don't know if there is a reason why the commit-msg is called before removing the patch. On Wed, Aug 26, 2015 at 8:28 AM, Jacob Keller wrote: > > It's always confused me why commit -v doesn't prepend every inserted > line with "#" to mark it as a comment. I think that would make interpret-trailers work properly too. Thanks both, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak writes:
> On Wed, Aug 26, 2015 at 12:46 AM, Junio C Hamano wrote:
>> Karthik Nayak writes:
>>
>> I didn't check how wide the original is supposed to be, but perhaps
>> changing builtin/tag.c this way
>>
>> if (filter->lines)
>> - format = "%(align:16,left)%(refname:short)%(end)";
>> + format = "%(align:15,left)%(refname:short)%(end) ";
>> else
>> format = "%(refname:short)";
>> }
>>
>> may be one way to fix it. Check the original "tag -l" output for
>> tags whose names are 14, 15, 16 and 17 display-columns wide and try
>> to match it.
>
> That should be the fix, since it's a space problem.
>
> The problem with doing this is, the lines need to be displayed
> immediately after the refname,
> followed by a newline, what you're suggesting breaks that flow.
That is only because show_ref_array_item() does too much. The
function is given a placeholder string and a set of data to fill the
placeholder with for an item, and the reason why the caller
repeatedly calls it, once per item it has, is because it wants to
produce a one-line-per-item output. An alternative valid way to
structure the API is to have it format into a string and leave the
printing to the caller. You can give a new format_ref_array_item()
that does not print but fills a strbuf to this caller, make
show_ref_array_item() a thin wrapper that calls it and prints it
with the final LF for other callers.
Another alternate approach, if you want to give "tag -l" annotation
available to for-each-ref, would be to do this:
if (filter->lines)
format = xstrfmt("%%(align:15,left)%%(refname:short)%%(end) "
"%%(contents:lines=%s)", filter->lines);
else
format = "%(refname:short)";
and teach a new %(contents:lines=1) atom. That way, you can lose
the ugly special case call to show_tag_lines() that can only be at
the end of the output. I somehow think this is a better approach.
Of course you can (and probably would want to) do both, giving a
bit lower level "emit to a strbuf" function to the callers _and_
losing hardcoded call to show_tag_lines().
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak writes: > On Tue, Aug 25, 2015 at 4:05 AM, Junio C Hamano wrote: >> Junio C Hamano writes: >> >>> Karthik Nayak writes: >>> On Mon, Aug 24, 2015 at 10:57 PM, Junio C Hamano wrote: > Karthik Nayak writes: > ... >> + performed. If used with '--quote' everything in between %(align:..) >> + and %(end) is quoted. ... I might have misunderstood you, But based on the discussion held here (thread.gmane.org/gmane.comp.version-control.git/276140) I thought we wanted everything inside the %(align) %(end) atoms to be quoted. >>> >>> Perhaps I misunderstood your intention in the doc. >>> >>> I took "everything in between %(align:...) and %(end) is quoted" to >>> mean that >>> >>> %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) >>> >>> can never satisfy %(if:empty), because %(align)%(end) would expand >>> to a string that has two single-quotes, that is not an empty string. >>> >>> If that is not what would happen in the "branch --list" enhancement, >>> then the proposed behaviour is good, but the above documentation would >>> need to be updated when it happens, I think. It at least is misleading. >> >> OK, now I checked the code, and I _think_ the recursive logic is >> doing the right thing (modulo minor nits on comment-vs-code >> discrepancy and code structure I sent separately). >> > > For the current code %(if:empty)%(align)%(end)%(then)Empty%(else)Not > Empty%(end) > would print non-empty, I guess the documentation holds in that case. > Not sure if we require it to print non-empty. You don't want the %(if) condition to depend on whether --shell/--python/... is used. Since %(if:empty)%(align)%(end)%(then) holds when you don't use --shell, you also want it to hold when you quote. IOW, you should check for emptyness before (or actually without) doing the quoting. I guess this is what you're doing, and if so, I think it's "The Right Thing". -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 August 2015 at 19:24, Luke Diamand wrote: > On 25/08/15 14:14, Lars Schneider wrote: >>> >>> >>> So the choices are: >>> >>> 1. A new command-line option which would silently set core.ignorecase >>> 2. Users just have to know to set core.ignorecase manually before >>> using git-p4 (i.e. Lars' patch v5) >>> 3. Fix fast-import to take a --casefold option (but that's a much bigger >>> change) >>> >> I vote for 2 because that solves the problem consistently with the >> existing implementation for now. That means we don’t surprise git-p4 users. >> In addition I would try to fix (3), the —casefold option, in a separate >> patch. Although this (3) patch could take a bit as I have two more git-p4 >> patches in the queue that I want to propose to the mailing list first. > > > That works for me. Ack. > > Thanks! > Luke Lars - could you resubmit PATCHv5 as v6, with Acked-by from me added after the Signed-off-by: line. It then this from SubmittingPatches: > After the list reached a consensus that it is a good idea to apply the > patch, re-send it with "To:" set to the maintainer [*1*] and "cc:" the > list [*2*] for inclusion. Thanks Luke -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
On Tue, Aug 25, 2015 at 4:05 AM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Karthik Nayak writes: >> >>> On Mon, Aug 24, 2015 at 10:57 PM, Junio C Hamano wrote: Karthik Nayak writes: ... > + performed. If used with '--quote' everything in between %(align:..) > + and %(end) is quoted. >>> ... >>> I might have misunderstood you, But based on the discussion held here >>> (thread.gmane.org/gmane.comp.version-control.git/276140) >>> I thought we wanted everything inside the %(align) %(end) atoms >>> to be quoted. >> >> Perhaps I misunderstood your intention in the doc. >> >> I took "everything in between %(align:...) and %(end) is quoted" to >> mean that >> >> %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) >> >> can never satisfy %(if:empty), because %(align)%(end) would expand >> to a string that has two single-quotes, that is not an empty string. >> >> If that is not what would happen in the "branch --list" enhancement, >> then the proposed behaviour is good, but the above documentation would >> need to be updated when it happens, I think. It at least is misleading. > > OK, now I checked the code, and I _think_ the recursive logic is > doing the right thing (modulo minor nits on comment-vs-code > discrepancy and code structure I sent separately). > For the current code %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) would print non-empty, I guess the documentation holds in that case. Not sure if we require it to print non-empty. -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-send-email: Delete additional From message body
Additional From added to message body if git-send-email run
with --from parameters
Signed-off-by: Brilliantov Kirill Vladimirovich
---
git-send-email.perl | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index b660cc2..92ec74a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1522,24 +1522,21 @@ foreach my $t (@files) {
$subject = quote_subject($subject, $auto_8bit_encoding);
}
- if (defined $sauthor and $sauthor ne $sender) {
- $message = "From: $author\n\n$message";
- if (defined $author_encoding) {
- if ($has_content_type) {
- if ($body_encoding eq $author_encoding) {
- # ok, we already have the right encoding
- }
- else {
- # uh oh, we should re-encode
- }
+ if (defined $author_encoding) {
+ if ($has_content_type) {
+ if ($body_encoding eq $author_encoding) {
+ # ok, we already have the right encoding
}
else {
- $xfer_encoding = '8bit' if not defined
$xfer_encoding;
- $has_content_type = 1;
- push @xh,
- "Content-Type: text/plain;
charset=$author_encoding";
+ # uh oh, we should re-encode
}
}
+ else {
+ $xfer_encoding = '8bit' if not defined $xfer_encoding;
+ $has_content_type = 1;
+ push @xh,
+ "Content-Type: text/plain; charset=$author_encoding";
+ }
}
if (defined $target_xfer_encoding) {
$xfer_encoding = '8bit' if not defined $xfer_encoding;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Porting builtin/branch.c to use the printing options of ref-filter.{c,h}
Karthik Nayak writes: > It's more than just colors. The whole format changes. > > $ git branch -a > For local: > "%(if)%(HEAD)%(then)%(HEAD) %(color:green)%(refname:short)%(else) > %(refname:short)%(end)" > For remote: > " remotes/%(color:red)%(refname:short)%(color:reset)%(if)%(symref)%(then) > -> %(symref:short)%(end)" I think both versions are not so different. You have %(if) on one format strings that would be disabled by construction on the second. For example, adding %(if)%(HEAD)%(then)%(HEAD) at the start of the format-string for remotes would be a no-op, right? And in case a local branch is a symref, "git branch" displays "-> ..." both for local and for remotes. You just normally don't have local symref branches other than HEAD, but I tried: $ git checkout -b branch $ cat .git/HEAD > .git/refs/heads/symref $ git branch -a * branch master symref -> branch The only remaining difference I see are the "remotes/" prefix and colors. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
