Re: [PATCH v3 0/7] allocate cache entries from memory pool

[Junio C Hamano]

Jameson Miller  writes:

> Changes from V2:
>
>   - Tweak logic of finding available memory block for memory
>   allocation
>   
> - Only search head block

Hmph.  Is that because we generally do not free() a lot so once a
block is filled, there is not much chance that we have reclaimed
space in the block later?

>   - Tweaked handling of large memory allocations.
>   
> - Large blocks now tracked in same manner as "regular"
> blocks
> 
> - Large blocks are placed at end of linked list of memory
> blocks

If we are only carving out of the most recently allocated block, it
seems that there is no point looking for "the end", no?


>   - Cache_entry type gains notion of whether it was allocated
>   from memory pool or not
>   
> - Collapsed cache_entry discard logic into single
> function. This should make code easier to maintain

That certainly should be safer to have a back-pointer pointing to
which pool each entry came from, but doesn't it result in memory
bloat?

Re: [PATCH v3 2/7] block alloc: add lifecycle APIs for cache_entry structs

[Junio C Hamano]

Jameson Miller  writes:

> Add an API around managing the lifetime of cache_entry
> structs. Abstracting memory management details behind an API will
> allow for alternative memory management strategies without affecting
> all the call sites.  This commit does not change how memory is
> allocated / freed. A later commit in this series will allocate cache
> entries from memory pools as appropriate.
>
> Motivation:
> It has been observed that the time spent loading an index with a large
> number of entries is partly dominated by malloc() calls. This change
> is in preparation for using memory pools to reduce the number of
> malloc() calls made when loading an index.
>
> This API makes a distinction between cache entries that are intended
> for use with a particular index and cache entries that are not. This
> enables us to use the knowledge about how a cache entry will be used
> to make informed decisions about how to handle the corresponding
> memory.

Yuck.  make_index_cache_entry()?

Generally we use "cache" when working on the_index without passing
istate, and otherwise "index", which means that readers can assume
that distim_cache_entry(...)" is a shorter and more limited way to
say "distim_index_entry(_index, ...)".  Having both index and
cache in the same name smells crazy.

If most of the alocations are for permanent kind, give it a shorter
name call it make_cache_entry(_index, ...), and call the other
non-permanent one with a longer and more cumbersome name, perhaps
make_transient_cache_entry(...).  Avoid saying "index" in the former
name, as the design decision this series is making to allocate
memory for a cache-entry from a pool associated to an index_state is
already seen by what its first parameter is.

> diff --git a/cache.h b/cache.h
> index f0a407602c..204f788438 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -339,6 +339,29 @@ extern void remove_name_hash(struct index_state *istate, 
> struct cache_entry *ce)
>  extern void free_name_hash(struct index_state *istate);
>  
>  
> +/* Cache entry creation and freeing */
> +
> +/*
> + * Create cache_entry intended for use in the specified index. Caller
> + * is responsible for discarding the cache_entry with
> + * `discard_cache_entry`.
> + */
> +extern struct cache_entry *make_index_cache_entry(struct index_state 
> *istate, unsigned int mode, const unsigned char *sha1, const char *path, int 
> stage, unsigned int refresh_options);
> +extern struct cache_entry *make_empty_index_cache_entry(struct index_state 
> *istate, size_t name_len);
> +
> +/*
> + * Create a cache_entry that is not intended to be added to an index.
> + * Caller is responsible for discarding the cache_entry
> + * with `discard_cache_entry`.
> + */
> +extern struct cache_entry *make_transient_cache_entry(unsigned int mode, 
> const unsigned char *sha1, const char *path, int stage);
> +extern struct cache_entry *make_empty_transient_cache_entry(size_t name_len);
> +
> +/*
> + * Discard cache entry.
> + */
> +void discard_cache_entry(struct cache_entry *ce);

I am not yet convinced that it is a good idea to require each istate
to hold a separate pool.  Anything that uses unpack_trees() can do
"starting from this src_index, perform various mergy operations and
deposit the result in dst_index".  Sometimes the two logical indices
point at the same istate, sometimes different.  When src and dst are
different istates, the code that used to simply add another pointer
to the same ce to the dst index now needs to duplicate it out of the
pool associated with dst?

In any case, perhaps it will become clearer why it is a good idea as
we read on, so let's do so.

Re: [RFC PATCH v3 1/2] Implement --first-parent for git rev-list --bisect

[Junio C Hamano]

Tiago Botelho  writes:

> -static int count_interesting_parents(struct commit *commit)
> +static int count_interesting_parents(struct commit *commit, unsigned 
> bisect_flags)
>  {
>   struct commit_list *p;
>   int count;
>  
>   for (count = 0, p = commit->parents; p; p = p->next) {
> - if (p->item->object.flags & UNINTERESTING)
> - continue;
> - count++;
> + if (!(p->item->object.flags & UNINTERESTING))
> + count++;
> + if (bisect_flags & BISECT_FIRST_PARENT)
> + break;
>   }
>   return count;
>  }

Since this change makes the function never return anything more than 1,...

