Re. Richard

2018-11-18 Thread Richard Thomas
Dear
I am a contractor with shell petroleum here in the Uk and want to
invest this $104M Dollars in real estate management and part in your
company or any other area of business you know best that will be of
good profit to both of us in your country

 kindly send to me your home or Office address,your phone and also
your ID for more details on how we can move forward

Waiting to here from you as soon as you receive this massage.

Thanks.
Richard Thomas


Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long

2018-11-18 Thread Torsten Bögershausen
On 2018-11-19 00:40, Junio C Hamano wrote:
> Derrick Stolee  writes:
> 
>>> This needs to go on top of pu, to cover all the good stuff
>>>cooking here.
>>
>> Better to work on top of 'master', as the work in 'pu' will be
>> rewritten several times, probably.
> 
> We may not be able to find any good moment to update some codepaths
> with deep callchains that reaches a basic API function that take
> ulong that way, as things are always in motion, but hopefully a lot
> of areas that need changes are rather isolated.  
> 
> For example, the changes I see around "offset" (which is "ulong" and
> the patch wants to change it to "size_t") in archive-tar.c in the
> patch do not have any interaction with the changes in this patch
> outside that single file, and I do not think any topic in-flight
> would interact with this change badly, either.  I didn't carefully
> look at the remainder of the patches, but I have a feeling that many
> can be separated out into independent and focused set of smaller
> patches that can be evaluated on their own.
> 

The archive-tar.c is actually a good example, why a step-by-step update
is not ideal (the code would not work any more on Win64).

If we look here:

static int stream_blocked(const struct object_id *oid)
{
struct git_istream *st;
enum object_type type;
size_t sz;
char buf[BLOCKSIZE];
ssize_t readlen;

st = open_istream(oid, , , NULL);
  ^
if (!st)
return error(_("cannot stream blob %s"), oid_to_hex(oid));
for (;;) {

The sz variable must follow whatever open_istream() uses, so if we start
with archive-tar.c, we must use either size_t or ulong, whatever
open_istream() needs. Otherwise things will break:
archive-tar.c uses ulong, open_istream() size_t, but we are passing pointers
around, and here  != _t

If we only update open_istream(), but not archive-tar.c, then
things are not better:
_t != 

I don't have a good idea how to split the patch.
However, "add a coccinelle script" may be a solution.


Re: Cygwin Git with Windows paths

2018-11-18 Thread Torsten Bögershausen
On 2018-11-19 04:33, Junio C Hamano wrote:
> "Randall S. Becker"  writes:
> 
>>> Torsten Bögershausen  writes:
>>>
 And it may even be that we need a special handling for the "\" to be
 treated as "/".
>>>
>>> I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do
>>> that.
>>
>> Heavy Cygwin user here. It is used in my environment for
>> cross-compilation. Everything should be done using / separators in
>> Cygwin, not \. So /cygdrive/c, /cygdrive/d always prefaces the
>> path rather than C:\ or D:\, which won't parse. It is,
>> essentially, a bash environment, including that git completions
>> work properly. Backslash ends up doing what it would in bash.
> 
> In short, in your opinion, the original message in this thread
> expresses an invalid wish, as C:\path\to\dir\ is not a valid way to
> spell the path to the directory, and it should be written as
> /cygdrive/c/path/to/dir instead?
> 
> How well does this argument work in the real world, when another
> claim in the original message
> 
> This causes problems for any non-Cygwin tools that might call Git:
> 
> http://github.com/golang/go/issues/23155
> 
> is taken into account, I wonder, though?
> 


Back to the original email, where the path embedded in ''
and the bash does not interpret the "\", I think.

>   $ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk'
>   Cloning into 'C:\cygwin64\tmp\goawk'...
>   fatal: Invalid path '/home/Steven/C:\cygwin64\tmp\goawk': No such file or
>   directory

>It seems the problem is that Git thinks the Windows form path is relative
>because it does not start with "/".

>A Git Bisect reveals this:
>05b458c104708141d2fad211d79703b3b99cc5a8 is the first bad commit
>commit 05b458c104708141d2fad211d79703b3b99cc5a8
>Author: Brandon Williams 
>Date:   Mon Dec 12 10:16:52 2016 -0800


The first question is, does this work under Git for Windows ?

Looking into 05b458c104708141d2fad, it seems as if the following functions
need to be "overridden" for cygwin, similar as we do it for mingw:
 is_dir_sep()
 offset_1st_component()
 find_last_dir_sep()


If nothing works,
it may help to add some fprintf(stderr,...) in the functions used
by 05b458c104708141d2f:

strip_last_component(),
get_next_component()
real_path_internal()


Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Junio C Hamano
Stefan Xenos  writes:

> The scenario you describe would not produce an origin edge in the
> metacommit graph. If the user amended X, there would be no origin
> edges - just a replacement. If you cherry-picked Z you'd get no
> replacements and just an origin. In neither case would you get both
> types of parent.

OK, that makes things a lot simpler.

I can see why we want to record "commit X obsoletes commit Y" to
help the "evolve" feature, which was the original motivation this
started the whole discussion.  But it is not immediately obvious to
me how it would help to have "Z was cherry-picked from W" in
"evolve".

The whole point of cherry-picking an old commit W to produce a new
commit Z is because the developer wanted to use the change between
W^ and W in a context that is quite different from W^, so it would
make no sense to "evolve" anything that was built on top of W on top
of Z.

It is of course OK to build a different feature that can take
advantage of the cherry-pick information on top of the same meta
commit concept in later steps, and to ensure that is doable, the
initial meta commit design must be done in a way that is flexible
enough to be extended, but it is not clear to me if this "origin"
thing is "while this does not have much to do with 'evolve', let's
throw in fields that would help another feature while we are at it"
or "in addition to 'X obsoletes Y', we need the cherry-pick
information for 'evolve' feature because..." (and because it is not
clear, I am assuming that it is the former).  If we can design the
"evolve" thing with only the "contents" and "obsoletes", that would
allow us to limit the scope of discussion we need to have around
meta commit and have something that works earlier, wouldn't it?

Thanks.


Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Junio C Hamano
Stefan Xenos  writes:

>> I meant the project's history, not the meta-graph thing.
>
> In that case, we agree. The proposal suggests that "origin" should be
> reachable from the meta-graph for the cherry-picked commit, NOT the
> cherry-picked commit itself. Does that resolve our disagreement, or is
> reachability from the meta-graph also undesirable for you?

Sorry, I confused myself.

Yes, I do mind that the "origin" thing in the meta history to pin
the old commit whose contents were cherry picked to create a new
commit, which is separate from the old commit that was rewritten to
create a new commit.  The latter (i.e. the old one) I do not mind to
get retrieved when such a meta commit is fetched, and all of us of
course would want the new one, too (which is the whole point of
adding the meta commit to help other commits built on the old one
migrate to the new one).  But I simply do not see the point of
having to drag the history leading to "origin", and that is why I am
moderately against recording "the change in this came from that
commit via cherry-pick" in a meta commit.


Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Stefan Xenos
> I meant the project's history, not the meta-graph thing.

In that case, we agree. The proposal suggests that "origin" should be
reachable from the meta-graph for the cherry-picked commit, NOT the
cherry-picked commit itself. Does that resolve our disagreement, or is
reachability from the meta-graph also undesirable for you?

> By having a "this was cherry-picked from that commit" in a commit
> that is not GC'ed, the original commit that has no longer have any
> relevance (because the newer one that is the result of the
> cherry-pick is the surviving version people will be building on) is
> kept reachable.  It is very much delierate that "cherry-pick -x"
> does not make the "origin" reachable and merely notes it in the
> human readable form that is ignored by the reachablity machinery.

Hmm. It sounds like you may be arguing against reachability from the
cherry-picked commit (which we agree on). I'm arguing for reachability
ONLY from the meta-graph. From your reply it's not completely clear to
me whether you also disapprove of reachability from the meta-graph or
if you thought the origin edges would be present on the cherry-picked
commit itself. Could you clarify? I suspect it may be the latter,
which suggests ambiguity in the proposal. If you could point to the
text that gave the impression origin parents would be present in the
cherry-picked commits themselves, I'll fix it.

> This is where we differ.  If commit X was rewritten (perhaps with
> help from change cherry-picked from commit Z, or without any) to
> produce Y, I do agree that it would be logical to keep X around
> until every dependent commit on it are migrated to be on top of Y.

The scenario you describe would not produce an origin edge in the
metacommit graph. If the user amended X, there would be no origin
edges - just a replacement. If you cherry-picked Z you'd get no
replacements and just an origin. In neither case would you get both
types of parent. I'd suggest we focus on the cherry-pick scenario
since it's the simplest real-world use case that produces origin
parents. All the more complex scenarios involving both parent types
only occur if you start from that simple case, so if you convince me
that the origin-only use case is unnecessary or undesirable, it would
also follow that the more complex origin-plus-obsolete-parent use case
is unnecessary.

So, if you don't mind - let me simplify that use-case: "If commit Z is
cherry-picked to produce Y, is there any need to keep Z around?". I
don't think we need X in the example to answer that question.

> But we do not need Z to transplant what used to be on X on top of Y,
> do we?

That's correct. The origin parent would be used to incorporate amended
versions of Z into Y, not to transplant things. It would also be used
to locate ancestors when merging code based on Z with code based on Y.

> So I do agree that in such a situation they want the
> relevant parts of the history retained, but I do not agree that
> "origin" is among them.

You may be entirely right, but at this point I'm not certain whether
we're disagreeing or miscommunicating. :-(


Re: Cygwin Git with Windows paths

2018-11-18 Thread Junio C Hamano
"Randall S. Becker"  writes:

>> Torsten Bögershausen  writes:
>> 
>> > And it may even be that we need a special handling for the "\" to be
>> > treated as "/".
>> 
>> I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do
>> that.
>
> Heavy Cygwin user here. It is used in my environment for
> cross-compilation. Everything should be done using / separators in
> Cygwin, not \. So /cygdrive/c, /cygdrive/d always prefaces the
> path rather than C:\ or D:\, which won't parse. It is,
> essentially, a bash environment, including that git completions
> work properly. Backslash ends up doing what it would in bash.

In short, in your opinion, the original message in this thread
expresses an invalid wish, as C:\path\to\dir\ is not a valid way to
spell the path to the directory, and it should be written as
/cygdrive/c/path/to/dir instead?

How well does this argument work in the real world, when another
claim in the original message

This causes problems for any non-Cygwin tools that might call Git:

http://github.com/golang/go/issues/23155

is taken into account, I wonder, though?


Git Test Coverage Report (v2.20.0-rc0)

2018-11-18 Thread Derrick Stolee
Here is a test coverage report for the uncovered lines introduced in 
v2.20.0-rc0 compared to v2.19.1.


Thanks,

-Stolee

[1] https://dev.azure.com/git/git/_build/results?buildId=263=logs

---


apply.c
eccb5a5f3d 4071) return get_oid_hex(p->old_oid_prefix, oid);
517fe807d6 4776) BUG_ON_OPT_NEG(unset);
735ca208c5 4830) return -1;

blame.c
a470beea39  113)  !strcmp(r->index->cache[-1 - pos]->name, path))
a470beea39  272) int pos = index_name_pos(r->index, path, len);
a470beea39  274) mode = r->index->cache[pos]->ce_mode;

builtin/add.c
d1664e73ad builtin/add.c 458) die(_("index file corrupt"));

builtin/am.c
2abf350385 1362) repo_init_revisions(the_repository, _info, NULL);
fce5664805 2117) *opt_value = PATCH_FORMAT_UNKNOWN;

builtin/blame.c
517fe807d6 builtin/blame.c    759) BUG_ON_OPT_NEG(unset);

builtin/cat-file.c
98f425b453 builtin/cat-file.c  56) die("unable to stream %s to stdout", 
oid_to_hex(oid));
0eb8d3767c builtin/cat-file.c 609) return error(_("only one batch option 
may be specified"));


builtin/checkout.c
fa655d8411 builtin/checkout.c  539) return 0;
fa655d8411 builtin/checkout.c  953) return error(_("index file corrupt"));

builtin/difftool.c
4a7e27e957 441) if (oideq(, ))

builtin/fast-export.c
4a7e27e957 builtin/fast-export.c  387) if (oideq(>oid, >oid) &&

builtin/fetch.c
builtin/fsck.c
b29759d89a builtin/fsck.c 613) fprintf(stderr, "Checking %s link\n", 
head_ref_name);

b29759d89a builtin/fsck.c 618) return error("Invalid %s", head_ref_name);
454ea2e4d7 builtin/fsck.c 769) for (p = get_all_packs(the_repository); p;
66ec0390e7 builtin/fsck.c 888) midx_argv[2] = "--object-dir";
66ec0390e7 builtin/fsck.c 889) midx_argv[3] = alt->path;
66ec0390e7 builtin/fsck.c 890) if (run_command(_verify))
66ec0390e7 builtin/fsck.c 891) errors_found |= ERROR_COMMIT_GRAPH;

builtin/gc.c
3029970275 builtin/gc.c 461) ret = error_errno(_("cannot stat '%s'"), 
gc_log_path);
3029970275 builtin/gc.c 470) ret = error_errno(_("cannot read '%s'"), 
gc_log_path);

fec2ed2187 builtin/gc.c 495) die(FAILED_RUN, pack_refs_cmd.argv[0]);
fec2ed2187 builtin/gc.c 498) die(FAILED_RUN, reflog.argv[0]);
3029970275 builtin/gc.c 585) exit(128);
fec2ed2187 builtin/gc.c 637) die(FAILED_RUN, repack.argv[0]);
fec2ed2187 builtin/gc.c 647) die(FAILED_RUN, prune.argv[0]);
fec2ed2187 builtin/gc.c 654) die(FAILED_RUN, prune_worktrees.argv[0]);
fec2ed2187 builtin/gc.c 658) die(FAILED_RUN, rerere.argv[0]);

builtin/grep.c
76e9bdc437 builtin/grep.c  424) grep_read_unlock();
fd6263fb73 builtin/grep.c 1051) warning(_("invalid option combination, 
ignoring --threads"));
fd6263fb73 builtin/grep.c 1057) die(_("invalid number of threads 
specified (%d)"), num_threads);


builtin/help.c
e6e76baaf4 builtin/help.c 429) if (!exclude_guides || alias[0] == '!') {
e6e76baaf4 builtin/help.c 430) printf_ln(_("'%s' is aliased to '%s'"), 
cmd, alias);

e6e76baaf4 builtin/help.c 431) free(alias);
e6e76baaf4 builtin/help.c 432) exit(0);
e6e76baaf4 builtin/help.c 441) fprintf_ln(stderr, _("'%s' is aliased to 
'%s'"), cmd, alias);

e6e76baaf4 builtin/help.c 442) count = split_cmdline(alias, );
e6e76baaf4 builtin/help.c 443) if (count < 0)
e6e76baaf4 builtin/help.c 444) die(_("bad alias.%s string: %s"), cmd,
e6e76baaf4 builtin/help.c 446) free(argv);
e6e76baaf4 builtin/help.c 448) return alias;

