[PATCH] completion: remove 'git column' from porcelain commands

[SZEDER Gábor]

'git column' is an internal helper, so it should not be offered on
'git ' along with porcelain commands.

Signed-off-by: SZEDER Gábor 
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 111b05302b..5dec8778f7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -664,6 +664,7 @@ __git_list_porcelain_commands ()
check-mailmap): plumbing;;
check-ref-format) : plumbing;;
checkout-index)   : plumbing;;
+   column)   : internal helper;;
commit-tree)  : plumbing;;
count-objects): infrequent;;
credential)   : credentials;;
-- 
2.6.3.420.g7bbb372

--
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

[PATCHv2] Makefile: add missing phony target

[Elia Pinto]

Add some missing phony target to Makefile. Also put the .PHONY
declaration immediately before the target declaration, where necessary,
for a better readability and a uniform style.

Signed-off-by: Elia Pinto 
Helped-by: Matthieu Moy 
---
This is the second version of this patch.
Added the corrections suggested by Matthieu Moy ($gmane/282221)

 Makefile | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 43ceeb9..afd2358 100644
--- a/Makefile
+++ b/Makefile
@@ -534,7 +534,7 @@ install-perl-script: $(SCRIPT_PERL_INS)
 install-python-script: $(SCRIPT_PYTHON_INS)
$(INSTALL) $^ '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
 
-.PHONY: clean-perl-script clean-sh-script clean-python-script
+.PHONY: clean-sh-script clean-perl-script clean-python-script
 clean-sh-script:
$(RM) $(SCRIPT_SH_GEN)
 clean-perl-script:
@@ -1644,6 +1644,9 @@ shell_compatibility_test: 
please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
$(STRIP) $(STRIP_OPTS) $^
 
+.PHONY: all profile profile-fast
+.PHONY: please_set_SHELL_PATH_to_a_more_modern_shell shell_compatibility_test 
strip
+
 ### Target-specific flags and dependencies
 
 # The generic compilation pattern rule and automatically
@@ -2011,6 +2014,7 @@ $(VCSSVN_LIB): $(VCSSVN_OBJS)
 
 export DEFAULT_EDITOR DEFAULT_PAGER
 
+.PHONY: doc man html info pdf
 doc:
$(MAKE) -C Documentation all
 
@@ -2054,6 +2058,7 @@ po/git.pot: $(GENERATED_H) FORCE
$(LOCALIZED_PERL)
mv $@+ $@
 
+.PHONY: pot
 pot: po/git.pot
 
 POFILES := $(wildcard po/*.po)
@@ -2074,6 +2079,7 @@ $(ETAGS_TARGET): FORCE
$(RM) $(ETAGS_TARGET)
$(FIND_SOURCE_FILES) | xargs etags -a -o $(ETAGS_TARGET)
 
+.PHONY: FORCE cscope
 tags: FORCE
$(RM) tags
$(FIND_SOURCE_FILES) | xargs ctags -a
@@ -2189,6 +2195,7 @@ export NO_SVN_TESTS
 export TEST_NO_MALLOC_CHECK
 
 ### Testing rules
+.PHONY: test perf
 
 test: all
$(MAKE) -C t/ all
@@ -2196,7 +2203,6 @@ test: all
 perf: all
$(MAKE) -C t/perf/ all
 
-.PHONY: test perf
 
 test-ctype$X: ctype.o
 
@@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a
 test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
$(filter %.a,$^) $(LIBS)
 
+.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1
 check-sha1:: test-sha1$X
./test-sha1.sh
 
@@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
$(SPARSE_FLAGS) $<
 
-.PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
 check: common-cmds.h
@@ -2237,6 +2243,7 @@ check: common-cmds.h
exit 1; \
fi
 
+
 ### Installation rules
 
 ifneq ($(filter /%,$(firstword $(template_dir))),)
@@ -2263,6 +2270,7 @@ mergetools_instdir_SQ = $(subst 
','\'',$(mergetools_instdir))
 
 install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) 
$(BINDIR_PROGRAMS_NO_X)
 
+.PHONY: profile-install profile-fast-install install
 profile-install: profile
$(MAKE) install
 
@@ -2329,6 +2337,8 @@ endif
done && \
./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
 
+.PHONY: install-gitweb install-man install-html install-info install-pdf
+.PHONY: quick-install-doc quick-install-man quick-install-html
 install-gitweb:
$(MAKE) -C gitweb install
 
@@ -2365,6 +2375,8 @@ git.spec: git.spec.in GIT-VERSION-FILE
mv $@+ $@
 
 GIT_TARNAME = git-$(GIT_VERSION)
+
+.PHONY: dist rpm dist-doc  distclean profile-clean clean
 dist: git.spec git-archive$(X) configure
./git-archive --format=tar \
--prefix=$(GIT_TARNAME)/ HEAD^{tree} > $(GIT_TARNAME).tar
@@ -2445,9 +2457,6 @@ endif
$(RM) GIT-USER-AGENT GIT-PREFIX
$(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
 
-.PHONY: all install profile-clean clean strip
-.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-.PHONY: FORCE cscope
 
 ### Check documentation
 #
@@ -2499,9 +2508,9 @@ check-builtins::
 
 ### Test suite coverage testing
 #
-.PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
-.PHONY: coverage-clean-results
 
+.PHONY: coverage coverage-clean coverage-compile coverage-test coverage-report
+.PHONY: coverage-clean-results coverage-untested-functions cover_db 
cover_db_html
 coverage:
$(MAKE) coverage-test
$(MAKE) coverage-untested-functions
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] Case insensitive URL rewrite

[Lars Schneider]

Hi,

the "url..insteadOf" git config is case sensitive. I understand that 
this makes sense on case sensitive file systems. However, URLs are mostly case 
insensitive:

Consider this:
git clone https://GIThub.com/GIT/GIT
git clone https://github.com/git/git

Both commands will clone the same repository.

Interestingly enough this works, too:
git clone g...@github.com:GIT/GIT

What do you think about a flag that makes these rewrites case insensitive? E.g. 
with the following config flag:

[url ""]
insteadOf = 
ignorecase = true

Thanks,
Lars

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] Makefile: add missing phony target

[Matthieu Moy]

Elia Pinto  writes:

> This is the second version of this patch.
> Added the corrections suggested by Matthieu Moy ($gmane/282221)

Sorry, but my main concern was that the patch could not be reviewed in
good conditions as-is, and I think it still cannot be. It's very hard to
spot which .PHONY rules you're adding and which are just code movement.
You should really split this into one "code movement" patch and one
"actual bugfix" patch. Or someone with better eyes than me should review
the patch ;-).

> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a
>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
>   $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
> $(filter %.a,$^) $(LIBS)
>  
> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1
>  check-sha1:: test-sha1$X
>   ./test-sha1.sh
>  
> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>   $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
>   $(SPARSE_FLAGS) $<
>  
> -.PHONY: sparse $(SP_OBJ)
>  sparse: $(SP_OBJ)

This "sparse" movement looks again contradictory with the goal announced
in the commit message.

> @@ -2237,6 +2243,7 @@ check: common-cmds.h
>   exit 1; \
>   fi
>  
> +
>  ### Installation rules

Useless hunk.

-- 
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: [RFC] Case insensitive URL rewrite

[Daniel Stenberg]

On Fri, 11 Dec 2015, Lars Schneider wrote:

the "url..insteadOf" git config is case sensitive. I understand 
that this makes sense on case sensitive file systems. However, URLs are 
mostly case insensitive:


Minor detail here perhaps, but...

I would object to URLs being "mostly case insensitive", even if github 
apparently seems to work that way. The path part of URLs are rarely case 
insensitive since it tends to map to a *nix file system, while the host name 
part is insensitive as section 3.2.2 in RFC 3986 says.


--

 / daniel.haxx.se
--
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: [PATCHv2] Makefile: add missing phony target

[Elia Pinto]

2015-12-11 15:40 GMT+01:00 Matthieu Moy :
> Elia Pinto  writes:
>
>> This is the second version of this patch.
>> Added the corrections suggested by Matthieu Moy ($gmane/282221)
>
> Sorry, but my main concern was that the patch could not be reviewed in
> good conditions as-is, and I think it still cannot be. It's very hard to
> spot which .PHONY rules you're adding and which are just code movement.
> You should really split this into one "code movement" patch and one
> "actual bugfix" patch. Or someone with better eyes than me should review
> the patch ;-).

Ok. No problem. I thought there was no need for a patch so simple. But ok.

Thank you. I will reroll.
>
>> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a
>>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
>>   $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) 
>> $(filter %.a,$^) $(LIBS)
>>
>> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1
>>  check-sha1:: test-sha1$X
>>   ./test-sha1.sh
>>
>> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>>   $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
>>   $(SPARSE_FLAGS) $<
>>
>> -.PHONY: sparse $(SP_OBJ)
>>  sparse: $(SP_OBJ)
>
> This "sparse" movement looks again contradictory with the goal announced
> in the commit message.
The idea was to group all the phony before all the target, not to put
the phony necessarily before the closest target. but ok
>
>> @@ -2237,6 +2243,7 @@ check: common-cmds.h
>>   exit 1; \
>>   fi
>>
>> +
My bad :=)
>>  ### Installation rules
>
> Useless hunk.
>
> --
> 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: [PATCHv2] Makefile: add missing phony target

[Matthieu Moy]

Elia Pinto  writes:

> 2015-12-11 15:40 GMT+01:00 Matthieu Moy :
>> Elia Pinto  writes:
>>
>>> This is the second version of this patch.
>>> Added the corrections suggested by Matthieu Moy ($gmane/282221)
>>
>> Sorry, but my main concern was that the patch could not be reviewed in
>> good conditions as-is, and I think it still cannot be. It's very hard to
>> spot which .PHONY rules you're adding and which are just code movement.
>> You should really split this into one "code movement" patch and one
>> "actual bugfix" patch. Or someone with better eyes than me should review
>> the patch ;-).
>
> Ok. No problem. I thought there was no need for a patch so simple. But ok.

The point is: once a tricky bug was found in a patch (and I did on v1),
you cannot claim anymore that it is "so simple". If it was that simple,
you would have cought it before sending.

>>> @@ -2215,6 +2221,7 @@ test-svn-fe$X: vcs-svn/lib.a
>>>  test-%$X: test-%.o GIT-LDFLAGS $(GITLIBS)
>>>   $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter 
>>> %.o,$^) $(filter %.a,$^) $(LIBS)
>>>
>>> +.PHONY: check_sha1 $(SP_OBJ) sparse check check-sha1
>>>  check-sha1:: test-sha1$X
>>>   ./test-sha1.sh
>>>
>>> @@ -2224,7 +2231,6 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>>>   $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
>>>   $(SPARSE_FLAGS) $<
>>>
>>> -.PHONY: sparse $(SP_OBJ)
>>>  sparse: $(SP_OBJ)
>>
>> This "sparse" movement looks again contradictory with the goal announced
>> in the commit message.
> The idea was to group all the phony before all the target, not to put
> the phony necessarily before the closest target. but ok

I personally prefer the old way. I have no strong objection to changing,
but currently your commit message says "Also put the .PHONY declaration
immediately before the target declaration", which is clearly not a
justification to do this change.

-- 
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

sb/submodule-parallel-fetch, was: Re: What's cooking in git.git (Dec 2015, #03; Thu, 10)

[Johannes Sixt]

Am 11.12.2015 um 00:55 schrieb Stefan Beller:
> On Thu, Dec 10, 2015 at 3:51 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
 * sb/submodule-parallel-fetch (2015-11-24) 17 commits
 ...
>>>
>>> I assume you plan on merging this after 2.7 settled and then we can
>>> also get the above sb/submodule-parallel-update going again.
>>
>> Yeah, thanks for reminding me.  I think that would be a good plan
>> that gives us an opportunity to clean up this topic, some parts of
>> which are have "an early patch that was too hastily merged to 'next'
>> had to be tweaked by an 'oops' follow-up patch in the topic"
>> pattern, e.g. "make waitpid the secondary and closed pipe the
>> primary way to monitor children".
>>
>> Thanks.
> 
> This makes it sound as if you would drop it from next once 2.7 is out,
> expecting a complete reroll, which does the right thing from the beginning?
> That was not was I was expecting, but thanks for clarifying.

Also, rebuilding the topic such that it takes the direct route to its
current state would help bisectability on Windows.

Generally, I'm already quite satisfied with the state of the
infrastructure at the tip of the branch.

Nevertheless, I have a few niggles.

The primary one is that we are using a global variable of type
struct parallel_processes to keep track of the processes. Fortunately,
most functions already take a pointer (I gather you did anticipate an
object oriented use of the functions). The only exception is pp_init():
It returns a pointer to the global object, which is then passed around
to the other functions. This does not conform to our usual style,
however, where the initializer function takes a pointer to the object,
too.

After converting pp_init, we can have a nicely object oriented
collection of functions and get rid of the global object ... almost.

We still need a global variable for the signal handler. But since
signals and their handlers are a global resource, it is not that bad to
have a global variable that is dedicated to signal handling.

Another small nit is that I found it confusing that the closure
parameters are arranged differently in the callback functions. Granted,
in get_next_task one of them is an out parameter, but that is actually
an argument to move it to the end of the parameter list, IMHO.

On top of that I found an error or two in the documentation, and I have
a few suggestions for improvements.

All this is summarized in the patch below.

diff --git a/run-command.c b/run-command.c
index db4d916..f3addb9 100644
--- a/run-command.c
+++ b/run-command.c
@@ -864,7 +864,7 @@ enum child_state {
GIT_CP_WAIT_CLEANUP,
 };
 
-static struct parallel_processes {
+struct parallel_processes {
void *data;
 
int max_processes;
@@ -890,7 +890,7 @@ static struct parallel_processes {
 
int output_owner;
struct strbuf buffered_output; /* of finished children */
-} parallel_processes_struct;
+};
 
 static int default_start_failure(struct child_process *cp,
 struct strbuf *err,
@@ -933,23 +933,23 @@ static void kill_children(struct parallel_processes *pp, 
int signo)
kill(pp->children[i].process.pid, signo);
 }
 
