financní

[Lucas Brown]

Jsem Lucas Brown, jsem financní dustojník, dávám pujcky jednotlivcum a firme 
pro obchodní a osobní úcely, kontaktujte me, pokud potrebujete jakýkoli druh 
pujcky. Poskytuji pujcky široké verejnosti s úrokovou sazbou 2%.

Wir bieten jedem ein GÜNSTIGES Darlehnen zu TOP Konditionen an

[Martin Kelly]

Sehr geehrte Damen und Herren,

Sie brauchen Geld? Sie sind auf der suche nach einem Darlehnen? Seriös und 
unkompliziert?
Dann sind Sie hier bei uns genau richtig.
Durch unsere jahrelange Erfahrung und kompetente Beratung sind wir Europaweit 
tätig.

Wir bieten jedem ein GÜNSTIGES Darlehnen zu TOP Konditionen an. 
Darlehnen zwischen 5000 CHF/Euro bis zu 20 Millionen CHF/Euro möglich.
Wir erheben dazu 2% Zinssatz.

Lassen Sie sich von unserem kompetenten Team beraten. 

Zögern Sie nicht und kontaktieren Sie mich unter für weitere Infos & Anfragen 
unter
der eingeblendeten Email Adresse: 
Ich freue mich von Ihnen zu hören.

Re: [PATCH v1 1/4] fastindex: speed up index load through parallelization

[Junio C Hamano]

Ben Peart  writes:

> OK.  I'll call this new extension "EOIE" ("end of index
> entries"). Other than the standard header/footer, it will only contain
> a 32 bit offset to the beginning of the extension entries.  I'll
> always write out that extension unless you would prefer it be behind a
> setting (if so, what do you want it called and what should the default
> be)?  I won't add support in update-index for this extension.

To make it robust, if I were doing this, I would at least add a checksum
of some sort.  

As each extension section consists of 4-byte extension type, 4-byte
size, followed by that many bytes of the "meat" of the section, what
I had in mind when I suggested this backpointer was something like:

"EOIE" <32-bit size> <32-bit offset> <20-byte hash>

where the size of the extension section is obviously 24-byte to
cover the offset plus hash, and the hash is computed over extension
types and their sizes (but not their contents---this is not about
protecting against file corruption and not worth wasting the cycles
for hashing) for all the extension sections this index file has
(except for "EOIE" at the end, for obvious reasons).  E.g. if we
have "TREE" extension that is N-bytes long, "REUC" extension that is
M-bytes long, followed by "EOIE", then the hash would be

SHA-1("TREE" +  +
  "REUC" + )

Then the reader would

 - Seek back 32-byte from the trailer to ensure it sees "EOIE"
   followed by a correct size (24?)

 - Jump to the offset and find 4-bytes that presumably is the type
   of the first extension, followed by its size.  

 - Feed these 8-bytes to the hash, skip that section based on its
   size (while making sure we won't run off the end of the file,
   which is a sign that we thought EOIE exists when there wasn't).
   Repeat this until we hit where we found "EOIE" (or we notice our
   mistake by overrunning it).

 - Check the hash to make sure we got it right.

> Since the goal was to find a way to load the IEOT extension before the
> cache entries, I'll also refactor the extension reading loop into a
> function that takes a function pointer and add a
> preread_index_extension() function that can be passed in when that
> loop is run before the cache entries are loaded.  When the loop is run
> again after the cache entries are loaded, it will pass in the existing
> read_index_extension() function.  Extensions can then choose which
> function they want to be loaded in.
>
> The code to read/write/use the IEOT to parallelize the cache entry
> loading will stay behind a config setting that defaults to false (at
> least for now).  I'll stick with "core.fastindex" until someone can
> (please) propose a better name.

Sounds good.

Re: [PATCH v2 2/2] diff: --ignore-cr-at-eol

[Junio C Hamano]

Johannes Schindelin  writes:

I notice that I left a few things unanswered even after giving
answers to the most important part (i.e. "what is this for was
sold incorrectly").  Here are the leftover bits.

>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index 89cc0f48de..aa2c0ff74d 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -519,6 +519,9 @@ endif::git-format-patch[]
>>  --text::
>>  Treat all files as text.
>>  
>> +--ignore-cr-at-eol::
>> +Ignore carrige-return at the end of line when doing a comparison.
>
> I am not a native speaker, either, yet I have the impression that "do a
> comparison" may be more colloquial than not. Also, it is a carriage-return
> (as in Sinatra's famous song about Love and Marriage) not a carrige-return.
>
> How about "Hide changed line endings"?

That felt like a good suggestion when I saw your reaction,
especially with only the parts visible in the patch and its context,
but after reviewing descriptions of other --ignore-* options, I no
longer think so.  The existing description of "--ignore-*" matches
manpages of GNU diff and they do not say "hide", either.

>> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
>> index 04d7b32e4e..b2cbcc818f 100644
>> --- a/xdiff/xutils.c
>> +++ b/xdiff/xutils.c
>> @@ -156,6 +156,24 @@ int xdl_blankline(const char *line, long size, long 
>> flags)
>>  return (i == size);
>>  }
>>  
>> +/*
>> + * Have we eaten everything on the line, except for an optional
>> + * CR at the very end?
>> + */
>> +static int ends_with_optional_cr(const char *l, long s, long i)
>> +{
>> +int complete = s && l[s-1] == '\n';
>> +
>> +if (complete)
>> +s--;
>> +if (s == i)
>> +return 1;
>
> What is the role of `s`, what of `i`? Maybe `length` and `current_offset`?

I'd agree with that sentiment if this file were not borrowed code
but our own, but after looking at xdiff/ code, I think the names of
these variables follow the convention used there better, which
consistently names the variable for lines they deal with l1, l2, etc.,
their sizes s1, s2, etc., and the indices into the line i1, i2, etc.

>> +/* do not ignore CR at the end of an incomplete line */
>> +if (complete && s == i + 1 && l[i] == '\r')
>> +return 1;
>
> This made me scratch my head: too many negations. The comment may better
> read "ignore CR only at the end of a complete line".

Perhaps.  "incomplete line" is a term with a specific definition
(and I think by "complete line" you mean a line that is not an
incomplete line), so I do not see the above comment as having too
many negations, though.  If you feel strongly about it, you could
"fix" it with a follow-up patch.

>> @@ -204,6 +223,14 @@ int xdl_recmatch(const char *l1, long s1, const char 
>> *l2, long s2, long flags)
>>  i1++;
>>  i2++;
>>  }
>> +} else if (flags & XDF_IGNORE_CR_AT_EOL) {
>> +/* Find the first difference and see how the line ends */
>> +while (i1 < s1 && i2 < s2 && l1[i1] == l2[i2]) {
>> +i1++;
>> +i2++;
>> +}
>> +return (ends_with_optional_cr(l1, s1, i1) &&
>> +ends_with_optional_cr(l2, s2, i2));
>
> There are extra parentheses around the `return` expression.

Yes, everybody knows that return is not a function that needs a
parentheses around its parameter.  I would drop them if this
expression were not split into two lines, but because the expression
is split at &&, I think it reads better with the extra parens.  So
I'll leave them as-is.

Thanks.

Re: [PATCH v1 1/4] fastindex: speed up index load through parallelization

[Ben Peart]

On 11/14/2017 8:12 PM, Junio C Hamano wrote:

Ben Peart  writes:


I have no thoughts or plans for changes in the future of IEOT (which I
plan to rename ITOC).  At this point in time, I can't even imagine
what else we'd want as the index only contains cache entries, ...


Yeah, but the thing is that this is open source, and the imagination
of the originator of the initial idea does not limit how the code
and data structure evolves.

Back when I added the index extensions to the system, I didn't have
perfect foresight, and I didn't have specific plans to add anything
beyond "TREE" to optimize "write-tree" and "diff-index --cached".

In hindsight, I got one thing right and one thing wrong.

Even though I didn't have any plan to add a mandatory extension, I
made the code to ignore optional ones and error out on mandatory
ones if an index extension section is not understood.  It turns out
that later (in fact much later---the "TREE" extension dates back to
April 2006, while "link" came in June 2014) we could add the
split-index mode without having to worry about older versions of Git
doing random wrong things when they see this new extension, thanks
to that design decision.  That went well.

On the other hand, I didn't think things through to realize that
some operations may want to peek only the extensions without ever
using the main table, that some other operations may want to read
some extensions first before reading the main table, or more
importantly, that these limitations would start mattering once Git
becomes popular enough and starts housing a project tree with very
many paths in the main table.

I really wish somebody had brought it up as a potential issue during
the review---I would have defined the extended index format to have
the simple extension at the end that points back to the tail end of
the main table back then, and we wouldn't be having this back and
forth now.  But I was just too happy and excited that I have found a
room to squeeze extension sections into the index file format
without breaking existing implementations of Git (which checked the
trailer checksum matches to the result of hashing the whole thing,
and read the recorded number of entries from the main table, without
even noticing that there is a gap in between), and that excitement
blinded me.


I understand the risk but the list of offsets into the cache entries
is pretty simple as well. I prefer the simplicity of a single TOC
extension that gives us random access to the entire index rather than
having to piece one together using multiple extensions.  That model
has its own set of risks and tradeoffs.


I thought that you are not using the "where does the series of
extensions begin" information in the first place, no?  That piece of
information is useful independent of the usefulness of "index into
the main table to list entries where the prefix-compression is
reset".  So if anything, I'd prefer the simplicity of introducing
that "where does the series of extensions begin" that does not say
anything else, and build on other things like ITOC as mere users of
the mechanism.



OK.  I'll call this new extension "EOIE" ("end of index entries"). Other 
than the standard header/footer, it will only contain a 32 bit offset to 
the beginning of the extension entries.  I'll always write out that 
extension unless you would prefer it be behind a setting (if so, what do 
you want it called and what should the default be)?  I won't add support 
in update-index for this extension.


Since the goal was to find a way to load the IEOT extension before the 
cache entries, I'll also refactor the extension reading loop into a 
function that takes a function pointer and add a 
preread_index_extension() function that can be passed in when that loop 
is run before the cache entries are loaded.  When the loop is run again 
after the cache entries are loaded, it will pass in the existing 
read_index_extension() function.  Extensions can then choose which 
function they want to be loaded in.


The code to read/write/use the IEOT to parallelize the cache entry 
loading will stay behind a config setting that defaults to false (at 
least for now).  I'll stick with "core.fastindex" until someone can 
(please) propose a better name.

Re: [PATCHv4 7/7] builtin/describe.c: describe a blob

[Jonathan Tan]

On Tue, 14 Nov 2017 16:30:43 -0800
Stefan Beller  wrote:

> The walking is performed in reverse order to show the introduction of a
> blob rather than its last occurrence.

The code as implemented here does not do this - it instead shows the last
occurrence.

>  NAME
>  
> -git-describe - Describe a commit using the most recent tag reachable from it
> +git-describe - Describe a commit or blob using the graph relations

I would write "Describe a commit or blob using a tag reachable from it".

> +If the given object refers to a blob, it will be described
> +as `:`, such that the blob can be found
> +at `` in the ``. Note, that the commit is likely
> +not the commit that introduced the blob, but the one that was found
> +first; to find the commit that introduced the blob, you need to find
> +the commit that last touched the path, e.g.
> +`git log  -- `.
> +As blobs do not point at the commits they are contained in,
> +describing blobs is slow as we have to walk the whole graph.

I think some of this needs to be updated?

> +static void process_object(struct object *obj, const char *path, void *data)
> +{
> + struct process_commit_data *pcd = data;
> +
> + if (!oidcmp(>looking_for, >oid) && !pcd->dst->len) {
> + reset_revision_walk();
> + describe_commit(>current_commit, pcd->dst);
> + strbuf_addf(pcd->dst, ":%s", path);
> + pcd->revs->max_count = 0;
> + }
> +}

Setting max_count to 0 does not work when reverse is used, because the
commits are first buffered into revs->commits (see get_revision() in
revision.c). There doesn't seem to be a convenient way to terminate the
traversal immediately - I think setting revs->commits to NULL should
work (but I didn't check). Remember to free revs->commits (using
free_commit_list) first.

> +test_expect_success 'describe a blob at a tag' '
> + echo "make it a unique blob" >file &&
> + git add file && git commit -m "content in file" &&
> + git tag -a -m "latest annotated tag" unique-file &&
> + git describe HEAD:file >actual &&
> + echo "unique-file:file" >expect &&
> + test_cmp expect actual
> +'

This is probably better named "describe a blob at a directly tagged
commit". (Should we also test the case where a blob is directly
tagged?)

> +test_expect_success 'describe a blob with its last introduction' '
> + git commit --allow-empty -m "empty commit" &&
> + git rm file &&
> + git commit -m "delete blob" &&
> + git revert HEAD &&
> + git commit --allow-empty -m "empty commit" &&
> + git describe HEAD:file >actual &&
> + grep unique-file-3-g actual
> +'

The description is not true: firstly, this shows the last occurrence,
not the last introduction (you can verify this by adding another commit
and noticing that the contents of "actual" changes), and what we want is
not the last introduction anyway, but the first one.

Re: [PATCHv4 6/7] builtin/describe.c: factor out describe_commit

[Jonathan Tan]

On Tue, 14 Nov 2017 16:30:42 -0800
Stefan Beller  wrote:

> In the next patch we'll learn how to describe more than just commits,
> so factor out describing commits into its own function.  That will make
> the next patches easy as we still need to describe a commit as part of
> describing blobs.
> 
> While factoring out the functionality to describe_commit, make sure
> that any output to stdout is put into a strbuf, which we can print
> afterwards, using puts which also adds the line ending.

I think the sentences here are a bit jumbled. The aim is to make the
next patch easier (your 2nd sentence), and how we accomplish that is by
factoring out the description of commits (1st sentence) *and* by
outputting to a strbuf because we need to postprocess the output further
when describing a commit as part of describing a blob (3rd sentence).

Re: [PATCHv4 5/7] builtin/describe.c: print debug statements earlier

[Jonathan Tan]

On Tue, 14 Nov 2017 16:30:41 -0800
Stefan Beller  wrote:

> For debuggers aid we'd want to print debug statements early, so
> introduce a new line in the debug output that describes the whole
> function, and then change the next debug output to describe why we
> need to search. Conveniently drop the arg from the second line;
> which will be useful in a follow up commit, that refactors the
> describe function.
> 
> This re-arrangement of debug printing is solely done for a better
> refactoring in the next commit, not to aid debugging git-describe,
> which is expected to have the same information readily available
> with the new prints.

This paragraph ("not to aid debugging") contradicts the previous one
("For debuggers aid").

Looking at this patch and the subsequent patches, I would write the
commit message like this:

When debugging, print the received argument at the start of the
function instead of in the middle. This ensures that the received
argument is printed in all code paths, and also allows a subsequent
refactoring to not need to move the "arg" parameter.

Also change the title to "print arg earlier when debugging" or something
like that.

Re: [PATCHv4 3/7] revision.h: introduce blob/tree walking in order of the commits

[Jonathan Tan]

On Tue, 14 Nov 2017 16:30:39 -0800
Stefan Beller  wrote:

> The change in list-objects.c is rather minimal as we'll be re-using
> the infrastructure put in place of the revision walking machinery. For
> example one could expect that add_pending_tree is not called, but rather
> commit->tree is directly passed to the tree traversal function. This
> however requires a lot more code than just emptying the queue containing
> trees after each commit.

Ah, I see. You're calling add_pending_tree(), then
traverse_trees_and_blobs() will immediately flush the list of pending
trees.

I'm not sure that passing commit->tree directly to the tree traversal
function will require a lot more code, but even if it does, you should
at least add a NEEDSWORK - currently, flushing the list of pending trees
frees the array containing the list of pending trees, so each invocation
will need to reallocate a new array.

Re: [PATCH 15/30] merge-recursive: Move the get_renames() function

[Junio C Hamano]

Elijah Newren  writes:

> Eek!  My apologies.  I will go through and fix them up.  I see no
> reference to checkpatch.pl in git, but a google search shows there's
> one in the linux source tree.  Is that were I get it from, or is there
> a different one?



> Also, would you like me to make a separate commit that cleans up
> pre-existing issues in merge-recursive.c so that it runs clean, or
> just remove the problems I added?

That's optional.  These three patch series are already sufficiently
large, so I do not mind a clean-up after dust settles down, instead
of preliminary clean-up.

[PATCHv4 6/7] builtin/describe.c: factor out describe_commit

[Stefan Beller]

In the next patch we'll learn how to describe more than just commits,
so factor out describing commits into its own function.  That will make
the next patches easy as we still need to describe a commit as part of
describing blobs.

While factoring out the functionality to describe_commit, make sure
that any output to stdout is put into a strbuf, which we can print
afterwards, using puts which also adds the line ending.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/describe.c | 63 --
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 3136efde31..9e9a5ed5d4 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -256,7 +256,7 @@ static unsigned long finish_depth_computation(
return seen_commits;
 }
 
-static void display_name(struct commit_name *n)
+static void append_name(struct commit_name *n, struct strbuf *dst)
 {
if (n->prio == 2 && !n->tag) {
n->tag = lookup_tag(>oid);
@@ -272,19 +272,18 @@ static void display_name(struct commit_name *n)
}
 
if (n->tag)
-   printf("%s", n->tag->tag);
+   strbuf_addstr(dst, n->tag->tag);
else
-   printf("%s", n->path);
+   strbuf_addstr(dst, n->path);
 }
 
-static void show_suffix(int depth, const struct object_id *oid)
+static void append_suffix(int depth, const struct object_id *oid, struct 
strbuf *dst)
 {
-   printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
+   strbuf_addf(dst, "-%d-g%s", depth, find_unique_abbrev(oid->hash, 
abbrev));
 }
 
-static void describe(const char *arg, int last_one)
+static void describe_commit(struct object_id *oid, struct strbuf *dst)
 {
-   struct object_id oid;
struct commit *cmit, *gave_up_on = NULL;
struct commit_list *list;
struct commit_name *n;
@@ -293,26 +292,18 @@ static void describe(const char *arg, int last_one)
unsigned long seen_commits = 0;
unsigned int unannotated_cnt = 0;
 
-   if (debug)
-   fprintf(stderr, _("describe %s\n"), arg);
-
-   if (get_oid(arg, ))
-   die(_("Not a valid object name %s"), arg);
-   cmit = lookup_commit_reference();
-   if (!cmit)
-   die(_("%s is not a valid '%s' object"), arg, commit_type);
+   cmit = lookup_commit_reference(oid);
 
n = find_commit_name(>object.oid);
if (n && (tags || all || n->prio == 2)) {
/*
 * Exact match to an existing ref.
 */
-   display_name(n);
+   append_name(n, dst);
if (longformat)
-   show_suffix(0, n->tag ? >tag->tagged->oid : );
+   append_suffix(0, n->tag ? >tag->tagged->oid : oid, 
dst);
if (suffix)
-   printf("%s", suffix);
-   printf("\n");
+   strbuf_addstr(dst, suffix);
return;
}
 
@@ -386,10 +377,9 @@ static void describe(const char *arg, int last_one)
if (!match_cnt) {
struct object_id *cmit_oid = >object.oid;
if (always) {
-   printf("%s", find_unique_abbrev(cmit_oid->hash, 
abbrev));
+   strbuf_addstr(dst, find_unique_abbrev(cmit_oid->hash, 
abbrev));
if (suffix)
-   printf("%s", suffix);
-   printf("\n");
+   strbuf_addstr(dst, suffix);
return;
}
if (unannotated_cnt)
@@ -437,15 +427,36 @@ static void describe(const char *arg, int last_one)
}
}
 
-   display_name(all_matches[0].name);
+   append_name(all_matches[0].name, dst);
if (abbrev)
-   show_suffix(all_matches[0].depth, >object.oid);
+   append_suffix(all_matches[0].depth, >object.oid, dst);
if (suffix)
-   printf("%s", suffix);
-   printf("\n");
+   strbuf_addstr(dst, suffix);
+}
+
+static void describe(const char *arg, int last_one)
+{
+   struct object_id oid;
+   struct commit *cmit;
+   struct strbuf sb = STRBUF_INIT;
+
+   if (debug)
+   fprintf(stderr, _("describe %s\n"), arg);
+
+   if (get_oid(arg, ))
+   die(_("Not a valid object name %s"), arg);
+   cmit = lookup_commit_reference();
+   if (!cmit)
+   die(_("%s is not a valid '%s' object"), arg, commit_type);
+
+   describe_commit(, );
+
+   puts(sb.buf);
 
if (!last_one)
clear_commit_marks(cmit, -1);
+
+   strbuf_release();
 }
 
 int cmd_describe(int argc, const char **argv, const char *prefix)
-- 
2.15.0.128.gcadd42da22

[PATCHv4 7/7] builtin/describe.c: describe a blob

[Stefan Beller]