builtin/log.c
517fe807d6 builtin/log.c 1196) BUG_ON_OPT_NEG(unset);
2e6fd71a52 builtin/log.c 1472) die(_("failed to infer range-diff ranges"));
ee6cbf712e builtin/log.c 1818) die(_("--interdiff requires 
--cover-letter or single patch"));

8631bf1cdd builtin/log.c 1828) else if (!rdiff_prev)
8631bf1cdd builtin/log.c 1829) die(_("--creation-factor requires 
--range-diff"));
40ce41604d builtin/log.c 1833) die(_("--range-diff requires 
--cover-letter or single patch"));


builtin/multi-pack-index.c
6d68e6a461 35) usage_with_options(builtin_multi_pack_index_usage,
6d68e6a461 39) die(_("too many arguments"));
6d68e6a461 48) die(_("unrecognized verb: %s"), argv[0]);

builtin/pack-objects.c
6a22d52126 builtin/pack-objects.c 1091) continue;
2fa233a554 builtin/pack-objects.c 1512) hashcpy(base_oid.hash, base_sha1);
2fa233a554 builtin/pack-objects.c 1513) if 
(!in_same_island(>idx.oid, _oid))

2fa233a554 builtin/pack-objects.c 1514) return 0;
28b8a73080 builtin/pack-objects.c 2793) depth++;
108f530385 builtin/pack-objects.c 2797) oe_set_tree_depth(_pack, ent, 
depth);

454ea2e4d7 builtin/pack-objects.c 2981) p = get_all_packs(the_repository);

builtin/pack-redundant.c
454ea2e4d7 builtin/pack-redundant.c 580) struct packed_git *p = 
get_all_packs(the_repository);
454ea2e4d7 builtin/pack-redundant.c 595) struct packed_git *p = 
get_all_packs(the_repository);


builtin/pull.c
01a31f3bca 565) die(_("unable to access commit %s"),

builtin/rebase--interactive.c
53bbcfbde7 builtin/rebase--interactive2.c  24) return error(_("no HEAD?"));
53bbcfbde7 builtin/rebase--interactive2.c  51) return 
error_errno(_("could not create temporary %s"), path_state_dir());
53bbcfbde7 

[RFC PATCH v2] technical doc: add a design doc for the evolve command

2018-11-18 Thread sxenos
From: Stefan Xenos 

This document describes what a change graph for
git would look like, the behavior of the evolve command,
and the changes planned for other commands.

Signed-off-by: Stefan Xenos 
---
 Documentation/technical/change-graph.txt | 928 +++
 1 file changed, 928 insertions(+)
 create mode 100644 Documentation/technical/change-graph.txt

diff --git a/Documentation/technical/change-graph.txt 
b/Documentation/technical/change-graph.txt
new file mode 100644
index 00..2f4051f65f
--- /dev/null
+++ b/Documentation/technical/change-graph.txt
@@ -0,0 +1,928 @@
+Change Graph
+
+
+Objective
+=
+Track the amendments to a commit over time in a change graph. Allow users to
+exchange such change graphs, and introduce a new "evolve" command that
+uses the graph to rebase commits based on obsolete parents.
+
+Background
+==
+Imagine you have three sequential changes up for review and you receive 
feedback
+that requires editing all three changes. We'll define the word "change"
+formally later, but for the moment let's say that a change is a 
work-in-progress
+whose final version will be submitted as a commit in the future.
+
+While you're editing one change, more feedback arrives on one of the others.
+What do you do?
+
+The evolve command is a convenient way to work with chains of commits that are
+under review. Whenever you rebase or amend a commit, the repository remembers
+that the old commit is obsolete and has been replaced by the new one. Then, at
+some point in the future, you can run "git evolve" and the correct sequence of
+rebases will occur in the correct order such that no commit has an obsolete
+parent.
+
+Part of making the "evolve" command work involves tracking the edits to a 
commit
+over time, which is why we need an change graph. However, the change
+graph will also bring other benefits:
+
+- Users can view the history of a change directly (the sequence of amends and
+  rebases it has undergone, orthogonal to the history of the branch it is on).
+- It will be possible to quickly locate and list all the changes the user
+  currently has in progress.
+- It can be used as part of other high-level commands that combine or split
+  changes.
+- It can be used to decorate commits (in git log, gitk, etc) that are either
+  obsolete or are the tip of a work in progress.
+- By pushing and pulling the change graph, users can collaborate more
+  easily on changes-in-progress. This is better than pushing and pulling the
+  changes themselves since the change graph can be used to locate a more
+  specific merge base, allowing for better merges between different versions of
+  the same change. 
+- It could be used to correctly rebase local changes and other local branches
+  after running git-filter-branch.
+- It can replace the change-id footer used by gerrit.
+
+Goals
+-
+Legend: Goals marked with P0 are required. Goals marked with Pn should be
+attempted unless they interfere with goals marked with Pn-1.
+
+P0. All commands that modify commits (such as the normal commit --amend or
+rebase command) should mark the old commit as being obsolete and replaced 
by
+the new one. No additional commands should be required to keep the
+change graph up-to-date.
+P0. Any commit that may be involved in a future evolve command should not be
+garbage collected. Specifically:
+- Commits that obsolete another should not be garbage collected until
+  user-specified conditions have occurred and the change has expired from
+  the reflog. User specified conditions for removing changes include:
+  - The user explicitly deleted the change.
+  - The change was merged into a specific branch.
+- Commits that have been obsoleted by another should not be garbage
+  collected if any of their replacements are still being retained.
+P0. A commit can be obsoleted by more than one replacement (called divergence).
+P0. Must be able to resolve divergence (convergence).
+P1. Users should be able to share chains of obsolete changes in order to
+collaborate on WIP changes.
+P2. Such sharing should be at the user’s option. That is, it should be possible
+to directly share a change without also sharing the file states or commit
+comments from the obsolete changes that led up to it, and the choice not to
+share those commits should not require changing any commit hashes.
+P2. It should be possible to discard part or all of the change graph
+without discarding the commits themselves that are already present in
+branches and the reflog.
+P2. Provide sufficient information to replace gerrit's Change-Id footers.
+
+Similar technologies
+
+There are some other technologies that address the same end-user problem.
+
+Rebase -i can be used to solve the same problem, but users can't easily switch
+tasks midway through an interactive rebase or have more than one interactive
+rebase going on at the same 

Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Junio C Hamano
Stefan Xenos  writes:

>> And the other half is that while I consider the "origin" thing is
>> unnecessary for the above reasons, having it means we need to not
>> just transfer the history reading to aa7ce555 and d664309ee (which
>> are necessary anyway while we have histories to transplant from
>> d664309ee to aa7ce555) but also have to pull in the history leading
>> to 7e1bbcd and we cannot discard it.
>
> I'll assume that by "history" you're referring to the change graph
> (the metacommits) and not the branches (the commits), which would have
> no origin edges or connection between replacements.

I meant the project's history, not the meta-graph thing.

By having a "this was cherry-picked from that commit" in a commit
that is not GC'ed, the original commit that has no longer have any
relevance (because the newer one that is the result of the
cherry-pick is the surviving version people will be building on) is
kept reachable.  It is very much delierate that "cherry-pick -x"
does not make the "origin" reachable and merely notes it in the
human readable form that is ignored by the reachablity machinery.

> If the user has kept a change around in their metas namespace, it's an
> indication that they (or their collaborators) are still working on it
> and want its history to be retained.

This is where we differ.  If commit X was rewritten (perhaps with
help from change cherry-picked from commit Z, or without any) to
produce Y, I do agree that it would be logical to keep X around
until every dependent commit on it are migrated to be on top of Y.
But we do not need Z to transplant what used to be on X on top of Y,
do we?  So I do agree that in such a situation they want the
relevant parts of the history retained, but I do not agree that
"origin" is among them.

Side note.  As long as we have commits yet to be migrated to
be on Y that still is on X, ew do not need the meta-commit
to be protecting from getting GC'ed, as X is reachable from
these "need to be updated" branch tips anyway.  What we gain
from extra reachability brought in by the meta commits is
that by fetching the "change", we get Y (and its anestors),
even if we are not following any branch that contains it, so
that we can migrate those that are still based on X to it.






Re: [RFC PATCH v2] technical doc: add a design doc for the evolve command

2018-11-18 Thread Stefan Xenos
Sorry, that should have been uploaded with a subject starting with
[RFC PATCH v2]. I'm not sure why git send-email ignored my argument to
--subject-prefix. I'm going to try to change the subject one more
time. Please excuse me in advance if I accidentally spam the list by
sending the same email again.
On Sun, Nov 18, 2018 at 6:02 PM  wrote:
>
> From: Stefan Xenos 
>
> This document describes what a change graph for
> git would look like, the behavior of the evolve command,
> and the changes planned for other commands.
>
> Signed-off-by: Stefan Xenos 
> ---
>  Documentation/technical/change-graph.txt | 928 +++
>  1 file changed, 928 insertions(+)
>  create mode 100644 Documentation/technical/change-graph.txt
>
> diff --git a/Documentation/technical/change-graph.txt 
> b/Documentation/technical/change-graph.txt
> new file mode 100644
> index 00..2f4051f65f
> --- /dev/null
> +++ b/Documentation/technical/change-graph.txt
> @@ -0,0 +1,928 @@
> +Change Graph
> +
> +
> +Objective
> +=
> +Track the amendments to a commit over time in a change graph. Allow users to
> +exchange such change graphs, and introduce a new "evolve" command that
> +uses the graph to rebase commits based on obsolete parents.
> +
> +Background
> +==
> +Imagine you have three sequential changes up for review and you receive 
> feedback
> +that requires editing all three changes. We'll define the word "change"
> +formally later, but for the moment let's say that a change is a 
> work-in-progress
> +whose final version will be submitted as a commit in the future.
> +
> +While you're editing one change, more feedback arrives on one of the others.
> +What do you do?
> +
> +The evolve command is a convenient way to work with chains of commits that 
> are
> +under review. Whenever you rebase or amend a commit, the repository remembers
> +that the old commit is obsolete and has been replaced by the new one. Then, 
> at
> +some point in the future, you can run "git evolve" and the correct sequence 
> of
> +rebases will occur in the correct order such that no commit has an obsolete
> +parent.
> +
> +Part of making the "evolve" command work involves tracking the edits to a 
> commit
> +over time, which is why we need an change graph. However, the change
> +graph will also bring other benefits:
> +
> +- Users can view the history of a change directly (the sequence of amends and
> +  rebases it has undergone, orthogonal to the history of the branch it is 
> on).
> +- It will be possible to quickly locate and list all the changes the user
> +  currently has in progress.
> +- It can be used as part of other high-level commands that combine or split
> +  changes.
> +- It can be used to decorate commits (in git log, gitk, etc) that are either
> +  obsolete or are the tip of a work in progress.
> +- By pushing and pulling the change graph, users can collaborate more
> +  easily on changes-in-progress. This is better than pushing and pulling the
> +  changes themselves since the change graph can be used to locate a more
> +  specific merge base, allowing for better merges between different versions 
> of
> +  the same change.
> +- It could be used to correctly rebase local changes and other local branches
> +  after running git-filter-branch.
> +- It can replace the change-id footer used by gerrit.
> +
> +Goals
> +-
> +Legend: Goals marked with P0 are required. Goals marked with Pn should be
> +attempted unless they interfere with goals marked with Pn-1.
> +
> +P0. All commands that modify commits (such as the normal commit --amend or
> +rebase command) should mark the old commit as being obsolete and 
> replaced by
> +the new one. No additional commands should be required to keep the
> +change graph up-to-date.
> +P0. Any commit that may be involved in a future evolve command should not be
> +garbage collected. Specifically:
> +- Commits that obsolete another should not be garbage collected until
> +  user-specified conditions have occurred and the change has expired from
> +  the reflog. User specified conditions for removing changes include:
> +  - The user explicitly deleted the change.
> +  - The change was merged into a specific branch.
> +- Commits that have been obsoleted by another should not be garbage
> +  collected if any of their replacements are still being retained.
> +P0. A commit can be obsoleted by more than one replacement (called 
> divergence).
> +P0. Must be able to resolve divergence (convergence).
> +P1. Users should be able to share chains of obsolete changes in order to
> +collaborate on WIP changes.
> +P2. Such sharing should be at the user’s option. That is, it should be 
> possible
> +to directly share a change without also sharing the file states or commit
> +comments from the obsolete changes that led up to it, and the choice not 
> to
> +share those commits should not require changing any commit hashes.
> 

RE: Cygwin Git with Windows paths

2018-11-18 Thread Randall S. Becker
> -Original Message-
> From: git-ow...@vger.kernel.org  On Behalf Of
> Junio C Hamano
> Sent: November 18, 2018 19:07
> To: Torsten Bögershausen 
> Cc: Steven Penny ; git@vger.kernel.org
> Subject: Re: Cygwin Git with Windows paths
> 
> Torsten Bögershausen  writes:
> 
> > And it may even be that we need a special handling for the "\" to be
> > treated as "/".
> 
> I do not do Windows, but is_dir_sep() needs to be tweaked if you want to do
> that.

Heavy Cygwin user here. It is used in my environment for cross-compilation. 
Everything should be done using / separators in Cygwin, not \. So /cygdrive/c, 
/cygdrive/d always prefaces the path rather than C:\ or D:\, which won't parse. 
It is, essentially, a bash environment, including that git completions work 
properly. Backslash ends up doing what it would in bash.



[RFC PATCH v2] technical doc: add a design doc for the evolve command

2018-11-18 Thread sxenos
From: Stefan Xenos 

This document describes what a change graph for
git would look like, the behavior of the evolve command,
and the changes planned for other commands.

Signed-off-by: Stefan Xenos 
---
 Documentation/technical/change-graph.txt | 928 +++
 1 file changed, 928 insertions(+)
 create mode 100644 Documentation/technical/change-graph.txt

diff --git a/Documentation/technical/change-graph.txt 
b/Documentation/technical/change-graph.txt
new file mode 100644
index 00..2f4051f65f
--- /dev/null
+++ b/Documentation/technical/change-graph.txt
@@ -0,0 +1,928 @@
+Change Graph
+
+
+Objective
+=
+Track the amendments to a commit over time in a change graph. Allow users to
+exchange such change graphs, and introduce a new "evolve" command that
+uses the graph to rebase commits based on obsolete parents.
+
+Background
+==
+Imagine you have three sequential changes up for review and you receive 
feedback
+that requires editing all three changes. We'll define the word "change"
+formally later, but for the moment let's say that a change is a 
work-in-progress
+whose final version will be submitted as a commit in the future.
+
+While you're editing one change, more feedback arrives on one of the others.
+What do you do?
+
+The evolve command is a convenient way to work with chains of commits that are
+under review. Whenever you rebase or amend a commit, the repository remembers
+that the old commit is obsolete and has been replaced by the new one. Then, at
+some point in the future, you can run "git evolve" and the correct sequence of
+rebases will occur in the correct order such that no commit has an obsolete
+parent.
+
+Part of making the "evolve" command work involves tracking the edits to a 
commit
+over time, which is why we need an change graph. However, the change
+graph will also bring other benefits:
+
+- Users can view the history of a change directly (the sequence of amends and
+  rebases it has undergone, orthogonal to the history of the branch it is on).
+- It will be possible to quickly locate and list all the changes the user
+  currently has in progress.
+- It can be used as part of other high-level commands that combine or split
+  changes.
+- It can be used to decorate commits (in git log, gitk, etc) that are either
+  obsolete or are the tip of a work in progress.
+- By pushing and pulling the change graph, users can collaborate more
+  easily on changes-in-progress. This is better than pushing and pulling the
+  changes themselves since the change graph can be used to locate a more
+  specific merge base, allowing for better merges between different versions of
+  the same change. 
+- It could be used to correctly rebase local changes and other local branches
+  after running git-filter-branch.
+- It can replace the change-id footer used by gerrit.
+
+Goals
+-
+Legend: Goals marked with P0 are required. Goals marked with Pn should be
+attempted unless they interfere with goals marked with Pn-1.
+
+P0. All commands that modify commits (such as the normal commit --amend or
+rebase command) should mark the old commit as being obsolete and replaced 
by
+the new one. No additional commands should be required to keep the
+change graph up-to-date.
+P0. Any commit that may be involved in a future evolve command should not be
+garbage collected. Specifically:
+- Commits that obsolete another should not be garbage collected until
+  user-specified conditions have occurred and the change has expired from
+  the reflog. User specified conditions for removing changes include:
+  - The user explicitly deleted the change.
+  - The change was merged into a specific branch.
+- Commits that have been obsoleted by another should not be garbage
+  collected if any of their replacements are still being retained.
+P0. A commit can be obsoleted by more than one replacement (called divergence).
+P0. Must be able to resolve divergence (convergence).
+P1. Users should be able to share chains of obsolete changes in order to
+collaborate on WIP changes.
+P2. Such sharing should be at the user’s option. That is, it should be possible
+to directly share a change without also sharing the file states or commit
+comments from the obsolete changes that led up to it, and the choice not to
+share those commits should not require changing any commit hashes.
+P2. It should be possible to discard part or all of the change graph
+without discarding the commits themselves that are already present in
+branches and the reflog.
+P2. Provide sufficient information to replace gerrit's Change-Id footers.
+
+Similar technologies
+
+There are some other technologies that address the same end-user problem.
+
+Rebase -i can be used to solve the same problem, but users can't easily switch
+tasks midway through an interactive rebase or have more than one interactive
+rebase going on at the same 

Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