+static struct parallel_processes *pp_for_signal;
+
 static void handle_children_on_signal(int signo)
 {
-   struct parallel_processes *pp = _processes_struct;
-
-   kill_children(pp, signo);
+   kill_children(pp_for_signal, signo);
sigchain_pop(signo);
raise(signo);
 }
 
-static struct parallel_processes *pp_init(int n,
- get_next_task_fn get_next_task,
- start_failure_fn start_failure,
- task_finished_fn task_finished,
- void *data)
+static void pp_init(struct parallel_processes *pp,
+   int n,
+   get_next_task_fn get_next_task,
+   start_failure_fn start_failure,
+   task_finished_fn task_finished,
+   void *data)
 {
int i;
-   struct parallel_processes *pp = _processes_struct;
 
if (n < 1)
n = online_cpus();
@@ -976,8 +976,9 @@ static struct parallel_processes *pp_init(int n,
pp->pfd[i].events = POLLIN | POLLHUP;
pp->pfd[i].fd = -1;
}
+
+   pp_for_signal = pp;
sigchain_push_common(handle_children_on_signal);
-   return pp;
 }
 
 static void pp_cleanup(struct parallel_processes *pp)
@@ -1019,10 +1020,10 @@ static int pp_start_one(struct parallel_processes *pp)
if (i == pp->max_processes)
die("BUG: bookkeeping is hard");
 