Sometimes users are given a hash of an object and they want to
identify it further (ex.: Use verify-pack to find the largest blobs,
but what are these? or [1])

When describing commits, we try to anchor them to tags or refs, as these
are conceptually on a higher level than the commit. And if there is no ref
or tag that matches exactly, we're out of luck.  So we employ a heuristic
to make up a name for the commit. These names are ambiguous, there might
be different tags or refs to anchor to, and there might be different
path in the DAG to travel to arrive at the commit precisely.

When describing a blob, we want to describe the blob from a higher layer
as well, which is a tuple of (commit, deep/path) as the tree objects
involved are rather uninteresting.  The same blob can be referenced by
multiple commits, so how we decide which commit to use?  This patch
implements a rather naive approach on this: As there are no back pointers
from blobs to commits in which the blob occurs, we'll start walking from
any tips available, listing the blobs in-order of the commit and once we
found the blob, we'll take the first commit that listed the blob. For
example

  git describe --tags v0.99:Makefile
  conversion-901-g7672db20c2:Makefile

tells us the Makefile as it was in v0.99 was introduced in commit 7672db20.

The walking is performed in reverse order to show the introduction of a
blob rather than its last occurrence.

[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob

Signed-off-by: Stefan Beller 
---
 Documentation/git-describe.txt | 13 -
 builtin/describe.c | 66 ++
 t/t6120-describe.sh| 19 
 3 files changed, 92 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index c924c945ba..a25443ca91 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -3,7 +3,7 @@ git-describe(1)
 
 NAME
 
-git-describe - Describe a commit using the most recent tag reachable from it
+git-describe - Describe a commit or blob using the graph relations
 
 
 SYNOPSIS
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git describe' [--all] [--tags] [--contains] [--abbrev=] [...]
 'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]
+'git describe' 
 
 DESCRIPTION
 ---
@@ -24,6 +25,16 @@ By default (without --all or --tags) `git describe` only 
shows
 annotated tags.  For more information about creating annotated tags
 see the -a and -s options to linkgit:git-tag[1].
 
+If the given object refers to a blob, it will be described
+as `:`, such that the blob can be found
+at `` in the ``. Note, that the commit is likely
+not the commit that introduced the blob, but the one that was found
+first; to find the commit that introduced the blob, you need to find
+the commit that last touched the path, e.g.
+`git log  -- `.
+As blobs do not point at the commits they are contained in,
+describing blobs is slow as we have to walk the whole graph.
+
 OPTIONS
 ---
 ...::
diff --git a/builtin/describe.c b/builtin/describe.c
index 9e9a5ed5d4..a2a5fdc48d 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -3,6 +3,7 @@
 #include "lockfile.h"
 #include "commit.h"
 #include "tag.h"
+#include "blob.h"
 #include "refs.h"
 #include "builtin.h"
 #include "exec_cmd.h"
@@ -11,8 +12,9 @@
 #include "hashmap.h"
 #include "argv-array.h"
 #include "run-command.h"
+#include "revision.h"
+#include "list-objects.h"
 
-#define SEEN   (1u << 0)
 #define MAX_TAGS   (FLAG_BITS - 1)
 
 static const char * const describe_usage[] = {
@@ -434,6 +436,57 @@ static void describe_commit(struct object_id *oid, struct 
strbuf *dst)
strbuf_addstr(dst, suffix);
 }
 
+struct process_commit_data {
+   struct object_id current_commit;
+   struct object_id looking_for;
+   struct strbuf *dst;
+   struct rev_info *revs;
+};
+
+static void process_commit(struct commit *commit, void *data)
+{
+   struct process_commit_data *pcd = data;
+   pcd->current_commit = commit->object.oid;
+}
+
+static void process_object(struct object *obj, const char *path, void *data)
+{
+   struct process_commit_data *pcd = data;
+
+   if (!oidcmp(>looking_for, >oid) && !pcd->dst->len) {
+   reset_revision_walk();
+   describe_commit(>current_commit, pcd->dst);
+   strbuf_addf(pcd->dst, ":%s", path);
+   pcd->revs->max_count = 0;
+   }
+}
+
+static void describe_blob(struct object_id oid, struct strbuf *dst)
+{
+   struct rev_info revs;
+   struct argv_array args = ARGV_ARRAY_INIT;
+   struct process_commit_data pcd = { null_oid, oid, dst, };
+
+   argv_array_pushl(, "internal: The first arg is not parsed",
+   "--all", "--reflog", /* as many starting points as possible */
+   /* NEEDSWORK: --all is incompatible with worktrees for now: 

[PATCHv4 5/7] builtin/describe.c: print debug statements earlier

[Stefan Beller]

For debuggers aid we'd want to print debug statements early, so
introduce a new line in the debug output that describes the whole
function, and then change the next debug output to describe why we
need to search. Conveniently drop the arg from the second line;
which will be useful in a follow up commit, that refactors the
describe function.

This re-arrangement of debug printing is solely done for a better
refactoring in the next commit, not to aid debugging git-describe,
which is expected to have the same information readily available
with the new prints.

Signed-off-by: Stefan Beller 
---
 builtin/describe.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index fd61f463cf..3136efde31 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)
unsigned long seen_commits = 0;
unsigned int unannotated_cnt = 0;
 
+   if (debug)
+   fprintf(stderr, _("describe %s\n"), arg);
+
if (get_oid(arg, ))
die(_("Not a valid object name %s"), arg);
cmit = lookup_commit_reference();
@@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)
if (!max_candidates)
die(_("no tag exactly matches '%s'"), 
oid_to_hex(>object.oid));
if (debug)
-   fprintf(stderr, _("searching to describe %s\n"), arg);
+   fprintf(stderr, _("No exact match on refs or tags, searching to 
describe\n"));
 
if (!have_util) {
struct hashmap_iter iter;
-- 
2.15.0.128.gcadd42da22

[PATCHv4 2/7] list-objects.c: factor out traverse_trees_and_blobs

[Stefan Beller]

With traverse_trees_and_blobs factored out of the main traverse function,
the next patch can introduce an in-order revision walking with ease.

In the next patch we'll call `traverse_trees_and_blobs` from within the
loop walking the commits, such that we'll have one invocation of that
function per commit.  That is why we do not want to have memory allocations
in that function, such as we'd have if we were to use a strbuf locally.
Pass a strbuf from traverse_commit_list into the blob and tree traversing
function as a scratch pad that only needs to be allocated once.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 list-objects.c | 50 +++---
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/list-objects.c b/list-objects.c
index b3931fa434..7c2ce9c4bd 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -183,25 +183,15 @@ static void add_pending_tree(struct rev_info *revs, 
struct tree *tree)
add_pending_object(revs, >object, "");
 }
 
-void traverse_commit_list(struct rev_info *revs,
- show_commit_fn show_commit,
- show_object_fn show_object,
- void *data)
+static void traverse_trees_and_blobs(struct rev_info *revs,
+struct strbuf *base,
+show_object_fn show_object,
+void *data)
 {
int i;
-   struct commit *commit;
-   struct strbuf base;
 
-   strbuf_init(, PATH_MAX);
-   while ((commit = get_revision(revs)) != NULL) {
-   /*
-* an uninteresting boundary commit may not have its tree
-* parsed yet, but we are not going to show them anyway
-*/
-   if (commit->tree)
-   add_pending_tree(revs, commit->tree);
-   show_commit(commit, data);
-   }
+   assert(base->len == 0);
+
for (i = 0; i < revs->pending.nr; i++) {
struct object_array_entry *pending = revs->pending.objects + i;
struct object *obj = pending->item;
@@ -218,17 +208,39 @@ void traverse_commit_list(struct rev_info *revs,
path = "";
if (obj->type == OBJ_TREE) {
process_tree(revs, (struct tree *)obj, show_object,
-, path, data);
+base, path, data);
continue;
}
if (obj->type == OBJ_BLOB) {
process_blob(revs, (struct blob *)obj, show_object,
-, path, data);
+base, path, data);
continue;
}
die("unknown pending object %s (%s)",
oid_to_hex(>oid), name);
}
object_array_clear(>pending);
-   strbuf_release();
+}
+
+void traverse_commit_list(struct rev_info *revs,
+ show_commit_fn show_commit,
+ show_object_fn show_object,
+ void *data)
+{
+   struct commit *commit;
+   struct strbuf csp; /* callee's scratch pad */
+   strbuf_init(, PATH_MAX);
+
+   while ((commit = get_revision(revs)) != NULL) {
+   /*
+* an uninteresting boundary commit may not have its tree
+* parsed yet, but we are not going to show them anyway
+*/
+   if (commit->tree)
+   add_pending_tree(revs, commit->tree);
+   show_commit(commit, data);
+   }
+   traverse_trees_and_blobs(revs, , show_object, data);
+
+   strbuf_release();
 }
-- 
2.15.0.128.gcadd42da22

[PATCHv4 3/7] revision.h: introduce blob/tree walking in order of the commits

[Stefan Beller]

The functionality to list tree objects in the order they were seen
while traversing the commits will be used in one of the next commits,
where we teach `git describe` to describe not only commits, but blobs, too.

The change in list-objects.c is rather minimal as we'll be re-using
the infrastructure put in place of the revision walking machinery. For
example one could expect that add_pending_tree is not called, but rather
commit->tree is directly passed to the tree traversal function. This
however requires a lot more code than just emptying the queue containing
trees after each commit.

Signed-off-by: Stefan Beller 
---
 Documentation/rev-list-options.txt |  5 +++
 list-objects.c |  2 +
 revision.c |  2 +
 revision.h |  3 +-
 t/t6100-rev-list-in-order.sh   | 77 ++
 5 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100755 t/t6100-rev-list-in-order.sh

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 13501e1556..9066e0c777 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -686,6 +686,11 @@ ifdef::git-rev-list[]
all object IDs which I need to download if I have the commit
object _bar_ but not _foo_''.
 
+--in-commit-order::
+   Print tree and blob ids in order of the commits. The tree
+   and blob ids are printed after they are first referenced
+   by a commit.
+
 --objects-edge::
Similar to `--objects`, but also print the IDs of excluded
commits prefixed with a ``-'' character.  This is used by
diff --git a/list-objects.c b/list-objects.c
index 7c2ce9c4bd..07a92f35fe 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -239,6 +239,8 @@ void traverse_commit_list(struct rev_info *revs,
if (commit->tree)
add_pending_tree(revs, commit->tree);
show_commit(commit, data);
+   if (revs->tree_blobs_in_commit_order)
+   traverse_trees_and_blobs(revs, , show_object, data);
}
traverse_trees_and_blobs(revs, , show_object, data);
 
diff --git a/revision.c b/revision.c
index d167223e69..9329d4ebbf 100644
--- a/revision.c
+++ b/revision.c
@@ -1845,6 +1845,8 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
revs->dense = 0;
} else if (!strcmp(arg, "--show-all")) {
revs->show_all = 1;
+   } else if (!strcmp(arg, "--in-commit-order")) {
+   revs->tree_blobs_in_commit_order = 1;
} else if (!strcmp(arg, "--remove-empty")) {
revs->remove_empty_trees = 1;
} else if (!strcmp(arg, "--merges")) {
diff --git a/revision.h b/revision.h
index 54761200ad..86985d68aa 100644
--- a/revision.h
+++ b/revision.h
@@ -121,7 +121,8 @@ struct rev_info {
bisect:1,
ancestry_path:1,
first_parent_only:1,
-   line_level_traverse:1;
+   line_level_traverse:1,
+   tree_blobs_in_commit_order:1;
 
/* Diff flags */
unsigned intdiff:1,
diff --git a/t/t6100-rev-list-in-order.sh b/t/t6100-rev-list-in-order.sh
new file mode 100755
index 00..b2bb0a7f61
--- /dev/null
+++ b/t/t6100-rev-list-in-order.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+
+test_description='rev-list testing in-commit-order'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a commit history with trees, blobs' '
+   for x in one two three four
+   do
+   echo $x >$x &&
+   git add $x &&
+   git commit -m "add file $x" ||
+   return 1
+   done &&
+   for x in four three
+   do
+   git rm $x &&
+   git commit -m "remove $x" ||
+   return 1
+   done
+'
+
+test_expect_success 'rev-list --in-commit-order' '
+   git rev-list --in-commit-order --objects HEAD >actual.raw &&
+   cut -c 1-40 >actual expect.raw <<-\EOF &&
+   HEAD^{commit}
+   HEAD^{tree}
+   HEAD^{tree}:one
+   HEAD^{tree}:two
+   HEAD~1^{commit}
+   HEAD~1^{tree}
+   HEAD~1^{tree}:three
+   HEAD~2^{commit}
+   HEAD~2^{tree}
+   HEAD~2^{tree}:four
+   HEAD~3^{commit}
+   # HEAD~3^{tree} skipped, same as HEAD~1^{tree}
+   HEAD~4^{commit}
+   # HEAD~4^{tree} skipped, same as HEAD^{tree}
+   HEAD~5^{commit}
+   HEAD~5^{tree}
+   EOF
+   grep -v "#" >expect actual.raw &&
+   cut -c 1-40 >actual expect.raw <<-\EOF &&
+   HEAD^{commit}
+   HEAD~1^{commit}
+   HEAD~2^{commit}
+   HEAD~3^{commit}
+   HEAD~4^{commit}
+   

[PATCHv4 1/7] t6120: fix typo in test name

[Stefan Beller]

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 t/t6120-describe.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 1c0e8659d9..c8b7ed82d9 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -304,7 +304,7 @@ test_expect_success 'describe chokes on severely broken 
submodules' '
mv .git/modules/sub1/ .git/modules/sub_moved &&
test_must_fail git describe --dirty
 '
-test_expect_success 'describe ignoring a borken submodule' '
+test_expect_success 'describe ignoring a broken submodule' '
git describe --broken >out &&
test_when_finished "mv .git/modules/sub_moved .git/modules/sub1" &&
grep broken out
-- 
2.15.0.128.gcadd42da22

[PATCHv4 4/7] builtin/describe.c: rename `oid` to avoid variable shadowing

[Stefan Beller]

The function `describe` has already a variable named `oid` declared at
the beginning of the function for an object id.  Do not shadow that
variable with a pointer to an object id.

Signed-off-by: Stefan Beller 
Signed-off-by: Junio C Hamano 
---
 builtin/describe.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index 29075dbd0f..fd61f463cf 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -381,9 +381,9 @@ static void describe(const char *arg, int last_one)
}
 
if (!match_cnt) {
-   struct object_id *oid = >object.oid;
+   struct object_id *cmit_oid = >object.oid;
if (always) {
-   printf("%s", find_unique_abbrev(oid->hash, abbrev));
+   printf("%s", find_unique_abbrev(cmit_oid->hash, 
abbrev));
if (suffix)
printf("%s", suffix);
printf("\n");
@@ -392,11 +392,11 @@ static void describe(const char *arg, int last_one)
if (unannotated_cnt)
die(_("No annotated tags can describe '%s'.\n"
"However, there were unannotated tags: try 
--tags."),
-   oid_to_hex(oid));
+   oid_to_hex(cmit_oid));
else
die(_("No tags can describe '%s'.\n"
"Try --always, or create some tags."),
-   oid_to_hex(oid));
+   oid_to_hex(cmit_oid));
}
 
QSORT(all_matches, match_cnt, compare_pt);
-- 
2.15.0.128.gcadd42da22

[PATCHv4 0/7] describe a blob

[Stefan Beller]

* omitted the check for options, none of them made sense.
* use of --reverse
* an additional test asked for by Jonathan

previous discussion
https://public-inbox.org/git/20171102194148.2124-1-sbel...@google.com/
interdiff to current queued patches below.

Thanks,
Stefan

Stefan Beller (7):
  t6120: fix typo in test name
  list-objects.c: factor out traverse_trees_and_blobs
  revision.h: introduce blob/tree walking in order of the commits
  builtin/describe.c: rename `oid` to avoid variable shadowing
  builtin/describe.c: print debug statements earlier
  builtin/describe.c: factor out describe_commit
  builtin/describe.c: describe a blob

 Documentation/git-describe.txt |  13 +++-
 Documentation/rev-list-options.txt |   5 ++
 builtin/describe.c | 126 -
 list-objects.c |  52 +--
 revision.c |   2 +
 revision.h |   3 +-
 t/t6100-rev-list-in-order.sh   |  77 +++
 t/t6120-describe.sh|  21 ++-
 8 files changed, 249 insertions(+), 50 deletions(-)
 create mode 100755 t/t6100-rev-list-in-order.sh

-- 
2.15.0.128.gcadd42da22

diff --git c/builtin/describe.c w/builtin/describe.c
index acfd853a30..a2a5fdc48d 100644
--- c/builtin/describe.c
+++ w/builtin/describe.c
@@ -473,6 +473,7 @@ static void describe_blob(struct object_id oid, struct 
strbuf *dst)
"--single-worktree",
"--objects",
"--in-commit-order",
+   "--reverse",
NULL);
 
init_revisions(, NULL);
@@ -501,13 +502,9 @@ static void describe(const char *arg, int last_one)
 
if (cmit)
describe_commit(, );
-   else if (lookup_blob()) {
-   if (all || tags || longformat || first_parent ||
-   patterns.nr || exclude_patterns.nr ||
-   always || dirty || broken)
-   die(_("options not available for describing blobs"));
+   else if (lookup_blob())
describe_blob(oid, );
-   } else
+   else
die(_("%s is neither a commit nor blob"), arg);
 
puts(sb.buf);
diff --git c/t/t6100-rev-list-in-order.sh w/t/t6100-rev-list-in-order.sh
index d4d539b0da..b2bb0a7f61 100755
--- c/t/t6100-rev-list-in-order.sh
+++ w/t/t6100-rev-list-in-order.sh
@@ -4,7 +4,7 @@ test_description='rev-list testing in-commit-order'
 
 . ./test-lib.sh
 
-test_expect_success 'rev-list --in-commit-order' '
+test_expect_success 'setup a commit history with trees, blobs' '
for x in one two three four
do
echo $x >$x &&
@@ -17,7 +17,10 @@ test_expect_success 'rev-list --in-commit-order' '
git rm $x &&
git commit -m "remove $x" ||
return 1
-   done &&
+   done
+'
+
+test_expect_success 'rev-list --in-commit-order' '
git rev-list --in-commit-order --objects HEAD >actual.raw &&
cut -c 1-40 >actual actual.raw &&
+   cut -c 1-40 >actual expect.raw <<-\EOF &&
+   HEAD^{commit}
+   HEAD~1^{commit}
+   HEAD~2^{commit}
+   HEAD~3^{commit}
+   HEAD~4^{commit}
+   HEAD~5^{commit}
+   HEAD^{tree}
+   HEAD^{tree}:one
+   HEAD^{tree}:two
+   HEAD~1^{tree}
+   HEAD~1^{tree}:three
+   HEAD~2^{tree}
+   HEAD~2^{tree}:four
+   # HEAD~3^{tree} skipped, same as HEAD~1^{tree}
+   # HEAD~4^{tree} skipped, same as HEAD^{tree}
+   HEAD~5^{tree}
+   EOF
+   grep -v "#" >expect actual &&
-   grep unique-file-1-g actual
+   grep unique-file-3-g actual
 '
 
 test_expect_failure ULIMIT_STACK_SIZE 'name-rev works in a deep repo' '

Re: Recovering from gc errors

[Jeff King]

On Tue, Nov 14, 2017 at 10:39:31AM -0500, Marc Branchaud wrote:

> >rm -rf .git/logs
> 
> Unfortunately, removing the logs directory does not fix "git gc".  So I
> restored it.
> 
> However I did find all of the bad SHAs in the HEAD logs of four of my
> worktrees.

Ah, right. There are more logs than .git/logs these days. That makes
sense.

> I'm willing to chalk this up to bugs in the early worktree code, unless one
> of the CC'd worktree developers thinks otherwise.

Perhaps fixed by acd9544a8f (revision.c: --reflog add HEAD reflog from
all worktrees, 2017-08-23). I think before then that prune/repack would
consider those objects unreachable.

-Peff

Re: [PATCH] builtin/describe.c: describe a blob

[Stefan Beller]