> @@ -310,7 +315,7 @@ static struct commit_list *do_find_bisection(struct 
> commit_list *list,
>   continue;
>   if (weight(p) != -2)
>   continue;
> - weight_set(p, count_distance(p));
> + weight_set(p, count_distance(p, bisect_flags));
>   clear_distance(list);

... this code will not be reached when in the first-parent mode,
where we count the weight for merge commits the hard way.

Am I reading the code correctly?  Not pointing out a problem; just
double checking how the code works.

> @@ -329,9 +334,10 @@ static struct commit_list *do_find_bisection(struct 
> commit_list *list,
>   if (0 <= weight(p))
>   continue;
>   for (q = p->item->parents; q; q = q->next) {
> - if (q->item->object.flags & UNINTERESTING)
> - continue;
> - if (0 <= weight(q))
> + if (!(q->item->object.flags & UNINTERESTING))
> + if (0 <= weight(q))
> + break;
> + if (bisect_flags & BISECT_FIRST_PARENT)
>   break;
>   }
>   if (!q)
>   continue;

This loop propagates the known weight of an ancestor through a
single strand of pearls.

When this loop is entered, any commit that has non-negative weight
has only one parent of interest, as we counted weight for all merges
the hard way and also weight for root and boundary commits, in the
previous loop.  The original goes through p's parents and see if an
interesting parent has non-negative weight (i.e. weight is known for
that parent), at which point break out of the loop, with q that is
non-NULL.  Immediately after the loop, q != NULL means we can now
compute weight for p based on q's weight.

I think this patch breaks the logic.  When we are looking at a
commit 'p' whose weight is not known yet, we grab its first parent
in 'q'.  Imagine that it is not an UNINTERESTING commit (i.e. a
commit whose weight matters when deciding the bisection) whose
weight is not yet known.  With the updated code under the
first-parent mode, we break out of the loop, 'q' pointing at the
commit whose weight is not yet known.  The computation done by the
code that immediately follows the above part is now broken, as it
will call weight(q) to get -1 and propagate it to p (or add 1 and
set 0 to p), no?

Perhaps something along this line may be closer to what we want:

if (0 <= weight(p))
continue; /* already known */
for (q = p->item->parents; q; q = q->next) {
if ((bisect_flags & BISECT_FIRST_PARENT)) {
if (weight(q) < 0)
q = NULL;
break;
}
if (q->item->object.flags & UNINTERESTING)
continue;
if (0 <= weight(q))
break;
}
if (!q)
continue; /* parent's weight not yet usable */

That is, under the first-parent mode, we would pay attention only to
the first parent of 'p' and break out of this loop without even
looking at q->next.  If the weight of that parent is not known yet,
we mark that fact by clearing q to NULL and break out of the loop.
If the weight is known, we do not clear q, so we compute weight of p
based on weight(q).

I am not offhand certain what should happen when p's first parent is
uninteresting.  The updated count_interesting_parents() would have
returned 0 for p in such a case, so at this point of the code where
p's weight is unknown, its first parent can never be UNINTERESTING,
I would think.

Re: [RFC PATCH v3 2/2] Add tests for rev-list --bisect* --first-parent

[Junio C Hamano]

Tiago Botelho  writes:

> Subject: [RFC PATCH v3 1/2] Implement --first-parent for git rev-list --bisect
> Subject: [RFC PATCH v3 2/2] Add tests for rev-list --bisect* --first-parent

perhaps

bisect: teach "git rev-list --bisect" to work with "--first-parent"
bisect: test "git rev-list --first-parent --bisect"

or soemthing?  I _think_ it is probably preferrable to have the test
in the primary patch, making these two patches into one.

> ---
>  t/t6002-rev-list-bisect.sh | 39 +++
>  1 file changed, 39 insertions(+)
>
> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh
> index a66140803..977c82157 100755
> --- a/t/t6002-rev-list-bisect.sh
> +++ b/t/t6002-rev-list-bisect.sh
> @@ -263,4 +263,43 @@ test_expect_success 'rev-parse --bisect can default to 
> good/bad refs' '
>   test_cmp expect.sorted actual.sorted
>  '
>  
> +# We generate the following commit graph:
> +#
> +# A
> +#/ \
> +#   D   B
> +#   |   |
> +#   EX  C
> +#\  /
> +# FX

Existing ascii art in the same script seems to draw the history
growing from bottom to top, the other way around.  Please be
consistent.  I think that we tend to draw simple histories growing
from left to right, and a more complex ones from bottom to top.

> +test_expect_success 'setup' '
> +  test_commit A &&
> +  test_commit B &&
> +  test_commit C &&
> +  git reset --hard A &&
> +  test_commit D &&
> +  test_commit EX &&
> +  test_merge FX C
> +'
> +
> +test_output_expect_success "--bisect --first-parent" 'git rev-list --bisect 
> --first-parent FX ^A' < +$(git rev-parse EX)
> +EOF

OK, because our range has odd number of commits on the first-parent
chain, the middle one is unambiguously the one to pick.

> +test_output_expect_success "--bisect-vars --first-parent" 'git rev-list 
> --bisect-vars --first-parent FX ^A' < +bisect_rev='$(git rev-parse EX)'
> +bisect_nr=1
> +bisect_good=0
> +bisect_bad=1
> +bisect_all=3
> +bisect_steps=1
> +EOF
> +
> +test_output_expect_success "--bisect-all --first-parent" 'git rev-list 
> --bisect-all --first-parent FX ^A' < +$(git rev-parse D) (dist=1)
> +$(git rev-parse EX) (dist=1)
> +$(git rev-parse FX) (dist=0)
> +EOF
> +
>  test_done

Proposal

[Miss Zeliha Omer Faruk]

Hello

Greetings to you please i have a business proposal for you contact me
for more detailes asap thanks.

Best Regards,
Miss.Zeliha ömer faruk
Esentepe Mahallesi Büyükdere
Caddesi Kristal Kule Binasi
No:215
Sisli - Istanbul, Turkey

What's cooking in git.git (May 2018, #03; Wed, 23)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* ab/perl-python-attrs (2018-04-27) 3 commits
  (merged to 'next' on 2018-05-08 at b440e9bbb9)
 + .gitattributes: add a diff driver for Python
 + .gitattributes: use the "perl" differ for Perl
 + .gitattributes: add *.pl extension for Perl

 We learned that our source files with ".pl" and ".py" extensions
 are Perl and Python files respectively and changes to them are
 better viewed as such with appropriate diff drivers.


* ah/misc-doc-updates (2018-05-06) 7 commits
  (merged to 'next' on 2018-05-16 at e2e3b68a66)
 + doc: normalize [--options] to [options] in git-diff
 + doc: add note about shell quoting to revision.txt
 + git-svn: remove ''--add-author-from' for 'commit-diff'
 + doc: add '-d' and '-o' for 'git push'
 + doc: clarify ignore rules for git ls-files
 + doc: align 'diff --no-index' in text and synopsis
 + doc: improve formatting in githooks.txt

 Misc doc fixes.


* ao/config-api-doc (2018-05-11) 1 commit
  (merged to 'next' on 2018-05-22 at a17d6dda92)
 + doc: fix config API documentation about config_with_options

 Doc update.


* bc/asciidoctor-tab-width (2018-05-07) 2 commits
  (merged to 'next' on 2018-05-16 at be2a42c473)
 + Documentation: render revisions correctly under Asciidoctor
 + Documentation: use 8-space tabs with Asciidoctor

 Asciidoctor gives a reasonable imitation for AsciiDoc, but does not
 render illustration in a literal block correctly when indented with
 HT by default. The problem is fixed by forcing 8-space tabs.


* bc/format-patch-cover-no-attach (2018-05-02) 1 commit
  (merged to 'next' on 2018-05-16 at fa1ffeb3fe)
 + format-patch: make cover letters always text/plain

 "git format-patch --cover --attach" created a broken MIME multipart
 message for the cover letter, which has been fixed by keeping the
 cover letter as plain text file.


* bc/mailmap-self (2018-05-08) 1 commit
  (merged to 'next' on 2018-05-16 at a009c64bd2)
 + mailmap: update brian m. carlson's email address


* bp/test-drop-caches (2018-05-04) 1 commit
  (merged to 'next' on 2018-05-16 at 0e40ab2408)
 + test-drop-caches: simplify delay loading of NtSetSystemInformation

 Code simplification.


* bw/server-options (2018-04-24) 4 commits
  (merged to 'next' on 2018-05-08 at a18ce56f3c)
 + fetch: send server options when using protocol v2
 + ls-remote: send server options when using protocol v2
 + serve: introduce the server-option capability
 + Merge branch 'bw/protocol-v2' into HEAD

 The transport protocol v2 is getting updated further.


* cc/perf-aggregate-unknown-option (2018-04-26) 1 commit
  (merged to 'next' on 2018-05-08 at db7d2870f8)
 + perf/aggregate: use Getopt::Long for option parsing

 Perf-test helper updates.


* cc/perf-bisect (2018-05-06) 1 commit
  (merged to 'next' on 2018-05-16 at 5a078a2fdf)
 + perf/bisect_run_script: disable codespeed

 Performance test updates.


* ds/lazy-load-trees (2018-05-02) 6 commits
  (merged to 'next' on 2018-05-02 at d54016d9e3)
 + coccinelle: avoid wrong transformation suggestions from commit.cocci
  (merged to 'next' on 2018-04-25 at b90813f421)
 + commit-graph: lazy-load trees for commits
 + treewide: replace maybe_tree with accessor methods
 + commit: create get_commit_tree() method
 + treewide: rename tree to maybe_tree
 + Merge branch 'bw/c-plus-plus' into ds/lazy-load-trees
 (this branch is used by ds/commit-graph-lockfile-fix and 
ds/generation-numbers.)

 The code has been taught to use the duplicated information stored
 in the commit-graph file to learn the tree object name for a commit
 to avoid opening and parsing the commit object when it makes sense
 to do so.


* em/status-rename-config (2018-05-06) 1 commit
  (merged to 'next' on 2018-05-16 at 33c1cc093c)
 + wt-status: use settings from git_diff_ui_config
 (this branch is used by bp/status-rename-config.)

 "git status" learned to pay attention to UI related diff
 configuration variables such as diff.renames.


* en/git-debugger (2018-04-25) 1 commit
  (merged to 'next' on 2018-05-08 at 73369cd1e5)
 + Make running git under other debugger-like programs easy

 Dev support.


* en/rename-directory-detection-reboot (2018-05-08) 36 commits
  (merged to 'next' on 2018-05-08 at be350ebc17)
 + merge-recursive: fix check for skipability of working tree updates
 + merge-recursive: make "Auto-merging" comment show for other merges
 + merge-recursive: fix remainder of was_dirty() to use original index
 + merge-recursive: fix was_tracked() to quit lying with some renamed paths
 + t6046: testcases 

Re: [PATCH] Add initial support for pax extended attributes

[Junio C Hamano]

Jeff King  writes:

> I do think we'd fail to notice the truncation, which isn't ideal. But it
> looks like the rest of the script suffers from the same issue.
>
> If anybody cares, it might not be too hard to wrap all of the 512-byte
> read calls into a helper that dies on bogus input.

Perhaps.  In any case, the patch presented here is an improvement
over the status quo, so let's move the world forward by taking it
without any further "while at it" fixes, which can come later when
people feel inclined to do so.

Thanks, both, for writing and reviewing ;-)

Re: should config options be treated as case-sensitive?

[Junio C Hamano]

"Robert P. J. Day"  writes:

>> Unfortunately, that line of thinking leads us to madness, as you are
>> exhibiting the typical symptom of "my today's immediate itch is the
>> most important one in the world"-itis
>
>   fair enough, point taken.

FWIW, everybody suffers from it, including me.

I was trying to come up with an update, and here is an attempted
rewrite of the earlier section.  I am not sure if this is a good
direction to go in, but if so, we'd need to reduce the duplicated
info from the Syntax section that immediately follows.

 Documentation/config.txt | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git i/Documentation/config.txt w/Documentation/config.txt
index 84e2891aed..5b79411b4b 100644
--- i/Documentation/config.txt
+++ w/Documentation/config.txt
@@ -9,13 +9,21 @@ fallback values for the `.git/config` file. The file 
`/etc/gitconfig`
 can be used to store a system-wide default configuration.
 
 The configuration variables are used by both the Git plumbing
-and the porcelains. The variables are divided into sections, wherein
-the fully qualified variable name of the variable itself is the last
-dot-separated segment and the section name is everything before the last
-dot. The variable names are case-insensitive, allow only alphanumeric
-characters and `-`, and must start with an alphabetic character.  Some
-variables may appear multiple times; we say then that the variable is
-multivalued.
+and the porcelains. The variables are divided into sections, and some
+sections can have subsections.  In a variable name that is fully
+spelled out, the part up to the first dot is the section, the part
+after the last dot is the variable.  If these two dots are not the
+same, what's in the middle is the subsection.
+
+The section and the variable names are case-insensitive, allow only
+alphanumeric characters and `-`, and must start with an alphabetic
+character.  Often multi-word variable names are spelled in CamelCase
+by convention for extra readability.
+
+Some variables may appear multiple times and their effects accumulate;
+we say then that such a variable is multivalued.  For other variables, 
+when they appear more than once, the last one takes effect.
+
 
 Syntax
 ~~

[PATCH v2 1/1] import-tars: read overlong names from pax extended header

[Pedro Alvarez]

From: Pedro Alvarez Piedehierro 

Importing gcc tarballs[1] with import-tars script (in contrib) fails
when hitting a pax extended header.

Make sure we always read the extended attributes from the pax entries,
and store the 'path' value if found to be used in the next ustar entry.

The code to parse pax extended headers was written consulting the Pax
Pax Interchange Format documentation [2].

[1] http://ftp.gnu.org/gnu/gcc/gcc-7.3.0/gcc-7.3.0.tar.xz
[2] 
https://www.freebsd.org/cgi/man.cgi?manpath=FreeBSD+8-current=tar=5

Signed-off-by: Pedro Alvarez 
---
 contrib/fast-import/import-tars.perl | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/contrib/fast-import/import-tars.perl 
b/contrib/fast-import/import-tars.perl
index d60b4315ed..e800d9f5c9 100755
--- a/contrib/fast-import/import-tars.perl
+++ b/contrib/fast-import/import-tars.perl
@@ -63,6 +63,8 @@ foreach my $tar_file (@ARGV)
my $have_top_dir = 1;
my ($top_dir, %files);
 
+   my $next_path = '';
+
while (read(I, $_, 512) == 512) {
my ($name, $mode, $uid, $gid, $size, $mtime,
$chksum, $typeflag, $linkname, $magic,
@@ -70,6 +72,13 @@ foreach my $tar_file (@ARGV)
$prefix) = unpack 'Z100 Z8 Z8 Z8 Z12 Z12
Z8 Z1 Z100 Z6
Z2 Z32 Z32 Z8 Z8 Z*', $_;
+
+   unless ($next_path eq '') {
+   # Recover name from previous extended header
+   $name = $next_path;
+   $next_path = '';
+   }
+
last unless length($name);
if ($name eq '././@LongLink') {
# GNU tar extension
@@ -90,13 +99,31 @@ foreach my $tar_file (@ARGV)
Z8 Z1 Z100 Z6
Z2 Z32 Z32 Z8 Z8 Z*', $_;
}
-   next if $name =~ m{/\z};
$mode = oct $mode;
$size = oct $size;
$mtime = oct $mtime;
next if $typeflag == 5; # directory
 
-   if ($typeflag != 1) { # handle hard links later
+   if ($typeflag eq 'x') { # extended header
+   # If extended header, check for path
+   my $pax_header = '';
+   while ($size > 0 && read(I, $_, 512) == 512) {
+   $pax_header = $pax_header . substr($_, 0, 
$size);
+   $size -= 512;
+   }
+
+   my @lines = split /\n/, $pax_header;
+   foreach my $line (@lines) {
+   my ($len, $entry) = split / /, $line;
+   my ($key, $value) = split /=/, $entry;
+   if ($key eq 'path') {
+   $next_path = $value;
+   }
+   }
+   next;
+   } elsif ($name =~ m{/\z}) { # directory
+   next;
+   } elsif ($typeflag != 1) { # handle hard links later
print FI "blob\n", "mark :$next_mark\n";
if ($typeflag == 2) { # symbolic link
print FI "data ", length($linkname), "\n",
-- 
2.11.0

[PATCH v2 0/1] import-tars: read overlong names from pax extended header

[Pedro Alvarez]

From: Pedro Alvarez Piedehierro 

Hello!

In this version I've trimmed and improved the commit message as suggested.

Regarding the error handling, as Jeff mentioned, could be improved
in general in the entire script. But I guess I could do it if needed
to get this patch approved.

Thanks for reviewing and giving me some feedback!

Pedro.

Pedro Alvarez Piedehierro (1):
  import-tars: read overlong names from pax extended header

 contrib/fast-import/import-tars.perl | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

-- 
2.11.0

Re: [PATCH 2/2] rev-parse: verify that commit looked up is not NULL

[Todd Zullinger]

I wrote:
> Thanks.  This fixes the segfault.  While I was testing this,
> I wondered if the following cases should differ:

Nevermind me.  Jeff beat me to a reply and included much
more useful details about why this occurs and suggestions
for fixing it. :)

> #  f*40
> $ ./git-rev-parse ^@ ; echo $?
> 0
> 
> #  f*39
> $ ./git-rev-parse fff^@ ; echo $?
> fff^@
> fatal: ambiguous argument 'fff^@': 
> unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git  [...] -- [...]'
> 128
> 
> Looking a little further, this is deeper than the rev-parse
> handling.  The difference in how these invalid refs are
> handled appears in 'git show' as well.  With 'git show' a
> (different) fatal error is returned in both cases.
> 
> #  f*40
> $ git show 
> fatal: bad object 
> 
> #  39*f
> $ git show fff
> fatal: ambiguous argument 'fff': unknown 
> revision or path not in the working tree.
> Use '--' to separate paths from revisions, like this:
> 'git  [...] -- [...]'
> 
> Should rev-parse return an error as well, rather than
> silenty succeeding?

-- 
Todd
~~
How can I tell that the past isn't a fiction designed to account for
the discrepancy between my immediate physical sensation and my state
of mind?
-- Douglas Adams

[PATCHv6 0/1] git-p4: unshelve

[Luke Diamand]

This just removes the verbose print change, which will end up causing
conflicts later since it's also being fixed in another commit.

Discussed here:

https://public-inbox.org/git/byapr08mb38455afe85ae6b04eb31ef92da...@byapr08mb3845.namprd08.prod.outlook.com/

Luke Diamand (1):
  git-p4: add unshelve command

 Documentation/git-p4.txt |  32 ++
 git-p4.py| 213 ---
 t/t9832-unshelve.sh  | 138 +
 3 files changed, 347 insertions(+), 36 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

-- 
2.17.0.392.gdeb1a6e9b7

[PATCHv6 1/1] git-p4: add unshelve command

[Luke Diamand]

This can be used to "unshelve" a shelved P4 commit into
a git commit.

For example:

  $ git p4 unshelve 12345

The resulting commit ends up in the branch:
   refs/remotes/p4/unshelved/12345

If that branch already exists, it is renamed - for example
the above branch would be saved as p4/unshelved/12345.1.

git-p4 checks that the shelved changelist is based on files
which are at the same Perforce revision as the origin branch
being used for the unshelve (HEAD by default). If they are not,
it will refuse to unshelve. This is to ensure that the unshelved
change does not contain other changes mixed-in.

The reference branch can be changed manually with the "--origin"
option.

The change adds a new Unshelve command class. This just runs the
existing P4Sync code tweaked to handle a shelved changelist.

Signed-off-by: Luke Diamand 
---
 Documentation/git-p4.txt |  32 ++
 git-p4.py| 213 ---
 t/t9832-unshelve.sh  | 138 +
 3 files changed, 347 insertions(+), 36 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..d3cb249fc2 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,31 @@ $ git p4 submit --shelve
 $ git p4 submit --update-shelve 1234 --update-shelve 2345
 
 
+
+Unshelve
+
+Unshelving will take a shelved P4 changelist, and produce the equivalent git 
commit
+in the branch refs/remotes/p4/unshelved/.
+
+The git commit is created relative to the current origin revision (HEAD by 
default).
+If the shelved changelist's parent revisions differ, git-p4 will refuse to 
unshelve;
+you need to be unshelving onto an equivalent tree.
+
+The origin revision can be changed with the "--origin" option.
+
+If the target branch in refs/remotes/p4/unshelved already exists, the old one 
will
+be renamed.
+
+
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+
+$ git p4 unshelve 12345
+
+
+
+
 OPTIONS
 ---
 
@@ -337,6 +362,13 @@ These options can be used to modify 'git p4 rebase' 
behavior.
 --import-labels::
Import p4 labels.
 
+Unshelve options
+
+
+--origin::
+Sets the git refspec against which the shelved P4 changelist is compared.
+Defaults to p4/master.
+
 DEPOT PATH SYNTAX
 -
 The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..74d58afad3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
 results = p4CmdList(["changes", "-m", "1"], skip_info=True)
 return int(results[0]['change'])
 
-def p4_describe(change):
+def p4_describe(change, shelved=False):
 """Make sure it returns a valid result by checking for
the presence of field "time".  Return a dict of the
results."""
 
-ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+cmd = ["describe", "-s"]
+if shelved:
+cmd += ["-S"]
+cmd += [str(change)]
+
+ds = p4CmdList(cmd, skip_info=True)
 if len(ds) != 1:
 die("p4 describe -s %d did not return 1 result: %s" % (change, 
str(ds)))
 
@@ -662,6 +667,12 @@ def gitBranchExists(branch):
 stderr=subprocess.PIPE, stdout=subprocess.PIPE);
 return proc.wait() == 0;
 
+def gitUpdateRef(ref, newvalue):
+subprocess.check_call(["git", "update-ref", ref, newvalue])
+
+def gitDeleteRef(ref):
+subprocess.check_call(["git", "update-ref", "-d", ref])
+
 _gitConfig = {}
 
 def gitConfig(key, typeSpecifier=None):
@@ -2411,6 +2422,7 @@ class P4Sync(Command, P4UserMap):
 self.tempBranches = []
 self.tempBranchLocation = "refs/git-p4-tmp"
 self.largeFileSystem = None
+self.suppress_meta_comment = False
 
 if gitConfig('git-p4.largeFileSystem'):
 largeFileSystemConstructor = 
globals()[gitConfig('git-p4.largeFileSystem')]
@@ -2421,6 +2433,18 @@ class P4Sync(Command, P4UserMap):
 if gitConfig("git-p4.syncFromOrigin") == "false":
 self.syncWithOrigin = False
 
+self.depotPaths = []
+self.changeRange = ""
+self.previousDepotPaths = []
+self.hasOrigin = False
+
+# map from branch depot path to parent branch
+self.knownBranches = {}
+self.initialParents = {}
+
+self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 
3600) / 60))
+self.labels = {}
+
 # Force a checkpoint in fast-import and wait for it to finish
 def checkpoint(self):
 self.gitStream.write("checkpoint\n\n")