-   code = pp->get_next_task(>children[i].data,
->children[i].process,
+   code = pp->get_next_task(>children[i].process,
 

Re: [PATCH 04/16] refs: add do_for_each_per_worktree_ref

[Junio C Hamano]

David Turner  writes:

> Alternate refs backends might still use files to store per-worktree
> refs.  So the files backend's ref-loading infrastructure should be
> available to those backends, just for use on per-worktree refs.  Add
> do_for_each_per_worktree_ref, which iterates over per-worktree refs.

Is this "might still use"?  I am instead getting an impression that,
we are declaring that the recommended way to store per-worktree refs
is with filesystem backend.

Not complaining against either such a design decision (if that is
what is made here) or the above description in the log message--just
want to understand the intention better.

I also wonder if it is cleaner to have a single interface that lets
anybody ask a pointer to the backend struct by name.  With such an
interface, a new backend that wants to delegate per-worktree refs to
files backend could

static int DB_for_each_ref(...)
{
struct ref_be *files = locate_ref_backend("files");
files->for_each_ref(... | WORKTREE_ONLY, ...);
... do its own enumeration of non-per-worktree refs ...
}

and such a delegation does not need to be limited to per-worktree
iteration.

Or is that a road to expose too much implementation details of
a backend to other backends?

> Signed-off-by: David Turner 
> ---
>  refs/files-backend.c | 15 ---
>  refs/refs-internal.h | 10 ++
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index d4bd6cf..bde4892 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -518,9 +518,6 @@ static void sort_ref_dir(struct ref_dir *dir)
>   dir->sorted = dir->nr = i;
>  }
>  
> -/* Include broken references in a do_for_each_ref*() iteration: */
> -#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
> -
>  /*
>   * Return true iff the reference described by entry can be resolved to
>   * an object in the database.  Emit a warning if the referred-to
> @@ -568,6 +565,10 @@ static int do_one_ref(struct ref_entry *entry, void 
> *cb_data)
>   struct ref_entry *old_current_ref;
>   int retval;
>  
> + if (data->flags & DO_FOR_EACH_PER_WORKTREE_ONLY &&
> + ref_type(entry->name) != REF_TYPE_PER_WORKTREE)
> + return 0;
> +
>   if (!starts_with(entry->name, data->base))
>   return 0;
>  
> @@ -1738,6 +1739,14 @@ static int do_for_each_ref(struct ref_cache *refs, 
> const char *base,
>   return do_for_each_entry(refs, base, do_one_ref, );
>  }
>  
> +int do_for_each_per_worktree_ref(const char *submodule, const char *base,
> +  each_ref_fn fn, int trim, int flags,
> +  void *cb_data)
> +{
> + return do_for_each_ref(get_ref_cache(submodule), base, fn, trim,
> +flags | DO_FOR_EACH_PER_WORKTREE_ONLY, cb_data);
> +}
> +
>  static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
>  {
>   struct object_id oid;
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index ad683df..433d0fe 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -42,6 +42,16 @@
>   * value to ref_update::flags
>   */
>  
> +/* Include broken references in a do_for_each_ref*() iteration */
> +#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
> +
> +/* Only include per-worktree refs in a do_for_each_ref*() iteration */
> +#define DO_FOR_EACH_PER_WORKTREE_ONLY 0x02
> +
> +int do_for_each_per_worktree_ref(const char *submodule, const char *base,
> +  each_ref_fn fn, int trim, int flags,
> +  void *cb_data);
> +
>  /*
>   * Return true iff refname is minimally safe. "Safe" here means that
>   * deleting a loose reference by this name will not do any damage, 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

Re: sb/submodule-parallel-fetch, was: Re: What's cooking in git.git (Dec 2015, #03; Thu, 10)

[Stefan Beller]

On Fri, Dec 11, 2015 at 1:37 PM, Johannes Sixt  wrote:
>
> Generally, I'm already quite satisfied with the state of the
> infrastructure at the tip of the branch.
>

I was about the rework the patch series.

> Nevertheless, I have a few niggles.

If you don't mind I'll split your patch into chunks and
squash these where appropriate and resend the series
with the suggestions included without the intermediate stages
of non compiling code for Windows.

>
> The primary one is that we are using a global variable of type
> struct parallel_processes to keep track of the processes. Fortunately,
> most functions already take a pointer (I gather you did anticipate an
> object oriented use of the functions). The only exception is pp_init():
> It returns a pointer to the global object, which is then passed around
> to the other functions. This does not conform to our usual style,
> however, where the initializer function takes a pointer to the object,
> too.

Noted. I thought about the init function as a constructor,
such as

foo *pp = new foo();

in C++ would be just syntactic sugar for what I did there.
I'll adhere to Git style then instead.

>
> After converting pp_init, we can have a nicely object oriented
> collection of functions and get rid of the global object ... almost.
>
> We still need a global variable for the signal handler. But since
> signals and their handlers are a global resource, it is not that bad to
> have a global variable that is dedicated to signal handling.
>
> Another small nit is that I found it confusing that the closure
> parameters are arranged differently in the callback functions. Granted,
> in get_next_task one of them is an out parameter, but that is actually
> an argument to move it to the end of the parameter list, IMHO.
>
> On top of that I found an error or two in the documentation, and I have
> a few suggestions for improvements.
>
> All this is summarized in the patch below.

Thanks for the improvements!

Stefan
--
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] completion: fix completing unstuck email alias arguments

[SZEDER Gábor]

Completing unstuck form of email aliases doesn't quite work:

  $ git send-email --to 
  alice   bob cecil
  $ git send-email --to a
  alice   bob cecil

While listing email aliases works as expected, the second case should
just complete to 'alice', but it keeps offering all email aliases
instead.

The cause for this behavior is that in this case we mistakenly tell
__gitcomp() explicitly that the current word to be completed is empty,
while in reality it is not.  As a result __gitcomp() doesn't filter
out non-matching aliases, so all aliases end up being offered over and
over again.

Fix this by not passing the current word to be completed to
__gitcomp() and letting it go the default route and grab it from the
'$cur' variable.  Don't pass empty prefix either, because it's assumed
to be empty when unspecified, so it's not necessary.

Signed-off-by: SZEDER Gábor 
---

On top of the recently merged dfbe5eeb32 (completion: add support for
completing email aliases, 2015-11-19).

 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 111b05302b..d9b995799c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1716,7 +1716,7 @@ _git_send_email ()
--to|--cc|--bcc|--from)
__gitcomp "
$(git --git-dir="$(__gitdir)" send-email --dump-aliases 
2>/dev/null)
-   " "" ""
+   "
return
;;
esac
-- 
2.7.0.rc0.37.g77d69b9

--
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/RFC 06/10] strbuf: introduce strbuf_split_str_without_term()

[Junio C Hamano]

Karthik Nayak  writes:

>>> diff --git a/ref-filter.c b/ref-filter.c
>>> @@ -892,14 +892,11 @@ static void populate_value(struct ref_array_item *ref)
>>>  * TODO: Implement a function similar to 
>>> strbuf_split_str()
>>>  * which would omit the separator from the end of 
>>> each value.
>>>  */
>>> -   s = to_free = strbuf_split_str(valp, ',', 0);
>>> +   s = to_free = strbuf_split_str_without_term(valp, 
>>> ',', 0);
>>>
>>> align->position = ALIGN_LEFT;
>>>
>>> while (*s) {
>>> -   /*  Strip trailing comma */
>>> -   if (s[1])
>>> -   strbuf_setlen(s[0], s[0]->len - 1);
>>
>> I'd prefer to see this ref-filter.c change split out as a separate
>> patch so as not to pollute the otherwise single-purpose change
>> introduced by this patch (i.e. capability to omit the terminator).
>>
>> Also, it might make sense to move this patch to the head of the
>> series, since it's conceptually distinct from the rest of the patches,
>> and could conceivably prove useful on its own, regardless of how the
>> rest of the series fares.
>>
>
> I guess it makes sense to split this into two separate patches. I'll do that 
> and
> push it to the top of the series.

Sounds good.  I also notice that the "TODO: Implement a function
similar to..." we see in the precontext can now be removed, as that
is what is done in this patch?
--
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: sb/submodule-parallel-fetch,

[Junio C Hamano]

Stefan Beller  writes:

> On Fri, Dec 11, 2015 at 1:37 PM, Johannes Sixt  wrote:
>>
>> Generally, I'm already quite satisfied with the state of the
>> infrastructure at the tip of the branch.
>
> I was about the rework the patch series.

OK.

I think the very early part of the series, up to 8fc3f2ee (sigchain:
add command to pop all common signals, 2015-09-30), may be fine
as-is (i.e. just rebase on top of updated 'master').

The step after that, i.e. asynch processor, is the most interesting
and important one.  Since it was written, I think the improvements
that we want to be rolled into it from the beginning are:

 - do not rely on waitpid(-1);

 - no need for set_nonglocking(), squashing a4433fd4a and
   6f963a895a9 in;

 - correct the early-shutdown bug 79f38577 and again in 63ce47e1;

 - child_process_clear() in 1c53754a, which probably will become
   unnecessary if the series is rebuilt on top of updated 'master';

 - follow-up fixes and enhancements to the tests in c3a5d11 and
   74cc04d;

 - debugging support in 7eb93e91069 (from the other series that
   builds on top).

That would slim down not just the total number of patches, but also
the amount of the code that needs to be looked at (as we would not
add code only to later remove or fixup).
--
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 02/16] refs: add methods for misc ref operations

[Junio C Hamano]

David Turner  writes:

>  struct ref_be {
>   struct ref_be *next;
>   const char *name;
>   ref_transaction_commit_fn *transaction_commit;
> +
> + pack_refs_fn *pack_refs;
> + peel_ref_fn *peel_ref;
> + create_symref_fn *create_symref;
> +
> + resolve_ref_unsafe_fn *resolve_ref_unsafe;
> + verify_refname_available_fn *verify_refname_available;
> + resolve_gitlink_ref_fn *resolve_gitlink_ref;
>  };

This may have been pointed out in the previous reviews by somebody
else, but I think it is more customary to declare a struct member
that is a pointer to a customization function without leading '*',
i.e.

typedef TYPE (*customize_fn)(ARGS);

struct vtable {
...
cutomize_fn fn;
...
};

in our codebase (cf. string_list::cmp, prio_queue::compare).
--
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: sb/submodule-parallel-fetch, was: Re: What's cooking in git.git (Dec 2015, #03; Thu, 10)

[Johannes Sixt]

Am 11.12.2015 um 23:52 schrieb Stefan Beller:

If you don't mind I'll split your patch into chunks and
squash these where appropriate


I don't mind at all; please pick what you like.

-- Hannes

--
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 02/16] refs: add methods for misc ref operations