On Sun, Nov 12, 2017 at 5:33 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Sometimes users are given a hash of an object and they want to
>> identify it further (ex.: Use verify-pack to find the largest blobs,
>> but what are these? or [1])
>
> Thanks for finishing the topic you started.
>
>> @@ -11,6 +11,7 @@ SYNOPSIS
>>  [verse]
>>  'git describe' [--all] [--tags] [--contains] [--abbrev=] 
>> [...]
>>  'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=]
>> +'git describe' 
>
> OK.
>
>> diff --git a/builtin/describe.c b/builtin/describe.c
>> index 9e9a5ed5d4..acfd853a30 100644
>> --- a/builtin/describe.c
>> +++ b/builtin/describe.c
>> ...
>>  static void describe(const char *arg, int last_one)
>>  {
>> ...
>> @@ -445,11 +497,18 @@ static void describe(const char *arg, int last_one)
>> ...
>> + cmit = lookup_commit_reference_gently(, 1);
>> +
>> + if (cmit)
>> + describe_commit(, );
>> + else if (lookup_blob()) {
>> + if (all || tags || longformat || first_parent ||
>> + patterns.nr || exclude_patterns.nr ||
>> + always || dirty || broken)
>> + die(_("options not available for describing blobs"));
>> + describe_blob(oid, );
>
> I am not sure if I agree with some of them.
>
>> + } else
>> + die(_("%s is neither a commit nor blob"), arg);
>
> This side I would agree with, though.
>
> The caller of the describe() function is either
>
>  * 'git describe' makes a single call to it with "HEAD" and
>exits; or
>  * 'git describe A B C...' makes one call to it for each of these
>command line arguments.
>
> And 'git describe ' code is most likely trigger from the latter,
> as it is not likely for HEAD to be pointing at a blob.
>
> $ blob=$(git rev-parse master:Makefile)
> $ git describe --always master $blob
>
> and trigger the above check.  Is the check against "always" useful,
> or is it better to simply ignore it while describing $blob, but
> still keeping it in effect while describing 'master'?
>
> The 'dirty' and 'broken' check is redundant because we would have
> already errored out if either of them is set before calling describe()
> on user-supplied object names.
>
> If I understand the way "describe " works correctly, it
> traverses the history with objects, doing a moral equivalent of
> "rev-list --objects", stops when it finds the blob object with the
> given name, and when it stops, it knows the commit object that
> contains the blob and path in that commit to the blob.  Then the
> commit is described to be a human-readable string, so that the path
> can be concatenated after it.
>
> Aren't these options that affect how a commit object is described in
> effect and useful when you do the "internal" describe of the commit
> you found the blob object in?  IOW, wouldn't this
>
> $ git describe --exclude='*-wip' $blob
>
> make sure that in its output $commit:$path, $commit is not described
> in terms of any "wip" refs?

yes, we would exclude those refs. But the name alone (without reading the docs)
suggests anything given in --exclude is excluded, so what about the blob

$commit:path-wip/test1.c

which I might want to exclude from the search using the exclude pattern?
After reading the docs, this is silly of course, and the exclusion only applies
to the commit description part.

--all is an interesting case, as we pass --all to the revision walking machinery
for blobs, but that is slightly different than the --all flag given to describe,
which also only relates to the commit name that should be produced.
So, I'll go through all the options and see which we can drop.

Thanks,
Stefan


>

Re: [PATCH 11/30] directory rename detection: testcases exploring possibly suboptimal merges

[Elijah Newren]

On Tue, Nov 14, 2017 at 12:33 PM, Stefan Beller  wrote:
> On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren  wrote:

>> +# Possible Resolutions:
>> +#   Previous git: y/{a,b,f},   z/{c,d},   x/e
>> +#   Expected: y/{a,b,e,f}, z/{c,d}
>> +#   Preferred:y/{a,b,e},   z/{c,d,f}
>
> it might be tricky in the future to know what "previous git" is;
> "Previous git" means without directory renames enabled;
>
> "expected" means we expect the algorithm presented in this series to produce
> this output, preferred is what we actually expect.

Yes, how about using:
  "Without dir rename detection:"
  "Currently expected:"
and
  "Optimal:"
?

>> +# Testcase 8b, Dual-directory rename, one into the others' way, with 
>> conflicting filenames
>> +#   Commit A. x/{a_1,b_1}, y/{a_2,b_2}
>> +#   Commit B. x/{a_1,b_1,e_1}, y/{a_2,b_2,e_2}
>> +#   Commit C. y/{a_1,b_1}, z/{a_2,b_2}
>> +#
>> +# Possible Resolutions:
>> +#   Previous git: y/{a_1,b_1,e_2}, z/{a_2,b_2}, x/e_1
>> +#   Scary:y/{a_1,b_1}, z/{a_2,b_2}, CONFLICT(add/add, e_1 vs. 
>> e_2)
>> +#   Preferred:y/{a_1,b_1,e_1}, z/{a_2,b_2,e_2}
>
> It may be common to have sub directories with the same path having different
> blobs, e.g. when having say multiple hardware configurations in different sub
> directories configured. Then renaming becomes a pain when they overlap.

Sure, agreed.  Although, the one nice thing about this particular
testcase is that despite showing suboptimal merge behavior, it's at
least the exact same suboptimal behavior as before when we didn't have
directory rename detection.

>> +# moves directories.  Implment directory rename detection suboptimally, and
>
> Implement

Thanks.

> ok, so add "Expected" as well? (repeating "Previous git", or so?)

Yeah, I should make that more explicit.

>> +# Testcase 8d, rename/delete...or not?
>> +#   (Related to testcase 5b; these may appear slightly inconsistent to 
>> users;
>> +#Also related to testcases 7d and 7e)
>
>> +#   Commit A: z/{b,c,d}
>> +#   Commit B: y/{b,c}
>> +#   Commit C: z/{b,c,d,e}
>> +#   Expected: y/{b,c,e}
>
> Why this?
> * d is deleted in B and not found in the result
> * the rename detection also worked well in z->y  for adding e
>
> I do not see the confusion, yet.

Um...yaay?  If you don't see it as confusing, then maybe others don't?
 I was wondering if folks would expect a rename/delete conflict (x/d
either deleted or renamed to y/d via directory rename detection), and
be annoyed if the merge succeeded and didn't even give so much as a
warning about what happened to 'd'.

>> +#   In this case, I'm leaning towards: commit B was the one that deleted z/d
>> +#   and it did the rename of z to y, so the two "conflicts" (rename vs.
>> +#   delete) are both coming from commit B, which is non-sensical.  Conflicts
>> +#   during merging are supposed to be about opposite sides doing things
>> +#   differently.
>
>   "Sensical has not yet become an "official" word in the English language, 
> which
>   would be why you can't use it. Nonsense is a word, therefore nonsensical can
>   used to describe something of nonsense. However, sense has different 
> meanings
>   and doesn't have an adjective for something of sense"
>
> from https://english.stackexchange.com/questions/38582/antonym-of-nonsensical
> I don't mind it, the spell checker just made me go on a detour. Maybe 
> illogical?

Illogical works for me.

>> +# Testcase 8e, Both sides rename, one side adds to original directory
>> +#   Commit A: z/{b,c}
>> +#   Commit B: y/{b,c}
>> +#   Commit C: w/{b,c}, z/d
>> +#
>> +# Possible Resolutions:
>> +#   Previous git: z/d, CONFLICT(z/b -> y/b vs. w/b), CONFLICT(z/c -> y/c 
>> vs. w/c)
>> +#   Expected: y/d, CONFLICT(z/b -> y/b vs. w/b), CONFLICT(z/c -> y/c 
>> vs. w/c)
>> +#   Preferred:??
>> +#
>> +# Notes: In commit B, directory z got renamed to y.  In commit C, directory 
>> z
>> +#did NOT get renamed; the directory is still present; instead it is
>> +#considered to have just renamed a subset of paths in directory z
>> +#elsewhere.  Therefore, the directory rename done in commit B to z/
>> +#applies to z/d and maps it to y/d.
>> +#
>> +#It's possible that users would get confused about this, but what
>> +#should we do instead?   Silently leaving at z/d seems just as bad 
>> or
>> +#maybe even worse.  Perhaps we could print a big warning about z/d
>> +#and how we're moving to y/d in this case, but when I started 
>> thinking
>> +#abouty the ramifications of doing that, I didn't know how to rule 
>> out
>> +#that opening other weird edge and corner cases so I just punted.
>
> s/about/abouty

I think you mean the other direction?  Thanks for catching, I'll fix that up.

> It sort of makes sense from a users POV.

I'm afraid I'm unsure what the antecedent of "It" is here.  (Are you
just saying that my rationale for what I listed as 

Re: [PATCHv3 7/7] builtin/describe.c: describe a blob

[Jonathan Tan]

On Tue, 14 Nov 2017 12:40:03 -0800
Stefan Beller  wrote:

> Thanks for the review!
> 
> This series was written with the mindset, that a user would only ever
> want to describe bad blobs. (bad in terms of file size, unwanted content, etc)
> 
> With the --reverse you only see the *first* introduction of said blob,
> so finding out if it was re-introduced is still not as easy, whereas "when
> was this blob last used" which is what the current algorithm does, covers
> that case better.

How does "when was this blob last used" cover reintroduction better? If
you want to check all introductions, then you'll need something like
what I describe (quoted below).

> > Alternatively, to me, it seems that listing commits that *introduces*
> > the blob (that is, where it references the blob, but none of its parents
> > do) would be the best way. That would then be independent of traversal
> > order (and we would no longer need to find a tag etc. to tie the blob
> > to).
> 
> What if it is introduced multiple times? (either in multiple competing
> side branches; or introduced, reverted and re-introduced?)

Then all of them should be listed.

> > If we do that, it seems to me that there is a future optimization that
> > could get the first commit to the user more quickly - once a commit
> > without the blob and a descendant commit with the blob is found, that
> > interval can be bisected, so that the first commit is found in O(log
> > number of commits) instead of O(commits). But this can be done later.
> 
> bisection assumes that we only have one "event" going from good to
> bad, which doesn't hold true here, as the blob can be there at different
> occasions of the history.

Yes, bisection would only be useful to output one commit more quickly.
We will still need to exhaustively search for all commits if the user
needs more than one.

Re: [PATCH 10/30] directory rename detection: more involved edge/corner testcases

[Elijah Newren]

On Mon, Nov 13, 2017 at 4:42 PM, Stefan Beller  wrote:
> On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren  wrote:

> "In my opinion" ... sounds like commit message?

Sure, I can move it there.


>> +# Testcase 7a, rename-dir vs. rename-dir (NOT split evenly) PLUS 
>> add-other-file
>> +#   Commit A: z/{b,c}
>> +#   Commit B: y/{b,c}
>> +#   Commit C: w/b, x/c, z/d
>> +#   Expected: y/d, CONFLICT(rename/rename for both z/b and z/c)
>> +#   NOTE: There's a rename of z/ here, y/ has more renames, so z/d -> y/d.
>
> But the creator of C intended to have z/d, not {w,x}/d, and as {w,x} == y,
> I am not sure I like this result. (I have no concrete counter example, just
> messy logic)

I'm open to alternative interpretations here.  The biggest issue for
me -- going back our discussion at the end of
https://public-inbox.org/git/cabpp-bfkiam6ak-gg_rzalulur-jz0kvv3tqshnhg5+htv_...@mail.gmail.com/
-- is "simple, predictable rule", which is consistent with the other
rules and limits the number of nasty corner cases as much as possible.
Perhaps you think this is one of those nasty corner cases, and that's
fair, but I think it'd be hard to do much better.

After spending quite a while trying to think of any other alternative
rules or ways of looking at this, I could only come up with two
points:

  1) One could view this as a case where commit C didn't in fact do
any directory rename -- note that directory z/ still exists in that
commit.  Thus, only B did a rename, it renamed z/ -> y/, thus C's z/d
should be moved to y/d.  So, this choice is consistent with the other
rules we've got.

  2) An alternate (or maybe additional?) rule: We could decide that if
a source path is renamed on both sides of history, then we'll just
ignore both renames for consideration of directory rename detection.

The new rule idea would "fix" this testcase to your liking, although
now we'd be somewhat inconsistent with the "directory still exists
implies no directory rename occurred rule".  But what other weirdness
could entail?  Here's a few I've thought of:

Commit O: z/{b,c,d}
Commit A: y/{b,c}
Commit B: z/{newb, newc, e}

Here, A renamed z/ -> y/.  Except B renamed z/b and z/c differently,
so all paths used to detect the z/ -> y/ rename are ignored, so there
isn't a rename after all.  I'm not so sure I like that decision.
Let's keep looking though, and change it up a bit more:

Commit O: z/{b,c,d}
Commit A: y/{b,c}, x/d
Commit B: z/{newb, newc, d, e}

Here, A has a split rename.  Since B renames z/b and z/c differently,
we have to ignore the z/ -> y/ rename, and thus the only rename left
implies z/ -> x/.  Thus we'd end up with z/e getting moved into x/e.
Seems weird to me, and less likely that a user would understand this
rule than the "majority wins" one.


>> +# Testcase 7c, rename/rename(1to...2or3); transitive rename may add 
>> complexity
>> +#   (Related to testcases 3b and 5c)
>> +#   Commit A: z/{b,c}, x/d
>> +#   Commit B: y/{b,c}, w/d
>> +#   Commit C: z/{b,c,d}
>> +#   Expected: y/{b,c}, CONFLICT(x/d -> w/d vs. y/d)
>
> CONFLICT(x/d -> y/d vs w/d) ?

I'm afraid I'm not following the question.

>
>> +#   NOTE: z/ was renamed to y/ so we do not want to report
>> +# either CONFLICT(x/d -> w/d vs. z/d)
>> +# or CONFLiCT x/d -> w/d vs. y/d vs. z/d)
>
> "neither ... nor" instead of "not either or"?

Yes, thanks.

>> +# Testcase 7e, transitive rename in rename/delete AND dirs in the way
>> +#   (Very similar to 'both rename source and destination involved in D/F 
>> conflict' from t6022-merge-rename.sh)
>> +#   (Also related to testcases 9c and 9d)
>> +#   Commit A: z/{b,c}, x/d_1
>> +#   Commit B: y/{b,c,d/g}, x/d/f
>> +#   Commit C: z/{b,c,d_1}
>> +#   Expected: rename/delete(x/d_1->y/d_1 vs. None) + D/F conflict on y/d
>> +# y/{b,c,d/g}, y/d_1~C^0, x/d/f
>> +#   NOTE: x/d/f may be slightly confusing here.  x/d_1 -> z/d_1 implies
>> +# there is a directory rename from x/ -> z/, performed by commit C.
>> +# However, on the side of commit B, it renamed z/ -> y/, thus
>> +# making a rename from x/ -> z/ when it was getting rid of z/ seems
>> +# non-sensical.  Further, putting x/d/f into y/d/f also doesn't
>> +# make a lot of sense because commit B did the renaming of z to y
>> +# and it created x/d/f, and it clearly made these things separate,
>> +# so it doesn't make much sense to push these together.
>
> This is confusing.

Indeed it is.  When I first wrote this testcase, I didn't realize that
I actually had two potentially directory renames involved and a
doubly-transitive rename from it, on top of the D/F conflict.  I can
see two ways to resolve this.

1) Leave the testcase alone, just try to make the NOTE more clear:

NOTE: The main path of interest here is d_1 and where it ends up, but
this is actually a case that has two potential directory renames
involved and D/F conflict(s), so it makes sense to walk 

Re: [PATCH] merge-recursive: Handle addition of submodule on our side of history

[Stefan Beller]

On Tue, Nov 14, 2017 at 10:48 AM, Stefan Beller  wrote:

> test_expect_success 'unrelated file/submodule conflict is ignored' '
> (
> cd a_repo &&
> git checkout with_file^0 &&
> git cherry-pick with_sub^0

This makes no sense, yet. Sorry about the noise.
I think your patch is fine as is.

Re: [PATCHv3 7/7] builtin/describe.c: describe a blob

[Stefan Beller]

On Tue, Nov 14, 2017 at 12:02 PM, Jonathan Tan  wrote:
> On Thu,  2 Nov 2017 12:41:48 -0700
> Stefan Beller  wrote:
>
>> Sometimes users are given a hash of an object and they want to
>> identify it further (ex.: Use verify-pack to find the largest blobs,
>> but what are these? or [1])
>>
>> "This is an interesting endeavor, because describing things is hard."
>>   -- me, upon writing this patch.
>>
>> When describing commits, we try to anchor them to tags or refs, as these
>> are conceptually on a higher level than the commit. And if there is no ref
>> or tag that matches exactly, we're out of luck.  So we employ a heuristic
>> to make up a name for the commit. These names are ambiguous, there might
>> be different tags or refs to anchor to, and there might be different
>> path in the DAG to travel to arrive at the commit precisely.
>>
>> When describing a blob, we want to describe the blob from a higher layer
>> as well, which is a tuple of (commit, deep/path) as the tree objects
>> involved are rather uninteresting.  The same blob can be referenced by
>> multiple commits, so how we decide which commit to use?  This patch
>> implements a rather naive approach on this: As there are no back pointers
>> from blobs to commits in which the blob occurs, we'll start walking from
>> any tips available, listing the blobs in-order of the commit and once we
>> found the blob, we'll take the first commit that listed the blob.  For
>> source code this is likely not the first commit that introduced the blob,
>> but rather the latest commit that contained the blob.  For example:
>>
>>   git describe v0.99:Makefile
>>   v0.99-5-gab6625e06a:Makefile
>>
>> tells us the latest commit that contained the Makefile as it was in tag
>> v0.99 is commit v0.99-5-gab6625e06a (and at the same path), as the next
>> commit on top v0.99-6-gb1de9de2b9 ([PATCH] Bootstrap "make dist",
>> 2005-07-11) touches the Makefile.
>>
>> Let's see how this description turns out, if it is useful in day-to-day
>> use as I have the intuition that we'd rather want to see the *first*
>> commit that this blob was introduced to the repository (which can be
>> achieved easily by giving the `--reverse` flag in the describe_blob rev
>> walk).
>
> The method of your intuition indeed seems better - could we just have
> this from the start?

Thanks for the review!

This series was written with the mindset, that a user would only ever
want to describe bad blobs. (bad in terms of file size, unwanted content, etc)

With the --reverse you only see the *first* introduction of said blob,
so finding out if it was re-introduced is still not as easy, whereas "when
was this blob last used" which is what the current algorithm does, covers
that case better.

> Alternatively, to me, it seems that listing commits that *introduces*
> the blob (that is, where it references the blob, but none of its parents
> do) would be the best way. That would then be independent of traversal
> order (and we would no longer need to find a tag etc. to tie the blob
> to).

What if it is introduced multiple times? (either in multiple competing
side branches; or introduced, reverted and re-introduced?)

> If we do that, it seems to me that there is a future optimization that
> could get the first commit to the user more quickly - once a commit
> without the blob and a descendant commit with the blob is found, that
> interval can be bisected, so that the first commit is found in O(log
> number of commits) instead of O(commits). But this can be done later.

bisection assumes that we only have one "event" going from good to
bad, which doesn't hold true here, as the blob can be there at different
occasions of the history.

Re: [PATCH v1 2/2] worktree: make add dwim

[Eric Sunshine]

On Tue, Nov 14, 2017 at 3:14 PM, Eric Sunshine  wrote:
> For my own edification...
> [...]
> git worktree add ../topic
>
> * Correctly errors out, refusing to create a new branch named "topic",
> if "topic" is already a branch.

By the way, there's an additional DWIM that could be done here instead
of erroring out. Specifically, for "git worktree add ../topic":

* If branch "topic" exists, check it out (rather than refusing to
create a new branch named "topic").

* If origin/topic exists, DWIM local "topic" branch into existence.

* Otherwise, create new local branch "topic".

> * Creates a new branch named "topic" if no such local branch exists.
>
> The desired new DWIMing would change the second bullet point to:
>
> * If no local branch named "topic" exists, DWIM it from "origin/topic"
> if possible, else create a new local branch named "topic".

Re: [PATCH v1 2/2] worktree: make add dwim

[Eric Sunshine]

On Tue, Nov 14, 2017 at 3:45 AM, Thomas Gummerer  wrote:
> On 11/13, Junio C Hamano wrote:
>> If so, as long as the new DWIM kicks in ONLY when "topic" does not
>> exist, I suspect that there is no backward compatibility worries.
>> The command line
>>
>> $ git worktree add ../a-new-worktree topic
>>
>> would just have failed because three was no 'topic' branch yet, no?
>
> Indeed, with this there would not be any backwards compatility
> worries.
>
> Ideally I'd still like to make
>
> $ git worktree add ../topic
>
> work as described above, to avoid having to type 'topic' twice (the
> directory name matches the branch name for the vast majority of my
> worktrees) but that should then come behind a flag/config option?
> Following your reasoning above it should probably be called something
> other than --guess.
>
> Maybe --guess-remote and worktree.guessRemote would do?  I'm quite bad
> at naming though, so other suggestions are very welcome.

For my own edification...

git worktree add ../dir branch

* Checks out branch into ../dir if branch exists.

* Errors out if branch does not exist. However, if we consider the
history of the implementation of worktrees[*1*], then this really
ought to try the "origin/branch -> branch" DWIM before erroring-out.
Implementing this DWIM could be considered a regression fix according
to [*1*] and, as Junio points out, should not pose backward
compatibility worries.

git worktree add ../topic

* Correctly errors out, refusing to create a new branch named "topic",
if "topic" is already a branch.

* Creates a new branch named "topic" if no such local branch exists.

The desired new DWIMing would change the second bullet point to:

* If no local branch named "topic" exists, DWIM it from "origin/topic"
if possible, else create a new local branch named "topic".

But that's a behavior change since, with the existing implementation,
a newly created local "topic" has no relation to, and does not track,
any origin/topic branch. The proposed --guess option is to avoid users
being hit by this backward incompatibility, however, one could also
view the proposed DWIMing as some sort of bug fix since, at least
some, users would expect the DWIMing since it already takes place
elsewhere.

So, at least two options exist:

* Just make the new DWIMing the default behavior, accepting that it
might bite a few people. Fallout can be mitigated somewhat by
prominently announcing that the DWIMing took place, in which case the
user can take corrective action (say, by choosing a different worktree
name); nothing is lost and no damage done since this is happening only
at worktree creation time.

* Add a new option to enable DWIMing; perhaps name it -t/--track,
which is familiar terminology and still gives you a relatively short
and sweet "git worktree add -t ../topic".

Given the mentioned mitigation factor and that some/many users likely
would expect it to DWIM "origin/topic -> topic" anyhow, I'd lean in
favor of the first option (but perhaps I'm too daring with other
people's workflows).

FOOTNOTES

[*1*]: When Duy first implemented worktree support, he incorporated it
directly into the git-checkout command ("git checkout --to worktree
..."), which means that he got all the git-checkout features for free,
including the "origin/branch -> branch" DWIM. When worktree support
was later moved to git-worktree, it lost most of the features
inherited implicitly from git-checkout, such as -b, -B, --detach, so
those were added back to git-worktree explicitly. However, at that
early stage, git-worktree was still piggy-backing atop git-checkout,
thus likely was still getting the "origin/branch -> branch" DWIM for
free. A final iteration converted git-worktree away from heavyweight
git-checkout to lightweight git-reset, at which point he DWIMing was
lost. If you take this history into account, then loss of
"origin/branch -> branch" DWIMing is a regression, so restoring it
could be considered a bug fix.

Re: some apparent inaccuracies in "man git-worktree"

[Robert P. J. Day]

On Tue, 14 Nov 2017, Eric Sunshine wrote:

> On Tue, Nov 14, 2017 at 3:43 AM, Robert P. J. Day  
> wrote:
> > from "man git-worktree", there seem to be some inaccuracies in the
> > SYNOPSIS regarding the "add" subcommand:
> >
> >   git worktree add \
> > [-f] [--detach] [--checkout] [--lock] [-b ]  
> > []
> >
> >   first, there's no mention of "-B" in that SYNOPSIS, even though it's
> > explained further down the man page.
>
> Omission of "-B" from the synopsis was intentional. From cbdf60fa18
> (worktree: add -b/-B options, 2015-07-06):
>
> worktree: add -b/-B options
>
> One of git-worktree's roles is to populate the new worktree, much like
> git-checkout, and thus, for convenience, ought to support several of the
> same shortcuts. Toward this goal, add -b/-B options to create a new
> branch and check it out in the new worktree.
>
> (For brevity, only -b is mentioned in the synopsis; -B is omitted.)
>
> Whether or not the omission was actually a good decision is
> questionable. The thinking, at the time, may have been that users
> already familiar with "-b" in 'git checkout' would likewise be
> familiar with (and be able to infer) "-B", thus it wasn't important to
> state its existence explicitly in the synopsis, which was already
> getting lengthy. Of course, that decision does not assist newcomers,
> so adding "-B" to the synopsis would help the page better stand on its
> own.
>
> >   next, the SYNOPSIS seems misleading as it doesn't make clear that
> > the options -b, -B and --detach are mutually exclusive, which is made
> > clear in the worktree.c source:
> >
> > if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
> > die(_("-b, -B, and --detach are mutually exclusive"));
>
> Failure to update the synopsis to indicate mutual exclusion appears to
> be a simple oversight in ab0b2c53ed (worktree: make --detach mutually
> exclusive with -b/-B, 2015-07-17) in response to:
> https://public-inbox.org/git/55a8f4b1.9060...@drmicha.warpmail.net/
>
> >   finally (and maybe i'm just not reading carefully enough), it's not
> > clear what happens if you add a worktree at a given commit without
> > specifying *any* of -b, -B or --detach. the obvious result should be a
> > new worktree checked out at a detached HEAD and, interestingly, if i
> > do that, then from the main tree, i see:
> >
> >   $ git worktree list
> >   /home/rpjday/k/git   516fb7f2e73d [master]
> >   /home/rpjday/k/temp  c470abd4fde4 (detached HEAD)
> >   $
> >
> > but from within the worktree, if i ask for the status, i see only:
> >
> >   $ git status
> >   Not currently on any branch.
> >   nothing to commit, working tree clean
> >   $
> >
> > where i would normally have expected to see "detached HEAD", is there
> > a reason that's not displayed?
>
> Someone more familiar with this bit can correct me if I'm wrong, but I
> believe that the "HEAD detached at/from " you normally see
> with 'git status' is derived from the reflog, and if it can't find the
> information in the reflog, it instead shows the generic "Not currently
> on any branch" (which is the equivalent of the "(detached HEAD)" you
> see in "git worktree list").
>
> Each worktree has its own newly-created reflog, which does _not_
> contain enough information for 'git status' to present the more
> detailed "detached" message, thus it falls back to the generic one.
> Perhaps seeding the worktree's reflog with a bit more information at
> creation time would be a good #leftoverbits task.

  i'm not sure what i can add to this, but i'm going to leave it to
folks higher up the food chain than me to resolve any of the above.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCHv3 7/7] builtin/describe.c: describe a blob

[Jonathan Tan]

On Thu,  2 Nov 2017 12:41:48 -0700
Stefan Beller  wrote:

> Sometimes users are given a hash of an object and they want to
> identify it further (ex.: Use verify-pack to find the largest blobs,
> but what are these? or [1])
> 
> "This is an interesting endeavor, because describing things is hard."
>   -- me, upon writing this patch.
> 
> When describing commits, we try to anchor them to tags or refs, as these
> are conceptually on a higher level than the commit. And if there is no ref
> or tag that matches exactly, we're out of luck.  So we employ a heuristic
> to make up a name for the commit. These names are ambiguous, there might
> be different tags or refs to anchor to, and there might be different
> path in the DAG to travel to arrive at the commit precisely.
> 
> When describing a blob, we want to describe the blob from a higher layer
> as well, which is a tuple of (commit, deep/path) as the tree objects
> involved are rather uninteresting.  The same blob can be referenced by
> multiple commits, so how we decide which commit to use?  This patch
> implements a rather naive approach on this: As there are no back pointers
> from blobs to commits in which the blob occurs, we'll start walking from
> any tips available, listing the blobs in-order of the commit and once we
> found the blob, we'll take the first commit that listed the blob.  For
> source code this is likely not the first commit that introduced the blob,
> but rather the latest commit that contained the blob.  For example:
> 
>   git describe v0.99:Makefile
>   v0.99-5-gab6625e06a:Makefile
> 
> tells us the latest commit that contained the Makefile as it was in tag
> v0.99 is commit v0.99-5-gab6625e06a (and at the same path), as the next
> commit on top v0.99-6-gb1de9de2b9 ([PATCH] Bootstrap "make dist",
> 2005-07-11) touches the Makefile.
> 
> Let's see how this description turns out, if it is useful in day-to-day
> use as I have the intuition that we'd rather want to see the *first*
> commit that this blob was introduced to the repository (which can be
> achieved easily by giving the `--reverse` flag in the describe_blob rev
> walk).

The method of your intuition indeed seems better - could we just have
this from the start?

Alternatively, to me, it seems that listing commits that *introduces*
the blob (that is, where it references the blob, but none of its parents
do) would be the best way. That would then be independent of traversal
order (and we would no longer need to find a tag etc. to tie the blob
to).

If we do that, it seems to me that there is a future optimization that
could get the first commit to the user more quickly - once a commit
without the blob and a descendant commit with the blob is found, that
interval can be bisected, so that the first commit is found in O(log
number of commits) instead of O(commits). But this can be done later.

Re: [PATCHv3 5/7] builtin/describe.c: print debug statements earlier

[Stefan Beller]

On Tue, Nov 14, 2017 at 11:55 AM, Jonathan Tan  wrote:
> On Thu,  2 Nov 2017 12:41:46 -0700
> Stefan Beller  wrote:
>
>> For debuggers aid we'd want to print debug statements early, so
>> introduce a new line in the debug output that describes the whole
>> function, and then change the next debug output to describe why we
>> need to search. Conveniently drop the arg from the second line;
>> which will be useful in a follow up commit, that refactors the\
>> describe function.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>>  builtin/describe.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/describe.c b/builtin/describe.c
>> index fd61f463cf..3136efde31 100644
>> --- a/builtin/describe.c
>> +++ b/builtin/describe.c
>> @@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)
>>   unsigned long seen_commits = 0;
>>   unsigned int unannotated_cnt = 0;
>>
>> + if (debug)
>> + fprintf(stderr, _("describe %s\n"), arg);
>> +
>
> Could you explain in the commit message why this wasn't needed before
> (if it wasn't needed), and why this is needed now?
>
>>   if (get_oid(arg, ))
>>   die(_("Not a valid object name %s"), arg);
>>   cmit = lookup_commit_reference();
>> @@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)
>>   if (!max_candidates)
>>   die(_("no tag exactly matches '%s'"), 
>> oid_to_hex(>object.oid));
>>   if (debug)
>> - fprintf(stderr, _("searching to describe %s\n"), arg);
>> + fprintf(stderr, _("No exact match on refs or tags, searching 
>> to describe\n"));
>
> What is this arg that can be safely dropped?
>
> You mention that it is for convenience (since the describe() function
> will be refactored), but could the arg just be passed to the new
> function?

It could, but I want to avoid that just to print a debugging statement
inside the
function. So I factor the debugging printing out of the function
introduced in the
next patch.

Re: [PATCHv3 5/7] builtin/describe.c: print debug statements earlier

[Jonathan Tan]

On Thu,  2 Nov 2017 12:41:46 -0700
Stefan Beller  wrote:

> For debuggers aid we'd want to print debug statements early, so
> introduce a new line in the debug output that describes the whole
> function, and then change the next debug output to describe why we
> need to search. Conveniently drop the arg from the second line;
> which will be useful in a follow up commit, that refactors the\
> describe function.
> 
> Signed-off-by: Stefan Beller 
> ---
>  builtin/describe.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/describe.c b/builtin/describe.c
> index fd61f463cf..3136efde31 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -293,6 +293,9 @@ static void describe(const char *arg, int last_one)
>   unsigned long seen_commits = 0;
>   unsigned int unannotated_cnt = 0;
>  
> + if (debug)
> + fprintf(stderr, _("describe %s\n"), arg);
> +

Could you explain in the commit message why this wasn't needed before
(if it wasn't needed), and why this is needed now?

>   if (get_oid(arg, ))
>   die(_("Not a valid object name %s"), arg);
>   cmit = lookup_commit_reference();
> @@ -316,7 +319,7 @@ static void describe(const char *arg, int last_one)
>   if (!max_candidates)
>   die(_("no tag exactly matches '%s'"), 
> oid_to_hex(>object.oid));
>   if (debug)
> - fprintf(stderr, _("searching to describe %s\n"), arg);
> + fprintf(stderr, _("No exact match on refs or tags, searching to 
> describe\n"));

What is this arg that can be safely dropped?

You mention that it is for convenience (since the describe() function
will be refactored), but could the arg just be passed to the new
function?

Re: [PATCHv3 3/7] revision.h: introduce blob/tree walking in order of the commits

[Jonathan Tan]

On Thu,  2 Nov 2017 12:41:44 -0700
Stefan Beller  wrote:

> @@ -239,6 +239,8 @@ void traverse_commit_list(struct rev_info *revs,
>   if (commit->tree)
>   add_pending_tree(revs, commit->tree);
>   show_commit(commit, data);
> + if (revs->tree_blobs_in_commit_order)
> + traverse_trees_and_blobs(revs, , show_object, data);
>   }
>   traverse_trees_and_blobs(revs, , show_object, data);
>  

I would have expected add_pending_tree() above to no longer be invoked.
If it still needs to be invoked, maybe add an explanation in the form of
a comment or commit message.

> +test_expect_success 'rev-list --in-commit-order' '
> + for x in one two three four
> + do
> + echo $x >$x &&
> + git add $x &&
> + git commit -m "add file $x" ||
> + return 1
> + done &&
> + for x in four three
> + do
> + git rm $x &&
> + git commit -m "remove $x" ||
> + return 1
> + done &&
> + git rev-list --in-commit-order --objects HEAD >actual.raw &&
> + cut -c 1-40 >actual  +
> + git cat-file --batch-check="%(objectname)" >expect.raw <<-\EOF &&
> + HEAD^{commit}
> + HEAD^{tree}
> + HEAD^{tree}:one
> + HEAD^{tree}:two
> + HEAD~1^{commit}
> + HEAD~1^{tree}
> + HEAD~1^{tree}:three
> + HEAD~2^{commit}
> + HEAD~2^{tree}
> + HEAD~2^{tree}:four
> + HEAD~3^{commit}
> + # HEAD~3^{tree} skipped, same as HEAD~1^{tree}
> + HEAD~4^{commit}
> + # HEAD~4^{tree} skipped, same as HEAD^{tree}
> + HEAD~5^{commit}
> + HEAD~5^{tree}
> + EOF
> + grep -v "#" >expect  +
> + test_cmp expect actual
> +'

Would it be useful to have another test without --in-commit-order, so
that we can see the difference (and ensure that existing behavior is
unchanged)?

Re: [PATCH] sha1: add gnutls as a sha1 provider

[Todd Zullinger]

Hi Shawn,

Shawn Landden wrote:
I think this is preferrable to bringing the assembly routines into 
the git code-base, as a way of getting access to these high-performance 
routines to a git available in Debian, Ubuntu, or Fedora (which 
all use BLK_SHA1=1 due to GPLv2 + OpenSSL license considerations, 
see Debian Bug #879459).


While it seems like it could be useful to have the choice of using the 
fast SHA1 implementation without concern about licensing issues, 
there's a few details I thought were worth mentioning.


Fedora moved from OpenSSL SHA1 to BLK_SHA1 to reduce the size of the 
binaries and dependencies, not due to licensing issues (Fedora 
considers OpenSSL a system library and allows linking GPLv2 code).


Fedora now uses the default DC_SHA1 (the collision-detecting SHA1 
implementation).  DC_SHA1 is not, as far as I know, as fast as the 
OpenSSL/GnuTLS SHA1, but it's safer given the increasingly successful 
attacks against SHA1.  I don't envision changing that to gain 
performance.  (And, of course, the speed of SHA1 should become less of 
an issue once git moves to a new, stronger hash.)


It looks like the Debian packages use the default DC_SHA1 
implementation as well.  Regardless of the licensing concerns 
regarding OpenSSL in Debian, I suspect they'll want to use the 
default, collision-detecting SHA1 implementation.  That doesn't mean a 
patch to add the option of GnuTLS isn't useful though.


Fedora does link with OpenSSL's libcrypto and libssl in Fedora for the 
remote-curl helpers and imap-send.  I believe the remote-curl helpers 
just link with curl, which happens to use OpenSSL on Fedora and could 
use GnuTLS instead.  The imap-send command might also use curl and 
whatever crypto library curl is built with too, but I'm not terribly 
familiar with imap-send. (I think those are the only uses of libcrypto 
or libssl in Fedora's packages, but I could be mistaken).


That's a lot of text without having anything to say about the actual 
patch.  Hopefully it's at least mildly useful to you or others. :)


--
Todd
~~
When we remember we are all mad, the mysteries of life disappear and
life stands explained.
   -- Mark Twain

Nice Day

[UN]

Please, I am aware that this is certainly not a conventional approach to
communicating with a  stranger or establish a relationship of trust,
but you will understand the need for my action. I am an independent
non-director of Hang Seng Bank Hong Kong. I have important matter to
talk with you about huge amount of funds from my deceased client who
died without any beneficiary, if you are interested in partner with me,
contact me on my private email address for more information.

My private E-mail: wn...@mit.tc

My regards,

Dr. Raymond Chien Kuo Fung.

Re: [PATCH] merge-recursive: Handle addition of submodule on our side of history

[Stefan Beller]

+cc Ефимов Василий  who reported the issue at
https://public-inbox.org/git/743acc29-85bb-3773-b6a0-68d4a0b8f...@ispras.ru/

On Tue, Nov 14, 2017 at 9:31 AM, Elijah Newren  wrote:
> The code for a newly added path assumed that the path was a normal file,
> and thus checked for there being a directory still being in the way of
> the file.  Note that since unpack_trees() does path-in-the-way checks
> already, the only way for there to be a directory in the way at this
> point in the code, is if there is some kind of D/F conflict in the merge.
>
> For a submodule addition on HEAD's side of history, the submodule would
> have already been present.  This means that we do expect there to be a
> directory present but should not consider it to be "in the way"; instead,
> it's the expected submodule.  So, when there's a submodule addition from
> HEAD's side, don't bother checking the working copy for a directory in
> the way.
>
> Signed-off-by: Elijah Newren 
> ---
> This commit is based on top of sb/test-cherry-pick-submodule-getting-in-a-way.

Thanks for getting the discussion started here (and fixing the bug),
based on your input in
https://public-inbox.org/git/cabpp-bhdrw_daesic3xk7kc3jmgkenqupqf69opbvyhrkbh...@mail.gmail.com/
I adapted the test case locally to have two tests one file/submodule
and a submodule/file conflict, after the setup which boroows a lot of
code from the
existing test, we'll have:

test_expect_success 'unrelated submodule/file conflict is ignored' '
(
cd a_repo &&
   git checkout with_sub^0 &&
git cherry-pick with_file^0
)
'

test_expect_success 'unrelated file/submodule conflict is ignored' '
(
cd a_repo &&
git checkout with_file^0 &&
git cherry-pick with_sub^0
)
'

and the other case now fails. I'll take a look into that. I think we'd need to
check either a_mode or b_mode to be a submodule depending on which side
the submodule occured.

So I'll build on top of this patch to fix the other way, too.

>
>  merge-recursive.c| 5 +++--
>  t/t3512-cherry-pick-submodule.sh | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 1d3f8f0d22..9fb0b9f8fd 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1901,8 +1901,9 @@ static int process_entry(struct merge_options *o,
> oid = b_oid;
> conf = _("directory/file");
> }
> -   if (dir_in_way(path, !o->call_depth,
> -  S_ISGITLINK(a_mode))) {
> +   if (dir_in_way(path,
> +  !o->call_depth && !S_ISGITLINK(a_mode),
> +  0)) {

The last flag is_empty_ok is ok to keep at 0, I think.

Thanks,
Stefan

Re: [PATCH 21/30] merge-recursive: Add get_directory_renames()

[Elijah Newren]

On Mon, Nov 13, 2017 at 9:30 PM, Junio C Hamano  wrote:
> Elijah Newren  writes:
>
>> + entry = dir_rename_find_entry(dir_renames, old_dir);
>> + if (!entry) {
>> + entry = xcalloc(1, sizeof(struct dir_rename_entry));
>> + hashmap_entry_init(entry, strhash(old_dir));
>
> Please make these two lines into its own dir_rename_entry_init()
> helper.

> we'd want to see its string_list member to be
> initialised explicitly

Will do.

Private Investition

[Khvostova Zhanna]

--
Diese E-Mail fordert Sie strikt auf, sich für eine große Investition mit 
mir zusammenzutun, um weitere Informationen zu erhalten.

Mit freundlichen Grüßen,
Khvostova Zhanna

Re: some apparent inaccuracies in "man git-worktree"

[Eric Sunshine]

On Tue, Nov 14, 2017 at 3:43 AM, Robert P. J. Day  wrote:
> from "man git-worktree", there seem to be some inaccuracies in the
> SYNOPSIS regarding the "add" subcommand:
>
>   git worktree add \
> [-f] [--detach] [--checkout] [--lock] [-b ]  []
>
>   first, there's no mention of "-B" in that SYNOPSIS, even though it's
> explained further down the man page.

Omission of "-B" from the synopsis was intentional. From cbdf60fa18
(worktree: add -b/-B options, 2015-07-06):

worktree: add -b/-B options

One of git-worktree's roles is to populate the new worktree, much like
git-checkout, and thus, for convenience, ought to support several of the
same shortcuts. Toward this goal, add -b/-B options to create a new
branch and check it out in the new worktree.

(For brevity, only -b is mentioned in the synopsis; -B is omitted.)

Whether or not the omission was actually a good decision is
questionable. The thinking, at the time, may have been that users
already familiar with "-b" in 'git checkout' would likewise be
familiar with (and be able to infer) "-B", thus it wasn't important to
state its existence explicitly in the synopsis, which was already
getting lengthy. Of course, that decision does not assist newcomers,
so adding "-B" to the synopsis would help the page better stand on its
own.

>   next, the SYNOPSIS seems misleading as it doesn't make clear that
> the options -b, -B and --detach are mutually exclusive, which is made
> clear in the worktree.c source:
>
> if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
> die(_("-b, -B, and --detach are mutually exclusive"));

Failure to update the synopsis to indicate mutual exclusion appears to
be a simple oversight in ab0b2c53ed (worktree: make --detach mutually
exclusive with -b/-B, 2015-07-17) in response to:
https://public-inbox.org/git/55a8f4b1.9060...@drmicha.warpmail.net/

>   finally (and maybe i'm just not reading carefully enough), it's not
> clear what happens if you add a worktree at a given commit without
> specifying *any* of -b, -B or --detach. the obvious result should be a
> new worktree checked out at a detached HEAD and, interestingly, if i
> do that, then from the main tree, i see:
>
>   $ git worktree list
>   /home/rpjday/k/git   516fb7f2e73d [master]
>   /home/rpjday/k/temp  c470abd4fde4 (detached HEAD)
>   $
>
> but from within the worktree, if i ask for the status, i see only:
>
>   $ git status
>   Not currently on any branch.
>   nothing to commit, working tree clean
>   $
>
> where i would normally have expected to see "detached HEAD", is there
> a reason that's not displayed?

Someone more familiar with this bit can correct me if I'm wrong, but I
believe that the "HEAD detached at/from " you normally see
with 'git status' is derived from the reflog, and if it can't find the
information in the reflog, it instead shows the generic "Not currently
on any branch" (which is the equivalent of the "(detached HEAD)" you
see in "git worktree list").

Each worktree has its own newly-created reflog, which does _not_
contain enough information for 'git status' to present the more
detailed "detached" message, thus it falls back to the generic one.
Perhaps seeding the worktree's reflog with a bit more information at
creation time would be a good #leftoverbits task.

Re: [PATCH 16/30] merge-recursive: Introduce new functions to handle rename logic

[Elijah Newren]

On Mon, Nov 13, 2017 at 9:14 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Elijah Newren  writes:
>>
>>> +struct rename_info {
>>> +struct string_list *head_renames;
>>> +struct string_list *merge_renames;
>>> +};
>>
>> This type is added in order to allow the caller and the helper to
>> communicate the findings in a single logical structure, instead of
>> having to pass them as separate parameters, etc.  If we anticipate
>> that the information that needs to be passed will grow richer in
>> later steps (or a follow-up series), such encapsulation makes a lot
>
> Hmph, I actually am quite confused with the existing code.
>
> The caller (originally in merge_trees(), now in handle_renames())
> calls get_renames() twice and have the list of renamed paths in
> these two string lists.  get_renames() mostly works with the
> elements in the "entries" list and adds the "struct rename" to the
> string list that is to be returned.  And the caller uses these two
> string lists get_renames() returns when calling process_renames(),
> but once process_renames() is done with them, these two string lists
> are never looked at by anybody.

Actually, if I remember correctly, my first stab was to do all the
cleanup at the end of handle_renames(), but then I ran into
use-after-free errors.  I'm not sure if I remember all the details,
but I'll try to lay out the path:

process_renames() can't handle conflicts immediately because of D/F
concerns (if all entries in the competing directory resolve away, then
there's no more D/F conflict, but we have to wait until each of those
entries is processed to find out if that happens or if a D/F conflict
remains).  Because of that, process_renames() needs to store
information into a rename_conflict_info struct for process_entry() to
look at later.  Included in rename_conflict_info are things like
diff_filepair and stage_data entries, both taken from the rename
lists.  If the rename lists are freed at the end of handle_renames(),
then this information is freed before process_entry() runs and thus we
get a use-after-free error.

Since both you and I thought to push this cleanup to the end of
handle_renames(), though, I should probably add that explanation to
the commit message.  Granted, it isn't actually needed for this
particular commit, because up to this point all the information used
in rename_conflict_info was leaked anyway.  But it becomes an issue
with patch 17 when we start freeing that info.

Re: submodules and merging (Was: Re: [PATCH 02/30] merge-recursive: Fix logic ordering issue)

[Stefan Beller]

Thanks for your reply!

On Tue, Nov 14, 2017 at 9:17 AM, Elijah Newren  wrote:
> On Mon, Nov 13, 2017 at 3:46 PM, Stefan Beller  wrote:
>> On Mon, Nov 13, 2017 at 3:39 PM, Elijah Newren  wrote:
>>> On Mon, Nov 13, 2017 at 2:12 PM, Stefan Beller  wrote:
 I wanted to debug a very similar issue today just after reviewing this
 series, see
 https://public-inbox.org/git/743acc29-85bb-3773-b6a0-68d4a0b8f...@ispras.ru/
>>>
>>> Oh, bleh.  That's not a D/F conflict at all, it's the code assuming
>>> there's a D/F conflict because the entry it is processing ("sub") is a
>>> submodule rather than a file, and it panics when it sees "a directory
>>> in the way" -- a directory that just so happens to be named "sub" and
>>> which is in fact the desired submodule, meaning that the working
>>> directory is already good and needs no changes.
>>
>> yup, I came to find the same snippet of code to be the offender,
>> I just haven't figured out how to fix this bug.
>>
>> Thanks for taking a look!
>>
>> As you have a lot of fresh knowledge in the merge-recursive case
>> currently, how would we approach the fix here?
>
> submodules and merging looks pretty broken, to me.  Backing off from
> the current bug and just looking at merging with submodules in
> general, makes me a little uneasy with what I see.  I mean, just look
> at update_file_flags, when it wants the working directory updated, the
> code for a submodule is the following:
>
> if (update_wd) {
> 
>
> if (S_ISGITLINK(mode)) {
> /*
>  * We may later decide to recursively descend into
>  * the submodule directory and update its index
>  * and/or work tree, but we do not do that now.
>  */
> update_wd = 0;
> goto update_index;
> }
>
> So, it just doesn't put anything there, so the working directory is
> made out-of-date immediately.  Users are happy with that?  Maybe it is
> what makes sense, but it surprised me.

Submodules are traditionally not touched by git commands and we are slowly
trying to get that changed. Some commands have a --recurse-submodules
flag now, including checkout, reset; merge is missing this flag as the semantics
are hard to define sensibly, yet.

> From there, we can start stacking on the weird.  For example let's say
> we have this setup:
> O (merge base): path/contents containing "whatever\n"
> A (ours): path is a submodule
> B (theirs): path/contents containing "whatever\nworks\n"
>
> Merging A into B results in
>
> CONFLICT (modify/delete): path/contents deleted in A and modified in
> HEAD. Version HEAD of path/contents left in tree.
> CONFLICT (directory/file): There is a directory with name path in
> HEAD. Adding path as path~A
> Automatic merge failed; fix conflicts and then commit the result.
>
> but path~A is never created, because of the update_file_flags code
> snippet above.  If the user looks at the path directory, it doesn't
> have any submodule info either.  It has just "disappeared", despite
> the claim made in the conflict notice.  Maybe okay, but slightly
> confusing.
>
> Now let's say the user just wants to back out of this merge.  What if
> they run 'git merge --abort'?  Well, they're greeted with:
>
> error: 'path/contents' appears as both a file and as a directory
> error: path/contents: cannot drop to stage #0
> error: Untracked working tree file 'path/contents' would be
> overwritten by merge.
> fatal: Could not reset index file to revision 'HEAD'.
>
> "Would be overwritten by merge"?  We're unmerging, and I agree it
> shouldn't be overwritten, but the fact that it thinks it needs to try
> is worrisome.  And I don't like the fact that it just outright failed.
> Okay, what about 'git reset --hard' at this point:
>
> error: 'path/contents' appears as both a file and as a directory
> error: path/contents: cannot drop to stage #0
> warning: unable to rmdir path: Directory not empty
> HEAD is now at 3e098a0 B
>
> Cannot drop to stage #0?  Oh, boy.  Things must be really messed up.
> Except they're not; it did drop path/contents to stage #0 despite the
> error, and git status is clean again.
>
> Now, let's switch tracks and look at things from the other side.  What
> if the user had been on A and tried to merge B into A?
>
> $ git checkout A
> Switched to branch 'A'
> $ git merge B
> CONFLICT (modify/delete): path/contents deleted in HEAD and modified
> in B. Version B of path/contents left in tree.
> Adding path
> Automatic merge failed; fix conflicts and then commit the result.
>
> path/contents is left in the tree?  But there's a submodule in the
> way!  Did it really put it there...?  Yep:
>
> $ ls -al path/
> total 16
> drwxrwxr-x 3 newren newren 4096 Nov 14 08:40 .
> drwxrwxr-x 4 newren newren 4096 Nov 14 08:38 ..
> -rw-rw-r-- 1 newren newren   15 Nov 14 08:40 contents
> -rw-rw-r-- 1 newren newren0 Nov 14 08:39 foo
> drwxrwxr-x 8 

Re: [PATCH 15/30] merge-recursive: Move the get_renames() function

[Elijah Newren]

On Mon, Nov 13, 2017 at 8:46 PM, Junio C Hamano  wrote:

> It took me a while to figure out that you are basing this on top of
> a slightly older tip of 'master'.  When rebasing on, or merging to,
> a newer codebase

Sorry about that.  Yes, I worked on the series over time and rebased a
couple times up to v2.15.0.  I assumed that was new enough, but
clearly I was wrong.

> By the way, checkpatch.pl complains about // C99 comments and binary
> operators missing SP on both ends, etc., on the entire series [*1*].
> These look like small issues, but they are distracting enough to
> break concentration while reading the changes to spot places with
> real issues and places that can be improved, so cleaning them up
> early would help the final result to get better reviews.
>
> I won't reproduce all of them here, but here are a representable
> few.

Eek!  My apologies.  I will go through and fix them up.  I see no
reference to checkpatch.pl in git, but a google search shows there's
one in the linux source tree.  Is that were I get it from, or is there
a different one?

Also, would you like me to make a separate commit that cleans up
pre-existing issues in merge-recursive.c so that it runs clean, or
just remove the problems I added?


Thanks for all the reviews!

[PATCH] merge-recursive: Handle addition of submodule on our side of history

[Elijah Newren]

The code for a newly added path assumed that the path was a normal file,
and thus checked for there being a directory still being in the way of
the file.  Note that since unpack_trees() does path-in-the-way checks
already, the only way for there to be a directory in the way at this
point in the code, is if there is some kind of D/F conflict in the merge.

For a submodule addition on HEAD's side of history, the submodule would
have already been present.  This means that we do expect there to be a
directory present but should not consider it to be "in the way"; instead,
it's the expected submodule.  So, when there's a submodule addition from
HEAD's side, don't bother checking the working copy for a directory in
the way.

Signed-off-by: Elijah Newren 
---
This commit is based on top of sb/test-cherry-pick-submodule-getting-in-a-way.

 merge-recursive.c| 5 +++--
 t/t3512-cherry-pick-submodule.sh | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 1d3f8f0d22..9fb0b9f8fd 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1901,8 +1901,9 @@ static int process_entry(struct merge_options *o,
oid = b_oid;
conf = _("directory/file");
}
-   if (dir_in_way(path, !o->call_depth,
-  S_ISGITLINK(a_mode))) {
+   if (dir_in_way(path,
+  !o->call_depth && !S_ISGITLINK(a_mode),
+  0)) {
char *new_path = unique_path(o, path, add_branch);
clean_merge = 0;
output(o, 1, _("CONFLICT (%s): There is a directory 
with name %s in %s. "
diff --git a/t/t3512-cherry-pick-submodule.sh b/t/t3512-cherry-pick-submodule.sh
index 1b1e31100f..ce48c4fcca 100755
--- a/t/t3512-cherry-pick-submodule.sh
+++ b/t/t3512-cherry-pick-submodule.sh
@@ -10,7 +10,7 @@ KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1
 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
 test_submodule_switch "git cherry-pick"
 
-test_expect_failure 'unrelated submodule/file conflict is ignored' '
+test_expect_success 'unrelated submodule/file conflict is ignored' '
test_create_repo sub &&
 
touch sub/file &&
-- 
2.15.0.2.g63e86ab1a0

submodules and merging (Was: Re: [PATCH 02/30] merge-recursive: Fix logic ordering issue)

[Elijah Newren]

On Mon, Nov 13, 2017 at 3:46 PM, Stefan Beller  wrote:
> On Mon, Nov 13, 2017 at 3:39 PM, Elijah Newren  wrote:
>> On Mon, Nov 13, 2017 at 2:12 PM, Stefan Beller  wrote:
>>> I wanted to debug a very similar issue today just after reviewing this
>>> series, see
>>> https://public-inbox.org/git/743acc29-85bb-3773-b6a0-68d4a0b8f...@ispras.ru/
>>
>> Oh, bleh.  That's not a D/F conflict at all, it's the code assuming
>> there's a D/F conflict because the entry it is processing ("sub") is a
>> submodule rather than a file, and it panics when it sees "a directory
>> in the way" -- a directory that just so happens to be named "sub" and
>> which is in fact the desired submodule, meaning that the working
>> directory is already good and needs no changes.
>
> yup, I came to find the same snippet of code to be the offender,
> I just haven't figured out how to fix this bug.
>
> Thanks for taking a look!
>
> As you have a lot of fresh knowledge in the merge-recursive case
> currently, how would we approach the fix here?

submodules and merging looks pretty broken, to me.  Backing off from
the current bug and just looking at merging with submodules in
general, makes me a little uneasy with what I see.  I mean, just look
at update_file_flags, when it wants the working directory updated, the
code for a submodule is the following:

if (update_wd) {


if (S_ISGITLINK(mode)) {
/*
 * We may later decide to recursively descend into
 * the submodule directory and update its index
 * and/or work tree, but we do not do that now.
 */
update_wd = 0;
goto update_index;
}

So, it just doesn't put anything there, so the working directory is
made out-of-date immediately.  Users are happy with that?  Maybe it is
what makes sense, but it surprised me.

>From there, we can start stacking on the weird.  For example let's say
we have this setup:
O (merge base): path/contents containing "whatever\n"
A (ours): path is a submodule
B (theirs): path/contents containing "whatever\nworks\n"

Merging A into B results in

CONFLICT (modify/delete): path/contents deleted in A and modified in
HEAD. Version HEAD of path/contents left in tree.
CONFLICT (directory/file): There is a directory with name path in
HEAD. Adding path as path~A
Automatic merge failed; fix conflicts and then commit the result.

but path~A is never created, because of the update_file_flags code
snippet above.  If the user looks at the path directory, it doesn't
have any submodule info either.  It has just "disappeared", despite
the claim made in the conflict notice.  Maybe okay, but slightly
confusing.

Now let's say the user just wants to back out of this merge.  What if
they run 'git merge --abort'?  Well, they're greeted with:

error: 'path/contents' appears as both a file and as a directory
error: path/contents: cannot drop to stage #0
error: Untracked working tree file 'path/contents' would be
overwritten by merge.
fatal: Could not reset index file to revision 'HEAD'.

"Would be overwritten by merge"?  We're unmerging, and I agree it
shouldn't be overwritten, but the fact that it thinks it needs to try
is worrisome.  And I don't like the fact that it just outright failed.
Okay, what about 'git reset --hard' at this point:

error: 'path/contents' appears as both a file and as a directory
error: path/contents: cannot drop to stage #0
warning: unable to rmdir path: Directory not empty
HEAD is now at 3e098a0 B

Cannot drop to stage #0?  Oh, boy.  Things must be really messed up.
Except they're not; it did drop path/contents to stage #0 despite the
error, and git status is clean again.

Now, let's switch tracks and look at things from the other side.  What
if the user had been on A and tried to merge B into A?

$ git checkout A
Switched to branch 'A'
$ git merge B
CONFLICT (modify/delete): path/contents deleted in HEAD and modified
in B. Version B of path/contents left in tree.
Adding path
Automatic merge failed; fix conflicts and then commit the result.

path/contents is left in the tree?  But there's a submodule in the
way!  Did it really put it there...?  Yep:

$ ls -al path/
total 16
drwxrwxr-x 3 newren newren 4096 Nov 14 08:40 .
drwxrwxr-x 4 newren newren 4096 Nov 14 08:38 ..
-rw-rw-r-- 1 newren newren   15 Nov 14 08:40 contents
-rw-rw-r-- 1 newren newren0 Nov 14 08:39 foo
drwxrwxr-x 8 newren newren 4096 Nov 14 08:40 .git

At least git add and git rm on that location, from the supermodule, do
the right thing.

$ git add path/contents
fatal: Pathspec 'path/contents' is in submodule 'path'
$ git rm path/contents
path/contents: needs merge
rm 'path/contents'

But the fact that merge-recursive wrote unmerged entries back to the
working tree within the submodule feels pretty weird and dirty to me.
(Luckily, if the locations conflict with something in the submodule,
it'll write things out to an alternate path, such as 

Re: Changing encoding of a file : What should happen to CRLF in file ?

[Torsten Bögershausen]

(Back to the beginning)

You have a file ApplicationManifest.xml
It is encoded in UTF-16 (and has CRLF)

You convert it into UTF-8
The file has still CRLF (in the worktree)

Now you add it and make a commit.
Under both Linux and Windows you have "text=auto".

I assume that you have efficiently core.eol=lf under Linux
and core.eol=crlf on Windows.

(That is the default, when you don't change anything)

Now, what happens to the CRLF?
If you commit the file, it will be stored with LF in the index,
on both systems.
On checkout, Windows will convert them into CRLF, but Linux will not.

That why you see
>On linux, during committing i get warning : warning: CRLF will be
>replaced by LF in …file_name..

All in all there is nothing wrong, at least as I see it.

The question remains:
Do you need CRLF in Linux ?
Probably not, but if yes, plase add a line

*.xml text eol=crlf

to your
.gitattributes

Otherwise your .gitconfig looks good to  me.

Re: Recovering from gc errors

[Eric Sunshine]

On Tue, Nov 14, 2017 at 10:39 AM, Marc Branchaud  wrote:
> I'm willing to chalk this up to bugs in the early worktree code, unless one
> of the CC'd worktree developers thinks otherwise.
>
> An explicit "git worktree delete" command would be nice for manually
> cleaning things up.  It's easy to just delete the directory, but having a
> "delete" command gives the user assurance that they're not missing
> something.

Duy does have a series in 'pu' which adds this functionality, but I
guess it's stalled for the moment. From "What's Cooking":

* nd/worktree-move (2017-04-20) 6 commits
- worktree remove: new command
- worktree move: refuse to move worktrees with submodules
- worktree move: accept destination as directory
- worktree move: new command
- worktree.c: add update_worktree_location()
- worktree.c: add validate_worktree()

"git worktree" learned move and remove subcommands.

Expecting a reroll.
cf. <20170420101024.7593-1-pclo...@gmail.com>
cf. <20170421145916.mknekgqzhxffu...@sigill.intra.peff.net>
cf. 

Re: Changing encoding of a file : What should happen to CRLF in file ?

[Torsten Bögershausen]

On 2017-11-14 17:13, Ashish Negi wrote:
> Running the command gives me :
> 
>   git ls-files --eol file_name
>   i/-text w/-text attr/text=auto  file_name
> 

That is strange to me:
According to that, Git would treat the file as text=auto.
And the content is "not next", so there is no need to convert.
Do you have configured any filters ?

Is this a public repo ?
if not: Is there any chance that you provide a public example,
so that we can have a look ?

[PATCH] notes: send "Automatic notes merge failed" messages to stderr

[Todd Zullinger]

All other error messages from notes use stderr.  Do the same when
alerting users of an unresolved notes merge.

Fix the output redirection in t3310 and t3320 as well.  Previously, the
tests directed output to a file, but stderr was either not captured or
not sent to the file due to the order of the redirection operators.

Signed-off-by: Todd Zullinger 
---

Johan Herland wrote:
> ACK :-)
> 
> Error messages should go to stderr, and redirection in the tests
> should be fixed.

Excellent, thanks Johan!

Here's what I came up with.  Hopefully I caught all the tests that need
adjustment.  The test suite passes for me, but it's always possible that I've
missed something.

Style-wise, I'm not sure about the re-wrapping of the error message text.  If
that should be avoided to make the patch's change from printf to fprintf
clearer or should be wrapped differently, let me know.

 builtin/notes.c   | 8 
 t/t3310-notes-merge-manual-resolve.sh | 8 
 t/t3320-notes-merge-worktrees.sh  | 2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 12afdf1907..4468adaf29 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -865,10 +865,10 @@ static int merge(int argc, const char **argv, const char 
*prefix)
if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
die(_("failed to store link to current notes ref (%s)"),
default_notes_ref());
-   printf(_("Automatic notes merge failed. Fix conflicts in %s and 
"
-"commit the result with 'git notes merge --commit', or 
"
-"abort the merge with 'git notes merge --abort'.\n"),
-  git_path(NOTES_MERGE_WORKTREE));
+   fprintf(stderr, _("Automatic notes merge failed. Fix conflicts 
in %s "
+ "and commit the result with 'git notes merge 
--commit', "
+ "or abort the merge with 'git notes merge 
--abort'.\n"),
+   git_path(NOTES_MERGE_WORKTREE));
}
 
free_notes(t);
diff --git a/t/t3310-notes-merge-manual-resolve.sh 
b/t/t3310-notes-merge-manual-resolve.sh
index baef2d6924..9c1bf6eb3d 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -176,7 +176,7 @@ git rev-parse refs/notes/z > pre_merge_z
 test_expect_success 'merge z into m (== y) with default ("manual") resolver => 
Conflicting 3-way merge' '
git update-ref refs/notes/m refs/notes/y &&
git config core.notesRef refs/notes/m &&
-   test_must_fail git notes merge z >output &&
+   test_must_fail git notes merge z >output 2>&1 &&
# Output should point to where to resolve conflicts
test_i18ngrep "\\.git/NOTES_MERGE_WORKTREE" output &&
# Inspect merge conflicts
@@ -379,7 +379,7 @@ git rev-parse refs/notes/z > pre_merge_z
 test_expect_success 'redo merge of z into m (== y) with default ("manual") 
resolver => Conflicting 3-way merge' '
git update-ref refs/notes/m refs/notes/y &&
git config core.notesRef refs/notes/m &&
-   test_must_fail git notes merge z >output &&
+   test_must_fail git notes merge z >output 2>&1 &&
# Output should point to where to resolve conflicts
test_i18ngrep "\\.git/NOTES_MERGE_WORKTREE" output &&
# Inspect merge conflicts
@@ -413,7 +413,7 @@ git rev-parse refs/notes/y > pre_merge_y
 git rev-parse refs/notes/z > pre_merge_z
 
 test_expect_success 'redo merge of z into m (== y) with default ("manual") 
resolver => Conflicting 3-way merge' '
-   test_must_fail git notes merge z >output &&
+   test_must_fail git notes merge z >output 2>&1 &&
# Output should point to where to resolve conflicts
test_i18ngrep "\\.git/NOTES_MERGE_WORKTREE" output &&
# Inspect merge conflicts
@@ -494,7 +494,7 @@ cp expect_log_y expect_log_m
 
 test_expect_success 'redo merge of z into m (== y) with default ("manual") 
resolver => Conflicting 3-way merge' '
git update-ref refs/notes/m refs/notes/y &&
-   test_must_fail git notes merge z >output &&
+   test_must_fail git notes merge z >output 2>&1 &&
# Output should point to where to resolve conflicts
test_i18ngrep "\\.git/NOTES_MERGE_WORKTREE" output &&
# Inspect merge conflicts
diff --git a/t/t3320-notes-merge-worktrees.sh b/t/t3320-notes-merge-worktrees.sh
index b9c3bc2487..10bfc8b947 100755
--- a/t/t3320-notes-merge-worktrees.sh
+++ b/t/t3320-notes-merge-worktrees.sh
@@ -61,7 +61,7 @@ test_expect_success 'merge z into x while mid-merge on y 
succeeds' '
(
cd worktree2 &&
git config core.notesRef refs/notes/x &&
-   test_must_fail git notes merge z 2>&1 >out &&
+   test_must_fail git notes merge z >out 2>&1 &&
test_i18ngrep 

Re: Changing encoding of a file : What should happen to CRLF in file ?

[Ashish Negi]

After changing the encoding of file to utf-8, same command gives:

git ls-files --eol file_name
i/lfw/crlf  attr/text=auto  ApplicationManifest.xml

On Tue, Nov 14, 2017 at 9:43 PM, Ashish Negi  wrote:
> Running the command gives me :
>
>   git ls-files --eol file_name
>   i/-text w/-text attr/text=auto  file_name

Re: Changing encoding of a file : What should happen to CRLF in file ?

[Ashish Negi]

Running the command gives me :

  git ls-files --eol file_name
  i/-text w/-text attr/text=auto  file_name

Re: [PATCH V2] config: add --expiry-date

[Marc Branchaud]

On 2017-11-14 01:21 AM, Christian Couder wrote:

On Tue, Nov 14, 2017 at 3:04 AM,   wrote:

From: Haaris 

Description:
This patch adds a new option to the config command.

Uses flag --expiry-date as a data-type to covert date-strings to
timestamps when reading from config files (GET).
This flag is ignored on write (SET) because the date-string is stored in
config without performing any normalization.

Creates a few test cases and documentation since its a new feature.

Motivation:
A parse_expiry_date() function already existed for api calls,
this patch simply allows the function to be used from the command line.

Signed-off-by: Haaris 


Documentation/SubmittingPatches contains the following:

"Also notice that a real name is used in the Signed-off-by: line. Please
don't hide your real name."

And there is the following example before that:

 Signed-off-by: Random J Developer 

So it looks like "a real name" actually means "a real firstname and a
real surname".

It would be nice if your "Signed-off-by:" could match this format.


It might already match that format if Haaris lives in a society that 
only uses single names.


Still, such names are unusual enough that it's good to check that new 
contributors are following the guidelines properly.


M.



Also if you have a "From:" line at the beginning of the patch, please
make sure that the name there is tha same as on the "Signed-off-by:".

Thanks for working on this,
Christian.

Re: [PATCH v1 1/4] fastindex: speed up index load through parallelization

[Ben Peart]

On 11/14/2017 10:04 AM, Junio C Hamano wrote:

Ben Peart  writes:


How about I add the logic to write out a special extension right
before the SHA1 that contains an offset to the beginning of the
extensions section.  I will also add the logic in do_read_index() to
search for and load this special extension if it exists.

This will provide a common framework for any future extension to take
advantage of if it wants to be loaded/processed before or in parallel
with the cache entries or other extensions.

For all existing extensions that assume they are loaded _after_ the
cache entries, in do_read_index() I'll add the logic to use the offset
(if it exists) to adjust the src_offset and then load them normally.

Given the IEOT extension is just another list of offsets into the
index to enable out of order processing, I'll add those offsets into
the same extension so that it is a more generic "table of contents"
for the entire index.  This enables us to have common/reusable way to
have random access to _all_ sections in the index while maintaining
backwards comparability with the existing index formats and code.

These additional offsets will initially only be used to parallelize
the loading of cache entries and only if the user explicitly enables
that option but I can think of other interesting uses for them in the
future.


If we freeze the format of IEOT extension so that we can guarantee
that the very first version of Git that understands IEOT can always
find the beginning of extension section in an index file that was
written by future versions of Git, then I'm all for that plan, but
my impression was that you are planning to make incompatible change
in the future to IEOT, judging from the way that IEOT records its own
version number in the section and the reader uses it to reject an
unknown one.


I have no thoughts or plans for changes in the future of IEOT (which I 
plan to rename ITOC).  At this point in time, I can't even imagine what 
else we'd want as the index only contains cache entries, extensions and 
the trailing SHA1 and with the TOC we have random access to each of 
those.  If nothing else, it gives another signature to verify to ensure 
we actually have a valid trailing extension. :)


I only added the version because I've learned my ability to predict the 
future is pretty bad and it leaves us that option _if_ something ever 
comes up that we need it _and_ are willing to live with the significant 
negative trade-offs you correctly point out.




With that plan, what I suspect would happen is that a version of Git
that understands another optional extension section that wants to be
findable without scanning the main table and the then-current
version of IEOT would not be able to use an index file written by a
new version of Git that enhances the format of the IEOT extension
bumps its extension version.

And if that is the case I would have to say that I strongly suspect
that you would regret the design decision to mix it into IEOT.  That
is why I keep suggesting that the back pointer extension should be
on its own, minimizing what it does and minimizing the need to be
updated across versions of Git.



I understand the risk but the list of offsets into the cache entries is 
pretty simple as well. I prefer the simplicity of a single TOC extension 
that gives us random access to the entire index rather than having to 
piece one together using multiple extensions.  That model has its own 
set of risks and tradeoffs.

Re: Recovering from gc errors

[Marc Branchaud]

(It turned out that this problem is related to worktrees.  CCing some 
worktree folks.)


On 2017-11-14 12:53 AM, Jeff King wrote:

On Mon, Nov 13, 2017 at 04:13:19PM -0500, Marc Branchaud wrote:


Various incantations of "git show ... 9c355a7726e31" only fail with the same
error, so I can't determine much about the problematic commit. Luckily I'm
not particularly concerned with losing objects, as I push any important
progress to named refs in backup repos.


Doing "git show" will require looking at the parent commit to produce
the diff. Probably "git show -s" would work. But in general for poking
at corruption, something bare-bones like "git cat-file commit 9c355a77"
is going to be your best bet.


Thanks, I'd forgotten about cat-file (show's -s did not work).

Only one or two of the bad commits could possibly belong in a submodule, 
so I don't think I'm seeing a worktree+submodule problem.


There are some definite "rebase -i" commits (e.g. "fixup!"), and a lot 
of what were probably cherry-picks.  I know I did these operations in a 
worktree (see below).



But I would like to clean this up in my local repo so that gc stops failing.
I tried simply removing this and other loose commits that trip up gc (i.e.
the objects/9c/355a7726e31b3033b8e714cf7edb4f0a41d8d4 file -- there are 49
such files, all of which are several months old), but now gc complains of a
bad tree object:


You can't generally fix corruption issues by deleting objects[1]. The
"source" that makes Git want to have these objects is the refs and
reflogs. So your best bet is to find which of those point to the
problematic objects and delete them.

I'd start by seeing if the breakage is reachable from any refs:

   git rev-list --objects --all >/dev/null


That command does succeed.


If that command succeeds, then all your refs are intact and the problem
is in the reflogs. You can try to figure out which, but I'd probably
just blow them all away:

   rm -rf .git/logs


Unfortunately, removing the logs directory does not fix "git gc".  So I 
restored it.


However I did find all of the bad SHAs in the HEAD logs of four of my 
worktrees.


All of those worktrees have directories in .git/worktrees/, but "git 
worktree list" does not show two of them.  "git worktree prune -v" 
displays and does nothing.  (I do not want to play with --expire, 
because I'd rather keep my other worktrees.)


I removed all of those worktrees' directories from .git/worktrees/, and 
now "git gc" succeeds.  I've also removed those worktrees' working 
directories, as I don't really need them anymore.


Thanks for your help!

I'm willing to chalk this up to bugs in the early worktree code, unless 
one of the CC'd worktree developers thinks otherwise.


An explicit "git worktree delete" command would be nice for manually 
cleaning things up.  It's easy to just delete the directory, but having 
a "delete" command gives the user assurance that they're not missing 
something.


M.


If the rev-list fails, then one or more branch is corrupted.
Unfortunately the usual efficient tools for asking "which branch
contains this object" are likely to be broken by the corruption. But you
can brute-force it, like:

   git for-each-ref --format='%(refname)' |
   while read ref; do
 git rev-list --objects "$ref" >/dev/null 2>&1 ||
 echo "$ref is broken"
   done

Hopefully that turns up only branches with little value, and you can
delete them:

   git update-ref -d $broken_ref

-Peff

[1] A note on my "you can't fix corruption by deleting objects".

 Since abcb86553d (pack-objects: match prune logic for discarding
 objects, 2014-10-15) , git-gc also traverses the history graph of
 unreachable but "recent" objects. This is to keep whole chunks of
 the history graph intact during the gc grace period (which is 2
 weeks by default). So object themselves _can_ be a source of
 traversal for git-gc.

 We do that traversal with the ignore_missing_links flag, so
 breakages in the unreachable objects _shouldn't_ cause what you're
 seeing. IIRC we did turn up a bug or two with ignore_missing_links.
 The only one I could find was a3ba6bf10a (revision.c: ignore broken
 tags with ignore_missing_links, 2017-05-20), which I think wouldn't
 generate the output you're seeing.

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Santiago Torres]

> If it's a small range of gnupg versions which fail badly when the GNUPGHOME
> dir is removed, then there's far less reason for git to do much more than
> make an effort to kill the agent.
> 

Yeah. FWIW, it may be reasonable to consider dropping the patch once we are
certain distros don't ship this range anymore.

> It seems like all the gnupg versions which may suffer from the bug also
> support gpgconf --kill, which would make any further change in the test to
> handle versions which lack the --kill option moot.
> 

This is also true. We should definitely not litter the stdout with ENOENT-like
error messages though...

Thanks again for catching this!
-Santiago.


signature.asc
Description: PGP signature

Re: Changing encoding of a file : What should happen to CRLF in file ?

[Torsten Bögershausen]

On 2017-11-14 13:31, Ashish Negi wrote:
> Hello
> 
> I have a cross platform project. I have a utf-16 file in it.
> I changed its encoding to urf-8 and committed. When i pulled the file
> in Linux, it shows that file is modified. This means that the commit
> which changed the encoding does not convert crlf to lf, when new
> format is text based (utf-8).
> 
> Steps to reproduce:
> 
>In windows :
> 
> Change encoding of file from utf-16 to utf-8.
> Commit the change.
> 
>In linux:
> 
> Pull your branch.
> You will see the issue of file being shown as modified even though
> you have not used it.
> 
> 
> If i change the file encoding in linux and commit. Then if i do git
> pull in windows, i don't see the issue.
> In linux, during committing i get warning : warning: CRLF will be
> replaced by LF in …file_name..
> 
> Here are my configuration :
> 
> 
>> git config --global --get core.autocrlf
> 
> false
> 
> 
>> git config  --get core.autocrlf
> 
> false
> 
> 
> 
>> cat E:\work\WindowsFabric\.gitattributes
> 
> 
> # Set the default behavior, in case people don't have core.autocrlf set.
> 
> * text=auto
> *.vcxproj eol=crlf
> *.sh  eol=lf
> 
> # Denote all files that are truly binary and should not be modified.
> *.exe binary
> *.dll binary
> *.pdb binary
> *.ico binary
> *.png binary
> *.jpg binary
> 
> 
>> git --version
> git version 2.14.2.windows.2
> 
> 
> I played around with core.autocrlf values like true and native, but
> that didn't help.
> 
> The behavior is inconsistent across platforms, and windows version is
> giving me problem.
> 
> Can someone suggest right settings for this ? Or is this a bug in Git.
> 

I don't think it is a bug :-)
What does
git ls-files --eol …file_name
give you under Windows ?

Re: [PATCH v2 1/2] branch: forbid refs/heads/HEAD

[Junio C Hamano]

Kaartic Sivaraam  writes:

> I should have been a little more clear with the numbering, sorry. The
> correct prefix should have been as follows,
>
>* [PATCH v2 1/2] --> [PATCH v2 3/3]
>
>* [PATCH v2 1/2] --> [PATCH v2 4/3]
>
> Sorry for the inconvenience.

I assume that the second one above actually talks about what was
sent as "v2 2/2" (not "v2 1/2") being "4/3"?

Are these two patches follow-up fixes (replacement of 3/3 plus an
extra patch) to jc/branch-name-sanity topic?

Thanks for working on these.

Re: [PATCH v1 1/4] fastindex: speed up index load through parallelization

[Junio C Hamano]

Ben Peart  writes:

> How about I add the logic to write out a special extension right
> before the SHA1 that contains an offset to the beginning of the
> extensions section.  I will also add the logic in do_read_index() to
> search for and load this special extension if it exists.
>
> This will provide a common framework for any future extension to take
> advantage of if it wants to be loaded/processed before or in parallel
> with the cache entries or other extensions.
>
> For all existing extensions that assume they are loaded _after_ the
> cache entries, in do_read_index() I'll add the logic to use the offset
> (if it exists) to adjust the src_offset and then load them normally.
>
> Given the IEOT extension is just another list of offsets into the
> index to enable out of order processing, I'll add those offsets into
> the same extension so that it is a more generic "table of contents"
> for the entire index.  This enables us to have common/reusable way to
> have random access to _all_ sections in the index while maintaining
> backwards comparability with the existing index formats and code.
>
> These additional offsets will initially only be used to parallelize
> the loading of cache entries and only if the user explicitly enables
> that option but I can think of other interesting uses for them in the
> future.

If we freeze the format of IEOT extension so that we can guarantee
that the very first version of Git that understands IEOT can always
find the beginning of extension section in an index file that was
written by future versions of Git, then I'm all for that plan, but
my impression was that you are planning to make incompatible change
in the future to IEOT, judging from the waythat IEOT records its own
version number in the section and the reader uses it to reject an
unknown one.

With that plan, what I suspect would happen is that a version of Git
that understands another optional extension section that wants to be
findable without scanning the main table and the then-current
version of IEOT would not be able to use an index file written by a
new version of Git that enhances the format of the IEOT extension
bumps its extension version.

And if that is the case I would have to say that I strongly suspect
that you would regret the design decision to mix it into IEOT.  That
is why I keep suggesting that the back pointer extension should be
on its own, minimizing what it does and minimizing the need to be
updated across versions of Git.

missing entries in "git help repository-layout"

[Robert P. J. Day]

  while writing up a cheat sheet with a visual layout of .git for my
students, i compared "git help repository-layout" to my git clone of
the kernel source tree, and noted the following things are not
mentioned in the repo layout help page -- i have no idea how complete
that page is supposed to be so it's entirely possible that these
omissions are deliberate:

  - ORIG_HEAD
  - FETCH_HEAD
  - description
  - gitk.cache
  - refs/
bisect
stash
  - logs/
HEAD
refs/
stash
remotes
  - worktrees//
commondir
HEAD
index
logs
ORIG_HEAD

  as i said, i have no idea whether the entries above are deliberately
omitted from the layout page, i just made a list of them.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [RFC 3/3] log: add an option to generate cover letter from a branch tip

[Junio C Hamano]

Nicolas Morey-Chaisemartin  writes:

>> So there probably needs a convention meant to be followed by human
>> users when writing cover letters, so a mechanical process can tell
>> which part of the text is to be made into the merge commit without
>> understanding human languages.
>
> In the long term, I agree this would be nice.  As a first step,
> could we force the --edit option when using --cover-at-tip ?  The
> basic merge message would come from the cover letter but
> can/should be edited to clear the extra stuff out.

Ah, "git merge" by default opens the editor these days, so there is
no need to do a special "force"-ing only when we are taking the
initial log message material from the empty commit at the tip.  So
that plan would work rather well, I would imagine.

Good thinking.  Thanks.

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Junio C Hamano]

Johan Herland  writes:

>> Sounds like a good plan.  If the message does move to stderr, there are also
>> a few tests in 3310 that need adjusted.  They presume an error message from
>> `git notes merge`, but they only redirect stdout to the output file.
>>
>> While I was bored, I prepared a commit with these changes and confirmed the
>> test suite passes, in case we get an ACK from Johan.
>
> ACK :-)
>
> Error messages should go to stderr, and redirection in the tests
> should be fixed.

Thanks, both.

Re: [PATCH v1 1/4] fastindex: speed up index load through parallelization

[Ben Peart]

On 11/13/2017 8:10 PM, Junio C Hamano wrote:

Ben Peart  writes:


The proposed format for extensions (ie having both a header and a
footer with name and size) in the current patch already enables having
multiple extensions that can be parsed forward or backward.  Any
extensions that would want to be parse-able in reverse would just all
need to be written one after the other after right before the trailing
SHA1 (and of course, include the proper footer).


In other words, anything that wants to be scannable from the tail is
welcome to reimplement the same validation structure used by IEOT to
check the section specific sanity constraint, and this series is not
interested in introducing a more general infrastructure to make it
easy for code that want to find where each extension section in the
file begins without pasing the body of the index.

I find it somewhat unsatisfactory in that it is a fundamental change
to allow jumping to the start of an extension section from the tail
that can benefit any future codepath, and have expected a feature
neutral extension whose sole purpose is to do so [*1*].

That way, extension sections whose names are all-caps can stay to be
optional, even if they allow locating from the tail of the file.  If
you require them to implement the same validation struture as IEOT
to perform section specific sanity constraint and also require them
to be placed consecutively at the end, the reader MUST know about
all such extensions---otherwise they cannot scan backwards and find
ones that appear before an unknown but optional one.  If you keep an
extension section at the end whose sole purpose is to point at the
beginning of extension sections, the reader can then scan forward as
usual, skipping over unknown but optional ones, and reading your
IEOT can merely be an user (and the first user) of that more generic
feature that is futureproof, no?




How about I add the logic to write out a special extension right before 
the SHA1 that contains an offset to the beginning of the extensions 
section.  I will also add the logic in do_read_index() to search for and 
load this special extension if it exists.


This will provide a common framework for any future extension to take 
advantage of if it wants to be loaded/processed before or in parallel 
with the cache entries or other extensions.


For all existing extensions that assume they are loaded _after_ the 
cache entries, in do_read_index() I'll add the logic to use the offset 
(if it exists) to adjust the src_offset and then load them normally.


Given the IEOT extension is just another list of offsets into the index 
to enable out of order processing, I'll add those offsets into the same 
extension so that it is a more generic "table of contents" for the 
entire index.  This enables us to have common/reusable way to have 
random access to _all_ sections in the index while maintaining backwards 
comparability with the existing index formats and code.


These additional offsets will initially only be used to parallelize the 
loading of cache entries and only if the user explicitly enables that 
option but I can think of other interesting uses for them in the future.

Re: [RFC 3/3] log: add an option to generate cover letter from a branch tip

[Nicolas Morey-Chaisemartin]

Le 14/11/2017 à 14:05, Junio C Hamano a écrit :
> Nicolas Morey-Chaisemartin  writes:
>
>> The triple dash is so that the diffstat/shortlog as not seen as
>> part of the cover letter.  As said in the cover letter for this
>> series, it kinda breaks legacy behaviour right now.  It should
>> either be printed only for cover-at-tip, or a new separator should
>> be added.
> This reminds me of a couple of random thoughts I had, so before I
> disconnect from my terminal and forget about them...
>
> [1] format-patch and am must round-trip.
>
> I mentioned four uses cases around the "cover letter at the
> tip" in my earlier message
>
> https://public-inbox.org/git/xmqqbmk68o9d@gitster.mtv.corp.google.com/
>
> Specifically, (2) we should be able to run "format-patch" and record
> the log message of the empty commit at the tip to the cover letter,
> and (4) we should be able to accept such output with "am" and end up
> with the same sequence of commits as the original (modulo committer
> identity and timestamps).  So from the output we produce with this
> step, "am" should be able to split the material that came from the
> original empty commit from the surrounding cruft like shortlog and
> diffstat.  The output format of this step needs to be designed with
> that in mind.

This should be the case with the current RFC (apart from the branch description 
which is kept at the moment).
Things like git am --signoff will break this of course.

>
> [2] reusing cover letter material in merge may not be ideal.
>
> When people write a cover letter, they write different things in it.
> What they wanted to achieve, why they chose the approach they took,
> how the series is organized, which part of the series they find iffy
> and/orneeds special attention from the reviewers, where to find the
> previous iteration, what changed since the previous iterations, etc.
>
> All of them are to help the reviewers, many of who have already
> looked at the previous rounds, to understand and judge this round of
> the submission.
>
> The message in a merge commit as a part of the final history,
> however, cannot refer to anything from "previous rounds", as the
> previous attempts are not part of the final history readers of "git
> log" can refer to whey they are trying to understand the merge.
> What exactly goes in a merge commit and how the messages are phrased
> may be different from project to project, but for this project, I've
> been trying to write them in an end-user facing terms, i.e. they are
> designed in such a way that "git log --first-parent --merges" can be
> read as if they were entries in the release notes, summarizing fixes
> and features by describing their user-visible effects.  This is only
> one part of what people write in their cover letters (i.e. "what
> they wanted to achive").
>
> So there probably needs a convention meant to be followed by human
> users when writing cover letters, so a mechanical process can tell
> which part of the text is to be made into the merge commit without
> understanding human languages.

In the long term, I agree this would be nice.
As a first step, could we force the --edit option when using --cover-at-tip ?
The basic merge message would come from the cover letter but can/should be 
edited to clear the extra stuff out.

Auto-stripping those extra infos, may also conflicts with the point (3) from 
your previous mail.
If git merge is to keep only the relevant infos, git format-patch from this 
merge will not be able to access those.

The advantage of a manual edit is that it's the commiter's choice. Keep the 
info if the branch is to be resubmitted. Drop them once it's merged for good.

Nicolas

Re: [RFC 3/3] log: add an option to generate cover letter from a branch tip

[Junio C Hamano]

Nicolas Morey-Chaisemartin  writes:

> The triple dash is so that the diffstat/shortlog as not seen as
> part of the cover letter.  As said in the cover letter for this
> series, it kinda breaks legacy behaviour right now.  It should
> either be printed only for cover-at-tip, or a new separator should
> be added.

This reminds me of a couple of random thoughts I had, so before I
disconnect from my terminal and forget about them...

[1] format-patch and am must round-trip.

I mentioned four uses cases around the "cover letter at the
tip" in my earlier message

https://public-inbox.org/git/xmqqbmk68o9d@gitster.mtv.corp.google.com/

Specifically, (2) we should be able to run "format-patch" and record
the log message of the empty commit at the tip to the cover letter,
and (4) we should be able to accept such output with "am" and end up
with the same sequence of commits as the original (modulo committer
identity and timestamps).  So from the output we produce with this
step, "am" should be able to split the material that came from the
original empty commit from the surrounding cruft like shortlog and
diffstat.  The output format of this step needs to be designed with
that in mind.

[2] reusing cover letter material in merge may not be ideal.

When people write a cover letter, they write different things in it.
What they wanted to achieve, why they chose the approach they took,
how the series is organized, which part of the series they find iffy
and/orneeds special attention from the reviewers, where to find the
previous iteration, what changed since the previous iterations, etc.

All of them are to help the reviewers, many of who have already
looked at the previous rounds, to understand and judge this round of
the submission.

The message in a merge commit as a part of the final history,
however, cannot refer to anything from "previous rounds", as the
previous attempts are not part of the final history readers of "git
log" can refer to whey they are trying to understand the merge.
What exactly goes in a merge commit and how the messages are phrased
may be different from project to project, but for this project, I've
been trying to write them in an end-user facing terms, i.e. they are
designed in such a way that "git log --first-parent --merges" can be
read as if they were entries in the release notes, summarizing fixes
and features by describing their user-visible effects.  This is only
one part of what people write in their cover letters (i.e. "what
they wanted to achive").

So there probably needs a convention meant to be followed by human
users when writing cover letters, so a mechanical process can tell
which part of the text is to be made into the merge commit without
understanding human languages.

Changing encoding of a file : What should happen to CRLF in file ?

[Ashish Negi]

Hello

I have a cross platform project. I have a utf-16 file in it.
I changed its encoding to urf-8 and committed. When i pulled the file
in Linux, it shows that file is modified. This means that the commit
which changed the encoding does not convert crlf to lf, when new
format is text based (utf-8).

Steps to reproduce:

   In windows :

Change encoding of file from utf-16 to utf-8.
Commit the change.

   In linux:

Pull your branch.
You will see the issue of file being shown as modified even though
you have not used it.


If i change the file encoding in linux and commit. Then if i do git
pull in windows, i don't see the issue.
In linux, during committing i get warning : warning: CRLF will be
replaced by LF in …file_name..

Here are my configuration :


> git config --global --get core.autocrlf

false


> git config  --get core.autocrlf

false



> cat E:\work\WindowsFabric\.gitattributes


# Set the default behavior, in case people don't have core.autocrlf set.

* text=auto
*.vcxproj eol=crlf
*.sh  eol=lf

# Denote all files that are truly binary and should not be modified.
*.exe binary
*.dll binary
*.pdb binary
*.ico binary
*.png binary
*.jpg binary


> git --version
git version 2.14.2.windows.2


I played around with core.autocrlf values like true and native, but
that didn't help.

The behavior is inconsistent across platforms, and windows version is
giving me problem.

Can someone suggest right settings for this ? Or is this a bug in Git.

Thanks

Re: [PATCH v2 1/2] branch: forbid refs/heads/HEAD

[Kaartic Sivaraam]

I should have been a little more clear with the numbering, sorry. The 
correct prefix should have been as follows,


   * [PATCH v2 1/2] --> [PATCH v2 3/3]

   * [PATCH v2 1/2] --> [PATCH v2 4/3]

Sorry for the inconvenience.


---
Kaartic

[PATCH v2 2/2] builtin/branch: remove redundant check for HEAD

[Kaartic Sivaraam]

The lower level code has been made to handle this case for the
sake of consistency. This has made this check redundant.

So, remove the redundant check.

Signed-off-by: Kaartic Sivaraam 
---
 builtin/branch.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ffd39333a..5fc57cdc9 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -793,9 +793,6 @@ int cmd_branch(int argc, const char **argv, const char 
*prefix)
} else if (argc > 0 && argc <= 2) {
struct branch *branch = branch_get(argv[0]);
 
-   if (!strcmp(argv[0], "HEAD"))
-   die(_("it does not make sense to create 'HEAD' 
manually"));
-
if (!branch)
die(_("no such branch '%s'"), argv[0]);
 
-- 
2.15.0.rc2.397.geff0134c7.dirty

[PATCH v2 1/2] branch: forbid refs/heads/HEAD

[Kaartic Sivaraam]

From: Junio C Hamano 

strbuf_check_branch_ref() is the central place where many codepaths
see if a proposed name is suitable for the name of a branch.  It was
designed to allow us to get stricter than the check_refname_format()
check used for refnames in general, and we already use it to reject
a branch whose name begins with a '-'.

Use it to also reject "HEAD" as a branch name.

Helped-by: Jeff King 
Signed-off-by: Junio C Hamano 
Signed-off-by: Kaartic Sivaraam 
---
  Changes in v2:

  * Fixed the issue of .git/HEAD being renamed when trying to do,

  $ git branch -m HEAD head

   This also allows to rename a branch named HEAD that was created by accident.

   cf. <1509209933.2256.4.ca...@gmail.com>

  * Added a test to ensure that it is possible to rename a branch named HEAD.

 sha1_name.c |  8 +++-
 t/t1430-bad-ref-name.sh | 29 +
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index 9a2d5caf3..657a060cb 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1438,9 +1438,15 @@ int strbuf_check_branch_ref(struct strbuf *sb, const 
char *name)
strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
else
strbuf_addstr(sb, name);
-   if (name[0] == '-')
+   if (*name == '-')
return -1;
+
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
+
+   /* HEAD is not to be used as a branch name */
+   if(!strcmp(sb->buf, "refs/heads/HEAD"))
+   return -1;
+
return check_refname_format(sb->buf, 0);
 }
 
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index e88349c8a..421e80a7a 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh
@@ -331,4 +331,33 @@ test_expect_success 'update-ref --stdin -z fails delete 
with bad ref name' '
grep "fatal: invalid ref format: ~a" err
 '
 
+test_expect_success 'branch rejects HEAD as a branch name' '
+   test_must_fail git branch HEAD HEAD^ &&
+   test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'checkout -b rejects HEAD as a branch name' '
+   test_must_fail git checkout -B HEAD HEAD^ &&
+   test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'update-ref can operate on refs/heads/HEAD' '
+   git update-ref refs/heads/HEAD HEAD^ &&
+   git show-ref refs/heads/HEAD &&
+   git update-ref -d refs/heads/HEAD &&
+   test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'branch -d can remove refs/heads/HEAD' '
+   git update-ref refs/heads/HEAD HEAD^ &&
+   git branch -d HEAD &&
+   test_must_fail git show-ref refs/heads/HEAD
+'
+
+test_expect_success 'branch -m can rename refs/heads/HEAD' '
+   git update-ref refs/heads/HEAD HEAD^ &&
+   git branch -m HEAD head &&
+   test_must_fail git show-ref refs/heads/HEAD
+'
+
 test_done
-- 
2.15.0.rc2.397.geff0134c7.dirty

Assalamu`Alaikum.

[Mohammad Ouattara]

Greetings from Dr. mohammad ouattara.
 
Assalamu`Alaikum.
 
My Name is Dr. mohammad ouattara, I am a banker by profession. I'm from 
Ouagadougou, Burkina Faso, West Africa. My reason for contacting you is to 
transfer an abandoned $14.6M to your account.
 
The owner of this fund died since 2004 with his Next Of Kin. I want to present 
you to the bank as the Next of Kin/beneficiary of this fund.
 
Further details of the transaction shall be forwarded to you as soon as I 
receive your return mail indicating your interest.

Have a great day,
Dr. mohammad ouattara.

Your long awaited part payment of $2.5.000.00Usd

[UNITED NATIONS]

Attention: Beneficiary,

Your long awaited part payment of $2.5.000.00Usd (TWO MILLION FIVE Hundred
Thousand United State Dollars)  is ready for immediate release to you, and
it was electronically credited into an ATM Visa Card for easy delivery.

Your new Payment Reference No.- 6363836,
Password No: 006786,
Pin Code No: 1787
Your Certificate of Merit Payment No: 05872,
Released Code No: 1134;
Immediate Telex confirmation No:- 043612;
Secret Code No: TBKTA28
Re-Confirm;
Your Names: |Address: |Cell Phone: |Fax: | Amount: |
Your immediate response is needed
Access Bank

Person to Contact:MR DANIEL ETIM  the Director of the International Audit
unit ATM Payment Center,

Email: bankplc12...@financier.com
TELEPHONE: +226 68699544

Note: Your file will expire after 7 days if there is no response, and then
we will not have any option than to return your fund to IMF as unclaimed.
Your swift response will enhance our service, thanks for your co-operation;

Regards.
Mr JOHN MARK.

Re: why can *some* git commands not be run from within .git directory?

[Robert P. J. Day]

On Tue, 14 Nov 2017, Bryan Turner wrote:

> On Tue, Nov 14, 2017 at 1:18 AM, Robert P. J. Day  
> wrote:
> >
> >   just noticed something i was unaware of -- some git commands can't
> > be run if i'm in the .git directory, while others can. for example,
> > if i "cd .git", commands like this work just fine:
> >
> >   $ git show
> >   $ git branch
> >   $ git log
> >
> > but others seem unwilling to determine the "working tree":
>
> Once Git finds a .git directory, or determines it's in one, it
> doesn't look "higher"; instead, it runs like it's in a bare clone.
> That means any command that requires a work tree or an index
> generally won't work. Of course, you can still tell Git where the
> work tree is and then the commands work fine from the .git
> directory:

  ah, thank you, that clarifies it.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: why can *some* git commands not be run from within .git directory?

[Bryan Turner]

On Tue, Nov 14, 2017 at 1:18 AM, Robert P. J. Day  wrote:
>
>   just noticed something i was unaware of -- some git commands can't
> be run if i'm in the .git directory, while others can. for example,
> if i "cd .git", commands like this work just fine:
>
>   $ git show
>   $ git branch
>   $ git log
>
> but others seem unwilling to determine the "working tree":

Once Git finds a .git directory, or determines it's in one, it doesn't
look "higher"; instead, it runs like it's in a bare clone. That means
any command that requires a work tree or an index generally won't
work. Of course, you can still tell Git where the work tree is and
then the commands work fine from the .git directory:

incom@Jael MINGW64 /c/Temp/example/.git (GIT_DIR!)
$ GIT_WORK_TREE=.. git status
On branch master
Your branch is up-to-date with 'origin/master'.

nothing to commit, working tree clean

>
>   $ git status
>   fatal: This operation must be run in a work tree
>   $
>
> and:
>
>   $ git stash list
>   fatal: /usr/libexec/git-core/git-stash cannot be used without a working 
> tree.
>   $
>
> what's the distinction between commands that can work this way, and
> commands that can't?
>
> rday
>
> p.s. i will refrain from pointing out the inconsistency in using the
> phrases "work tree" and "working tree" to mean the same thing, when
> there is a distinct "worktree" entity.

No you won't, clearly.

>
> --
>
> 
> Robert P. J. Day Ottawa, Ontario, CANADA
> 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__crashcourse.ca=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=yPGW_MCaDCBECSMjz40m8vsxUhljCB3dnrQ4TBi8PtE=-JNZXqYtyY7CIJGy65WWa88Kq7BaelyajFehPQZkvo4=
>
> Twitter:   
> https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter.com_rpjday=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=yPGW_MCaDCBECSMjz40m8vsxUhljCB3dnrQ4TBi8PtE=-9gIAxcdRGPmIz1rpLLb7nl14azEwoE-fOJ-IxAYi18=
> LinkedIn:   
> https://urldefense.proofpoint.com/v2/url?u=http-3A__ca.linkedin.com_in_rpjday=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=yPGW_MCaDCBECSMjz40m8vsxUhljCB3dnrQ4TBi8PtE=prnIhreU-Vhx0V6lRCBQEPbJk2P8bPlHcRFus1X_6wM=
> 

[PATCH] sha1: add gnutls as a sha1 provider

[Shawn Landden]

GNUTLS uses the same cryptograms SHA1 routines (Cryptograms)
by Andy Polyakov  as OpenSSL, but with a license
that is acceptable for downstream packagers.

This is not the cleanest way to use the GNUTLS library,
as it is reallocating the context every time, and GNUTLS itsself
fudges an OpenSSL CTX to use the cryptograms code, HOWEVER
in my benchmarks the code performs as well as both the OpenSSL library,
and my own integration of cryptograms with git.

I think this is preferrable to bringing the assembly routines into
the git code-base, as a way of getting access to these high-performance
routines to a git available in Debian, Ubuntu, or Fedora (which
all use BLK_SHA1=1 due to GPLv2 + OpenSSL license considerations,
see Debian Bug #879459).

I struggle with autotools, and I suspect something is wrong with that
part of the patch.

This laptop is ancient, Intel(R) Core(TM) i5 CPU M 520.
When I get arm64 hardware in a week I will update with new benchmarks.
Builtin (BLK_SHA1=1):
~/git/git$ time git fsck
Checking object directories: 100% (256/256), done.
Checking objects: 100% (238410/238410), done.
Checking connectivity: 236605, done.

real0m25.806s
user0m25.187s
sys 0m0.579s

This patch:
~/git/git$ time ./git fsck
Checking object directories: 100% (256/256), done.
Checking objects: 100% (238410/238410), done.
Checking connectivity: 236606, done.

real0m22.368s
user0m21.790s
sys 0m0.539s

Signed-off-by: Shawn Landden 
---
 Makefile   | 10 ++
 configure.ac   | 31 +++
 gnutls-sha1/sha1.c | 25 +
 gnutls-sha1/sha1.h | 12 
 hash.h |  2 ++
 5 files changed, 80 insertions(+)
 create mode 100644 gnutls-sha1/sha1.c
 create mode 100644 gnutls-sha1/sha1.h

diff --git a/Makefile b/Makefile
index cd7598599..e23648dbd 100644
--- a/Makefile
+++ b/Makefile
@@ -1252,7 +1252,9 @@ ifndef NO_OPENSSL
endif
 else
BASIC_CFLAGS += -DNO_OPENSSL
+ifndef GNUTLS_SHA1
BLK_SHA1 = 1
+endif
OPENSSL_LIBSSL =
 endif
 ifdef NO_OPENSSL
@@ -1481,6 +1483,11 @@ ifdef BLK_SHA1
LIB_OBJS += block-sha1/sha1.o
BASIC_CFLAGS += -DSHA1_BLK
 else
+ifdef GNUTLS_SHA1
+   LIB_OBJS += gnutls-sha1/sha1.o
+   BASIC_CFLAGS += -DSHA1_GNUTLS
+   EXTLIBS += -lgnutls
+endif
 ifdef PPC_SHA1
LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
BASIC_CFLAGS += -DSHA1_PPC
@@ -1488,6 +1495,8 @@ else
 ifdef APPLE_COMMON_CRYPTO
COMPAT_CFLAGS += -DCOMMON_DIGEST_FOR_OPENSSL
BASIC_CFLAGS += -DSHA1_APPLE
+else
+ifdef GNUTLS_SHA1
 else
DC_SHA1 := YesPlease
BASIC_CFLAGS += -DSHA1_DC
@@ -1506,6 +1515,7 @@ ifdef DC_SHA1_SUBMODULE
 else
LIB_OBJS += sha1dc/sha1.o
LIB_OBJS += sha1dc/ubc_check.o
+endif
 endif
BASIC_CFLAGS += \
-DSHA1DC_NO_STANDARD_INCLUDES \
diff --git a/configure.ac b/configure.ac
index 2f55237e6..109c4758d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -250,6 +250,23 @@ AS_HELP_STRING([--with-openssl],[use OpenSSL library 
(default is YES)])
 AS_HELP_STRING([],  [ARG can be prefix for openssl library and 
headers]),
 GIT_PARSE_WITH([openssl]))
 
+# Define GNUTLS_SHA1 if you have and want to use libgnutls. This offers
+# similar sha1 routines as openssl.
+AC_ARG_WITH(gnutls,
+AS_HELP_STRING([--with-gnutls],[use GNUTLS library (default is YES)]),
+if test "$withval" = "no"; then
+USE_GNUTLS=
+elif test "$withval" = "yes"; then
+   USE_GNUTLS=YesPlease
+else
+   USE_GNUTLS=YesPlease
+   LIBGNUTLSDIR=$withval
+   AC_MSG_NOTICE([Setting LIBGNUTLSDIR to $LIBGNUTLSDIR])
+dnl USE_LIBGNUTLS can still be modified below, so don't substitute
+dnl it yet.
+   GIT_CONF_SUBST([LIBGNUTLSDIR])
+fi)
+
 # Define USE_LIBPCRE if you have and want to use libpcre. Various
 # commands such as log and grep offer runtime options to use
 # Perl-compatible regular expressions instead of standard or extended
@@ -540,6 +557,20 @@ GIT_UNSTASH_FLAGS($OPENSSLDIR)
 GIT_CONF_SUBST([NEEDS_SSL_WITH_CRYPTO])
 GIT_CONF_SUBST([NO_OPENSSL])
 
+#
+# Handle USE_GNUTLS from above
+#
+if test -n "$USE_GNUTLS"; then
+
+GIT_STASH_FLAGS($LIBGNUTLSDIR)
+
+AC_CHECK_LIB([gnutls], [gnutls_hash_init],
+[GNUTLS_SHA1=YesPlease],
+[GNUTLS_SHA1=])
+
+GIT_UNSTASH_FLAGS($LIBGNUTLSDIR)
+
+fi
 #
 # Handle the USE_LIBPCRE1 and USE_LIBPCRE2 options potentially set
 # above.
diff --git a/gnutls-sha1/sha1.c b/gnutls-sha1/sha1.c
new file mode 100644
index 0..f7ede4ddf
--- /dev/null
+++ b/gnutls-sha1/sha1.c
@@ -0,0 +1,25 @@
+/* this is only to get definitions for memcpy(), ntohl() and htonl() */
+#include "../git-compat-util.h"
+
+#include 
+#include 
+
+#include "sha1.h"
+
+void gnutls_SHA1_Init(gnutls_SHA_CTX *ctx)
+{
+   int ret;
+   ret = gnutls_hash_init((void *) >handle, GNUTLS_DIG_SHA1);
+   if (ret < 0)
+   abort();
+}
+
+void 

Re: [PATCH] t/lib-gpg: fix gpgconf stderr redirect to /dev/null

[Johan Herland]

On Tue, Nov 14, 2017 at 6:15 AM, Todd Zullinger  wrote:
> Junio C Hamano wrote:
>>
>> The message goes to the standard output stream since it was introduced in
>> 809f38c8 ("git notes merge: Manual conflict resolution, part 1/2",
>> 2010-11-09) and 6abb3655 ("git notes merge:
>> Manual conflict resolution, part 2/2", 2010-11-09).  I do think it makes
>> more sense to send it to the standard error stream, but just in case if the
>> original author thinks of a reason why it shouldn't, let's summon Johan and
>> ask his input.
>
>
> Sounds like a good plan.  If the message does move to stderr, there are also
> a few tests in 3310 that need adjusted.  They presume an error message from
> `git notes merge`, but they only redirect stdout to the output file.
>
> While I was bored, I prepared a commit with these changes and confirmed the
> test suite passes, in case we get an ACK from Johan.

ACK :-)

Error messages should go to stderr, and redirection in the tests
should be fixed.

...Johan

-- 
Johan Herland, 
www.herland.net

Re: [RFC 3/3] log: add an option to generate cover letter from a branch tip

[Nicolas Morey-Chaisemartin]

Le 14/11/2017 à 07:14, Junio C Hamano a écrit :
> Nicolas Morey-Chaisemartin  writes:
>
>> -const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
>> -const char *msg;
>> +const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n\n";
> H.

The \n from fprintf(rev->diffopt.file, "%s\n", sb.buf); added an extra line 
after the cover which is fine for the default one but changed the commit value 
(at least at some point while I was playing with scissors)
so I moved it up there to avoid adding a if() around the fprintf

>
>> @@ -1021,17 +1021,21 @@ static void make_cover_letter(struct rev_info *rev, 
>> int use_stdout,
>>  if (!branch_name)
>>  branch_name = find_branch_name(rev);
>>  
>> -msg = body;
>>  pp.fmt = CMIT_FMT_EMAIL;
>>  pp.date_mode.type = DATE_RFC2822;
>>  pp.rev = rev;
>>  pp.print_email_subject = 1;
>> -pp_user_info(, NULL, , committer, encoding);
>> -pp_title_line(, , , encoding, need_8bit_cte);
>> -pp_remainder(, , , 0);
>> -add_branch_description(, branch_name);
>> -fprintf(rev->diffopt.file, "%s\n", sb.buf);
>>  
>> +if (!cover_at_tip_commit) {
>> +pp_user_info(, NULL, , committer, encoding);
>> +pp_title_line(, , , encoding, need_8bit_cte);
>> +pp_remainder(, , , 0);
>> +} else {
>> +pretty_print_commit(, cover_at_tip_commit, );
>> +}
>> +add_branch_description(, branch_name);
>> +fprintf(rev->diffopt.file, "%s", sb.buf);
>> +fprintf(rev->diffopt.file, "---\n", sb.buf);
>>  strbuf_release();
> I would have expected that this feature would not change anything
> other than replacing the constant string *body we unconditionally
> print with the log message of the empty commit at the tip, so from
> that expectation, I was hoping that a patch looked nothing more than
> this:
>
>  builtin/log.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 6c1fa896ad..0af19d5b36 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -986,6 +986,7 @@ static void make_cover_letter(struct rev_info *rev, int 
> use_stdout,
> struct commit *origin,
> int nr, struct commit **list,
> const char *branch_name,
> +   struct commit *cover,
> int quiet)
>  {
>   const char *committer;
> @@ -1021,7 +1022,10 @@ static void make_cover_letter(struct rev_info *rev, 
> int use_stdout,
>   if (!branch_name)
>   branch_name = find_branch_name(rev);
>  
> - msg = body;
> + if (cover)
> + msg = get_cover_from_commit(cover);
> + else
> + msg = body;
>   pp.fmt = CMIT_FMT_EMAIL;
>   pp.date_mode.type = DATE_RFC2822;
>   pp.rev = rev;
>
>
> plus a newly written function get_cover_from_commit().  Why does
> this patch need to change a lot more than that, I have to wonder.

The added code is to avoid the get_cover_from_commit generating a single strbuf 
that needs to be reparse/resplit by pp_user_info/pp_title_line/pp_remainder.
There was a helper that did the right job in my case so I took it ;)

The triple dash is so that the diffstat/shortlog as not seen as part of the 
cover letter.
As said in the cover letter for this series, it kinda breaks legacy behaviour 
right now.
It should either be printed only for cover-at-tip, or a new separator should be 
added.

>
> This is totally unrelated, but I wonder if it makes sense to do
> something similar for branch.description, too.  If the user has a
> meaningful description prepared with "git branch --edit-desc", it is
> somewhat insulting to the user to still add "*** BLURB HERE ***".

I guess so.
And the branch description should probably not be added when using cover-at-tip 
either.

[PATCH] t: correct obvious typo 'detahced'

[Robert P. J. Day]

Signed-off-by: Robert P. J. Day 

---

  by god, i'm going to get a patch into the code base if it kills me.

diff --git a/t/t7409-submodule-detached-work-tree.sh 
b/t/t7409-submodule-detached-work-tree.sh
index c20717181..fc018e363 100755
--- a/t/t7409-submodule-detached-work-tree.sh
+++ b/t/t7409-submodule-detached-work-tree.sh
@@ -6,7 +6,7 @@
 test_description='Test submodules on detached working tree

 This test verifies that "git submodule" initialization, update and addition 
works
-on detahced working trees
+on detached working trees
 '

 TEST_NO_CREATE_REPO=1

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

why can *some* git commands not be run from within .git directory?

[Robert P. J. Day]

  just noticed something i was unaware of -- some git commands can't
be run if i'm in the .git directory, while others can. for example,
if i "cd .git", commands like this work just fine:

  $ git show
  $ git branch
  $ git log

but others seem unwilling to determine the "working tree":

  $ git status
  fatal: This operation must be run in a work tree
  $

and:

  $ git stash list
  fatal: /usr/libexec/git-core/git-stash cannot be used without a working tree.
  $

what's the distinction between commands that can work this way, and
commands that can't?

rday

p.s. i will refrain from pointing out the inconsistency in using the
phrases "work tree" and "working tree" to mean the same thing, when
there is a distinct "worktree" entity.

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [RFC 2/3] am: semi working --cover-at-tip

[Nicolas Morey-Chaisemartin]

Le 14/11/2017 à 07:00, Junio C Hamano a écrit :
> Nicolas Morey-Chaisemartin  writes:
>
>>  if (!git_config_get_bool("commit.gpgsign", ))
>>  state->sign_commit = gpgsign ? "" : NULL;
>> +
>>  }
> Please give at least a cursory proof-reading before sending things
> out.
>
>> @@ -1106,14 +1131,6 @@ static void am_next(struct am_state *state)
>>  
>>  oidclr(>orig_commit);
>>  unlink(am_path(state, "original-commit"));
>> -
>> -if (!get_oid("HEAD", ))
>> -write_state_text(state, "abort-safety", oid_to_hex());
>> -else
>> -write_state_text(state, "abort-safety", "");
>> -
>> -state->cur++;
>> -write_state_count(state, "next", state->cur);
> Moving these lines to a later part of the source file is fine, but
> can you do so as a separate preparatory patch that does not change
> anything else?  That would unclutter the main patch that adds the
> feature, allowing better reviews from reviewers.
>
> The hunk below...

Sure. I usually do all this later in the process.
>> +/**
>> + * Increments the patch pointer, and cleans am_state for the application of 
>> the
>> + * next patch.
>> + */
>> +static void am_next(struct am_state *state)
>> +{
>> +struct object_id head;
>> +
>> +/* Flush the cover letter if needed */
>> +if (state->cover_at_tip == 1 &&
>> +state->series_len > 0 &&
>> +state->series_id == state->series_len &&
>> +state->cover_id > 0)
>> +do_apply_cover(state);
>> +
>> +am_clean(state);
>> +
>> +if (!get_oid("HEAD", ))
>> +write_state_text(state, "abort-safety", oid_to_hex());
>> +else
>> +write_state_text(state, "abort-safety", "");
>> +
>> +state->cur++;
>> +write_state_count(state, "next", state->cur);
>> +}
> ... if you followed that "separate preparatory step" approach, would
> show clearly that you added the logic to call do_apply_cover() when
> we transition after applying the Nth patch of a series with N patches,
> as all the existing lines will show only as unchanged context lines.

Agreed. The split of am_clean should probably have its own commit too.

>
> By the way, don't we want to sanity check state->last (which we
> learn by running "git mailsplit" that splits the incoming mbox into
> pieces and counts the number of messages) against state->series_len?
> Sometimes people send [PATCH 0-6/6], a 6-patch series with a cover
> letter, and then follow-up with [PATCH 7/6].  For somebody like me,
> it would be more convenient if the above code (more-or-less) ignored
> series_len and called do_apply_cover() after applying the last patch
> (which would be [PATCH 7/6]) based on what state->last says.

I thought about that.
Is there a use case for cover after the last patch works and removes the need 
to touch am_next (can be done out of the loop in am_run).

If that multiple series in a mbox is something people do, your concern could be 
solved by flushing the cover when state->series_id goes back to a lower value.

Nicolas

Re: [RFC 1/3] mailinfo: extract patch series id

[Nicolas Morey-Chaisemartin]

Le 14/11/2017 à 06:47, Junio C Hamano a écrit :
> Nicolas Morey-Chaisemartin  writes:
>
>> Extract the patch ID and series length from the [PATCH N/M]
>>  prefix in the mail header
>>
>> Signed-off-by: Nicolas Morey-Chaisemartin 
>> ---
>>  mailinfo.c | 35 +++
>>  mailinfo.h |  2 ++
>>  2 files changed, 37 insertions(+)
> As JTan already mentioned, relying on a substring "PATCH" may not be
> very reliable, and trying to locate "%d/%d]" feels like a better
> approach.
>
> cleanup_subject() is called only when keep_subject is false, so this
> code will not trigger in that case at all.  Is this intended?
>
> I would have expected that a new helper function would be written,
> without changing existing helpers like cleanup_subject(), and that
> new helper gets called by handle_info() after output_header_lines()
> helper is called for the "Subject".
Isn't that too late ?
If keep subject is not set, will cleanup_subject not drop all those info from 
the strbuf  ?
But yes it is not in the right function now.
But I would call the function before
            if (!mi->keep_subject) {

>
> Whenever mailinfo learns to glean a new useful piece of information,
> it should be made available to scripts that run "git mailinfo", too.
> Perhaps show something like
>
>   PatchNumber: 1
>   TotalPatches: 3
>
> at the end of handle_info() to mi->output?  I do not think existing
> tools mind too much, even if we added a for-debug output e.g.
>
>   RawSubject: [RFC 1/3] mailinfo: extract patch series id
>
> to the output.
Will do.

Re: man page for "git-worktree" is a bit confusing WRT "prune"

[Robert P. J. Day]

On Mon, 13 Nov 2017, Eric Sunshine wrote:

> On Mon, Nov 13, 2017 at 4:06 PM, Robert P. J. Day  
> wrote:

... snip ...

> >   how is this "expire time" measured? relative to what? i've
> > looked under .git/worktrees/, and i see a bunch of files
> > defining that worktree, but it's not clear how a worktree stores
> > the relevant time to be used for the determination of when a
> > worktree "expires".
>
> According to Documentation/gitrepository-layout.txt:
>
> worktrees//gitdir::
> A text file containing the absolute path back to the .git file
> that points to here. This is used to check if the linked
> repository has been manually removed and there is no need to
> keep this directory any more. The mtime of this file should be
> updated every time the linked repository is accessed.
>
> So, the expire time is relative to the mtime of the 'gitdir' file for
> that worktree...

  yup, that clears it up, thanks.

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCH v1 1/2] checkout: factor out functions to new lib file

[Thomas Gummerer]

On 11/13, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > Factor the functions out, so they can be re-used from other places.  In
> > particular these functions will be re-used in builtin/worktree.c to make
> > git worktree add dwim more.
> >
> > While there add a description to the function.
> >
> > Signed-off-by: Thomas Gummerer 
> > ---
> >
> > I'm not sure where these functions should go ideally, they don't seem
> > to fit in any existing library file, so I created a new one for now.
> > Any suggestions for a better home are welcome!
> >
> >  Makefile   |  1 +
> >  builtin/checkout.c | 41 +
> >  checkout.c | 43 +++
> >  checkout.h | 14 ++
> >  4 files changed, 59 insertions(+), 40 deletions(-)
> >  create mode 100644 checkout.c
> >  create mode 100644 checkout.h
> 
> I am not sure either.  I've always considered that entry.c is the
> libified thing codepath that want to do a "checkout" should call,
> but the functions that you are moving is at a "higher layer" that
> does not concern the core-git data structures (i.e. the 'tracking'
> is a mere "user" of the ref API, unlike things in entry.c such as
> checkout_entry() that is a more "maintainer" of the index integrity),
> so perhaps it makes sense to have new file for it.

Makes sense, let me keep it as a new file then.

> > diff --git a/checkout.c b/checkout.c
> > new file mode 100644
> > index 00..a9cf378453
> > --- /dev/null
> > +++ b/checkout.c
> > @@ -0,0 +1,43 @@
> > +#include "cache.h"
> > +
> 
> A useless blank line.
> 
> > +#include "remote.h"
> > +
> 
> This one is useful ;-)
> 
> > +struct tracking_name_data {
> > ...
> > diff --git a/checkout.h b/checkout.h
> > new file mode 100644
> > index 00..9272fe1449
> > --- /dev/null
> > +++ b/checkout.h
> > @@ -0,0 +1,14 @@
> > +#ifndef CHECKOUT_H
> > +#define CHECKOUT_H
> > +
> > +#include "git-compat-util.h"
> > +#include "cache.h"
> 
> Try "git grep -e git-compat-util Documentation/CodingGuidelines" and
> just keep inclusion of "cache.h".

Ah I forgot about that.  Will fix, thanks!

some apparent inaccuracies in "man git-worktree"

[Robert P. J. Day]

  more annoying pedantry (but you're used to that by now, right?) ...
from "man git-worktree", there seem to be some inaccuracies in the
SYNOPSIS regarding the "add" subcommand:

  git worktree add \
[-f] [--detach] [--checkout] [--lock] [-b ]  []

  first, there's no mention of "-B" in that SYNOPSIS, even though it's
explained further down the man page.

  next, the SYNOPSIS seems misleading as it doesn't make clear that
the options -b, -B and --detach are mutually exclusive, which is made
clear in the worktree.c source:

if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
die(_("-b, -B, and --detach are mutually exclusive"));

and, to some extent, further down the man page:

  "If  is omitted and neither -b nor -B nor --detach used,
   then, as a convenience, a new branch based at HEAD is created
   automatically, as if -b $(basename ) was specified."

  finally (and maybe i'm just not reading carefully enough), it's not
clear what happens if you add a worktree at a given commit without
specifying *any* of -b, -B or --detach. the obvious result should be a
new worktree checked out at a detached HEAD and, interestingly, if i
do that, then from the main tree, i see:

  $ git worktree list
  /home/rpjday/k/git   516fb7f2e73d [master]
  /home/rpjday/k/temp  c470abd4fde4 (detached HEAD)
  $

but from within the worktree, if i ask for the status, i see only:

  $ git status
  Not currently on any branch.
  nothing to commit, working tree clean
  $

where i would normally have expected to see "detached HEAD", is there
a reason that's not displayed?

rday

-- 


Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca

Twitter:   http://twitter.com/rpjday
LinkedIn:   http://ca.linkedin.com/in/rpjday

Re: [PATCH v1 2/2] worktree: make add dwim

[Thomas Gummerer]

On 11/13, Junio C Hamano wrote:
> Thomas Gummerer  writes:
> 
> > I'm a bit torn about hiding his behind an additional flag in git
> > worktree add or not.  I would like to have the feature without the
> > additional flag, but it might break some peoples expectations, so
> > dunno.
> 
> Yeah, I agree with the sentiment.  But what expectation would we be
> breaking, I wonder (see below)?
> 
> At the conceptual level, it is even more unfortunate that "git
> worktree --help" says this for "git worktree add  []":
> 
> If `` is omitted and neither `-b` nor `-B` nor
> `--detach` used, then, as a convenience, a new branch based at
> HEAD is created automatically, as if `-b $(basename )` was
> specified.
> 
> which means that it already does a DWIM, namely "since you didn't
> say what branch to create to track what other branch, we'll fork one
> for you from the HEAD, and give a name to it".  That may be a useful
> DWIM for some existing users sometimes, and you may even find it
> useful some of the time but not always.  Different people mean
> different things in different situations, and there is no single
> definition for DWIMming that would fit all of them.
> 
> Which in turn means that the new variable name DWIM_NEW_BRANCH and
> the new option name GUESS are way underspecified.  You'd need to
> name them after the "kind" of dwim; otherwise, not only the future
> additions for new (third) kind of dwim would become cumbersome, but
> readers of the code would be confused if they are about the existing
> dwim (fork a new branch from HEAD and give it a name guessed by the
> pathname) or your new dwim.

Yeah I definitely agree with that.  I was mainly thinking about my
personal use case with --guess, and didn't fully think through that it
might mean different things to other people.

> This also needs a documentation update.  Unlike the existing DWIM,
> it is totally unclear when you are dwimming in absence of which
> option.

Sorry about that, I totally forgot about the documentation for this.
Will add in the next round.

> I am guessing that, when you do not have a branch "topic" but your
> upstream does, saying
> 
> $ git worktree add ../a-new-worktree topic
> 
> would create "refs/heads/topic" to build on top of
> "refs/remotes/origin/topic" just like "git checkout topic" would.
> 
> IOW, when fully spelled out:
> 
> $ git branch -t -b topic origin/topic
> $ git worktree add ../a-new-worktree topic
> 
> is what your DWIM does?  Am I following you correctly?

It's not quite what my DWIM does/what I originally intended my DWIM to
do.  What it does is when

$ git worktree add ../topic

and omitting the branch argument, it would behave the way you spelled
out, i.e.

$ git branch -t -b topic origin/topic
$ git worktree add ../topic topic

instead of just checking it out from HEAD.  I must confess I missed
the branch argument, so that still behaves the old way with my code,
while what you have above would make a lot more sense.

> If so, as long as the new DWIM kicks in ONLY when "topic" does not
> exist, I suspect that there is no backward compatibility worries.
> The command line
> 
> $ git worktree add ../a-new-worktree topic
> 
> would just have failed because three was no 'topic' branch yet, no?

Indeed, with this there would not be any backwards compatility
worries.

Ideally I'd still like to make

$ git worktree add ../topic

work as described above, to avoid having to type 'topic' twice (the
directory name matches the branch name for the vast majority of my
worktrees) but that should then come behind a flag/config option?
Following your reasoning above it should probably be called something
other than --guess.

Maybe --guess-remote and worktree.guessRemote would do?  I'm quite bad
at naming though, so other suggestions are very welcome.

Private Investition

[Khvostova Zhanna]

Diese E-Mail fordert Sie strikt auf, sich für eine große Investition mit 
mir zusammenzutun, um weitere Informationen zu erhalten.

Mit freundlichen Grüßen,
Khvostova Zhanna