@@ -2429,7 +2453,20 @@ class P4Sync(Command, P4UserMap):
 if self.verbose:
 print "checkpoint finished: " + out
 
-def extractFilesFromCommit(self, commit):
+def cmp_shelved(self, path, filerev, revision):
+""" Determine if a path at revision 

Re: [PATCH 2/2] rev-parse: verify that commit looked up is not NULL

[Todd Zullinger]

Elijah Newren wrote:
> In commit 2122f8b963d4 ("rev-parse: Add support for the ^! and ^@ syntax",
> 2008-07-26), try_parent_shorthands() was introduced to parse the special
> ^! and ^@ syntax.  However, it did not check the commit returned from
> lookup_commit_reference() before proceeding to use it.  If it is NULL,
> bail early and notify the caller that this cannot be a valid revision
> range.

Thanks.  This fixes the segfault.  While I was testing this,
I wondered if the following cases should differ:

#  f*40
$ ./git-rev-parse ^@ ; echo $?
0

#  f*39
$ ./git-rev-parse fff^@ ; echo $?
fff^@
fatal: ambiguous argument 'fff^@': unknown 
revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git  [...] -- [...]'
128

Looking a little further, this is deeper than the rev-parse
handling.  The difference in how these invalid refs are
handled appears in 'git show' as well.  With 'git show' a
(different) fatal error is returned in both cases.

#  f*40
$ git show 
fatal: bad object 

#  39*f
$ git show fff
fatal: ambiguous argument 'fff': unknown 
revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git  [...] -- [...]'

Should rev-parse return an error as well, rather than
silenty succeeding?

-- 
Todd
~~
I refuse to spend my life worrying about what I eat. There is no
pleasure worth foregoing just for an extra three years in the
geriatric ward.
-- John Mortimer

Re: [PATCH 1/2] t6101: add a test for rev-parse $garbage^@

[Jeff King]

On Wed, May 23, 2018 at 01:46:12PM -0700, Elijah Newren wrote:

> diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
> index 8c617981a3..7b1b2dbdf2 100755
> --- a/t/t6101-rev-parse-parents.sh
> +++ b/t/t6101-rev-parse-parents.sh
> @@ -214,4 +214,8 @@ test_expect_success 'rev-list merge^-1x (garbage after 
> ^-1)' '
>   test_must_fail git rev-list merge^-1x
>  '
>  
> +test_expect_failure 'rev-parse $garbage^@ should not segfault' '
> + git rev-parse ^@
> +'

Two small nits. :)

It may just be me, but for a trivial test+fix like this, I'd rather see
them in the same commit (both for reviewing, and when I'm digging in the
history later).

The second nit is that we may want to use something a little more
symbolic and easier to read here. Thirty-nine f's behaves quite
differently than forty. And eventually we'd like to move away from
having hard-coded commit ids anyway (this is obviously a fake one, but
the length may end up changing).

Perhaps "git rev-parse $EMPTY_TREE^@", which triggers the same bug?

-Peff

Re: [PATCH 2/2] rev-parse: verify that commit looked up is not NULL

[Jeff King]

On Wed, May 23, 2018 at 01:46:13PM -0700, Elijah Newren wrote:

> In commit 2122f8b963d4 ("rev-parse: Add support for the ^! and ^@ syntax",
> 2008-07-26), try_parent_shorthands() was introduced to parse the special
> ^! and ^@ syntax.  However, it did not check the commit returned from
> lookup_commit_reference() before proceeding to use it.  If it is NULL,
> bail early and notify the caller that this cannot be a valid revision
> range.

Yep, this is definitely the right track. But...

> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 55c0b90441..4e9ba9641a 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -334,6 +334,8 @@ static int try_parent_shorthands(const char *arg)
>   }
>  
>   commit = lookup_commit_reference();
> + if (!commit)
> + return 1;
>   if (exclude_parent &&
>   exclude_parent > commit_list_count(commit->parents)) {
>   *dotdot = '^';

...I don't think this is quite right. I see two issues:

  1. We need to restore "*dotdot" like the other exit code-paths do.

  2. I think a return of 1 means "yes, I handled this". We want to
 return 0 so that the bogus name eventually triggers an error.

I also wondered if we need to print an error message, but since we are
using the non-gentle form of lookup_commit_reference(), it will complain
for us (and then the caller will issue some errors as well).

It might make sense to just lump this into the get_oid check above.
E.g., something like:

  if (get_oid_committish(arg, ) ||
  !(commit = lookup_commit_reference())) {
  *dotdot = '^';
  return 0;
  }

though I am fine with it either way.

> diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
> index 7b1b2dbdf2..f91cc417bd 100755
> --- a/t/t6101-rev-parse-parents.sh
> +++ b/t/t6101-rev-parse-parents.sh
> @@ -214,7 +214,7 @@ test_expect_success 'rev-list merge^-1x (garbage after 
> ^-1)' '
>   test_must_fail git rev-list merge^-1x
>  '
>  
> -test_expect_failure 'rev-parse $garbage^@ should not segfault' '
> +test_expect_success 'rev-parse $garbage^@ should not segfault' '
>   git rev-parse ^@
>  '

Once we flip the return value as above, I think this needs to be
test_must_fail, which matches how I'd expect it to behave.

This code (sadly) duplicates the functionality in revision.c. I checked
there to see if it has the same problem, but it's fine.

Unfortunately I think rev-parse has one other instance, though:

  bogus=

  # this is ok; we just normalize to "$bogus ^$bogus" without looking at
  # the object, which is OK
  git rev-parse $bogus..$bogus

  # this segfaults, because we try to feed NULL to get_merge_bases()
  git rev-parse $bogus...$bogus

We should probably fix that at the same time.

-Peff

[PATCH 1/2] t6101: add a test for rev-parse $garbage^@

[Elijah Newren]

Reported by Florian Weimer and Todd Zullinger.

Signed-off-by: Elijah Newren 
---
 t/t6101-rev-parse-parents.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 8c617981a3..7b1b2dbdf2 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -214,4 +214,8 @@ test_expect_success 'rev-list merge^-1x (garbage after 
^-1)' '
test_must_fail git rev-list merge^-1x
 '
 
+test_expect_failure 'rev-parse $garbage^@ should not segfault' '
+   git rev-parse ^@
+'
+
 test_done
-- 
2.17.0.1025.g36b5c64692

[PATCH 2/2] rev-parse: verify that commit looked up is not NULL

[Elijah Newren]

In commit 2122f8b963d4 ("rev-parse: Add support for the ^! and ^@ syntax",
2008-07-26), try_parent_shorthands() was introduced to parse the special
^! and ^@ syntax.  However, it did not check the commit returned from
lookup_commit_reference() before proceeding to use it.  If it is NULL,
bail early and notify the caller that this cannot be a valid revision
range.

Signed-off-by: Elijah Newren 
---
 builtin/rev-parse.c  | 2 ++
 t/t6101-rev-parse-parents.sh | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 55c0b90441..4e9ba9641a 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -334,6 +334,8 @@ static int try_parent_shorthands(const char *arg)
}
 
commit = lookup_commit_reference();
+   if (!commit)
+   return 1;
if (exclude_parent &&
exclude_parent > commit_list_count(commit->parents)) {
*dotdot = '^';
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 7b1b2dbdf2..f91cc417bd 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -214,7 +214,7 @@ test_expect_success 'rev-list merge^-1x (garbage after 
^-1)' '
test_must_fail git rev-list merge^-1x
 '
 
-test_expect_failure 'rev-parse $garbage^@ should not segfault' '
+test_expect_success 'rev-parse $garbage^@ should not segfault' '
git rev-parse ^@
 '
 
-- 
2.17.0.1025.g36b5c64692

Re: BUG: rev-parse segfault with invalid input

[Todd Zullinger]

Hi,

Elijah Newren wrote:
> Thanks for the detailed report.  This apparently goes back to
> git-1.6.0 with commit 2122f8b963d4 ("rev-parse: Add support for the ^!
> and ^@ syntax", 2008-07-26).  We aren't checking that the commit from
> lookup_commit_reference() is non-NULL before proceeding.  Looks like
> it's simple to fix.  I'll send a patch shortly...

Thanks Elijah!  I thought it was likely to be a simple fix.
But I also don't know the area well and that kept me from
being too ambitious about suggesting a fix or the difficulty
of one. :)

-- 
Todd
~~
I believe in the noble, aristocratic art of doing absolutely nothing.
And someday, I hope to be in a position where I can do even less.

Re: BUG: rev-parse segfault with invalid input

[Elijah Newren]

On Wed, May 23, 2018 at 12:52 PM, Todd Zullinger  wrote:
> Hi,
>
> Certain invalid input causes git rev-parse to crash rather
> than return a 'fatal: ambiguous argument ...' error.
>
> This was reported against the Fedora git package:
>
> https://bugzilla.redhat.com/1581678
>
> Simple reproduction recipe and analysis, from the bug:
>
> $ git init
> Initialized empty Git repository in /tmp/t/.git/
> $ git rev-parse ^@
> Segmentation fault (core dumped)
>
> gdb) break lookup_commit_reference
> Breakpoint 1 at 0x55609f00: lookup_commit_reference. (3 locations)
> (gdb) r
> Starting program: /usr/bin/git rev-parse 
> \^@
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
>
> Breakpoint 1, lookup_commit_reference (oid=oid@entry=0x7fffd550) at 
> commit.c:34
> 34  return lookup_commit_reference_gently(oid, 0);
> (gdb) finish
> Run till exit from #0  lookup_commit_reference 
> (oid=oid@entry=0x7fffd550) at commit.c:34
> try_parent_shorthands (arg=0x7fffdd44 'f' ) at 
> builtin/rev-parse.c:314
> 314 include_parents = 1;
> Value returned is $1 = (struct commit *) 0x0
> (gdb) c
>
> (gdb) c
> Continuing.
>
> Program received signal SIGSEGV, Segmentation fault.
> try_parent_shorthands (arg=0x7fffdd44 'f' ) at 
> builtin/rev-parse.c:345
> 345 for (parents = commit->parents, parent_number = 1;
> (gdb) l 336,+15
> 336 commit = lookup_commit_reference();
> 337 if (exclude_parent &&
> 338 exclude_parent > commit_list_count(commit->parents)) {
> 339 *dotdot = '^';
> 340 return 0;
> 341 }
> 342
> 343 if (include_rev)
> 344 show_rev(NORMAL, , arg);
> 345 for (parents = commit->parents, parent_number = 1;
> 346  parents;
> 347  parents = parents->next, parent_number++) {
> 348 char *name = NULL;
> 349
> 350 if (exclude_parent && parent_number != 
> exclude_parent)
> 351 continue;
>
> Looks like a null pointer check is missing.
>
> This occurs on master and as far back as 1.8.3.1 (what's in
> RHEL-6, I didn't try to test anything older).  Only a string
> with 40 valid hex characters and ^@, @-, of ^!  seems to
> trigger it.

Thanks for the detailed report.  This apparently goes back to
git-1.6.0 with commit 2122f8b963d4 ("rev-parse: Add support for the ^!
and ^@ syntax", 2008-07-26).  We aren't checking that the commit from
lookup_commit_reference() is non-NULL before proceeding.  Looks like
it's simple to fix.  I'll send a patch shortly...

BUG: rev-parse segfault with invalid input

[Todd Zullinger]

Hi,

Certain invalid input causes git rev-parse to crash rather
than return a 'fatal: ambiguous argument ...' error.

This was reported against the Fedora git package:

https://bugzilla.redhat.com/1581678

Simple reproduction recipe and analysis, from the bug:

$ git init
Initialized empty Git repository in /tmp/t/.git/
$ git rev-parse ^@
Segmentation fault (core dumped)

gdb) break lookup_commit_reference
Breakpoint 1 at 0x55609f00: lookup_commit_reference. (3 locations)
(gdb) r
Starting program: /usr/bin/git rev-parse 
\^@
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, lookup_commit_reference (oid=oid@entry=0x7fffd550) at 
commit.c:34
34  return lookup_commit_reference_gently(oid, 0);
(gdb) finish
Run till exit from #0  lookup_commit_reference 
(oid=oid@entry=0x7fffd550) at commit.c:34
try_parent_shorthands (arg=0x7fffdd44 'f' ) at 
builtin/rev-parse.c:314
314 include_parents = 1;
Value returned is $1 = (struct commit *) 0x0
(gdb) c

(gdb) c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
try_parent_shorthands (arg=0x7fffdd44 'f' ) at 
builtin/rev-parse.c:345
345 for (parents = commit->parents, parent_number = 1;
(gdb) l 336,+15
336 commit = lookup_commit_reference();
337 if (exclude_parent &&
338 exclude_parent > commit_list_count(commit->parents)) {
339 *dotdot = '^';
340 return 0;
341 }
342 
343 if (include_rev)
344 show_rev(NORMAL, , arg);
345 for (parents = commit->parents, parent_number = 1;
346  parents;
347  parents = parents->next, parent_number++) {
348 char *name = NULL;
349 
350 if (exclude_parent && parent_number != 
exclude_parent)
351 continue;

Looks like a null pointer check is missing.

This occurs on master and as far back as 1.8.3.1 (what's in
RHEL-6, I didn't try to test anything older).  Only a string
with 40 valid hex characters and ^@, @-, of ^!  seems to
trigger it.

-- 
Todd
~~
I don't mind arguing with myself. It's when I lose that it bothers me.
-- Richard Powers

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Mazo, Andrey]

>> This was actually discussed in a separate thread [1] some time ago with 
>> patches proposed by Thandesha and me.
>> I haven't yet got time to cook a final patch, which addresses both 
>> Thandesha's and mine use-cases though,
>> so this wasn't submitted to Junio yet.
>> In the meantime, I guess, one of the patches [2] from that thread can be 
>> taken as is.
>>
>> [1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
>>    
>>https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
>> [2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
>>    
>>https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/
>
> Should I re-roll my patch without this change then?
If it's the question to me, then I'm fine either way -- I can rebase my changes 
easily.
However, re-rolling your patch would probably make the aforementioned fileSize 
patch apply cleanly to both maint and master branches.


Thank you,
Andrey Mazo

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Luke Diamand]

On 23 May 2018 at 17:41, Mazo, Andrey  wrote:
>> The last one (i.e. "even if it is verbose, if fileSize is not
>> reported, do not write the verbose output") does not look like it is
>> limited to the unshelve feature, so it might, even though it is a
>> one-liner, deserve to be a separate preparatory patch if you want.
>> But I do not feel strongly about either way.
>
> This was actually discussed in a separate thread [1] some time ago with 
> patches proposed by Thandesha and me.
> I haven't yet got time to cook a final patch, which addresses both 
> Thandesha's and mine use-cases though,
> so this wasn't submitted to Junio yet.
> In the meantime, I guess, one of the patches [2] from that thread can be 
> taken as is.
>
> [1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
>   
> https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
> [2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
>   
> https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/

Should I re-roll my patch without this change then?

Luke

"git grep" and "working tree" vs "working directory"

[Robert P. J. Day]

  more puzzling terminology, this time in the man page for "git grep".
the SYNOPSIS shows, at the very end, the clearly optional
"[...]",

git grep ...
   ... snip ...
   [--] [...]

but nowhere in the man page is there an explanation as to the default
value used if there is no pathspec, and here's why that's confusing.

  first, what is the proper phrase for the *entire* checked out repo?
working tree? working directory? either? and is that the proper phrase
to use *regardless* of where you happen to be located, perhaps in a
subdirectory?

  i did a quick test and, if i don't supply a pathspec, then "git
grep" (quite reasonably) recursively searches only the *current*
working directory (example from linux kernel source repo):

  $ cd scripts
  $ git grep -il torvalds --
  checkstack.pl
  get_maintainer.pl
  package/mkdebian
  $

however, if you peruse the very first part of the OPTIONS section of
that man page, you read:

  --cached
  Instead of searching tracked files in the working tree,
  search blobs registered in the index file.

  --no-index
  Search files in the current directory that is not managed by Git.

  --untracked
  In addition to searching in the tracked files in the
  working tree, search also in untracked files.

  ... snip ...

note how a couple of those options are described as searching "the
working tree", when they clearly(?) do no such thing if you happen to
be located in a subdirectory.

  also, at the bottom of that section, one reads:

  ...
  If given, limit the search to paths matching at least one
  pattern. Both leading paths match and glob(7) patterns are supported.

  For more details about the  syntax, see the pathspec
  entry in gitglossary(7).

but, again, what if  is *not* given? then what?

  finally, the first example has the same problem:

  git grep 'time_t' -- '*.[ch]'
  Looks for time_t in all tracked .c and .h files in the
  working directory and its subdirectories.

in "the working directory"?

  what is the proper terminology to be used here?

rday

-- 


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

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

Re: Weird revision walk behaviour

[Jeff King]

On Wed, May 23, 2018 at 01:32:46PM -0400, Jeff King wrote:

> On Wed, May 23, 2018 at 07:10:58PM +0200, SZEDER Gábor wrote:
> 
> >   $ git log --oneline master..ba95710a3b -- ci/
> >   ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2
> > 
> > But as far as I can tell, there are no changes in the 'ci/' directory
> > on any of the merge's parents:
> > 
> >   $ git log --oneline master..ea44c0a594^1 -- ci/
> >   # Nothing.
> >   $ git log --oneline master..ea44c0a594^2 -- ci/
> >   # Nothing!
> 
> Hmm. That commit does touch "ci/" with respect to one of its parents.
> It should get simplified away because it completely matches the other
> parent, so it does sound like a bug.
> 
> > This is not specific to the 'ci/' directory, it seems that any
> > untouched directory does the trick:
> > 
> >   $ git log --oneline master..ea44c0a594 -- contrib/coccinelle/ t/lib-httpd/
> >   ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2
> 
> Both of those directories also differ between one parent. If you try it
> with "contrib/remote-helpers", which does not, then the commit does not
> appear.
> 
> So it does seem like a bug where we should be simplifying away the merge
> but are not (or I'm missing the corner case, too ;) ).
> 
> > I get the same behavior with Git built from current master and from
> > past releases as well (tried it as far back as v2.0.0).
> 
> I keep some older builds around, and it does not reproduce with v1.6.6.3
> (that's my usual goto for "old"). Bisecting turns up d0af663e42
> (revision.c: Make --full-history consider more merges, 2013-05-16).  It
> looks like an unintended change (the commit message claims that the
> non-full-history case shouldn't be affected).

There's more discussion in the thread at:

  
https://public-inbox.org/git/1366658602-12254-1-git-send-email-ke...@bracey.fi/

I haven't absorbed it all yet, but I'm adding Junio to the cc.

-Peff

Re: Weird revision walk behaviour

[Jeff King]

On Wed, May 23, 2018 at 07:10:58PM +0200, SZEDER Gábor wrote:

>   $ git log --oneline master..ba95710a3b -- ci/
>   ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2
> 
> But as far as I can tell, there are no changes in the 'ci/' directory
> on any of the merge's parents:
> 
>   $ git log --oneline master..ea44c0a594^1 -- ci/
>   # Nothing.
>   $ git log --oneline master..ea44c0a594^2 -- ci/
>   # Nothing!

Hmm. That commit does touch "ci/" with respect to one of its parents.
It should get simplified away because it completely matches the other
parent, so it does sound like a bug.

> This is not specific to the 'ci/' directory, it seems that any
> untouched directory does the trick:
> 
>   $ git log --oneline master..ea44c0a594 -- contrib/coccinelle/ t/lib-httpd/
>   ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2

Both of those directories also differ between one parent. If you try it
with "contrib/remote-helpers", which does not, then the commit does not
appear.

So it does seem like a bug where we should be simplifying away the merge
but are not (or I'm missing the corner case, too ;) ).

> I get the same behavior with Git built from current master and from
> past releases as well (tried it as far back as v2.0.0).

I keep some older builds around, and it does not reproduce with v1.6.6.3
(that's my usual goto for "old"). Bisecting turns up d0af663e42
(revision.c: Make --full-history consider more merges, 2013-05-16).  It
looks like an unintended change (the commit message claims that the
non-full-history case shouldn't be affected).

-Peff

Weird revision walk behaviour

[SZEDER Gábor]

There is this topic 'jt/partial-clone-proto-v2' currently cooking in
'next' and pointing to ba95710a3b ({fetch,upload}-pack: support filter
in protocol v2, 2018-05-03).  This topic is built on top of the merge
commit ea44c0a594 (Merge branch 'bw/protocol-v2' into
jt/partial-clone-proto-v2, 2018-05-02), which gives me the creeps,
because it shows up in some pathspec-limited revision walks where in
my opinion it should not:

  $ git log --oneline master..ba95710a3b -- ci/
  ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2

But as far as I can tell, there are no changes in the 'ci/' directory
on any of the merge's parents:

  $ git log --oneline master..ea44c0a594^1 -- ci/
  # Nothing.
  $ git log --oneline master..ea44c0a594^2 -- ci/
  # Nothing!

And to add to my confusion:

  $ git log -1 --oneline master@{1.week.ago}
  ccdcbd54c4 The fifth batch for 2.18
  $ git log --oneline master@{1.week.ago}..ea44c0a594 -- ci/
  ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2
  $ git log -1 --oneline master@{3.week.ago}
  1f1cddd558 The fourth batch for 2.18
  $ git log --oneline master@{3.week.ago}..ea44c0a594 -- ci/
  # Nothing, as it is supposed to be, IMHO.

This is not specific to the 'ci/' directory, it seems that any
untouched directory does the trick:

  $ git log --oneline master..ea44c0a594 -- contrib/coccinelle/ t/lib-httpd/
  ea44c0a594 Merge branch 'bw/protocol-v2' into jt/partial-clone-proto-v2
  $ git log --oneline master..ea44c0a594^1 -- contrib/coccinelle/ t/lib-httpd/
  # Nothing.
  $ git log --oneline master..ea44c0a594^2 -- contrib/coccinelle/ t/lib-httpd/
  # Nothing.
  $ git log --oneline master@{3.week.ago}..ea44c0a594 --
contrib/coccinelle/ t/lib-httpd/
  # Nothing, but this is what I would expect.

I get the same behavior with Git built from current master and from
past releases as well (tried it as far back as v2.0.0).

So...  what's going on here? :)
A bug?  Or am I missing something?  Some history simplification corner
case that I'm unaware of?

Re: [PATCH 1/1] git-p4: unshelve: use action==add instead of rev==none

[Mazo, Andrey]

> The last one (i.e. "even if it is verbose, if fileSize is not
> reported, do not write the verbose output") does not look like it is
> limited to the unshelve feature, so it might, even though it is a
> one-liner, deserve to be a separate preparatory patch if you want.
> But I do not feel strongly about either way.

This was actually discussed in a separate thread [1] some time ago with patches 
proposed by Thandesha and me.
I haven't yet got time to cook a final patch, which addresses both Thandesha's 
and mine use-cases though,
so this wasn't submitted to Junio yet.
In the meantime, I guess, one of the patches [2] from that thread can be taken 
as is.

[1] "[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key"
  
https://public-inbox.org/git/cajjpmi-plb4qcka5alkxa8b1vozfff+oaq0fguq9yviobrp...@mail.gmail.com/t/#mee2ec50a40242089741f808f06214a44278055b3
[2] "[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'"
  
https://public-inbox.org/git/2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.am...@checkvideo.com/

Thank you,
Andrey Mazo

Re: bug: --shallow-since misbehaves on old branch heads

[Duy Nguyen]

I probably can't look into this until the weekend. Just wanted to let
you know that I've seen this mail and, being the one who introduced
--shallow-since, I will look into it soon (unless someone beats me to
it of course).
-- 
Duy

[PATCH v3 7/7] block alloc: add validations around cache_entry lifecyle

[Jameson Miller]

Add an option (controlled by an environment variable) perform extra
validations on mem_pool allocated cache entries. When set:

  1) Invalidate cache_entry memory when discarding cache_entry.

  2) When discarding index_state struct, verify that all cache_entries
 were allocated from expected mem_pool.

  3) When discarding mem_pools, invalidate mem_pool memory.

This should provide extra checks that mem_pools and their allocated
cache_entries are being used as expected.

Signed-off-by: Jameson Miller 
---
 cache.h  |  6 ++
 git.c|  3 +++
 mem-pool.c   |  7 ++-
 mem-pool.h   |  2 +-
 read-cache.c | 55 +--
 5 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index 7aae9c8db0..2916e953ad 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,12 @@ extern struct cache_entry 
*make_empty_transient_cache_entry(size_t name_len);
  */
 void discard_cache_entry(struct cache_entry *ce);
 
+/*
+ * Check configuration if we should perform extra validation on cache
+ * entries.
+ */
+int should_validate_cache_entries(void);
+
 /*
  * Duplicate a cache_entry. Allocate memory for the new entry from a
  * memory_pool. Takes into account cache_entry fields that are meant
diff --git a/git.c b/git.c
index bab6bbfded..7ce65eab78 100644
--- a/git.c
+++ b/git.c
@@ -347,7 +347,10 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
 
trace_argv_printf(argv, "trace: built-in: git");
 
+   validate_cache_entries(_index);
status = p->fn(argc, argv, prefix);
+   validate_cache_entries(_index);
+
if (status)
return status;
 
diff --git a/mem-pool.c b/mem-pool.c
index cc7d3a7ab1..6770b4f740 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -63,13 +63,18 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t 
initial_size)
*mem_pool = pool;
 }
 
-void mem_pool_discard(struct mem_pool *mem_pool)
+void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
 {
struct mp_block *block, *block_to_free;
+
for (block = mem_pool->mp_block; block;)
{
block_to_free = block;
block = block->next_block;
+
+   if (invalidate_memory)
+   memset(block_to_free->space, 0xDD, ((char 
*)block_to_free->end) - ((char *)block_to_free->space));
+
free(block_to_free);
}
 
diff --git a/mem-pool.h b/mem-pool.h
index 5c892d3bdb..68d8428902 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -29,7 +29,7 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t 
initial_size);
 /*
  * Discard a memory pool and free all the memory it is responsible for.
  */
-void mem_pool_discard(struct mem_pool *mem_pool);
+void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory);
 
 /*
  * Alloc memory from the mem_pool.
diff --git a/read-cache.c b/read-cache.c
index 02fe5d333c..fb2cec6ac6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2023,8 +2023,10 @@ int discard_index(struct index_state *istate)
 * Cache entries in istate->cache[] should have been allocated
 * from the memory pool associated with this index, or from an
 * associated split_index. There is no need to free individual
-* cache entries.
+* cache entries. validate_cache_entries can detect when this
+* assertion does not hold.
 */
+   validate_cache_entries(istate);
 
resolve_undo_clear_index(istate);
istate->cache_nr = 0;
@@ -2041,13 +2043,45 @@ int discard_index(struct index_state *istate)
istate->untracked = NULL;
 
if (istate->ce_mem_pool) {
-   mem_pool_discard(istate->ce_mem_pool);
+   mem_pool_discard(istate->ce_mem_pool, 
should_validate_cache_entries());
istate->ce_mem_pool = NULL;
}
 
return 0;
 }
 
+/*
+ * Validate the cache entries of this index.
+ * All cache entries associated with this index
+ * should have been allocated by the memory pool
+ * associated with this index, or by a referenced
+ * split index.
+ */
+void validate_cache_entries(const struct index_state *istate)
+{
+   int i;
+
+   if (!should_validate_cache_entries() ||!istate || !istate->initialized)
+   return;
+
+   for (i = 0; i < istate->cache_nr; i++) {
+   if (!istate) {
+   die("internal error: cache entry is not allocated from 
expected memory pool");
+   } else if (!istate->ce_mem_pool ||
+   !mem_pool_contains(istate->ce_mem_pool, 
istate->cache[i])) {
+   if (!istate->split_index ||
+   !istate->split_index->base ||
+   !istate->split_index->base->ce_mem_pool ||
+   
!mem_pool_contains(istate->split_index->base->ce_mem_pool, istate->cache[i])) {
+ 

[PATCH v3 4/7] mem-pool: add lifecycle management functions

[Jameson Miller]

Add initialization and discard functions to mem-pool type. As part of
this, we now also track "large" allocations of memory so that these
can also be cleaned up when discarding the memory pool.

These changes are in preparation for a future commit that will utilize
creating and discarding memory pool.

Signed-off-by: Jameson Miller 
---
 fast-import.c |  2 +-
 mem-pool.c| 63 +++
 mem-pool.h| 12 +++-
 3 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 34edf3fb8f..571898e5db 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -300,7 +300,7 @@ static int global_argc;
 static const char **global_argv;
 
 /* Memory pools */
-static struct mem_pool fi_mem_pool =  {NULL, 2*1024*1024 -
+static struct mem_pool fi_mem_pool =  {NULL, NULL, 2*1024*1024 -
   sizeof(struct mp_block), 0 };
 
 /* Atom management */
diff --git a/mem-pool.c b/mem-pool.c
index c80124f1fe..01595bcca5 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -5,20 +5,77 @@
 #include "cache.h"
 #include "mem-pool.h"
 
+#define BLOCK_GROWTH_SIZE 1024*1024 - sizeof(struct mp_block);
+
 static struct mp_block *mem_pool_alloc_block(struct mem_pool *mem_pool, size_t 
block_alloc)
 {
struct mp_block *p;
 
mem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;
p = xmalloc(st_add(sizeof(struct mp_block), block_alloc));
+
p->next_block = mem_pool->mp_block;
p->next_free = (char *)p->space;
p->end = p->next_free + block_alloc;
+
mem_pool->mp_block = p;
 
+   if (!mem_pool->mp_block_tail)
+   mem_pool->mp_block_tail = p;
+
+   return p;
+}
+
+static void *mem_pool_alloc_custom(struct mem_pool *mem_pool, size_t 
block_alloc)
+{
+   struct mp_block *p;
+
+   mem_pool->pool_alloc += sizeof(struct mp_block) + block_alloc;
+   p = xmalloc(st_add(sizeof(struct mp_block), block_alloc));
+
+   p->next_block = NULL;
+   p->next_free = (char *)p->space;
+   p->end = p->next_free + block_alloc;
+
+   if (mem_pool->mp_block_tail)
+   mem_pool->mp_block_tail->next_block = p;
+   else
+   mem_pool->mp_block = p;
+
+   mem_pool->mp_block_tail = p;
return p;
 }
 
+void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size)
+{
+   struct mem_pool *pool;
+
+   if (*mem_pool)
+   return;
+
+   pool = xcalloc(1, sizeof(*pool));
+
+   pool->block_alloc = BLOCK_GROWTH_SIZE;
+
+   if (initial_size > 0)
+   mem_pool_alloc_block(pool, initial_size);
+
+   *mem_pool = pool;
+}
+
+void mem_pool_discard(struct mem_pool *mem_pool)
+{
+   struct mp_block *block, *block_to_free;
+   for (block = mem_pool->mp_block; block;)
+   {
+   block_to_free = block;
+   block = block->next_block;
+   free(block_to_free);
+   }
+
+   free(mem_pool);
+}
+
 void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)
 {
struct mp_block *p = NULL;
@@ -33,10 +90,8 @@ void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)
p = mem_pool->mp_block;
 
if (!p) {
-   if (len >= (mem_pool->block_alloc / 2)) {
-   mem_pool->pool_alloc += len;
-   return xmalloc(len);
-   }
+   if (len >= (mem_pool->block_alloc / 2))
+   return mem_pool_alloc_custom(mem_pool, len);
 
p = mem_pool_alloc_block(mem_pool, mem_pool->block_alloc);
}
diff --git a/mem-pool.h b/mem-pool.h
index 829ad58ecf..5d3e6a367a 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -9,7 +9,7 @@ struct mp_block {
 };
 
 struct mem_pool {
-   struct mp_block *mp_block;
+   struct mp_block *mp_block, *mp_block_tail;
 
/*
 * The amount of available memory to grow the pool by.
@@ -21,6 +21,16 @@ struct mem_pool {
size_t pool_alloc;
 };
 
+/*
+ * Initialize mem_pool with specified initial size.
+ */
+void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size);
+
+/*
+ * Discard a memory pool and free all the memory it is responsible for.
+ */
+void mem_pool_discard(struct mem_pool *mem_pool);
+
 /*
  * Alloc memory from the mem_pool.
  */
-- 
2.14.3

[PATCH v3 5/7] mem-pool: fill out functionality

[Jameson Miller]

Add functions for:

- combining two memory pools

- determining if a memory address is within the range managed by a
  memory pool

These functions will be used by future commits.

Signed-off-by: Jameson Miller 
---
 mem-pool.c | 40 
 mem-pool.h | 13 +
 2 files changed, 53 insertions(+)

diff --git a/mem-pool.c b/mem-pool.c
index 01595bcca5..cc7d3a7ab1 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -108,3 +108,43 @@ void *mem_pool_calloc(struct mem_pool *mem_pool, size_t 
count, size_t size)
memset(r, 0, len);
return r;
 }
+
+int mem_pool_contains(struct mem_pool *mem_pool, void *mem)
+{
+   struct mp_block *p;
+
+   /* Check if memory is allocated in a block */
+   for (p = mem_pool->mp_block; p; p = p->next_block)
+   if ((mem >= ((void *)p->space)) &&
+   (mem < ((void *)p->end)))
+   return 1;
+
+   return 0;
+}
+
+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src)
+{
+   /* Append the blocks from src to dst */
+   if (dst->mp_block && src->mp_block) {
+   /*
+* src and dst have blocks, append
+* blocks from src to dst.
+*/
+   dst->mp_block_tail->next_block = src->mp_block;
+   dst->mp_block_tail = src->mp_block_tail;
+   } else if (src->mp_block) {
+   /*
+* src has blocks, dst is empty
+* use pointers from src to set up dst.
+*/
+   dst->mp_block = src->mp_block;
+   dst->mp_block_tail = src->mp_block_tail;
+   } else {
+   // src is empty, nothing to do.
+   }
+
+   dst->pool_alloc += src->pool_alloc;
+   src->pool_alloc = 0;
+   src->mp_block = NULL;
+   src->mp_block_tail = NULL;
+}
diff --git a/mem-pool.h b/mem-pool.h
index 5d3e6a367a..5c892d3bdb 100644
--- a/mem-pool.h
+++ b/mem-pool.h
@@ -41,4 +41,17 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len);
  */
 void *mem_pool_calloc(struct mem_pool *pool, size_t count, size_t size);
 
+/*
+ * Move the memory associated with the 'src' pool to the 'dst' pool. The 'src'
+ * pool will be empty and not contain any memory. It still needs to be free'd
+ * with a call to `mem_pool_discard`.
+ */
+void mem_pool_combine(struct mem_pool *dst, struct mem_pool *src);
+
+/*
+ * Check if a memory pointed at by 'mem' is part of the range of
+ * memory managed by the specified mem_pool.
+ */
+int mem_pool_contains(struct mem_pool *mem_pool, void *mem);
+
 #endif
-- 
2.14.3

[PATCH v3 3/7] mem-pool: only search head block for available space

[Jameson Miller]

Instead of searching all memory blocks for available space to fulfill
a memory request, only search the head block. If the head block does
not have space, assume that previous block would most likely not be
able to fulfill request either. This could potentially lead to more
memory fragmentation, but also avoids searching memory blocks that
probably will not be able to fulfill request.

This pattern will benefit consumers that are able to generate a good
estimate for how much memory will be needed, or if they are performing
fixed sized allocations, so that once a block is exhausted it will
never be able to fulfill a future request.

Signed-off-by: Jameson Miller 
---
 mem-pool.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mem-pool.c b/mem-pool.c
index 389d7af447..c80124f1fe 100644
--- a/mem-pool.c
+++ b/mem-pool.c
@@ -21,16 +21,16 @@ static struct mp_block *mem_pool_alloc_block(struct 
mem_pool *mem_pool, size_t b
 
 void *mem_pool_alloc(struct mem_pool *mem_pool, size_t len)
 {
-   struct mp_block *p;
+   struct mp_block *p = NULL;
void *r;
 
/* round up to a 'uintmax_t' alignment */
if (len & (sizeof(uintmax_t) - 1))
len += sizeof(uintmax_t) - (len & (sizeof(uintmax_t) - 1));
 
-   for (p = mem_pool->mp_block; p; p = p->next_block)
-   if (p->end - p->next_free >= len)
-   break;
+   if (mem_pool->mp_block &&
+   mem_pool->mp_block->end - mem_pool->mp_block->next_free >= len)
+   p = mem_pool->mp_block;
 
if (!p) {
if (len >= (mem_pool->block_alloc / 2)) {
-- 
2.14.3

[PATCH v3 6/7] block alloc: allocate cache entries from mem_pool

[Jameson Miller]

When reading large indexes from disk, a portion of the time is
dominated in malloc() calls. This can be mitigated by allocating a
large block of memory and manage it ourselves via memory pools.

This change moves the cache entry allocation to be on top of memory
pools.

Design:

The index_state struct will gain a notion of an associated memory_pool
from which cache_entries will be allocated from. When reading in the
index from disk, we have information on the number of entries and
their size, which can guide us in deciding how large our initial
memory allocation should be. When an index is discarded, the
associated memory_pool will be discarded as well - so the lifetime of
a cache_entry is tied to the lifetime of the index_state that it was
allocated for.

In the case of a Split Index, the following rules are followed. 1st,
some terminology is defined:

Terminology:
  - 'the_index': represents the logical view of the index

  - 'split_index': represents the "base" cache entries. Read from the
split index file.

'the_index' can reference a single split_index, as well as
cache_entries from the split_index. `the_index` will be discarded
before the `split_index` is.  This means that when we are allocating
cache_entries in the presence of a split index, we need to allocate
the entries from the `split_index`'s memory pool.  This allows us to
follow the pattern that `the_index` can reference cache_entries from
the `split_index`, and that the cache_entries will not be freed while
they are still being referenced.

Signed-off-by: Jameson Miller 
---
 cache.h|  21 ++
 read-cache.c   | 119 -
 split-index.c  |  50 
 unpack-trees.c |  13 +--
 4 files changed, 165 insertions(+), 38 deletions(-)

diff --git a/cache.h b/cache.h
index 204f788438..7aae9c8db0 100644
--- a/cache.h
+++ b/cache.h
@@ -15,6 +15,7 @@
 #include "path.h"
 #include "sha1-array.h"
 #include "repository.h"
+#include "mem-pool.h"
 
 #include 
 typedef struct git_zstream {
@@ -156,6 +157,7 @@ struct cache_entry {
struct stat_data ce_stat_data;
unsigned int ce_mode;
unsigned int ce_flags;
+   unsigned int mem_pool_allocated;
unsigned int ce_namelen;
unsigned int index; /* for link extension */
struct object_id oid;
@@ -227,6 +229,7 @@ static inline void copy_cache_entry(struct cache_entry *dst,
const struct cache_entry *src)
 {
unsigned int state = dst->ce_flags & CE_HASHED;
+   int mem_pool_allocated = dst->mem_pool_allocated;
 
/* Don't copy hash chain and name */
memcpy(>ce_stat_data, >ce_stat_data,
@@ -235,6 +238,9 @@ static inline void copy_cache_entry(struct cache_entry *dst,
 
/* Restore the hash state */
dst->ce_flags = (dst->ce_flags & ~CE_HASHED) | state;
+
+   /* Restore the mem_pool_allocated flag */
+   dst->mem_pool_allocated = mem_pool_allocated;
 }
 
 static inline unsigned create_ce_flags(unsigned stage)
@@ -328,6 +334,7 @@ struct index_state {
struct untracked_cache *untracked;
uint64_t fsmonitor_last_update;
struct ewah_bitmap *fsmonitor_dirty;
+   struct mem_pool *ce_mem_pool;
 };
 
 extern struct index_state the_index;
@@ -362,6 +369,20 @@ extern struct cache_entry 
*make_empty_transient_cache_entry(size_t name_len);
  */
 void discard_cache_entry(struct cache_entry *ce);
 
+/*
+ * Duplicate a cache_entry. Allocate memory for the new entry from a
+ * memory_pool. Takes into account cache_entry fields that are meant
+ * for managing the underlying memory allocation of the cache_entry.
+ */
+struct cache_entry *dup_cache_entry(const struct cache_entry *ce, struct 
index_state *istate);
+
+/*
+ * Validate the cache entries in the index.  This is an internal
+ * consistency check that the cache_entry structs are allocated from
+ * the expected memory pool.
+ */
+void validate_cache_entries(const struct index_state *istate);
+
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
 #define active_cache (the_index.cache)
 #define active_nr (the_index.cache_nr)
diff --git a/read-cache.c b/read-cache.c
index d51cc83312..02fe5d333c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -46,6 +46,48 @@
 CE_ENTRY_ADDED | CE_ENTRY_REMOVED | CE_ENTRY_CHANGED | \
 SPLIT_INDEX_ORDERED | UNTRACKED_CHANGED | FSMONITOR_CHANGED)
 
+
+/*
+ * This is an estimate of the pathname length in the index.  We use
+ * this for V4 index files to guess the un-deltafied size of the index
+ * in memory because of pathname deltafication.  This is not required
+ * for V2/V3 index formats because their pathnames are not compressed.
+ * If the initial amount of memory set aside is not sufficient, the
+ * mem pool will allocate extra memory.
+ */
+#define CACHE_ENTRY_PATH_LENGTH 80
+
+static inline struct cache_entry *mem_pool__ce_alloc(struct mem_pool 
*mem_pool, 

[PATCH v3 1/7] read-cache: teach refresh_cache_entry() to take istate

[Jameson Miller]

Refactor refresh_cache_entry() to work on a specific index, instead of
implicitly using the_index. This is in preparation for making the
make_cache_entry function work on a specific index.

Signed-off-by: Jameson Miller 
---
 cache.h   | 2 +-
 merge-recursive.c | 2 +-
 read-cache.c  | 7 ---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/cache.h b/cache.h
index 0c1fb9fbcc..f0a407602c 100644
--- a/cache.h
+++ b/cache.h
@@ -744,7 +744,7 @@ extern void fill_stat_cache_info(struct cache_entry *ce, 
struct stat *st);
 #define REFRESH_IGNORE_SUBMODULES  0x0010  /* ignore submodules */
 #define REFRESH_IN_PORCELAIN   0x0020  /* user friendly output, not "needs 
update" */
 extern int refresh_index(struct index_state *, unsigned int flags, const 
struct pathspec *pathspec, char *seen, const char *header_msg);
-extern struct cache_entry *refresh_cache_entry(struct cache_entry *, unsigned 
int);
+extern struct cache_entry *refresh_cache_entry(struct index_state *, struct 
cache_entry *, unsigned int);
 
 /*
  * Opportunistically update the index but do not complain if we can't.
diff --git a/merge-recursive.c b/merge-recursive.c
index 0c0d48624d..693f60e0a3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -260,7 +260,7 @@ static int add_cacheinfo(struct merge_options *o,
if (refresh) {
struct cache_entry *nce;
 
-   nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | 
CE_MATCH_IGNORE_MISSING);
+   nce = refresh_cache_entry(_index, ce, CE_MATCH_REFRESH | 
CE_MATCH_IGNORE_MISSING);
if (!nce)
return err(o, _("addinfo_cache failed for path '%s'"), 
path);
if (nce != ce)
diff --git a/read-cache.c b/read-cache.c
index 10f1c6bb8a..2cb4f53b57 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -767,7 +767,7 @@ struct cache_entry *make_cache_entry(unsigned int mode,
ce->ce_namelen = len;
ce->ce_mode = create_ce_mode(mode);
 
-   ret = refresh_cache_entry(ce, refresh_options);
+   ret = refresh_cache_entry(_index, ce, refresh_options);
if (ret != ce)
free(ce);
return ret;
@@ -1448,10 +1448,11 @@ int refresh_index(struct index_state *istate, unsigned 
int flags,
return has_errors;
 }
 
-struct cache_entry *refresh_cache_entry(struct cache_entry *ce,
+struct cache_entry *refresh_cache_entry(struct index_state *istate,
+  struct cache_entry *ce,
   unsigned int options)
 {
-   return refresh_cache_ent(_index, ce, options, NULL, NULL);
+   return refresh_cache_ent(istate, ce, options, NULL, NULL);
 }
 
 
-- 
2.14.3

[PATCH v3 0/7] allocate cache entries from memory pool

[Jameson Miller]

Changes from V2:

- Tweak logic of finding available memory block for memory
  allocation

  - Only search head block
  
- Tweaked handling of large memory allocations.

  - Large blocks now tracked in same manner as "regular"
blocks
  
  - Large blocks are placed at end of linked list of memory
blocks

- Cache_entry type gains notion of whether it was allocated
  from memory pool or not

  - Collapsed cache_entry discard logic into single
function. This should make code easier to maintain

- Small tweaks based on V1 feedback

Base Ref: master
Web-Diff: g...@github.com:jamill/git.git/commit/d608515f9e
Checkout: git fetch g...@github.com:jamill/git.git 
users/jamill/block_allocation-v3 && git checkout d608515f9e


Jameson Miller (7):
  read-cache: teach refresh_cache_entry() to take istate
  block alloc: add lifecycle APIs for cache_entry structs
  mem-pool: only search head block for available space
  mem-pool: add lifecycle management functions
  mem-pool: fill out functionality
  block alloc: allocate cache entries from mem_pool
  block alloc: add validations around cache_entry lifecyle

 apply.c|  26 +++--
 blame.c|   5 +-
 builtin/checkout.c |   8 +-
 builtin/difftool.c |   8 +-
 builtin/reset.c|   6 +-
 builtin/update-index.c |  26 +++--
 cache.h|  53 +-
 fast-import.c  |   2 +-
 git.c  |   3 +
 mem-pool.c | 116 --
 mem-pool.h |  25 -
 merge-recursive.c  |   4 +-
 read-cache.c   | 261 -
 resolve-undo.c |   6 +-
 split-index.c  |  58 ---
 tree.c |   4 +-
 unpack-trees.c |  38 +++
 17 files changed, 514 insertions(+), 135 deletions(-)


base-commit: ccdcbd54c4475c2238b310f7113ab3075b5abc9c
-- 
2.14.3

[PATCH v3 2/7] block alloc: add lifecycle APIs for cache_entry structs

[Jameson Miller]

Add an API around managing the lifetime of cache_entry
structs. Abstracting memory management details behind an API will
allow for alternative memory management strategies without affecting
all the call sites.  This commit does not change how memory is
allocated / freed. A later commit in this series will allocate cache
entries from memory pools as appropriate.

Motivation:
It has been observed that the time spent loading an index with a large
number of entries is partly dominated by malloc() calls. This change
is in preparation for using memory pools to reduce the number of
malloc() calls made when loading an index.

This API makes a distinction between cache entries that are intended
for use with a particular index and cache entries that are not. This
enables us to use the knowledge about how a cache entry will be used
to make informed decisions about how to handle the corresponding
memory.

Signed-off-by: Jameson Miller 
---
 apply.c|  26 ++---
 blame.c|   5 +--
 builtin/checkout.c |   8 ++--
 builtin/difftool.c |   8 ++--
 builtin/reset.c|   6 +--
 builtin/update-index.c |  26 ++---
 cache.h|  24 +++-
 merge-recursive.c  |   2 +-
 read-cache.c   | 100 ++---
 resolve-undo.c |   6 ++-
 split-index.c  |   8 ++--
 tree.c |   4 +-
 unpack-trees.c |  33 +++-
 13 files changed, 162 insertions(+), 94 deletions(-)

diff --git a/apply.c b/apply.c
index 7e5792c996..b769fe0d15 100644
--- a/apply.c
+++ b/apply.c
@@ -4090,12 +4090,12 @@ static int build_fake_ancestor(struct apply_state 
*state, struct patch *list)
return error(_("sha1 information is lacking or useless "
   "(%s)."), name);
 
-   ce = make_cache_entry(patch->old_mode, oid.hash, name, 0, 0);
+   ce = make_index_cache_entry(, patch->old_mode, oid.hash, 
name, 0, 0);
if (!ce)
-   return error(_("make_cache_entry failed for path '%s'"),
+   return error(_("make_index_cache_entry failed for path 
'%s'"),
 name);
if (add_index_entry(, ce, ADD_CACHE_OK_TO_ADD)) {
-   free(ce);
+   discard_cache_entry(ce);
return error(_("could not add %s to temporary index"),
 name);
}
@@ -4263,12 +4263,11 @@ static int add_index_file(struct apply_state *state,
struct stat st;
struct cache_entry *ce;
int namelen = strlen(path);
-   unsigned ce_size = cache_entry_size(namelen);
 
if (!state->update_index)
return 0;
 
-   ce = xcalloc(1, ce_size);
+   ce = make_empty_index_cache_entry(_index, namelen);
memcpy(ce->name, path, namelen);
ce->ce_mode = create_ce_mode(mode);
ce->ce_flags = create_ce_flags(0);
@@ -4278,13 +4277,13 @@ static int add_index_file(struct apply_state *state,
 
if (!skip_prefix(buf, "Subproject commit ", ) ||
get_oid_hex(s, >oid)) {
-   free(ce);
-  return error(_("corrupt patch for submodule %s"), path);
+   discard_cache_entry(ce);
+   return error(_("corrupt patch for submodule %s"), path);
}
} else {
if (!state->cached) {
if (lstat(path, ) < 0) {
-   free(ce);
+   discard_cache_entry(ce);
return error_errno(_("unable to stat newly "
 "created file '%s'"),
   path);
@@ -4292,13 +4291,13 @@ static int add_index_file(struct apply_state *state,
fill_stat_cache_info(ce, );
}
if (write_object_file(buf, size, blob_type, >oid) < 0) {
-   free(ce);
+   discard_cache_entry(ce);
return error(_("unable to create backing store "
   "for newly created file %s"), path);
}
}
if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0) {
-   free(ce);
+   discard_cache_entry(ce);
return error(_("unable to add cache entry for %s"), path);
}
 
@@ -4422,27 +4421,26 @@ static int add_conflicted_stages_file(struct 
apply_state *state,
   struct patch *patch)
 {
int stage, namelen;
-   unsigned ce_size, mode;
+   unsigned mode;
struct cache_entry *ce;
 
if (!state->update_index)
return 0;
namelen = 

[RFC PATCH v3 1/2] Implement --first-parent for git rev-list --bisect

[Tiago Botelho]

This will enable users to implement bisecting on first parents
which can be useful for when the commits from a feature branch
that we want to merge are not always tested.

Signed-off-by: Tiago Botelho 
---
 bisect.c   | 53 +++--
 bisect.h   |  3 ++-
 builtin/rev-list.c |  3 +++
 revision.c |  3 ---
 4 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/bisect.c b/bisect.c
index 4eafc8262..f43713574 100644
--- a/bisect.c
+++ b/bisect.c
@@ -34,7 +34,7 @@ static const char *term_good;
  * We care just barely enough to avoid recursing for
  * non-merge entries.
  */
-static int count_distance(struct commit_list *entry)
+static int count_distance(struct commit_list *entry, unsigned bisect_flags)
 {
int nr = 0;
 
@@ -49,10 +49,10 @@ static int count_distance(struct commit_list *entry)
commit->object.flags |= COUNTED;
p = commit->parents;
entry = p;
-   if (p) {
+   if (p && !(bisect_flags & BISECT_FIRST_PARENT)) {
p = p->next;
while (p) {
-   nr += count_distance(p);
+   nr += count_distance(p, bisect_flags);
p = p->next;
}
}
@@ -82,15 +82,16 @@ static inline void weight_set(struct commit_list *elem, int 
weight)
*((int*)(elem->item->util)) = weight;
 }
 
-static int count_interesting_parents(struct commit *commit)
+static int count_interesting_parents(struct commit *commit, unsigned 
bisect_flags)
 {
struct commit_list *p;
int count;
 
for (count = 0, p = commit->parents; p; p = p->next) {
-   if (p->item->object.flags & UNINTERESTING)
-   continue;
-   count++;
+   if (!(p->item->object.flags & UNINTERESTING))
+   count++;
+   if (bisect_flags & BISECT_FIRST_PARENT)
+   break;
}
return count;
 }
@@ -117,10 +118,10 @@ static inline int halfway(struct commit_list *p, int nr)
 }
 
 #if !DEBUG_BISECT
-#define show_list(a,b,c,d) do { ; } while (0)
+#define show_list(a,b,c,d,e) do { ; } while (0)
 #else
 static void show_list(const char *debug, int counted, int nr,
- struct commit_list *list)
+ struct commit_list *list, unsigned bisect_flags)
 {
struct commit_list *p;
 
@@ -146,10 +147,14 @@ static void show_list(const char *debug, int counted, int 
nr,
else
fprintf(stderr, "---");
fprintf(stderr, " %.*s", 8, oid_to_hex(>object.oid));
-   for (pp = commit->parents; pp; pp = pp->next)
+   for (pp = commit->parents; pp; pp = pp->next) {
fprintf(stderr, " %.*s", 8,
oid_to_hex(>item->object.oid));
 
+   if (bisect_flags & BISECT_FIRST_PARENT)
+   break;
+   }
+
subject_len = find_commit_subject(buf, _start);
if (subject_len)
fprintf(stderr, " %.*s", subject_len, subject_start);
@@ -267,13 +272,13 @@ static struct commit_list *do_find_bisection(struct 
commit_list *list,
unsigned flags = commit->object.flags;
 
p->item->util = [n++];
-   switch (count_interesting_parents(commit)) {
+   switch (count_interesting_parents(commit, bisect_flags)) {
case 0:
if (!(flags & TREESAME)) {
weight_set(p, 1);
counted++;
show_list("bisection 2 count one",
- counted, nr, list);
+ counted, nr, list, bisect_flags);
}
/*
 * otherwise, it is known not to reach any
@@ -289,7 +294,7 @@ static struct commit_list *do_find_bisection(struct 
commit_list *list,
}
}
 
-   show_list("bisection 2 initialize", counted, nr, list);
+   show_list("bisection 2 initialize", counted, nr, list, bisect_flags);
 
/*
 * If you have only one parent in the resulting set
@@ -310,7 +315,7 @@ static struct commit_list *do_find_bisection(struct 
commit_list *list,
continue;
if (weight(p) != -2)
continue;
-   weight_set(p, count_distance(p));
+   weight_set(p, count_distance(p, bisect_flags));
clear_distance(list);
 
/* Does it happen to be at exactly half-way? */
@@ -319,7 +324,7 @@ static struct commit_list *do_find_bisection(struct 
commit_list *list,
 

[RFC PATCH v3 2/2] Add tests for rev-list --bisect* --first-parent

[Tiago Botelho]

---
 t/t6002-rev-list-bisect.sh | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh
index a66140803..977c82157 100755
--- a/t/t6002-rev-list-bisect.sh
+++ b/t/t6002-rev-list-bisect.sh
@@ -263,4 +263,43 @@ test_expect_success 'rev-parse --bisect can default to 
good/bad refs' '
test_cmp expect.sorted actual.sorted
 '
 
+# We generate the following commit graph:
+#
+# A
+#/ \
+#   D   B
+#   |   |
+#   EX  C
+#\  /
+# FX
+
+test_expect_success 'setup' '
+  test_commit A &&
+  test_commit B &&
+  test_commit C &&
+  git reset --hard A &&
+  test_commit D &&
+  test_commit EX &&
+  test_merge FX C
+'
+
+test_output_expect_success "--bisect --first-parent" 'git rev-list --bisect 
--first-parent FX ^A' <

Re: git push => git: 'credential-winstore' is not a git command.

[Chris]

Thanks, Peff. I should have thought about the configuration
hierarchy... This evening I need to do some trial-and-error with the
three credential entries that found.
Want what you have,
Chris


On Wed, May 23, 2018 at 1:16 AM, Jeff King  wrote:
> On Sun, May 20, 2018 at 10:17:54AM -0500, Chris wrote:
>
>> git config --global --unset credential.helper
>>
>>
>> This did help me, because previously Git was trying to authenticate me
>> with the Microsoft account I use to log into my Windows, which is
>> unrelated to the account I need to use to push code. And it removed
>> one of the two "git: 'credential-winstore' is not a git command."
>> messages I was receiving.
>>
>> But I still get one of them, so I tried reinstalling Git for Windows
>> with the credential helper disabled, but that didn't help. Then I ran
>> this command:
>>
>> git config -e
>>
>>
>> And couldn't find any mention of [credential].
>
> That command will only edit the local repository's config file. You may
> have other config for your user (--global) or for the machine
> (--system).
>
> Try:
>
>   git config --show-origin --get-regexp credential.*
>
> to see any related config you have, and which file it comes from (you
> can also just do "--show-origin --list" to see all of the config).
>
>> What can I do to get rid of this annoying message (and, for all I
>> know, potential symptom of a larger problem)?
>
> I don't know enough about Git for Windows packaging to know whether
> you're supposed to have the winstore credential helper installed. So it
> could be a symptom of some kind of installation problem. But in general,
> a missing credential helper isn't a big deal (it just means that Git
> can't ask it for a credential and will end up prompting you or using a
> different helper).
>
> -Peff

From: Mr James Tan (Urgent & Confidential)

[Mr James Tan]

-- 
From: Mr James Tan (Urgent & Confidential)

Good Day, Please Read.

My name is Mr. James Tan, I am the Bill and Exchange manager here in Bank
of Africa (BOA) Lome/Togo.West-Africa. I have a business proposal in the
tune of $9.7m, (Nine Million Seven hundred Thousand only) after the
successful transfer;we shall share in ratio of 40% for you and 60% for me.

Should you be interested, please contact me through my private email (
jamestanta...@gmail.com ) so we can commence all arrangements and i will
give you more information on how we would handle this project. I will want
you to call me as soon as you can (+228 79 82 32 85)

Please treat this business with utmost confidentiality and send me the
Following:

1. Your Full Name:
2. Your Phone Number:.
3. Your Age:..
4. Your Sex:..
5. Your Occupations:..
6. Your Country and City:

Kind Regards,

Mr.James Tan .
Bill & Exchange manager
(BOA) Bank of Africa.
Lome-Togo.
Email: jamestanta...@gmail.com

Re: should config options be treated as case-sensitive?

[Robert P. J. Day]

On Wed, 23 May 2018, Junio C Hamano wrote:

> "Robert P. J. Day"  writes:
>
> >> If the documention does not make it clear, then we have
> >> documentation bug ...
> >
> >   personally, i would add a short, really emphatic note at the top of
> > "man git-config" pointing this out -- i wouldn't require people to
> > read all the way down to "Syntax" to learn this. an example just like
> > the one you provide above would be perfect, with an extra line
> > pointing out that the documentation uses "camel case" for nothing more
> > than readability.
>
> Unfortunately, that line of thinking leads us to madness, as you are
> exhibiting the typical symptom of "my today's immediate itch is the
> most important one in the world"-itis.  Tomorrow you would start
> saying that we must have a short, really emphatic note at the top
> that says that the second level name can even have spaces, and on
> the day after that, you would instead have a note that says that you
> cannot use an underscore in the name, and continuing that line of
> thought will lead us to fill the top part of the documentation with
> 47 different short and emphatic sentences.  Let's not go there.

  fair enough, point taken.

rday

-- 


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

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

Re: should config options be treated as case-sensitive?

[Junio C Hamano]

"Robert P. J. Day"  writes:

>> If the documention does not make it clear, then we have
>> documentation bug ...
>
>   personally, i would add a short, really emphatic note at the top of
> "man git-config" pointing this out -- i wouldn't require people to
> read all the way down to "Syntax" to learn this. an example just like
> the one you provide above would be perfect, with an extra line
> pointing out that the documentation uses "camel case" for nothing more
> than readability.

Unfortunately, that line of thinking leads us to madness, as you are
exhibiting the typical symptom of "my today's immediate itch is the
most important one in the world"-itis.  Tomorrow you would start
saying that we must have a short, really emphatic note at the top
that says that the second level name can even have spaces, and on
the day after that, you would instead have a note that says that you
cannot use an underscore in the name, and continuing that line of
thought will lead us to fill the top part of the documentation with
47 different short and emphatic sentences.  Let's not go there.

Re: which files are "known to git"?

[Robert P. J. Day]

On Mon, 21 May 2018, Jonathan Nieder wrote:

> Robert P. J. Day wrote:
> > On Mon, 21 May 2018, Elijah Newren wrote:
>
> >> Hi Robert,
> >>
> >> I had always assumed prior to your email that 'known to Git'
> >> meant 'tracked' or 'recorded in the index'...
> >
> >   i *know* i've been in this discussion before, but i don't
> > remember where, i *assume* it was on this list, and i recall
> > someone (again, don't remember who) who opined that there are two
> > categories of files that are "known to git":
>
> My understanding was the same as Elijah's.
>
> I would be in favor of a patch that replaces the phrase "known to
> Git" in Git's documentation with something less confusing.

  ironically, the 2nd edition of o'reilly's "version control with git"
uses the phrases "known to Git" and "unknown to Git" on p. 378 (and
nowhere else that i can see):

"Furthermore, for the purposes of this [git clean] command, Git uses a
slightly more conservative concept of under version control.
Specifically, the manual page uses the phrase “files that are unknown
to Git” for a good reason: even files that are mentioned in the
.gitignore and .git/info/exclude files are actually known to Git. They
represent files that are not version controlled, but Git does know
about them. And because those files are called out in the .gitignore
files, they must have some known (to you) behavior that shouldn’t be
disturbed by Git. So Git won’t clean out the ignored files unless you
explicitly request it with the -x option."

  that phrase even occurs in git-produced diagnostic messages such as:

  dir.c: error("pathspec '%s' did not match any file(s) known to git.",

in any event, perhaps the phrase "known to Git" has some value, as
long as it's defined very precisely and used consistently, which it
obviously isn't right now.

rday

-- 


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

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

Re: should config options be treated as case-sensitive?

[Robert P. J. Day]

On Wed, 23 May 2018, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  writes:
>
> > The issues you note about the docs using foo.barbaz instead of
> > foo.barBaz should be fixed, but as noted in the "Syntax" section
> > of "git-config" we already document that the config keys are all
> > case-insensitive. We just like talking about them as foo.barBaz
> > because it makes for easier reading.
>
> The first and the last level of configuration variable names are
> case insensitive.
>
> I said "first and last", as there are variables with 2-level and
> 3-level names.  "foo.barBaz" is two-level and it is the same
> variable as "Foo.barbaz".  "remote.origin.url" is three-level, and
> it is the same variable as "Remote.origin.URL", but it is not the
> same variable as "remote.ORIGIN.url".
>
> If the documention does not make it clear, then we have
> documentation bug ...

  personally, i would add a short, really emphatic note at the top of
"man git-config" pointing this out -- i wouldn't require people to
read all the way down to "Syntax" to learn this. an example just like
the one you provide above would be perfect, with an extra line
pointing out that the documentation uses "camel case" for nothing more
than readability.

rday

-- 


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

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

lening

[Funding Trusts Finance]

Goede dag,
  
We zijn Funding Trusts Finance verstrekt leningen per postadvertentie. Wij 
bieden verschillende soorten leningen of projectleningen (korte en lange 
termijnleningen, persoonlijke leningen, leningen aan bedrijven enz.) Met een 
rentetarief van 3%. We verstrekken leningen aan mensen in nood, ongeacht hun 
locatie, geslacht, burgerlijke staat, opleiding of baan, maar moeten een legale 
manier van terugbetaling hebben. Onze leningen variren tussen 5.000,00 
tot 20.000.000,00 US Dollar of Euro of Pond met een maximale duur van 15 jaar. 
Als u genteresseerd bent in meer informatie, hebben we investeerders die 
genteresseerd zijn in het financieren van projecten van groot volume. De 
procedures zijn als volgt: -

1-De klant moet een korte samenvatting van het project verzenden. Dit moet het 
totale bedrag omvatten dat vereist is voor het project, geschat rendement op 
investering, terugbetalingsperiode van de lening, dit mag niet meer dan 20 jaar 
zijn

Neem contact met ons op: i...@fundingtrustsfinance.com

INFORMATIE NODIG

Jullie namen:
Adres: ...
Telefoon: ...
Benodigde hoeveelheid: ...
Looptijd: ...
Beroep: ...
Maandelijks inkomensniveau: ..
Geslacht: ..
Geboortedatum: ...
Staat: ...
Land: .
Doel: .

"Miljoenen mensen in de wereld hebben een kredietprobleem van een of andere 
vorm. Je bent niet de enige. We hebben een hele reeks leningopties die kunnen 
helpen. Ontdek nu je opties!"

Met vriendelijke groet,
Ronny Hens,
E-mail: i...@fundingtrustsfinance.com
WEBSITE: www.fundingtrustfinance.com

lening

[Funding Trusts Finance]

Goede dag,
  
We zijn Funding Trusts Finance verstrekt leningen per postadvertentie. Wij 
bieden verschillende soorten leningen of projectleningen (korte en lange 
termijnleningen, persoonlijke leningen, leningen aan bedrijven enz.) Met een 
rentetarief van 3%. We verstrekken leningen aan mensen in nood, ongeacht hun 
locatie, geslacht, burgerlijke staat, opleiding of baan, maar moeten een legale 
manier van terugbetaling hebben. Onze leningen variren tussen 5.000,00 
tot 20.000.000,00 US Dollar of Euro of Pond met een maximale duur van 15 jaar. 
Als u genteresseerd bent in meer informatie, hebben we investeerders die 
genteresseerd zijn in het financieren van projecten van groot volume. De 
procedures zijn als volgt: -

1-De klant moet een korte samenvatting van het project verzenden. Dit moet het 
totale bedrag omvatten dat vereist is voor het project, geschat rendement op 
investering, terugbetalingsperiode van de lening, dit mag niet meer dan 20 jaar 
zijn

Neem contact met ons op: i...@fundingtrustsfinance.com

INFORMATIE NODIG

Jullie namen:
Adres: ...
Telefoon: ...
Benodigde hoeveelheid: ...
Looptijd: ...
Beroep: ...
Maandelijks inkomensniveau: ..
Geslacht: ..
Geboortedatum: ...
Staat: ...
Land: .
Doel: .

"Miljoenen mensen in de wereld hebben een kredietprobleem van een of andere 
vorm. Je bent niet de enige. We hebben een hele reeks leningopties die kunnen 
helpen. Ontdek nu je opties!"

Met vriendelijke groet,
Ronny Hens,
E-mail: i...@fundingtrustsfinance.com
WEBSITE: www.fundingtrustfinance.com

[PATCHv5 1/1] git-p4: add unshelve command

[Luke Diamand]

This can be used to "unshelve" a shelved P4 commit into
a git commit.

For example:

  $ git p4 unshelve 12345

The resulting commit ends up in the branch:
   refs/remotes/p4/unshelved/12345

If that branch already exists, it is renamed - for example
the above branch would be saved as p4/unshelved/12345.1.

git-p4 checks that the shelved changelist is based on files
which are at the same Perforce revision as the origin branch
being used for the unshelve (HEAD by default). If they are not,
it will refuse to unshelve. This is to ensure that the unshelved
change does not contain other changes mixed-in.

The reference branch can be changed manually with the "--origin"
option.

The change adds a new Unshelve command class. This just runs the
existing P4Sync code tweaked to handle a shelved changelist.

Signed-off-by: Luke Diamand 
---
 Documentation/git-p4.txt |  32 ++
 git-p4.py| 215 ---
 t/t9832-unshelve.sh  | 138 +
 3 files changed, 348 insertions(+), 37 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index d8c8f11c9f..d3cb249fc2 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -164,6 +164,31 @@ $ git p4 submit --shelve
 $ git p4 submit --update-shelve 1234 --update-shelve 2345
 
 
+
+Unshelve
+
+Unshelving will take a shelved P4 changelist, and produce the equivalent git 
commit
+in the branch refs/remotes/p4/unshelved/.
+
+The git commit is created relative to the current origin revision (HEAD by 
default).
+If the shelved changelist's parent revisions differ, git-p4 will refuse to 
unshelve;
+you need to be unshelving onto an equivalent tree.
+
+The origin revision can be changed with the "--origin" option.
+
+If the target branch in refs/remotes/p4/unshelved already exists, the old one 
will
+be renamed.
+
+
+$ git p4 sync
+$ git p4 unshelve 12345
+$ git show refs/remotes/p4/unshelved/12345
+
+$ git p4 unshelve 12345
+
+
+
+
 OPTIONS
 ---
 
@@ -337,6 +362,13 @@ These options can be used to modify 'git p4 rebase' 
behavior.
 --import-labels::
Import p4 labels.
 
+Unshelve options
+
+
+--origin::
+Sets the git refspec against which the shelved P4 changelist is compared.
+Defaults to p4/master.
+
 DEPOT PATH SYNTAX
 -
 The p4 depot path argument to 'git p4 sync' and 'git p4 clone' can
diff --git a/git-p4.py b/git-p4.py
index 7bb9cadc69..c80d85af89 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -316,12 +316,17 @@ def p4_last_change():
 results = p4CmdList(["changes", "-m", "1"], skip_info=True)
 return int(results[0]['change'])
 
-def p4_describe(change):
+def p4_describe(change, shelved=False):
 """Make sure it returns a valid result by checking for
