Re: Bug? git worktree fails with master on bare repo
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
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
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
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
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
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?
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
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
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
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
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 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
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 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
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
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?
> -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
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
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
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
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?
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
Sure, http://pastebin.com/bUFBDj0Q
Re: Problem with submodules
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
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?
> -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
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
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
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
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
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
> 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
> 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
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
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
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
> 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
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
> 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
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. >