2018-11-18 Thread Junio C Hamano
Jeff King  writes:

> And it seems we worked around this in de2f95ebed (mailmap: work around
> implementations with pure inline strcasecmp, 2013-09-12). So I don't
> think there is any blocker there.
>
> (Though of course I have no idea on other portability questions around
> _stricmp(); I'll leave that for Windows folks).

Likewise.  As to the placement for the replacement #define, the
patch puts it where the inline version was, but I would think it
would work better if it were in the block of #defines, immediately
next to #define strncasecmp above.
.




Re: [PATCH 0/5] Make :(attr) pathspec work with "git log"

2018-11-18 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> But this also reveals an interesting thing: even though we walk on a
> tree, we check attributes from _worktree_ (and optionally fall back to
> the index). This is how attributes are implemented since forever. I
> think this is not a big deal if we communicate clearly with the user.

This is pretty much deliberately designed to be so; the set of the
attributes in HEAD may be stale but by taking the contents from the
working tree the user can work it around.

> The main patch is the last one. The others are just to open a path to
> pass "struct index_state *" down to tree_entry_interesting(). This may
> become standard procedure because we don't want to stick the_index (or
> the_repository) here and there.

Yup.



Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Stefan Xenos
> Am I correct to understand that the reason why a commit object is
> (ab|re)used to represent a meta-commit is because by doing so we
> would get connectivity (i.e. fetching & pushing would transfer all
> the associated objects along) for free, and by not representing it
> as a new and different object type, existing implementations can
> just pass them along without understanding what they are, and as
> long as these are not mixed as parts of the main history of the
> project (e.g. when enumerating commits that has aa7ce5 as its
> parents, because somebody else obsoleted aa7ce5 and you want to
> evolve anything that built on it, you do not want to mistake the
> above "meta" commit as a commit that is part of the ordinary history
> and rebuild on top of the new version of aa7ce5, which would lead to
> a disaster), everything would work just fine?

Yes, sir. That's it exactly. My first draft of the proposal suggested
creating a new top-level object type, but when I started digging
through the code I realized that the new object was so similar to a
commit that there was no need for a new type.

> Perhaps you'd use something like "presence of parent-type header
> marks that a commit is a meta-commit and not part of the main
> history".

Yes, that's called out explicitly as part of the proposal (see the
first sentence in the Parent-type subsection). Fsck would enforce this
invariant.

> How are these meta commits anchored so that it won't be reclaimed by
> repack?

They would either be anchored by a ref in the metas/ namespace (for
active changes currently under consideration by evolve) or by the
reflog (for recently deleted changes).

> I do not see any "parent" field used to chain them together,

They point to one another using the usual "parent" field present in
all commit objects. For an example of what the raw struct would look
like with parent pointers, see the top of the "Detailed design"
section or search the doc for the string . For
examples of how the metacommits in a change graph would be connected
after various operations, see the "Commit" section and the "Merge"
section. Please let me know if any of these examples are
insufficiently explained or if there's any other examples you'd like
to see.

> but I do not think we can afford to spend one ref per meta
> commit, as refs are not designed to point into each and every object
> in the repository.

Agreed. This is actually one of the reasons I'm proposing the use of
chains of meta-commits as opposed to using a purely ref-based
approach. I describe several other ref-based approaches in the "Other
options considered" section, and I made essentially the same point
there. We only create refs in the metas/ namespace to point to the
head of each change, and the rest of the commits and metacommits used
by the graph are reached via the parent pointers in the metacommits.

> I have a moderately strong opposition against "origin" thing.  If
> aa7ce555 replaces d664309ee, in order for the tool to be able to
> "evolve" other histories that build on top of d664309ee, it only
> needs the history between aa7ce555 and d664309ee and it would not
> matter how aa7ce555 was built relative to its parent.

I see I haven't justified the "origin" thing well enough. I'll
elaborate in the document, but here's the short version. The "origin"
edges are needed to address several use-cases:

1. Gerrit needs to know about cherry picks.

This is one of the lesser-known things that it uses the change-id
footers for and if we want to be able to eliminate the gerrit
change-id footers we need to record and communicate information about
cherry-picks somehow. This is the main reason for the origin edges -
the early drafts of this proposal didn't have them but it came up when
I asked a kind Gerrit maintainer to whether the proposal would be
sufficient to eliminate gerrit's change-ids. However there may be
alternatives I didn't think of. If we were to omit the origin edges,
can you suggest an alternative way for git to record the fact that one
commit was cherry-picked from another and communicate this fact to
gerrit?

I see that I forgot to call out "replacing gerrit change-ids" as an
explicit goal. I'll add that to the doc.

2. Obsolescence across cherry-picks.

In your example, it *may* actually matter how aa7ce55 was constructed.
One such scenario is what I'm calling obsolescence across
cherry-picks. Let me describe the use-case for it:

Alice creates commit A1.

Bob cherry-picks A1 to another branch, producing B1. At this point,
Bob has a metacommit saying that A1 is the origin of B1.

Alice amends A1, producing A2. She shares this with Bob.

At this point, Bob probably wants to amend B1 to include whatever
bugfix Alice did in A2 since the thing he cherry-picked is now out of
date. That's what the obsolescence across cherry-picks feature does.
If bob runs evolve with this option enabled, the evolve command will
produce B2 by amending B1 with whatever diff Alice did between A1 and

Re: Cygwin Git with Windows paths

2018-11-18 Thread Junio C Hamano
Torsten Bögershausen  writes:

> And it may even be that we need a special handling for the "\" to be treated
> as "/".

I do not do Windows, but is_dir_sep() needs to be tweaked if you
want to do that.


Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long

2018-11-18 Thread Junio C Hamano
Derrick Stolee  writes:

>> This needs to go on top of pu, to cover all the good stuff
>>cooking here.
>
> Better to work on top of 'master', as the work in 'pu' will be
> rewritten several times, probably.

We may not be able to find any good moment to update some codepaths
with deep callchains that reaches a basic API function that take
ulong that way, as things are always in motion, but hopefully a lot
of areas that need changes are rather isolated.  

For example, the changes I see around "offset" (which is "ulong" and
the patch wants to change it to "size_t") in archive-tar.c in the
patch do not have any interaction with the changes in this patch
outside that single file, and I do not think any topic in-flight
would interact with this change badly, either.  I didn't carefully
look at the remainder of the patches, but I have a feeling that many
can be separated out into independent and focused set of smaller
patches that can be evaluated on their own.



Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Junio C Hamano
Stefan Xenos  writes:

>> I don't think this counts as a typical modification and is probably hard to 
>> detect automatically.
>
> Clever use of commands! (side: wouldn't it just be easier to just use
> git commit --amend, though?)

When an original commit is mostly an early part of a feature, mixed
with a small but an urgent bugfix, it is not unusual to start your
work from "reset HEAD^" (or "reset --soft HEAD^") and recreate a
commit that has the main part of the change from the original,
leaving the remainder in the working tree to be worked into another
bugfix commit, most likely to be on a new branch forked from an
earlier point in the history, i.e.

git reset HEAD^
git add -p
git commit -c @{1}
git checkout -m -b a-small-bugfix-split-out master
edit
git commit -a

I agree with both of you that we want to have a way to mark that the
first commit we made by partially committing what was in the
original came from the original one, and also that the second one
has contents from the same original one.

It is unclear, without human involvement, if we can mechanically
infer that anything that used to be built on top of the original
commit would want to be rebuilt on top of the first half of the
split commit (i.e. the early part of the feature with the bugfix
separated out) but not on the other half (i.e. the bugfix alone).


Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Stefan Xenos
> This breaks the "git change" symmetry with "git branch", but after
> responding to other messages regarding that command, I'm starting to
> think that's not really a problem.

Sorry, I appended that sentence to the wrong paragraph. It should have
gone with the previous one that regarding the "git change" command.
On Sun, Nov 18, 2018 at 2:27 PM Stefan Xenos  wrote:
>
> > I don't think this counts as a typical modification and is probably hard to 
> > detect automatically.
>
> Clever use of commands! (side: wouldn't it just be easier to just use
> git commit --amend, though?)
>
> Either way, I agree that there should be a way to manually create a
> change graph or modify one into any possible shape. I've updated the
> "change" command to do what you want - the new version will have
> subcommands for creating arbitrary change graphs.
>
> > subject line will change over time and the original one may become 
> > irrelevant.
>
> There's a section on change naming further down the document. My
> criteria for name selection was that good names should be unique,
> short to type, and memorable to the user. Being relevant to the commit
> wasn't actually a requirement for me except insofar as it helps make
> them memorable. If we agree that these are reasonable criteria, commit
> hashes wouldn't be as good a choice since they'd satisfy the
> uniqueness criteria but would not be short or memorable. I expect that
> whatever criteria we select probably won't be optimal for all users
> which is why the design also includes a new hook for name selection. I
> believe that selected words from the commit comment should cover all
> three criteria in the majority of cases, and that the hook and the
> "change rename" command should cover the remaining corner cases. This
> breaks the "git change" symmetry with "git branch", but after
> responding to other messages regarding that command, I'm starting to
> think that's not really a problem.
>
> > How do we group changes of a topic together? I think branch-diff could take 
> > advantage of that.
>
> Could you clarify your use-case for me? I'm not sure what you mean by
> "changes of a topic". Are you referring to gerrit topics here? Topic
> branches? Or are you asking for some way for end-users to classify and
> organize their unsubmitted changes?
>
> > Could we just organize it like a normal history?
> > Basically all commits will be linked in a new merge history.
>
> From what I can tell, you're suggesting the following changes:
> 1. Reorder the parents such that the content parent comes last rather
> than first.
> 2. Move parent-type from the structured portion of the header to the
> unstructured portion of the commit message.
>
> I'm fine with 1 if that makes something easier.
>
> Regarding 2, I can see some good reasons to put parent-type in the
> header rather than the user-readable portion of the commit message
> - fsck can rely on them when checking the database for validity (for
> example, it can assert that the current repository version doesn't
> attach a non-empty tree, that the content parent always points to a
> real commit, the commit message is empty, that the number of
> parent-types matches the number of parents, that the enum values are
> valid, that the parent orders are correct, etc.).
> - accidental collisions are impossible (users can't accidentally
> corrupt their database by adding or removing the word "parent-type" in
> a commit message).
> - it doesn't spam the user-readable region with machine-readable
> repository internals.
>
> > This makes it possible to just use "git log --first-parent
> > --patch" (or "git log --oneline --graph") to examine the change.
>
> The "git log --oneline --graph" thing should work fine with the
> proposal as it currently is, but I'm not sure that the --first-parent
> --patch thing would be very useful no matter how we order the parents.
> The metacommits have empty trees and commit messages, so such a log
> would just list the metacommit hashes and nothing else. That certainly
> has some utility, but I'd guess it's probably not what you were going
> for. Were you intending to suggest that the metacommit should also use
> the same tree and commit message as its content commit? If so, we
> briefly considered this option while preparing this proposal. That
> would make some commands do approximately the right thing for free.
> However, when we started working through the use-cases (for example,
> checking out a metacommit) we found that all the ones we looked at
> would still need special cases for metacommits and those special cases
> wouldn't be much simpler than they'd be with an empty tree and
> message. Admittedly, git log wasn't one of the use-cases we worked
> through.
>
>   - Stefan
>
> On Fri, Nov 16, 2018 at 10:07 PM Duy Nguyen  wrote:
> >
> > On Thu, Nov 15, 2018 at 2:00 AM  wrote:
> > > +Goals
> > > +-
> > > +Legend: Goals marked with P0 are required. Goals marked with Pn should be
> > > +attempted unless 

Re: [PATCH] technical doc: add a design doc for the evolve command

2018-11-18 Thread Stefan Xenos
> I don't think this counts as a typical modification and is probably hard to 
> detect automatically.

Clever use of commands! (side: wouldn't it just be easier to just use
git commit --amend, though?)

Either way, I agree that there should be a way to manually create a
change graph or modify one into any possible shape. I've updated the
"change" command to do what you want - the new version will have
subcommands for creating arbitrary change graphs.

> subject line will change over time and the original one may become irrelevant.

There's a section on change naming further down the document. My
criteria for name selection was that good names should be unique,
short to type, and memorable to the user. Being relevant to the commit
wasn't actually a requirement for me except insofar as it helps make
them memorable. If we agree that these are reasonable criteria, commit
hashes wouldn't be as good a choice since they'd satisfy the
uniqueness criteria but would not be short or memorable. I expect that
whatever criteria we select probably won't be optimal for all users
which is why the design also includes a new hook for name selection. I
believe that selected words from the commit comment should cover all
three criteria in the majority of cases, and that the hook and the
"change rename" command should cover the remaining corner cases. This
breaks the "git change" symmetry with "git branch", but after
responding to other messages regarding that command, I'm starting to
think that's not really a problem.

> How do we group changes of a topic together? I think branch-diff could take 
> advantage of that.

Could you clarify your use-case for me? I'm not sure what you mean by
"changes of a topic". Are you referring to gerrit topics here? Topic
branches? Or are you asking for some way for end-users to classify and
organize their unsubmitted changes?

> Could we just organize it like a normal history?
> Basically all commits will be linked in a new merge history.

>From what I can tell, you're suggesting the following changes:
1. Reorder the parents such that the content parent comes last rather
than first.
2. Move parent-type from the structured portion of the header to the
unstructured portion of the commit message.

I'm fine with 1 if that makes something easier.

Regarding 2, I can see some good reasons to put parent-type in the
header rather than the user-readable portion of the commit message
- fsck can rely on them when checking the database for validity (for
example, it can assert that the current repository version doesn't
attach a non-empty tree, that the content parent always points to a
real commit, the commit message is empty, that the number of
parent-types matches the number of parents, that the enum values are
valid, that the parent orders are correct, etc.).
- accidental collisions are impossible (users can't accidentally
corrupt their database by adding or removing the word "parent-type" in
a commit message).
- it doesn't spam the user-readable region with machine-readable
repository internals.