the presence of field "time".  Return a dict of the
results."""
 
-ds = p4CmdList(["describe", "-s", str(change)], skip_info=True)
+cmd = ["describe", "-s"]
+if shelved:
+cmd += ["-S"]
+cmd += [str(change)]
+
+ds = p4CmdList(cmd, skip_info=True)
 if len(ds) != 1:
 die("p4 describe -s %d did not return 1 result: %s" % (change, 
str(ds)))
 
@@ -662,6 +667,12 @@ def gitBranchExists(branch):
 stderr=subprocess.PIPE, stdout=subprocess.PIPE);
 return proc.wait() == 0;
 
+def gitUpdateRef(ref, newvalue):
+subprocess.check_call(["git", "update-ref", ref, newvalue])
+
+def gitDeleteRef(ref):
+subprocess.check_call(["git", "update-ref", "-d", ref])
+
 _gitConfig = {}
 
 def gitConfig(key, typeSpecifier=None):
@@ -2411,6 +2422,7 @@ class P4Sync(Command, P4UserMap):
 self.tempBranches = []
 self.tempBranchLocation = "refs/git-p4-tmp"
 self.largeFileSystem = None
+self.suppress_meta_comment = False
 
 if gitConfig('git-p4.largeFileSystem'):
 largeFileSystemConstructor = 
globals()[gitConfig('git-p4.largeFileSystem')]
@@ -2421,6 +2433,18 @@ class P4Sync(Command, P4UserMap):
 if gitConfig("git-p4.syncFromOrigin") == "false":
 self.syncWithOrigin = False
 
+self.depotPaths = []
+self.changeRange = ""
+self.previousDepotPaths = []
+self.hasOrigin = False
+
+# map from branch depot path to parent branch
+self.knownBranches = {}
+self.initialParents = {}
+
+self.tz = "%+03d%02d" % (- time.timezone / 3600, ((- time.timezone % 
3600) / 60))
+self.labels = {}
+
 # Force a checkpoint in fast-import and wait for it to finish
 def checkpoint(self):
 self.gitStream.write("checkpoint\n\n")
@@ -2429,7 +2453,20 @@ class P4Sync(Command, P4UserMap):
 if self.verbose:
 print "checkpoint finished: " + out
 
-def extractFilesFromCommit(self, commit):
+def cmp_shelved(self, path, filerev, revision):
+""" Determine if a path at revision 