[Junio C Hamano]

David Turner  writes:

> diff --git a/cache.h b/cache.h
> index 51c35c3..707455a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -,6 +,13 @@ extern char *oid_to_hex(const struct object_id *oid);  
> /* same static buffer as s
>  extern int interpret_branch_name(const char *str, int len, struct strbuf *);
>  extern int get_sha1_mb(const char *str, unsigned char *sha1);
>  
> +/*
> + * Return true iff abbrev_name is a possible abbreviation for
> + * full_name according to the rules defined by ref_rev_parse_rules in
> + * refs.c.
> + */
> +extern int refname_match(const char *abbrev_name, const char *full_name);
> +
>  extern int validate_headref(const char *ref);

This hunk is strange.  It duplicates what is in refs.h, risking the
two copies to drift away from each other over time.

As the only callsites are in remote.c that includes refs.h, this
probably should be dropped.

Other changes that give redirection to various operations via vtable
looked all sensible.

>  struct ref_be {
>   struct ref_be *next;
>   const char *name;
>   ref_transaction_commit_fn *transaction_commit;
> +
> + pack_refs_fn *pack_refs;
> + peel_ref_fn *peel_ref;
> + create_symref_fn *create_symref;
> +
> + resolve_ref_unsafe_fn *resolve_ref_unsafe;
> + verify_refname_available_fn *verify_refname_available;
> + resolve_gitlink_ref_fn *resolve_gitlink_ref;
>  };
>  
>  #endif /* REFS_REFS_INTERNAL_H */
--
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 02/16] refs: add methods for misc ref operations

[David Turner]

On Fri, 2015-12-11 at 15:33 -0800, Junio C Hamano wrote:
> David Turner  writes:
> 
> > diff --git a/cache.h b/cache.h
> > index 51c35c3..707455a 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -,6 +,13 @@ extern char *oid_to_hex(const struct object_id 
> > *oid);/* same static buffer as s
> >  extern int interpret_branch_name(const char *str, int len, struct strbuf 
> > *);
> >  extern int get_sha1_mb(const char *str, unsigned char *sha1);
> >  
> > +/*
> > + * Return true iff abbrev_name is a possible abbreviation for
> > + * full_name according to the rules defined by ref_rev_parse_rules in
> > + * refs.c.
> > + */
> > +extern int refname_match(const char *abbrev_name, const char *full_name);
> > +
> >  extern int validate_headref(const char *ref);
> 
> This hunk is strange.  It duplicates what is in refs.h, risking the
> two copies to drift away from each other over time.
> 
> As the only callsites are in remote.c that includes refs.h, this
> probably should be dropped.

Will fix, 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 04/16] refs: add do_for_each_per_worktree_ref

[David Turner]

On Fri, 2015-12-11 at 15:52 -0800, Junio C Hamano wrote:
> David Turner  writes:
> 
> > Alternate refs backends might still use files to store per-worktree
> > refs.  So the files backend's ref-loading infrastructure should be
> > available to those backends, just for use on per-worktree refs.  Add
> > do_for_each_per_worktree_ref, which iterates over per-worktree refs.
> 
> Is this "might still use"?  I am instead getting an impression that,
> we are declaring that the recommended way to store per-worktree refs
> is with filesystem backend.
> 
> Not complaining against either such a design decision (if that is
> what is made here) or the above description in the log message--just
> want to understand the intention better.

I'm not super-happy with the notion of splitting data across backends;
it's a bit messy.  I've resigned myself to it because it seems like the
simplest way forward in the presence of per-worktree refs.  But I
figured the message could leave it open in case we change our minds.

> I also wonder if it is cleaner to have a single interface that lets
> anybody ask a pointer to the backend struct by name.  With such an
> interface, a new backend that wants to delegate per-worktree refs to
> files backend could
> 
>   static int DB_for_each_ref(...)
> {
>   struct ref_be *files = locate_ref_backend("files");
>   files->for_each_ref(... | WORKTREE_ONLY, ...);
>   ... do its own enumeration of non-per-worktree refs ...
>   }
> 
> and such a delegation does not need to be limited to per-worktree
> iteration.
> 
> Or is that a road to expose too much implementation details of
> a backend to other backends?

Given that the files backend is special in terms of its rights and
responsibilities, I think it's OK to just refer to it directly.  I think
allowing backends to generally pick and choose bits from other backends
would be confusing.


--
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/RFC 01/10] ref-filter: introduce a parsing function for each atom in valid_atom

[Junio C Hamano]

Karthik Nayak  writes:

>> This problem will go away if you introduce the 'valid_atom' field in
>> the patch which actually needs it (as suggested above) rather than in
>> this patch.
>
> Yup, agreed.
> Thanks for your suggestions.

In addition, most of the lines in this patch should become
unnecessary even after you start using the "parser" field.

As the majority of fields are compared as strings, and only a
selected few fields need custom parsers, you do not have to
explicitly write FIELD_STR everywhere, as long as you make sure the
value of FIELD_STR is zero (and use parser==NULL as a sign that no
custom parser is used, which I think you already 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: [PATCH/RFC 00/10] ref-filter: use parsing functions

[Junio C Hamano]

Karthik Nayak  writes:

> Karthik Nayak (10):
>   ref-filter: introduce a parsing function for each atom in valid_atom
>   ref-filter: introduce struct used_atom
>   ref-fitler: bump match_atom() name to the top
>   ref-filter: skip deref specifier in match_atom_name()
>   ref-filter: introduce color_atom_parser()
>   strbuf: introduce strbuf_split_str_without_term()
>   ref-filter: introduce align_atom_parser()
>   ref-filter: introduce remote_ref_atom_parser()
>   ref-filter: introduce contents_atom_parser()
>   ref-filter: introduce objectname_atom_parser()

It seems that this series had seen quite a good inputs, mostly from
Eric.  I finished reading it over and I didn't find anything more to
add.  The patches are mostly good and would be ready once these
points raised during the review are addressed, I think

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 02/16] refs: add methods for misc ref operations

[David Turner]

On Fri, 2015-12-11 at 15:39 -0800, Junio C Hamano wrote:
> David Turner  writes:
> 
> >  struct ref_be {
> > struct ref_be *next;
> > const char *name;
> > ref_transaction_commit_fn *transaction_commit;
> > +
> > +   pack_refs_fn *pack_refs;
> > +   peel_ref_fn *peel_ref;
> > +   create_symref_fn *create_symref;
> > +
> > +   resolve_ref_unsafe_fn *resolve_ref_unsafe;
> > +   verify_refname_available_fn *verify_refname_available;
> > +   resolve_gitlink_ref_fn *resolve_gitlink_ref;
> >  };
> 
> This may have been pointed out in the previous reviews by somebody
> else, but I think it is more customary to declare a struct member
> that is a pointer to a customization function without leading '*',
> i.e.
> 
>   typedef TYPE (*customize_fn)(ARGS);
> 
> struct vtable {
>   ...
>   cutomize_fn fn;
>   ...
>   };
> 
> in our codebase (cf. string_list::cmp, prio_queue::compare).

The previous review was here:
http://permalink.gmane.org/gmane.comp.version-control.git/279062

Michael wrote:
> Hmmm, I thought our convention was to define typedefs for functions
> themselves, not for the pointer-to-function; e.g.,
>
> typedef struct ref_transaction *ref_transaction_begin_fn(struct
> strbuf *err);
>
> (which would require `struct ref_be` to be changed to
>
> ref_transaction_begin_fn *transaction_begin;

And you agreed.  So I changed it.  Do you want me to change it back?  

--
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: Why does send-pack call pack-objects for all remote refs?

[Nasser Grainawi]

> On Dec 9, 2015, at 9:19 PM, Jeff King  wrote:
> 
> On Tue, Dec 08, 2015 at 05:34:43PM +, Daniel Koverman wrote:
> 
>> It is also good to know that 2000 remote refs is insane. The lower
>> hanging fruit here sounds like trimming that to a reasonable
>> number, so I'll try that approach first.
> 
> It's definitely a lot, but it's not unheard of. The git project has over
> 500 tags. That's not 2000, but you're within an order of magnitude.
> 
> I have seen repositories with 20,000+ tags. I consider that a bit more
> ridiculous, but it does work in practice.
> 

We have one at $DAY_JOB with 400,000+ refs. It presents some issues, but
Martin has raised those with the community and it works pretty well now.

Ref advertisement is still a pain...

> --
> 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

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, 
a Linux Foundation Collaborative Project

--
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

[L10N] Kickoff of translation for Git 2.7.0 round 1

[Jiang Xin]

Hi guys,

Git v2.7.0-rc0 has been released, and it's time to start new round of git l10n.
This time there are 66 updated messages need to be translated since last
update:

l10n: git.pot: v2.7.0 round 1 (66 new, 29 removed)

Generate po/git.pot from v2.7.0-rc0 for git v2.7.0 l10n round 1.

Signed-off-by: Jiang Xin 

You can get it from the usual place:

https://github.com/git-l10n/git-po/

As how to update your XX.po and help to translate Git, please see
"Updating a XX.po file" and other sections in “po/README" file.

--
Jiang Xin
--
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 02/16] refs: add methods for misc ref operations