> This makes it possible to just use "git log --first-parent
> --patch" (or "git log --oneline --graph") to examine the change.

The "git log --oneline --graph" thing should work fine with the
proposal as it currently is, but I'm not sure that the --first-parent
--patch thing would be very useful no matter how we order the parents.
The metacommits have empty trees and commit messages, so such a log
would just list the metacommit hashes and nothing else. That certainly
has some utility, but I'd guess it's probably not what you were going
for. Were you intending to suggest that the metacommit should also use
the same tree and commit message as its content commit? If so, we
briefly considered this option while preparing this proposal. That
would make some commands do approximately the right thing for free.
However, when we started working through the use-cases (for example,
checking out a metacommit) we found that all the ones we looked at
would still need special cases for metacommits and those special cases
wouldn't be much simpler than they'd be with an empty tree and
message. Admittedly, git log wasn't one of the use-cases we worked
through.

  - Stefan

On Fri, Nov 16, 2018 at 10:07 PM Duy Nguyen  wrote:
>
> On Thu, Nov 15, 2018 at 2:00 AM  wrote:
> > +Goals
> > +-
> > +Legend: Goals marked with P0 are required. Goals marked with Pn should be
> > +attempted unless they interfere with goals marked with Pn-1.
> > +
> > +P0. All commands that modify commits (such as the normal commit --amend or
> > +rebase command) should mark the old commit as being obsolete and 
> > replaced by
> > +the new one. No additional commands should be required to keep the
> > +obsolescence graph up-to-date.
>
> I sometimes "modify" a commit by "git reset @^", pick up the changes
> then "git commit -c @{1}". I don't think this counts as a typical
> modification and is probably hard to detect automatically. But I 

Re: [PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

2018-11-18 Thread Jeff King
On Sun, Nov 18, 2018 at 10:02:02PM +0100, Sven Strickroth wrote:

> This also removes an implicit conversion from size_t (unsigned) to int 
> (signed).
> 
> _stricmp as well as _strnicmp are both available since VS2012.

Once upon a time we had problems with taking a function pointer of
strcasecmp (to use as a comparator with string_list), so I wondered if
that might be part of why it's defined the way it is.

But the current definition is already inline:

> -
> -static __inline int strcasecmp (const char *s1, const char *s2)
> -{
> -   int size1 = strlen(s1);
> -   int sisz2 = strlen(s2);
> -   return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
> -}
> +#define strcasecmp   _stricmp

And it seems we worked around this in de2f95ebed (mailmap: work around
implementations with pure inline strcasecmp, 2013-09-12). So I don't
think there is any blocker there.

(Though of course I have no idea on other portability questions around
_stricmp(); I'll leave that for Windows folks).

-Peff


[PATCH] msvc: Directly use MS version (_stricmp) of strcasecmp

2018-11-18 Thread Sven Strickroth
This also removes an implicit conversion from size_t (unsigned) to int (signed).

_stricmp as well as _strnicmp are both available since VS2012.

Signed-off-by: Sven Strickroth 
---
 compat/msvc.h | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/compat/msvc.h b/compat/msvc.h
index 580bb55bf4..ea6527f8b6 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -14,13 +14,7 @@
 #define ftruncate_chsize
 #define strtoull _strtoui64
 #define strtoll  _strtoi64
-
-static __inline int strcasecmp (const char *s1, const char *s2)
-{
-   int size1 = strlen(s1);
-   int sisz2 = strlen(s2);
-   return _strnicmp(s1, s2, sisz2 > size1 ? sisz2 : size1);
-}
+#define strcasecmp   _stricmp

 #undef ERROR

--
2.19.1.windows.1


[no subject]

2018-11-18 Thread Major Dennis Hornbeck



I am in the military unit here in Afghanistan, we have some amount of funds 
that we want to move out of the country. My partners and I need a good partner 
someone we can trust. It is risk free and legal. Reply to this email: 
hornbeckmajordennis...@gmail.com
Regards,Major Dennis Hornbeck.


Re: [PATCH/RFC v1 1/1] Use size_t instead of unsigned long

2018-11-18 Thread Derrick Stolee

On 11/17/2018 10:11 AM, tbo...@web.de wrote:

From: Torsten Bögershausen 

Currently Git users can not commit files >4Gib under 64 bit Windows,
where "long" is 32 bit but size_t is 64 bit.
Improve the code base in small steps, as small as possible.
What started with a small patch to replace "unsigned long" with size_t
in one file (convert.c) ended up with a change in many files.

Signed-off-by: Torsten Bögershausen 
---

This needs to go on top of pu, to cover all the good stuff
   cooking here.


Better to work on top of 'master', as the work in 'pu' will be rewritten 
several times, probably.



I have started this series on November 1st, since that 2 or 3 rebases
   had been done to catch up, and now it is on pu from November 15.

I couldn't find a reason why changing "unsigned ling"
   into "size_t" may break anything, any thoughts, please ?


IIRC, the blocker for why we haven't done this already is that "size_t", 
"timestamp_t" and "off_t" are all 64-bit types that give more implied 
meaning, so we should swap types specifically to these as we want. One 
example series does a specific, small change [1].


If we wanted to do a single swap that removes 'unsigned long' with an 
unambiguously 64-bit type, I would recommend "uint64_t". Later 
replacements to size_t, off_t, and timestamp_t could happen on a 
case-by-case basis for type clarity.


It may benefit reviewers to split this change into multiple patches, 
starting at the lowest levels of the call stack (so higher 'unsigned 
long's can up-cast to the 64-bit types when calling a function) and 
focusing the changes to one or two files at a time (X.c and X.h, 
preferrably).


Since you are talking about the benefits for Git for Windows to accept 
4GB files, I wonder if we can add a test that verifies such a file will 
work. If you have such a test, then I could help verify that the test 
fails before the change and succeeds afterward.


Finally, it may be good to add a coccinelle script that replaces 
'unsigned long' with 'uint64_t' so we can easily fix any new 
introductions that happen in the future.


Thanks! I do think we should make this change, but we must be careful. 
It may be disruptive to topics in flight.


-Stolee

[1] https://public-inbox.org/git/20181112084031.11769-1-care...@gmail.com/



Git Test Coverage Report (Sunday, Nov 18th)

2018-11-18 Thread Derrick Stolee

Here is the test coverage report for today.

Thanks,

-Stolee

[1] https://dev.azure.com/git/git/_build/results?buildId=257=logs

---


pu: d02f4432dcda003413023869ebe412f03155f230
jch: af77f5458d76b45843ab70c577a54db24b4ea92f
next: 6e8e63e21ad73680486866b4870b45db87c3d939
master: 26aa9fc81d4c7f6c3b456a29da0b7ec72e5c6595
master@{1}: d166e6afe5f257217836ef24a73764eba390c58d

Uncovered code in 'pu' not in 'jch'
--

builtin/blame.c
74e8221b52 builtin/blame.c    928) blame_date_width = sizeof("Thu Oct 19 
16:00");

74e8221b52 builtin/blame.c    929) break;

builtin/remote.c
b7f4e371e7 builtin/remote.c 1551) die(_("--save-to-push cannot be used 
with other options"));
b7f4e371e7 builtin/remote.c 1575) die(_("--save-to-push can only be used 
when only one url is defined"));


date.c
74e8221b52  113) die("Timestamp too large for this system: %"PRItime, time);
74e8221b52  216) if (tm->tm_mon == human_tm->tm_mon) {
74e8221b52  217) if (tm->tm_mday > human_tm->tm_mday) {
74e8221b52  219) } else if (tm->tm_mday == human_tm->tm_mday) {
74e8221b52  220) hide.date = hide.wday = 1;
74e8221b52  221) } else if (tm->tm_mday + 5 > human_tm->tm_mday) {
74e8221b52  223) hide.date = 1;
74e8221b52  231) gettimeofday(, NULL);
74e8221b52  232) show_date_relative(time, tz, , buf);
74e8221b52  233) return;
74e8221b52  246) hide.seconds = 1;
74e8221b52  247) hide.tz |= !hide.date;
74e8221b52  248) hide.wday = hide.time = !hide.year;
74e8221b52  262) strbuf_rtrim(buf);
74e8221b52  287) gettimeofday(, NULL);
74e8221b52  290) human_tz = local_time_tzoffset(now.tv_sec, _tm);
74e8221b52  886) static int auto_date_style(void)
74e8221b52  888) return (isatty(1) || pager_in_use()) ? DATE_HUMAN : 
DATE_NORMAL;

74e8221b52  909) return DATE_HUMAN;
74e8221b52  911) return auto_date_style();

protocol.c
24c10f7473  37) die(_("Unrecognized protocol version"));
24c10f7473  39) die(_("Unrecognized protocol_version"));

remote-curl.c
24c10f7473  403) return 0;

sha1-array.c
bba406749a 91) oidcpy([dst], [src]);

submodule.c
e2419f7e30 1376) strbuf_release();
7454fe5cb6 1499) struct get_next_submodule_task *task = task_cb;
7454fe5cb6 1503) get_next_submodule_task_release(task);
7454fe5cb6 1530) return 0;
7454fe5cb6 1534) goto out;
7454fe5cb6 1549) return 0;

wrapper.c
5efde212fc  70) die("Out of memory, malloc failed (tried to allocate %" 
PRIuMAX " bytes)",
5efde212fc  73) error("Out of memory, malloc failed (tried to allocate 
%" PRIuMAX " bytes)",


Commits introducing uncovered code:
Denton Liu  b7f4e371e: remote: add --save-to-push option to git 
remote set-url
Josh Steadmon  24c10f747: protocol: advertise multiple supported 
versions

Linus Torvalds  74e8221b5: Add 'human' date format
Martin Koegler  5efde212f: zlib.c: use size_t for size
Stefan Beller  7454fe5cb: fetch: try fetching submodules if needed 
objects were not fetched

Stefan Beller  bba406749: sha1-array: provide oid_array_filter
Stefan Beller  e2419f7e3: submodule: migrate get_next_submodule to 
use repository structs




Uncovered code in 'jch' not in 'next'


apply.c
0f086e6dca 3355) if (checkout_entry(ce, , NULL, NULL) ||
0f086e6dca 3356) lstat(ce->name, st))

builtin/branch.c
0ecb1fc726 builtin/branch.c 456) die(_("could not resolve HEAD"));
0ecb1fc726 builtin/branch.c 462) die(_("HEAD (%s) points outside of 
refs/heads/"), refname);


builtin/pull.c
88eeb24cb1 647) argv_array_push(, opt_cleanup);

hex.c
47edb64997  93) char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
47edb64997  95) return hash_to_hex_algop_r(buffer, sha1, 
_algos[GIT_HASH_SHA1]);

47edb64997 116) char *hash_to_hex(const unsigned char *hash)
47edb64997 118) return hash_to_hex_algop(hash, the_hash_algo);

sequencer.c
18e711a162 2387) opts->quiet = 1;

sha1-file.c
2f90b9d9b4 sha1-file.c  172) int hash_algo_by_name(const char *name)
2f90b9d9b4 sha1-file.c  175) if (!name)
2f90b9d9b4 sha1-file.c  176) return GIT_HASH_UNKNOWN;
2f90b9d9b4 sha1-file.c  177) for (i = 1; i < GIT_HASH_NALGOS; i++)
2f90b9d9b4 sha1-file.c  178) if (!strcmp(name, hash_algos[i].name))
2f90b9d9b4 sha1-file.c  179) return i;
2f90b9d9b4 sha1-file.c  180) return GIT_HASH_UNKNOWN;
2f90b9d9b4 sha1-file.c  183) int hash_algo_by_id(uint32_t format_id)
2f90b9d9b4 sha1-file.c  186) for (i = 1; i < GIT_HASH_NALGOS; i++)
2f90b9d9b4 sha1-file.c  187) if (format_id == hash_algos[i].format_id)
2f90b9d9b4 sha1-file.c  188) return i;
2f90b9d9b4 sha1-file.c  189) return GIT_HASH_UNKNOWN;

Commits introducing uncovered code:
brian m. carlson  2f90b9d9b: sha1-file: provide functions to look up 
hash algorithms
brian m. carlson  47edb6499: hex: introduce functions to print 
arbitrary hashes
Daniels Umanovskis  0ecb1fc72: branch: introduce --show-current 
display option

Denton Liu  88eeb24cb: merge: add scissors line on merge conflict
Elijah Newren  18e711a16: git-rebase, sequencer: extend --quiet 
option for the 

Re: [PATCH 5/5] tree-walk: support :(attr) matching

2018-11-18 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 18 2018, Nguyễn Thái Ngọc Duy wrote:

As noted in
https://public-inbox.org/git/87d0r217vr@evledraar.gmail.com/ I'm
happy to see this implemented. I have not read this patch in much
detail...

> [...]
>  Documentation/glossary-content.txt |  2 +
> [...]
> diff --git a/Documentation/glossary-content.txt 
> b/Documentation/glossary-content.txt
> index 0d2aa48c63..023ca95e7c 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -404,6 +404,8 @@ these forms:
>  - "`!ATTR`" requires that the attribute `ATTR` be
>unspecified.
>  +
> +Note that when matching against a tree object, attributes are still
> +obtained from working tree, not from the given tree object.
>
>  exclude;;
>   After a path matches any non-exclude pathspec, it will be run

Just a poke again about what I brought up in the thread you replied to
in
https://public-inbox.org/git/cacsjy8clhq0mkhkxvtdaqy9tlwefbsvheu5ubpxhx4is2mk...@mail.gmail.com/

I.e. the documentation of these various wildmatch() / attributes
patterns we support is all over the place. Some in gitignore(5), some
not documented at all, and some in gitglossary(7) (which really should
not be serving as primary documentation for anything).


Re: [PATCH 0/5] Make :(attr) pathspec work with "git log"

2018-11-18 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 18 2018, Nguyễn Thái Ngọc Duy wrote:

> When :(attr) was added, it supported one of the two main pathspec
> matching functions, the one that works on a list of paths. The other
> one works on a tree, tree_entry_interesting(), which gets :(attr)
> support in this series.
>
> With this, "git grep   -- :(attr)" or "git log :(attr)"
> will not abort with BUG() anymore.
>
> But this also reveals an interesting thing: even though we walk on a
> tree, we check attributes from _worktree_ (and optionally fall back to
> the index). This is how attributes are implemented since forever. I
> think this is not a big deal if we communicate clearly with the user.
> But otherwise, this series can be scraped, as reading attributes from
> a specific tree could be a lot of work.

I'm very happy to see this implemented, and I think the behavior
described here is the right way to go. E.g. in git.git we have diff=perl
entries in .gitattributes. It would suck if:

git log ':(attr:diff=perl)'

