Re: Bug? git worktree fails with master on bare repo

2016-10-09 Thread Dennis Kaarsemaker
On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
> Hey all,
> I'm working on some server-side software to do a merge. By using git
> worktree it's possible to check out a given branch for a bare repo and
> merge another branch into it. It's very fast, even with large
> repositories.
> 
> The only exception seems to be merging to master. When I do git
> worktree add /tmp/path/to/worktree master I get an error:
> 
> [fatal: 'master' is already checked out at '/path/to/bare/repo']
> 
> But this is clearly not true, git worktree list gives:
> 
> [/path/to/bare/repo (bare)]
> 
> ...and of course, there is no work tree at that path, just the bare
> repo files you'd expect.

The worktree code treats the base repo as a worktree, even if it's
bare. For the purpose of being able to do a checkout of the main branch
of a bare repo, this patch should do:

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335..b618d6b 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without 
"add"' '
)
 '
 
+test_expect_success '"add" default branch of a bare repo' '
+   (
+   git clone --bare . bare2 &&
+   cd bare2 &&
+   git worktree add ../there3 master
+   )
+'
+
 test_expect_success 'checkout with grafts' '
test_when_finished rm .git/info/grafts &&
test_commit abc &&
diff --git a/worktree.c b/worktree.c
index 5acfe4c..35e95b7 100644
--- a/worktree.c
+++ b/worktree.c
@@ -345,6 +345,8 @@ const struct worktree *find_shared_symref(const char 
*symref,
 
for (i = 0; worktrees[i]; i++) {
struct worktree *wt = worktrees[i];
+   if(wt->is_bare)
+   continue;
 
if (wt->is_detached && !strcmp(symref, "HEAD")) {
if (is_worktree_being_rebased(wt, target)) {


But I'm wondering why the worktree code does this. A bare repo isn't a
worktree and I think it shouldn't treat it as one. A patch that rips
out this feature and updates the tests to match would look like this:


diff --git a/builtin/worktree.c b/builtin/worktree.c
index 5c4854d..3600530 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -382,15 +382,11 @@ static int add(int ac, const char **av, const char 
*prefix)
 static void show_worktree_porcelain(struct worktree *wt)
 {
    printf("worktree %s\n", wt->path);
-   if (wt->is_bare)
-   printf("bare\n");
-   else {
-   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
-   if (wt->is_detached)
-   printf("detached\n");
-   else
-   printf("branch %s\n", wt->head_ref);
-   }
+   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
+   if (wt->is_detached)
+   printf("detached\n");
+   else
+   printf("branch %s\n", wt->head_ref);
    printf("\n");
 }
 
@@ -401,16 +397,12 @@ static void show_worktree(struct worktree *wt, int 
path_maxlen, int abbrev_len)
    int path_adj = cur_path_len - utf8_strwidth(wt->path);
 
    strbuf_addf(&sb, "%-*s ", 1 + path_maxlen + path_adj, wt->path);
-   if (wt->is_bare)
-   strbuf_addstr(&sb, "(bare)");
-   else {
-   strbuf_addf(&sb, "%-*s ", abbrev_len,
-   find_unique_abbrev(wt->head_sha1, 
DEFAULT_ABBREV));
-   if (!wt->is_detached)
-   strbuf_addf(&sb, "[%s]", 
shorten_unambiguous_ref(wt->head_ref, 0));
-   else
-   strbuf_addstr(&sb, "(detached HEAD)");
-   }
+   strbuf_addf(&sb, "%-*s ", abbrev_len,
+   find_unique_abbrev(wt->head_sha1, DEFAULT_ABBREV));
+   if (!wt->is_detached)
+   strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(wt->head_ref, 
0));
+   else
+   strbuf_addstr(&sb, "(detached HEAD)");
    printf("%s\n", sb.buf);
 
    strbuf_release(&sb);
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 4bcc335..b618d6b 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without 
"add"' '
    )
 '
 
+test_expect_success '"add" default branch of a bare repo' '
+   (
+   git clone --bare . bare2 &&
+   cd bare2 &&
+   git worktree add ../there3 master
+   )
+'
+
 test_expect_success 'checkout with grafts' '
    test_when_finished rm .git/info/grafts &&
    test_commit abc &&
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
index 1b1b65a..842e9d9 100755
--- a/t/t2027-worktree-list.sh
+++ b/t/t2027-worktree-list.sh
@@ -62,9 +62,8 @@ test_expect_success 'bare repo setup' '
 
 test_expect_success '"list" all worktrees from bare main' '
    test_when_finished "rm -rf there && git -C bare1 worktree prune" &&
-  

Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

2016-10-09 Thread Johannes Schindelin
Hi Hannes,

On Thu, 6 Oct 2016, Johannes Sixt wrote:

> [PATCH] sequencer: strip CR from the end of exec insns
> 
> It is not unheard of that editors on Windows write CRLF even if the file
> originally had only LF. This is particularly awkward for exec lines of a
> rebase -i todo sheet. Take for example the insn "exec echo": The shell
> script parser (either of the sequencer or of the shell that is invoked,
> I do not know) splits at the LF and leaves the CR attached to "echo",
> which leads to the unknown command "echo\r".
> 
> Work it around by stripping CR before the command specified with exec is
> passed to the shell.
> 
> This happens to fix t9903.14 and .15 in my environment: the insn sheet
> constructed here contains CRLF thanks to MSYS1's bash cleverness.

Good point. I decided to do it at a different level, though:
parse_insn_line() should already receive the line without trailing
end-of-line markers (this was already the case for LF-only todo scripts).

I reused your commit message and touched it up a bit, hope you don't mind!

Ciao,
Dscho


[PATCH v1 0/2] convert: stream and early out

2016-10-09 Thread tboegi
From: Torsten Bögershausen 

An optimization when autocrlf is used and the binary/text detection is run.
Or git ls-files --eol is run to analyze the content of files or blobs.

Torsten Bögershausen (2):
  read-cache: factor out get_sha1_from_index() helper
  convert.c: stream and early out

 cache.h  |   3 +
 convert.c| 195 +++
 read-cache.c |  29 +
 3 files changed, 151 insertions(+), 76 deletions(-)

-- 
2.10.0



[PATCH v1 2/2] convert.c: stream and early out

2016-10-09 Thread tboegi
From: Torsten Bögershausen 

When statistics are done for the autocrlf handling, the search in
the content can be stopped, if e.g
- a search for binary is done, and a NUL character is found
- a search for CRLF is done, and the first CRLF is found.

Similar when statistics for binary vs non-binary are gathered:
Whenever a lone CR or NUL is found, the search can be aborted.

When checking out files in "auto" mode, any file that has a "lone CR"
or a CRLF will not be converted, so the search can be aborted early.

Add the new bit, CONVERT_STAT_BITS_ANY_CR,
which is set for either lone CR or CRLF.

Many binary files have a NUL very early (within the first few bytes,
latest within the first 1..2K).
It is often not necessary to load the whole content of a file or blob
into memory.

Use a streaming handling for blobs and files in the worktree.
---
 convert.c | 195 +-
 1 file changed, 130 insertions(+), 65 deletions(-)

diff --git a/convert.c b/convert.c
index 077f5e6..6a625e5 100644
--- a/convert.c
+++ b/convert.c
@@ -3,6 +3,7 @@
 #include "run-command.h"
 #include "quote.h"
 #include "sigchain.h"
+#include "streaming.h"
 
 /*
  * convert.c - convert a file when checking it out and checking it in.
@@ -13,10 +14,10 @@
  * translation when the "text" attribute or "auto_crlf" option is set.
  */
 
-/* Stat bits: When BIN is set, the txt bits are unset */
 #define CONVERT_STAT_BITS_TXT_LF0x1
 #define CONVERT_STAT_BITS_TXT_CRLF  0x2
 #define CONVERT_STAT_BITS_BIN   0x4
+#define CONVERT_STAT_BITS_ANY_CR0x8
 
 enum crlf_action {
CRLF_UNDEFINED,
@@ -31,30 +32,36 @@ enum crlf_action {
 
 struct text_stat {
/* NUL, CR, LF and CRLF counts */
-   unsigned nul, lonecr, lonelf, crlf;
+   unsigned stat_bits, lonecr, lonelf, crlf;
 
/* These are just approximations! */
unsigned printable, nonprintable;
 };
 
-static void gather_stats(const char *buf, unsigned long size, struct text_stat 
*stats)
+static void gather_stats_partly(const char *buf, unsigned long len,
+   struct text_stat *stats, unsigned earlyout)
 {
unsigned long i;
 
-   memset(stats, 0, sizeof(*stats));
-
-   for (i = 0; i < size; i++) {
+   if (!buf || !len)
+   return;
+   for (i = 0; i < len; i++) {
unsigned char c = buf[i];
if (c == '\r') {
-   if (i+1 < size && buf[i+1] == '\n') {
+   stats->stat_bits |= CONVERT_STAT_BITS_ANY_CR;
+   if (i+1 < len && buf[i+1] == '\n') {
stats->crlf++;
i++;
-   } else
+   stats->stat_bits |= CONVERT_STAT_BITS_TXT_CRLF;
+   } else {
stats->lonecr++;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
+   }
continue;
}
if (c == '\n') {
stats->lonelf++;
+   stats->stat_bits |= CONVERT_STAT_BITS_TXT_LF;
continue;
}
if (c == 127)
@@ -67,7 +74,7 @@ static void gather_stats(const char *buf, unsigned long size, 
struct text_stat *
stats->printable++;
break;
case 0:
-   stats->nul++;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
/* fall through */
default:
stats->nonprintable++;
@@ -75,10 +82,12 @@ static void gather_stats(const char *buf, unsigned long 
size, struct text_stat *
}
else
stats->printable++;
+   if (stats->stat_bits & earlyout)
+   break; /* We found what we have been searching for */
}
 
/* If file ends with EOF then don't count this EOF as non-printable. */
-   if (size >= 1 && buf[size-1] == '\032')
+   if (len >= 1 && buf[len-1] == '\032')
stats->nonprintable--;
 }
 
@@ -86,41 +95,62 @@ static void gather_stats(const char *buf, unsigned long 
size, struct text_stat *
  * The same heuristics as diff.c::mmfile_is_binary()
  * We treat files with bare CR as binary
  */
-static int convert_is_binary(unsigned long size, const struct text_stat *stats)
+static void convert_nonprintable(struct text_stat *stats)
 {
-   if (stats->lonecr)
-   return 1;
-   if (stats->nul)
-   return 1;
if ((stats->printable >> 7) < stats->nonprintable)
-   return 1;
-   return 0;
+   stats->stat_bits |= CONVERT_STAT_BITS_BIN;
 }
 
-static unsigned int gather_convert_stats(const char *data, unsigned long s

[PATCH v1 1/2] read-cache: factor out get_sha1_from_index() helper

2016-10-09 Thread tboegi
From: Torsten Bögershausen 

Factor out the retrieval of the sha1 for a given path in
read_blob_data_from_index() into the function get_sha1_from_index().

This will be used in the next commit, when convert.c can do the
analyze for "text=auto" without slurping the whole blob into memory
at once.

Add a wrapper definition get_sha1_from_cache().
---
 cache.h  |  3 +++
 read-cache.c | 29 ++---
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/cache.h b/cache.h
index 1604e29..04de209 100644
--- a/cache.h
+++ b/cache.h
@@ -380,6 +380,7 @@ extern void free_name_hash(struct index_state *istate);
 #define unmerge_cache_entry_at(at) unmerge_index_entry_at(&the_index, at)
 #define unmerge_cache(pathspec) unmerge_index(&the_index, pathspec)
 #define read_blob_data_from_cache(path, sz) 
read_blob_data_from_index(&the_index, (path), (sz))
+#define get_sha1_from_cache(path)  get_sha1_from_index (&the_index, (path))
 #endif
 
 enum object_type {
@@ -1089,6 +1090,8 @@ static inline void *read_sha1_file(const unsigned char 
*sha1, enum object_type *
return read_sha1_file_extended(sha1, type, size, LOOKUP_REPLACE_OBJECT);
 }
 
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path);
+
 /*
  * This internal function is only declared here for the benefit of
  * lookup_replace_object().  Please do not call it directly.
diff --git a/read-cache.c b/read-cache.c
index 38d67fa..5a1df14 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2290,13 +2290,27 @@ int index_name_is_other(const struct index_state 
*istate, const char *name,
 
 void *read_blob_data_from_index(struct index_state *istate, const char *path, 
unsigned long *size)
 {
-   int pos, len;
+   const unsigned char *sha1;
unsigned long sz;
enum object_type type;
void *data;
 
-   len = strlen(path);
-   pos = index_name_pos(istate, path, len);
+   sha1 = get_sha1_from_index(istate, path);
+   if (!sha1)
+   return NULL;
+   data = read_sha1_file(sha1, &type, &sz);
+   if (!data || type != OBJ_BLOB) {
+   free(data);
+   return NULL;
+   }
+   if (size)
+   *size = sz;
+   return data;
+}
+
+const unsigned char *get_sha1_from_index(struct index_state *istate, const 
char *path)
+{
+   int pos = index_name_pos(istate, path, strlen(path));
if (pos < 0) {
/*
 * We might be in the middle of a merge, in which
@@ -2312,14 +2326,7 @@ void *read_blob_data_from_index(struct index_state 
*istate, const char *path, un
}
if (pos < 0)
return NULL;
-   data = read_sha1_file(istate->cache[pos]->oid.hash, &type, &sz);
-   if (!data || type != OBJ_BLOB) {
-   free(data);
-   return NULL;
-   }
-   if (size)
-   *size = sz;
-   return data;
+   return istate->cache[pos]->oid.hash;
 }
 
 void stat_validity_clear(struct stat_validity *sv)
-- 
2.10.0



Re: %C(auto) not working as expected

2016-10-09 Thread Tom Hale

On 2016-10-09 13:47, René Scharfe wrote:


%Cgreen emits color codes unconditionally.  %C(auto,green) would respect
the config settings.


Thanks, I've never seen the (,) syntax documented before!

What's strange is that this works:
  %C(auto,green bold)
but
  %C(auto,green,bold)
does not.

Also:
Given it's very rare to want only part of a string to emit colour codes, 
if something like "bold" carries through until a "no-bold", why doesn't 
"auto" do the same thing?


--
Tom Hale


Re: A head scratcher, clone results in modified files (tested linux and cygwin) - .gitattributes file?

2016-10-09 Thread Torsten Bögershausen



On 09/10/16 08:48, Jason Pyeron wrote:


The whole .gitattributes needs to be adopted, I think

Git 2.10 or higher has "git ls-files --eol":

git ls-files --eol   | grep "i/crlf.*auto"
i/crlf  w/crlf  attr/text=auto src/site/xdoc/upgradeto2_3.xml
i/crlf  w/crlf  attr/text=auto 
src/test/resources/org/apache/commons/io/FileUtilsTestDataCRLF.dat

i/crlf  w/crlf  attr/text=auto src/test/resources/test-file-gbk.bin
i/crlf  w/crlf  attr/text=auto 
src/test/resources/test-file-iso8859-1-shortlines-win-linebr.bin

i/crlf  w/crlf  attr/text=auto src/test/resources/test-file-utf8-win-linebr.bin
i/crlf  w/crlf  attr/text=auto src/test/resources/test-file-windows-31j.bin
i/crlf  w/crlf  attr/text=auto src/test/resources/test-file-x-windows-949.bin
i/crlf  w/crlf  attr/text=auto src/test/resources/test-file-x-windows-950.bin

Problem:
xml file had been commited  with CRLF : either normalize it or declare "-text".

The same is valid for the other files as well.
They are identified by auto as text, and commited with CRLF.
My feeling is that they should be declared as "-text".
Or, to be more compatible, with "-crlf":

Solution:
Make up your mind about the xml file and the html files.
If they are text, they need to be normalized.


Question:
What happens, if you do this:
# Auto detect text files and perform LF normalization
*crlf=auto

*.bin-crlf
*.dat-crlf
*.java   crlf diff=java
*.html   crlf diff=html
*.csscrlf
*.js crlf
*.sqlcrlf





Re: [PATCH v2 20/25] sequencer: left-trim lines read from the script

2016-10-09 Thread Johannes Sixt

Am 09.10.2016 um 10:57 schrieb Johannes Schindelin:

Good point. I decided to do it at a different level, though:
parse_insn_line() should already receive the line without trailing
end-of-line markers (this was already the case for LF-only todo scripts).

I reused your commit message and touched it up a bit, hope you don't mind!


I don't mind at all.

Thanks,
-- Hannes



Re: Bug? git worktree fails with master on bare repo

2016-10-09 Thread Duy Nguyen
On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker
 wrote:
> On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
>> Hey all,
>> I'm working on some server-side software to do a merge. By using git
>> worktree it's possible to check out a given branch for a bare repo and
>> merge another branch into it. It's very fast, even with large
>> repositories.
>>
>> The only exception seems to be merging to master. When I do git
>> worktree add /tmp/path/to/worktree master I get an error:
>>
>> [fatal: 'master' is already checked out at '/path/to/bare/repo']
>>
>> But this is clearly not true, git worktree list gives:
>>
>> [/path/to/bare/repo (bare)]
>>
>> ...and of course, there is no work tree at that path, just the bare
>> repo files you'd expect.
>
> The worktree code treats the base repo as a worktree, even if it's
> bare. For the purpose of being able to do a checkout of the main branch
> of a bare repo, this patch should do:
>
> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
> index 4bcc335..b618d6b 100755
> --- a/t/t2025-worktree-add.sh
> +++ b/t/t2025-worktree-add.sh
> @@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without 
> "add"' '
> )
>  '
>
> +test_expect_success '"add" default branch of a bare repo' '
> +   (
> +   git clone --bare . bare2 &&
> +   cd bare2 &&
> +   git worktree add ../there3 master
> +   )
> +'
> +
>  test_expect_success 'checkout with grafts' '
> test_when_finished rm .git/info/grafts &&
> test_commit abc &&
> diff --git a/worktree.c b/worktree.c
> index 5acfe4c..35e95b7 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -345,6 +345,8 @@ const struct worktree *find_shared_symref(const char 
> *symref,
>
> for (i = 0; worktrees[i]; i++) {
> struct worktree *wt = worktrees[i];
> +   if(wt->is_bare)
> +   continue;
>
> if (wt->is_detached && !strcmp(symref, "HEAD")) {
> if (is_worktree_being_rebased(wt, target)) {
>
>

You're fast :) I'm still studying  8d9fdd7 (worktree.c: check whether
branch is rebased in another worktree - 2016-04-22). But yeah that
should fix it.

> But I'm wondering why the worktree code does this. A bare repo isn't a
> worktree and I think it shouldn't treat it as one. A patch that rips
> out this feature and updates the tests to match would look like this:
>
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 5c4854d..3600530 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -382,15 +382,11 @@ static int add(int ac, const char **av, const char 
> *prefix)
>  static void show_worktree_porcelain(struct worktree *wt)
>  {
> printf("worktree %s\n", wt->path);
> -   if (wt->is_bare)
> -   printf("bare\n");
> -   else {
> -   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
> -   if (wt->is_detached)
> -   printf("detached\n");
> -   else
> -   printf("branch %s\n", wt->head_ref);
> -   }
> +   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
> +   if (wt->is_detached)
> +   printf("detached\n");
> +   else
> +   printf("branch %s\n", wt->head_ref);
> printf("\n");
>  }

This goes back to the first very first commit of "git worktree list":
bb9c03b (worktree: add 'list' command - 2015-10-08) and was sort of
pointed out during review [1] but nobody answered it.

A bare repo does not have an associated worktree. However only main
worktree can be bare. If we take this out, "git worktree list"'s first
line will no longer be about the main worktree (because it does not
exist). That may cause trouble since we promised in "git-worktree.txt"
that the main worktree is listed first. I don't think we have any way
else to determine if the main worktree exists. Showing "bare" may be
the way to see if we have a main worktree or not. So we probably want
to keep this function unchanged.

[1] 
https://public-inbox.org/git/%3CCANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=azn1a3so+w...@mail.gmail.com%3E/
-- 
Duy


Re: Feature request: use relative path in worktree config files

2016-10-09 Thread Duy Nguyen
On Sat, Oct 8, 2016 at 4:35 PM, Stéphane Klein
 wrote:
> Hi,
>
> "git worktree add" write absolute path in ".git/gitdir"
>
> The code source is here
> https://git.kernel.org/cgit/git/git.git/tree/builtin/worktree.c?h=v2.10.1#n256
>
> Is it possible to use relative path in this config files:
>
> * [main_worktree]/.git/worktrees/[worktree_foobar]/gitdir

The problem with relative is the question "relative to where" and the
answer has to be the same when asked from any worktree. For this file,
it may be ok after we find a good anchor point (which I have avoided
because it gives me headache and absolute paths just work).

> * [worktree_foobar]/.git

This is made absolute on purpose. So that if you move worktree_foobar
away manually, it can still point back to
"[main_worktree]/.git/worktrees/[woktree_foobar]". I'm not sure if we
want relative paths here.

> Why:
>
> 1. I configure worktree on my host
> 2. next I use this git working copy in Docker with volume share
> 3. next I've some git error in Docker because config files use absolute path

I think the common way of dealing with this in docker is put things in
the same path where it actually is outside docker. If you have stuff
at /path/to/foo, then you create the same /path/to/foo inside docker
and bind the data to that path. Does that work?
-- 
Duy


Re: [PATCH v2 3/3] gitweb: Link to "git describe"'d commits in log messages

2016-10-09 Thread Ævar Arnfjörð Bjarmason
On Thu, Oct 6, 2016 at 9:51 PM, Junio C Hamano  wrote:
> Ævar Arnfjörð Bjarmason   writes:
>
>> Change the log formatting function to know about "git describe" output
>> such as "v2.8.0-4-g867ad08", in addition to just plain "867ad08".
>>
>> There are still many valid refnames that we don't link to
>> e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
>> v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
>> similarly it's trivially possible to create some refnames like
>> "æ/var-gf6727b0" or which won't be picked up by this regex.
>
> Not a serious counter-proposal or suggestion, and certainly not an
> objection to the patch I am responding to, but I wonder if it is so
> bad if we made the 867ad08 into link when showing v2.8.0-4-g867ad08.
>
> IOW, in addition to \b followed by [0-9a-f]+ followed by \b, if we
> allowed an optional 'g' in front of the hex, e.g.
>
> -   $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> +   $line =~ s{\bg?([0-9a-fA-F]{7,40})\b}{
>
> wouldn't that be much simpler, covers more cases and sufficient?

It would be more simpler, maybe that's the right approach. I have a
preference for making the entire reference a link. I think it makes
more sense for the UI.

>> There's surely room for improvement here, but I just wanted to address
>> the very common case of sticking "git describe" output into commit
>> messages without trying to link to all possible refnames, that's going
>> to be a rather futile exercise given that this is free text, and it
>> would be prohibitively expensive to look up whether the references in
>> question exist in our repository.
>>
>> There was on-list discussion about how we could do better than this
>> patch. Junio suggested to update parse_commits() to call a new
>> "gitweb--helper" command which would pass each of the revision
>> candidates through "rev-parse --verify --quiet". That would cut down
>> on our false positives (e.g. we'll link to "deadbeef"), and also allow
>> us to be more aggressive in selecting candidate revisions.
>>
>> That may be too expensive to work in practice, or it may
>> not. Investigating that would be a good follow-up to this patch.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>>  gitweb/gitweb.perl | 18 --
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index 92b5e91..7cf68f0 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
>>   my $line = shift;
>>
>>   $line = esc_html($line, -nbsp=>1);
>> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
>> + $line =~ s{
>> +\b
>> +(
>> +# The output of "git describe", e.g. v2.10.0-297-gf6727b0
>> +# or hadoop-20160921-113441-20-g094fb7d
>> +(?> +[A-Za-z0-9.-]+
>> +(?!\.) # refs can't end with ".", see check_refname_format()
>> +-g[0-9a-fA-F]{7,40}
>> +|
>> +# Just a normal looking Git SHA1
>> +[0-9a-fA-F]{7,40}
>> +)
>> +\b
>> +}{
>>   $cgi->a({-href => href(action=>"object", hash=>$1),
>>   -class => "text"}, $1);
>> - }eg;
>> + }egx;
>>
>>   return $line;
>>  }


Re: Feature request: use relative path in worktree config files

2016-10-09 Thread Stéphane Klein
2016-10-09 13:11 GMT+02:00 Duy Nguyen :

>> * [worktree_foobar]/.git
> This is made absolute on purpose. So that if you move worktree_foobar
> away manually, it can still point back to
> "[main_worktree]/.git/worktrees/[woktree_foobar]".

Same problem if you move origin git repository.

>
>> Why:
>>
>> 1. I configure worktree on my host
>> 2. next I use this git working copy in Docker with volume share
>> 3. next I've some git error in Docker because config files use absolute path
>
> I think the common way of dealing with this in docker is put things in
> the same path where it actually is outside docker. If you have stuff
> at /path/to/foo, then you create the same /path/to/foo inside docker
> and bind the data to that path. Does that work?

It's not always possible. I can't in my project.

I think there are some pros and some cons for relative path and absolute path.
Maybe append a "--relative" option with `git worktree add` ?

I've converted all path to relative and all work with success.

What do you think to append this --relative option.

Best regards,
Stéphane


Re: Feature request: use relative path in worktree config files

2016-10-09 Thread Duy Nguyen
On Sun, Oct 9, 2016 at 6:22 PM, Stéphane Klein
 wrote:
> 2016-10-09 13:11 GMT+02:00 Duy Nguyen :
>
>>> * [worktree_foobar]/.git
>> This is made absolute on purpose. So that if you move worktree_foobar
>> away manually, it can still point back to
>> "[main_worktree]/.git/worktrees/[woktree_foobar]".
>
> Same problem if you move origin git repository.

We could fix up after moving the origin repository (because "gitdir"
file so far uses absolute paths, so we know where all the worktrees
are). But we have not done that.

>>> Why:
>>>
>>> 1. I configure worktree on my host
>>> 2. next I use this git working copy in Docker with volume share
>>> 3. next I've some git error in Docker because config files use absolute path
>>
>> I think the common way of dealing with this in docker is put things in
>> the same path where it actually is outside docker. If you have stuff
>> at /path/to/foo, then you create the same /path/to/foo inside docker
>> and bind the data to that path. Does that work?
>
> It's not always possible. I can't in my project.
>
> I think there are some pros and some cons for relative path and absolute path.
> Maybe append a "--relative" option with `git worktree add` ?
>
> I've converted all path to relative and all work with success.
>
> What do you think to append this --relative option.

Patches are welcome.
-- 
Duy


Re: Feature request: use relative path in worktree config files

2016-10-09 Thread Stéphane Klein
2016-10-09 13:37 GMT+02:00 Duy Nguyen :
> On Sun, Oct 9, 2016 at 6:22 PM, Stéphane Klein
>  wrote:
>> 2016-10-09 13:11 GMT+02:00 Duy Nguyen :
 Why:

 1. I configure worktree on my host
 2. next I use this git working copy in Docker with volume share
 3. next I've some git error in Docker because config files use absolute 
 path
>>>
>>> I think the common way of dealing with this in docker is put things in
>>> the same path where it actually is outside docker. If you have stuff
>>> at /path/to/foo, then you create the same /path/to/foo inside docker
>>> and bind the data to that path. Does that work?
>>
>> It's not always possible. I can't in my project.
>>
>> I think there are some pros and some cons for relative path and absolute 
>> path.
>> Maybe append a "--relative" option with `git worktree add` ?
>>
>> I've converted all path to relative and all work with success.
>>
>> What do you think to append this --relative option.
>
> Patches are welcome.

Thanks :)


[PATCH] contrib: add credential helper for libsecret

2016-10-09 Thread Mantas Mikulėnas
This is based on the existing gnome-keyring helper, but instead of
libgnome-keyring (which was specific to GNOME and is deprecated), it
uses libsecret which can support other implementations of XDG Secret
Service API.

Passes t0303-credential-external.sh.

Signed-off-by: Mantas Mikulėnas 
---
 contrib/credential/libsecret/Makefile  |  25 ++
 .../libsecret/git-credential-libsecret.c   | 370 +
 2 files changed, 395 insertions(+)
 create mode 100644 contrib/credential/libsecret/Makefile
 create mode 100644 contrib/credential/libsecret/git-credential-libsecret.c

diff --git a/contrib/credential/libsecret/Makefile 
b/contrib/credential/libsecret/Makefile
new file mode 100644
index ..3e67552cc5b5
--- /dev/null
+++ b/contrib/credential/libsecret/Makefile
@@ -0,0 +1,25 @@
+MAIN:=git-credential-libsecret
+all:: $(MAIN)
+
+CC = gcc
+RM = rm -f
+CFLAGS = -g -O2 -Wall
+PKG_CONFIG = pkg-config
+
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+INCS:=$(shell $(PKG_CONFIG) --cflags libsecret-1 glib-2.0)
+LIBS:=$(shell $(PKG_CONFIG) --libs libsecret-1 glib-2.0)
+
+SRCS:=$(MAIN).c
+OBJS:=$(SRCS:.c=.o)
+
+%.o: %.c
+   $(CC) $(CFLAGS) $(CPPFLAGS) $(INCS) -o $@ -c $<
+
+$(MAIN): $(OBJS)
+   $(CC) -o $@ $(LDFLAGS) $^ $(LIBS)
+
+clean:
+   @$(RM) $(MAIN) $(OBJS)
diff --git a/contrib/credential/libsecret/git-credential-libsecret.c 
b/contrib/credential/libsecret/git-credential-libsecret.c
new file mode 100644
index ..4c56979d8a08
--- /dev/null
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -0,0 +1,370 @@
+/*
+ * Copyright (C) 2011 John Szakmeister 
+ *   2012 Philipp A. Hartmann 
+ *   2016 Mantas Mikulėnas 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+/*
+ * Credits:
+ * - GNOME Keyring API handling originally written by John Szakmeister
+ * - ported to credential helper API by Philipp A. Hartmann
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/*
+ * This credential struct and API is simplified from git's credential.{h,c}
+ */
+struct credential {
+   char *protocol;
+   char *host;
+   unsigned short port;
+   char *path;
+   char *username;
+   char *password;
+};
+
+#define CREDENTIAL_INIT { NULL, NULL, 0, NULL, NULL, NULL }
+
+typedef int (*credential_op_cb)(struct credential *);
+
+struct credential_operation {
+   char *name;
+   credential_op_cb op;
+};
+
+#define CREDENTIAL_OP_END { NULL, NULL }
+
+/* - Secret Service functions - */
+
+static char *make_label(struct credential *c)
+{
+   if (c->port)
+   return g_strdup_printf("Git: %s://%s:%hu/%s",
+   c->protocol, c->host, c->port, c->path 
? c->path : "");
+   else
+   return g_strdup_printf("Git: %s://%s/%s",
+   c->protocol, c->host, c->path ? c->path 
: "");
+}
+
+static GHashTable *make_attr_list(struct credential *c)
+{
+   GHashTable *al = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, 
g_free);
+
+   if (c->username)
+   g_hash_table_insert(al, "user", g_strdup(c->username));
+   if (c->protocol)
+   g_hash_table_insert(al, "protocol", g_strdup(c->protocol));
+   if (c->host)
+   g_hash_table_insert(al, "server", g_strdup(c->host));
+   if (c->port)
+   g_hash_table_insert(al, "port", g_strdup_printf("%hu", 
c->port));
+   if (c->path)
+   g_hash_table_insert(al, "object", g_strdup(c->path));
+
+   return al;
+}
+
+static int keyring_get(struct credential *c)
+{
+   SecretService *service = NULL;
+   GHashTable *attributes = NULL;
+   GError *error = NULL;
+   GList *items = NULL;
+
+   if (!c->protocol || !(c->host || c->path))
+   return EXIT_FAILURE;
+
+   service = secret_service_get_sync(0, NULL, &error);
+   if (error != NULL) {
+   g_critical("could not connect to Secret Service: %s", 
error->message);
+   g_error_free(error);
+   return EXIT_FAILURE;
+   }
+
+   attributes = make_attr_list(c);
+   items = secret_service_search_sync(service,
+  

Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

2016-10-09 Thread Duy Nguyen
On Sun, Oct 9, 2016 at 1:01 PM, Jeff King  wrote:
> On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote:
>
>> > > Maybe it's time to aim for
>> > >
>> > >   git config alias.d2u.shell \
>> > >'f() { git ls-files "$@" | xargs dos2unix; }; f'
>> > >   git config alias.d2u.cdup false
>> > >   git d2u *.c   # yada!
>> >
>> > That would be nice. It would also allow "alias.foo_bar.shell"; right now
>> > "alias.foo_bar" is forbidden because of the config syntax, which does
>> > not allow underscores outside of the "subsection" name.
>>
>> So what about this?
>>
>>   [alias]
>>   d2u = !dos2unix
>>   [alias "d2u"]
>>   shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f'
>>   exec = C:/cygwin64/bin/dos2unix.exe
>>
>> You introduce all kinds of ambiguities here that did not exist before...
>
> If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> then yes, I think that's an unavoidable part of the transition.  IMHO,
> the new should take precedence over the old, and people will gradually
> move from one to the other.

Do we really need to treat this differently than

[alias]
d2u = !dos2unix
d2u = C:/cygwin/bin/dos3unix.exe

?

Another similar case is one d2u (could be either old syntax or new) is
defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
either case, the "latest" d2u definition wins.

> If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> example, the problem is that you have keys with overlapping meanings.
> One solution is "don't do that" (so have a key like "cmd", and another
> to select "shell or git-cmd", etc). Another is to define some rule, like
> "last one wins" (so "exec" overrides "shell" in your example).
>
> I'd prefer the "don't do that" path. The config you showed is
> nonsensical, and it doesn't really matter that much how we behave. But
> it is better still if we have a config scheme that makes it hard to
> write nonsensical things in the first place.

Any suggestion? I suppose we can have _one_ key for the command. How
to execute that command (exec, shell, nocd...) are boolean options.
People can still write conflicting things. We have been nice so far,
always dying when the user specify conflicting command line options.
We could do the same here, I guess.
-- 
Duy


RE: A head scratcher, clone results in modified files (tested linux and cygwin) - .gitattributes file?

2016-10-09 Thread Jason Pyeron
> -Original Message-
> From: Torsten Bögershausen
> Sent: Sunday, October 09, 2016 06:27
> 
> On 09/10/16 08:48, Jason Pyeron wrote:
> 
> 
> The whole .gitattributes needs to be adopted, I think
> 
> Git 2.10 or higher has "git ls-files --eol":
> 
> git ls-files --eol   | grep "i/crlf.*auto"
> i/crlf  w/crlf  attr/text=auto src/site/xdoc/upgradeto2_3.xml
> i/crlf  w/crlf  attr/text=auto 
> src/test/resources/org/apache/commons/io/FileUtilsTestDataCRLF.dat
> i/crlf  w/crlf  attr/text=auto src/test/resources/test-file-gbk.bin
> i/crlf  w/crlf  attr/text=auto 
> src/test/resources/test-file-iso8859-1-shortlines-win-linebr.bin
> i/crlf  w/crlf  attr/text=auto 
> src/test/resources/test-file-utf8-win-linebr.bin
> i/crlf  w/crlf  attr/text=auto 
> src/test/resources/test-file-windows-31j.bin
> i/crlf  w/crlf  attr/text=auto 
> src/test/resources/test-file-x-windows-949.bin
> i/crlf  w/crlf  attr/text=auto 
> src/test/resources/test-file-x-windows-950.bin
> 
> Problem:
> xml file had been commited  with CRLF : either normalize it 
> or declare "-text".
> 
> The same is valid for the other files as well.
> They are identified by auto as text, and commited with CRLF.
> My feeling is that they should be declared as "-text".
> Or, to be more compatible, with "-crlf":
> 

Good call.

> Solution:
> Make up your mind about the xml file and the html files.
> If they are text, they need to be normalized.
> 
> 
> Question:
> What happens, if you do this:
> # Auto detect text files and perform LF normalization
> *crlf=auto
> 
> *.bin-crlf
> *.dat-crlf

*.bin -text
*.dat -text

#fix that issue

> *.java   crlf diff=java
> *.html   crlf diff=html
> *.csscrlf
> *.js crlf
> *.sqlcrlf
> 

Or create a subordinate

src/test/resources/.gitattributes:
*-text

Since these are "test" resources, some with text extensions from above.

Thanks!

-Jason



Re: %C(auto) not working as expected

2016-10-09 Thread René Scharfe
Am 09.10.2016 um 12:04 schrieb Tom Hale:
> On 2016-10-09 13:47, René Scharfe wrote:
> 
>> %Cgreen emits color codes unconditionally.  %C(auto,green) would respect
>> the config settings.
> 
> Thanks, I've never seen the (,) syntax documented before!

Both the prefix "auto," for terminal-detection and "%C(auto)" for
choosing colors automatically are mentioned in the manpage for git log
(from Documentation/pretty-formats.txt):

- '%C(...)': color specification, as described in color.branch.* config option;
  adding `auto,` at the beginning will emit color only when colors are
  enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
  respecting the `auto` settings of the former if we are going to a
  terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
  on the next placeholders until the color is switched again.

> What's strange is that this works:
>   %C(auto,green bold)
> but
>   %C(auto,green,bold)
> does not.

Looking at the code that's not surprising; colors and attributes are
interpreted as a space-separated list.  The prefix "auto," is handled
specially.  For a user it may look strange, admittedly.

Supporting "auto " as well would be easy.  Supporting it in such a way
that it can be mixed freely with colors and attributes as in
%C(bold auto green) would be a bit harder.  Could this lead to confusion
between the auto for terminal-detection and the one for automatic color
selection?

The documentation cited above says the color specification was explained
together with the color.branch.* config option, but that part only says
(from Documentation/config.txt):

color.branch.::
Use customized color for branch coloration. `` is one of
`current` (the current branch), `local` (a local branch),
`remote` (a remote-tracking branch in refs/remotes/),
`upstream` (upstream tracking branch), `plain` (other
refs).

It really is described earlier in the same file, in the Values section
(a fitting place, I think).  Here's just the first sentence:

color::
   The value for a variable that takes a color is a list of
   colors (at most two, one for foreground and one for background)
   and attributes (as many as you want), separated by spaces.

Patch below.  Does it help a little?

> Also:
> Given it's very rare to want only part of a string to emit colour codes,
> if something like "bold" carries through until a "no-bold", why doesn't
> "auto" do the same thing?

No state is kept for "auto,".  Attributes and colors are switched
separately by terminals, that's why e.g. bold stays in effect through
a color change -- unless you specify an attribute change as well.

Offering a way to enable terminal-detection for all color codes of a
format would be useful, but using the existing "auto," prefix for that
would be a behaviour change that could surprise users.

René


-- >8 --
Subject: [PATCH] pretty: fix document reference for color specification

Signed-off-by: Rene Scharfe 
---
 Documentation/pretty-formats.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a942d57..89e3bc6 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -166,7 +166,8 @@ endif::git-rev-list[]
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
 - '%Creset': reset color
-- '%C(...)': color specification, as described in color.branch.* config option;
+- '%C(...)': color specification, as described under Values, color in the
+  "CONFIGURATION FILE" section of linkgit:git-config[1];
   adding `auto,` at the beginning will emit color only when colors are
   enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
   respecting the `auto` settings of the former if we are going to a
-- 
2.10.1



Re: Bug? git worktree fails with master on bare repo

2016-10-09 Thread Michael Tutty
Dennis,
Thanks for the great response, and for spending time on my issue.
I'll try that first patch and see what happens.

In the meantime, it got weirder...

I created a brand-new (bare) repo and was able to git add worktree
/path master.  I was able to do this repeatedly, even using the
worktree to merge other branches to master.  I didn't find any
condition or step that caused some kind of orphan master work tree,
which was what I thought the underlying problem might be.

So, on the one hand, you found code validating my initial experience.
But on the other hand, I found a test case that didn't appear to have
that problem.

WAT.
  M.

On Sun, Oct 9, 2016 at 5:52 AM, Duy Nguyen  wrote:
> On Sun, Oct 9, 2016 at 2:51 PM, Dennis Kaarsemaker
>  wrote:
>> On Sat, 2016-10-08 at 19:30 -0500, Michael Tutty wrote:
>>> Hey all,
>>> I'm working on some server-side software to do a merge. By using git
>>> worktree it's possible to check out a given branch for a bare repo and
>>> merge another branch into it. It's very fast, even with large
>>> repositories.
>>>
>>> The only exception seems to be merging to master. When I do git
>>> worktree add /tmp/path/to/worktree master I get an error:
>>>
>>> [fatal: 'master' is already checked out at '/path/to/bare/repo']
>>>
>>> But this is clearly not true, git worktree list gives:
>>>
>>> [/path/to/bare/repo (bare)]
>>>
>>> ...and of course, there is no work tree at that path, just the bare
>>> repo files you'd expect.
>>
>> The worktree code treats the base repo as a worktree, even if it's
>> bare. For the purpose of being able to do a checkout of the main branch
>> of a bare repo, this patch should do:
>>
>> diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
>> index 4bcc335..b618d6b 100755
>> --- a/t/t2025-worktree-add.sh
>> +++ b/t/t2025-worktree-add.sh
>> @@ -138,6 +138,14 @@ test_expect_success 'checkout from a bare repo without 
>> "add"' '
>> )
>>  '
>>
>> +test_expect_success '"add" default branch of a bare repo' '
>> +   (
>> +   git clone --bare . bare2 &&
>> +   cd bare2 &&
>> +   git worktree add ../there3 master
>> +   )
>> +'
>> +
>>  test_expect_success 'checkout with grafts' '
>> test_when_finished rm .git/info/grafts &&
>> test_commit abc &&
>> diff --git a/worktree.c b/worktree.c
>> index 5acfe4c..35e95b7 100644
>> --- a/worktree.c
>> +++ b/worktree.c
>> @@ -345,6 +345,8 @@ const struct worktree *find_shared_symref(const char 
>> *symref,
>>
>> for (i = 0; worktrees[i]; i++) {
>> struct worktree *wt = worktrees[i];
>> +   if(wt->is_bare)
>> +   continue;
>>
>> if (wt->is_detached && !strcmp(symref, "HEAD")) {
>> if (is_worktree_being_rebased(wt, target)) {
>>
>>
>
> You're fast :) I'm still studying  8d9fdd7 (worktree.c: check whether
> branch is rebased in another worktree - 2016-04-22). But yeah that
> should fix it.
>
>> But I'm wondering why the worktree code does this. A bare repo isn't a
>> worktree and I think it shouldn't treat it as one. A patch that rips
>> out this feature and updates the tests to match would look like this:
>>
>>
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index 5c4854d..3600530 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -382,15 +382,11 @@ static int add(int ac, const char **av, const char 
>> *prefix)
>>  static void show_worktree_porcelain(struct worktree *wt)
>>  {
>> printf("worktree %s\n", wt->path);
>> -   if (wt->is_bare)
>> -   printf("bare\n");
>> -   else {
>> -   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
>> -   if (wt->is_detached)
>> -   printf("detached\n");
>> -   else
>> -   printf("branch %s\n", wt->head_ref);
>> -   }
>> +   printf("HEAD %s\n", sha1_to_hex(wt->head_sha1));
>> +   if (wt->is_detached)
>> +   printf("detached\n");
>> +   else
>> +   printf("branch %s\n", wt->head_ref);
>> printf("\n");
>>  }
>
> This goes back to the first very first commit of "git worktree list":
> bb9c03b (worktree: add 'list' command - 2015-10-08) and was sort of
> pointed out during review [1] but nobody answered it.
>
> A bare repo does not have an associated worktree. However only main
> worktree can be bare. If we take this out, "git worktree list"'s first
> line will no longer be about the main worktree (because it does not
> exist). That may cause trouble since we promised in "git-worktree.txt"
> that the main worktree is listed first. I don't think we have any way
> else to determine if the main worktree exists. Showing "bare" may be
> the way to see if we have a main worktree or not. So we probably want
> to keep this function unchanged.
>
> [1] 
> https://public-inbox.org/git/%3CCANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=azn1a3so+w...@mail.gmail.com%

Problem with submodules

2016-10-09 Thread ven...@gmail.com
Hi, I want to report a regression.

After cloning for example https://git.gnome.org/browse/epiphany with
git 2.10 and running ./autogen.sh I get the following errors:
http://pastebin.com/93AunRhu

The developer told me that it is probably not an issue caused by
epiphany and I should try an older git version. I installed 2.7.2 and
it works perfectly. So I think theres a bug in git 2.10.

Thanks and regards


Re: Problem with submodules

2016-10-09 Thread Dennis Kaarsemaker
On Sun, 2016-10-09 at 16:41 +0200, ven...@gmail.com wrote:
> Hi, I want to report a regression.
> 
> After cloning for example https://git.gnome.org/browse/epiphany with
> git 2.10 and running ./autogen.sh I get the following errors:
> http://pastebin.com/93AunRhu
> 
> The developer told me that it is probably not an issue caused by
> epiphany and I should try an older git version. I installed 2.7.2 and
> it works perfectly. So I think theres a bug in git 2.10.

I can't reproduce the problem with git 2.10.1

Can you share the .git/config file in your clone of epiphany please.

D.


How to watch a mailing list & repo for patches which affect a certain area of code?

2016-10-09 Thread Ian Kelling
I've got patches in various projects, and I don't have time to keep up
with the mailing list, but I'd like to help out with maintenance of that
code, or the functions/files it touches. People don't cc me. I figure I
could filter the list, test patches submitted, commits made, mentions of
files/functions, build filters based on the code I have in the repo even
if it's been moved or changed subsequently. I'm wondering what other
people have implemented already for automation around this, or general
thoughts. Web search is not showing me much.


Re: Problem with submodules

2016-10-09 Thread ven...@gmail.com
Sure, http://pastebin.com/bUFBDj0Q


Re: Problem with submodules

2016-10-09 Thread ven...@gmail.com
hm okay, it works with 2.10.0, when I remove the word 'epiphany' from
the urls in line 13 and 15

2016-10-09 21:15 GMT+02:00 ven...@gmail.com :
> Sure, http://pastebin.com/bUFBDj0Q


Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd

2016-10-09 Thread Jeff King
On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote:

> > If you mean ambiguity between the old "alias.X" and the new "alias.X.*",
> > then yes, I think that's an unavoidable part of the transition.  IMHO,
> > the new should take precedence over the old, and people will gradually
> > move from one to the other.
> 
> Do we really need to treat this differently than
> 
> [alias]
> d2u = !dos2unix
> d2u = C:/cygwin/bin/dos3unix.exe
> 
> ?
> 
> Another similar case is one d2u (could be either old syntax or new) is
> defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In
> either case, the "latest" d2u definition wins.

Yeah, that's reasonable, too. So:

  [alias]
d2u = "!dos2unix"

acts exactly as if:

  [alias "d2u"]
command = dos2unix
type = shell

was specified at that point, which is easy to understand.

> > If you mean the ambiguity between alias.X.shell and alias.X.exec in your
> > example, the problem is that you have keys with overlapping meanings.
> > One solution is "don't do that" (so have a key like "cmd", and another
> > to select "shell or git-cmd", etc). Another is to define some rule, like
> > "last one wins" (so "exec" overrides "shell" in your example).
> [...]
> 
> Any suggestion? I suppose we can have _one_ key for the command. How
> to execute that command (exec, shell, nocd...) are boolean options.
> People can still write conflicting things. We have been nice so far,
> always dying when the user specify conflicting command line options.
> We could do the same here, I guess.

Having separate exec/shell boolean options just punts the overlap from
the command key to those keys. If you have two mutually exclusive
options, I think the best thing is a single option, like:

  type = 

and then it is obvious that a second appearance of "type" overrides an
earlier one, by our usual "last one wins" convention. As opposed to:

  shell = true
  exec = true

where you have to understand the meaning of each option to know that
"exec" overrides "shell".

-Peff


RE: How to watch a mailing list & repo for patches which affect a certain area of code?

2016-10-09 Thread Jason Pyeron
> -Original Message-
> From: Ian Kelling
> Sent: Sunday, October 09, 2016 15:03
> 
> I've got patches in various projects, and I don't have time to keep up
> with the mailing list, but I'd like to help out with 
> maintenance of that
> code, or the functions/files it touches. People don't cc me. 
> I figure I
> could filter the list, test patches submitted, commits made, 
> mentions of
> files/functions, build filters based on the code I have in 
> the repo even
> if it's been moved or changed subsequently. I'm wondering what other
> people have implemented already for automation around this, or general
> thoughts. Web search is not showing me much.
> 

One thought would be to apply every patch automatically (to the branches of 
interest?). Then trigger on the [successful] changed
code. This would simplify the logic to working on the source only and not 
parsing the emails.

-Jason



Re: %C(auto) not working as expected

2016-10-09 Thread Jeff King
On Sun, Oct 09, 2016 at 03:24:17PM +0200, René Scharfe wrote:

> Offering a way to enable terminal-detection for all color codes of a
> format would be useful, but using the existing "auto," prefix for that
> would be a behaviour change that could surprise users.

Yeah. In retrospect, it probably would have been saner to make %C(red) a
noop when --color is not in effect (either because of --no-color, or
more likely when --color=auto is in effect and stdout is not a
terminal). But that ship has long since sailed, I think.

If we do a revamp of the pretty-formats to bring them more in line with
ref-filter (e.g., something like "%(color:red)") maybe that would be an
opportunity to make minor adjustments. Though, hmm, it looks like
for-each-ref already knows "%(color:red)", and it's unconditional.
 So perhaps we would need to go through some deprecation period or
other transition.

> ---
>  Documentation/pretty-formats.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/pretty-formats.txt 
> b/Documentation/pretty-formats.txt
> index a942d57..89e3bc6 100644
> --- a/Documentation/pretty-formats.txt
> +++ b/Documentation/pretty-formats.txt
> @@ -166,7 +166,8 @@ endif::git-rev-list[]
>  - '%Cgreen': switch color to green
>  - '%Cblue': switch color to blue
>  - '%Creset': reset color
> -- '%C(...)': color specification, as described in color.branch.* config 
> option;
> +- '%C(...)': color specification, as described under Values, color in the
> +  "CONFIGURATION FILE" section of linkgit:git-config[1];

I like the intent here, though I found "Values, color" hard to parse (it
was not immediately clear that you mean "the color paragraph of the
Values section", as commas are already being used in that sentence for
the parenthetical phrase).

I'm not sure how to say that succinctly, as we are four levels deep
(git-config -> configuration file -> values -> color). Too bad there is
no good link syntax for it. Maybe:

  ...color specification, as described in linkgit:git-config[1] (see the
  paragraph on colors in the "Values" section, under "CONFIGURATION
  FILE")

or something.

-Peff


Re: [PATCH] use strbuf_add_unique_abbrev() for adding short hashes, part 3

2016-10-09 Thread Jeff King
On Sat, Oct 08, 2016 at 05:38:47PM +0200, René Scharfe wrote:

> Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
> instead of taking detours through find_unique_abbrev() and its static
> buffer.  This is shorter in most cases and a bit more efficient.
> 
> The changes here are not easily handled by a semantic patch because
> they involve removing temporary variables and deconstructing format
> strings for strbuf_addf().

Yeah, the thing to look for here is whether the sha1 variable holds the
same value at both times.

These all look OK to me. Mild rambling below.

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 5200d5c..9041c2f 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -202,9 +202,9 @@ static void output_commit_title(struct merge_options *o, 
> struct commit *commit)
>   strbuf_addf(&o->obuf, "virtual %s\n",
>   merge_remote_util(commit)->name);
>   else {
> - strbuf_addf(&o->obuf, "%s ",
> - find_unique_abbrev(commit->object.oid.hash,
> - DEFAULT_ABBREV));
> + strbuf_add_unique_abbrev(&o->obuf, commit->object.oid.hash,
> +  DEFAULT_ABBREV);
> + strbuf_addch(&o->obuf, ' ');

I've often wondered whether a big strbuf_addf() is more efficient than
several strbuf_addstrs. It amortizes the size-checks, certainly, though
those are probably not very big. It shouldn't matter much for amortizing
the cost of malloc, as it's very unlikely to have a case where:

  strbuf_addf("%s%s", foo, bar);

would require one malloc, but:

  strbuf_addstr(foo);
  strbuf_addstr(bar);

would require two (one of the strings would have to be around the same
size as the ALLOC_GROW() doubling).

So it probably doesn't matter much in practice (not that most of these
cases are very performance sensitive anyway). Mostly just something I've
pondered while tweaking strbuf invocations.

And anyway, in this case replacing find_unique_abbrev lets us write
directly into the final buffer rather than copying through a static
buffer, so it's almost certainly a win there.

> diff --git a/pretty.c b/pretty.c
> index 25efbca..0c31495 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -544,15 +544,13 @@ static void add_merge_info(const struct 
> pretty_print_context *pp,
>   strbuf_addstr(sb, "Merge:");
>  
>   while (parent) {
> - struct commit *p = parent->item;
> - const char *hex = NULL;
> + struct object_id *oidp = &parent->item->object.oid;
> + strbuf_addch(sb, ' ');
>   if (pp->abbrev)
> - hex = find_unique_abbrev(p->object.oid.hash, 
> pp->abbrev);
> - if (!hex)
> - hex = oid_to_hex(&p->object.oid);

Wow, this existing code was hard to follow. I wondered when
find_unique_abbrev() would return NULL, but it never does. This "if
(!hex)" should have been an "else" all along.

> + strbuf_add_unique_abbrev(sb, oidp->hash, pp->abbrev);
> + else
> + strbuf_addstr(sb, oid_to_hex(oidp));

...which I see you changed here, though perhaps it is not immediately
obvious that it is correct without knowing find_unique_abbrev's return
value assumptions.

Just thinking aloud, I've also wondered if strbuf_addoid() would be
handy.  We already have oid_to_hex_r(). In fact, you could write it
already as:

  strbuf_add_unique_abbrev(sb, oidp->hash, 0);

but that is probably not helping maintainability. ;)

> diff --git a/submodule.c b/submodule.c
> index 2de06a3..476f60b 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -392,10 +392,9 @@ static void show_submodule_header(FILE *f, const char 
> *path,
>   }
>  
>  output_header:
> - strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
> - find_unique_abbrev(one->hash, DEFAULT_ABBREV));
> - if (!fast_backward && !fast_forward)
> - strbuf_addch(&sb, '.');
> + strbuf_addf(&sb, "%s%sSubmodule %s ", line_prefix, meta, path);
> + strbuf_add_unique_abbrev(&sb, one->hash, DEFAULT_ABBREV);
> + strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");

Spelling out ".." and "..." as you do makes this much clearer, IMHO.

-Peff


Re: git 2.10.1 test regression in t4014-format-patch.sh

2016-10-09 Thread Josh Triplett
On October 9, 2016 5:15:22 PM PDT, Jeremy Huddleston Sequoia 
 wrote:
>Hey Josh,
>
>Hope you're doing well.
>
>I wanted to let you know that this patch of yours, which landed in git
>2.10.1, introduced some test failures, seen on macOS.
>
>Let me know if you need any additional information to track these down.
>
>Thanks,
>Jeremy
>
>
>not ok 65 - format-patch default signature
>#  
>#  git format-patch --stdout -1 | tail -n 3 >output &&
>#  signature >expect &&
>#  test_cmp expect output
>#  
>
>not ok 132 - format-patch --base
>#  
>#  git checkout side &&
>#  git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual 
>&&
>#  echo >expected &&
>#  echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
>#  echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git
>patch-id --stable | awk "{print \$1}")" >>expected &&
>#  echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git
>patch-id --stable | awk "{print \$1}")" >>expected &&
>#  signature >> expected &&
>#  test_cmp expected actual
>#  

Can you run the test with the option to show the expected and actual strings?

Did the testsuite run with the wrong git somehow?




git 2.10.1 test regression in t4014-format-patch.sh

2016-10-09 Thread Jeremy Huddleston Sequoia
Hey Josh,

Hope you're doing well.

I wanted to let you know that this patch of yours, which landed in git 2.10.1, 
introduced some test failures, seen on macOS.

Let me know if you need any additional information to track these down.

Thanks,
Jeremy


not ok 65 - format-patch default signature
#   
#   git format-patch --stdout -1 | tail -n 3 >output &&
#   signature >expect &&
#   test_cmp expect output
#   

not ok 132 - format-patch --base
#   
#   git checkout side &&
#   git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual 
&&
#   echo >expected &&
#   echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
#   echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git 
patch-id --stable | awk "{print \$1}")" >>expected &&
#   echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git 
patch-id --stable | awk "{print \$1}")" >>expected &&
#   signature >> expected &&
#   test_cmp expected actual
#   


commit 480871e09ed2e5275b4ba16b278681e5a8c122ae
Author: Josh Triplett 
Date:   Wed Sep 7 18:12:01 2016 -0700

format-patch: show base info before email signature

Any text below the "-- " for the email signature gets treated as part of
the signature, and many mail clients will trim it from the quoted text
for a reply.  Move it above the signature, so people can reply to it
more easily.

Similarly, when producing the patch as a MIME attachment, the
original code placed the base info after the attached part, which
would be discarded.  Move the base info to the end of the part,
still inside the part boundary.

Add tests for the exact format of the email signature, and add tests
to ensure that the base info appears before the email signature when
producing a plain-text output, and that it appears before the part
boundary when producing a MIME attachment.

Signed-off-by: Josh Triplett 
Signed-off-by: Junio C Hamano 




smime.p7s
Description: S/MIME cryptographic signature


git 2.10.1 test regression in t3700-add.sh

2016-10-09 Thread Jeremy Huddleston Sequoia
Hi Thomas,

I wanted to let you know that this patch of yours, which landed in git 2.10.1, 
introduced some test failures, seen on macOS.

Let me know if you need any additional information to track these down.

Thanks,
Jeremy

not ok 40 - git add --chmod=[+-]x changes index with already added file
#   
#   echo foo >foo3 &&
#   git add foo3 &&
#   git add --chmod=+x foo3 &&
#   test_mode_in_index 100755 foo3 &&
#   echo foo >xfoo3 &&
#   chmod 755 xfoo3 &&
#   git add xfoo3 &&
#   git add --chmod=-x xfoo3 &&
#   test_mode_in_index 100644 xfoo3
#   

commit 610d55af0f082f6b866dc858e144c03d8ed4424c
Author: Thomas Gummerer 
Date:   Wed Sep 14 22:07:47 2016 +0100

add: modify already added files when --chmod is given

When the chmod option was added to git add, it was hooked up to the diff
machinery, meaning that it only works when the version in the index
differs from the version on disk.

As the option was supposed to mirror the chmod option in update-index,
which always changes the mode in the index, regardless of the status of
the file, make sure the option behaves the same way in git add.

Signed-off-by: Thomas Gummerer 
Signed-off-by: Junio C Hamano 




smime.p7s
Description: S/MIME cryptographic signature


Re: git 2.10.1 test regression in t3700-add.sh

2016-10-09 Thread Jeremy Huddleston Sequoia

> On Oct 9, 2016, at 17:15, Jeremy Huddleston Sequoia 
>  wrote:
> 
> Hi Thomas,
> 
> I wanted to let you know that this patch of yours, which landed in git 
> 2.10.1, introduced some test failures, seen on macOS.
> 
> Let me know if you need any additional information to track these down.
> 
> Thanks,
> Jeremy
> 
> not ok 40 - git add --chmod=[+-]x changes index with already added file
> # 
> # echo foo >foo3 &&
> # git add foo3 &&
> # git add --chmod=+x foo3 &&
> # test_mode_in_index 100755 foo3 &&
> # echo foo >xfoo3 &&
> # chmod 755 xfoo3 &&
> # git add xfoo3 &&
> # git add --chmod=-x xfoo3 &&
> # test_mode_in_index 100644 xfoo3
> # 
> 
> commit 610d55af0f082f6b866dc858e144c03d8ed4424c
> Author: Thomas Gummerer 
> Date:   Wed Sep 14 22:07:47 2016 +0100
> 
>add: modify already added files when --chmod is given
> 
>When the chmod option was added to git add, it was hooked up to the diff
>machinery, meaning that it only works when the version in the index
>differs from the version on disk.
> 
>As the option was supposed to mirror the chmod option in update-index,
>which always changes the mode in the index, regardless of the status of
>the file, make sure the option behaves the same way in git add.
> 
>Signed-off-by: Thomas Gummerer 
>Signed-off-by: Junio C Hamano 


This failure looks odd.  I'll dig into it a bit more as it looks like something 
odd is going on here...

expecting success: 
echo foo >foo3 &&
git add foo3 &&
git add --chmod=+x foo3 &&
test_mode_in_index 100755 foo3 &&
echo foo >xfoo3 &&
chmod 755 xfoo3 &&
git add xfoo3 &&
git add --chmod=-x xfoo3 &&
test_mode_in_index 100644 xfoo3

pass
cannot chmod 'xfoo3'fail
12 c5c4ca97a3a080c32920941b665e94a997901491 0   xfoo3
not ok 40 - git add --chmod=[+-]x changes index with already added file
#   
#   echo foo >foo3 &&
#   git add foo3 &&
#   git add --chmod=+x foo3 &&
#   test_mode_in_index 100755 foo3 &&
#   echo foo >xfoo3 &&
#   chmod 755 xfoo3 &&
#   git add xfoo3 &&
#   git add --chmod=-x xfoo3 &&
#   test_mode_in_index 100644 xfoo3
#   



smime.p7s
Description: S/MIME cryptographic signature


Re: git 2.10.1 test regression in t4014-format-patch.sh

2016-10-09 Thread Jeremy Huddleston Sequoia

> On Oct 9, 2016, at 17:18, Josh Triplett  wrote:
> 
> On October 9, 2016 5:15:22 PM PDT, Jeremy Huddleston Sequoia 
>  wrote:
>> Hey Josh,
>> 
>> Hope you're doing well.
>> 
>> I wanted to let you know that this patch of yours, which landed in git
>> 2.10.1, introduced some test failures, seen on macOS.
>> 
>> Let me know if you need any additional information to track these down.
>> 
>> Thanks,
>> Jeremy
>> 
>> 
>> not ok 65 - format-patch default signature
>> #
>> #git format-patch --stdout -1 | tail -n 3 >output &&
>> #signature >expect &&
>> #test_cmp expect output
>> #
>> 
>> not ok 132 - format-patch --base
>> #
>> #git checkout side &&
>> #git format-patch --stdout --base=HEAD~3 -1 | tail -n 7 >actual 
>> &&
>> #echo >expected &&
>> #echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
>> #echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git
>> patch-id --stable | awk "{print \$1}")" >>expected &&
>> #echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git
>> patch-id --stable | awk "{print \$1}")" >>expected &&
>> #signature >> expected &&
>> #test_cmp expected actual
>> #
> 
> Can you run the test with the option to show the expected and actual strings?
> Did the testsuite run with the wrong git somehow?

Nope, it's the right version being tested.  The failure seems due to your 
git_version change not liking our formatting

$ git --version
git version 2.10.1 (Apple Git-99)

(the 'Apple Git-XX' being added because this was from a build that had Apple's 
patch series, including 
https://github.com/jeremyhu/git/commit/f99905d0752d923e5ec61e14c675a300c6d04284)

We modify DEF_VER, which confused your regex.  Simple patch inc in a separate 
email.

Thanks,
Jeremy



smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER

2016-10-09 Thread Josh Triplett
On October 9, 2016 7:53:23 PM PDT, Jeremy Huddleston Sequoia 
 wrote:
>Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>Signed-off-by: Jeremy Huddleston Sequoia 
>CC: Josh Triplett 
>CC: Junio C Hamano 

Looks reasonable to me. Didn't realize git versions could have spaces.
Reviewed-by: Josh Triplett 

> t/t4014-format-patch.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>index 8d90a6e..33f6940 100755
>--- a/t/t4014-format-patch.sh
>+++ b/t/t4014-format-patch.sh
>@@ -754,7 +754,7 @@ test_expect_success 'format-patch
>--ignore-if-in-upstream HEAD' '
>   git format-patch --ignore-if-in-upstream HEAD
> '
> 
>-git_version="$(git --version | sed "s/.* //")"
>+git_version="$(git --version | sed "s/git version //")"
> 
> signature() {
>   printf "%s\n%s\n\n" "-- " "${1:-$git_version}"




[PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER

2016-10-09 Thread Jeremy Huddleston Sequoia
Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
Signed-off-by: Jeremy Huddleston Sequoia 
CC: Josh Triplett 
CC: Junio C Hamano 
---
 t/t4014-format-patch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 8d90a6e..33f6940 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -754,7 +754,7 @@ test_expect_success 'format-patch --ignore-if-in-upstream 
HEAD' '
git format-patch --ignore-if-in-upstream HEAD
 '
 
-git_version="$(git --version | sed "s/.* //")"
+git_version="$(git --version | sed "s/git version //")"
 
 signature() {
printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
-- 
2.10.1 (Apple Git-99)



Re: git 2.10.1 test regression in t3700-add.sh

2016-10-09 Thread Jeremy Huddleston Sequoia
The issue is that the whitespace before the filename in $(git ls-files -s "$2") 
is a tab, and test_mode_in_index only looks for a space.

><

> On Oct 9, 2016, at 19:51, Jeremy Huddleston Sequoia 
>  wrote:
> 
> 
>> On Oct 9, 2016, at 17:15, Jeremy Huddleston Sequoia 
>>  wrote:
>> 
>> Hi Thomas,
>> 
>> I wanted to let you know that this patch of yours, which landed in git 
>> 2.10.1, introduced some test failures, seen on macOS.
>> 
>> Let me know if you need any additional information to track these down.
>> 
>> Thanks,
>> Jeremy
>> 
>> not ok 40 - git add --chmod=[+-]x changes index with already added file
>> #
>> #echo foo >foo3 &&
>> #git add foo3 &&
>> #git add --chmod=+x foo3 &&
>> #test_mode_in_index 100755 foo3 &&
>> #echo foo >xfoo3 &&
>> #chmod 755 xfoo3 &&
>> #git add xfoo3 &&
>> #git add --chmod=-x xfoo3 &&
>> #test_mode_in_index 100644 xfoo3
>> #
>> 
>> commit 610d55af0f082f6b866dc858e144c03d8ed4424c
>> Author: Thomas Gummerer 
>> Date:   Wed Sep 14 22:07:47 2016 +0100
>> 
>>   add: modify already added files when --chmod is given
>> 
>>   When the chmod option was added to git add, it was hooked up to the diff
>>   machinery, meaning that it only works when the version in the index
>>   differs from the version on disk.
>> 
>>   As the option was supposed to mirror the chmod option in update-index,
>>   which always changes the mode in the index, regardless of the status of
>>   the file, make sure the option behaves the same way in git add.
>> 
>>   Signed-off-by: Thomas Gummerer 
>>   Signed-off-by: Junio C Hamano 
> 
> 
> This failure looks odd.  I'll dig into it a bit more as it looks like 
> something odd is going on here...
> 
> expecting success: 
>   echo foo >foo3 &&
>   git add foo3 &&
>   git add --chmod=+x foo3 &&
>   test_mode_in_index 100755 foo3 &&
>   echo foo >xfoo3 &&
>   chmod 755 xfoo3 &&
>   git add xfoo3 &&
>   git add --chmod=-x xfoo3 &&
>   test_mode_in_index 100644 xfoo3
> 
> pass
> cannot chmod 'xfoo3'fail
> 12 c5c4ca97a3a080c32920941b665e94a997901491 0 xfoo3
> not ok 40 - git add --chmod=[+-]x changes index with already added file
> # 
> # echo foo >foo3 &&
> # git add foo3 &&
> # git add --chmod=+x foo3 &&
> # test_mode_in_index 100755 foo3 &&
> # echo foo >xfoo3 &&
> # chmod 755 xfoo3 &&
> # git add xfoo3 &&
> # git add --chmod=-x xfoo3 &&
> # test_mode_in_index 100644 xfoo3
> # 
> 



smime.p7s
Description: S/MIME cryptographic signature


Re: git 2.10.1 test regression in t3700-add.sh

2016-10-09 Thread Jeremy Huddleston Sequoia

> On Oct 9, 2016, at 20:22, Jeremy Huddleston Sequoia 
>  wrote:
> 
> The issue is that the whitespace before the filename in $(git ls-files -s 
> "$2") is a tab, and test_mode_in_index only looks for a space.

Actually, looks like that as just a rabbit hole.  The real issue looks to be 
because an earlier test drops down xfoo3 as a symlink, which causes this test 
to fail due to the collision.  I'll get out a patch in a bit.

> 
>> <
> 
>> On Oct 9, 2016, at 19:51, Jeremy Huddleston Sequoia 
>>  wrote:
>> 
>> 
>>> On Oct 9, 2016, at 17:15, Jeremy Huddleston Sequoia 
>>>  wrote:
>>> 
>>> Hi Thomas,
>>> 
>>> I wanted to let you know that this patch of yours, which landed in git 
>>> 2.10.1, introduced some test failures, seen on macOS.
>>> 
>>> Let me know if you need any additional information to track these down.
>>> 
>>> Thanks,
>>> Jeremy
>>> 
>>> not ok 40 - git add --chmod=[+-]x changes index with already added file
>>> #   
>>> #   echo foo >foo3 &&
>>> #   git add foo3 &&
>>> #   git add --chmod=+x foo3 &&
>>> #   test_mode_in_index 100755 foo3 &&
>>> #   echo foo >xfoo3 &&
>>> #   chmod 755 xfoo3 &&
>>> #   git add xfoo3 &&
>>> #   git add --chmod=-x xfoo3 &&
>>> #   test_mode_in_index 100644 xfoo3
>>> #   
>>> 
>>> commit 610d55af0f082f6b866dc858e144c03d8ed4424c
>>> Author: Thomas Gummerer 
>>> Date:   Wed Sep 14 22:07:47 2016 +0100
>>> 
>>>  add: modify already added files when --chmod is given
>>> 
>>>  When the chmod option was added to git add, it was hooked up to the diff
>>>  machinery, meaning that it only works when the version in the index
>>>  differs from the version on disk.
>>> 
>>>  As the option was supposed to mirror the chmod option in update-index,
>>>  which always changes the mode in the index, regardless of the status of
>>>  the file, make sure the option behaves the same way in git add.
>>> 
>>>  Signed-off-by: Thomas Gummerer 
>>>  Signed-off-by: Junio C Hamano 
>> 
>> 
>> This failure looks odd.  I'll dig into it a bit more as it looks like 
>> something odd is going on here...
>> 
>> expecting success: 
>>  echo foo >foo3 &&
>>  git add foo3 &&
>>  git add --chmod=+x foo3 &&
>>  test_mode_in_index 100755 foo3 &&
>>  echo foo >xfoo3 &&
>>  chmod 755 xfoo3 &&
>>  git add xfoo3 &&
>>  git add --chmod=-x xfoo3 &&
>>  test_mode_in_index 100644 xfoo3
>> 
>> pass
>> cannot chmod 'xfoo3'fail
>> 12 c5c4ca97a3a080c32920941b665e94a997901491 0xfoo3
>> not ok 40 - git add --chmod=[+-]x changes index with already added file
>> #
>> #echo foo >foo3 &&
>> #git add foo3 &&
>> #git add --chmod=+x foo3 &&
>> #test_mode_in_index 100755 foo3 &&
>> #echo foo >xfoo3 &&
>> #chmod 755 xfoo3 &&
>> #git add xfoo3 &&
>> #git add --chmod=-x xfoo3 &&
>> #test_mode_in_index 100644 xfoo3
>> #
>> 
> 



smime.p7s
Description: S/MIME cryptographic signature


[PATCH] t3700-add.sh: Avoid filename collission between tests which could lead to test failure

2016-10-09 Thread Jeremy Huddleston Sequoia
Regressed-in: 610d55af0f082f6b866dc858e144c03d8ed4424c
Signed-off-by: Jeremy Huddleston Sequoia 
CC: Thomas Gummerer 
CC: Junio C Hamano 
---
 t/t3700-add.sh | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 924a266..3ccb19b 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -354,11 +354,11 @@ test_expect_success 'git add --chmod=[+-]x changes index 
with already added file
git add foo3 &&
git add --chmod=+x foo3 &&
test_mode_in_index 100755 foo3 &&
-   echo foo >xfoo3 &&
-   chmod 755 xfoo3 &&
-   git add xfoo3 &&
-   git add --chmod=-x xfoo3 &&
-   test_mode_in_index 100644 xfoo3
+   echo foo >xfoo4 &&
+   chmod 755 xfoo4 &&
+   git add xfoo4 &&
+   git add --chmod=-x xfoo4 &&
+   test_mode_in_index 100644 xfoo4
 '
 
 test_expect_success POSIXPERM 'git add --chmod=[+-]x does not change the 
working tree' '
-- 
2.10.1



Re: [PATCH] t4014-format-patch: Adjust git_version regex to better handle distro changes to DEF_VER

2016-10-09 Thread Jeremy Huddleston Sequoia

> On Oct 9, 2016, at 20:04, Josh Triplett  wrote:
> 
> On October 9, 2016 7:53:23 PM PDT, Jeremy Huddleston Sequoia 
>  wrote:
>> Regressed-in: 480871e09ed2e5275b4ba16b278681e5a8c122ae
>> Signed-off-by: Jeremy Huddleston Sequoia 
>> CC: Josh Triplett 
>> CC: Junio C Hamano 
> 
> Looks reasonable to me. Didn't realize git versions could have spaces.
> Reviewed-by: Josh Triplett 

Thanks, Josh.

If anyone feels strongly that they shouldn't, I'd be happy to change our 
DEF_VER patch to play nicer.

cf 
https://github.com/jeremyhu/git/commit/f99905d0752d923e5ec61e14c675a300c6d04284

> 
>> t/t4014-format-patch.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
>> index 8d90a6e..33f6940 100755
>> --- a/t/t4014-format-patch.sh
>> +++ b/t/t4014-format-patch.sh
>> @@ -754,7 +754,7 @@ test_expect_success 'format-patch
>> --ignore-if-in-upstream HEAD' '
>>  git format-patch --ignore-if-in-upstream HEAD
>> '
>> 
>> -git_version="$(git --version | sed "s/.* //")"
>> +git_version="$(git --version | sed "s/git version //")"
>> 
>> signature() {
>>  printf "%s\n%s\n\n" "-- " "${1:-$git_version}"
> 
> 



smime.p7s
Description: S/MIME cryptographic signature


Re: Problem with submodules

2016-10-09 Thread Stefan Beller
Adding the list back on.

On Sun, Oct 9, 2016 at 1:56 PM, Dennis Kaarsemaker
 wrote:
> On Sun, 2016-10-09 at 21:15 +0200, ven...@gmail.com wrote:
>> Sure, http://pastebin.com/bUFBDj0Q
>
> So you actually cloned from a path ending in epihany/, not epiphany.
> Turns out the trainling slash matters when using relative urls for
> submodules:
>
> $ cat test.sh
> url=http://remote.example/repo
>
> for url in $url "$url/"; do
> echo "Remote: $url"
> rm -rf main
> git init -q main
> (
> cd main
> git remote add origin $url
> git init -q sub
> git -C sub commit --allow-empty -mtest
> printf '[submodule "sub"]\n\tpath = sub\n\t\turl = ../sub\n' > 
> .gitmodules
> git add sub .gitmodules
> git commit -mtest
> git submodule init
> ) >/dev/null
> done
>
> $ sh test.sh
> Remote: http://remote.example/repo
> Submodule 'sub' (http://remote.example/sub) registered for path 'sub'
> Remote: http://remote.example/repo/
> Submodule 'sub' (http://remote.example/repo/sub) registered for path 'sub'
>
> I don't know whether this is a bug or a feature. I find using relative
> paths for submodules a pretty dodgy idea anyway and would fix up the
> .gitmodules file.

I disagree here, IMHO relative path/urls are better than absolute URLs as
it allows to hand over a project to a different organisation that wants to have
its own fork including submodule changes just easily (no need to muck around
the submodule config, "it just works" ;)

>
> Stefan, is it possible that this is a regression in the C rewrite?

Totally possible!

Thanks for the regression test, I'll dive into the code tomorrow.

Thanks,
Stefan

>
> D.
>