[David Turner]

On Fri, 2015-12-11 at 16:23 -0800, Junio C Hamano wrote:
> David Turner  writes:
> 
> > The previous review was here:
> > http://permalink.gmane.org/gmane.comp.version-control.git/279062
> >
> > Michael wrote:
> >> Hmmm, I thought our convention was to define typedefs for functions
> >> themselves, not for the pointer-to-function; e.g.,
> >>
> >> typedef struct ref_transaction *ref_transaction_begin_fn(struct
> >> strbuf *err);
> >>
> >> (which would require `struct ref_be` to be changed to
> >>
> >> ref_transaction_begin_fn *transaction_begin;
> >
> > And you agreed.  So I changed it.  Do you want me to change it back?  
> 
> Sorry about that. I was agreeing to my mistaken understanding of
> what was pointed out X-<.
> 
> We do prefer fn(args) over (*fn)(args) and somehow I mixed that up
> with the declaration style.

So I should change it back?

--
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: git-p4 can skip changes when syncing large from multiple depot paths

[James Farwell]

Reproduction Steps:

1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. 
//depot/foo/ and //depot/bar/).
2. Add changes to the perforce repo in both depot paths. (e.g. 5 changes in 
each)
2. Do a "git p4 sync --changes_block_size n" where n is smaller than the number 
of changes applied to each depot path. (e.g. 2)


Expected Behavior:

All changes should sync and become commits in the git repo.


Actual Behavior:

All changes from the first depot path (if any) sync. After that only a small 
subset of changes from the remaining depot paths sync, causing some changes to 
be skipped entirely.


Best Guess:

I believe this was introduced in commit 
1051ef00636357061d72bcf673da86054fb14a12. The important code in question is the 
p4ChangesForPaths function, which contains a for loop that iterates over the 
depot paths, which then contains a while loop which iterates over the blocks. 
This change modified the inner while loop so that with every iteration it 
modifies changeStart, which causes the original value of changeStart to be 
lost. The first iteration of the for loop will correctly iterate across all the 
blocks until changeStart is within one block of the last change number, but 
then all subsequent iterations of the for loop will use that final changeStart 
value, causing any changes in those depot paths in earlier blocks to be skipped.

This can probably be easily remedied by using a temporary "start" variable for 
the block iteration, much like there is already a temporary "end" variable, and 
resetting it to the value of changeStart at the top of the for loop. (Note: 
this appears to be how the code prior to 
1051ef00636357061d72bcf673da86054fb14a12 functioned).


Thanks!
- James--
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 02/16] refs: add methods for misc ref operations

[Junio C Hamano]

David Turner  writes:

> The previous review was here:
> http://permalink.gmane.org/gmane.comp.version-control.git/279062
>
> Michael wrote:
>> Hmmm, I thought our convention was to define typedefs for functions
>> themselves, not for the pointer-to-function; e.g.,
>>
>> typedef struct ref_transaction *ref_transaction_begin_fn(struct
>> strbuf *err);
>>
>> (which would require `struct ref_be` to be changed to
>>
>> ref_transaction_begin_fn *transaction_begin;
>
> And you agreed.  So I changed it.  Do you want me to change it back?  

Sorry about that. I was agreeing to my mistaken understanding of
what was pointed out X-<.

We do prefer fn(args) over (*fn)(args) and somehow I mixed that up
with the declaration style.

--
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] completion: fix completing unstuck email alias arguments

[Jacob Keller]

On Fri, Dec 11, 2015 at 4:18 PM, SZEDER Gábor  wrote:
> Completing unstuck form of email aliases doesn't quite work:
>
>   $ git send-email --to 
>   alice   bob cecil
>   $ git send-email --to a
>   alice   bob cecil
>

Woops. Is it possible to add a test for this case? I honestly don't
know how the tests for the completion works.

> While listing email aliases works as expected, the second case should
> just complete to 'alice', but it keeps offering all email aliases
> instead.
>
> The cause for this behavior is that in this case we mistakenly tell
> __gitcomp() explicitly that the current word to be completed is empty,
> while in reality it is not.  As a result __gitcomp() doesn't filter
> out non-matching aliases, so all aliases end up being offered over and
> over again.
>
> Fix this by not passing the current word to be completed to
> __gitcomp() and letting it go the default route and grab it from the
> '$cur' variable.  Don't pass empty prefix either, because it's assumed
> to be empty when unspecified, so it's not necessary.
>
> Signed-off-by: SZEDER Gábor 
> ---
>
> On top of the recently merged dfbe5eeb32 (completion: add support for
> completing email aliases, 2015-11-19).
>
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 111b05302b..d9b995799c 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1716,7 +1716,7 @@ _git_send_email ()
> --to|--cc|--bcc|--from)
> __gitcomp "
> $(git --git-dir="$(__gitdir)" send-email --dump-aliases 
> 2>/dev/null)
> -   " "" ""
> +   "

Yes this looks reasonable to me.

> return
> ;;
> esac
> --
> 2.7.0.rc0.37.g77d69b9
>
--
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 2/8] update-index: use enum for untracked cache options

[Christian Couder]

On Thu, Dec 10, 2015 at 7:46 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
 +/* Untracked cache mode */
 +enum uc_mode {
 + UNDEF_UC = -1,
 + NO_UC = 0,
 + UC,
 + FORCE_UC
 +};
 +
>>>
>>> With these, the code is much easier to read than with the mystery
>>> constants, but did you consider making UC_ a common prefix for
>>> further ease-of-reading?  E.g.
>>>
>>> UC_UNSPECIFIED
>>> UC_DONTUSE
>>> UC_USE
>>> UC_FORCE
>>>
>>> or something?
>>
>> Ok, I will use what you suggest.
>
> As you know I am bad at bikeshedding; the only suggestion in the
> above is to have common UC_ prefix ;-)  Don't take what follow UC_
> as my suggestion.

I am bad at finding names too, so I think what you wrote is pretty good.

I thought about the following:

 UC_UNDEF
 UC_DISABLE
 UC_ENABLE
 UC_TEST
 UC_FORCE

but I am not sure it is much better.
--
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 jk/prune-mtime] prune: close directory earlier during loose-object directory traversal

[Junio C Hamano]

Jeff King  writes:

> On Wed, Aug 12, 2015 at 07:43:01PM +0200, Johannes Sixt wrote:
>
>> 27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16)
>> introduced a new function for_each_loose_file_in_objdir() with a helper
>> for_each_file_in_obj_subdir(). The latter calls callbacks for each file
>> found during a directory traversal and finally also a callback for the
>> directory itself.
>> 
>> git-prune uses the function to clean up the object directory. In
>> particular, in the directory callback it calls rmdir(). On Windows XP,
>> this rmdir call fails, because the directory is still open while the
>> callback is called. Close the directory before calling the callback.
>
> Makes sense, and the patch looks good to me. Sorry for breaking things
> on Windows.
>
> Acked-by: Jeff King 

Sorry for reviving this old thread, but I noticed that we do not
have this patch in our tree yet.  I'll queue to 'pu' for now lest I
forget.  If I missed a good argument or concensus against the change
please let me know, otherwise I'll fast track the change to 2.7 final
--
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 jk/prune-mtime] prune: close directory earlier during loose-object directory traversal

[Jeff King]

On Fri, Dec 11, 2015 at 11:37:54AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Wed, Aug 12, 2015 at 07:43:01PM +0200, Johannes Sixt wrote:
> >
> >> 27e1e22d (prune: factor out loose-object directory traversal, 2014-10-16)
> >> introduced a new function for_each_loose_file_in_objdir() with a helper
> >> for_each_file_in_obj_subdir(). The latter calls callbacks for each file
> >> found during a directory traversal and finally also a callback for the
> >> directory itself.
> >> 
> >> git-prune uses the function to clean up the object directory. In
> >> particular, in the directory callback it calls rmdir(). On Windows XP,
> >> this rmdir call fails, because the directory is still open while the
> >> callback is called. Close the directory before calling the callback.
> >
> > Makes sense, and the patch looks good to me. Sorry for breaking things
> > on Windows.
> >
> > Acked-by: Jeff King 
> 
> Sorry for reviving this old thread, but I noticed that we do not
> have this patch in our tree yet.  I'll queue to 'pu' for now lest I
> forget.  If I missed a good argument or concensus against the change
> please let me know, otherwise I'll fast track the change to 2.7 final

