[PATCH 1/5] FIXUP submodule: implement `module_clone` as a builtin helper
This closes the memory leaks as pointed out by Jeff.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/submodule--helper.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ae74b80..7e298b4 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -220,7 +220,6 @@ static int module_clone(int argc, const char **argv, const
char *prefix)
strbuf_addf(sb, %s/modules/%s, get_git_dir(), name);
sm_gitdir = strbuf_detach(sb, NULL);
- strbuf_reset(sb);
if (!file_exists(sm_gitdir)) {
safe_create_leading_directories_const(sm_gitdir);
@@ -259,12 +258,16 @@ static int module_clone(int argc, const char **argv,
const char *prefix)
/* Redirect the worktree of the submodule in the superprojects config */
if (!is_absolute_path(sm_gitdir)) {
char *s = (char*)sm_gitdir;
- strbuf_addf(sb, %s/%s, xgetcwd(), sm_gitdir);
+ if (strbuf_getcwd(sb))
+ die_errno(unable to get current working directory);
+ strbuf_addf(sb, /%s, sm_gitdir);
sm_gitdir = strbuf_detach(sb, NULL);
- strbuf_reset(sb);
free(s);
}
- strbuf_addf(sb, %s/%s, xgetcwd(), path);
+
+ if (strbuf_getcwd(sb))
+ die_errno(unable to get current working directory);
+ strbuf_addf(sb, /%s, path);
p = git_pathdup_submodule(path, config);
if (!p)
--
2.5.0.400.gff86faf
--
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 3/5] submodule: helper to run foreach in parallel
This runs a command on each submodule in parallel and should eventually
replace `git submodule foreach`.
There is a new option -j/--jobs (inspired by make) to specify the number
of parallel threads.
The jobs=1 case needs to be special cases to exactly replicate the current
default behavior of `git submodule foreach` such as working stdin input.
For more than one job there is no input possible and the output of both
stdout/stderr of the command are put into the stderr in an ordered fashion,
i.e. the tasks to not intermingle their output in races.
what currently works:
git submodule--helper foreach echo \$toplevel-\$name-\$path-\$sha1
which I took for testing during development from t7407.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/submodule--helper.c | 148 +++-
git-submodule.sh| 9 +++
2 files changed, 155 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7e298b4..adfa0e4 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 semaphore.h
+#endif
static const struct cache_entry **ce_entries;
static int ce_alloc, ce_used;
static const char *alternative_path;
@@ -278,6 +281,144 @@ 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;
+ struct submodule_output *out;
+ sem_t *mutex;
+};
+
+int run_cmd_submodule(struct task_queue *aq, void *task)
+{
+ int i;
+ 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));
+ if (len 0)
+ die(Read from child failed);
+ else if (len == 0)
+ break;
+ else {
+ strbuf_add(out, buf, len);
+ }
+ }
+ if (finish_command(cp))
+ die(command died with error);
+
+ sem_wait(args-mutex);
+ fputs(out.buf, stderr);
+ sem_post(args-mutex);
+
+ return 0;
+}
+
+int module_foreach_parallel(int argc, const char **argv, const char *prefix)
+{
+ int i, recursive = 0, number_threads = 1, quiet = 0;
+ static struct pathspec pathspec;
+ struct strbuf sb = STRBUF_INIT;
+ struct task_queue *aq;
+ char **cmd;
+ const char **nullargv = {NULL};
+ sem_t *mutex = xmalloc(sizeof(*mutex));
+
+ struct option module_update_options[] = {
+ OPT_STRING(0, prefix, alternative_path,
+ N_(path),
+ N_(alternative anchor for relative paths)),
+ OPT_STRING(0, cmd, cmd,
+ N_(string),
+ N_(command to run)),
+ OPT_BOOL('r', --recursive, recursive,
+N_(Recurse into nexted submodules)),
+ OPT_INTEGER('j', jobs, number_threads,
+ N_(Recurse into nexted submodules)),
+ OPT__QUIET(quiet, N_(Suppress output)),
+ OPT_END()
+ };
+
+ static const char * const git_submodule_helper_usage[] = {
+ N_(git submodule--helper foreach [--prefix=path]
[path...]),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, module_update_options,
+git_submodule_helper_usage, 0);
+
+ if (module_list_compute(0, nullargv, NULL, pathspec) 0)
+ return 1;
+
+ gitmodules_config();
+
+ aq =
Re: [PATCH 5/8] checkout(-index): do not checkout i-t-a entries
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
The cached blob of i-t-a entries are empty blob. By checkout, we delete
the content we have. Don't do it.
This is done higher up instead of inside checkout_entry() because we
would have limited options in there: silently ignore, loudly ignore,
die. At higher level we can do better reporting. For example, git
checkout -- foo will complain that foo does not match pathspec, just
like when foo is not registered with git add -N
Hmm, I am not sure about the example, though. The user _told_ Git
that 'foo' is a path she cares about so does not match pathspec is
a very unfriendly thing to say. foo does not exist in the index
hence we cannot check it out is a very good thing to say, though
(and of course, I agree that not touching the working tree is a good
thing to do).
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/checkout-index.c | 5 -
builtin/checkout.c | 2 ++
t/t2203-add-intent.sh| 34 ++
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 8028c37..eca975d 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -56,7 +56,8 @@ static int checkout_file(const char *name, const char
*prefix)
while (pos active_nr) {
struct cache_entry *ce = active_cache[pos];
if (ce_namelen(ce) != namelen ||
- memcmp(ce-name, name, namelen))
+ memcmp(ce-name, name, namelen) ||
+ ce_intent_to_add(ce))
break;
has_same_name = 1;
pos++;
'pos' here comes from cache_name_pos(), and it is either the
location an entry with the same name exists, or the location a new
entry with the given name would be inserted at.
When we detect that the entry at pos is different from the given
name, we break out of the loop, because we _know_ that an entry with
the same name cannot exist. Does the same hold for an i-t-a entry
that exactly records the given name?
The answer is yes, as you cannot have an unmerged i-t-a entry. If a
path marked as i-t-a is in the index, no other entry with the same
name would exist.
So this change is good. I would actually have introduced another
bool has_i_t_a to make the reporting richer, as it is your primary
reason why you are touching this part of the code instead of
checkout_entry(). The reporting part then would become
fprintf(stderr, git checkout-index: %s , name);
- if (!has_same_name)
+ if (has_i_t_a)
+ fprintf(stderr, is not yet in the cache);
else if (!has_same_name)
fprintf(stderr, is not in the cache);
;-)
@@ -99,6 +100,8 @@ static void checkout_all(const char *prefix, int
prefix_length)
if (ce_stage(ce) != checkout_stage
(CHECKOUT_ALL != checkout_stage || !ce_stage(ce)))
continue;
+ if (ce_intent_to_add(ce))
+ continue;
This one is obviously good ;-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e1403be..02889d4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -300,6 +300,8 @@ static int checkout_paths(const struct checkout_opts
*opts,
* anything to this entry at all.
*/
continue;
+ if (ce_intent_to_add(ce))
+ continue;
/*
* Either this entry came from the tree-ish we are
* checking the paths out of, or we are checking out
Hmm, while this does prevent the later code from checking it out, I
am not sure how well this interacts with ps_matched[] logic here.
If the user told Git that 'foo' is a path that she cares about with
add -N foo, and said git checkout -- foo, should we be somehow
saying that 'foo' did match but there is nothing to check out, or
something?
--
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 sbel...@google.com
---
run-command.c | 29 ---
thread-utils.c | 237 +
thread-utils.h | 40 ++
3 files changed, 294 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..936b3672 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 sys/pstat.h
@@ -75,3 +77,238 @@ 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 {
+ /*
+* To avoid deadlocks always acquire the semaphores with lowest priority
+* first, priorites are in descending order as listed.
+*
+* The `mutex` is a general purpose lock for modifying data in the async
+* queue, such as adding a new task or adding a return value from
+* an already run task.
+*
+* `workingcount` and `freecount` are opposing semaphores, the sum of
+* their values should equal `max_threads` at any time while the `mutex`
+* is available.
+*/
+ sem_t mutex;
+ sem_t workingcount;
+ sem_t freecount;
+
+ pthread_t *threads;
+ unsigned max_threads;
+
+ struct job_list *first;
+ struct job_list *last;
+
+ 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;
+
+ sem_wait(tq-workingcount);
+ sem_wait(tq-mutex);
+
+ if (*early_return) {
+ tq-early_return |= *early_return;
+ *fct = NULL;
+ *task = NULL;
+ } else {
+ if (!tq-first)
+ die(BUG: internal error with dequeuing jobs
[PATCH 5/5] pack-objects: Use new worker pool
Before we had n 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 useful.
Signed-off-by: Stefan Beller sbel...@google.com
---
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 */
- return NULL;
+
+ return 0;
}
static void
[PATCH 4/5] index-pack: Use the new worker pool
By treating each object as its own task the workflow is easier to follow
as the function used in the worker threads doesn't need any control logic
any more.
Signed-off-by: Stefan Beller sbel...@google.com
---
builtin/index-pack.c | 71 +++-
1 file changed, 32 insertions(+), 39 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3f10840..826bd22 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -11,6 +11,7 @@
#include exec_cmd.h
#include streaming.h
#include thread-utils.h
+#include run-command.h
static const char index_pack_usage[] =
git index-pack [-v] [-o index-file] [--keep | --keep=msg] [--verify]
[--strict] (pack-file | --stdin [--fix-thin] [pack-file]);
@@ -95,7 +96,6 @@ static const char *curr_pack;
#ifndef NO_PTHREADS
static struct thread_local *thread_data;
-static int nr_dispatched;
static int threads_active;
static pthread_mutex_t read_mutex;
@@ -106,10 +106,6 @@ static pthread_mutex_t counter_mutex;
#define counter_lock() lock_mutex(counter_mutex)
#define counter_unlock() unlock_mutex(counter_mutex)
-static pthread_mutex_t work_mutex;
-#define work_lock()lock_mutex(work_mutex)
-#define work_unlock() unlock_mutex(work_mutex)
-
static pthread_mutex_t deepest_delta_mutex;
#define deepest_delta_lock() lock_mutex(deepest_delta_mutex)
#define deepest_delta_unlock() unlock_mutex(deepest_delta_mutex)
@@ -140,7 +136,6 @@ static void init_thread(void)
int i;
init_recursive_mutex(read_mutex);
pthread_mutex_init(counter_mutex, NULL);
- pthread_mutex_init(work_mutex, NULL);
pthread_mutex_init(type_cas_mutex, NULL);
if (show_stat)
pthread_mutex_init(deepest_delta_mutex, NULL);
@@ -163,7 +158,6 @@ static void cleanup_thread(void)
threads_active = 0;
pthread_mutex_destroy(read_mutex);
pthread_mutex_destroy(counter_mutex);
- pthread_mutex_destroy(work_mutex);
pthread_mutex_destroy(type_cas_mutex);
if (show_stat)
pthread_mutex_destroy(deepest_delta_mutex);
@@ -181,9 +175,6 @@ static void cleanup_thread(void)
#define counter_lock()
#define counter_unlock()
-#define work_lock()
-#define work_unlock()
-
#define deepest_delta_lock()
#define deepest_delta_unlock()
@@ -1075,28 +1066,29 @@ 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 (;;) {
- int i;
- counter_lock();
- display_progress(progress, nr_resolved_deltas);
- counter_unlock();
- work_lock();
- while (nr_dispatched nr_objects
- is_delta_type(objects[nr_dispatched].type))
- nr_dispatched++;
- if (nr_dispatched = nr_objects) {
- work_unlock();
- break;
- }
- i = nr_dispatched++;
- work_unlock();
+ if (!get_thread_data()) {
+ struct thread_local *t = xmalloc(sizeof(*t));
+ t-pack_fd = open(curr_pack, O_RDONLY);
+ if (t-pack_fd == -1)
+ die_errno(_(unable to open %s), curr_pack);
- resolve_base(objects[i]);
+ set_thread_data(t);
}
- return NULL;
+
+ resolve_base(data);
+
+ counter_lock();
+ display_progress(progress, nr_resolved_deltas);
+ counter_unlock();
+ return 0;
+}
+
+void cleanup_threaded_second_pass(struct task_queue *aq)
+{
+ struct thread_local *t = get_thread_data();
+ free(t);
}
#endif
@@ -1195,18 +1187,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;
+
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));
- }
- for (i = 0; i nr_threads; i++)
- pthread_join(thread_data[i].thread, NULL);
+ tq = create_task_queue(nr_threads);
+
+ for (i = 0; i nr_objects; i++)
+ if (!is_delta_type(objects[i].type))
+ add_task(tq, threaded_second_pass, objects[i]);
+
+ if (finish_task_queue(tq, cleanup_threaded_second_pass))
+
[RFC PATCH 0/5] Demonstrate new parallel threading API
This series build on top of origin/sb/submodule-helper. The first patch is a fixup to the last commit in the target branch to fix a memory leak. The patch is not really part of the series, but as I chose to build on top of that series I can fix it up as we go. The patch 2 adds a new API to easily use a threaded work pool. patch 3 adds the command `submodule foreach_parallel` (completely untested, RFC!) which was the original goal of this series. But oh well, I got tricked by Peff to prove how awesome the new API for parallel threading is, so that's shown off in patches 4 and 5. Note: Both patch 4 and 5 delete more lines of code than they add, improving readability a lot as you can focus on the actual task and not on the threading stuff. Any feedback welcome! Thanks, Stefan Stefan Beller (5): FIXUP 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| 71 ++--- builtin/pack-objects.c | 175 +++- builtin/submodule--helper.c | 159 +++-- git-submodule.sh| 9 ++ run-command.c | 29 +++--- thread-utils.c | 237 thread-utils.h | 40 7 files changed, 545 insertions(+), 175 deletions(-) -- 2.5.0.400.gff86faf -- 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 6/8] grep: make it clear i-t-a entries are ignored
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
The expression !S_ISREG(ce) covers i-t-a entries as well because
ce-ce_mode would be zero then. I could make a comment saying that, but
it's probably better just to comment with code, in case i-t-a entry
content changes in future.
OK. Thansk.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/grep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..f508179 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -375,7 +375,7 @@ static int grep_cache(struct grep_opt *opt, const struct
pathspec *pathspec, int
for (nr = 0; nr active_nr; nr++) {
const struct cache_entry *ce = active_cache[nr];
- if (!S_ISREG(ce-ce_mode))
+ if (!S_ISREG(ce-ce_mode) || ce_intent_to_add(ce))
continue;
if (!ce_path_match(ce, pathspec, NULL))
continue;
--
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 6/8] grep: make it clear i-t-a entries are ignored
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
The expression !S_ISREG(ce) covers i-t-a entries as well because
ce-ce_mode would be zero then. I could make a comment saying that, but
it's probably better just to comment with code, in case i-t-a entry
content changes in future.
OK. Thanks.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/grep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index d04f440..f508179 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -375,7 +375,7 @@ static int grep_cache(struct grep_opt *opt, const struct
pathspec *pathspec, int
for (nr = 0; nr active_nr; nr++) {
const struct cache_entry *ce = active_cache[nr];
- if (!S_ISREG(ce-ce_mode))
+ if (!S_ISREG(ce-ce_mode) || ce_intent_to_add(ce))
continue;
if (!ce_path_match(ce, pathspec, NULL))
continue;
--
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 v3] gc: save log from daemonized gc --auto and print it next time
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
While commit 9f673f9 (gc: config option for running --auto in
background - 2014-02-08) helps reduce some complaints about 'gc
--auto' hogging the terminal, it creates another set of problems.
The latest in this set is, as the result of daemonizing, stderr is
closed and all warnings are lost. This warning at the end of cmd_gc()
is particularly important because it tells the user how to avoid gc
--auto running repeatedly. Because stderr is closed, the user does
not know, naturally they complain about 'gc --auto' wasting CPU.
Daemonized gc now saves stderr to $GIT_DIR/gc.log. Following gc runs
will not be daemonized and gc.log printed out until the user removes
gc.log.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Let's try again. Compared to v2 [1], this version does not delete
gc.log automatically any more. The user needs to take action, then
delete gc.log to bring it background again.
Sounds a bit more sensible, but I wonder if it is a good idea to
keep going in the first place. If the last gc run reported an
issue, would it make it likely that we would hit the same issue?
An alternative design I have in mind is to exit gc --auto with
success without doing anything, after giving the new warning
message. What would be the pros-and-cons between this patch and
that alternative design?
[1]
http://thread.gmane.org/gmane.comp.version-control.git/266182/focus=266320
builtin/gc.c | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index bcc75d9..00a83e1 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -43,9 +43,20 @@ static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
static struct argv_array rerere = ARGV_ARRAY_INIT;
static char *pidfile;
+static struct strbuf log_filename = STRBUF_INIT;
+static int daemonized;
static void remove_pidfile(void)
{
+ if (daemonized log_filename.len) {
+ struct stat st;
+
+ close(2);
+ if (stat(log_filename.buf, st) ||
+ !st.st_size ||
+ rename(log_filename.buf, git_path(gc.log)))
+ unlink(log_filename.buf);
+ }
if (pidfile)
unlink(pidfile);
}
@@ -330,13 +341,24 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
fprintf(stderr, _(See \git help gc\ for manual
housekeeping.\n));
}
if (detach_auto) {
+ struct strbuf sb = STRBUF_INIT;
+ if (strbuf_read_file(sb, git_path(gc.log), 0) 0) {
+ warning(_(last gc run reported:\n
+ %s\n
+ running in foreground until %s is
removed),
+ sb.buf, git_path(gc.log));
+ detach_auto = 0;
+ }
+ strbuf_release(sb);
+ }
+ if (detach_auto) {
if (gc_before_repack())
return -1;
/*
* failure to daemonize is ok, we'll continue
* in foreground
*/
- daemonize();
+ daemonized = !daemonize();
}
} else
add_repack_all_option();
@@ -349,6 +371,18 @@ int cmd_gc(int argc, const char **argv, const char
*prefix)
name, (uintmax_t)pid);
}
+ if (daemonized) {
+ int fd;
+
+ strbuf_addstr(log_filename, git_path(gc.log_XX));
+ fd = xmkstemp(log_filename.buf);
+ if (fd = 0) {
+ dup2(fd, 2);
+ close(fd);
+ } else
+ strbuf_release(log_filename);
+ }
+
if (gc_before_repack())
return -1;
--
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 04/12] ref-filter: implement an `align` atom
Matthieu Moy matthieu@grenoble-inp.fr writes:
Junio C Hamano gits...@pobox.com writes:
You can see that I expected that if !state.stack-prev check to be
inside append_atom(), and I would imagine future readers would have
the same expectation when reading this code. I.e.
append_atom(struct atom_value *v, struct ref_f_s *state)
{
if (state-stack-prev)
strbuf_addstr(state-stack-output, v-s);
else
quote_format(state-stack-output, v-s,
state-quote_style);
}
The end result may be the same,
There's another call to append_atom() when inserting the reset color at
end of line if needed, so moving this if inside append_atom means we
would do the check also for the reset color. It would not change the
behavior (by construction, we insert it only when the stack has only the
initial element), so it's OK.
Thanks for checking---I did overlook that other callsite and did not
check if the suggested change was sensible there.
--
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 04/12] ref-filter: implement an `align` atom
Karthik Nayak karthik@gmail.com writes: I like the idea of using atomv-handler() a lot, mostly cause this would eventually help us clean up populate_atom() which currently seems like a huge dump of code. I think you already said that last time we had this discussion ;-) http://thread.gmane.org/gmane.comp.version-control.git/275537/focus=275778 -- 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/8] apply: fix adding new files on i-t-a entries
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Applying a patch that adds a file when that file is registered with git add -N will fail with message already exists in index because git-apply checks, sees those i-t-a entries and aborts. git-apply does not realize those are for bookkeeping only, they do not really exist in the index. This patch tightens the exists in index check, ignoring i-t-a entries. Suppose that you came up with some contents to register at path F in your working tree, told Git about your intention with add -N F, and then tried to apply a patch that wants to _create_ F: Without this patch, we would say F already exists so a patch to create is incompatible with our current state. With this patch, we do not say that, but instead we'll hopefully trigger does it exist in the working tree check, unless you are running under --cached. Which means that this change will not lead to data loss in the untracked file F in the working tree that was merely added to the index with i-t-a bit. As you did not say the equivalent of the last paragraph above in the log message, my knee-jerk reaction was why could this possibly be a good idea? Please try again with a better log message. 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/6] builtin/am: make sure state files are text
Jeff King p...@peff.net writes: On Mon, Aug 24, 2015 at 01:58:06PM -0700, Junio C Hamano wrote: We forgot to terminate the payload given to write_file() with LF, resulting in files that end with an incomplete line. Teach the wrappers builtin/am uses to make sure it adds LF at the end as necessary. Is it even worth doing this step? It's completely reverted later in the series. I understand that we do not want to hold the fix to git-am hostage to write_file refactoring, but I don't see any reason these cannot all graduate as part of the same topic. Ignore me if you really are planning on doing the first two to maint and holding the others back for master. Not really. The primary reason why this step exists and 1-2/6 make a sufficient fix by themselves is because I wasn't even sure if 3-6/6 were worth doing. As to flags exposed to callers vs with and without gently, when we change the system to allow new modes of operations (e.g. somebody wants to write a binary file, or allocate more flag bits for their special case), I'd expect that we'd add a more general and verbose write_file_with_options(path, flags, fmt, ...)), gain experience with that function, and then possibly introduce canned thin wrappers (e.g. write_binary_file() that is a synonym to passing BINARY but not GENTLY) if the new thing proves widely useful, just like I left write_file() and write_file_gently() in as fairly common things to do. -- 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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
On Tue, Aug 25, 2015 at 05:01:01PM +0200, Gabor Bernat wrote: So it would be great if the filter-branch beside the Rewrite f8f0b351ae35ff7ac4bd58078cbba1aa34243779 (523/22625), would also append a basic ETA signaling the end of the operation. It could be as simple as the the average number of milliseconds per step up to this point multiplied with the remaining number of steps, then convert this into a day:hour:minutes:seconds format. It sound simple enough, but really handy for long running filter branch operations. I could also contribute if one could direct me towards the appropriate files this should go to. Yeah, I agree the current filter-branch progress reporting is pretty simplistic. The line you want to tweak is in git-filter-branch.sh: printf \rRewrite $commit ($git_filter_branch__commit_count/$commits) But the real trick is getting accurate timing in a shell script. You can probably do the math within a $(()) arithmetic block if you're OK with integers. But you'd have to run `date` on each loop iteration to get the current time, which may have a noticeable speed impact. Of course, filter-branch is so slow in the first place, maybe it would not matter. :) Something like this seems to work: diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 5b3f63d..04e45bc 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -276,10 +276,21 @@ test $commits -eq 0 die Found nothing to rewrite # Rewrite the commits +start=$(date +%s) git_filter_branch__commit_count=0 while read commit parents; do git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1)) - printf \rRewrite $commit ($git_filter_branch__commit_count/$commits) + now=$(date +%s) + elapsed=$(($now - $start)) + # work in integer percentages as a sort of fixed-point + pct=$(($git_filter_branch__commit_count * 100 / $commits)) + if test $pct -eq 0; then + remain= + else + eta=$(($elapsed * 100 / $pct)) + remain=($(($eta - $elapsed)) seconds remaining) + fi + printf \rRewrite $commit ($git_filter_branch__commit_count/$commits) $remain case $filter_subdir in ) but the time jumps around early on because of the lack of precision. And of course there's no smoothing, and no emphasis on recent history versus the whole operation. I'll leave those as an exercise to the reader. :) -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 7/8] diff.h: extend flags field to 64 bits because we're out of bits
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: I renamed both flags and touched_flags fields while making this patch to make sure I was aware of how these flags were manipulated (besides DIFF_OPT* macros). So hopefully I didn't miss anything. It is a bad taste to use user_defined_t typedef (I think it actually is a standard violation), isn't it? The diff-struct is not like objects where we need million copies of in-core while running. What do you need many more flags for? -- 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
[RFC] Porting builtin/branch.c to use the printing options of ref-filter.{c,h}
I'm working on porting over the printing options of ref-filter to `git branch -l`. This is a follow up to http://article.gmane.org/gmane.comp.version-control.git/276377 Theres a slight issue with this which I'd like to discuss about. When we use `-a` option in `git branch -l` It lists local branches and remotes with each having a different output format. This with reference to ref-filter would mean either 1. Change format in between local and remote refs 2. Have a format like %(if)...local check%(then)local branch format%(else)remote format %(end). I prefer the latter, but that might lead to a large format. Any suggestions? -- 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: index file list files not found in working tree
On 08/25/2015 12:32 PM, John Keeping wrote: On Tue, Aug 25, 2015 at 12:16:43PM +0200, Rafik E Younan wrote: I got a recommendation to use reset --hard. I tried it and it says the HEAD is now at correct commit, but missing files are not restored! I tried `ls-tree --name-only` and it lists missing files and folders, but the actual working tree doesn't have these files and folders. Is there any chance that you have enabled sparse checkouts? See the documentation at the bottom of git-read-tree(1). There was `.git/info/sparse-checkout` file in the local repo, but not in other cloned repo. I remove the file, the `.git/index` and did reset --hard, and all seems works now. Thanks for your help. Git community rocks! Regards, Rafik -- 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 Tue, Aug 25, 2015 at 1:01 AM, Graeme Geldenhuys grae...@gmail.com wrote: Even though I have worked with Git since 2009, I still have to reference the help to remind me of what parameter to use in certain situation simply because similar tasks differ so much. Maybe we could address this in the next major version of Git? Has anybody else thought about this or started work on this? Or was this discussed before and declined (link?). http://article.gmane.org/gmane.comp.version-control.git/231478 comes to mind, which has been linked from this entry: Discuss and decide if we want to choose between the mode word UI (e.g. git submodule add) and the mode option UI (e.g. git tag --delete) and standardise on one; if it turns out to be a good idea, devise the migration plan to break the backward-compatibility. in http://git-blame.blogspot.com/p/leftover-bits.html -- 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
[FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
Hello, So it would be great if the filter-branch beside the Rewrite f8f0b351ae35ff7ac4bd58078cbba1aa34243779 (523/22625), would also append a basic ETA signaling the end of the operation. It could be as simple as the the average number of milliseconds per step up to this point multiplied with the remaining number of steps, then convert this into a day:hour:minutes:seconds format. It sound simple enough, but really handy for long running filter branch operations. I could also contribute if one could direct me towards the appropriate files this should go to. Thanks, Bernát GÁBOR -- 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] setup: update the right file in multiple checkouts
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: This code is introduced in 23af91d (prune: strategies for linked checkouts - 2014-11-30), and it's supposed to implement this rule from that commit's message: - linked checkouts are supposed to keep its location in $R/gitdir up to date. The use case is auto fixup after a manual checkout move. Note the name, $R/gitdir, not $R/gitfile. Correct the path to be updated accordingly. While at there, make sure I/O errors are not silently dropped. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- The code was right in v2 [1] and became gitfile since v3 [2]. I need to reconsider my code quality after this :( Heh, don't sweat it. Everybody makes mistakes and sometimes becomes sloppy. Thanks for double checking and correcting. Perhaps this could have caught if we had some test coverage, I wonder? [1] http://article.gmane.org/gmane.comp.version-control.git/239299 [2] http://article.gmane.org/gmane.comp.version-control.git/242325 setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.c b/setup.c index 5f9f07d..64bf2b4 100644 --- a/setup.c +++ b/setup.c @@ -402,9 +402,9 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir) struct strbuf path = STRBUF_INIT; struct stat st; - strbuf_addf(path, %s/gitfile, gitdir); + strbuf_addf(path, %s/gitdir, gitdir); if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL)) - write_file(path.buf, 0, %s\n, gitfile); + write_file(path.buf, 1, %s\n, gitfile); strbuf_release(path); } -- 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/6] builtin/am: make sure state files are text
On Tue, Aug 25, 2015 at 09:19:13AM -0700, Junio C Hamano wrote: As to flags exposed to callers vs with and without gently, when we change the system to allow new modes of operations (e.g. somebody wants to write a binary file, or allocate more flag bits for their special case), I'd expect that we'd add a more general and verbose write_file_with_options(path, flags, fmt, ...)), gain experience with that function, and then possibly introduce canned thin wrappers (e.g. write_binary_file() that is a synonym to passing BINARY but not GENTLY) if the new thing proves widely useful, just like I left write_file() and write_file_gently() in as fairly common things to do. Yeah, that works. It is a bit of a gamble to me. If we never add a lot more options, the end result is much nicer (callers do not deal with the flag option at all). But if we do, we end up with the mess that get_sha1_with_* and add_pending_object() got into. One can always refactor later, too. In that sense, the BINARY flag is not useful (nobody uses it, and we do not plan to do so). We could just make write_file_v unconditionally complete lines[1]. But I'm OK with what you posted, as well. I think this interface is not worth spending a lot of time micro-nit-picking. -Peff [1] In fact, I'd be surprised if this function works well for non-text data anyway, as it relies on printf-style formatting. You cannot use it to write a string with embedded NULs, for example. If we wanted to support that case, we would probably break out: int write_buf_to_file(const char *filename, const char *buf, size_t len); as a thin wrapper for open/write_in_full/close. And then write_to_file() would format into a strbuf, complete a newline, and pass the result to it. -- 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] Documentation: read-tree consistent usage of working tree
Lars Vogel lars.vo...@vogella.com writes: http://git-scm.com/docs/git-clone speaks only about working tree, the usage of working directory for working tree is confusing. Working directory describes the current directory while working tree describes all files and sub directories. Actually I think the $cwd is the Current working directory; the phrase Working directory alone does not specifically refer to $cwd. Being consistent to call the checked out file hierarchy working tree is probably a good idea nevertheless. Signed-off-by: Lars Vogel lars.vo...@vogella.com --- If you apply this patch and then run $ git grep -B1 '^directory' Documentation/git-read-tree.txt you would notice that the patch has missed a few more. Also $ git grep 'work tree' Documentation/git-read-tree.txt has a handful of hits. Perhaps try again? 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 4/8] apply: make sure check_preimage() does not leave empty file on error
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
The test case probably describes the test scenario the best. We have a
patch to modify some file but the base file is gone. Because
check_preimage() finds an index entry with the same old_name, it tries
to restore the on-disk base file with cached content with
checkout_target() and move on.
If this old_name is i-t-a, before this patch apply -3 will call
checkout_target() which will write an empty file (i-t-a entries always
have empty blob SHA-1), then it'll fail at verify_index_match()
(i-t-a entries are always modified) and the operation is aborted.
This empty file should not be created. Compare to the case where
old_name does not exist in index, apply -3 fails with a different
error message ...: does not exist but it does not touch
worktree. This patch makes sure the same happens to i-t-a entries.
This part (unlike 3/8) is very well explained, and makes sense to
me.
load_current() shares the same code pattern (look up an index entry,
if on-disk version is not found, restore using the cached
version).
The fall-back to the cached one is very much deliberate to allow
us to work with an empty working tree as long as the index is
correct, so this perhaps makes sense. If the user said add -N F,
and F is involved in the patch application, we do not want to
consider F exists in the index and we would instead want to pretend
that we do not have F ourselves. It does not even make sense to do
apply -3 for such a path, and rejecting with does not exist is a
good thing to do.
Good.
Fix it too (even though it's not exercised in any test case)
Hmm, as this is adding new test for the other case, perhaps this
case is covered by another one, too?
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/apply.c | 4 ++--
t/t2203-add-intent.sh | 16
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 76b58a1..b185c97 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3348,7 +3348,7 @@ static int load_current(struct image *image, struct
patch *patch)
die(BUG: patch to %s is not a creation, patch-old_name);
pos = cache_name_pos(name, strlen(name));
- if (pos 0)
+ if (pos 0 || ce_intent_to_add(active_cache[pos]))
return error(_(%s: does not exist in index), name);
ce = active_cache[pos];
if (lstat(name, st)) {
@@ -3501,7 +3501,7 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
if (check_index !previous) {
int pos = cache_name_pos(old_name, strlen(old_name));
- if (pos 0) {
+ if (pos 0 || ce_intent_to_add(active_cache[pos])) {
if (patch-is_new 0)
goto is_new;
return error(_(%s: does not exist in index),
old_name);
diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh
index bb5ef2b..96c8755 100755
--- a/t/t2203-add-intent.sh
+++ b/t/t2203-add-intent.sh
@@ -95,5 +95,21 @@ test_expect_success 'apply adds new file on i-t-a entry' '
)
'
+test_expect_success 'apply:check_preimage() not creating empty file' '
+ git init check-preimage
+ (
+ cd check-preimage
+ echo oldcontent newfile
+ git add newfile
+ echo newcontent newfile
+ git diff patch
+ rm .git/index
+ git add -N newfile
+ rm newfile
+ test_must_fail git apply -3 patch
+ ! test -f newfile
+ )
+'
+
test_done
--
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
Fwd: bug: git branch -d and case-insensitive file-systems
On Tue, Aug 25, 2015 at 1:21 AM, Jeff King p...@peff.net wrote:
On Mon, Aug 24, 2015 at 12:11:13PM -0400, Aaron Dufour wrote:
I use git (2.2.1) on OS X (10.9.5) and recently my repo got into a bad
state. I think this involves a mis-handling of case-insensitive file
systems.
This reproduces the problem:
git init
Initialized empty Git repository in
/Users/aarond_local/code/git-test/.git/
git commit --allow-empty -m 'first commit'
[master (root-commit) 923d8b8] first commit
git checkout -b feature
Switched to a new branch 'feature'
git checkout -b Feature
fatal: A branch named 'Feature' already exists.
git checkout -B Feature
Switched to and reset branch 'Feature'
git branch -d feature
Deleted branch feature (was 923d8b8).
git log
fatal: bad default revision 'HEAD'
I don't work on a case-insensitive filesystem, so my knowledge may be
out of date, but as far as I know, we do not do anything special to
handle ref case-sensitivity. I expect your problem would go away with
this patch:
diff --git a/builtin/branch.c b/builtin/branch.c
index 58aa84f..c5545de 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -19,6 +19,7 @@
#include column.h
#include utf8.h
#include wt-status.h
+#include dir.h
static const char * const builtin_branch_usage[] = {
N_(git branch [options] [-r | -a] [--merged | --no-merged]),
@@ -223,7 +224,7 @@ static int delete_branches(int argc, const char **argv,
int force, int kinds,
int flags = 0;
strbuf_branchname(bname, argv[i]);
- if (kinds == REF_LOCAL_BRANCH !strcmp(head, bname.buf)) {
+ if (kinds == REF_LOCAL_BRANCH !strcmp_icase(head,
bname.buf)) {
error(_(Cannot delete the branch '%s'
which you are currently on.), bname.buf);
ret = 1;
but I think that is just the tip of the iceberg. E.g. (on a vfat
filesystem I just created):
$ git init
$ git commit -q --allow-empty -m one
$ git branch foo
$ git branch FOO
fatal: A branch named 'FOO' already exists.
$ git pack-refs --all --prune ;# usually run as part of git-gc
$ git commit -q --allow-empty -m two
$ git branch FOO
$ git for-each-ref --format='%(refname) %(subject)'
refs/heads/FOO two
refs/heads/foo one
refs/heads/master two
Now the patch I showed above would do the wrong thing. Running git
checkout foo; git branch -d FOO would be rejected, even though I really
do have two separate branches.
It would be a much more invasive change to fix this correctly. It is
probably less work overall to move to a pluggable ref system, and to
design ref storage that isn't dependent on the filesystem (this work is
already underway).
That's great news!
In the meantime, I think the best advice for mixed-case branch names on
a case-insensitive filesystem is: don't.
Yeah, that's definitely the solution. I got into a weird place
because our build system uses branch names, but it restricts them to
lowercase letters and I made the mistake of camel-casing. I'll just
be more careful.
-Peff
-Aaron
--
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 Tue, Aug 25, 2015 at 4:43 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: $ git tag --delete master $ echo $? # 0 # actually works as of today! $ git tag delete master # Due to the planned switch to command words, this doesn't work. # For details see road map at `man git commandwords-roadmaps` $ echo $? # 128 maybe ? This is way too aggressive behaviour and is unacceptable as the first step. The first step of a transition that breaks backward compatibility should warn loudly about a command line that would behave differently in the endgame version (either the command line that will not do anything or do a totally different thing), but still perform the operation asked for the current version. e.g. git tag delete master would create a tag named 'delete' out of 'master', but tell the user that this will instead delete 'master' in future versions of Git. git tag create master would create a tag named 'create' out of 'master', but tell the user that this will instead create 'master' out of HEAD in future versions of Git. e.g. git tag -d foo would delete a tag named 'foo', but tell the user that this will have to be spelled 'git tag delete foo' in the future versions of Git. One thing that I am not enthused about the transition plan is that git tag delete master will *never* be an invalid operation during the transition. When making an operation that used to mean one thing to mean something else, a good transition plan should be to * First warn but do the old thing, and tell users a new way to do that in the new world order. At the same time, find the new way that used to be an invalid operation in the old world order, and implement it. * Then stop supporting the old thing and support only the new thing. Then during the transition period, while transitioning to the new way, people can gradually start using the new way with the new system, and when they occasionally have to interact with an old system, the new way will _error out_, because we make sure we find the new way that used to be an invalid operation when planning the whole transition plan, without causing any harm. And once people retrain their finger after 2-3 years, nobody will be hurt if we dropped the old way. 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. So don't hold your breath. What you two are discussing is way too uncooked for 2.6 timeframe. Ya, there isn't really a way to make it work, because we can't exactly stop supporting git tag create master by turning it into a no-op, because there is no equivalent tag option that would work for now. Since there is no alternative syntax for create I think this is the issue. One way might be to use the -- splitter to say, if you really mean to create a tag named create, use git tag -- create master So we'd do: - step 1 - git tag create master # warn that this will change behavior in the future and they must explicitely pass -- before it - step 2 - break create, but don't add anything new. If user really needs it, they can pass git tag -- create master as per above warning, but keep warning on git tag create master to say they must be explicit. - step 3 - implement git tag create master to actually perform tag creation I think this might work, as long as git tag -- create master is acceptable? then, eventually we can make it so that git tag doesn't mean create by default if we ever wanted? How does this sound? By the way, this wouldn't be necessarily done over 2.6 or even over only a single release, I think the time frame would have to be fairly long. The downside is that there is no point where new and old syntax are usable at the same time... but I don't think that will ever be the case. We'd need to way the concern of whether this is actually worth doing to streamline the overall feel at some point in the future or we just live with the warts. 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
Bug report: 'git commit --dry-run' corner case: returns error (nothing to commit) when all conflicts resolved to HEAD
Hi git guys, The bug is fairly simple: if we have a conflicted merge, AND all the conflicts have been resolved to the version in HEAD, the commit --dry-run error code says nothing to commit. As expected, git commit goes through. The commit message IS correct (-ish), just not the error code: All conflicts fixed but you are still merging. (use git commit to conclude merge) nothing to commit, working directory clean The script below demonstrates the problem; version tested was 2.5.0. Let me know if you need any more details. Best, Ovidiu -- #!/bin/bash mkdir test-repository || exit 1 cd test-repository git init echo Initial contents, unimportant test-file git add test-file git commit -m Initial commit echo commit-1-state test-file git commit -m commit 1 -i test-file git tag commit-1 git checkout -b branch-2 HEAD^1 echo commit-2-state test-file git commit -m commit 2 -i test-file # Creates conflicted state. git merge --no-commit commit-1 # Resolved entirely to commit-2, aka HEAD. echo commit-2-state test-file # If we'd set to commit-1=state, all would work as expected (changes vs HEAD). git add test-file # = Bug is here. git commit --dry-run echo Git said something to commit \ || echo Git said NOTHING to commit git commit -m Something to commit after all echo Commit went through git log --pretty=oneline cd .. -- 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 v2 1/2] trailer: ignore first line of message
When looking for the start of the trailers in the message
we are passed, we should ignore the first line of the message.
The reason is that if we are passed a patch or commit message
then the first line should be the patch title.
If we are passed only trailers we can expect that they start
with an empty line that can be ignored too.
This way we can properly process commit messages that have
only one line with something that looks like a trailer, for
example like area of code: change we made.
---
t/t7513-interpret-trailers.sh | 15 ++-
trailer.c | 4 +++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index bd0ab46..9577b4e 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -93,12 +93,25 @@ test_expect_success 'with config option on the command
line' '
Acked-by: Johan
Reviewed-by: Peff
EOF
- echo Acked-by: Johan |
+ { echo; echo Acked-by: Johan; } |
git -c trailer.Acked-by.ifexists=addifdifferent interpret-trailers \
--trailer Reviewed-by: Peff --trailer Acked-by: Johan
actual
test_cmp expected actual
'
+test_expect_success 'with only a title in the message' '
+ cat expected -\EOF
+ area: change
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ echo area: change |
+ git interpret-trailers --trailer Reviewed-by: Peff \
+ --trailer Acked-by: Johan actual
+ test_cmp expected actual
+'
+
test_expect_success 'with config setup' '
git config trailer.ack.key Acked-by:
cat expected -\EOF
diff --git a/trailer.c b/trailer.c
index 4b14a56..b808868 100644
--- a/trailer.c
+++ b/trailer.c
@@ -740,8 +740,10 @@ static int find_trailer_start(struct strbuf **lines, int
count)
/*
* 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 = 0; start--) {
+ for (start = count - 1; start = 1; start--) {
if (lines[start]-buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]-buf)) {
--
2.5.0.401.g009ef9b.dirty
--
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 v2 2/2] trailer: support multiline title
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. --- t/t7513-interpret-trailers.sh | 14 ++ trailer.c | 8 +++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 9577b4e..56efe88 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -112,6 +112,20 @@ test_expect_success 'with only a title in the message' ' test_cmp expected actual ' +test_expect_success 'with multiline title in the message' ' + cat expected -\EOF + place of + code: change + + Reviewed-by: Peff + Acked-by: Johan + EOF + printf %s\n%s\n place of code: change | + git interpret-trailers --trailer Reviewed-by: Peff \ + --trailer Acked-by: Johan actual + test_cmp expected actual +' + test_expect_success 'with config setup' ' git config trailer.ack.key Acked-by: cat expected -\EOF diff --git a/trailer.c b/trailer.c index b808868..9a54449 100644 --- a/trailer.c +++ b/trailer.c @@ -759,7 +759,13 @@ static int find_trailer_start(struct strbuf **lines, int count) return count; } - return only_spaces ? count : 0; + if (only_spaces) + return count; + + if (contains_only_spaces(lines[0]-buf)) + return 1; + + return count; } /* Get the index of the end of the trailers */ -- 2.5.0.401.g009ef9b.dirty -- 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 12:46 AM, Junio C Hamano gits...@pobox.com wrote:
Karthik Nayak karthik@gmail.com writes:
Here is what I see...
ok 98 - verifying rfc1991 signature
expecting success:
echo rfc1991 gpghome/gpg.conf
echo rfc1991-signed-tag RFC1991 signed tag expect
git tag -l -n1 rfc1991-signed-tag actual
test_cmp expect actual
git tag -l -n2 rfc1991-signed-tag actual
test_cmp expect actual
git tag -l -n999 rfc1991-signed-tag actual
test_cmp expect actual
--- expect 2015-08-24 22:54:44.607272653 +
+++ actual 2015-08-24 22:54:44.611272643 +
@@ -1 +1 @@
-rfc1991-signed-tag RFC1991 signed tag
+rfc1991-signed-tagRFC1991 signed tag
not ok 99 - list tag with rfc1991 signature
Thats weird, I just ran all tests, and nothing failed.
You may be missing GPG or RFC1991 prerequisites and not running this
particular test, which could be why you missed it.
That explains.
Your builtin/tag.c calls show_ref_array_item() with
%(align:16,left)%(refname:short)%(end)
as the format, and rfc1991-signed-tag is longer than 16.
And immediately after showing that, there is this hack at the end of
show_ref_array_item() function, which I _think_ should not be there
in ref-filter.c:show_ref_array_item() in the first place.
if (lines 0) {
struct object_id oid;
hashcpy(oid.hash, info-objectname);
show_tag_lines(oid, lines);
}
This belongs to the caller who knows that it is dealing with a tag.
Explained the idea behind this below.
But that broken design aside, the reason why this breaks is because
you do not have a separating SP after the aligned short refs.
Makes sense.
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.
And then move the tag-specific code at the end of
show_ref_array_item() to here:
verify_ref_format(format);
filter_refs(array, filter, FILTER_REFS_TAGS);
ref_array_sort(sorting, array);
- for (i = 0; i array.nr; i++)
+ for (i = 0; i array.nr; i++) {
show_ref_array_item(array.items[i], format, QUOTE_NONE,
filter-lines);
+ if (lines) {
+ struct object_id oid;
+ hashcpy(oid.hash, info-objectname);
+ show_tag_lines(oid, lines);
+ }
+ }
perhaps.
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.
We could pass a boolean to show_ref_array_item() and print a newline if no
lines are to be printed and probably print the newline in tag.c
itself, but seems confusing to me.
Heh, I did it myself. %(align:15) with trailing whitespace is what
you want.
An alternative way to spell it would be
%(align:16,left)%(refname:short) %(end)
I don't know which one is more readable, though.
I find this more readable, since the space is clearly visible, unlike
format = %(align:15,left)%(refname:short)%(end) ; in which the space
after %(end) is easily unnoticeable.
--
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 v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 08/25/2015 08:54 AM, Luke Diamand wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular 8 - Clone path (ignorecase) and Add a new file and clone path with new file (ignorecase) fail with the current implementation on OS X and Linux. That's a lot simpler, thanks! Could we give this its own command line option and git config variable? Core.ignorecase gets set if the client is on a filing system that ignores case. This is slightly different - it squashes case in depot files for people with depots that have incorrectly jumbled-up case. Conflating the two seems like it would cause confusion at some point - for example, I have no idea how the rest of git behaves if core.ignorecase is set to True on a case-preserving file system. That doesn't work as expected and is not allowed (or say strictly forbidden, or strongly recommended not to do) Remembering older discussions about importers from foreign VCS: This should work best for most people: Look at the command line option, if no one is given, look at core.ignorecase. So the command line option overrides core.ignorecase, and the user can either run --ignore-path-case or --no-ignore-path-case PS: Need to drop Eric from the list: the MX/A from from web.de problem still exists -- 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 Aug 2015, at 08:54, Luke Diamand l...@diamand.org wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular 8 - Clone path (ignorecase) and Add a new file and clone path with new file (ignorecase) fail with the current implementation on OS X and Linux. That's a lot simpler, thanks! Could we give this its own command line option and git config variable? Can you outline the command line option you have in mind? Core.ignorecase gets set if the client is on a filing system that ignores case. This is slightly different - it squashes case in depot files for people with depots that have incorrectly jumbled-up case. Well, you can’t tell if the depots have incorrectly jumbled-up case. It could be intentional after all. Therefore I believe the “ignorecase” flag is a good fit because we explicitly state that we are not interested in case sensitivity on the client. Conflating the two seems like it would cause confusion at some point - for example, I have no idea how the rest of git behaves if core.ignorecase is set to True on a case-preserving file system. It would probably be necessary to change p4PathStartsWith() to also check the same flag. Lars Schneider (1): git-p4: Obey core.ignorecase when using P4 client specs. 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 -- 1.9.5 (Apple Git-50.3) -- 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 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular 8 - Clone path (ignorecase) and Add a new file and clone path with new file (ignorecase) fail with the current implementation on OS X and Linux. That's a lot simpler, thanks! Could we give this its own command line option and git config variable? Core.ignorecase gets set if the client is on a filing system that ignores case. This is slightly different - it squashes case in depot files for people with depots that have incorrectly jumbled-up case. Conflating the two seems like it would cause confusion at some point - for example, I have no idea how the rest of git behaves if core.ignorecase is set to True on a case-preserving file system. It would probably be necessary to change p4PathStartsWith() to also check the same flag. Lars Schneider (1): git-p4: Obey core.ignorecase when using P4 client specs. 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 -- 1.9.5 (Apple Git-50.3) -- 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's inconsistent command line options
Hi, I've used Git for years and this has always bothered me. Has anybody else noticed the inconsistent command line parameteres for seemingly similar tasks. There are many examples, but I'll list only two (I can supply a more extensive list if needed). eg: Renaming things. * When working with branches it uses -m or -M to rename a branch. * When working with remote repos it use rename (not even the more common -- prefix either). eg: Deleting things * When working with branches it uses -d or -D to delete a branch * When working with remote branch it uses only a push command. * When deleting a remote repo it uses remove (again not even with the commonly used -- prefix) Even though I have worked with Git since 2009, I still have to reference the help to remind me of what parameter to use in certain situation simply because similar tasks differ so much. Maybe we could address this in the next major version of Git? Has anybody else thought about this or started work on this? Or was this discussed before and declined (link?). Regards, Graeme -- 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 04/12] ref-filter: implement an `align` atom
Junio C Hamano gits...@pobox.com writes:
You can see that I expected that if !state.stack-prev check to be
inside append_atom(), and I would imagine future readers would have
the same expectation when reading this code. I.e.
append_atom(struct atom_value *v, struct ref_f_s *state)
{
if (state-stack-prev)
strbuf_addstr(state-stack-output, v-s);
else
quote_format(state-stack-output, v-s,
state-quote_style);
}
The end result may be the same,
There's another call to append_atom() when inserting the reset color at
end of line if needed, so moving this if inside append_atom means we
would do the check also for the reset color. It would not change the
behavior (by construction, we insert it only when the stack has only the
initial element), so it's OK.
I agree that this is a good thing to do.
Moreover, notice that the function signature of append_atom() is
exactly the same as atomv-handler's. I wonder if it would be
easier to understand if you made append_atom() the handler for a
non-magic atoms, which would let you do the above without any if/else
and just a single unconditional
I can't decide between ah, very elegant and no, too much magic ;-).
I lean towards the former.
--
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/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 -- 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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
Jeff King p...@peff.net writes: +start=$(date +%s) Is that a GNU extension? git_filter_branch__commit_count=0 while read commit parents; do git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1)) - printf \rRewrite $commit ($git_filter_branch__commit_count/$commits) + now=$(date +%s) + elapsed=$(($now - $start)) + # work in integer percentages as a sort of fixed-point + pct=$(($git_filter_branch__commit_count * 100 / $commits)) + if test $pct -eq 0; then + remain= + else + eta=$(($elapsed * 100 / $pct)) + remain=($(($eta - $elapsed)) seconds remaining) + fi + printf \rRewrite $commit ($git_filter_branch__commit_count/$commits) $remain case $filter_subdir in ) but the time jumps around early on because of the lack of precision. And of course there's no smoothing, and no emphasis on recent history versus the whole operation. I'll leave those as an exercise to the reader. :) ;-) An alternative implementation may be to ask `date` every 1000 commits (or whatever sufficiently large value that we can amortise the cost) to measure the rate and compute $remain based on that measurement. That way, we can afford to use more portable ways to ask `date` about the current time and compute the how many seconds ourselves. -- 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/6] builtin/am: make sure state files are text
Jeff King p...@peff.net writes: On Tue, Aug 25, 2015 at 09:19:13AM -0700, Junio C Hamano wrote: As to flags exposed to callers vs with and without gently, when we change the system to allow new modes of operations (e.g. somebody wants to write a binary file, or allocate more flag bits for their special case), I'd expect that we'd add a more general and verbose write_file_with_options(path, flags, fmt, ...)), gain experience with that function, and then possibly introduce canned thin wrappers (e.g. write_binary_file() that is a synonym to passing BINARY but not GENTLY) if the new thing proves widely useful, just like I left write_file() and write_file_gently() in as fairly common things to do. Yeah, that works. It is a bit of a gamble to me. If we never add a lot more options, the end result is much nicer (callers do not deal with the flag option at all). But if we do, we end up with the mess that get_sha1_with_* and add_pending_object() got into. Yeah. I do not know. Perhaps a good intermim solution for now would be to make - write_file_l(path, flags, fmt, ...); the low-level helper, with a single convenience wrapper: - write_file(path, fmt, ...) for everybody other than two gently ones to use. Two gently ones can call write_file_l(path, WRITE_FILE_GENTLY, fmt,...). You are right about binary stuff. You could do fmt=...%c... and pass '\0' to corresponding place if your NUL is in the fixed part of the data you are generating, but otherwise write_file() interface is a very useful way to handle binary. -- 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 karthik@gmail.com writes: I'm working on porting over the printing options of ref-filter to `git branch -l`. This is a follow up to http://article.gmane.org/gmane.comp.version-control.git/276377 Theres a slight issue with this which I'd like to discuss about. When we use `-a` option in `git branch -l` It lists local branches and remotes with each having a different output format. This with reference to ref-filter would mean either 1. Change format in between local and remote refs I thought the end result of consolidation would be for for-each-ref, branch -l, and tag -l to be first collect what to show, and then show them according to a single format given. How could it be possible to switch format? It's not even like each element collected in the first phase is marked so that the second display phase know which one is which. 2. Have a format like %(if)...local check%(then)local branch format%(else)remote format %(end). That, if we had a working if/then/else/end, would be a good demonstration of it. If this were only for internal use by branch -l, it might be overkill, but the end users with for-each-ref --format options would want to do the same thing as built-in patterns, so it is a good goal to aim at. I prefer the latter, but that might lead to a large format. Any suggestions? A less ambitious option might be: 3. Invent %(refname:some magic) format similar to %(refname:short) but does your thing depending on the prefix refs/heads/ and refs/remotes/. but that will not work if the difference between local and remote format is not merely textual (e.g. paint them in different color). -- 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 karthik@gmail.com writes:
Here is what I see...
ok 98 - verifying rfc1991 signature
expecting success:
echo rfc1991 gpghome/gpg.conf
echo rfc1991-signed-tag RFC1991 signed tag expect
git tag -l -n1 rfc1991-signed-tag actual
test_cmp expect actual
git tag -l -n2 rfc1991-signed-tag actual
test_cmp expect actual
git tag -l -n999 rfc1991-signed-tag actual
test_cmp expect actual
--- expect 2015-08-24 22:54:44.607272653 +
+++ actual 2015-08-24 22:54:44.611272643 +
@@ -1 +1 @@
-rfc1991-signed-tag RFC1991 signed tag
+rfc1991-signed-tagRFC1991 signed tag
not ok 99 - list tag with rfc1991 signature
Thats weird, I just ran all tests, and nothing failed.
You may be missing GPG or RFC1991 prerequisites and not running this
particular test, which could be why you missed it.
Your builtin/tag.c calls show_ref_array_item() with
%(align:16,left)%(refname:short)%(end)
as the format, and rfc1991-signed-tag is longer than 16.
And immediately after showing that, there is this hack at the end of
show_ref_array_item() function, which I _think_ should not be there
in ref-filter.c:show_ref_array_item() in the first place.
if (lines 0) {
struct object_id oid;
hashcpy(oid.hash, info-objectname);
show_tag_lines(oid, lines);
}
This belongs to the caller who knows that it is dealing with a tag.
But that broken design aside, the reason why this breaks is because
you do not have a separating SP after the aligned short refs.
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.
And then move the tag-specific code at the end of
show_ref_array_item() to here:
verify_ref_format(format);
filter_refs(array, filter, FILTER_REFS_TAGS);
ref_array_sort(sorting, array);
- for (i = 0; i array.nr; i++)
+ for (i = 0; i array.nr; i++) {
show_ref_array_item(array.items[i], format, QUOTE_NONE,
filter-lines);
+ if (lines) {
+ struct object_id oid;
+ hashcpy(oid.hash, info-objectname);
+ show_tag_lines(oid, lines);
+ }
+ }
perhaps.
--
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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
On Tue, Aug 25, 2015 at 11:33:49AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: +start=$(date +%s) Is that a GNU extension? Thanks, I meant to mention that, too. POSIX has + formats, but apparently no way to get an integer number of seconds. I don't know how widely %s is supported; BSD date seems to know about it. An alternative implementation may be to ask `date` every 1000 commits (or whatever sufficiently large value that we can amortise the cost) to measure the rate and compute $remain based on that measurement. That way, we can afford to use more portable ways to ask `date` about the current time and compute the how many seconds ourselves. Yeah, that would probably be a good solution, assuming there is a portable how many seconds (I do not relish the thought of reconstructing it based on the current hours/minutes/seconds). I wonder how awful it would be to make a tool like git-progress, where you'd tell it --total=$commits --eta on the command line, and then occasionally print the current count its stdin. It might be a little painful to use, though. You'd want to background it with a pipe to its stdin, which is annoying without bash-style () anonymous pipes. -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 4/5] index-pack: Use the new worker pool
On Tue, Aug 25, 2015 at 10:28:25AM -0700, Stefan Beller wrote: By treating each object as its own task the workflow is easier to follow as the function used in the worker threads doesn't need any control logic any more. Have you tried running t/perf/p5302 on this? I seem to get a pretty consistent 2%-ish slowdown, both against git.git and linux.git. That's not a lot, but I'm wondering if there is some low-hanging fruit in the locking, or in the pattern of work being dispatched. Or it may just be noise, but it seems fairly consistent. -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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
On Tue, Aug 25, 2015 at 02:52:10PM -0400, Jeff King wrote:
Yeah, that would probably be a good solution, assuming there is a
portable how many seconds (I do not relish the thought of
reconstructing it based on the current hours/minutes/seconds).
A little googling came up with:
awk 'END { print systime() }' /dev/null
which probably (?) works everywhere. I feel dirty just having typed
that, though.
-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] write_file(): introduce an explicit WRITE_FILE_GENTLY request
On Tue, Aug 25, 2015 at 1:41 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: All callers except for two ask this function to die upon error by passing fatal=1; turn the parameter to a more generic unsigned flag bag of bits, introduce an explicit WRITE_FILE_GENTLY bit and change these two callers to pass that bit. There is a huge iffyness around one of these two oddball callers. diff --git a/setup.c b/setup.c index 5f9f07d..718f4e1 100644 --- a/setup.c +++ b/setup.c @@ -404,7 +404,7 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir) strbuf_addf(path, %s/gitfile, gitdir); if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL)) - write_file(path.buf, 0, %s\n, gitfile); + write_file(path.buf, WRITE_FILE_GENTLY, %s\n, gitfile); strbuf_release(path); } This comes from 23af91d1 (prune: strategies for linked checkouts, 2014-11-30). I cannot tell what the justification is to treat a failure to write a gitfile as a non-error event. Just a sloppy coding that lets the program go through to its finish, ignoring the harm done by possibly corrupting user repository silently? Failing to write to this file is not a big deal _if_ the file is not corrupted because of this write operation. But we should not be so silent about this. If the file content is corrupted and it's old enough, this checkout may be pruned. I think there's another bug here... wrong name.. -- Duy -- 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 02/12] ref-filter: introduce ref_formatting_state and ref_formatting_stack
On Tue, Aug 25, 2015 at 3:26 AM, Junio C Hamano gits...@pobox.com wrote:
Karthik Nayak karthik@gmail.com writes:
+static void push_new_stack_element(struct ref_formatting_stack **stack)
+{
Micronit. Perhaps s/_new//;, as you do not call the other function
pop_old_stack_element().
The remainder of this step looks pretty straight-forward and was a
pleasant read.
Thanks.
Will do, thanks for reviewing :)
--
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: index file list files not found in working tree
Hi, I got a recommendation to use reset --hard. I tried it and it says the HEAD is now at correct commit, but missing files are not restored! I tried `ls-tree --name-only` and it lists missing files and folders, but the actual working tree doesn't have these files and folders. The question I'd like to answer, how does git generates the `index` file between checkouts? Thanks, Rafik On 08/24/2015 12:34 PM, Rafik E Younan wrote: Hi, After several merges and rebases I finally got my branches and history to reflect valid commits and proper history. Everything is pushed to internal bare repo and the remotes seems OK. When I clone the updated repository, all branches reflect the correct updated trees and blobs. The problem occurs only on the original local repository where all the merging and re-basing took place! When I checkout a branch, several files and folders are deleted from the working tree. When I examine the history of these files, there are only commits of adding them and modifying them but no log for deleting them, and they aren't deleted when I checkout the same branch in another fresh cloned repo. Git status command doesn't indicate any changes in these files. I found the files and folders names in the `.git/index` file. So after manually removing the `.git/index` file and usinge `git reset` command, `git status` indicates that the files and folders are deleted. I use `git checkout -- File_or_folder_names...` and restore all missing files and folders, just then the working tree matches the fresh checkout of the same branch on any other cloned repo. After examining the tree object of the current commit, all files and folders exists, although clearly the checkout missed some of them! Because the repository is local and private, I can't share any url for publicly accessible repository, and if one exists, no problem could be found, because the problem resides in just this certain local clone. Answering the following questions might give some clues for the problem: * How does git populate the index file after every branch checkout? * Is there any object to reflect the content of the index file? I would appreciate any pointers for where the problem could be. Thanks, Rafik -- 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] setup: update the right file in multiple checkouts
This code is introduced in 23af91d (prune: strategies for linked checkouts - 2014-11-30), and it's supposed to implement this rule from that commit's message: - linked checkouts are supposed to keep its location in $R/gitdir up to date. The use case is auto fixup after a manual checkout move. Note the name, $R/gitdir, not $R/gitfile. Correct the path to be updated accordingly. While at there, make sure I/O errors are not silently dropped. Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- The code was right in v2 [1] and became gitfile since v3 [2]. I need to reconsider my code quality after this :( [1] http://article.gmane.org/gmane.comp.version-control.git/239299 [2] http://article.gmane.org/gmane.comp.version-control.git/242325 setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.c b/setup.c index 5f9f07d..64bf2b4 100644 --- a/setup.c +++ b/setup.c @@ -402,9 +402,9 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir) struct strbuf path = STRBUF_INIT; struct stat st; - strbuf_addf(path, %s/gitfile, gitdir); + strbuf_addf(path, %s/gitdir, gitdir); if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL)) - write_file(path.buf, 0, %s\n, gitfile); + write_file(path.buf, 1, %s\n, gitfile); strbuf_release(path); } -- 2.3.0.rc1.137.g477eb31 -- 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: index file list files not found in working tree
On Tue, Aug 25, 2015 at 12:16:43PM +0200, Rafik E Younan wrote: I got a recommendation to use reset --hard. I tried it and it says the HEAD is now at correct commit, but missing files are not restored! I tried `ls-tree --name-only` and it lists missing files and folders, but the actual working tree doesn't have these files and folders. Is there any chance that you have enabled sparse checkouts? See the documentation at the bottom of git-read-tree(1). On 08/24/2015 12:34 PM, Rafik E Younan wrote: Hi, After several merges and rebases I finally got my branches and history to reflect valid commits and proper history. Everything is pushed to internal bare repo and the remotes seems OK. When I clone the updated repository, all branches reflect the correct updated trees and blobs. The problem occurs only on the original local repository where all the merging and re-basing took place! When I checkout a branch, several files and folders are deleted from the working tree. When I examine the history of these files, there are only commits of adding them and modifying them but no log for deleting them, and they aren't deleted when I checkout the same branch in another fresh cloned repo. Git status command doesn't indicate any changes in these files. I found the files and folders names in the `.git/index` file. So after manually removing the `.git/index` file and usinge `git reset` command, `git status` indicates that the files and folders are deleted. I use `git checkout -- File_or_folder_names...` and restore all missing files and folders, just then the working tree matches the fresh checkout of the same branch on any other cloned repo. After examining the tree object of the current commit, all files and folders exists, although clearly the checkout missed some of them! Because the repository is local and private, I can't share any url for publicly accessible repository, and if one exists, no problem could be found, because the problem resides in just this certain local clone. Answering the following questions might give some clues for the problem: * How does git populate the index file after every branch checkout? * Is there any object to reflect the content of the index file? I would appreciate any pointers for where the problem could be. Thanks, Rafik -- 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 -- 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 Aug 2015, at 10:33, Torsten Bögershausen tbo...@web.de wrote: On 08/25/2015 08:54 AM, Luke Diamand wrote: On 24/08/15 22:30, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular 8 - Clone path (ignorecase) and Add a new file and clone path with new file (ignorecase) fail with the current implementation on OS X and Linux. That's a lot simpler, thanks! Could we give this its own command line option and git config variable? Core.ignorecase gets set if the client is on a filing system that ignores case. This is slightly different - it squashes case in depot files for people with depots that have incorrectly jumbled-up case. Conflating the two seems like it would cause confusion at some point - for example, I have no idea how the rest of git behaves if core.ignorecase is set to True on a case-preserving file system. That doesn't work as expected and is not allowed (or say strictly forbidden, or strongly recommended not to do) Remembering older discussions about importers from foreign VCS: This should work best for most people: Look at the command line option, if no one is given, look at core.ignorecase. So the command line option overrides core.ignorecase, and the user can either run --ignore-path-case or --no-ignore-path-case Unfortunately the command line option is not sufficient as the resulting paths are still messed up. I added the switch but it looks like as core.ignorecase does some additional magic on fast-import. You can see my changes here: https://github.com/larsxschneider/git/commit/b4399179ff542161c2c5b83c34c5b4901287ceb0 You can also run the unit tests to see the results here: https://github.com/larsxschneider/git/tree/lars/fix-path-v5-with-command-switch The only way I could image to fix that is to request every path from P4 as shown in my PATCH v4. This would be slow and the change would be rather huge. I am curious: I run all my P4 - git migrations on a Linux box with EXT4 and core.ignorecase=True. I did not realize that this might cause trouble. What could happen and what should I look out for? One thing to keep in mind: This is a one time migration and we don’t develop on these Linux boxes. We usually develop on Windows and Mac. I just use the Linux boxes as migration workers as these scripts usually run a while. - 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: Git's inconsistent command line options
On Tue, Aug 25, 2015 at 2:49 PM, Jacob Keller jacob.kel...@gmail.com wrote: On Tue, Aug 25, 2015 at 8:13 AM, Junio C Hamano gits...@pobox.com wrote: On Tue, Aug 25, 2015 at 1:01 AM, Graeme Geldenhuys grae...@gmail.com wrote: Even though I have worked with Git since 2009, I still have to reference the help to remind me of what parameter to use in certain situation simply because similar tasks differ so much. Maybe we could address this in the next major version of Git? Has anybody else thought about this or started work on this? Or was this discussed before and declined (link?). http://article.gmane.org/gmane.comp.version-control.git/231478 comes to mind, which has been linked from this entry: Discuss and decide if we want to choose between the mode word UI (e.g. git submodule add) and the mode option UI (e.g. git tag --delete) and standardise on one; if it turns out to be a good idea, devise the migration plan to break the backward-compatibility. in http://git-blame.blogspot.com/p/leftover-bits.html I would vote for command words, as this is clean and simple. me too after rereading the arguments in that thread. The downside is in converting all the old options based commands, git-tag, and similar. These commands cannot easily convert because valid sequences would become invalid with no easy way to deprecate for example in the linked gmane above, git tag delete master can't be a call to delete master as it is currently a call to create a tag delete at the commit marked by master. git-tag being a porcelain command (i.e. we do not give a promise to keep it set to stone) can be changed with a deprecation announcement period. Say starting with Git 2.6 we would put out warnings for upcoming commands: $ git tag --delete master $ echo $? # 0 # actually works as of today! $ git tag delete master # Due to the planned switch to command words, this doesn't work. # For details see road map at `man git commandwords-roadmaps` $ echo $? # 128 maybe ? $ git tag create delete And after a while (maybe 3-5 years, once this version is picked up by debian stable as well as red hat stable) we can change it, so with Git 3.4(?) $ git tag --delete master # --delete is deprecated since 3.4, use `git tag delete` instead $ echo $? # 128 $ git tag delete master # --delete is deprecated since 2.6, use `git tag delete` instead $ echo $? # 0 # actually works! I can't think of an easy way to deprecate the change in behavior over time, which means that making a conversion would require some other as yet unknown way? It may be possible to convert other options based commands, such as how git-branch and git-checkout do things which seem highly unrelated. A good example is how checkout is used to both change branches, as well as can create a branch, and can also checkout a file. The reset command is used to rewind history, as well as potentially reset *all* files, but it can't be used to reset a single file, and is completely different from revert. Some of these distinctions are ok because it's just no good way to make everything easy. Some of these could be fixed by the command word setup, but as many have mentioned an actual migration plan is difficult. Personally, I don't want to move to the command option --status format, as I think these aren't really options, and are very much sub-subcommands. I think we should try to push more uses of this style, and try to determine a possible migration path towards using them. Maybe some warts simply aren't worth the effort to fix though. Other issues are tricker to solve, and are result of git exposing more complex functionality and users eventually simply have to learn and understand. 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 -- 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 Tue, Aug 25, 2015 at 3:06 PM, Stefan Beller sbel...@google.com wrote: On Tue, Aug 25, 2015 at 2:49 PM, Jacob Keller jacob.kel...@gmail.com wrote: On Tue, Aug 25, 2015 at 8:13 AM, Junio C Hamano gits...@pobox.com wrote: On Tue, Aug 25, 2015 at 1:01 AM, Graeme Geldenhuys grae...@gmail.com wrote: Even though I have worked with Git since 2009, I still have to reference the help to remind me of what parameter to use in certain situation simply because similar tasks differ so much. Maybe we could address this in the next major version of Git? Has anybody else thought about this or started work on this? Or was this discussed before and declined (link?). http://article.gmane.org/gmane.comp.version-control.git/231478 comes to mind, which has been linked from this entry: Discuss and decide if we want to choose between the mode word UI (e.g. git submodule add) and the mode option UI (e.g. git tag --delete) and standardise on one; if it turns out to be a good idea, devise the migration plan to break the backward-compatibility. in http://git-blame.blogspot.com/p/leftover-bits.html I would vote for command words, as this is clean and simple. me too after rereading the arguments in that thread. The downside is in converting all the old options based commands, git-tag, and similar. These commands cannot easily convert because valid sequences would become invalid with no easy way to deprecate for example in the linked gmane above, git tag delete master can't be a call to delete master as it is currently a call to create a tag delete at the commit marked by master. git-tag being a porcelain command (i.e. we do not give a promise to keep it set to stone) can be changed with a deprecation announcement period. Say starting with Git 2.6 we would put out warnings for upcoming commands: $ git tag --delete master $ echo $? # 0 # actually works as of today! $ git tag delete master # Due to the planned switch to command words, this doesn't work. # For details see road map at `man git commandwords-roadmaps` $ echo $? # 128 maybe ? $ git tag create delete And after a while (maybe 3-5 years, once this version is picked up by debian stable as well as red hat stable) we can change it, so with Git 3.4(?) $ git tag --delete master # --delete is deprecated since 3.4, use `git tag delete` instead $ echo $? # 128 $ git tag delete master # --delete is deprecated since 2.6, use `git tag delete` instead $ echo $? # 0 # actually works! This seems like a possible strategy for converging on command words. So basically, we force all uses of the command words to just fail and then once that's picked up we can migrate to the command words. 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: [PATCH 3/5] submodule: helper to run foreach in parallel
Stefan Beller sbel...@google.com writes:
+ while (1) {
+ ssize_t len = xread(cp-err, buf, sizeof(buf));
+ if (len 0)
+ die(Read from child failed);
+ else if (len == 0)
+ break;
+ else {
+ strbuf_add(out, buf, len);
+ }
... and the whole thing is accumulated in core???
The pipes have a limit, so we need to empty them to prevent back-pressure?
Of course. But that does not lead to we hold everything in core.
This side could choose to emit (under protection of args-mutex)
early, e.g. after reading a line, emit it to our standard output (or
our standard error).
And because we want to have the output of one task at a time, we need to
save it up until we can put out the whole output, no?
I do not necessarily agree, and I think I said that already:
http://thread.gmane.org/gmane.comp.version-control.git/276273/focus=276321
+ }
+ if (finish_command(cp))
+ die(command died with error);
+
+ sem_wait(args-mutex);
+ fputs(out.buf, stderr);
+ sem_post(args-mutex);
... and emitted to standard error?
I would have expected that the standard error would be left alone
`git fetch` which may be a good candidate for such an operation
provides progress on stderr, and we don't want to intermingle
2 different submodule fetch progress displays
(I need to work offline for a bit, so let me get all of the latest stuff,
so I'll run `git submodule foreach -j 16 -- git fetch --all though ideally
we want to have `git fetch --recurse-submodules -j16` instead )
(i.e. letting warnings from multiple jobs to be mixed together
simply because everybody writes to the same file descriptor), while
the standard output would be line-buffered, perhaps captured by the
above loop and then emitted under mutex, or something.
I think I said this earlier, but latency to the first output counts
to the first stderr
in this case?
I didn't mean output==the standard output stream. As I said in
$gmane/276321, an early output, as an indication that we are doing
something, is important.
Why would we want to unplug the task queue from somewhere else?
When you have a dispatcher more intelligent than a stupid FIFO, I
would imagine that you would want to be able to do this pattern,
especially when coming up with a task (not performing a task) takes
non-trivial amount of work:
prepare task queue and have N threads waiting on it;
plug the queue, i.e. tell threads that do not start picking
tasks out of it yet;
large enough loop to fill the queue to a reasonable size
while keeping the threads waiting;
unplug the queue. Now the threads can pick tasks from the
queue, but they have many to choose from, and a dispatcher
can do better than simple FIFO can take advantage of it;
keep filling the queue with more tasks, if necessary;
and finally, wait for everything to finish.
Without plug/unplug interface, you _could_ do the above by doing
something stupid like
prepare a task queue and have N threads waiting on it;
loop to find enough number of tasks but do not put them to
task queue, as FIFO will eat them one-by-one; instead hold
onto them in a custom data structure that is outside the
task queue system;
tight and quick loop to move them to the task queue;
keep finding more tasks and feed them to the task queue;
and finally, wait for everything to finish.
--
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 4/5] index-pack: Use the new worker pool
On Tue, Aug 25, 2015 at 2:12 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: Not sure I follow there. Original implementation: We have M threads sitting around the table, all of them trying to obtain food from the one bowl on the table and then eating it. Once the bowl is all eaten, we can stop. New pattern: One cook puts all the food items on the sushi-go-round-belt with a fancy plate and the threads grab them one by one still using locks (but they are internal to the belt). Are you saying we're content with just a bowl and everyone helps themselves for getting food? No. I am questioning how big overhead is for having the go-round-belt that must hold all dishes to be eaten, which did not exist in the original arrangement. Then please don't pick up this patch. This and patch 5 are there to convince Jeff this is a good API, worth being introduced and not over engineered, just solving a problem we're interested in with a minimal amount of code to side track from the actual goal we want to pursue. -- 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 4/5] index-pack: Use the new worker pool
Jeff King p...@peff.net writes: On Tue, Aug 25, 2015 at 10:28:25AM -0700, Stefan Beller wrote: By treating each object as its own task the workflow is easier to follow as the function used in the worker threads doesn't need any control logic any more. Have you tried running t/perf/p5302 on this? I seem to get a pretty consistent 2%-ish slowdown, both against git.git and linux.git. That's not a lot, but I'm wondering if there is some low-hanging fruit in the locking, or in the pattern of work being dispatched. Or it may just be noise, but it seems fairly consistent. The pattern of work dispatch hopefully is the same, no? add_task() does the append at the end thing and next_task() picks from the front of the queue. The original is we have globally N things, so far M things have been handled, and we want a new one, so we pick the M+1th one and do it. The amount of memory that is used to represent a single task may be much larger than the original, with overhead coming from job_list structure and the doubly-linked list. We may not be able to spin up 30 threads and throw a million tasks at them using this, because of the overhead. It would be more suited to handle a pattern in which an overlord actively creates new tasks while worker threads chew them, using the add_task/dispatch as the medium for communication between them. -- 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 4/5] index-pack: Use the new worker pool
On Tue, Aug 25, 2015 at 12:03 PM, Jeff King p...@peff.net wrote: On Tue, Aug 25, 2015 at 10:28:25AM -0700, Stefan Beller wrote: By treating each object as its own task the workflow is easier to follow as the function used in the worker threads doesn't need any control logic any more. Have you tried running t/perf/p5302 on this? I seem to get a pretty consistent 2%-ish slowdown, both against git.git and linux.git. That's not a lot, but I'm wondering if there is some low-hanging fruit in the locking, or in the pattern of work being dispatched. Or it may just be noise, but it seems fairly consistent. I did not run any perf tests, just the standard tests. Maybe the progress display can be moved out to another thread, such that it doesn't block the threads doing actual work. Also the progress display can be done a bit more sloppy, we are allowed to drop old events, if we have newer information. [Though this reasoning is not looking at the threading pool code, because ..um.. my code is perfect] -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 1/2] dir.c: make last_exclude_matching_from_list() run til the end
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Because? Title just tells what the patch meant to do (i.e. instead
of returning it keeps looping), but does not say why it is a good
idea. Besides, this a no-op patch and does not make it keep looping.
---
dir.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/dir.c b/dir.c
index c00c7e2..3a7630a 100644
--- a/dir.c
+++ b/dir.c
@@ -901,6 +901,7 @@ static struct exclude
*last_exclude_matching_from_list(const char *pathname,
int *dtype,
struct exclude_list *el)
{
+ struct exclude *exc = NULL; /* undecided */
int i;
if (!el-nr)
@@ -922,18 +923,22 @@ static struct exclude
*last_exclude_matching_from_list(const char *pathname,
Note that we are in a big for() loop that scans backwards an array.
if (match_basename(basename,
pathlen - (basename - pathname),
exclude, prefix, x-patternlen,
-x-flags))
- return x;
+x-flags)) {
+ exc = x;
+ break;
+ }
We used to return x immediately; now we store x to exc and break,
i.e. leave the loop.
continue;
}
assert(x-baselen == 0 || x-base[x-baselen - 1] == '/');
if (match_pathname(pathname, pathlen,
x-base, x-baselen ? x-baselen - 1 : 0,
-exclude, prefix, x-patternlen, x-flags))
- return x;
+exclude, prefix, x-patternlen, x-flags)) {
+ exc = x;
+ break;
We used to return x immediately; now we store x to exc and break,
i.e. leave the loop.
+ }
}
- return NULL; /* undecided */
+ return exc;
And then we return exc.
}
--
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}
Junio C Hamano gits...@pobox.com writes: A less ambitious option might be: 3. Invent %(refname:some magic) format similar to %(refname:short) but does your thing depending on the prefix refs/heads/ and refs/remotes/. Actually, this is the option I suggest offline. but that will not work if the difference between local and remote format is not merely textual (e.g. paint them in different color). Hmm, yes, colors would be difficult to get with this solution. Perhaps a %(refname:autoprefix,autocolor) that would pick the color and do the textual rendering? -- 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
[PATCH] compat/inet_ntop.c: Use INET_ADDRSTRLEN and INET6_ADDRSTRLEN macroses
From: Brilliantov Kirill Vladimirovich brillian...@inbox.ru
Signed-off-by: Brilliantov Kirill Vladimirovich brillian...@inbox.ru
---
compat/inet_ntop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index 90b7cc4..fcd3b15 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -47,7 +47,7 @@ static const char *
inet_ntop4(const u_char *src, char *dst, size_t size)
{
static const char fmt[] = %u.%u.%u.%u;
- char tmp[sizeof 255.255.255.255];
+ char tmp[INET_ADDRSTRLEN];
int nprinted;
nprinted = snprintf(tmp, sizeof(tmp), fmt, src[0], src[1], src[2],
src[3]);
@@ -78,7 +78,7 @@ inet_ntop6(const u_char *src, char *dst, size_t size)
* Keep this in mind if you think this function should have been coded
* to use pointer overlays. All the world's not a VAX.
*/
- char tmp[sizeof ::::::255.255.255.255], *tp;
+ char tmp[INET6_ADDRSTRLEN], *tp;
struct { int base, len; } best, cur;
unsigned int words[NS_IN6ADDRSZ / NS_INT16SZ];
int i;
--
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: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25/08/15 11:30, larsxschnei...@gmail.com wrote: Unfortunately the command line option is not sufficient as the resulting paths are still messed up. I added the switch but it looks like as core.ignorecase does some additional magic on fast-import. You can see my changes here: https://github.com/larsxschneider/git/commit/b4399179ff542161c2c5b83c34c5b4901287ceb0 You can also run the unit tests to see the results here: https://github.com/larsxschneider/git/tree/lars/fix-path-v5-with-command-switch The only way I could image to fix that is to request every path from P4 as shown in my PATCH v4. This would be slow and the change would be rather huge. Yes, you're right - fast-import has special handling based on core.ignorecase. There was a thread a while back saying that it shouldn't do this, and instead should have a new --casefold option, which would make more sense, but isn't the case at present. http://www.spinics.net/lists/git/msg243264.html I am curious: I run all my P4 - git migrations on a Linux box with EXT4 and core.ignorecase=True. I did not realize that this might cause trouble. What could happen and what should I look out for? An obvious way it could go wrong would be if you had a a repo that _did_ care about case (e.g. had Makefile and makefile in the same directory) and you then tried to git-p4 clone a separate repo into a different branch. In an ideal world, you would only use the case-folding on the git-p4 based repo. I think while fast-import just checks core.ignorecase, that's not possible. 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) 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 v4 3/3] untracked cache: fix entry invalidation
On Wed, 2015-08-19 at 20:01 +0700, Nguyễn Thái Ngọc Duy wrote: First, the current code in untracked_cache_invalidate_path() is wrong because it can only handle paths a or a/b, not a/b/c because lookup_untracked() only looks for entries directly under the given directory. In the last case, it will look for the entry b/c in directory a instead. This means if you delete or add an entry in a subdirectory, untracked cache may become out of date because it does not invalidate properly. This is noticed by David Turner. The second problem is about invalidation inside a fully untracked/excluded directory. In this case we may have to invalidate back to root. See the comment block for detail. LGTM. -- 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 gits...@pobox.com 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. Heh, I did it myself. %(align:15) with trailing whitespace is what you want. An alternative way to spell it would be %(align:16,left)%(refname:short) %(end) I don't know which one is more readable, though. -- 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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
I would lean for an extra on-demand flag for this, and a per commit
measurement, initial noise is okay for the first iteration I think.
Secondly note that on the output other messages could also be present
(other than the rewrite), as the command running may have its own
output. I will try to create a initial version just have some time for
it :)
On 8/25/15, Jeff King p...@peff.net wrote:
On Tue, Aug 25, 2015 at 02:52:10PM -0400, Jeff King wrote:
Yeah, that would probably be a good solution, assuming there is a
portable how many seconds (I do not relish the thought of
reconstructing it based on the current hours/minutes/seconds).
A little googling came up with:
awk 'END { print systime() }' /dev/null
which probably (?) works everywhere. I feel dirty just having typed
that, though.
-Peff
--
Bernát Gábor
Student - Budapest University of Technology and Economics - Computer
Engineering, M.Sc. - Budapest, Hungary
System Integrator - Gravity RD | Rock Solid Recommendations - Budapest
office | 5-7 Expo ter | H-1101 | Budapest | Hungary
--
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 3/3] untracked cache: fix entry invalidation
David Turner dtur...@twopensource.com writes: On Wed, 2015-08-19 at 20:01 +0700, Nguyễn Thái Ngọc Duy wrote: First, the current code in untracked_cache_invalidate_path() is wrong because it can only handle paths a or a/b, not a/b/c because lookup_untracked() only looks for entries directly under the given directory. In the last case, it will look for the entry b/c in directory a instead. This means if you delete or add an entry in a subdirectory, untracked cache may become out of date because it does not invalidate properly. This is noticed by David Turner. The second problem is about invalidation inside a fully untracked/excluded directory. In this case we may have to invalidate back to root. See the comment block for detail. LGTM. Thanks, both. Let's move this forward. -- 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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
On Tue, Aug 25, 2015 at 2:54 PM, Jeff King p...@peff.net wrote:
On Tue, Aug 25, 2015 at 02:52:10PM -0400, Jeff King wrote:
Yeah, that would probably be a good solution, assuming there is a
portable how many seconds (I do not relish the thought of
reconstructing it based on the current hours/minutes/seconds).
A little googling came up with:
awk 'END { print systime() }' /dev/null
which probably (?) works everywhere.
On Mac OS X and FreeBSD:
$ awk 'END { print systime() }' /dev/null
awk: calling undefined function systime
source line number 1
$
--
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}
Matthieu Moy matthieu@grenoble-inp.fr writes: Hmm, yes, colors would be difficult to get with this solution. Perhaps a %(refname:autoprefix,autocolor) that would pick the color and do the textual rendering? Yeah, that's workable. -- 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 4/5] index-pack: Use the new worker pool
On Tue, Aug 25, 2015 at 1:41 PM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: On Tue, Aug 25, 2015 at 10:28:25AM -0700, Stefan Beller wrote: By treating each object as its own task the workflow is easier to follow as the function used in the worker threads doesn't need any control logic any more. Have you tried running t/perf/p5302 on this? I seem to get a pretty consistent 2%-ish slowdown, both against git.git and linux.git. That's not a lot, but I'm wondering if there is some low-hanging fruit in the locking, or in the pattern of work being dispatched. Or it may just be noise, but it seems fairly consistent. The pattern of work dispatch hopefully is the same, no? add_task() does the append at the end thing and next_task() picks from the front of the queue. The original is we have globally N things, so far M things have been handled, and we want a new one, so we pick the M+1th one and do it. The amount of memory that is used to represent a single task may be much larger than the original, with overhead coming from job_list structure and the doubly-linked list. We may not be able to spin up 30 threads and throw a million tasks at them using this, because of the overhead. I thought about making the add_task block after a certain size of job_list such that there is always just enough tasks available to process. Then we would not need to used a huge amount of memory for having a long line of waiting tasks; the shorter the line is the less we can tolerate burst behavior in the threads. It would be more suited to handle a pattern in which an overlord actively creates new tasks while worker threads chew them, using the add_task/dispatch as the medium for communication between them. Not sure I follow there. Original implementation: We have M threads sitting around the table, all of them trying to obtain food from the one bowl on the table and then eating it. Once the bowl is all eaten, we can stop. New pattern: One cook puts all the food items on the sushi-go-round-belt with a fancy plate and the threads grab them one by one still using locks (but they are internal to the belt). Are you saying we're content with just a bowl and everyone helps themselves for getting food? -- 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
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? Jeff 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 http://stackoverflow.com/questions/13519203/git-compilingdocumentation-git-add-xml-does-not-validate Jeff --- sent via the nexus --- On Mon, Aug 24, 2015 at 6:04 PM, brian m. carlson sand...@crustytoothpaste.net wrote: On Mon, Aug 24, 2015 at 02:30:39AM -0700, Jeff S wrote: XSLTPROC user-manual.html warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/l10n.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 29 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/l10n.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/utility.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 31 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/utility.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/labels.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 32 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/labels.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/autotoc.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 39 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/autotoc.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/verbatim.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 43 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/verbatim.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/formal.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 46 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/formal.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/table.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/table.xsl line 11 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/table.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/footnote.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 51 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/footnote.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/block.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 65 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/block.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/stripns.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 76 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/stripns.xsl make[1]: *** [user-manual.html] Error 5 make: *** [doc] Error 2 It's clear from the message that your catalogs are not properly set up. Once you get that working, the documentation should build correctly. nextCatalog catalog=file:///usr/local/Cellar/docbook-xsl/1.78.1/docbook-xsl/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook-xsl/1.78.1/docbook-xsl-ns/catalog.xml/ /catalog Unfortunately, this doesn't tell us much, since these are all references to other catalogs. You need to look in the other catalogs, specifically the two mentioned above, and ensure that there are appropriate rewriteURI and rewriteSystem entries pointing to the right place. On my system, that looks like the following: rewriteURI uriStartString=http://docbook.sourceforge.net/release/xsl/current/; rewritePrefix=.// rewriteSystem systemIdStartString=http://docbook.sourceforge.net/release/xsl/current/; rewritePrefix=.// I don't know how your system is configured, so I can't tell you what entries
Re: [PATCH v13 05/12] ref-filter: add option to filter out tags, branches and remotes
On Tue, Aug 25, 2015 at 3:54 AM, Junio C Hamano gits...@pobox.com wrote:
Karthik Nayak karthik@gmail.com writes:
From: Karthik Nayak karthik@gmail.com
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.
For this part, I think you would want to borrow an extra pair of
eyes from Michael.
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 christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
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;
+
+ 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;
+
When filter-kind is set to some single-bit thing (e.g.
FILTER_REFS_BRANCHES) by the caller of ref_filter_handler(), then
this call of filter_ref_kind() will just parrot that without even
looking at refname. And then the if statement says yes, they have
common bit(s). Even when refname is refs/tags/v1.0 or HEAD.
This happens for branches remotes and tags, and this is intentional so as to
skip the uncessary checking to be done, if you see filter_refs() if it's called
with a single bit (branches, tags or remotes) then it calls
for_each_reftype_fullpath()
with the corresponding path, which would only give us the refs which we needed
hence checking them is not really needed. But the call to
filter_ref_kind() would just
give us the kind of the ref.
And if this code knows that refs/tags/v1.0 or HEAD will never
come when filter-kind is exactly FILTER_REFS_BRANCHES, then I do
not see the point of calling filter_ref_kind().
The point is to have a common function which returns the current ref kind,
Even if we know that the current ref kind is the same as the filter-kind.
I am not sure what this is trying to do. Can you clarify the
thinking behind this as a comment to filter_ref_kind()?
I should added a comment here, I will do so now.
@@ -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
Re: [PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
On 25 Aug 2015, at 13:57, Luke Diamand l...@diamand.org wrote: On 25/08/15 11:30, larsxschnei...@gmail.com wrote: Unfortunately the command line option is not sufficient as the resulting paths are still messed up. I added the switch but it looks like as core.ignorecase does some additional magic on fast-import. You can see my changes here: https://github.com/larsxschneider/git/commit/b4399179ff542161c2c5b83c34c5b4901287ceb0 You can also run the unit tests to see the results here: https://github.com/larsxschneider/git/tree/lars/fix-path-v5-with-command-switch The only way I could image to fix that is to request every path from P4 as shown in my PATCH v4. This would be slow and the change would be rather huge. Yes, you're right - fast-import has special handling based on core.ignorecase. There was a thread a while back saying that it shouldn't do this, and instead should have a new --casefold option, which would make more sense, but isn't the case at present. http://www.spinics.net/lists/git/msg243264.html I am curious: I run all my P4 - git migrations on a Linux box with EXT4 and core.ignorecase=True. I did not realize that this might cause trouble. What could happen and what should I look out for? An obvious way it could go wrong would be if you had a a repo that _did_ care about case (e.g. had Makefile and makefile in the same directory) and you then tried to git-p4 clone a separate repo into a different branch. In an ideal world, you would only use the case-folding on the git-p4 based repo. I think while fast-import just checks core.ignorecase, that's not possible. OK. Thanks for the explanation. 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. - 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 v13 00/12] port tag.c to use ref-filter APIs
On Tue, Aug 25, 2015 at 4:04 AM, Junio C Hamano gits...@pobox.com wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is -performed. +performed. If used with '--quote' everything in between %(align:..) +and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). I had already commented on a preliminary version of this series off-list. I think all my previous comments have been taken into account. Thanks, both. I think this is pretty close to being satisfactory ;-) There may probably be a handful of minor nits like the above that need to be addressed, but I do not think I saw anything glaringly wrong that makes the series unsalvageable. It was a very pleasant read. It's almost there, and I am very happy to see how this and other series evolved so far ;-) I like how it's totally different from what I started off with, it's been a great learning curve for me. Thanks for reviews. -- 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 3/5] submodule: helper to run foreach in parallel
Junio C Hamano gits...@pobox.com writes: Why would we want to unplug the task queue from somewhere else? When you have a dispatcher more intelligent than a stupid FIFO, I would imagine that you would want to be able to do this pattern, especially when coming up with a task (not performing a task) takes non-trivial amount of work: prepare task queue and have N threads waiting on it; plug the queue, i.e. tell threads that do not start picking tasks out of it yet; s/that do not/not to/; sorry for grammo resulting from a lot of editing while doing other things X-. ... and finally, wait for everything to finish. Without plug/unplug interface, you _could_ do the above by doing something stupid like ... and finally, wait for everything to finish. ... but having to use yet another queue outside only because the task queue cannot be plugged feels stupid. But because we do not yet have a way to plug custom dispatching logic to the dispatcher, the above is in the but that can wait until the need arises. category, as I said. -- 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 4/5] index-pack: Use the new worker pool
Stefan Beller sbel...@google.com writes: Then please don't pick up this patch. This and patch 5 are there to convince Jeff this is a good API, worth being introduced and not over engineered, just solving a problem we're interested in with a minimal amount of code to side track from the actual goal we want to pursue. Don't worry. I did not have intention to queue index-pack and pack-objects rewrite unless the use of the new API makes them demonstratably better. The criteria for worth being introduced would include not excessively heavyweight, I would think. And it was good that we have these two patches to judge if the earlier one (2-3/5) is a good thing to add in a concrete way. Perhaps the 2% slowdown might be showing the performance characteristics of your worker pool model, and Peff's suggestion to at least see (if not solve) where the overhead is coming from was a very reasonable one, I would 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
What's cooking in git.git (Aug 2015, #04; Tue, 25)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html Note that code.google.com/ has gone into read-only mode and the branches and tags there will not be updated. -- [Graduated to master] * cb/open-noatime-clear-errno (2015-08-12) 1 commit (merged to 'next' on 2015-08-17 at 6aa43a1) + git_open_noatime: return with errno=0 on success When trying to see that an object does not exist, a state errno leaked from our first try to open a packfile with O_NOATIME and then if it fails retry without it logic on a system that refuses O_NOATIME. This confused us and caused us to die, saying that the packfile is unreadable, when we should have just reported that the object does not exist in that packfile to the caller. * dt/notes-multiple (2015-08-11) 2 commits (merged to 'next' on 2015-08-12 at 0052055) + notes: handle multiple worktrees + worktrees: add find_shared_symref When linked worktree is used, simultaneous notes merge instances for the same ref in refs/notes/* are prevented from stomping on each other. * dt/refs-pseudo (2015-08-11) 6 commits (merged to 'next' on 2015-08-12 at 7078eb6) + pseudoref: check return values from read_ref() (merged to 'next' on 2015-08-03 at 3eafd33) + sequencer: replace write_cherry_pick_head with update_ref + bisect: use update_ref + pseudorefs: create and use pseudoref update and delete functions + refs: add ref_type function + refs: introduce pseudoref and per-worktree ref concepts To prepare for allowing a different ref backend to be plugged in to the system, update_ref()/delete_ref() have been taught about ref-like things like MERGE_HEAD that are per-worktree (they will always be written to the filesystem inside $GIT_DIR). * ee/clean-remove-dirs (2015-08-11) 1 commit (merged to 'next' on 2015-08-12 at fc41b09) + t7300-clean: require POSIXPERM for chmod 0 test Test updates for Windows. * jc/finalize-temp-file (2015-08-10) 1 commit (merged to 'next' on 2015-08-12 at 6fe62fe) + sha1_file.c: rename move_temp_to_file() to finalize_object_file() Long overdue micro clean-up. * jh/strbuf-read-use-read-in-full (2015-08-10) 1 commit (merged to 'next' on 2015-08-12 at db16247) + strbuf_read(): skip unnecessary strbuf_grow() at eof strbuf_read() used to have one extra iteration (and an unnecessary strbuf_grow() of 8kB), which was eliminated. * jk/git-path (2015-08-10) 16 commits (merged to 'next' on 2015-08-12 at 7ebe864) + memoize common git-path constant files + get_repo_path: refactor path-allocation + find_hook: keep our own static buffer + refs.c: remove_empty_directories can take a strbuf + refs.c: avoid git_path assignment in lock_ref_sha1_basic + refs.c: avoid repeated git_path calls in rename_tmp_log + refs.c: simplify strbufs in reflog setup and writing + path.c: drop git_path_submodule + refs.c: remove extra git_path calls from read_loose_refs + remote.c: drop extraneous local variable from migrate_file + prefer mkpathdup to mkpath in assignments + prefer git_pathdup to git_path in some possibly-dangerous cases + add_to_alternates_file: don't add duplicate entries + t5700: modernize style + cache.h: complete set of git_path_submodule helpers + cache.h: clarify documentation for git_path, et al (this branch is used by sb/submodule-helper.) git_path() and mkpath() are handy helper functions but it is easy to misuse, as the callers need to be careful to keep the number of active results below 4. Their uses have been reduced. * jk/guess-repo-name-regression-fix (2015-08-10) 2 commits (merged to 'next' on 2015-08-12 at 4cba33c) + clone: use computed length in guess_dir_name + clone: add tests for output directory (this branch is used by ps/guess-repo-name-at-root.) git clone $URL in recent releases of Git contains a regression in the code that invents a new repository name incorrectly based on the $URL. This has been corrected. * jk/long-error-messages (2015-08-11) 2 commits (merged to 'next' on 2015-08-12 at 36303cd) + vreportf: avoid intermediate buffer + vreportf: report to arbitrary filehandles The codepath to produce error messages had a hard-coded limit to the size of the message, primarily to avoid memory allocation while calling die(). * jk/negative-hiderefs (2015-08-07) 2 commits (merged to 'next' on 2015-08-12 at bdc478d) + refs: support negative transfer.hideRefs + docs/config.txt: reorder hideRefs config A negative !ref entry in multi-value transfer.hideRefs configuration can be used to say don't hide this one. * jk/test-with-x (2015-08-07) 2 commits (merged to 'next' on 2015-08-12 at 06576a1) + test-lib: disable trace when test is not
Re: What's cooking in git.git (Aug 2015, #04; Tue, 25)
On Tue, Aug 25, 2015 at 4:28 PM, Junio C Hamano gits...@pobox.com wrote: * jk/notes-merge-config (2015-08-17) 6 commits - notes: teach git-notes about notes.name.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=how 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. 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
Stefan Beller sbel...@google.com writes: $ git tag --delete master $ echo $? # 0 # actually works as of today! $ git tag delete master # Due to the planned switch to command words, this doesn't work. # For details see road map at `man git commandwords-roadmaps` $ echo $? # 128 maybe ? This is way too aggressive behaviour and is unacceptable as the first step. The first step of a transition that breaks backward compatibility should warn loudly about a command line that would behave differently in the endgame version (either the command line that will not do anything or do a totally different thing), but still perform the operation asked for the current version. e.g. git tag delete master would create a tag named 'delete' out of 'master', but tell the user that this will instead delete 'master' in future versions of Git. git tag create master would create a tag named 'create' out of 'master', but tell the user that this will instead create 'master' out of HEAD in future versions of Git. e.g. git tag -d foo would delete a tag named 'foo', but tell the user that this will have to be spelled 'git tag delete foo' in the future versions of Git. One thing that I am not enthused about the transition plan is that git tag delete master will *never* be an invalid operation during the transition. When making an operation that used to mean one thing to mean something else, a good transition plan should be to * First warn but do the old thing, and tell users a new way to do that in the new world order. At the same time, find the new way that used to be an invalid operation in the old world order, and implement it. * Then stop supporting the old thing and support only the new thing. Then during the transition period, while transitioning to the new way, people can gradually start using the new way with the new system, and when they occasionally have to interact with an old system, the new way will _error out_, because we make sure we find the new way that used to be an invalid operation when planning the whole transition plan, without causing any harm. And once people retrain their finger after 2-3 years, nobody will be hurt if we dropped the old way. 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. So don't hold your breath. What you two are discussing is way too uncooked for 2.6 timeframe. -- 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 2:09 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
This runs a command on each submodule in parallel and should eventually
replace `git submodule foreach`.
There is a new option -j/--jobs (inspired by make) to specify the number
of parallel threads.
The jobs=1 case needs to be special cases to exactly replicate the current
default behavior of `git submodule foreach` such as working stdin input.
git submodule foreach-parallel -j1 feels like a roundabout way to
say git submodule foreach; I want you to go parallel. Oh, not
really, I want you to do one thing at a time.
Eventually I want to drop the -parallel, such that git foreach ... will
just work as it always had, and if there is a --jobs argument we
may want to change the semantics a bit to make it work with the
piping to stdout/err.
I do not think these two have to be equivalent---users who need the
traditional one-by-one foreach behaviour (including the standard
input and other aspects that are unreasonable to expect
foreach-parallel -jN to replicate) can and should choose to use
foreach, not foreach-parallel -j1.
In any case, I do not think I saw any special casing of -j1 case that
attempts to leave the standard input operational. Did I miss
something, or is the above describing what is left to do?
It is the later, I am not quite confident yet about this very patch,
but I was rather waiting for feedback what people deem is important.
For more than one job there is no input possible and the output of both
stdout/stderr of the command are put into the stderr in an ordered fashion,
i.e. the tasks to not intermingle their output in races.
To rephrase what I said earlier, for parallel version, the above
things happen, even with numthreads==1, is perfectly fine.
+ cp-no_stdin = 1;
+ cp-out = 0;
+ cp-err = -1;
+ cp-dir = args-path;
+ cp-stdout_to_stderr = 1;
So the standard error and the standard output are mixed to a single
pipe ...
I was very focused on fetch, which would report progress and
information to stderr only.
+ 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));
+ if (len 0)
+ die(Read from child failed);
+ else if (len == 0)
+ break;
+ else {
+ strbuf_add(out, buf, len);
+ }
... and the whole thing is accumulated in core???
The pipes have a limit, so we need to empty them to prevent back-pressure?
And because we want to have the output of one task at a time, we need to
save it up until we can put out the whole output, no?
+ }
+ if (finish_command(cp))
+ die(command died with error);
+
+ sem_wait(args-mutex);
+ fputs(out.buf, stderr);
+ sem_post(args-mutex);
... and emitted to standard error?
I would have expected that the standard error would be left alone
`git fetch` which may be a good candidate for such an operation
provides progress on stderr, and we don't want to intermingle
2 different submodule fetch progress displays
(I need to work offline for a bit, so let me get all of the latest stuff,
so I'll run `git submodule foreach -j 16 -- git fetch --all though ideally
we want to have `git fetch --recurse-submodules -j16` instead )
(i.e. letting warnings from multiple jobs to be mixed together
simply because everybody writes to the same file descriptor), while
the standard output would be line-buffered, perhaps captured by the
above loop and then emitted under mutex, or something.
I think I said this earlier, but latency to the first output counts
to the first stderr
in this case?
So you would want one channel (stderr) for a fast reporting possibility
and another channel (stdout) for a well ordered output mode to cover
both options. And the command to be run must adhere to the our
selection of stderr for fast reporting and stdout for output to wait on.
as an activity feedback mechanism.
+ if (module_list_compute(0, nullargv, NULL, pathspec) 0)
+ return 1;
+
+ gitmodules_config();
+
+ aq = create_task_queue(number_threads);
+
+ 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))
+
Re: Git's inconsistent command line options
On Tue, Aug 25, 2015 at 8:13 AM, Junio C Hamano gits...@pobox.com wrote: On Tue, Aug 25, 2015 at 1:01 AM, Graeme Geldenhuys grae...@gmail.com wrote: Even though I have worked with Git since 2009, I still have to reference the help to remind me of what parameter to use in certain situation simply because similar tasks differ so much. Maybe we could address this in the next major version of Git? Has anybody else thought about this or started work on this? Or was this discussed before and declined (link?). http://article.gmane.org/gmane.comp.version-control.git/231478 comes to mind, which has been linked from this entry: Discuss and decide if we want to choose between the mode word UI (e.g. git submodule add) and the mode option UI (e.g. git tag --delete) and standardise on one; if it turns out to be a good idea, devise the migration plan to break the backward-compatibility. in http://git-blame.blogspot.com/p/leftover-bits.html I would vote for command words, as this is clean and simple. The downside is in converting all the old options based commands, git-tag, and similar. These commands cannot easily convert because valid sequences would become invalid with no easy way to deprecate for example in the linked gmane above, git tag delete master can't be a call to delete master as it is currently a call to create a tag delete at the commit marked by master. I can't think of an easy way to deprecate the change in behavior over time, which means that making a conversion would require some other as yet unknown way? It may be possible to convert other options based commands, such as how git-branch and git-checkout do things which seem highly unrelated. A good example is how checkout is used to both change branches, as well as can create a branch, and can also checkout a file. The reset command is used to rewind history, as well as potentially reset *all* files, but it can't be used to reset a single file, and is completely different from revert. Some of these distinctions are ok because it's just no good way to make everything easy. Some of these could be fixed by the command word setup, but as many have mentioned an actual migration plan is difficult. Personally, I don't want to move to the command option --status format, as I think these aren't really options, and are very much sub-subcommands. I think we should try to push more uses of this style, and try to determine a possible migration path towards using them. Maybe some warts simply aren't worth the effort to fix though. Other issues are tricker to solve, and are result of git exposing more complex functionality and users eventually simply have to learn and understand. 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: [PATCH 3/5] submodule: helper to run foreach in parallel
Stefan Beller sbel...@google.com writes:
This runs a command on each submodule in parallel and should eventually
replace `git submodule foreach`.
There is a new option -j/--jobs (inspired by make) to specify the number
of parallel threads.
The jobs=1 case needs to be special cases to exactly replicate the current
default behavior of `git submodule foreach` such as working stdin input.
git submodule foreach-parallel -j1 feels like a roundabout way to
say git submodule foreach; I want you to go parallel. Oh, not
really, I want you to do one thing at a time.
I do not think these two have to be equivalent---users who need the
traditional one-by-one foreach behaviour (including the standard
input and other aspects that are unreasonable to expect
foreach-parallel -jN to replicate) can and should choose to use
foreach, not foreach-parallel -j1.
In any case, I do not think I saw any special casing of -j1 case that
attempts to leave the standard input operational. Did I miss
something, or is the above describing what is left to do?
For more than one job there is no input possible and the output of both
stdout/stderr of the command are put into the stderr in an ordered fashion,
i.e. the tasks to not intermingle their output in races.
To rephrase what I said earlier, for parallel version, the above
things happen, even with numthreads==1, is perfectly fine.
+ cp-no_stdin = 1;
+ cp-out = 0;
+ cp-err = -1;
+ cp-dir = args-path;
+ cp-stdout_to_stderr = 1;
So the standard error and the standard output are mixed to a single
pipe ...
+ 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));
+ if (len 0)
+ die(Read from child failed);
+ else if (len == 0)
+ break;
+ else {
+ strbuf_add(out, buf, len);
+ }
... and the whole thing is accumulated in core???
+ }
+ if (finish_command(cp))
+ die(command died with error);
+
+ sem_wait(args-mutex);
+ fputs(out.buf, stderr);
+ sem_post(args-mutex);
... and emitted to standard error?
I would have expected that the standard error would be left alone
(i.e. letting warnings from multiple jobs to be mixed together
simply because everybody writes to the same file descriptor), while
the standard output would be line-buffered, perhaps captured by the
above loop and then emitted under mutex, or something.
I think I said this earlier, but latency to the first output counts
as an activity feedback mechanism.
+ if (module_list_compute(0, nullargv, NULL, pathspec) 0)
+ return 1;
+
+ gitmodules_config();
+
+ aq = create_task_queue(number_threads);
+
+ 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-path = ce-name;
+ sub = submodule_from_path(null_sha1, args-path);
+ if (!sub)
+ die(No submodule mapping found in .gitmodules for path
'%s', args-path);
+
+ args-name = sub-name;
+ args-toplevel = xstrdup(xgetcwd());
+ args-cmd = argv;
+ args-mutex = mutex;
+ args-prefix = alternative_path;
+ add_task(aq, run_cmd_submodule, args);
+ }
+
+ finish_task_queue(aq, NULL);
This is very nice. Declare a task queue with N workers, throw bunch
of task to it and then wait for all of them to complete. Things
can't be simpler than that ;-). One thing that other callers of the
mechanism might want may be to plug and unplug the task queue, but
that can wait until the need arises.
--
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 4/5] index-pack: Use the new worker pool
Stefan Beller sbel...@google.com writes: Not sure I follow there. Original implementation: We have M threads sitting around the table, all of them trying to obtain food from the one bowl on the table and then eating it. Once the bowl is all eaten, we can stop. New pattern: One cook puts all the food items on the sushi-go-round-belt with a fancy plate and the threads grab them one by one still using locks (but they are internal to the belt). Are you saying we're content with just a bowl and everyone helps themselves for getting food? No. I am questioning how big overhead is for having the go-round-belt that must hold all dishes to be eaten, which did not exist in the original arrangement. -- 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] compat/inet_ntop.c: Use INET_ADDRSTRLEN and INET6_ADDRSTRLEN macroses
Hi Kirill,
On 2015-08-25 12:34, brillian...@inbox.ru wrote:
From: Brilliantov Kirill Vladimirovich brillian...@inbox.ru
Signed-off-by: Brilliantov Kirill Vladimirovich brillian...@inbox.ru
The commit message makes for an excellent place to fill in the reader on
information that is not obvious from reading the patch. For example...
diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index 90b7cc4..fcd3b15 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -47,7 +47,7 @@ static const char *
inet_ntop4(const u_char *src, char *dst, size_t size)
{
static const char fmt[] = %u.%u.%u.%u;
- char tmp[sizeof 255.255.255.255];
+ char tmp[INET_ADDRSTRLEN];
Given that we are in compat/ space, i.e. in a space meant for platforms lacking
POSIX functions and definitions, is it really safe to assume that
INET_ADDRSTRLEN is available there?
@@ -78,7 +78,7 @@ inet_ntop6(const u_char *src, char *dst, size_t size)
* Keep this in mind if you think this function should have been coded
* to use pointer overlays. All the world's not a VAX.
*/
- char tmp[sizeof ::::::255.255.255.255], *tp;
+ char tmp[INET6_ADDRSTRLEN], *tp;
Are we really, absolutely sure that INET6_ADDRSTRLEN is defined?
Also I wonder: what problem does this patch solve? Is the current way to
determine the size of the `tmp` array incorrect?
Please use the commit message space to answer at least these questions.
Ciao,
Johannes
--
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:28 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). I had already commented on a preliminary version of this series off-list. I think all my previous comments have been taken into account. Thanks, both. I think this is pretty close to being satisfactory ;-) There may probably be a handful of minor nits like the above that need to be addressed, but I do not think I saw anything glaringly wrong that makes the series unsalvageable. It was a very pleasant read. It's almost there, and I am very happy to see how this and other series evolved so far ;-) Having said all that, it seems that there is some trivial typo or thinko in the formatting code to break t7004. Here is what I see... ok 98 - verifying rfc1991 signature expecting success: echo rfc1991 gpghome/gpg.conf echo rfc1991-signed-tag RFC1991 signed tag expect git tag -l -n1 rfc1991-signed-tag actual test_cmp expect actual git tag -l -n2 rfc1991-signed-tag actual test_cmp expect actual git tag -l -n999 rfc1991-signed-tag actual test_cmp expect actual --- expect 2015-08-24 22:54:44.607272653 + +++ actual 2015-08-24 22:54:44.611272643 + @@ -1 +1 @@ -rfc1991-signed-tag RFC1991 signed tag +rfc1991-signed-tagRFC1991 signed tag not ok 99 - list tag with rfc1991 signature Thats weird, I just ran all tests, and nothing failed. Is this mid-way of the patch series? I tried it on the entire series and seems fine to me -- 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 04/12] ref-filter: implement an `align` atom
On Tue, Aug 25, 2015 at 3:43 AM, Junio C Hamano gits...@pobox.com wrote:
Karthik Nayak karthik@gmail.com writes:
+static void perform_quote_formatting(struct strbuf *s, const char *str, int
quote_style);
+
+static void end_atom_handler(struct atom_value *atomv, struct
ref_formatting_state *state)
+{
+ struct ref_formatting_stack *current = state-stack;
+ struct strbuf s = STRBUF_INIT;
+
+ if (!current-at_end)
+ die(_(format: `end` atom used without a supporting atom));
+ current-at_end(current);
+ /*
+ * Whenever we have more than one stack element that means we
+ * are using a certain modifier atom. In that case we need to
+ * perform quote formatting.
+ */
+ if (!state-stack-prev-prev) {
The comment and the condition seem to be saying opposite things.
The code says If the stack only has one prev that is the very
initial one, then we do the quoting, i.e. the result of expanding
the enclosed string in %(start-something)...%(end) is quoted only
when that appears at the top level, which feels more correct than
the comment that says if we are about to pop after seeing the
first %(end) in %(start)...%(another)...%(end)...%(end) sequence,
we quote what is between %(another)...%(end).
That sounds misleading indeed will need to change that.
+ perform_quote_formatting(s, current-output.buf,
state-quote_style);
+ strbuf_reset(current-output);
+ strbuf_addbuf(current-output, s);
+ }
+ strbuf_release(s);
+ pop_stack_element(state-stack);
+}
+
@@ -1228,29 +1315,33 @@ void ref_array_sort(struct ref_sorting *sorting,
struct ref_array *array)
qsort(array-items, array-nr, sizeof(struct ref_array_item *),
compare_refs);
}
-static void append_atom(struct atom_value *v, struct ref_formatting_state
*state)
+static void perform_quote_formatting(struct strbuf *s, const char *str, int
quote_style)
{
- struct strbuf *s = state-stack-output;
-
- switch (state-quote_style) {
+ switch (quote_style) {
case QUOTE_NONE:
- strbuf_addstr(s, v-s);
+ strbuf_addstr(s, str);
break;
case QUOTE_SHELL:
- sq_quote_buf(s, v-s);
+ sq_quote_buf(s, str);
break;
case QUOTE_PERL:
- perl_quote_buf(s, v-s);
+ perl_quote_buf(s, str);
break;
case QUOTE_PYTHON:
- python_quote_buf(s, v-s);
+ python_quote_buf(s, str);
break;
case QUOTE_TCL:
- tcl_quote_buf(s, v-s);
+ tcl_quote_buf(s, str);
break;
}
}
+static void append_atom(struct atom_value *v, struct ref_formatting_state
*state)
+{
+ struct strbuf *s = state-stack-output;
+ perform_quote_formatting(s, v-s, state-quote_style);
Hmmm, do we want to unconditionally do the quote here, or only when
we are not being captured by upcoming %(end) to be consistent with
the behaviour of end_atom_handler() above?
@@ -1307,7 +1398,18 @@ void show_ref_array_item(struct ref_array_item *info,
const char *format, int qu
if (cp sp)
append_literal(cp, sp, state);
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep),
atomv);
- append_atom(atomv, state);
+ /*
+ * If the atom is a modifier atom, then call the handler
function.
+ * Else, if this is the first element on the stack, then we
need to
+ * format the atom as per the given quote. Else we just add
the atom value
+ * to the current stack element and handle quote formatting at
the end.
+ */
+ if (atomv-handler)
+ atomv-handler(atomv, state);
+ else if (!state.stack-prev)
+ append_atom(atomv, state);
+ else
+ strbuf_addstr(state.stack-output, atomv-s);
Ahh, this explains why you are not doing it above, but I do not
think if this is a good division of labor.
You can see that I expected that if !state.stack-prev check to be
inside append_atom(), and I would imagine future readers would have
the same expectation when reading this code. I.e.
append_atom(struct atom_value *v, struct ref_f_s *state)
{
if (state-stack-prev)
strbuf_addstr(state-stack-output, v-s);
else
quote_format(state-stack-output, v-s,
state-quote_style);
}
The end result may be the same, but I do think append_atom is to
always quote, so we do an unquoted appending by hand is a bad way
to do this.
Moreover, notice that the function signature of append_atom() is
exactly the same as atomv-handler's. I wonder if it would be
easier to understand if you made
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
On Tue, Aug 25, 2015 at 12:23 AM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 10:57 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com 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. Thanks. I'm not sure I checked this condition, will have a look at this, thanks for pointing it out. 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). I'm not so sure about that, I'll get back on this. (I didn't think about empty string alignment and its effect on %(if:empty) and so on). -- 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 04/12] ref-filter: implement an `align` atom
On Tue, Aug 25, 2015 at 12:17 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
Junio C Hamano gits...@pobox.com writes:
You can see that I expected that if !state.stack-prev check to be
inside append_atom(), and I would imagine future readers would have
the same expectation when reading this code. I.e.
append_atom(struct atom_value *v, struct ref_f_s *state)
{
if (state-stack-prev)
strbuf_addstr(state-stack-output, v-s);
else
quote_format(state-stack-output, v-s,
state-quote_style);
}
The end result may be the same,
There's another call to append_atom() when inserting the reset color at
end of line if needed, so moving this if inside append_atom means we
would do the check also for the reset color. It would not change the
behavior (by construction, we insert it only when the stack has only the
initial element), so it's OK.
I agree that this is a good thing to do.
Moreover, notice that the function signature of append_atom() is
exactly the same as atomv-handler's. I wonder if it would be
easier to understand if you made append_atom() the handler for a
non-magic atoms, which would let you do the above without any if/else
and just a single unconditional
I can't decide between ah, very elegant and no, too much magic ;-).
I lean towards the former.
I like the idea of using atomv-handler() a lot, mostly cause this
would eventually
help us clean up populate_atom() which currently seems like a huge dump of code.
--
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 04/12] ref-filter: implement an `align` atom
On Tue, Aug 25, 2015 at 3:45 AM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
Karthik Nayak karthik@gmail.com writes:
+static void end_atom_handler(struct atom_value *atomv, struct
ref_formatting_state *state)
+{
+struct ref_formatting_stack *current = state-stack;
+struct strbuf s = STRBUF_INIT;
+
+if (!current-at_end)
+die(_(format: `end` atom used without a supporting atom));
+current-at_end(current);
+/*
+ * Whenever we have more than one stack element that means we
+ * are using a certain modifier atom. In that case we need to
+ * perform quote formatting.
+ */
+if (!state-stack-prev-prev) {
The comment and the condition seem to be saying opposite things.
The code says If the stack only has one prev that is the very
initial one, then we do the quoting, i.e. the result of expanding
the enclosed string in %(start-something)...%(end) is quoted only
when that appears at the top level, which feels more correct...
As this already allows us to use nested construct, I think we would
want to have test that uses
%(align:30,left)%(align:20,right)%(refname:short)%(end)%(end)
or something like that ;-)
Very nice.
Ok, will do that :)
--
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 25 August 2015 at 16:43, Junio C Hamano gits...@pobox.com 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)? The default would be 2 (since we're already at Git 2.x). Git commands could check for this setting and abort/introduce/prevent/change behaviour/functionality as appropriate. During Git 2.x the API 2 would be the default but users could explicitly request 3 in preparation of Git 3.x. (With the knowledge that API 3 would still be [to some extent at least] in flux.) API 2 could start warning about future changes where appropriate. With the introduction of Git 3.x, the default would become API 3 but users could still request API 2. Then for Git 4.x the default would go to 4, with an option to request 3 but 2 would no longer be supported (and all code supporting API 2 could be removed). 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? -- 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: [FEATURE REQUEST] Filter-branch extend progress with a simple estimated time remaning
On Tue, Aug 25, 2015 at 04:12:54PM -0400, Eric Sunshine wrote:
A little googling came up with:
awk 'END { print systime() }' /dev/null
which probably (?) works everywhere.
On Mac OS X and FreeBSD:
$ awk 'END { print systime() }' /dev/null
awk: calling undefined function systime
source line number 1
$
Oh, well. The reference I saw was that the old Kernighan nawk had it,
but that seems not to be the case:
http://www.cs.princeton.edu/~bwk/btl.mirror/
date +%s seems to work on OS X, and so presumably on other BSDs. No
clue what would work on stuff like SunOS, AIX, etc.
-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