[PATCHv5 0/1] git-p4: unshelve: fix problem with newer p4d

[Luke Diamand]

This is v5 of my git-p4 unshelve patch. It fixes a problem found by
SZEDER Gábor with newer versions of Perforce (2016 vs 2015).  

Luke Diamand (1):
  git-p4: add unshelve command

 Documentation/git-p4.txt |  32 ++
 git-p4.py| 215 ---
 t/t9832-unshelve.sh  | 138 +
 3 files changed, 348 insertions(+), 37 deletions(-)
 create mode 100755 t/t9832-unshelve.sh

-- 
2.17.0.392.gdeb1a6e9b7

Re: [PATCH v2 3/3] config: let `config_store_data_clear()` handle `key`

[Eric Sunshine]

On Sun, May 20, 2018 at 6:42 AM, Martin Ågren  wrote:
> Instead of remembering to free `key` in each code path, let
> `config_store_data_clear()` handle that.
>
> We still need to free it before replacing it, though. Move that freeing
> closer to the replacing to be safe. Note that in that same part of the
> code, we can no longer set `key` to the original pointer, but need to
> `xstrdup()` it.

That casting away of 'const' was an oddball case anyhow, so it's nice
to see it go away (even at the expense of an extra xstrdup()).

Overall, this change makes it quite a bit easier to reason about the
cleanup of 'store.key'.