Ah, thanks for doing that. I noticed it when picking through "git branch
--no-merged pu" of your workspace a few weeks ago, but forgot to follow
up. I certainly have no objections.

-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] notes: allow merging from arbitrary references

[Junio C Hamano]

Jeff King  writes:

> On Tue, Nov 24, 2015 at 05:47:09PM -0500, Jeff King wrote:
>
>> On Fri, Nov 13, 2015 at 08:34:22AM -0800, Jacob Keller wrote:
>> 
>> > ---
>> > I do not remember what version this was since it has been an age ago
>> > that I sent the previous code. This is mostly just a rebase onto current
>> > next. I believe I have covered everything previous reviewers noted.
>> 
>> Please keep topics branched from master where possible. And if not
>> possible, please indicate which topic in 'next' is required to build on.
>> 
>> We never merge 'next' itself, only individual topics from it. So I can't
>> just apply your patch on top of 'next'.
>> 
>> I did get it to apply on the current master with "am -3", but some tests
>> in t3310 seem to fail. Can you take a look?
>
> I just noticed v2, which I missed earlier. But the same complaints
> apply. :)

I tried to queue
<1447432462-21192-1-git-send-email-jacob.e.kel...@intel.com> from
Nov 13 directly on top of 'master', and t3310 does seem to fail.

I'll discard the topic for now, expecting the resurrection sometime
later.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-gui: Remove ancient Cygwin compatibility code

[Junio C Hamano]

Adam Dinwoodie  writes:

> Remove special path handling for Cygwin in the git-gui Makefile.  This
> used to be necessary, but has been being patched out of the official
> Cygwin distribution builds since Git v1.7.9, and should really be
> patched out of the upstream code rather than being patched every time in
> the Cygwin build process.
>
> Signed-off-by: Adam Dinwoodie 
> ---
> I'm the current Cygwin maintainer for Git; this code has been patched
> out of the Cygwin Git builds since v1.7.9,* well before I took over.
> It's clearly stable and causing no problems, so having it in the
> upstream code rather than patching every time seems The Right Thing To
> Do.
>
> (* The actual patch used in the Cygwin builds just replaces `ifeq
> ($(uname_O,Cygwin))` with `ifeq ($(uname_O,noThanks))`, but that's
> clearly the appropriate solution for a quick manual patch, not for the
> actual upstream code.)
>
> Sending with apologies to Shawn Pearce for the noise; I'd misread the
> SubmittingPatches doc and sent to him alone first.
>
> I've based this patch off the git-gui tree rather than the main Git tree
> per the SubmittingPatches doc.  Pipe it through `sed
> 's!Makekfile!git-gui/\0!g'` for a version that applies cleanly to the
> main Git source tree.

Pat, this looks like a good thing to have.

More generally, should I expect a pull request from you sometime in
a week or two for updates in the git-gui part?

Thanks.

>  Makefile | 21 +++--
>  1 file changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4f00bdd..e369046 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -136,25 +136,10 @@
>  GITGUI_RELATIVE :=
>  GITGUI_MACOSXAPP :=
>  
> -ifeq ($(uname_O),Cygwin)
> - GITGUI_SCRIPT := `cygpath --windows --absolute "$(GITGUI_SCRIPT)"`
> -
> - # Is this a Cygwin Tcl/Tk binary?  If so it knows how to do
> - # POSIX path translation just like cygpath does and we must
> - # keep libdir in POSIX format so Cygwin packages of git-gui
> - # work no matter where the user installs them.
> - #
> - ifeq ($(shell echo 'puts [file normalize /]' | 
> '$(TCL_PATH_SQ)'),$(shell cygpath --mixed --absolute /))
> - gg_libdir_sed_in := $(gg_libdir)
> - else
> - gg_libdir_sed_in := $(shell cygpath --windows --absolute 
> "$(gg_libdir)")
> - endif
> -else
> - ifeq ($(exedir),$(gg_libdir))
> - GITGUI_RELATIVE := 1
> - endif
> - gg_libdir_sed_in := $(gg_libdir)
> -endif
> +ifeq ($(exedir),$(gg_libdir))
> + GITGUI_RELATIVE := 1
> +endif
> +gg_libdir_sed_in := $(gg_libdir)
>  ifeq ($(uname_S),Darwin)
>   ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)
>   GITGUI_MACOSXAPP := YesPlease
--
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] rebase -i: remember merge options beyond continue actions

[Ralf Thielow]

From: Fabian Ruch 

If the user specifies a merge strategy or strategy options in
"rebase --interactive", also use these in subsequent calls of
"rebase --continue".

In the implementation, the "do_merge" guard to check for a given
merge strategy or strategy options is implied by passing these
options, but not stored.  This prevents subsequent calls of
"rebase --continue" to use this setting again later.  Remove this
"do_merge" guard to allow a later usage.

Reported-by: Diogo de Campos 
Signed-off-by: Fabian Ruch 
Helped-by: Michael Haggerty 
Signed-off-by: Ralf Thielow 
---
I've been applying the original patch for a long time to my tree,
as it helps me to resolve conflicts e.g. when rebasing .po files.
I also think it's a reasonable change for git-rebase.

I've rebased it agains the current master and rewrote the
commit message to try to make this change technically a bit more
easy to understand.

Original patch submit:
http://thread.gmane.org/gmane.comp.version-control.git/251147/

 git-rebase--interactive.sh| 18 +++---
 t/t3404-rebase-interactive.sh | 16 
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b938a6d..c0cfe88 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -81,17 +81,13 @@ rewritten_pending="$state_dir"/rewritten-pending
 # and leaves CR at the end instead.
 cr=$(printf "\015")
 
-strategy_args=
-if test -n "$do_merge"
-then
-   strategy_args=${strategy:+--strategy=$strategy}
-   eval '
-   for strategy_opt in '"$strategy_opts"'
-   do
-   strategy_args="$strategy_args -X$(git rev-parse 
--sq-quote "${strategy_opt#--}")"
-   done
-   '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+   for strategy_opt in '"$strategy_opts"'
+   do
+   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
"${strategy_opt#--}")"
+   done
+'
 
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 98eb49a..9a2461c 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1006,6 +1006,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
 '
 
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+   git checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&
+   git reset --hard HEAD^ &&
+   >breakpoint &&
+   git add breakpoint &&
+   git commit -m "breakpoint for interactive mode" &&
+   echo five >conflict &&
+   echo Z >file1 &&
+   git commit -a -m "one file conflict" &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 2" git rebase -i --strategy=recursive -Xours 
conflict-branch &&
+   git rebase --continue &&
+   test $(git show conflict-branch:conflict) = $(cat conflict) &&
+   test $(cat file1) = Z
+'
+
 test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD) &&
test_when_finished "git rebase --abort; git reset --hard $current_head; 
rm -f error" &&
-- 
2.7.0.rc0.174.g1b62464

--
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] notes: allow merging from arbitrary references

[Jacob Keller]

On Fri, Dec 11, 2015 at 11:47 AM, Junio C Hamano  wrote:
> I'll discard the topic for now, expecting the resurrection sometime
> later.
>

Hi,

Yes I fully intend to work this against master once I have some time
again. It may be a few weeks as I am extra busy for the holidays, and
the next posting will be fresh on the tip of the future-master :)

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] rebase -i: remember merge options beyond continue actions

[Junio C Hamano]

On Fri, Dec 11, 2015 at 11:54 AM, Ralf Thielow  wrote:
> From: Fabian Ruch 
>
> If the user specifies a merge strategy or strategy options in
> "rebase --interactive", also use these in subsequent calls of
> "rebase --continue".
>
> In the implementation, the "do_merge" guard to check for a given
> merge strategy or strategy options is implied by passing these
> options, but not stored.  This prevents subsequent calls of
> "rebase --continue" to use this setting again later.  Remove this
> "do_merge" guard to allow a later usage.
>
> Reported-by: Diogo de Campos 
> Signed-off-by: Fabian Ruch 
> Helped-by: Michael Haggerty 
> Signed-off-by: Ralf Thielow 
> ---
> I've been applying the original patch for a long time to my tree,
> as it helps me to resolve conflicts e.g. when rebasing .po files.
> I also think it's a reasonable change for git-rebase.
>
> I've rebased it agains the current master and rewrote the
> commit message to try to make this change technically a bit more
> easy to understand.

Thanks. I wonder if Michael's rephrasing in $gmane/251386 still applies, which
I found by far the most readable.

http://thread.gmane.org/gmane.comp.version-control.git/251147/focus=251386