Would only list commits as far as 20460635a8 (".gitattributes: use the
"perl" differ for Perl", 2018-04-26), since that's when we stop having
that attribute. Ditto for wanting to run "grep" on e.g. perl files in
2.12.0.

I have also run into cases where I want to use a .gitattributes file
from a specific commit. E.g. when writing pre-receive hooks where I've
wanted the .gitattributes of the commit being pushed to configure
something about it. But as you note this isn't supported at all.

But a concern is whether we should be making :(attr:*) behave like this
for now. Are we going to regret it later? I don't think so, I think
wanting to use the current working tree's / index's is the most sane
default, and if we get the ability to read it from revisions as we
e.g. walk the log it would make most sense to just call that
:(treeattr:*) or something like that.


Important

2018-11-18 Thread Reem Al-Hashimi
Hello,

My name is ms. Reem Al-Hashimi. The UAE minister of state for international 
cooparation. I got your contact from a certain email database from your country 
while i was looking for someone to handle a huge financial transaction for me 
in confidence. Can you receive and invest on behalf of my only son. Please 
reply to reem2...@daum.net, for more details if you are interested.

Regards,

Ms. Reem Al-Hashimy


[PATCH v3] read-cache: make the split index obey umask settings

2018-11-18 Thread Ævar Arnfjörð Bjarmason
Make the split index write out its .git/sharedindex_* files with the
same permissions as .git/index. This only changes the behavior when
core.sharedRepository isn't set, i.e. the user's umask settings will
be respected.

This hasn't been the case ever since the split index was originally
implemented in c18b80a0e8 ("update-index: new options to
enable/disable split index mode", 2014-06-13). A mkstemp()-like
function has always been used to create it. First mkstemp() itself,
and then later our own mkstemp()-like in
f6ecc62dbf ("write_shared_index(): use tempfile module", 2015-08-10)

A related bug was fixed in df801f3f9f ("read-cache: use shared perms
when writing shared index", 2017-06-25). Since then the split index
has respected core.sharedRepository.

However, using that setting should not be required simply to make git
obey the user's umask setting. It's intended for the use-case of
overriding whatever that umask is set to. This fixes cases where the
user has e.g. set his umask to 022 on a shared server in anticipation
of other user's needing to run "status", "log" etc. in his repository.

Signed-off-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Christian Couder 
Signed-off-by: Junio C Hamano 
---

Here it is with a rewritten commit message & adjusted comment as
discussed in the v2 discussion.

 read-cache.c   |  3 ++-
 t/t1700-split-index.sh | 20 
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/read-cache.c b/read-cache.c
index 4ca81286c0..e7e77e780b 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3150,7 +3150,8 @@ int write_locked_index(struct index_state *istate, struct 
lock_file *lock,
struct tempfile *temp;
int saved_errno;
 
-   temp = mks_tempfile(git_path("sharedindex_XX"));
+   /* Same initial permissions as the main .git/index file */
+   temp = mks_tempfile_sm(git_path("sharedindex_XX"), 0, 0666);
if (!temp) {
oidclr(>base_oid);
ret = do_write_locked_index(istate, lock, flags);
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 2ac47aa0e4..fa1d3d468b 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -381,6 +381,26 @@ test_expect_success 'check splitIndex.sharedIndexExpire 
set to "never" and "now"
test $(ls .git/sharedindex.* | wc -l) -le 2
 '
 
+test_expect_success POSIXPERM 'same mode for index & split index' '
+   git init same-mode &&
+   (
+   cd same-mode &&
+   test_commit A &&
+   test_modebits .git/index >index_mode &&
+   test_must_fail git config core.sharedRepository &&
+   git -c core.splitIndex=true status &&
+   shared=$(ls .git/sharedindex.*) &&
+   case "$shared" in
+   *" "*)
+   # we have more than one???
+   false ;;
+   *)
+   test_modebits "$shared" >split_index_mode &&
+   test_cmp index_mode split_index_mode ;;
+   esac
+   )
+'
+
 while read -r mode modebits
 do
test_expect_success POSIXPERM "split index respects 
core.sharedrepository $mode" '
-- 
2.19.1.1182.g4ecb1133ce



Re: Cygwin Git with Windows paths

2018-11-18 Thread Steven Penny
On Sun, Nov 18, 2018 at 12:28 PM Torsten Bögershausen wrote:
> Thanks for testing.
> It looks as if there is more work to be done then just a simple patch.
>
> My last question for today:
> Does
>
> git clone  '/cgdrive/c/my/dir'
>
> work ?

yes - these all work and resolve to same path:

   git clone  /tmp/goawk
   git clone  /cygdrive/c/cygwin64/tmp/goawk
   git clone  /proc/cygdrive/c/cygwin64/tmp/goawk

however i would caution that any fix should not rely on "C:", as users are
allowed to install to other volumes such as "D:". Perhaps a better solution
would be for Git to just take the path as is, rather than converting it to an
absolute path?


Re: Cygwin Git with Windows paths

2018-11-18 Thread Torsten Bögershausen
On Sun, Nov 18, 2018 at 11:34:04AM -0600, Steven Penny wrote:
> On Sun, Nov 18, 2018 at 11:15 AM Torsten Bögershausen wrote:
> > But it may be that we need to pull in more stuff, similar to mingw,
> > to get the C: stuff working, see
> > "skip_dos_drive_prefix"
> >
> > And it may even be that we need a special handling for the "\" to be treated
> > as "/".
> >
> > If you implement "skip_dos_drive_prefix" similar to mingw,
> > (rename mingw into cygwin) does
> >
> > git clone  C:/my/dir/
> > work ?
> 
> I added this to "compat/cygwin.h":
> 
> #define has_dos_drive_prefix(path) \
>   (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
> int mingw_skip_dos_drive_prefix(char **path);
> #define skip_dos_drive_prefix mingw_skip_dos_drive_prefix
> 
> and added this to "compat/cygwin.c":
> 
> int mingw_skip_dos_drive_prefix(char **path) {
>   int ret = has_dos_drive_prefix(*path);
>   *path += ret;
>   return ret;
> }
> 
> but still, these dont work:
> 
> git clone  C:/my/dir
> git clone  'C:\my\dir'

Thanks for testing.
It looks as if there is more work to be done then just a simple patch.

My last question for today:
Does 

git clone  '/cgdrive/c/my/dir'

work ?



Re: Cygwin Git with Windows paths

2018-11-18 Thread Steven Penny
On Sun, Nov 18, 2018 at 11:15 AM Torsten Bögershausen wrote:
> But it may be that we need to pull in more stuff, similar to mingw,
> to get the C: stuff working, see
> "skip_dos_drive_prefix"
>
> And it may even be that we need a special handling for the "\" to be treated
> as "/".
>
> If you implement "skip_dos_drive_prefix" similar to mingw,
> (rename mingw into cygwin) does
>
> git clone  C:/my/dir/
> work ?

I added this to "compat/cygwin.h":

#define has_dos_drive_prefix(path) \
  (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
int mingw_skip_dos_drive_prefix(char **path);
#define skip_dos_drive_prefix mingw_skip_dos_drive_prefix

and added this to "compat/cygwin.c":

int mingw_skip_dos_drive_prefix(char **path) {
  int ret = has_dos_drive_prefix(*path);
  *path += ret;
  return ret;
}

but still, these dont work:

git clone  C:/my/dir
git clone  'C:\my\dir'


Re: Cygwin Git with Windows paths

2018-11-18 Thread Torsten Bögershausen
On Sun, Nov 18, 2018 at 10:23:19AM -0600, Steven Penny wrote:
> On Sun, Nov 18, 2018 at 9:41 AM Torsten Bögershausen wrote:
> > Thanks for the report
> > It seams as if "C:" is not recognized as an absolute path under
> > cygwin.
> > May be it should ?
> >
> > Does the following help ? (fully untested)
> 
> that looks promising - but its not getting pulled in where it needs to be.
> perhaps another file need to be modified to utilize that macro?

The macro should be utilized, see git-compat-util.h:

#if defined(__CYGWIN__)
#include "compat/cygwin.h"
#endif

And further down

#ifndef has_dos_drive_prefix
static inline int git_has_dos_drive_prefix(const char *path)
{
return 0;
}
#define has_dos_drive_prefix git_has_dos_drive_prefix
#endif

#ifndef skip_dos_drive_prefix
static inline int git_skip_dos_drive_prefix(char **path)
{
return 0;
}

But it may be that we need to pull in more stuff, similar to mingw,
to get the C: stuff working, see
"skip_dos_drive_prefix"

And it may even be that we need a special handling for the "\" to be treated
as "/".

If you implement "skip_dos_drive_prefix" similar to mingw,
(rename mingw into cygwin) does

git clone  C:/my/dir/
work ?

That would be a progress, I think.


[PATCH 2/5] tree-walk.c: make tree_entry_interesting() take an index

2018-11-18 Thread Nguyễn Thái Ngọc Duy
In order to support :(attr) when matching pathspec on a tree,
tree_entry_interesting() needs to take an index (because
git_check_attr() needs it). This is the preparation step for it. This
also makes it clearer what index we fall back to when looking up
attributes during an unpack-trees operation: the source index.

This also fixes revs->pruning.repo initialization that should have
been done in 2abf350385 (revision.c: remove implicit dependency on
the_index - 2018-09-21). Without it, skip_uninteresting() will
dereference a NULL pointer through this call chain

  get_revision(revs)
  get_revision_internal
  get_revision_1
  try_to_simplify_commit
  rev_compare_tree
  diff_tree_oid(..., >pruning)
  ll_diff_tree_oid
  diff_tree_paths
  ll_diff_tree
  skip_uninteresting

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/grep.c   |  3 ++-
 builtin/merge-tree.c |  2 +-
 list-objects.c   |  3 ++-
 revision.c   |  1 +
 tree-diff.c  |  3 ++-
 tree-walk.c  | 22 ++
 tree-walk.h  | 10 ++
 tree.c   |  3 ++-
 unpack-trees.c   |  6 +++---
 9 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 56e4a11052..f6e086c287 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -568,7 +568,8 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
 
if (match != all_entries_interesting) {
strbuf_addstr(, base->buf + tn_len);
-   match = tree_entry_interesting(, ,
+   match = tree_entry_interesting(repo->index,
+  , ,
   0, pathspec);
strbuf_setlen(, name_base_len);
 
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 70f6fc9167..4984b7e12e 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -346,7 +346,7 @@ static void merge_trees(struct tree_desc t[3], const char 
*base)
 
setup_traverse_info(, base);
info.fn = threeway_callback;
-   traverse_trees(3, t, );
+   traverse_trees(_index, 3, t, );
 }
 
 static void *get_tree_descriptor(struct tree_desc *desc, const char *rev)
diff --git a/list-objects.c b/list-objects.c
index c41cc80db5..63c395d9c2 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -113,7 +113,8 @@ static void process_tree_contents(struct traversal_context 
*ctx,
 
while (tree_entry(, )) {
if (match != all_entries_interesting) {
-   match = tree_entry_interesting(, base, 0,
+   match = tree_entry_interesting(ctx->revs->repo->index,
+  , base, 0,
   
>revs->diffopt.pathspec);
if (match == all_entries_not_interesting)
break;
diff --git a/revision.c b/revision.c
index bdd3e7c9f1..7ced1ee02b 100644
--- a/revision.c
+++ b/revision.c
@@ -1459,6 +1459,7 @@ void repo_init_revisions(struct repository *r,
revs->abbrev = DEFAULT_ABBREV;
revs->ignore_merges = 1;
revs->simplify_history = 1;
+   revs->pruning.repo = r;
revs->pruning.flags.recursive = 1;
revs->pruning.flags.quick = 1;
revs->pruning.add_remove = file_add_remove;
diff --git a/tree-diff.c b/tree-diff.c
index 0e54324610..34ee3b13b8 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -299,7 +299,8 @@ static void skip_uninteresting(struct tree_desc *t, struct 
strbuf *base,
enum interesting match;
 
while (t->size) {
-   match = tree_entry_interesting(>entry, base, 0, 
>pathspec);
+   match = tree_entry_interesting(opt->repo->index, >entry,
+  base, 0, >pathspec);
if (match) {
if (match == all_entries_not_interesting)
t->size = 0;
diff --git a/tree-walk.c b/tree-walk.c
index 79bafbd1a2..517bcdecf9 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -365,7 +365,8 @@ static void free_extended_entry(struct tree_desc_x *t)
}
 }
 
-static inline int prune_traversal(struct name_entry *e,
+static inline int prune_traversal(struct index_state *istate,
+ struct name_entry *e,
  struct traverse_info *info,
  struct strbuf *base,
  int still_interesting)
@@ -374,10 +375,13 @@ static inline int prune_traversal(struct name_entry *e,
return 2;
if (still_interesting < 0)
return still_interesting;
-   return tree_entry_interesting(e, base, 0, info->pathspec);
+   return tree_entry_interesting(istate, e, base,
+ 0, info->pathspec);
 }
 
-int 

[PATCH 3/5] pathspec.h: clean up "extern" in function declarations

2018-11-18 Thread Nguyễn Thái Ngọc Duy
"extern" on functions is not required and the trend has been removing
it from header files.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 pathspec.h | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/pathspec.h b/pathspec.h
index a6525a6551..5fd781d695 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -80,13 +80,13 @@ struct pathspec {
  * Any arguments used are copied. It is safe for the caller to modify
  * or free 'prefix' and 'args' after calling this function.
  */
-extern void parse_pathspec(struct pathspec *pathspec,
-  unsigned magic_mask,
-  unsigned flags,
-  const char *prefix,
-  const char **args);
-extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
-extern void clear_pathspec(struct pathspec *);
+void parse_pathspec(struct pathspec *pathspec,
+   unsigned magic_mask,
+   unsigned flags,
+   const char *prefix,
+   const char **args);
+void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
+void clear_pathspec(struct pathspec *);
 
 static inline int ps_strncmp(const struct pathspec_item *item,
 const char *s1, const char *s2, size_t n)
@@ -106,10 +106,10 @@ static inline int ps_strcmp(const struct pathspec_item 
*item,
return strcmp(s1, s2);
 }
 
-extern void add_pathspec_matches_against_index(const struct pathspec *pathspec,
-  const struct index_state *istate,
-  char *seen);
-extern char *find_pathspecs_matching_against_index(const struct pathspec 
*pathspec,
-  const struct index_state 
*istate);
+void add_pathspec_matches_against_index(const struct pathspec *pathspec,
+   const struct index_state *istate,
+   char *seen);
+char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
+   const struct index_state *istate);
 
 #endif /* PATHSPEC_H */
-- 
2.19.1.1327.g328c130451.dirty



[PATCH 5/5] tree-walk: support :(attr) matching

2018-11-18 Thread Nguyễn Thái Ngọc Duy
This lets us use :(attr) with "git grep " or "git log".

:(attr) requires another round of checking before we can declare that
a path is matched. This is done after path matching since we have lots
of optimization to take a shortcut when things don't match.

Note that if :(attr) is present, we can't return
all_entries_interesting / all_entries_not_interesting anymore because
we can't be certain about that. Not until match_pathspec_attrs() can
tell us "yes all these paths satisfy :(attr)".

Second note. Even though we walk a specific tree, we use attributes
from _worktree_ (or falling back to the index), not from .gitattributes
files on that tree. This by itself is not necessarily wrong, but the
user just have to be aware of this.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/glossary-content.txt |  2 +
 t/t6135-pathspec-with-attrs.sh | 58 +-
 tree-walk.c| 67 +++---
 3 files changed, 112 insertions(+), 15 deletions(-)

diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 0d2aa48c63..023ca95e7c 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -404,6 +404,8 @@ these forms:
 - "`!ATTR`" requires that the attribute `ATTR` be
   unspecified.
 +
+Note that when matching against a tree object, attributes are still
+obtained from working tree, not from the given tree object.
 
 exclude;;
After a path matches any non-exclude pathspec, it will be run
diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh
index e436a73962..457cc167c7 100755
--- a/t/t6135-pathspec-with-attrs.sh
+++ b/t/t6135-pathspec-with-attrs.sh
@@ -31,7 +31,7 @@ test_expect_success 'setup a tree' '
mkdir sub &&
while read path
do
-   : >$path &&
+   echo content >$path &&
git add $path || return 1
done .gitattributes &&
fileA labelA
@@ -74,6 +78,15 @@ test_expect_success 'check specific set attr' '
test_cmp expect actual
 '
 
+test_expect_success 'check specific set attr (2)' '
+   cat <<-\EOF >expect &&
+   HEAD:fileSetLabel
+   HEAD:sub/fileSetLabel
+   EOF
+   git grep -l content HEAD ":(attr:label)" >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'check specific unset attr' '
cat <<-\EOF >expect &&
fileUnsetLabel
@@ -83,6 +96,15 @@ test_expect_success 'check specific unset attr' '
test_cmp expect actual
 '
 
+test_expect_success 'check specific unset attr (2)' '
+   cat <<-\EOF >expect &&
+   HEAD:fileUnsetLabel
+   HEAD:sub/fileUnsetLabel
+   EOF
+   git grep -l content HEAD ":(attr:-label)" >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'check specific value attr' '
cat <<-\EOF >expect &&
fileValue
@@ -94,6 +116,16 @@ test_expect_success 'check specific value attr' '
test_must_be_empty actual
 '
 
+test_expect_success 'check specific value attr (2)' '
+   cat <<-\EOF >expect &&
+   HEAD:fileValue
+   HEAD:sub/fileValue
+   EOF
+   git grep -l content HEAD ":(attr:label=foo)" >actual &&
+   test_cmp expect actual &&
+   test_must_fail git grep -l content HEAD ":(attr:label=bar)"
+'
+
 test_expect_success 'check unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
@@ -118,6 +150,30 @@ test_expect_success 'check unspecified attr' '
test_cmp expect actual
 '
 
+test_expect_success 'check unspecified attr (2)' '
+   cat <<-\EOF >expect &&
+   HEAD:.gitattributes
+   HEAD:fileA
+   HEAD:fileAB
+   HEAD:fileAC
+   HEAD:fileB
+   HEAD:fileBC
+   HEAD:fileC
+   HEAD:fileNoLabel
+   HEAD:fileWrongLabel
+   HEAD:sub/fileA
+   HEAD:sub/fileAB
+   HEAD:sub/fileAC
+   HEAD:sub/fileB
+   HEAD:sub/fileBC
+   HEAD:sub/fileC
+   HEAD:sub/fileNoLabel
+   HEAD:sub/fileWrongLabel
+   EOF
+   git grep -l ^ HEAD ":(attr:!label)" >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'check multiple unspecified attr' '
cat <<-\EOF >expect &&
.gitattributes
diff --git a/tree-walk.c b/tree-walk.c
index 517bcdecf9..08210a4109 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -949,7 +949,8 @@ static enum interesting do_match(struct index_state *istate,
   PATHSPEC_LITERAL |
   PATHSPEC_GLOB |
   PATHSPEC_ICASE |
-  PATHSPEC_EXCLUDE);
+  PATHSPEC_EXCLUDE |
+  PATHSPEC_ATTR);
 
if (!ps->nr) {
if (!ps->recursive ||
@@ -981,14 +982,20 @@ static enum interesting do_match(struct index_state 
*istate,
 
if (!ps->recursive ||
!(ps->magic & PATHSPEC_MAXDEPTH) ||
-   ps->max_depth == -1)
- 

[PATCH 4/5] dir.c: move, rename and export match_attrs()

2018-11-18 Thread Nguyễn Thái Ngọc Duy
The function will be reused for matching attributes in pathspec when
walking trees (currently it's used for matching pathspec when walking
a list). pathspec.c would be a more neutral place for this.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 dir.c  | 41 ++---
 pathspec.c | 38 ++
 pathspec.h |  3 +++
 3 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/dir.c b/dir.c
index ab6477d777..2128448219 100644
--- a/dir.c
+++ b/dir.c
@@ -276,44 +276,6 @@ static int do_read_blob(const struct object_id *oid, 
struct oid_stat *oid_stat,
 #define DO_MATCH_DIRECTORY (1<<1)
 #define DO_MATCH_SUBMODULE (1<<2)
 
-static int match_attrs(const struct index_state *istate,
-  const char *name, int namelen,
-  const struct pathspec_item *item)
-{
-   int i;
-   char *to_free = NULL;
-
-   if (name[namelen])
-   name = to_free = xmemdupz(name, namelen);
-
-   git_check_attr(istate, name, item->attr_check);
-
-   free(to_free);
-
-   for (i = 0; i < item->attr_match_nr; i++) {
-   const char *value;
-   int matched;
-   enum attr_match_mode match_mode;
-
-   value = item->attr_check->items[i].value;
-   match_mode = item->attr_match[i].match_mode;
-
-   if (ATTR_TRUE(value))
-   matched = (match_mode == MATCH_SET);
-   else if (ATTR_FALSE(value))
-   matched = (match_mode == MATCH_UNSET);
-   else if (ATTR_UNSET(value))
-   matched = (match_mode == MATCH_UNSPECIFIED);
-   else
-   matched = (match_mode == MATCH_VALUE &&
-  !strcmp(item->attr_match[i].value, value));
-   if (!matched)
-   return 0;
-   }
-
-   return 1;
-}
-
 /*
  * Does 'match' match the given name?
  * A match is found if
@@ -367,7 +329,8 @@ static int match_pathspec_item(const struct index_state 
*istate,
strncmp(item->match, name - prefix, item->prefix))
return 0;
 
-   if (item->attr_match_nr && !match_attrs(istate, name, namelen, item))
+   if (item->attr_match_nr &&
+   !match_pathspec_attrs(istate, name, namelen, item))
return 0;
 
/* If the match was just the prefix, we matched */
diff --git a/pathspec.c b/pathspec.c
index 6f005996fd..e85298f68c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -659,3 +659,41 @@ void clear_pathspec(struct pathspec *pathspec)
FREE_AND_NULL(pathspec->items);
pathspec->nr = 0;
 }
+
+int match_pathspec_attrs(const struct index_state *istate,
+const char *name, int namelen,
+const struct pathspec_item *item)
+{
+   int i;
+   char *to_free = NULL;
+
+   if (name[namelen])
+   name = to_free = xmemdupz(name, namelen);
+
+   git_check_attr(istate, name, item->attr_check);
+
+   free(to_free);
+
+   for (i = 0; i < item->attr_match_nr; i++) {
+   const char *value;
+   int matched;
+   enum attr_match_mode match_mode;
+
+   value = item->attr_check->items[i].value;
+   match_mode = item->attr_match[i].match_mode;
+
+   if (ATTR_TRUE(value))
+   matched = (match_mode == MATCH_SET);
+   else if (ATTR_FALSE(value))
+   matched = (match_mode == MATCH_UNSET);
+   else if (ATTR_UNSET(value))
+   matched = (match_mode == MATCH_UNSPECIFIED);
+   else
+   matched = (match_mode == MATCH_VALUE &&
+  !strcmp(item->attr_match[i].value, value));
+   if (!matched)
+   return 0;
+   }
+
+   return 1;
+}
diff --git a/pathspec.h b/pathspec.h
index 5fd781d695..1c18a2c90c 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -111,5 +111,8 @@ void add_pathspec_matches_against_index(const struct 
pathspec *pathspec,
char *seen);
 char *find_pathspecs_matching_against_index(const struct pathspec *pathspec,
const struct index_state *istate);
+int match_pathspec_attrs(const struct index_state *istate,
+const char *name, int namelen,
+const struct pathspec_item *item);
 
 #endif /* PATHSPEC_H */
-- 
2.19.1.1327.g328c130451.dirty



[PATCH 0/5] Make :(attr) pathspec work with "git log"

2018-11-18 Thread Nguyễn Thái Ngọc Duy
When :(attr) was added, it supported one of the two main pathspec
matching functions, the one that works on a list of paths. The other
one works on a tree, tree_entry_interesting(), which gets :(attr)
support in this series.

With this, "git grep   -- :(attr)" or "git log :(attr)"
will not abort with BUG() anymore.

But this also reveals an interesting thing: even though we walk on a
tree, we check attributes from _worktree_ (and optionally fall back to
the index). This is how attributes are implemented since forever. I
think this is not a big deal if we communicate clearly with the user.
But otherwise, this series can be scraped, as reading attributes from
a specific tree could be a lot of work.

The main patch is the last one. The others are just to open a path to
pass "struct index_state *" down to tree_entry_interesting(). This may
become standard procedure because we don't want to stick the_index (or
the_repository) here and there.

Nguyễn Thái Ngọc Duy (5):
  tree.c: make read_tree*() take 'struct repository *'
  tree-walk.c: make tree_entry_interesting() take an index
  pathspec.h: clean up "extern" in function declarations
  dir.c: move, rename and export match_attrs()
  tree-walk: support :(attr) matching

 Documentation/glossary-content.txt |  2 +
 archive.c  |  6 +-
 builtin/checkout.c |  3 +-
 builtin/grep.c |  3 +-
 builtin/log.c  |  5 +-
 builtin/ls-files.c |  2 +-
 builtin/ls-tree.c  |  3 +-
 builtin/merge-tree.c   |  2 +-
 dir.c  | 41 +-
 list-objects.c |  3 +-
 merge-recursive.c  |  3 +-
 pathspec.c | 38 +
 pathspec.h | 27 +
 revision.c |  1 +
 t/t6135-pathspec-with-attrs.sh | 58 ++-
 tree-diff.c|  3 +-
 tree-walk.c| 89 ++
 tree-walk.h| 10 ++--
 tree.c | 21 ---
 tree.h | 18 +++---
 unpack-trees.c |  6 +-
 21 files changed, 235 insertions(+), 109 deletions(-)

-- 
2.19.1.1327.g328c130451.dirty



[PATCH 1/5] tree.c: make read_tree*() take 'struct repository *'

2018-11-18 Thread Nguyễn Thái Ngọc Duy
These functions call tree_entry_interesting() which will soon require
a 'struct index_state *' to be passed in. Instead of just changing the
function signature to take an index, update to take a repo instead
because these functions do need object database access.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 archive.c  |  6 --
 builtin/checkout.c |  3 ++-
 builtin/log.c  |  5 +++--
 builtin/ls-files.c |  2 +-
 builtin/ls-tree.c  |  3 ++-
 merge-recursive.c  |  3 ++-
 tree.c | 18 ++
 tree.h | 18 +++---
 8 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/archive.c b/archive.c
index fd556c28e4..bfa9cc20c9 100644
--- a/archive.c
+++ b/archive.c
@@ -285,7 +285,8 @@ int write_archive_entries(struct archiver_args *args,
git_attr_set_direction(GIT_ATTR_INDEX);
}
 
-   err = read_tree_recursive(args->tree, "", 0, 0, >pathspec,
+   err = read_tree_recursive(args->repo, args->tree, "",
+ 0, 0, >pathspec,
  queue_or_write_archive_entry,
  );
if (err == READ_TREE_RECURSIVE)
@@ -346,7 +347,8 @@ static int path_exists(struct archiver_args *args, const 
char *path)
ctx.args = args;
parse_pathspec(, 0, 0, "", paths);
ctx.pathspec.recursive = 1;
-   ret = read_tree_recursive(args->tree, "", 0, 0, ,
+   ret = read_tree_recursive(args->repo, args->tree, "",
+ 0, 0, ,
  reject_entry, );
clear_pathspec();
return ret != 0;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acdafc6e4c..c9dda8e82e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -115,7 +115,8 @@ static int update_some(const struct object_id *oid, struct 
strbuf *base,
 
 static int read_tree_some(struct tree *tree, const struct pathspec *pathspec)
 {
-   read_tree_recursive(tree, "", 0, 0, pathspec, update_some, NULL);
+   read_tree_recursive(the_repository, tree, "", 0, 0,
+   pathspec, update_some, NULL);
 
/* update the index with the given tree's info
 * for all args, expanding wildcards, and exit
diff --git a/builtin/log.c b/builtin/log.c
index 061d4fd864..d493fa915e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -639,8 +639,9 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)
diff_get_color_opt(, 
DIFF_COMMIT),
name,
diff_get_color_opt(, 
DIFF_RESET));
-   read_tree_recursive((struct tree *)o, "", 0, 0, 
_all,
-   show_tree_object, rev.diffopt.file);
+   read_tree_recursive(the_repository, (struct tree *)o, 
"",
+   0, 0, _all, show_tree_object,
+   rev.diffopt.file);
rev.shown_one = 1;
break;
case OBJ_COMMIT:
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 7f9919a362..8db0cccbf3 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -441,7 +441,7 @@ void overlay_tree_on_index(struct index_state *istate,
   PATHSPEC_PREFER_CWD, prefix, matchbuf);
} else
memset(, 0, sizeof(pathspec));
-   if (read_tree(tree, 1, , istate))
+   if (read_tree(the_repository, tree, 1, , istate))
die("unable to read tree entries %s", tree_name);
 
for (i = 0; i < istate->cache_nr; i++) {
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index fe3b952cb3..6855f7fea5 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -185,5 +185,6 @@ int cmd_ls_tree(int argc, const char **argv, const char 
*prefix)
tree = parse_tree_indirect();
if (!tree)
die("not a tree object");
-   return !!read_tree_recursive(tree, "", 0, 0, , show_tree, 
NULL);
+   return !!read_tree_recursive(the_repository, tree, "", 0, 0,
+, show_tree, NULL);
 }
diff --git a/merge-recursive.c b/merge-recursive.c
index acc2f64a4e..b9467f5ecf 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -463,7 +463,8 @@ static void get_files_dirs(struct merge_options *o, struct 
tree *tree)
 {
struct pathspec match_all;
memset(_all, 0, sizeof(match_all));
-   read_tree_recursive(tree, "", 0, 0, _all, save_files_dirs, o);
+   read_tree_recursive(the_repository, tree, "", 0, 0,
+   _all, save_files_dirs, o);
 }
 
 static int get_tree_entry_if_blob(const struct object_id *tree,
diff --git a/tree.c b/tree.c
index 215d3fdc7c..ecd8c8ac29 100644
--- a/tree.c
+++ b/tree.c
@@ -60,7 +60,8 @@ static int read_one_entry_quick(const struct object_id *oid, 
struct strbuf *base
   

[PATCH] grep: use grep_opt->repo instead of explict repo argument

2018-11-18 Thread Nguyễn Thái Ngọc Duy
This command is probably the first one that operates on a repository
other than the_repository, in f9ee2fcdfa (grep: recurse in-process
using 'struct repository' - 2017-08-02). An explicit 'struct
repository *' was added in that commit to pass around the repository
that we're supposed to grep from.

Since 38bbc2ea39 (grep.c: remove implicit dependency on the_index -
2018-09-21). 'struct grep_opt *' carries in itself a repository
parameter for grepping. We should now be able to reuse grep_opt to
hold the submodule repo instead of a separate argument, which is just
room for mistakes.

While at there, use the right reference instead of the_repository and
the_index in this code. I was a bit careless in my attempt to kick
the_repository / the_index out of library code. It's normally safe to
just stick the_repository / the_index in bultin/ code, but it's not
the case for grep.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/grep.c | 41 -
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 56e4a11052..bdc49cd34e 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -408,18 +408,20 @@ static void run_pager(struct grep_opt *opt, const char 
*prefix)
exit(status);
 }
 
-static int grep_cache(struct grep_opt *opt, struct repository *repo,
+static int grep_cache(struct grep_opt *opt,
  const struct pathspec *pathspec, int cached);
 static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 struct tree_desc *tree, struct strbuf *base, int tn_len,
-int check_attr, struct repository *repo);
+int check_attr);
 
-static int grep_submodule(struct grep_opt *opt, struct repository 
*superproject,
+static int grep_submodule(struct grep_opt *opt,
  const struct pathspec *pathspec,
  const struct object_id *oid,
  const char *filename, const char *path)
 {
+   struct repository *superproject = opt->repo;
struct repository submodule;
+   struct grep_opt subopt;
int hit;
 
/*
@@ -455,6 +457,9 @@ static int grep_submodule(struct grep_opt *opt, struct 
repository *superproject,
add_to_alternates_memory(submodule.objects->objectdir);
grep_read_unlock();
 
+   memcpy(, opt, sizeof(subopt));
+   subopt.repo = 
+
if (oid) {
struct object *object;
struct tree_desc tree;
@@ -476,21 +481,22 @@ static int grep_submodule(struct grep_opt *opt, struct 
repository *superproject,
strbuf_addch(, '/');
 
init_tree_desc(, data, size);
-   hit = grep_tree(opt, pathspec, , , base.len,
-   object->type == OBJ_COMMIT, );
+   hit = grep_tree(, pathspec, , , base.len,
+   object->type == OBJ_COMMIT);
strbuf_release();
free(data);
} else {
-   hit = grep_cache(opt, , pathspec, 1);
+   hit = grep_cache(, pathspec, 1);
}
 
repo_clear();
return hit;
 }
 
-static int grep_cache(struct grep_opt *opt, struct repository *repo,
+static int grep_cache(struct grep_opt *opt,
  const struct pathspec *pathspec, int cached)
 {
+   struct repository *repo = opt->repo;
int hit = 0;
int nr;
struct strbuf name = STRBUF_INIT;
@@ -528,7 +534,7 @@ static int grep_cache(struct grep_opt *opt, struct 
repository *repo,
}
} else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
   submodule_path_match(repo->index, pathspec, 
name.buf, NULL)) {
-   hit |= grep_submodule(opt, repo, pathspec, NULL, 
ce->name, ce->name);
+   hit |= grep_submodule(opt, pathspec, NULL, ce->name, 
ce->name);
} else {
continue;
}
@@ -550,8 +556,9 @@ static int grep_cache(struct grep_opt *opt, struct 
repository *repo,
 
 static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 struct tree_desc *tree, struct strbuf *base, int tn_len,
-int check_attr, struct repository *repo)
+int check_attr)
 {
+   struct repository *repo = opt->repo;
int hit = 0;
enum interesting match = entry_not_interesting;
struct name_entry entry;
@@ -597,10 +604,10 @@ static int grep_tree(struct grep_opt *opt, const struct 
pathspec *pathspec,
strbuf_addch(base, '/');
init_tree_desc(, data, size);
hit |= grep_tree(opt, pathspec, , base, tn_len,
-check_attr, repo);
+check_attr);

2% Loan Offer

2018-11-18 Thread NORTON FINANCE
Loans for all kind of purpose
We offer from a minimum amount of 10,000.00 dollars to 20 million dollars
And at a low interest rate of 2%
Loan Term: 25 years maximum depending on the loan amount you need.
Customers should be above the age of 18
This loan transaction is 100% guarantee for serious customers
For more information about the loan please contact us through email:
nortonfinanc...@live.co.uk

القروض لجميع أنواع الغرض
نحن نقدم من مبلغ لا يقل عن 10،000.00 دولار إلى 20 مليون دولار
وبسعر فائدة منخفض قدره 2٪
مدة القرض: 25 سنة كحد أقصى حسب مبلغ القرض الذي تحتاجه.
يجب أن يكون عمر العملاء فوق سن 18 عامًا
هذه الصفقة القرض هو ضمان 100 ٪ للعملاء خطيرة
لمزيد من المعلومات حول القرض ، يرجى الاتصال بنا عبر البريد الإلكتروني:
nortonfinanc...@live.co.uk


Re: Cygwin Git with Windows paths

2018-11-18 Thread Steven Penny
On Sun, Nov 18, 2018 at 9:41 AM Torsten Bögershausen wrote:
> Thanks for the report
> It seams as if "C:" is not recognized as an absolute path under
> cygwin.
> May be it should ?
>
> Does the following help ? (fully untested)

that looks promising - but its not getting pulled in where it needs to be.
perhaps another file need to be modified to utilize that macro?


Re: Cygwin Git with Windows paths

2018-11-18 Thread Torsten Bögershausen
On Sun, Nov 18, 2018 at 07:21:58AM -0800, Steven Penny wrote:
> Cygwin programs can handle Unix form paths:
> 
>$ ls /var
>cache  lib  log  run  tmp
> 
> and also Windows form paths:
> 
>$ ls 'C:\cygwin64\var'
>cache  lib  log  run  tmp
> 
> However current Cygwin Git cannot:
> 
>$ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk'
>Cloning into 'C:\cygwin64\tmp\goawk'...
>fatal: Invalid path '/home/Steven/C:\cygwin64\tmp\goawk': No such file or
>directory
> 
> It seems the problem is that Git thinks the Windows form path is relative
> because it does not start with "/". A Git Bisect reveals this:
> 
> 05b458c104708141d2fad211d79703b3b99cc5a8 is the first bad commit
> commit 05b458c104708141d2fad211d79703b3b99cc5a8
> Author: Brandon Williams 
> Date:   Mon Dec 12 10:16:52 2016 -0800
> 
>real_path: resolve symlinks by hand
> 
>The current implementation of real_path uses chdir() in order to resolve
>symlinks.  Unfortunately this isn't thread-safe as chdir() affects a
>process as a whole and not just an individual thread.  Instead perform
>the symlink resolution by hand so that the calls to chdir() can be
>removed, making real_path one step closer to being reentrant.
> 
>Signed-off-by: Brandon Williams 
>Signed-off-by: Junio C Hamano 
> 
> This causes problems for any non-Cygwin tools that might call Git:
> 
> http://github.com/golang/go/issues/23155
> 

Thanks for the report
It seams as if "C:" is not recognized as an absolute path under
cygwin.
May be it should ?

Does the following help ? (fully untested)


diff --git a/compat/cygwin.h b/compat/cygwin.h
index 8e52de4644..12814e1edb 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -1,2 +1,4 @@
 int cygwin_offset_1st_component(const char *path);
 #define offset_1st_component cygwin_offset_1st_component
+#define has_dos_drive_prefix(path) \
+   (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)


Cygwin Git with Windows paths

2018-11-18 Thread Steven Penny

Cygwin programs can handle Unix form paths:

   $ ls /var
   cache  lib  log  run  tmp

and also Windows form paths:

   $ ls 'C:\cygwin64\var'
   cache  lib  log  run  tmp

However current Cygwin Git cannot:

   $ git clone git://github.com/benhoyt/goawk 'C:\cygwin64\tmp\goawk'
   Cloning into 'C:\cygwin64\tmp\goawk'...
   fatal: Invalid path '/home/Steven/C:\cygwin64\tmp\goawk': No such file or
   directory

It seems the problem is that Git thinks the Windows form path is relative
because it does not start with "/". A Git Bisect reveals this:

05b458c104708141d2fad211d79703b3b99cc5a8 is the first bad commit
commit 05b458c104708141d2fad211d79703b3b99cc5a8
Author: Brandon Williams 
Date:   Mon Dec 12 10:16:52 2016 -0800

   real_path: resolve symlinks by hand

   The current implementation of real_path uses chdir() in order to resolve
   symlinks.  Unfortunately this isn't thread-safe as chdir() affects a
   process as a whole and not just an individual thread.  Instead perform
   the symlink resolution by hand so that the calls to chdir() can be
   removed, making real_path one step closer to being reentrant.

   Signed-off-by: Brandon Williams 
   Signed-off-by: Junio C Hamano 

This causes problems for any non-Cygwin tools that might call Git:

http://github.com/golang/go/issues/23155



Re: [PATCH v3 1/1] merge: add scissors line on merge conflict

2018-11-18 Thread SZEDER Gábor
On Sat, Nov 17, 2018 at 06:32:33PM -0500, Denton Liu wrote:
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 106148254d..0d3db34f08 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -247,6 +247,54 @@ test_expect_success 'merge --squash c3 with c7' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success 'merge c3 with c7 with commit.cleanup = scissors' '
> + git config commit.cleanup scissors &&
> + git reset --hard c3 &&
> + test_must_fail git merge c7 &&
> + cat result.9z >file &&
> + git commit --no-edit -a &&
> +
> + {
> + cat <<-EOF
> + Merge tag '"'"'c7'"'"'
> +
> + #  >8 
> + # Do not modify or remove the line above.
> + # Everything below it will be ignored.

Note that these two lines of advice text are translated; see the
consequences below.

> + #
> + # Conflicts:
> + #   file
> + EOF
> + } >expect &&

The {...} block is unnecessary, because there is only a single command
in there.

> + git cat-file commit HEAD | sed -e '1,/^$/d' >actual &&

Please don't run git commands upstream of a pipe, because the pipe
hides their exit code.  Furthermore, put the sed script inside double
quotes, because the whole test is already in a single-quoted block.

I presume you wrote the test this way because you simply followed suit
of the previous test 'merge --squash c3 with c7', which did all the
same.  Bonus points for a preparatory patch that cleans up the
previous test ;)

> + test_cmp expect actual

But most importantly, here 'test_cmp' compares translated advice text
as well, which fails in the GETTEXT_POISON build.  Use 'test_i18ncmp'
instead.

> +'
> +
> +test_expect_success 'merge c3 with c7 with --squash commit.cleanup = 
> scissors' '
> + git config commit.cleanup scissors &&
> + git reset --hard c3 &&
> + test_must_fail git merge --squash c7 &&
> + cat result.9z >file &&
> + git commit --no-edit -a &&
> +
> + {
> + cat <<-EOF
> + Squashed commit of the following:
> +
> + $(git show -s c7)
> +
> + #  >8 
> + # Do not modify or remove the line above.
> + # Everything below it will be ignored.
> + #
> + # Conflicts:
> + #   file
> + EOF
> + } >expect &&
> + git cat-file commit HEAD | sed -e '1,/^$/d' >actual &&
> + test_cmp expect actual

Likewise.

> +'
> +
>  test_debug 'git log --graph --decorate --oneline --all'
>  
>  test_expect_success 'merge c1 with c2 and c3' '
> -- 
> 2.19.1
> 


[PATCH 1/1 v3] stash: tolerate missing user identity

2018-11-18 Thread Slavica Djukic
The "git stash" command insists on having a usable user identity to
the same degree as the "git commit-tree" and "git commit" commands
do, because it uses the same codepath that creates commit objects
as these commands.

It is not strictly necesary to do so. Check if we will barf before
creating commit objects and then supply fake identity to please the
machinery that creates commits.
Add test to document that stash executes correctly both with and
without valid ident.

This is not that much of usability improvement, as the users who run
"git stash" would eventually want to record their changes that are
temporarily stored in the stashes in a more permanent history by
committing, and they must do "git config user.{name,email}" at that
point anyway, so arguably this change is only delaying a step that
is necessary to work in the repository.

Helped-by: Junio C Hamano 
Signed-off-by: Slavica Djukic 
---
 git-stash.sh | 17 +
 t/t3903-stash.sh | 28 
 2 files changed, 45 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 94793c1a9..789ce2f41 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -55,6 +55,20 @@ untracked_files () {
git ls-files -o $z $excl_opt -- "$@"
 }
 
+prepare_fallback_ident () {
+   if ! git -c user.useconfigonly=yes var GIT_COMMITTER_IDENT >/dev/null 
2>&1
+   then
+   GIT_AUTHOR_NAME="git stash"
+   GIT_AUTHOR_EMAIL=git@stash
+   GIT_COMMITTER_NAME="git stash"
+   GIT_COMMITTER_EMAIL=git@stash
+   export GIT_AUTHOR_NAME
+   export GIT_AUTHOR_EMAIL
+   export GIT_COMMITTER_NAME
+   export GIT_COMMITTER_EMAIL
+   fi
+}
+
 clear_stash () {
if test $# != 0
then
@@ -67,6 +81,9 @@ clear_stash () {
 }
 
 create_stash () {
+
+   prepare_fallback_ident
+
stash_msg=
untracked=
while test $# != 0
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index cd216655b..5f8272b6f 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1096,4 +1096,32 @@ test_expect_success 'stash --  works with binary 
files' '
test_path_is_file subdir/untracked
 '
 
+test_expect_success 'stash works when user.name and user.email are not set' '
+   git reset &&
+   >1 &&
+   git add 1 &&
+   echo "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" >expect &&
+   git stash &&
+   git show -s --format="%an <%ae>" refs/stash >actual &&
+   test_cmp expect actual &&
+   >2 &&
+   git add 2 &&
+   test_config user.useconfigonly true &&
+   test_config stash.usebuiltin true &&
+   (
+   sane_unset GIT_AUTHOR_NAME &&
+   sane_unset GIT_AUTHOR_EMAIL &&
+   sane_unset GIT_COMMITTER_NAME &&
+   sane_unset GIT_COMMITTER_EMAIL &&
+   test_unconfig user.email &&
+   test_unconfig user.name &&
+   test_must_fail git commit -m "should fail" &&
+   echo "git stash " >expect &&
+   >2 &&
+   git stash &&
+   git show -s --format="%an <%ae>" refs/stash >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_done
-- 
2.19.1.1052.gd166e6afe



[PATCH 0/1 v3] make stash work if user.name and user.email are not configured

2018-11-18 Thread Slavica Djukic
Changes since v2:
* squash patch 1/2 and patch 2/2 into a single patch
* modify first part of test when there is valid ident
  present: create a stash, grab %an and %ae out of the 
  resulting commit object and compare to original ident
  
Slavica Djukic (1):
  stash: tolerate missing user identity

 git-stash.sh | 17 +
 t/t3903-stash.sh | 28 
 2 files changed, 45 insertions(+)

-- 
2.19.1.1052.gd166e6afe



Re: [PATCH v2] read-cache: write all indexes with the same permissions

2018-11-18 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 18 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  writes:
>
>> Do you mean that you don't agree that following should always create
>> both "foo" and e.g. ".git/refs/heads/master" with the same 644
>> (-rw-rw-r--) mode:
>>
>> (
>> rm -rf /tmp/repo &&
>> umask 022 &&
>> git init /tmp/repo &&
>> cd /tmp/repo &&
>> echo hi >foo &&
>> git add foo &&
>> git commit -m"first"
>> )
>>
>> To me what we should do with the standard umask and what
>> core.sharedRepository are for are completely different things.
>
> Ahh, of course.  If you put it that way, I do agree that it gives us
> a valid use case where core.sharedRepository is false and the umask
> of repository owner is set to 022 (or anything that does not allow
> write to group or others, and allows read to group) to let group
> members only peek but not touch the contents of the repository.
>
> I think I was distracted by the mention of ore.sharedRepository in
> the proposed log message.

Thanks. I'll submit a v3 with a less confusing commit message.


[PATCH v3 0/5] %(trailers) improvements in pretty format

2018-11-18 Thread Anders Waldenborg
Updated since v2:
 * Allow trailing colon in 'key=' argument
 * Clarify documentation on how matching is done
 * Rename option to "valueonly"
 * Make trailing matching a callback function
 * Avoid copying match string
 * Simplify generation of "expected" in tests
 * Rename function to strbuf_expand_literal_cb
 * cocci suggested fixes



Anders Waldenborg (5):
  pretty: single return path in %(trailers) handling
  pretty: allow showing specific trailers
  pretty: add support for "valueonly" option in %(trailers)
  strbuf: separate callback for strbuf_expand:ing literals
  pretty: add support for separator option in %(trailers)

 Documentation/pretty-formats.txt | 26 ---
 pretty.c | 68 ++--
 strbuf.c | 21 +
 strbuf.h |  8 
 t/t4205-log-pretty-formats.sh| 78 
 trailer.c| 25 --
 trailer.h|  4 ++
 7 files changed, 206 insertions(+), 24 deletions(-)

-- 
2.17.1



[PATCH v3 1/5] pretty: single return path in %(trailers) handling

2018-11-18 Thread Anders Waldenborg
No functional change intended.

This change may not seem useful on its own, but upcoming commits will do
memory allocation in there, and a single return path makes deallocation
easier.

Signed-off-by: Anders Waldenborg 
---
 pretty.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pretty.c b/pretty.c
index b83a3ecd2..aa03d5b23 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1312,6 +1312,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
 
if (skip_prefix(placeholder, "(trailers", )) {
struct process_trailer_options opts = 
PROCESS_TRAILER_OPTIONS_INIT;
+   size_t ret = 0;
 
opts.no_divider = 1;
 
@@ -1328,8 +1329,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
}
if (*arg == ')') {
format_trailers_from_commit(sb, msg + c->subject_off, 
);
-   return arg - placeholder + 1;
+   ret = arg - placeholder + 1;
}
+   return ret;
}
 
return 0;   /* unknown placeholder */
-- 
2.17.1



[PATCH v3 4/5] strbuf: separate callback for strbuf_expand:ing literals

2018-11-18 Thread Anders Waldenborg
Expanding '%n' and '%xNN' is generic functionality, so extract that from
the pretty.c formatter into a callback that can be reused.

No functional change intended

Signed-off-by: Anders Waldenborg 
---
 pretty.c | 16 +---
 strbuf.c | 21 +
 strbuf.h |  8 
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/pretty.c b/pretty.c
index 2e99f2418..819c5c50a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1094,9 +1094,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
const char *msg = c->message;
struct commit_list *p;
const char *arg;
-   int ch;
+   size_t res;
 
/* these are independent of the commit */
+   res = strbuf_expand_literal_cb(sb, placeholder, NULL);
+   if (res)
+   return res;
+
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
@@ -1115,16 +1119,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
 */
return ret;
}
-   case 'n':   /* newline */
-   strbuf_addch(sb, '\n');
-   return 1;
-   case 'x':
-   /* %x00 == NUL, %x0a == LF, etc. */
-   ch = hex2chr(placeholder + 1);
-   if (ch < 0)
-   return 0;
-   strbuf_addch(sb, ch);
-   return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
diff --git a/strbuf.c b/strbuf.c
index f6a6cf78b..78eecd29f 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -380,6 +380,27 @@ void strbuf_expand(struct strbuf *sb, const char *format, 
expand_fn_t fn,
}
 }
 
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+   const char *placeholder,
+   void *context)
+{
+   int ch;
+
+   switch (placeholder[0]) {
+   case 'n':   /* newline */
+   strbuf_addch(sb, '\n');
+   return 1;
+   case 'x':
+   /* %x00 == NUL, %x0a == LF, etc. */
+   ch = hex2chr(placeholder + 1);
+   if (ch < 0)
+   return 0;
+   strbuf_addch(sb, ch);
+   return 3;
+   }
+   return 0;
+}
+
 size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
void *context)
 {
diff --git a/strbuf.h b/strbuf.h
index fc40873b6..52e44c9ab 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -320,6 +320,14 @@ void strbuf_expand(struct strbuf *sb,
   expand_fn_t fn,
   void *context);
 
+/**
+ * Used as callback for `strbuf_expand` to only expand literals
+ * (i.e. %n and %xNN). The context argument is ignored.
+ */
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+   const char *placeholder,
+   void *context);
+
 /**
  * Used as callback for `strbuf_expand()`, expects an array of
  * struct strbuf_expand_dict_entry as context, i.e. pairs of
-- 
2.17.1



[PATCH v3 5/5] pretty: add support for separator option in %(trailers)

2018-11-18 Thread Anders Waldenborg
By default trailer lines are terminated by linebreaks ('\n'). By
specifying the new 'separator' option they will instead be separated by
user provided string and have separator semantics rather than terminator
semantics. The separator string can contain the literal formatting codes
%n and %xNN allowing it to be things that are otherwise hard to type as
%x00, or comma and end-parenthesis which would break parsing.

E.g:
 $ git log --pretty='%(trailers:key=Reviewed-by,valueonly,separator=%x00)'

Signed-off-by: Anders Waldenborg 
---
 Documentation/pretty-formats.txt | 13 +---
 pretty.c | 15 +
 t/t4205-log-pretty-formats.sh| 36 
 trailer.c| 15 +++--
 trailer.h|  1 +
 5 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 8cc8c3f9f..30e238338 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -218,9 +218,16 @@ endif::git-rev-list[]
  is given multiple times only last one is used.
   ** 'valueonly': skip over the key part of the trailer and only show
  the its value part.
-  ** Examples: `%(trailers:only,unfold)` unfolds and shows all trailer
- lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
- trailer lines with key `Reviewed-by`.
+  ** 'separator=': specifying an alternative separator than the
+ default line feed character. SEP may can contain the literal
+ formatting codes %n and %xNN allowing it to contain characters
+ that are hard to type such as %x00, or comma and end-parenthesis
+ which would break parsing. If option is given multiple times only
+ the last one is used.
+  ** Examples: `%(trailers:only,unfold,separator=%x00)` unfolds and
+ shows all trailer lines separated by NUL character,
+ `%(trailers:key=Reviewed-by,unfold)` unfolds and shows trailer
+ lines with key `Reviewed-by`.
 
 NOTE: Some placeholders may depend on other options given to the
 revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 819c5c50a..5b22a7237 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1318,6 +1318,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
if (skip_prefix(placeholder, "(trailers", )) {
struct process_trailer_options opts = 
PROCESS_TRAILER_OPTIONS_INIT;
struct format_trailer_match_data filter_data;
+   struct strbuf sepbuf = STRBUF_INIT;
size_t ret = 0;
 
opts.no_divider = 1;
@@ -1348,6 +1349,19 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
arg = end;
if (*arg == ',')
arg++;
+   } else if (skip_prefix(arg, "separator=", 
)) {
+   size_t seplen = strcspn(arg, ",)");
+   char *fmt;
+
+   strbuf_reset();
+   fmt = xstrndup(arg, seplen);
+   strbuf_expand(, fmt, 
strbuf_expand_literal_cb, NULL);
+   free(fmt);
+   opts.separator = 
+
+   arg += seplen;
+   if (*arg == ',')
+   arg++;
} else
break;
}
@@ -1356,6 +1370,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
format_trailers_from_commit(sb, msg + c->subject_off, 
);
ret = arg - placeholder + 1;
}
+   strbuf_release();
return ret;
}
 
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 095208d6b..562b56dda 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -640,6 +640,42 @@ test_expect_success '%(trailers:key=foo,valueonly) shows 
only value' '
test_cmp expect actual
 '
 
+test_expect_success 'pretty format %(trailers:separator) changes separator' '
+   git log --no-walk --pretty=format:"X%(trailers:separator=%x00,unfold)X" 
>actual &&
+   printf "XSigned-off-by: A U Thor \0Acked-by: A U 
Thor \0[ v2 updated patch description ]\0Signed-off-by: A U 
Thor X" >expect &&
+   test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers) combining 
separator/key/valueonly' '
+   git commit --allow-empty -F - <<-\EOF &&
+   Important fix
+
+   The fix is explained here
+
+   Closes: #1234
+   EOF
+
+   git commit --allow-empty -F - 

[PATCH v3 3/5] pretty: add support for "valueonly" option in %(trailers)

2018-11-18 Thread Anders Waldenborg
With the new "key=" option to %(trailers) it often makes little sense to
show the key, as it by definition already is know which trailer is
printed there. This new "valueonly" option makes it omit the key when
printing trailers.

E.g.:
 $ git show -s --pretty='%s%n%(trailers:key=Signed-off-by,valueonly)' 88182
will show:
 > upload-pack: fix broken if/else chain in config callback
 > Jeff King 
 > Junio C Hamano 

Signed-off-by: Anders Waldenborg 
---
 Documentation/pretty-formats.txt | 2 ++
 pretty.c | 2 ++
 t/t4205-log-pretty-formats.sh| 6 ++
 trailer.c| 6 --
 trailer.h| 1 +
 5 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 75c2e838d..8cc8c3f9f 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -216,6 +216,8 @@ endif::git-rev-list[]
   ** 'key=': only show trailers with specified key. Matching is
  done case-insensitively and trailing colon is optional. If option
  is given multiple times only last one is used.
+  ** 'valueonly': skip over the key part of the trailer and only show
+ the its value part.
   ** Examples: `%(trailers:only,unfold)` unfolds and shows all trailer
  lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
  trailer lines with key `Reviewed-by`.
diff --git a/pretty.c b/pretty.c
index aaedc8447..2e99f2418 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1335,6 +1335,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
opts.only_trailers = 1;
else if (match_placeholder_arg(arg, "unfold", 
))
opts.unfold = 1;
+   else if (match_placeholder_arg(arg, 
"valueonly", ))
+   opts.value_only = 1;
else if (skip_prefix(arg, "key=", )) {
const char *end = arg + strcspn(arg, 
",)");
 
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index aba7ba206..095208d6b 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -634,6 +634,12 @@ test_expect_success '%(trailers:key=foo,unfold) properly 
unfolds' '
test_cmp expect actual
 '
 
+test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
+   git log --no-walk --pretty="format:%(trailers:key=Acked-by,valueonly)" 
>actual &&
+   echo "A U Thor " >expect &&
+   test_cmp expect actual
+'
+
 test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index 97c8f2762..662c7ff03 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1150,8 +1150,10 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter || opts->filter(, 
opts->filter_data)) {
if (opts->unfold)
unfold_value();
-
-   strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+   if (!opts->value_only)
+   strbuf_addf(out, "%s: ", tok.buf);
+   strbuf_addbuf(out, );
+   strbuf_addch(out, '\n');
}
strbuf_release();
strbuf_release();
diff --git a/trailer.h b/trailer.h
index 5255b676d..06d417fe9 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,7 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+   int value_only;
int (*filter)(const struct strbuf *, void *);
void *filter_data;
 };
-- 
2.17.1



[PATCH v3 2/5] pretty: allow showing specific trailers

2018-11-18 Thread Anders Waldenborg
Adds a new "key=X" option to "%(trailers)" which will cause it to only
print trailers lines which match the specified key.

Signed-off-by: Anders Waldenborg 
---
 Documentation/pretty-formats.txt | 17 +--
 pretty.c | 31 ++-
 t/t4205-log-pretty-formats.sh| 36 
 trailer.c|  8 ---
 trailer.h|  2 ++
 5 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd..75c2e838d 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -207,13 +207,18 @@ endif::git-rev-list[]
   than given and there are spaces on its left, use those spaces
 - '%><()', '%><|()': similar to '%<()', '%<|()'
   respectively, but padding both sides (i.e. the text is centered)
-- %(trailers[:options]): display the trailers of the body as interpreted
+- '%(trailers[:options])': display the trailers of the body as interpreted
   by linkgit:git-interpret-trailers[1]. The `trailers` string may be
-  followed by a colon and zero or more comma-separated options. If the
-  `only` option is given, omit non-trailer lines from the trailer block.
-  If the `unfold` option is given, behave as if interpret-trailer's
-  `--unfold` option was given.  E.g., `%(trailers:only,unfold)` to do
-  both.
+  followed by a colon and zero or more comma-separated options:
+  ** 'only': omit non-trailer lines from the trailer block.
+  ** 'unfold': make it behave as if interpret-trailer's `--unfold`
+ option was given.
+  ** 'key=': only show trailers with specified key. Matching is
+ done case-insensitively and trailing colon is optional. If option
+ is given multiple times only last one is used.
+  ** Examples: `%(trailers:only,unfold)` unfolds and shows all trailer
+ lines, `%(trailers:key=Reviewed-by,unfold)` unfolds and shows
+ trailer lines with key `Reviewed-by`.
 
 NOTE: Some placeholders may depend on other options given to the
 revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index aa03d5b23..aaedc8447 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1074,6 +1074,17 @@ static int match_placeholder_arg(const char *to_parse, 
const char *candidate,
return 0;
 }
 
+struct format_trailer_match_data {
+   const char *trailer;
+   size_t trailer_len;
+};
+
+static int format_trailer_match_cb(const struct strbuf *sb, void *ud)
+{
+   struct format_trailer_match_data *data = ud;
+   return data->trailer_len == sb->len && !strncasecmp (data->trailer, 
sb->buf, sb->len);
+}
+
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1312,6 +1323,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
 
if (skip_prefix(placeholder, "(trailers", )) {
struct process_trailer_options opts = 
PROCESS_TRAILER_OPTIONS_INIT;
+   struct format_trailer_match_data filter_data;
size_t ret = 0;
 
opts.no_divider = 1;
@@ -1323,7 +1335,24 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
opts.only_trailers = 1;
else if (match_placeholder_arg(arg, "unfold", 
))
opts.unfold = 1;
-   else
+   else if (skip_prefix(arg, "key=", )) {
+   const char *end = arg + strcspn(arg, 
",)");
+
+   filter_data.trailer = arg;
+   filter_data.trailer_len = end - arg;
+
+   if (filter_data.trailer_len &&
+   
filter_data.trailer[filter_data.trailer_len - 1] == ':')
+   filter_data.trailer_len--;
+
+   opts.filter = format_trailer_match_cb;
+   opts.filter_data = _data;
+   opts.only_trailers = 1;
+
+   arg = end;
+   if (*arg == ',')
+   arg++;
+   } else
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66f..aba7ba206 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -598,6 +598,42 @@ test_expect_success ':only and :unfold work together' '
test_cmp expect actual
 '
 
+test_expect_success 'pretty format %(trailers:key=foo) shows that 

Attention: E-mail Address Owner,

2018-11-18 Thread WESTERN UNION OFFICE
Website: www.westernunion.com
Address: Plot 1261, Adela Hopewell Street CO/B/REP, Republic Of Benin.

Email: westernunibe...@seznam.cz


Attention: E-mail Address Owner,

Sequel to the meeting held with Federal Bureau of Investigation, The 
International Monetary Fund (IMF) is compensating all the scam victims and some 
email users which your name and email address was found on the list.

However, we have concluded to effect your own payment through Western Union® 
Money Transfer, $5,000 daily until the total sum of your compensation fund is 
transferred to you.

This is your first payment information:

MTCN#: 6486232830

Amount Programmed: $5.000

Track your first payment on-line now

https://www.westernunion.com/gb/en/self-service/app/tracktransfer

You are advised to get back to the contact person trough the email below for 
more direction on how to be receiving your payment

Contact person: . . SIR. INNOCENT JOHNSON
Email address: . . (westernunibe...@seznam.cz)


Thanks,
SIR.INNOCENT JOHNSON
Director Western Union Money Transfer,
Head Office Benin Republic.