aviso final da microsoft

2018-11-23 Thread Floriana.Petrogalli
Outlook

Prezado usuário da conta,
Alguns de seus e-mails recebidos foram colocados em status pendente devido à 
recente atualização em nosso banco de dados. Para receber suas mensagens,
Clique no link abaixo para entrar e aguardar a resposta do Webmail.
CLIQUE AQUI
Pedimos desculpas por qualquer inconveniente e agradecemos sua compreensão.
Obrigado.

Copyright © 2018 Webmail .Inc. Todos os direitos reservados.


[PATCH] doc: update diff-format.txt for removed ellipses

2018-11-23 Thread Greg Hurrell
Commit 7cb6ac1e4b made the diff format omit ellipses by default, but
there is still this place in the documentation where we show examples of
output with ellipses.

The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
to bring back the old output format, but that is already documented in
git.txt, so I am not mentioning it here.
---
 Documentation/diff-format.txt | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
index 706916c94c..cdcc17f0ad 100644
--- a/Documentation/diff-format.txt
+++ b/Documentation/diff-format.txt
@@ -26,12 +26,12 @@ line per changed file.
 An output line is formatted this way:
 
 
-in-place edit  :100644 100644 bcd1234... 0123456... M file0
-copy-edit  :100644 100644 abcd123... 1234567... C68 file1 file2
-rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
-create :00 100644 000... 1234567... A file4
-delete :100644 00 1234567... 000... D file5
-unmerged   :00 00 000... 000... U file6
+in-place edit  :100644 100644 bcd1234 0123456 M file0
+copy-edit  :100644 100644 abcd123 1234567 C68 file1 file2
+rename-edit:100644 100644 abcd123 1234567 R86 file1 file3
+create :00 100644 000 1234567 A file4
+delete :100644 00 1234567 000 D file5
+unmerged   :00 00 000 000 U file6
 
 
 That is, from the left to the right:
@@ -75,7 +75,7 @@ and it is out of sync with the index.
 Example:
 
 
-:100644 100644 5be4a4.. 00.. M file.c
+:100644 100644 5be4a4a 000 M file.c
 
 
 Without the `-z` option, pathnames with "unusual" characters are
@@ -100,7 +100,7 @@ from the format described above in the following way:
 Example:
 
 
-::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM describe.c
+::100644 100644 100644 fabadb8 cc95eb0 4866510 MM  describe.c
 
 
 Note that 'combined diff' lists only files which were modified from
-- 
2.19.0



Document change in format of raw diff output format

2018-11-23 Thread Greg Hurrell
Jeff King wrote:

> On Thu, Nov 22, 2018 at 11:58:36AM +0100, Greg Hurrell wrote:
> 
> > diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
> > index 706916c94c..33776459d0 100644
> > --- a/Documentation/diff-format.txt
> > +++ b/Documentation/diff-format.txt
> > @@ -26,12 +26,12 @@ line per changed file.
> >  An output line is formatted this way:
> > 
> >  
> > -in-place edit  :100644 100644 bcd1234... 0123456... M file0
> > -copy-edit  :100644 100644 abcd123... 1234567... C68 file1 file2
> > -rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
> > -create :00 100644 000... 1234567... A file4
> > -delete :100644 00 1234567... 000... D file5
> > -unmerged   :00 00 000... 000... U file6
> > +in-place edit  :100644 100644 bcd123456 012345678 M file0
> > +copy-edit  :100644 100644 abcd12345 123456789 C68 file1 file2
> > +rename-edit:100644 100644 abcd12345 123456789 R86 file1 file3
> > +create :00 100644 0 123456789 A file4
> > +delete :100644 00 123456789 0 D file5
> > +unmerged   :00 00 0 0 U file6
> >  
> 
> Yeah, this looks like an improvement.
> 
> I think in general that we'd continue to show 7 characters now, just
> without the extra dots (though it's auto-scaled based on the number of
> objects in the repo these days, so it's not even really a constant).

That's funny. I looked at the output on (what I thought was) a small
repo and it was showing me 9-character abbreviated hashes. I guess I
just got lucky. Tested on a basically empty repo and 7 does look to be
the default.

> PS As you noticed, "git log" we don't promise that git-log output will
>never change between versions. For machine-consumption you probably
>want to use plumbing like "git rev-list | git diff-tree --stdin",
>which produces unabbreviated hashes.

Thanks for the tip. My mistake was thinking that the `--raw` made the
`git log` output somehow more plumbing-ish, but I've gone ahead and
switched to using git-rev-list plus git-diff-tree instead.

Anyway, patch follows.




Microsoft laatste waarschuwing

2018-11-23 Thread Floriana.Petrogalli
Outlook

Prezado usuário da conta,
Alguns de seus e-mails recebidos foram colocados em status pendente devido à 
recente atualização em nosso banco de dados. Para receber suas mensagens,
Clique no link abaixo para entrar e aguardar a resposta do Webmail.
CLIQUE AQUI
Pedimos desculpas por qualquer inconveniente e agradecemos sua compreensão.
Obrigado.

Copyright © 2018 Webmail .Inc. Todos os direitos reservados.


Re: 2.19.2 wont launch

2018-11-23 Thread Johannes Schindelin
Hi Paul,

On Thu, 22 Nov 2018, Paul Gureghian wrote:

> I installed 2.19.2 on windows 7 , 32 bit and it wont launch.

This has been reported on Gitter and fixed in
https://github.com/git-for-windows/MINGW-packages/commit/deb0395d031401ffe55024fb066267e2ea8d032b

For the time being, please either use v2.19.1, or copy the file
`git-bash.exe` from a portable Git v2.19.1 into your v2.19.2 installation.

Sorry for the trouble,
Johannes


Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C

2018-11-23 Thread Johannes Schindelin
Hi Pranit,

(Cc:ing Tanushree because they will try to pick up this patch series as
part of the Outreachy program.)

On Mon, 30 Oct 2017, Pranit Bauva wrote:

> On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren  wrote:
> > On 27 October 2017 at 17:06, Pranit Bauva  wrote:
> >> +static void free_terms(struct bisect_terms *terms)
> >> +{
> >> +   if (!terms->term_good)
> >> +   free((void *) terms->term_good);
> >> +   if (!terms->term_bad)
> >> +   free((void *) terms->term_bad);
> >> +}
> >
> > These look like no-ops. Remove `!` for correctness, or `if (...)` for
> > simplicity, since `free()` can handle NULL.
> 
> I probably forgot to do this here. I will make the change.
> 
> > You leave the pointers dangling, but you're ok for now since this is the
> > last thing that happens in `cmd_bisect__helper()`. Your later patches
> > add more users, but they're also ok, since they immediately assign new
> > values.
> >
> > In case you (and others) find it useful, the below is a patch I've been
> > sitting on for a while as part of a series to plug various memory-leaks.
> > `FREE_AND_NULL_CONST()` would be useful in precisely these situations.
> 
> Honestly, I wouldn't be the best person to judge this.

Git's source code implicitly assumes that any `const` pointer refers to
memory owned by another code path. It is therefore probably not a good
idea to encourage `free()`ing a `const` pointer.

Which brings me back to the question: who really owns that allocated
memory to which `term_good` and `term_bad` refer?

Ciao,
Johannes

How to efficiently backup a bare repository?

2018-11-23 Thread Guilhem Bonnefille
Hi,

I'm managing many bare repositories for development teams.

One service we want to offer is to let developers retrieve old state
of the repository up to 30 days. For example, one developer
(accidently) removed (push -f) a branch/tag and realize few days later
(after vacations) that it was an error.

What is the best approach to do this?

Currently, we use a classical approach, backuping all the repo every
day. But this is far from efficient as:
- we accumulate 30th copies of the repository
- due to packing logic of Git, even if the content is mostly similar,
from one backup to another, there is no way to deduplicate.

Is there any tricks based on reflog? Even for deleted refs (branch/tags)?
Is there any tooling playing with the internal of git to offer such
feature, like copying all refs in a timestamped refs directory to
retain objects?

Thanks in advance for any tips letting improve the backup.
-- 
Guilhem BONNEFILLE
-=- JID: gu...@im.apinc.org MSN: guilhem_bonnefi...@hotmail.com
-=- mailto:guilhem.bonnefi...@gmail.com
-=- http://nathguil.free.fr/


[PATCH v2 7/9] diff --color-moved-ws: optimize allow-indentation-change

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

When running

  git diff --color-moved-ws=allow-indentation-change v2.18.0 v2.19.0

cmp_in_block_with_wsd() is called 694908327 times. Of those 42.7%
return after comparing a and b. By comparing the lengths first we can
return early in all but 0.03% of those cases without dereferencing the
string pointers. The comparison between a and c fails in 6.8% of
calls, by comparing the lengths first we reject all the failing calls
without dereferencing the string pointers.

This reduces the time to run the command above by by 42% from 14.6s to
8.5s. This is still much slower than the normal --color-moved which
takes ~0.6-0.7s to run but is a significant improvement.

The next commits will replace the current implementation with one that
works with mixed tabs and spaces in the indentation. I think it is
worth optimizing the current implementation first to enable a fair
comparison between the two implementations.

Signed-off-by: Phillip Wood 
---
 diff.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 8c08dd68df..c378ce3daf 100644
--- a/diff.c
+++ b/diff.c
@@ -829,20 +829,23 @@ static int cmp_in_block_with_wsd(const struct 
diff_options *o,
 int n)
 {
struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
-   int al = cur->es->len, cl = l->len;
+   int al = cur->es->len, bl = match->es->len, cl = l->len;
const char *a = cur->es->line,
   *b = match->es->line,
   *c = l->line;
-
+   const char *orig_a = a;
int wslen;
 
/*
-* We need to check if 'cur' is equal to 'match'.
-* As those are from the same (+/-) side, we do not need to adjust for
-* indent changes. However these were found using fuzzy matching
-* so we do have to check if they are equal.
+* We need to check if 'cur' is equal to 'match'.  As those
+* are from the same (+/-) side, we do not need to adjust for
+* indent changes. However these were found using fuzzy
+* matching so we do have to check if they are equal. Here we
+* just check the lengths. We delay calling memcmp() to check
+* the contents until later as if the length comparison for a
+* and c fails we can avoid the call all together.
 */
-   if (strcmp(a, b))
+   if (al != bl)
return 1;
 
if (!pmb->wsd.string)
@@ -870,7 +873,7 @@ static int cmp_in_block_with_wsd(const struct diff_options 
*o,
al -= wslen;
}
 
-   if (al != cl || memcmp(a, c, al))
+   if (al != cl || memcmp(orig_a, b, bl) || memcmp(a, c, al))
return 1;
 
return 0;
-- 
2.19.1.1690.g258b440b18



[PATCH v2 6/9] diff --color-moved=zebra: be stricter with color alternation

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Currently when using --color-moved=zebra the color of moved blocks
depends on the number of lines separating them. This means that adding
an odd number of unmoved lines between blocks that are already separated
by one or more unmoved lines will change the color of subsequent moved
blocks. This does not make much sense as the blocks were already
separated by unmoved lines and causes problems when adding lines to test
cases.

Fix this by only using the alternate colors for adjacent moved blocks.

Signed-off-by: Phillip Wood 
---
 diff.c | 27 +++
 t/t4015-diff-whitespace.sh |  6 +++---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/diff.c b/diff.c
index 53a7ab5aca..8c08dd68df 100644
--- a/diff.c
+++ b/diff.c
@@ -1038,26 +1038,30 @@ static int shrink_potential_moved_blocks(struct 
moved_block *pmb,
  * The last block consists of the (n - block_length)'th line up to but not
  * including the nth line.
  *
+ * Returns 0 if the last block is empty or is unset by this function, non zero
+ * otherwise.
+ *
  * NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
  * Think of a way to unify them.
  */
-static void adjust_last_block(struct diff_options *o, int n, int block_length)
+static int adjust_last_block(struct diff_options *o, int n, int block_length)
 {
int i, alnum_count = 0;
if (o->color_moved == COLOR_MOVED_PLAIN)
-   return;
+   return block_length;
for (i = 1; i < block_length + 1; i++) {
const char *c = o->emitted_symbols->buf[n - i].line;
for (; *c; c++) {
if (!isalnum(*c))
continue;
alnum_count++;
if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
-   return;
+   return 1;
}
}
for (i = 1; i < block_length + 1; i++)
o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
+   return 0;
 }
 
 /* Find blocks of moved code, delegate actual coloring decision to helper */
@@ -1067,14 +1071,15 @@ static void mark_color_as_moved(struct diff_options *o,
 {
struct moved_block *pmb = NULL; /* potentially moved blocks */
int pmb_nr = 0, pmb_alloc = 0;
-   int n, flipped_block = 1, block_length = 0;
+   int n, flipped_block = 0, block_length = 0;
 
 
for (n = 0; n < o->emitted_symbols->nr; n++) {
struct hashmap *hm = NULL;
struct moved_entry *key;
struct moved_entry *match = NULL;
struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
+   enum diff_symbol last_symbol = 0;
 
switch (l->s) {
case DIFF_SYMBOL_PLUS:
@@ -1090,7 +1095,7 @@ static void mark_color_as_moved(struct diff_options *o,
free(key);
break;
default:
-   flipped_block = 1;
+   flipped_block = 0;
}
 
if (!match) {
@@ -1101,10 +1106,13 @@ static void mark_color_as_moved(struct diff_options *o,
moved_block_clear(&pmb[i]);
pmb_nr = 0;
block_length = 0;
+   flipped_block = 0;
+   last_symbol = l->s;
continue;
}
 
if (o->color_moved == COLOR_MOVED_PLAIN) {
+   last_symbol = l->s;
l->flags |= DIFF_SYMBOL_MOVED_LINE;
continue;
}
@@ -1135,19 +1143,22 @@ static void mark_color_as_moved(struct diff_options *o,
}
}
 
-   flipped_block = (flipped_block + 1) % 2;
+   if (adjust_last_block(o, n, block_length) &&
+   pmb_nr && last_symbol != l->s)
+   flipped_block = (flipped_block + 1) % 2;
+   else
+   flipped_block = 0;
 
-   adjust_last_block(o, n, block_length);
block_length = 0;
}
 
if (pmb_nr) {
block_length++;
-
l->flags |= DIFF_SYMBOL_MOVED_LINE;
if (flipped_block && o->color_moved != 
COLOR_MOVED_BLOCKS)
l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
}
+   last_symbol = l->s;
}
adjust_last_block(o, n, block_length);
 
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index eee81a1987..fe8a2ab06e 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1802,14 +1802,14 @@ test_expect_su

[PATCH v2 0/9] diff --color-moved-ws fixes and enhancment

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Thanks to Stefan for his feedback on v1. I've updated patches 2 & 8 in
response to those comments - see the range-diff below for details (the
patch numbers are off by one in the range diff, I think because the
first patch is unchanged and so it was used as the merge base by
--range-diff=. For some reason the range-diff also includes
the notes even though I did not give --notes to format-patch)

When trying out the new --color-moved-ws=allow-indentation-change I
was disappointed to discover it did not work if the indentation
contains a mix of spaces and tabs. This series reworks it so that it
does.


Phillip Wood (9):
  diff: document --no-color-moved
  Use "whitespace" consistently
  diff: allow --no-color-moved-ws
  diff --color-moved-ws: demonstrate false positives
  diff --color-moved-ws: fix false positives
  diff --color-moved=zebra: be stricter with color alternation
  diff --color-moved-ws: optimize allow-indentation-change
  diff --color-moved-ws: modify allow-indentation-change
  diff --color-moved-ws: handle blank lines

 Documentation/diff-options.txt |  15 ++-
 Documentation/git-cat-file.txt |   8 +-
 diff.c | 219 +
 t/t4015-diff-whitespace.sh |  99 ++-
 4 files changed, 255 insertions(+), 86 deletions(-)

Range-diff against v1:
1:  ae58ae4f29 ! 1:  4939ee371d diff: use whitespace consistently
@@ -1,9 +1,10 @@
 Author: Phillip Wood 
 
-diff: use whitespace consistently
+Use "whitespace" consistently
 
-Most of the documentation uses 'whitespace' rather than 'white space'
-or 'white spaces' convert to latter two to the former for consistency.
+Most of the messages and documentation use 'whitespace' rather than
+'white space' or 'white spaces' convert to latter two to the former for
+consistency.
 
 Signed-off-by: Phillip Wood 
 
@@ -29,6 +30,39 @@
whitespace is the same per line. This is incompatible with the
other modes.
 
+ diff --git a/Documentation/git-cat-file.txt 
b/Documentation/git-cat-file.txt
+ --- a/Documentation/git-cat-file.txt
+ +++ b/Documentation/git-cat-file.txt
+@@
+ stdin, and the SHA-1, type, and size of each object is printed on stdout. 
The
+ output format can be overridden using the optional `` argument. If
+ either `--textconv` or `--filters` was specified, the input is expected to
+-list the object names followed by the path name, separated by a single 
white
+-space, so that the appropriate drivers can be determined.
++list the object names followed by the path name, separated by a single
++whitespace, so that the appropriate drivers can be determined.
+ 
+ OPTIONS
+ ---
+@@
+   Print object information and contents for each object provided
+   on stdin.  May not be combined with any other options or arguments
+   except `--textconv` or `--filters`, in which case the input lines
+-  also need to specify the path, separated by white space.  See the
++  also need to specify the path, separated by whitespace.  See the
+   section `BATCH OUTPUT` below for details.
+ 
+ --batch-check::
+ --batch-check=::
+   Print object information for each object provided on stdin.  May
+   not be combined with any other options or arguments except
+   `--textconv` or `--filters`, in which case the input lines also
+-  need to specify the path, separated by white space.  See the
++  need to specify the path, separated by whitespace.  See the
+   section `BATCH OUTPUT` below for details.
+ 
+ --batch-all-objects::
+
  diff --git a/diff.c b/diff.c
  --- a/diff.c
  +++ b/diff.c
2:  7072bc6211 = 2:  204c7fea9d diff: allow --no-color-moved-ws
3:  ce3ad19eea = 3:  542b79b215 diff --color-moved-ws: demonstrate false 
positives
4:  700e0b61e7 = 4:  4ffb5c4122 diff --color-moved-ws: fix false positives
5:  9ecd8159a7 = 5:  a3a84f90c5 diff --color-moved=zebra: be stricter with 
color alternation
6:  1b1158b1ca = 6:  f94f2e0bae diff --color-moved-ws: optimize 
allow-indentation-change
7:  d8a362be6a ! 7:  fe8eb9cdbc diff --color-moved-ws: modify 
allow-indentation-change
@@ -17,7 +17,7 @@
 
 This commit changes the way the indentation is handled to track the
 visual size of the indentation rather than the characters in the
-indentation. This has they benefit that any whitespace errors do not
+indentation. This has the benefit that any whitespace errors do not
 interfer with the move detection (the whitespace errors will still be
 highlighted according to --ws-error-highlight). During the discussion
 of this feature there were concerns about the correct detection of
@@ -30,7 +30,7 @@
 they are uncolored.
 
 Signed-off-by: Phillip Wood 
-Changes since rfc:
 

[PATCH v2 5/9] diff --color-moved-ws: fix false positives

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

'diff --color-moved-ws=allow-indentation-change' can color lines as
moved when they are in fact different. For example in commit
1a07e59c3e ("Update messages in preparation for i18n", 2018-07-21) the
lines

-   die (_("must end with a color"));
+   die(_("must end with a color"));

are colored as moved even though they are different.

This is because if there is a fuzzy match for the first line of
a potential moved block the line is marked as moved before the
potential match is checked to see if it actually matches. The fix is
to delay marking the line as moved until after we have checked that
there really is at least one matching potential moved block.

Note that the test modified in the last commit still fails because
adding an unmoved line between two moved blocks that are already
separated by unmoved lines changes the color of the block following the
addition. This should not be the case and will be fixed in the next
commit.

Signed-off-by: Phillip Wood 
---
 diff.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/diff.c b/diff.c
index 9b9811988b..53a7ab5aca 100644
--- a/diff.c
+++ b/diff.c
@@ -1104,10 +1104,10 @@ static void mark_color_as_moved(struct diff_options *o,
continue;
}
 
-   l->flags |= DIFF_SYMBOL_MOVED_LINE;
-
-   if (o->color_moved == COLOR_MOVED_PLAIN)
+   if (o->color_moved == COLOR_MOVED_PLAIN) {
+   l->flags |= DIFF_SYMBOL_MOVED_LINE;
continue;
+   }
 
if (o->color_moved_ws_handling &
COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE)
@@ -1141,10 +1141,13 @@ static void mark_color_as_moved(struct diff_options *o,
block_length = 0;
}
 
-   block_length++;
+   if (pmb_nr) {
+   block_length++;
 
-   if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
-   l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
+   l->flags |= DIFF_SYMBOL_MOVED_LINE;
+   if (flipped_block && o->color_moved != 
COLOR_MOVED_BLOCKS)
+   l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
+   }
}
adjust_last_block(o, n, block_length);
 
-- 
2.19.1.1690.g258b440b18



[PATCH v2 2/9] Use "whitespace" consistently

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Most of the messages and documentation use 'whitespace' rather than
'white space' or 'white spaces' convert to latter two to the former for
consistency.

Signed-off-by: Phillip Wood 
---
 Documentation/diff-options.txt | 4 ++--
 Documentation/git-cat-file.txt | 8 
 diff.c | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 151690f814..57a2f4cb7a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -298,7 +298,7 @@ dimmed-zebra::
settings. It is the same as `--color-moved=no`.
 
 --color-moved-ws=::
-   This configures how white spaces are ignored when performing the
+   This configures how whitespace is ignored when performing the
move detection for `--color-moved`.
 ifdef::git-diff[]
It can be set by the `diff.colorMovedWS` configuration setting.
@@ -316,7 +316,7 @@ ignore-all-space::
Ignore whitespace when comparing lines. This ignores differences
even if one line has whitespace where the other line has none.
 allow-indentation-change::
-   Initially ignore any white spaces in the move detection, then
+   Initially ignore any whitespace in the move detection, then
group the moved code blocks only into a block if the change in
whitespace is the same per line. This is incompatible with the
other modes.
diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 74013335a1..9a2e9cdafb 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -23,8 +23,8 @@ In the second form, a list of objects (separated by 
linefeeds) is provided on
 stdin, and the SHA-1, type, and size of each object is printed on stdout. The
 output format can be overridden using the optional `` argument. If
 either `--textconv` or `--filters` was specified, the input is expected to
-list the object names followed by the path name, separated by a single white
-space, so that the appropriate drivers can be determined.
+list the object names followed by the path name, separated by a single
+whitespace, so that the appropriate drivers can be determined.
 
 OPTIONS
 ---
@@ -79,15 +79,15 @@ OPTIONS
Print object information and contents for each object provided
on stdin.  May not be combined with any other options or arguments
except `--textconv` or `--filters`, in which case the input lines
-   also need to specify the path, separated by white space.  See the
+   also need to specify the path, separated by whitespace.  See the
section `BATCH OUTPUT` below for details.
 
 --batch-check::
 --batch-check=::
Print object information for each object provided on stdin.  May
not be combined with any other options or arguments except
`--textconv` or `--filters`, in which case the input lines also
-   need to specify the path, separated by white space.  See the
+   need to specify the path, separated by whitespace.  See the
section `BATCH OUTPUT` below for details.
 
 --batch-all-objects::
diff --git a/diff.c b/diff.c
index c29b1cce14..78cd3958f4 100644
--- a/diff.c
+++ b/diff.c
@@ -320,7 +320,7 @@ static int parse_color_moved_ws(const char *arg)
 
if ((ret & COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) &&
(ret & XDF_WHITESPACE_FLAGS))
-   die(_("color-moved-ws: allow-indentation-change cannot be 
combined with other white space modes"));
+   die(_("color-moved-ws: allow-indentation-change cannot be 
combined with other whitespace modes"));
 
string_list_clear(&l, 0);
 
-- 
2.19.1.1690.g258b440b18



[PATCH v2 3/9] diff: allow --no-color-moved-ws

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Allow --no-color-moved-ws and --color-moved-ws=no to cancel any previous
--color-moved-ws option.

Signed-off-by: Phillip Wood 
---
 Documentation/diff-options.txt | 7 +++
 diff.c | 6 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 57a2f4cb7a..e1744fa80d 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -306,6 +306,8 @@ endif::git-diff[]
These modes can be given as a comma separated list:
 +
 --
+no::
+   Do not ignore whitespace when performing move detection.
 ignore-space-at-eol::
Ignore changes in whitespace at EOL.
 ignore-space-change::
@@ -322,6 +324,11 @@ allow-indentation-change::
other modes.
 --
 
+--no-color-moved-ws::
+   Do not ignore whitespace when performing move detection. This can be
+   used to override configuration settings. It is the same as
+   `--color-moved-ws=no`.
+
 --word-diff[=]::
Show a word diff, using the  to delimit changed words.
By default, words are delimited by whitespace; see
diff --git a/diff.c b/diff.c
index 78cd3958f4..9b9811988b 100644
--- a/diff.c
+++ b/diff.c
@@ -304,7 +304,9 @@ static int parse_color_moved_ws(const char *arg)
strbuf_addstr(&sb, i->string);
strbuf_trim(&sb);
 
-   if (!strcmp(sb.buf, "ignore-space-change"))
+   if (!strcmp(sb.buf, "no"))
+   ret = 0;
+   else if (!strcmp(sb.buf, "ignore-space-change"))
ret |= XDF_IGNORE_WHITESPACE_CHANGE;
else if (!strcmp(sb.buf, "ignore-space-at-eol"))
ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
@@ -5008,6 +5010,8 @@ int diff_opt_parse(struct diff_options *options,
if (cm < 0)
die("bad --color-moved argument: %s", arg);
options->color_moved = cm;
+   } else if (!strcmp(arg, "--no-color-moved-ws")) {
+   options->color_moved_ws_handling = 0;
} else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
options->color_moved_ws_handling = parse_color_moved_ws(arg);
} else if (skip_to_optional_arg_default(arg, "--color-words", 
&options->word_regex, NULL)) {
-- 
2.19.1.1690.g258b440b18



[PATCH v2 8/9] diff --color-moved-ws: modify allow-indentation-change

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Currently diff --color-moved-ws=allow-indentation-change does not
support indentation that contains a mix of tabs and spaces. For
example in commit 546f70f377 ("convert.h: drop 'extern' from function
declaration", 2018-06-30) the function parameters in the following
lines are not colored as moved [1].

-extern int stream_filter(struct stream_filter *,
-const char *input, size_t *isize_p,
-char *output, size_t *osize_p);
+int stream_filter(struct stream_filter *,
+ const char *input, size_t *isize_p,
+ char *output, size_t *osize_p);

This commit changes the way the indentation is handled to track the
visual size of the indentation rather than the characters in the
indentation. This has the benefit that any whitespace errors do not
interfer with the move detection (the whitespace errors will still be
highlighted according to --ws-error-highlight). During the discussion
of this feature there were concerns about the correct detection of
indentation for python. However those concerns apply whether or not
we're detecting moved lines so no attempt is made to determine if the
indentation is 'pythonic'.

[1] Note that before the commit to fix the erroneous coloring of moved
lines each line was colored as a different block, since that commit
they are uncolored.

Signed-off-by: Phillip Wood 
---
 diff.c | 130 +
 t/t4015-diff-whitespace.sh |  56 
 2 files changed, 129 insertions(+), 57 deletions(-)

diff --git a/diff.c b/diff.c
index c378ce3daf..148503e49c 100644
--- a/diff.c
+++ b/diff.c
@@ -750,6 +750,8 @@ struct emitted_diff_symbol {
const char *line;
int len;
int flags;
+   int indent_off;   /* Offset to first non-whitespace character */
+   int indent_width; /* The visual width of the indentation */
enum diff_symbol s;
 };
 #define EMITTED_DIFF_SYMBOL_INIT {NULL}
@@ -780,44 +782,68 @@ struct moved_entry {
struct moved_entry *next_line;
 };
 
-/**
- * The struct ws_delta holds white space differences between moved lines, i.e.
- * between '+' and '-' lines that have been detected to be a move.
- * The string contains the difference in leading white spaces, before the
- * rest of the line is compared using the white space config for move
- * coloring. The current_longer indicates if the first string in the
- * comparision is longer than the second.
- */
-struct ws_delta {
-   char *string;
-   unsigned int current_longer : 1;
-};
-#define WS_DELTA_INIT { NULL, 0 }
-
 struct moved_block {
struct moved_entry *match;
-   struct ws_delta wsd;
+   int wsd; /* The whitespace delta of this block */
 };
 
 static void moved_block_clear(struct moved_block *b)
 {
-   FREE_AND_NULL(b->wsd.string);
-   b->match = NULL;
+   memset(b, 0, sizeof(*b));
+}
+
+static void fill_es_indent_data(struct emitted_diff_symbol *es)
+{
+   unsigned int off = 0;
+   int width = 0, tab_width = es->flags & WS_TAB_WIDTH_MASK;
+   const char *s = es->line;
+   const int len = es->len;
+
+   /* skip any \v \f \r at start of indentation */
+   while (s[off] == '\f' || s[off] == '\v' ||
+  (s[off] == '\r' && off < len - 1))
+   off++;
+
+   /* calculate the visual width of indentation */
+   while(1) {
+   if (s[off] == ' ') {
+   width++;
+   off++;
+   } else if (s[off] == '\t') {
+   width += tab_width - (width % tab_width);
+   while (s[++off] == '\t')
+   width += tab_width;
+   } else {
+   break;
+   }
+   }
+
+   es->indent_off = off;
+   es->indent_width = width;
 }
 
 static int compute_ws_delta(const struct emitted_diff_symbol *a,
-const struct emitted_diff_symbol *b,
-struct ws_delta *out)
+   const struct emitted_diff_symbol *b,
+   int *out)
 {
-   const struct emitted_diff_symbol *longer =  a->len > b->len ? a : b;
-   const struct emitted_diff_symbol *shorter = a->len > b->len ? b : a;
-   int d = longer->len - shorter->len;
+   int a_len = a->len,
+   b_len = b->len,
+   a_off = a->indent_off,
+   a_width = a->indent_width,
+   b_off = b->indent_off,
+   b_width = b->indent_width;
+   int delta;
 
-   if (strncmp(longer->line + d, shorter->line, shorter->len))
+   if (a->s == DIFF_SYMBOL_PLUS)
+   delta = a_width - b_width;
+   else
+   delta = b_width - a_width;
+
+   if (a_len - a_off != b_len - b_off ||
+   memcmp(a->line + a_off, b->line + b_off, a_len - a_off))
return 0;
 
-   out->string = xmemdupz(lo

[PATCH v2 4/9] diff --color-moved-ws: demonstrate false positives

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

'diff --color-moved-ws=allow-indentation-change' can highlight lines
that have internal whitespace changes rather than indentation
changes. For example in commit 1a07e59c3e ("Update messages in
preparation for i18n", 2018-07-21) the lines

-   die (_("must end with a color"));
+   die(_("must end with a color"));

are highlighted as moved when they should not be. Modify an existing
test to show the problem that will be fixed in the next commit.

Signed-off-by: Phillip Wood 
---
 t/t4015-diff-whitespace.sh | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index a9fb226c5a..eee81a1987 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1809,7 +1809,7 @@ test_expect_success 'only move detection ignores white 
spaces' '
test_cmp expected actual
 '
 
-test_expect_success 'compare whitespace delta across moved blocks' '
+test_expect_failure 'compare whitespace delta across moved blocks' '
 
git reset --hard &&
q_to_tab <<-\EOF >text.txt &&
@@ -1827,6 +1827,7 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
QQQthat has similar lines
QQQto previous blocks, but with different indent
QQQYetQAnotherQoutlierQ
+   QLine with internal w h i t e s p a c e change
EOF
 
git add text.txt &&
@@ -1847,6 +1848,7 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
QQthat has similar lines
QQto previous blocks, but with different indent
QQYetQAnotherQoutlier
+   QLine with internal whitespace change
EOF
 
git diff --color --color-moved 
--color-moved-ws=allow-indentation-change >actual.raw &&
@@ -1856,7 +1858,7 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
diff --git a/text.txt b/text.txt
--- a/text.txt
+++ b/text.txt
-   @@ -1,14 +1,14 @@
+   @@ -1,15 +1,15 @@
-QIndented
-QText across
-Qsome lines
@@ -1871,6 +1873,7 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
-QQQthat has similar lines
-QQQto previous blocks, but with different 
indent
-QQQYetQAnotherQoutlierQ
+   -QLine with internal w h i t e s p a c e change
+QQIndented
+QQText across
+QQsome lines
@@ -1885,6 +1888,7 @@ test_expect_success 'compare whitespace delta across 
moved blocks' '
+QQthat has similar lines
+QQto previous blocks, but with 
different indent
+QQYetQAnotherQoutlier
+   +QLine with internal whitespace 
change
EOF
 
test_cmp expected actual
-- 
2.19.1.1690.g258b440b18



[PATCH v2 9/9] diff --color-moved-ws: handle blank lines

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

When using --color-moved-ws=allow-indentation-change allow lines with
the same indentation change to be grouped across blank lines. For now
this only works if the blank lines have been moved as well, not for
blocks that have just had their indentation changed.

This completes the changes to the implementation of
--color-moved=allow-indentation-change. Running

  git diff --color-moved=allow-indentation-change v2.18.0 v2.19.0

now takes 5.0s. This is a saving of 41% from 8.5s for the optimized
version of the previous implementation and 66% from the original which
took 14.6s.

Signed-off-by: Phillip Wood 
---
 diff.c | 34 ---
 t/t4015-diff-whitespace.sh | 41 ++
 2 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index 148503e49c..ef5b8c78d7 100644
--- a/diff.c
+++ b/diff.c
@@ -792,9 +792,11 @@ static void moved_block_clear(struct moved_block *b)
memset(b, 0, sizeof(*b));
 }
 
+#define INDENT_BLANKLINE INT_MIN
+
 static void fill_es_indent_data(struct emitted_diff_symbol *es)
 {
-   unsigned int off = 0;
+   unsigned int off = 0, i;
int width = 0, tab_width = es->flags & WS_TAB_WIDTH_MASK;
const char *s = es->line;
const int len = es->len;
@@ -818,8 +820,18 @@ static void fill_es_indent_data(struct emitted_diff_symbol 
*es)
}
}
 
-   es->indent_off = off;
-   es->indent_width = width;
+   /* check if this line is blank */
+   for (i = off; i < len; i++)
+   if (!isspace(s[i]))
+   break;
+
+   if (i == len) {
+   es->indent_width = INDENT_BLANKLINE;
+   es->indent_off = len;
+   } else {
+   es->indent_off = off;
+   es->indent_width = width;
+   }
 }
 
 static int compute_ws_delta(const struct emitted_diff_symbol *a,
@@ -834,6 +846,11 @@ static int compute_ws_delta(const struct 
emitted_diff_symbol *a,
b_width = b->indent_width;
int delta;
 
+   if (a_width == INDENT_BLANKLINE && b_width == INDENT_BLANKLINE) {
+   *out = INDENT_BLANKLINE;
+   return 1;
+   }
+
if (a->s == DIFF_SYMBOL_PLUS)
delta = a_width - b_width;
else
@@ -877,6 +894,10 @@ static int cmp_in_block_with_wsd(const struct diff_options 
*o,
if (al != bl)
return 1;
 
+   /* If 'l' and 'cur' are both blank then they match. */
+   if (a_width == INDENT_BLANKLINE && c_width == INDENT_BLANKLINE)
+   return 0;
+
/*
 * The indent changes of the block are known and stored in pmb->wsd;
 * however we need to check if the indent changes of the current line
@@ -888,6 +909,13 @@ static int cmp_in_block_with_wsd(const struct diff_options 
*o,
else
delta = c_width - a_width;
 
+   /*
+* If the previous lines of this block were all blank then set its
+* whitespace delta.
+*/
+   if (pmb->wsd == INDENT_BLANKLINE)
+   pmb->wsd = delta;
+
return !(delta == pmb->wsd && al - a_off == cl - c_off &&
 !memcmp(a, b, al) && !
 memcmp(a + a_off, c + c_off, al - a_off));
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index e023839ba6..9d6f88b07f 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1901,10 +1901,20 @@ test_expect_success 'compare whitespace delta 
incompatible with other space opti
test_i18ngrep allow-indentation-change err
 '
 
+EMPTY=''
 test_expect_success 'compare mixed whitespace delta across moved blocks' '
 
git reset --hard &&
tr Q_ "\t " <<-EOF >text.txt &&
+   ${EMPTY}
+   too short without
+   ${EMPTY}
+   ___being grouped across blank line
+   ${EMPTY}
+   context
+   lines
+   to
+   anchor
Indented text to
_Qbe further indented by four spaces across
Qseveral lines
@@ -1918,9 +1928,18 @@ test_expect_success 'compare mixed whitespace delta 
across moved blocks' '
git commit -m "add text.txt" &&
 
tr Q_ "\t " <<-EOF >text.txt &&
+   context
+   lines
+   to
+   anchor
QIndented text to
QQbe further indented by four spaces across
Qseveral lines
+   ${EMPTY}
+   QQtoo short without
+   ${EMPTY}
+   Q___being grouped across blank line
+   ${EMPTY}
Q_QThese two lines have had their
indentation reduced by four spaces
QQdifferent indentation change
@@ -1937,7 +1956,16 @@ test_expect_success 'compare mixed whitespace delta 
across moved blocks' '
diff --git a/text.txt b/text.txt
--- a/text.txt
+++ b/text.txt
-   @@ -1,7 +1,7 @@
+   @@ -1,16 +1,16 @@
+   -
+   -too short without
+   -
+   -   being gro

[PATCH v2 1/9] diff: document --no-color-moved

2018-11-23 Thread Phillip Wood
From: Phillip Wood 

Add documentation for --no-color-moved.

Signed-off-by: Phillip Wood 
---
 Documentation/diff-options.txt | 4 
 1 file changed, 4 insertions(+)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 0378cd574e..151690f814 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -293,6 +293,10 @@ dimmed-zebra::
`dimmed_zebra` is a deprecated synonym.
 --
 
+--no-color-moved::
+   Turn off move detection. This can be used to override configuration
+   settings. It is the same as `--color-moved=no`.
+
 --color-moved-ws=::
This configures how white spaces are ignored when performing the
move detection for `--color-moved`.
-- 
2.19.1.1690.g258b440b18



Re: [RFC/PATCH 0/5] stop installing old libexec aliases like "git-init"

2018-11-23 Thread Johannes Schindelin
Hi Peff,

On Thu, 22 Nov 2018, Jeff King wrote:

> On Thu, Nov 22, 2018 at 01:48:53PM +0100, Johannes Schindelin wrote:
> 
> > So YMMV with git-s. My rule of thumb is: if I want to use this
> > myself only, I'll make it an alias. If I want to ship it (e.g. with Git
> > for Windows), I'll make it a git-.
> 
> I have a handful of personal git-* scripts: mostly ones that are big
> enough to be unwieldy as an alias. But then, $PATH management is pretty
> straightforward on my platforms, so it's easier to drop a script there
> than to point an alias to a non-git-* script.

Oh, my Git aliases pretty much *all* point to a single script that takes
subcommands.

Ciao,
Dscho


Re: [PATCH v1 1/1] t5601-99: Enable colliding file detection for MINGW

2018-11-23 Thread Johannes Schindelin
Hi Carlo,

On Thu, 22 Nov 2018, Carlo Marcelo Arenas Belón wrote:

> Subject: [PATCH] entry: remove windows fallback to inode checking
> 
> this test is really FS specific, so is better to avoid any compiled
> assumptions about the platform and let the user drive the fallback
> through core.checkStat instead
> 
> Signed-off-by: Carlo Marcelo Arenas Belón 
> ---
>  entry.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/entry.c b/entry.c
> index 0a3c451f5f..5ae74856e6 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -404,10 +404,6 @@ static void mark_colliding_entries(const struct checkout 
> *state,
>  {
>   int i, trust_ino = check_stat;
>  
> -#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
> - trust_ino = 0;
> -#endif
> -

No, we cannot drop this. You may not see it in git.git's source code, but
in Git for Windows' patches, we have an experimental feature (which had
seemed to stabilize, but Ben Peart is currently doing wonders with it,
improving the performance substantially) for accelerating the file
metadata enumeration in a noticeable manner. The only way we can do that
is by *not* insisting on a correct inode.

Besides, IIRC even our regular stat() now "fails" to fill the inode field.

So no, we cannot do that. We can probably drop the `||
defined(__CYGINW__)` part (Cygwin even generates a fake inode for FAT,
where no equivalent is available, by hashing the full normalized path).
But you cannot drop the `GIT_WINDOWS_NATIVE` part.

Ciao,
Johannes

>   ce->ce_flags |= CE_MATCHED;
>  
>   for (i = 0; i < state->istate->cache_nr; i++) {
> -- 
> 2.20.0.rc1
> 
> 

Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C

2018-11-23 Thread Martin Ågren
On Fri, 23 Nov 2018 at 11:13, Johannes Schindelin
 wrote:
> On Mon, 30 Oct 2017, Pranit Bauva wrote:
>
> > On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren  
> > wrote:
> > > On 27 October 2017 at 17:06, Pranit Bauva  wrote:
> > >> +static void free_terms(struct bisect_terms *terms)
> > >> +{
> > >> +   if (!terms->term_good)
> > >> +   free((void *) terms->term_good);
> > >> +   if (!terms->term_bad)
> > >> +   free((void *) terms->term_bad);
> > >> +}

> > > You leave the pointers dangling, but you're ok for now since this is the
> > > last thing that happens in `cmd_bisect__helper()`. Your later patches
> > > add more users, but they're also ok, since they immediately assign new
> > > values.
> > >
> > > In case you (and others) find it useful, the below is a patch I've been
> > > sitting on for a while as part of a series to plug various memory-leaks.
> > > `FREE_AND_NULL_CONST()` would be useful in precisely these situations.
> >
> > Honestly, I wouldn't be the best person to judge this.
>
> Git's source code implicitly assumes that any `const` pointer refers to
> memory owned by another code path. It is therefore probably not a good
> idea to encourage `free()`ing a `const` pointer.

Yeah, I never submitted that patch as part of a real series. I remember
having a funky feeling about it, and whatever use-case I had for this
macro, I managed to solve the memory leak in some other way in a
rewrite.

Thanks for a sanity check.

Martin


Re: [PATCH] t5562: skip if NO_CURL is enabled

2018-11-23 Thread Carlo Arenas
On Thu, Nov 22, 2018 at 3:43 PM Max Kirillov  wrote:
> also edited the test to include only push_plain case,
> and repeat it several times, to avoid running irrelevant
> cases, the failure never happened again.

as I explained previously[1] and as odd as it might seem the
push_plain case ONLY
fails if your run them together with the other tests that return
errors with compressed input

frankly I don't understand how one could affect the other as they
should be running in independent processes but it happens fairly
consistently in NetBSD (tested 7.1, 7.2, 8) with only one CPU (tested
i386 and amd64)

Carlo

[1] 
https://public-inbox.org/git/20181119213924.ga2...@sigill.intra.peff.net/T/#m041e9703432c39dcb04fe10e86fc53d5254474b4


Re: [PATCH] t5562: fix perl path

2018-11-23 Thread Carlo Arenas
Tested-by: Carlo Marcelo Arenas Belón 

IMHO leaving the shebang might be better if only for consistency but
could go eitherway

Carlo


BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF

2018-11-23 Thread Frank Schäfer
The CR marker ^M doesn't show up in '-' lines of diffs when the ending
of the removed line is CR+LF.
It shows up as expected in '-' lines when the ending of the removed line
is CR only.
It also always shows up as expected in '+' lines.

These are the diffs of the 6 possible line ending changes:

LF to CR+LF:
@@ -1,2 +1,2 @@
-aaa
+aaa^M
 bbb
 
CR+LF to LF:
@@ -1,2 +1,2 @@
-aaa    => BUG: should be -aaa^M
+aaa
 bbb

CR to CR+LF:
@@ -1 +1,2 @@
-aaa^Mbbb
+aaa^M
+bbb

CR+LF to CR:
@@ -1,2 +1 @@
-aaa    => BUG: should be -aaa^M
-bbb
+aaa^Mbbb

LF to CR:
@@ -1,2 +1 @@
-aaa
-bbb
+aaa^Mbbb

CR to LF:
@@ -1 +1,2 @@
-aaa^Mbbb
+aaa
+bbb

Tested with version 2.19.1.

Regards,
Frank Schäfer



Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF

2018-11-23 Thread Johannes Sixt

Am 23.11.18 um 19:19 schrieb Frank Schäfer:

The CR marker ^M doesn't show up in '-' lines of diffs when the ending
of the removed line is CR+LF.
It shows up as expected in '-' lines when the ending of the removed line
is CR only.
It also always shows up as expected in '+' lines.


Is your repository configured to (1) highlight whitespace errors in diff 
output and (2) to leave CRLF alone in text files?


If so, then it is just a side-effect of this combination, an illusion, 
so to say: The CR in the CRLF combo is trailing whitespace. The 'git 
diff' marks it by inserting an escape sequence to switch the color 
before ^M and another escape sequence to reset to color after ^M. This 
breaks the CRLF combination apart, so that the pager does not process it 
as a combined CRLF sequence; it displays the lone CR as ^M.


It is easy to achieve the opposite effect, i.e., that ^M is not 
displayed. For example with these lines in .git/info/attributes or 
.gitattributes:


*.cpp 
whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab

*.h whitespace=trailing-space,cr-at-eol,indent-with-non-tab,space-before-tab

Note the cr-at-eol. (There may be shorter versions to achieve a similar 
effect.)


-- Hannes


Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

2018-11-23 Thread Junio C Hamano
Johannes Schindelin  writes:

> To fix this, I prepared a GitGitGadget PR
> (https://github.com/gitgitgadget/git/pull/87) and will submit it as soon
> as I am satisfied that the build works.

Thanks.  This won't be in the upcoming release anyway, so we can fix
it up without "oops, let's pile another fix on it" if we wanted to
after the release by kicking it back to 'pu', but in the meantime,
keeping the tip of 'next' free from known breakage certainly is a
sensible thing to do.



Re: [PATCH v4 0/2] Fix scissors bug during merge conflict

2018-11-23 Thread Junio C Hamano
Denton Liu  writes:

> I just realised that there is a slight problem with the proposed change.
> When we do a merge and there are no merge conflicts, at the end of the
> merge, we get dropped into an editor with this text:
>
>   Merge branch 'master' into new
>
>   # Please enter a commit message to explain why this merge is necessary,
>   # especially if it merges an updated upstream into a topic branch.
>   #
>   # Lines starting with '#' will be ignored, and an empty message aborts
>   # the commit.
>
> Note that in git-merge, the cleanup only removes commented lines and
> this cannot be configured to be scissors or whatever else. I think that
> the fact that it's not configurable isn't a problem; most hardcore
> commit message editing happens in git-commit anyway.

OK.

> However, since we taught git-merge the --cleanup option, this might be
> misleading for the end-user since they would expect the MERGE_MSG to be
> cleaned up as specified.
>
> I see two resolutions for this. We can either rename --cleanup more
> precisely so users won't be confused (perhaps something like
> --merge-conflict-scissors but a lot more snappy) or we can actually make
> git-merge respect the cleanup option and post-process the message
> according to the specified cleanup rule.

The former certainly would be simpler to implement, but feels more
like an excuse for not doing the right thing to me, when I put
myself in shoes of users who use 'scissors' clean-up option in
commit.  I dunno.



Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)

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

> Yeah, my intent had been to circle back around to this, but I just
> hadn't gotten to it. I'm still pondering a config option or similar,
> though I remain unconvinced that the cases in which you've showed it
> being slow are actually realistic or worth worrying about (and certainly
> having an obscure config option is not enough to help most people). If
> we could have it kick in heuristically, that would be better.

Me neither and I agree.

> However, note that the cache-load for finding abbreviations _must_ have
> the complete list. And has been loading it for some time. So if you run
> "git-fetch", for example, you've already been running this code for
> months (and using the cache in more places is now a free speedup).
>
> At the very least, we'd want this patch on top, too. I also think René's
> suggestion use access() is worth pursuing (though to some degree is
> orthogonal to the cache).

OK, will queue, but I wonder where 66f0 comes from before silently
doing s/66f04152be/3a2e08245c/ myself.



> -- >8 --
> Subject: [PATCH] odb_load_loose_cache: fix strbuf leak
>
> Commit 66f04152be (object-store: provide helpers for
> loose_objects_cache, 2018-11-12) moved the cache-loading code from
> find_short_object_filename(), but forgot the line that releases the path
> strbuf.
>
> Reported-by: René Scharfe 
> Signed-off-by: Jeff King 
> ---
>  sha1-file.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/sha1-file.c b/sha1-file.c
> index 5894e48ea4..5a272f70de 100644
> --- a/sha1-file.c
> +++ b/sha1-file.c
> @@ -2169,6 +2169,7 @@ void odb_load_loose_cache(struct object_directory *odb, 
> int subdir_nr)
>   NULL, NULL,
>   &odb->loose_objects_cache);
>   odb->loose_objects_subdir_seen[subdir_nr] = 1;
> + strbuf_release(&buf);
>  }
>  
>  static int check_stream_sha1(git_zstream *stream,


Re: [PATCH v1 2/2] log -S: Add test which searches in binary files

2018-11-23 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason  writes:

> On Wed, Nov 21 2018, Thomas Braun wrote:
>
>> The -S  option of log looks for differences that changes the
>> number of occurrences of the specified string (i.e. addition/deletion)
>> in a file.
>>
> ...
> This should just be part of 1/2 since the behavior is changed there &
> the commit message should describe both cases.

Sensible suggestion.  Thanks.


Re: [PATCH 2/2] format-patch: don't include --stat with --range-diff output

2018-11-23 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

>   if (rev->rdiff1) {
> + struct diff_options opts;
> + memcpy(&opts, &rev->diffopt, sizeof(opts));
> + opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT | 
> DIFF_FORMAT_SUMMARY);
> +
>   fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
>   show_range_diff(rev->rdiff1, rev->rdiff2,
> - rev->creation_factor, 1, &rev->diffopt);
> + rev->creation_factor, 1, &opts);

I am not quite convinced if this shallow copy is a safe thing to do.
Quite honestly at this late in the release cycle, as a band-aid, I'd
rather see a simpler revert than a change like this that we have to
worry about what happens if/when show_range_diff() _thinks_ it is
done with the opts and ends up discarding resources (e.g. "FILE *")
that are shared with rev->diffopt that would still have to be used
later.



Re: [PATCH v1 1/2] log -G: Ignore binary files

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

>> +if ((o->pickaxe_opts & DIFF_PICKAXE_KIND_G) &&
>> +((!textconv_one && diff_filespec_is_binary(o->repo, p->one)) ||
>> + (!textconv_two && diff_filespec_is_binary(o->repo, p->two
>> +return 0;
>
> If the user passes "-a" to treat binary files as text, we should
> probably skip the binary check. I think we'd need to check
> "o->flags.text" here.

Yeah, I forgot about that option.  It would give an escape hatch
that has a sane explanation.


Re: [PATCH] doc: update diff-format.txt for removed ellipses

2018-11-23 Thread Junio C Hamano
Thanks for a patch.

Greg Hurrell  writes:

> Commit 7cb6ac1e4b made the diff format omit ellipses by default, but
> there is still this place in the documentation where we show examples of
> output with ellipses.

We prefer to cite an existing commit with its title and date these
days, not just with its object name.

Since 7cb6ac1e ("diff: diff_aligned_abbrev: remove ellipsis after
abbreviated SHA-1 value", 2017-12-03), the "--raw" format of diff
does not add ellipsis in an attempt to align the output, but...

or something like that.  Note that saying this is about the raw format
is quite essential thing to tell the readers to explain this hange.

> The GIT_PRINT_SHA1_ELLIPSIS environment variable can be used, for now,
> to bring back the old output format, but that is already documented in
> git.txt, so I am not mentioning it here.

Yeah, I do not think it makes sense to use the workaround that is
planned for removal, which will later make us revise the example in
the documentation again, to end up with the text that you have right
now.  I do not think this three-line paragraph needs to be in the
log message, either, though.  Perhaps below the three-dash line.


Also please sign-off your patch here (see
Documentation/SubmittingPatches).

> ---

Thanks.

>  Documentation/diff-format.txt | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/diff-format.txt b/Documentation/diff-format.txt
> index 706916c94c..cdcc17f0ad 100644
> --- a/Documentation/diff-format.txt
> +++ b/Documentation/diff-format.txt
> @@ -26,12 +26,12 @@ line per changed file.
>  An output line is formatted this way:
>  
>  
> -in-place edit  :100644 100644 bcd1234... 0123456... M file0
> -copy-edit  :100644 100644 abcd123... 1234567... C68 file1 file2
> -rename-edit:100644 100644 abcd123... 1234567... R86 file1 file3
> -create :00 100644 000... 1234567... A file4
> -delete :100644 00 1234567... 000... D file5
> -unmerged   :00 00 000... 000... U file6
> +in-place edit  :100644 100644 bcd1234 0123456 M file0
> +copy-edit  :100644 100644 abcd123 1234567 C68 file1 file2
> +rename-edit:100644 100644 abcd123 1234567 R86 file1 file3
> +create :00 100644 000 1234567 A file4
> +delete :100644 00 1234567 000 D file5
> +unmerged   :00 00 000 000 U file6
>  
>  
>  That is, from the left to the right:
> @@ -75,7 +75,7 @@ and it is out of sync with the index.
>  Example:
>  
>  
> -:100644 100644 5be4a4.. 00.. M file.c
> +:100644 100644 5be4a4a 000 M file.c
>  
>  
>  Without the `-z` option, pathnames with "unusual" characters are
> @@ -100,7 +100,7 @@ from the format described above in the following way:
>  Example:
>  
>  
> -::100644 100644 100644 fabadb8... cc95eb0... 4866510... MM   describe.c
> +::100644 100644 100644 fabadb8 cc95eb0 4866510 MMdescribe.c
>  
>  
>  Note that 'combined diff' lists only files which were modified from


Re: [PATCH 1/1] ref-filter: replace unportable `%lld` format

2018-11-23 Thread Junio C Hamano
"Johannes Schindelin via GitGitGadget" 
writes:

> From: Johannes Schindelin 
>
> The `%lld` format is supported on Linux and macOS, but not on Windows.
> This issue has been reported ten days ago (Message-ID:
> nycvar.qro.7.76.6.1811121300520...@tvgsbejvaqbjf.bet), but the
> corresponding topic still advanced to `next` in the meantime, breaking
> the Windows build.
>
> Let's use `PRIdMAX` and a cast to `intmax_t` instead, which unbreaks the
> build, and imitates how we do things e.g. in `json-writer.c` already.

We seem to be already using PRIdMAX in 'master', so this is safe
thing to do and I will merge it directly to 'next', but the fact
that PRIdMAX is used without any fallback definition in
git-compat-util.h makes me suspect that we can now drop the fallback
for PRIuMAX.

Thanks.


> Signed-off-by: Johannes Schindelin 
> ---
>  ref-filter.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 3cfe01a039..69cdf2dbb5 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -897,7 +897,7 @@ static void grab_common_values(struct atom_value *val, 
> int deref, struct expand_
>   v->s = xstrdup(type_name(oi->type));
>   else if (!strcmp(name, "objectsize:disk")) {
>   v->value = oi->disk_size;
> - v->s = xstrfmt("%lld", (long long)oi->disk_size);
> + v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);
>   } else if (!strcmp(name, "objectsize")) {
>   v->value = oi->size;
>   v->s = xstrfmt("%lu", oi->size);


git overwriting local ignored files?

2018-11-23 Thread David Mandelberg

Hi,

It seems that git is overwriting my local files on merge if they're in 
.gitignore. See command transcript below. I searched `git help config` 
and Google, but I couldn't find any way to prevent it. Am I missing 
something? (The reason I care about ignored files is that I'm using git 
with a working directory of $HOME to manage my dotfiles, and most files 
in my $HOME are not tracked by git but are still important.)


dseomn@solaria:~$ mktemp -d
/tmp/tmp.RHyyMxJ5Zp
dseomn@solaria:~$ cd /tmp/tmp.RHyyMxJ5Zp
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp$ git init --bare repo.git
Initialized empty Git repository in /tmp/tmp.RHyyMxJ5Zp/repo.git/
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp$ git clone repo.git foo
Cloning into 'foo'...
warning: You appear to have cloned an empty repository.
done.
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp$ git clone repo.git bar
Cloning into 'bar'...
warning: You appear to have cloned an empty repository.
done.
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp$ cd foo
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ echo '*' > .gitignore
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ git add -f .gitignore
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ git commit -m 'gitignore'
[master (root-commit) affd2fb] gitignore
 1 file changed, 1 insertion(+)
 create mode 100644 .gitignore
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ git push
Counting objects: 3, done.
Writing objects: 100% (3/3), 216 bytes | 216.00 KiB/s, done.
Total 3 (delta 0), reused 0 (delta 0)
To /tmp/tmp.RHyyMxJ5Zp/repo.git
 * [new branch]  master -> master
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ cd ../bar
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ git fetch
remote: Counting objects: 3, done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From /tmp/tmp.RHyyMxJ5Zp/repo
 * [new branch]  master -> origin/master
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ git merge @{u}
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ echo hi > quux
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ git add -f quux
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ git commit -m 'quux'
[master aee4b54] quux
 1 file changed, 1 insertion(+)
 create mode 100644 quux
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ git push
Counting objects: 3, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (2/2), done.
Writing objects: 100% (3/3), 273 bytes | 273.00 KiB/s, done.
Total 3 (delta 0), reused 0 (delta 0)
To /tmp/tmp.RHyyMxJ5Zp/repo.git
   affd2fb..aee4b54  master -> master
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/bar (master)$ cd ../foo
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ echo bye > quux
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ git fetch
remote: Counting objects: 3, done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From /tmp/tmp.RHyyMxJ5Zp/repo
   affd2fb..aee4b54  master -> origin/master
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ cat quux
bye
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ git merge @{u}
Updating affd2fb..aee4b54
Fast-forward
 quux | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 quux
dseomn@solaria:/tmp/tmp.RHyyMxJ5Zp/foo (master)$ cat quux
hi

--
https://david.mandelberg.org/


Re: [PATCH 1/2] format-patch: add a more exhaustive --range-diff test

2018-11-23 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> Change the narrow test added in 31e2617a5f ("format-patch: add
> --range-diff option to embed diff in cover letter", 2018-07-22) to
> test the full output. This test would have spotted a regression in the
> output if it wasn't beating around the bush and tested the full
> output, let's do that.

This looks wrong, given that we are declaring that showing the
broken --stat in the output is wrong.  It makes us to expect a
known-wrong output from the command.

If the theme of the topic were "what we are giving by default is a
sensible output when --stat is asked for, but it is rather too noisy
and our default should be quieter---let's tone it down", then this
patch in 1/2 and then a behaviour-change patch with updated
expectation would make sense, but I do not think that is what you
are aiming for with this series.

Perhaps squash this into the real "fix" to show what the desired
output should look like with the same patch?

> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
>  t/t3206-range-diff.sh | 27 ++-
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> index e497c1358f..0235c038be 100755
> --- a/t/t3206-range-diff.sh
> +++ b/t/t3206-range-diff.sh
> @@ -249,11 +249,28 @@ for prev in topic master..topic
>  do
>   test_expect_success "format-patch --range-diff=$prev" '
>   git format-patch --stdout --cover-letter --range-diff=$prev \
> - master..unmodified >actual &&
> - grep "= 1: .* s/5/A" actual &&
> - grep "= 2: .* s/4/A" actual &&
> - grep "= 3: .* s/11/B" actual &&
> - grep "= 4: .* s/12/B" actual
> + master..unmodified >actual.raw &&
> + sed -e "s|^:||" -e "s|:$||" >expect <<-\EOF &&
> + :1:  4de457d = 1:  35b9b25 s/5/A/
> + : a => b | 0
> + : 1 file changed, 0 insertions(+), 0 deletions(-)
> + ::
> + :2:  fccce22 = 2:  de345ab s/4/A/
> + : a => b | 0
> + : 1 file changed, 0 insertions(+), 0 deletions(-)
> + ::
> + :3:  147e64e = 3:  9af6654 s/11/B/
> + : a => b | 0
> + : 1 file changed, 0 insertions(+), 0 deletions(-)
> + ::
> + :4:  a63e992 = 4:  2901f77 s/12/B/
> + : a => b | 0
> + : 1 file changed, 0 insertions(+), 0 deletions(-)
> + ::
> + :-- :
> + EOF
> + sed -ne "/^1:/,/^--/p" actual &&
> + test_cmp expect actual
>   '
>  done


Re: [PATCH 2/2] format-patch: don't include --stat with --range-diff output

2018-11-23 Thread Junio C Hamano
Junio C Hamano  writes:

> Ævar Arnfjörð Bjarmason   writes:
>
>>  if (rev->rdiff1) {
>> +struct diff_options opts;
>> +memcpy(&opts, &rev->diffopt, sizeof(opts));
>> +opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT | 
>> DIFF_FORMAT_SUMMARY);
>> +
>>  fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
>>  show_range_diff(rev->rdiff1, rev->rdiff2,
>> -rev->creation_factor, 1, &rev->diffopt);
>> +rev->creation_factor, 1, &opts);
>
> I am not quite convinced if this shallow copy is a safe thing to do.
> Quite honestly at this late in the release cycle, as a band-aid, I'd
> rather see a simpler revert than a change like this that we have to
> worry about what happens if/when show_range_diff() _thinks_ it is
> done with the opts and ends up discarding resources (e.g. "FILE *")
> that are shared with rev->diffopt that would still have to be used
> later.

Well, let's take it anyway as-is, at least for today, as I notice
show_range_diff() itself does another shallow copy, so we are not
making anything dramatically worse.

Thanks.


Re: git overwriting local ignored files?

2018-11-23 Thread Junio C Hamano
David Mandelberg  writes:

> It seems that git is overwriting my local files on merge if they're in
> .gitignore. See command transcript below. I searched `git help config`
> and Google, but I couldn't find any way to prevent it. Am I missing
> something? (The reason I care about ignored files is that I'm using
> git with a working directory of $HOME to manage my dotfiles, and most
> files in my $HOME are not tracked by git but are still important.)

The .gitignore file is to list "ignored and expendable" class of
files; there is no "ignored but precious class" in Git.


[PATCH] t5562: do not reuse output files

2018-11-23 Thread Max Kirillov
Some expected failures of git-http-backend leave running its children
(receive-pack or upload-pack) which still hold opened descriptors
to act.err and with some probability they live long enough to write
their failure messages after next test has already truncated
the files. This causes occasional failures of the test script.

Avoid the issue by unlinking the older files before writing to them.

Reported-by: Carlo Arenas 
Helped-by: Carlo Arenas 
Signed-off-by: Max Kirillov 
---
Thanks for the analysis. I seem to have guessed the reason.
This patch should prevent it.

I think the tests should somehow make sure there are no such late
processes. I can see 2 options:
* somehow find out in the tests all children and wait for them. I have no idea 
how.
* make http-backend close handle to its child and wait for it to exit before 
dying.
  This would not prevent childrenc in general, because http-backend may be 
killed,
  but not in our expected failure cases

Actually, don't the children receive some SIGHUP? Maybe thy should. However, it
would still take some time for them to handle it, so it does not fully solve 
the issue
 t/t5562-http-backend-content-length.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/t5562-http-backend-content-length.sh 
b/t/t5562-http-backend-content-length.sh
index 90d890d02f..bb53f82c0c 100755
--- a/t/t5562-http-backend-content-length.sh
+++ b/t/t5562-http-backend-content-length.sh
@@ -25,6 +25,8 @@ test_http_env() {
handler_type="$1"
request_body="$2"
shift
+   (rm -f act.out || true) &&
+   (rm -f act.err || true) &&
env \
CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
QUERY_STRING="/repo.git/git-$handler_type-pack" \
@@ -155,6 +157,8 @@ test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
 '
 
 test_expect_success 'empty CONTENT_LENGTH' '
+   (rm -f act.out || true) &&
+   (rm -f act.err || true) &&
env \
QUERY_STRING="service=git-receive-pack" \
PATH_TRANSLATED="$PWD"/.git/info/refs \
-- 
2.19.0.1202.g68e1e8f04e



Re: [PATCH] t5562: do not reuse output files

2018-11-23 Thread Junio C Hamano
Max Kirillov  writes:

> diff --git a/t/t5562-http-backend-content-length.sh 
> b/t/t5562-http-backend-content-length.sh
> index 90d890d02f..bb53f82c0c 100755
> --- a/t/t5562-http-backend-content-length.sh
> +++ b/t/t5562-http-backend-content-length.sh
> @@ -25,6 +25,8 @@ test_http_env() {
>   handler_type="$1"
>   request_body="$2"
>   shift
> + (rm -f act.out || true) &&
> + (rm -f act.err || true) &&

Why "||true"?  If the named file doesn't exist, "rm -f" would
succeed, and if it does exist but somehow we fail to remove, then
these added lines are not preveting the next part from reusing,
i.e. they are not doing what they are supposed to be doing, so we
should detect such a failure (if happens) as an error, no?

IOW, shouldn't it just be more like

+   rm -f act.out act.err &&

The same comment applies to the other hunk.


>   env \
>   CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
>   QUERY_STRING="/repo.git/git-$handler_type-pack" \
> @@ -155,6 +157,8 @@ test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
>  '
>  
>  test_expect_success 'empty CONTENT_LENGTH' '
> + (rm -f act.out || true) &&
> + (rm -f act.err || true) &&
>   env \
>   QUERY_STRING="service=git-receive-pack" \
>   PATH_TRANSLATED="$PWD"/.git/info/refs \


Re: [PATCH] t5562: do not reuse output files

2018-11-23 Thread Junio C Hamano
Junio C Hamano  writes:

> Max Kirillov  writes:
>
>> diff --git a/t/t5562-http-backend-content-length.sh 
>> b/t/t5562-http-backend-content-length.sh
>> index 90d890d02f..bb53f82c0c 100755
>> --- a/t/t5562-http-backend-content-length.sh
>> +++ b/t/t5562-http-backend-content-length.sh
>> @@ -25,6 +25,8 @@ test_http_env() {
>>  handler_type="$1"
>>  request_body="$2"
>>  shift
>> +(rm -f act.out || true) &&
>> +(rm -f act.err || true) &&
>
> Why "||true"?  If the named file doesn't exist, "rm -f" would
> succeed, and if it does exist but somehow we fail to remove, then
> these added lines are not preveting the next part from reusing,
> i.e. they are not doing what they are supposed to be doing,...

Another thing.  The analysis in your log message talks about a stray
process holding open filehandles to these files.  An attempt to remove
them in such a stat would fail on some systems, so "||true" would not
help, would it?  It just hides the failure to remove, and when ||true
is useful in hiding th failure _is_ when such a stray process is still
there, waiting to corrupt the output of the next request and breaking
the test, no?

I do agree that forcing the parent to wait, like you described in
the comment, would be far more preferrable, but until that happens,
a better workaround might be to write into unique output filenames
(act1.out, act2.out, etc.); that way, you do not have to worry about
the output file for the next request getting clobbered by a stale
process handling the previous request.  But at the same time,
wouldn't this suggest that the test or the previous request may see
an incomplete output, as the analysed problem is that such a process
is writing to the output file very late, while we are preparing to
test the enxt request, which meas we have already checked the output
file for the previous request, right?  So even without ||true, I am
not sure how true a "solution" this change is to the issue.



[PATCH v2] t5562: do not reuse output files

2018-11-23 Thread Max Kirillov
Some expected failures of git-http-backend leaves running its children
(receive-pack or upload-pack) which still hold opened descriptors
to act.err and with some probability they live long enough to write
there their failure messages after next test has already truncated
the files. This causes occasional failures of the test script.

Avoid the issue by unlinking the older files before writing to them.

Reported-by: Carlo Arenas 
Helped-by: Carlo Arenas 
Helped-by: Junio C Hamano 
Signed-off-by: Max Kirillov 
---
Thanks. Updated
 t/t5562-http-backend-content-length.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t5562-http-backend-content-length.sh 
b/t/t5562-http-backend-content-length.sh
index 90d890d02f..3a9f7a14e2 100755
--- a/t/t5562-http-backend-content-length.sh
+++ b/t/t5562-http-backend-content-length.sh
@@ -25,6 +25,7 @@ test_http_env() {
handler_type="$1"
request_body="$2"
shift
+   rm -f act.out act.err &&
env \
CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
QUERY_STRING="/repo.git/git-$handler_type-pack" \
@@ -155,6 +156,7 @@ test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
 '
 
 test_expect_success 'empty CONTENT_LENGTH' '
+   rm -f act.out act.err &&
env \
QUERY_STRING="service=git-receive-pack" \
PATH_TRANSLATED="$PWD"/.git/info/refs \
-- 
2.19.0.1202.g68e1e8f04e



Re: [PATCH] t5562: do not reuse output files

2018-11-23 Thread Max Kirillov
On Sat, Nov 24, 2018 at 04:47:19PM +0900, Junio C Hamano wrote:
> Junio C Hamano  writes:
> a better workaround might be to write into unique output filenames
> (act1.out, act2.out, etc.); that way, you do not have to worry about
> the output file for the next request getting clobbered by a stale
> process handling the previous request.

Yes I agree

> But at the same time,
> wouldn't this suggest that the test or the previous request may see
> an incomplete output,

Yes it may miss the child's message. But in this case of failed
http-backend process, there should be already one "fatal:"
message in the act.err from the parent, and missing another
one does not change the outcome.