>
> Original patch submit:
> http://thread.gmane.org/gmane.comp.version-control.git/251147/
>
>  git-rebase--interactive.sh| 18 +++---
>  t/t3404-rebase-interactive.sh | 16 
>  2 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index b938a6d..c0cfe88 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -81,17 +81,13 @@ rewritten_pending="$state_dir"/rewritten-pending
>  # and leaves CR at the end instead.
>  cr=$(printf "\015")
>
> -strategy_args=
> -if test -n "$do_merge"
> -then
> -   strategy_args=${strategy:+--strategy=$strategy}
> -   eval '
> -   for strategy_opt in '"$strategy_opts"'
> -   do
> -   strategy_args="$strategy_args -X$(git rev-parse 
> --sq-quote "${strategy_opt#--}")"
> -   done
> -   '
> -fi
> +strategy_args=${strategy:+--strategy=$strategy}
> +eval '
> +   for strategy_opt in '"$strategy_opts"'
> +   do
> +   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
> "${strategy_opt#--}")"
> +   done
> +'
>
>  GIT_CHERRY_PICK_HELP="$resolvemsg"
>  export GIT_CHERRY_PICK_HELP
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 98eb49a..9a2461c 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1006,6 +1006,22 @@ test_expect_success 'rebase -i with --strategy and -X' 
> '
> test $(cat file1) = Z
>  '
>
> +test_expect_success 'interrupted rebase -i with --strategy and -X' '
> +   git checkout -b conflict-merge-use-theirs-interrupted conflict-branch 
> &&
> +   git reset --hard HEAD^ &&
> +   >breakpoint &&
> +   git add breakpoint &&
> +   git commit -m "breakpoint for interactive mode" &&
> +   echo five >conflict &&
> +   echo Z >file1 &&
> +   git commit -a -m "one file conflict" &&
> +   set_fake_editor &&
> +   FAKE_LINES="edit 1 2" git rebase -i --strategy=recursive -Xours 
> conflict-branch &&
> +   git rebase --continue &&
> +   test $(git show conflict-branch:conflict) = $(cat conflict) &&
> +   test $(cat file1) = Z
> +'
> +
>  test_expect_success 'rebase -i error on commits with \ in message' '
> current_head=$(git rev-parse HEAD) &&
> test_when_finished "git rebase --abort; git reset --hard 
> $current_head; rm -f error" &&
> --
> 2.7.0.rc0.174.g1b62464
>
> --
> 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: What's cooking in git.git (Dec 2015, #03; Thu, 10)

[Junio C Hamano]

Stefan Beller  writes:

> On Thu, Dec 10, 2015 at 3:51 PM, Junio C Hamano  wrote:
>> Stefan Beller  writes:
>>
 * sb/submodule-parallel-fetch (2015-11-24) 17 commits
 ...
>>>
>>> I assume you plan on merging this after 2.7 settled and then we can
>>> also get the above sb/submodule-parallel-update going again.
>>
>> Yeah, thanks for reminding me.  I think that would be a good plan
>> that gives us an opportunity to clean up this topic, some parts of
>> which are have "an early patch that was too hastily merged to 'next'
>> had to be tweaked by an 'oops' follow-up patch in the topic"
>> pattern, e.g. "make waitpid the secondary and closed pipe the
>> primary way to monitor children".
>>
>> Thanks.
>
> This makes it sound as if you would drop it from next once 2.7 is out,
> expecting a complete reroll, which does the right thing from the beginning?

Yes, what is still in 'next' when a new release is made has the
chance to (re)do the right thing from the beginning, and it also can
lose merges from other topics in the middle of the topic if they
have graduated to 'master'.

A topic that did the right thing from this cycle already, but needs
to stay in 'next' only because the area it touches is so important
and deserves more real world testing by those who run 'next', may
not have to reroll.

I think two sb/submodule-parallel-* topics fall into the former
category, i.e. ones that can take advantage of the chance to escape
the rigidity of 'next' at the release cycle boundary, and I think
we should grab that opportunity to clean these series up.

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: What's cooking in git.git (Dec 2015, #03; Thu, 10)

[Junio C Hamano]

John Keeping  writes:

> On Thu, Dec 10, 2015 at 02:46:40PM -0800, Junio C Hamano wrote:
>> * jk/send-email-ssl-errors (2015-11-24) 1 commit
>>  - send-email: enable SSL level 1 debug output
>> 
>>  Improve error reporting when SMTP TLS fails.
>> 
>>  Waiting for a reroll.
>>  ($gmane/281693)
>
> It looks like this got lost in the noise:
>
> http://article.gmane.org/gmane.comp.version-control.git/281975

Indeed it was; thanks for a pointer.
--
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 2/8] update-index: use enum for untracked cache options

[Junio C Hamano]

Christian Couder  writes:

>> As you know I am bad at bikeshedding; the only suggestion in the
>> above is to have common UC_ prefix ;-)  Don't take what follow UC_
>> as my suggestion.
>
> I am bad at finding names too, so I think what you wrote is pretty good.
>
> I thought about the following:
>
>  UC_UNDEF
>  UC_DISABLE
>  UC_ENABLE
>  UC_TEST
>  UC_FORCE
>
> but I am not sure it is much better.

I hated the DONTUSE/USE I listed, and I think DISABLE/ENABLE is much
much better.  I do prefer unspecified over undef, though.

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] completion: remove 'git column' from porcelain commands

[Junio C Hamano]

SZEDER Gábor  writes:

> 'git column' is an internal helper, so it should not be offered on
> 'git ' along with porcelain commands.
>
> Signed-off-by: SZEDER Gábor 
> ---

I think this makes sense.
Objections?

>  contrib/completion/git-completion.bash | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 111b05302b..5dec8778f7 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -664,6 +664,7 @@ __git_list_porcelain_commands ()
>   check-mailmap): plumbing;;
>   check-ref-format) : plumbing;;
>   checkout-index)   : plumbing;;
> + column)   : internal helper;;
>   commit-tree)  : plumbing;;
>   count-objects): infrequent;;
>   credential)   : credentials;;
--
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] Case insensitive URL rewrite

[Jeff King]

On Fri, Dec 11, 2015 at 10:26:41AM -0800, Junio C Hamano wrote:

> Lars Schneider  writes:
> 
> > What do you think about a flag that makes these rewrites case insensitive? 
> > E.g. with the following config flag:
> >
> > [url ""]
> > insteadOf = 
> > ignorecase = true
> 
> I am with Daniel on this.
> 
> It is perfectly fine to consider these two equivalent.
> 
> git clone https://github.com/git/git (canonical one)
> git clone https://GitHub.com/git/git (host part in funny case)
> 
> In fact, I think we should do this without any additional
> configuration variable.
> 
> On the other hand, these two MUST be treated as different by
> default:
> 
> git clone https://github.com/GIT/GIT (differences outside host part)
> git clone g...@github.com:GIT/GIT (differences outside host part)

I haven't looked, but the code in urlmatch.c probably gets all of this
right already. It was written much later than the insteadOf code, as it
was part of the http.$url.* matching, and it may be reasonable to simply
teach the insteadOf code to use it. OTOH, it may need tweaked because
I'm not sure how it would handle non-URLs like the "host:path" ssh form.

-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

[PATCH v2] rebase -i: remember merge options beyond continue actions

[Ralf Thielow]

From: Fabian Ruch 

If the user explicitly specified a merge strategy or strategy
options, continue to use that strategy/option after
"rebase --continue".  Add a test of the corrected behavior.

If --merge is specified or implied by -s or -X, then "strategy and
"strategy_opts" are set to values from which "strategy_args" can be
derived; otherwise they are set to empty strings.  Either way,
their values are propagated from one step of an interactive rebase
to the next via state files.

"do_merge", on the other hand, is *not* propagated to later steps of
an interactive rebase.  Therefore, making the initialization of
"strategy_args" conditional on "do_merge" being set prevents later
steps of an interactive rebase from setting it correctly.

Luckily, we don't need the "do_merge" guard at all.  If the rebase
was started without --merge, then "strategy" and "strategy_opts"
are both the empty string, which results in "strategy_args" also
being set to the empty string, which is just what we want in that
situation.  So remove the "do_merge" guard and derive
"strategy_args" from "strategy" and "strategy_opts" every time.

Reported-by: Diogo de Campos 
Signed-off-by: Fabian Ruch 
Helped-by: Michael Haggerty 
Signed-off-by: Ralf Thielow 
---
2015-12-11 21:07 GMT+01:00 Junio C Hamano :
> On Fri, Dec 11, 2015 at 11:54 AM, Ralf Thielow  wrote:

>
> Thanks. I wonder if Michael's rephrasing in $gmane/251386 still applies, which
> I found by far the most readable.
>
> http://thread.gmane.org/gmane.comp.version-control.git/251147/focus=251386
>