Thanks.

> Signed-off-by: Martin Ågren 

Re: [PATCH v2 0/3] config: free resources of `struct config_store_data`

[Eric Sunshine]

On Sun, May 20, 2018 at 6:42 AM, Martin Ågren  wrote:
> On 14 May 2018 at 05:03, Eric Sunshine  wrote:
>> On Sun, May 13, 2018 at 5:58 AM, Martin Ågren  wrote:
>>> How about the following two patches as patches 2/3 and 3/3? I would also
>>> need to mention in the commit message of this patch (1/3) that the
>>> function will soon learn to clean up more members.
>>
>> Yep, making this a multi-part patch series and updating the commit
>> message of the patch which introduces config_store_data_clear(), as
>> you suggest, makes sense. The patch series could be organized
>> differently -- such as first moving freeing of 'value_regex' into new
>> config_store_data_clear(), then freeing additional fields in later
>> patches -- but I don't think it matters much in practice, so the
>> current organization is likely good enough.
>
> I tried such a re-ordering but wasn't entirely happy about the result
> (maybe I didn't try hard enough), so here are these patches again, as a
> proper series and with improved commit messages.

The re-roll looks good; it address my concern about v1. Thanks.

Re: git push => git: 'credential-winstore' is not a git command.

[Jeff King]

On Sun, May 20, 2018 at 10:17:54AM -0500, Chris wrote:

> git config --global --unset credential.helper
> 
> 
> This did help me, because previously Git was trying to authenticate me
> with the Microsoft account I use to log into my Windows, which is
> unrelated to the account I need to use to push code. And it removed
> one of the two "git: 'credential-winstore' is not a git command."
> messages I was receiving.
> 
> But I still get one of them, so I tried reinstalling Git for Windows
> with the credential helper disabled, but that didn't help. Then I ran
> this command:
> 
> git config -e
> 
> 
> And couldn't find any mention of [credential].

That command will only edit the local repository's config file. You may
have other config for your user (--global) or for the machine
(--system).

Try:

  git config --show-origin --get-regexp credential.*

to see any related config you have, and which file it comes from (you
can also just do "--show-origin --list" to see all of the config).

> What can I do to get rid of this annoying message (and, for all I
> know, potential symptom of a larger problem)?

I don't know enough about Git for Windows packaging to know whether
you're supposed to have the winstore credential helper installed. So it
could be a symptom of some kind of installation problem. But in general,
a missing credential helper isn't a big deal (it just means that Git
can't ask it for a credential and will end up prompting you or using a
different helper).

-Peff

Re: [PATCH v2] t: make many tests depend less on the refs being files

[Junio C Hamano]

Christian Couder  writes:

> The internals of the loose refs backend are still tested in
> t1400-update-ref.sh, whereas the tests changed in this patch focus
> on testing other aspects.
>
> This patch just takes care of many low hanging fruits. It does not
> try to completely solves the issue.

Thanks.  All conversions in this patch look correct to me.

Will queue.