Sure.

 git-rebase--interactive.sh| 18 +++---
 t/t3404-rebase-interactive.sh | 16 
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b938a6d..c0cfe88 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -81,17 +81,13 @@ rewritten_pending="$state_dir"/rewritten-pending
 # and leaves CR at the end instead.
 cr=$(printf "\015")
 
-strategy_args=
-if test -n "$do_merge"
-then
-   strategy_args=${strategy:+--strategy=$strategy}
-   eval '
-   for strategy_opt in '"$strategy_opts"'
-   do
-   strategy_args="$strategy_args -X$(git rev-parse 
--sq-quote "${strategy_opt#--}")"
-   done
-   '
-fi
+strategy_args=${strategy:+--strategy=$strategy}
+eval '
+   for strategy_opt in '"$strategy_opts"'
+   do
+   strategy_args="$strategy_args -X$(git rev-parse --sq-quote 
"${strategy_opt#--}")"
+   done
+'
 
 GIT_CHERRY_PICK_HELP="$resolvemsg"
 export GIT_CHERRY_PICK_HELP
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 98eb49a..9a2461c 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1006,6 +1006,22 @@ test_expect_success 'rebase -i with --strategy and -X' '
test $(cat file1) = Z
 '
 
+test_expect_success 'interrupted rebase -i with --strategy and -X' '
+   git checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&
+   git reset --hard HEAD^ &&
+   >breakpoint &&
+   git add breakpoint &&
+   git commit -m "breakpoint for interactive mode" &&
+   echo five >conflict &&
+   echo Z >file1 &&
+   git commit -a -m "one file conflict" &&
+   set_fake_editor &&
+   FAKE_LINES="edit 1 2" git rebase -i --strategy=recursive -Xours 
conflict-branch &&
+   git rebase --continue &&
+   test $(git show conflict-branch:conflict) = $(cat conflict) &&
+   test $(cat file1) = Z
+'
+
 test_expect_success 'rebase -i error on commits with \ in message' '
current_head=$(git rev-parse HEAD) &&
test_when_finished "git rebase --abort; git reset --hard $current_head; 
rm -f error" &&
-- 
2.7.0.rc0.174.g1b62464

--
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] Case insensitive URL rewrite

[Junio C Hamano]

Stefan Beller  writes:

> On Fri, Dec 11, 2015 at 10:26 AM, Junio C Hamano  wrote:
>> On the other hand, these two MUST be treated as different by
>> default:
>>
>> git clone https://github.com/GIT/GIT (differences outside host part)
>> git clone g...@github.com:GIT/GIT (differences outside host part)
>
> This is one of the more obvious examples I would think as the protocol is
> different.

Oh, I should have been more careful; I did not think Lars meant
g...@github.com should somehow match https://github.com but a more
obvious "g...@github.com:GIT/GIT and g...@github.com:git/git behave
the same", and I thought everybody who read it understood the
example as such.

> What about:
>
>  git clone g...@example.com:git/git (differences outside host part)
>  git clone g...@example.com:GIT/GIT (differences outside host part)
>
> If the host has a filesystem which respects capitalization, these may be
> two different repositories.

That is what I meant.  Thanks for clarification.
--
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] completion: remove 'git column' from porcelain commands

[Duy Nguyen]

On Fri, Dec 11, 2015 at 7:16 PM, Junio C Hamano  wrote:
> SZEDER Gábor  writes:
>
>> 'git column' is an internal helper, so it should not be offered on
>> 'git ' along with porcelain commands.
>>
>> Signed-off-by: SZEDER Gábor 
>> ---
>
> I think this makes sense.
> Objections?

Not from me.
-- 
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: [RFC] Case insensitive URL rewrite

[Junio C Hamano]

Lars Schneider  writes:

> What do you think about a flag that makes these rewrites case insensitive? 
> E.g. with the following config flag:
>
> [url ""]
>   insteadOf = 
>   ignorecase = true

I am with Daniel on this.

It is perfectly fine to consider these two equivalent.

git clone https://github.com/git/git (canonical one)
git clone https://GitHub.com/git/git (host part in funny case)

In fact, I think we should do this without any additional
configuration variable.

On the other hand, these two MUST be treated as different by
default:

git clone https://github.com/GIT/GIT (differences outside host part)
git clone g...@github.com:GIT/GIT (differences outside host part)

Although I am OK if this is hidden behind a configuration variable,
I am not sure how much value you would be giving to the users.

In any case, I expect that additional change (on top of what is
required to make hostname part treated case insensitively) would not
be too bad, if we were going to fix the hostname part.
--
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] Case insensitive URL rewrite

[Stefan Beller]

On Fri, Dec 11, 2015 at 10:26 AM, Junio C Hamano  wrote:
> On the other hand, these two MUST be treated as different by
> default:
>
> git clone https://github.com/GIT/GIT (differences outside host part)
> git clone g...@github.com:GIT/GIT (differences outside host part)

This is one of the more obvious examples I would think as the protocol is
different. What about:

 git clone g...@example.com:git/git (differences outside host part)
 git clone g...@example.com:GIT/GIT (differences outside host part)

If the host has a filesystem which respects capitalization, these may be
two different repositories.
--
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/8] update-index: add untracked cache notifications

[Christian Couder]

On Tue, Dec 8, 2015 at 8:03 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> Doing:
>>
>>   cd /tmp
>>   git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>>
>> doesn't work how one would expect. It hardcodes "/tmp" as the directory
>> that "works" into the index, so if you use the working tree, you'll
>> never use the untracked cache.
>
> I think your "expectation" needs to be more explicitly spelled out.
>
> "git -C /tmp --git-dir=/git/somewhere/else/.git" is a valid way to
> use that repository you have in somewhere else to track things under
> /tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the
> cwd, i.e. /tmp, is the root level of the working tree), and for such
> a usage, the above command works as expected.  Perhaps
>
> Attempting to flip the untracked-cache feature on for a random index
> file with
>
>cd /random/unrelated/place
>git --git-dir=/somewhere/else/.git update-index --untracked-cache
>
> would not work as you might expect.  Because flipping the
> feature on in the index also records the location of the
> corresponding working tree (/random/unrelated/place in the above
> example), when the index is subsequently used to keep track of
> files in the working tree in /somewhere/else, the feature is
> disabled.
>
> may be an improvement.

Yeah, I agree that it is better. I have included your explanations in
the next version I will send. Thanks.

> The index already implicitly records where the working tree was and
> that is not limited to untracked-cache option.  For example, if you
> have your repository and its working tree in /git/somewhere/else,
> which does not have a path X, then doing:
>
> cd /tmp && >tmp/X
> git --git-dir=/git/somewhere/else/.git update-index --add X
>
> would store X taken from /tmp in the index, so subsequent use of the
> index "knows" about X that was taken outside /git/somewhere/else/
> after the above command finishes and the subsequent use is made
> without the --git-dir parameter, e.g.
>
> cd /git/somewhere/else/ && git diff-index --cached HEAD'
>
> would say that you added X, even though /git/somewhere/else/ may not
> have that X at all.  And this is not limited to update-index,
> either.  You can temporarily use --git-dir with "git add X" and the
> result would persist the same way in the index.
>
> I think the moral of the story is that you are not expected to
> randomly use git-dir and git-work-tree to point at different places
> without knowing what you are doing, and we may need a way to help
> people understand what is going on when it is done by a mistake.

Yeah, I agree, and I think displaying more information might be a good way.

> The patch to show result from xgetcwd() and get_git_work_tree() may
> be a step in the right direction, but if the user is not doing
> anything fancy, "Testing mtime in /long/path/to/the/directory" would
> be irritatingly verbose.

Yeah, but after this series only "--test-untracked-cache" does any
testing, and the 10 seconds time it takes are probably more irritating
than its output.

> I wonder if it is easy to tell when the user is doing such an
> unnatural thing.  Off the top of my head, when the working tree is
> anywhere other than one level above $GIT_DIR, the user is doing
> something unusual; I do not know if there are cases where the user
> is doing something unnatural if $GIT_WORK_TREE is one level above
> $GIT_DIR, though.

Yeah, it could only print a warning in case something unusual is done.
I am not sure it is worth it though.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] dir: add add_untracked_cache()

[Christian Couder]

On Wed, Dec 9, 2015 at 8:37 AM, Torsten Bögershausen  wrote:
> On 08.12.15 18:15, Christian Couder wrote:
>> This new function will be used in a later patch.
> May be
> Factor out code into add_untracked_cache(), which will be used in the next 
> commit.

Thanks for the suggestion. I think I will put something like this:

Factor out code into add_untracked_cache(), which will be used
in a later commit.
--
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] gitk: sv.po: Update Swedish translation (311t)

[Peter Krefting]

(Attached gzipped to avoid character encoding issues)

0001-gitk-sv.po-Update-Swedish-translation-311t.patch.gz
Description: application/gzip