date:20171020

On Fri, 2017-10-13 at 14:11 +0900, Junio C Hamano wrote:
> 
> diff --git a/branch.c b/branch.c
> index 7404597678..2c3a364a0b 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -178,19 +178,31 @@ int read_branch_desc(struct strbuf *buf, const char 
> *branch_name)
>   return 0;
>  }
>  
> -int validate_new_branchname(const char *name, struct strbuf *ref,
> - int force, int attr_only)
> +/*
> + * Check if 'name' can be a valid name for a branch; die otherwise.
> + * Return 1 if the named branch already exists; return 0 otherwise.
> + * Fill ref with the full refname for the branch.
> + */
> +int validate_branchname(const char *name, struct strbuf *ref)
>  {
> - const char *head;
> -
>   if (strbuf_check_branch_ref(ref, name))
>   die(_("'%s' is not a valid branch name."), name);
>  
> - if (!ref_exists(ref->buf))
> - return 0;
> + return ref_exists(ref->buf);
> +}
>  
> - if (attr_only)
> - return 1;
> +/*
> + * Check if a branch 'name' can be created as a new branch; die otherwise.
> + * 'force' can be used when it is OK for the named branch already exists.
> + * Return 1 if the named branch already exists; return 0 otherwise.
> + * Fill ref with the full refname for the branch.
> + */

I guess it's better to avoid repeating the comments in both the .h and
.c file as they might quite easily become stale. I would prefer keeping
it in the header file alone.

> +int validate_new_branchname(const char *name, struct strbuf *ref, int force)
> +{
> + const char *head;
> +
> + if (!validate_branchname(name, ref))
> + return 0;
>  
>   if (!force)
>   die(_("A branch named '%s' already exists."),
> @@ -246,9 +258,9 @@ void create_branch(const char *name, const char 
> *start_name,
>   if (track == BRANCH_TRACK_EXPLICIT || track == BRANCH_TRACK_OVERRIDE)
>   explicit_tracking = 1;
>  
> - if (validate_new_branchname(name, , force,
> - track == BRANCH_TRACK_OVERRIDE ||
> - clobber_head)) {
> + if ((track == BRANCH_TRACK_OVERRIDE || clobber_head)
> + ? validate_branchname(name, )
> + : validate_new_branchname(name, , force)) {
>   if (!force)
>   dont_change_ref = 1;
> 

The change was simple by splitting the function into two and calling
the right function as required at every call site! As far as I could
see this doesn't seem to be reducing the confusion that the 'attr_only'
parameter caused. That's because the new validate_branchname function
seems to be called in some cases when the 'force' parameter is true and
in other cases the 'force' parameter is sent to the
'validate_new_branchname' function. So, I think consistency is lacking
in this change. That's just my opinion, of course.

-- 
Kaartic

Re: [PATCH 3/3] branch: forbid refs/heads/HEAD

Junio C Hamano  writes:
> 
> > Perhaps.  Also we may want to make sure that "git branch -D HEAD"
> > still works as a way to recover from historical mistakes.
> 
> The only difference is improved tests where we use show-ref to make
> sure refs/heads/HEAD does not exist when it shouldn't, exercise
> update-ref to create and delete refs/heads/HEAD, and also make sure
> it can be deleted with "git branch -d".
> 

In which case you might also like to ensure that it's possible to
"rename" the branch with a name "HEAD" to recover from historical
mistakes.

> diff --git a/sha1_name.c b/sha1_name.c
> index c7c5ab376c..1b8c503095 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -1332,7 +1332,7 @@ void strbuf_branchname(struct strbuf *sb, const char 
> *name, unsigned allowed)
>  int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
>  {
>   strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> - if (name[0] == '-')
> + if (*name == '-' || !strcmp(name, "HEAD"))
>   return -1;

I guess this makes the check for "HEAD" in builtin/branch::cmd_branch()
 (line 796) redundant. May be it could be removed?

>   strbuf_splice(sb, 0, 0, "refs/heads/", 11);
>   return check_refname_format(sb->buf, 0);
> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
> index e88349c8a0..4556aa66b8 100755
> --- a/t/t1430-bad-ref-name.sh
> +++ b/t/t1430-bad-ref-name.sh
> @@ -331,4 +331,27 @@ test_expect_success 'update-ref --stdin -z fails delete 
> with bad ref name' '
>   grep "fatal: invalid ref format: ~a" err
>  '
>  
> +test_expect_success 'branch rejects HEAD as a branch name' '
> + test_must_fail git branch HEAD HEAD^ &&
> + test_must_fail git show-ref refs/heads/HEAD
> +'
> +
> +test_expect_success 'checkout -b rejects HEAD as a branch name' '
> + test_must_fail git checkout -B HEAD HEAD^ &&
> + test_must_fail git show-ref refs/heads/HEAD
> +'
> +
> +test_expect_success 'update-ref can operate on refs/heads/HEAD' '
> + git update-ref refs/heads/HEAD HEAD^ &&
> + git show-ref refs/heads/HEAD &&
> + git update-ref -d refs/heads/HEAD &&
> + test_must_fail git show-ref refs/heads/HEAD
> +'
> +
> +test_expect_success 'branch -d can remove refs/heads/HEAD' '
> + git update-ref refs/heads/HEAD HEAD^ &&
> + git branch -d HEAD &&
> + test_must_fail git show-ref refs/heads/HEAD
> +'
> +
>  test_done

So, may be the following test could also be added (untested yet),

test_expect_success 'branch -m can rename refs/heads/HEAD' '
git update-ref refs/heads/HEAD HEAD^ &&
git branch -m HEAD head &&
test_must_fail git show-ref refs/heads/HEAD
'

-- 
Kaartic

Re: [PATCH 4/4] fsmonitor: Delay updating state until after split index is merged

On Fri, Oct 20, 2017 at 03:16:20PM +0200, Johannes Schindelin wrote:

> >  void tweak_fsmonitor(struct index_state *istate)
> >  {
> > +   int i;
> > +
> > +   if (istate->fsmonitor_dirty) {
> > +   /* Mark all entries valid */
> > +   trace_printf_key(_fsmonitor, "fsmonitor is enabled; cache 
> > is %d", istate->cache_nr);
> 
> Sadly, a call to trace_printf_key() is not really a noop when tracing is
> disabled. The call to trace_printf_key() hands off to trace_vprintf_fl(),
> which in turn calls prepare_trace_line() which asks trace_want() whether
> we need to trace, which finally calls get_trace_fd(). This last function
> initializes a trace key if needed, and this entire call stack takes time.

It seems like we could pretty easily turn noop traces into a trivial
conditional, like:

diff --git a/trace.h b/trace.h
index 179b249c59..c46b92cbde 100644
--- a/trace.h
+++ b/trace.h
@@ -80,8 +80,11 @@ extern void trace_performance_since(uint64_t start, const 
char *format, ...);
 #define trace_printf(...) \
trace_printf_key_fl(TRACE_CONTEXT, __LINE__, NULL, __VA_ARGS__)
 
-#define trace_printf_key(key, ...) \
-   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__)
+#define trace_printf_key(key, ...) do { \
+   if (!key->initialized || key->fd) \
+   trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, __VA_ARGS__) \
+} while(0)
+
 
 #define trace_argv_printf(argv, ...) \
trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, argv, __VA_ARGS__)


(OK, that's got an OR, but if we are really pinching instructions we
could obviously store a single "I've been initialized and am disabled"
flag).

I don't have an opinion one way or the other on these particular
messages, but in general I'd like to see _more_ tracing in Git, not
less. I've often traced Git with a debugger or other tools like perf,
but there's real value in the author of code annotating high-level
logical events.

-Peff

Re: hot to get file sizes in git log output

On Fri, Oct 20, 2017 at 07:38:00PM -0700, David Lang wrote:

> git whatchanged shows commits like:
> 
> commit fb7e54c12ddc7c87c4862806d583f5c6abf3e731
> Author: David Lang 
> Date:   Fri Oct 20 11:00:01 2017 -0700
> 
> update
> 
> :100644 100644 1a842ca... 290e9dd... M  Default/Bookmarks
> :100644 100644 1cd745c... 388a455... M  Default/Current Session
> :100644 100644 51074ad... c4dce40... M  Default/Current Tabs
> 
> If there was a way to add file size to this output, it would be perfect for
> what I'm needing.

There isn't. You'll have to process the output to pass the post-image
hashes to cat-file to get the size (i.e., what I showed before, though
of course you could make the output prettier if you wanted to).

-Peff

Re: [PATCH 2/3] t5615: avoid re-using descriptor 4

On Sat, Oct 21, 2017 at 02:19:16AM +0200, Simon Ruderich wrote:

> On Fri, Oct 20, 2017 at 06:46:08PM -0400, Jeff King wrote:
> >> I agree. Maybe just stick with the original patch?
> >
> > OK. Why don't we live with that for now, then. The only advantage of the
> > "999" trickery is that it's less likely to come up again. If it doesn't,
> > then we're happy. If it does, then we can always switch then.
> 
> I think switching the 4 to 9 (which you already brought up in
> this thread) is a good idea. It makes accidental conflicts less
> likely (it's rare to use so many file descriptors) and is easy to
> implement.

I'm not sure it does make accidental conflicts less likely. Grepping for
'9>' shows a problematic one in t0008, and one in t9300. That's two
versus the one for "4". :)

We often use descriptors 8 or 9 as "high and not taken for any specific
use" in our tests, and do things like:

  mkfifo foo
  exec 9>foo
  ...
  exec 9>&-

This is unlike a redirection in a sub-command (like "foo 9>bar") because
it effects the whole test suite's state. So it would break the test
under "-x" (because we'd get random cruft sent to the fifo), as well as
breaking the "-x" output itself (because we close the descriptor).

I actually think "7" is the safest descriptor right now. It's not used
for anything, and it's not high enough for tests to think "this probably
isn't used for anything".

-Peff

Re: [PATCH 4/3] t/Makefile: introduce TEST_SHELL_PATH

On Sat, Oct 21, 2017 at 01:50:01AM +0200, SZEDER Gábor wrote:

> > On Thu, Oct 19, 2017 at 05:01:40PM -0400, Jeff King wrote:
> > 
> > > I sometimes run git's test suite as part of an automated testing
> > > process. I was hoping to add "-x" support to get more details when a
> > > test fails (since failures are sometimes hard to reproduce).
> 
> Would it make sense (or be feasible) to enable "-x" on Travis CI?

Yes, after this series I think it may be workable to do so.

> > @@ -2350,6 +2357,7 @@ GIT-LDFLAGS: FORCE
> >  # and the first level quoting from the shell that runs "echo".
> >  GIT-BUILD-OPTIONS: FORCE
> > @echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@+
> > +   @echo TEST_SHELL_PATH=\''$(subst ','\'',$(TEST_SHELL_PATH_SQ))'\' >$@+
> 
> This redirection overwrites, loosing the just written SHELL_PATH.  It
> should append like the lines below.

Doh, thank you. It's interesting that nothing broke with this error. But
I think when run via Make that we end up with SHELL_PATH via the
environment (set to the default /bin/sh, which was suitable for my
system).

I'll fix it.

-Peff

Re: [PATCH 0/3] a small branch API clean-up

On Fri, 2017-10-13 at 14:11 +0900, Junio C Hamano wrote:
> This started as a little clean-up for a NEEDSWORK comment in
> branch.h, but it ended up adding a rather useful safety feature.
> 

Part of this series seems to duplicate the work done in part of my
series that tries to give more useful error messages when renaming a
branch.

https://public-inbox.org/git/%3c20170919071525.9404-1-kaarticsivaraam91...@gmail.com%3E/

Any reasons for this?

-- 
Kaartic

Re: [RFC PATCH v2 2/5] branch: re-order function arguments to group related arguments

On Fri, 2017-10-20 at 14:50 -0700, Stefan Beller wrote:
> On Mon, Sep 25, 2017 at 1:20 AM, Kaartic Sivaraam
>  wrote:
> > The ad-hoc patches to add new arguments to a function when needed
> > resulted in the related arguments not being close to each other.
> > This misleads the person reading the code to believe that there isn't
> > much relation between those arguments while it's not the case.
> > 
> > So, re-order the arguments to keep the related arguments close to each
> > other.
> 
> Thanks for taking a lot at the code quality in detail.
> 
> In my currently checked out version of Git, there are two occurrences
> of create_branch in builtin/branch.c, this patch converts only one occurrence.
> 
> (So you'd need to convert that second one, too. Oh wait; it is converted
> implicitly as the arguments are both '0':
> create_branch(branch->name, new_upstream, 0, 0, 0, \
> quiet, BRANCH_TRACK_OVERRIDE);
> )
> 
> This leads to the generic problem of this patch: Assume there are other
> contributors that write patches. They may introduce new calls to
> `create_branch`, but using the order of parameters as they may
> be unaware of this patch and they'd test without this patch.
> 
> As the signature of the function call doesn't change the compiler
> doesn't assist us in finding such a potential race between patches.
> 
> I am not aware of any other patches in flight, so we *might* be fine
> with this patch. But hope is rarely a good strategy.
> 
> Can we change the function signature (the name or another order
> of arguments that also makes sense, or making one of the
> parameters an enum) such that a potential race can be detected
> easier?
> 

I don't have a great interest in keeping this patch in case it might
conflict with other patches. Anyways, I guess we could avoid the issue
by making the last 'enum' parameter as the third parameter. It pretty
well changes the order by moving the flag-like parameters to the last
but it doesn't change the signature very strongly as you can pass
integers in the place of enums. (I guess that also obviates the
suggestion of making one parameter an enum)

So, the only way around is to rename the function which is something I
wouldn't like to do myself unless other people like the idea. So,
should I drop this patch or should I rename the function?


-- 
Kaartic

Re: hot to get file sizes in git log output

2017-10-20 Thread David Lang


On Fri, 20 Oct 2017, David Lang wrote:


On Fri, 20 Oct 2017, Eric Sunshine wrote:


I'm not exactly sure what you mean by size, but if you want to show
how many lines were added and removed by a given commit for each file,
you can use the "--stat" option to produce a diffstat. The "size" of
the files in each commit isn't very meaningful to the commit itself,
but a stat of how much was removed might be more accurate to what
you're looking for.


That's a good suggestion, and hopefully could help David answer his
original question.

I took the request to mean "walk through history, and for each file that
a commit touches, show its size". Which is a bit harder to do, and I
think you need to script a little:


David's mention of "a particular file", suggests to me that something
"bad" happened to one file, and he wants to know in which commit that
"badness" happened. If so, then it sounds like a job for git-bisect.


summarizing (and removing the long explination of why I'm doing this)

for each file (or each file changed in the commit), what is the byte count of 
that file at the time of that commit.


git whatschanged currently reports

commit 17be1c1e1f80086e8ddda1706c8c8d6cf80d26b7
Author: David Lang 
Date:   Thu Oct 19 22:00:01 2017 -0700

update

:100644 100644 bb9dcd3... 8635d2b... M  Default/Current Session

commit d3f94d406e0d5c6ee7b6f6bcea019adff438127c
Author: David Lang 
Date:   Thu Oct 19 21:00:01 2017 -0700

update

:100644 100644 88ece53... bb9dcd3... M  Default/Current Session

commit fea290bd235a444bbd4bc4430fa0844501ae2b8c
Author: David Lang 
Date:   Thu Oct 19 06:00:01 2017 -0700

update

:100644 100644 ff04089... 88ece53... M  Default/Current Session

what is the size of the file "Current Session" for each commit?

David Lang

Re: hot to get file sizes in git log output

2017-10-20 Thread David Lang


On Fri, 20 Oct 2017, Eric Sunshine wrote:


I'm not exactly sure what you mean by size, but if you want to show
how many lines were added and removed by a given commit for each file,
you can use the "--stat" option to produce a diffstat. The "size" of
the files in each commit isn't very meaningful to the commit itself,
but a stat of how much was removed might be more accurate to what
you're looking for.


That's a good suggestion, and hopefully could help David answer his
original question.

I took the request to mean "walk through history, and for each file that
a commit touches, show its size". Which is a bit harder to do, and I
think you need to script a little:


David's mention of "a particular file", suggests to me that something
"bad" happened to one file, and he wants to know in which commit that
"badness" happened. If so, then it sounds like a job for git-bisect.


In this case, I have git store a copy of the state file for chromium (and do a 
similar thing for firefox), so that if something bad happens and it crashes and 
looses all 200-400 tabs in a way that it's recovery doesn't work, I can go back 
to a prior version.


This is done by having the following crontab entries, along with smudge filters 
that change the one-line json to pretty printed json before the commit.


0 * * * * dlang cd /home/dlang/.config/chromium/Default; git add *Session *Tabs Bookmarks 
History ; git commit -mupdate > /dev/null 2>&1

0 0 3 * * dlang cd /home/dlang/.config/chromium/Default; git gc --aggressive > 
/dev/null 2>&1

0 * * * * dlang cd /home/dlang/.mozilla/firefox/bux6mwl1.default/sessionstore-backups; 
git add *.js ; git commit -mupdate > /dev/null 2>&1

0 0 3 * * dlang cd /home/dlang/.mozilla/firefox/bux6mwl1.default/sessionstore-backups; 
git gc --aggressive > /dev/null 2>&1

This has saved me many times in the past. But this time I didn't recognize when 
the problem happened because instead of a crash, it just closed all the tabs 
except the one that was open. Once I realized all my other tabs were gone, I 
didn't have time to mess with it for a few days. So the problem could have 
happened anytime in the last week or two.


I'm sure that when this happened, the files shrunk drastically (from several 
hundred tabs to a dozen or so will be very obvious).


But I don't have any specific line I can look at, the lines that are there 
change pretty regularly, and/or would not have changed at the transition.


git whatchanged shows commits like:

commit fb7e54c12ddc7c87c4862806d583f5c6abf3e731
Author: David Lang 
Date:   Fri Oct 20 11:00:01 2017 -0700

update

:100644 100644 1a842ca... 290e9dd... M  Default/Bookmarks
:100644 100644 1cd745c... 388a455... M  Default/Current Session
:100644 100644 51074ad... c4dce40... M  Default/Current Tabs

If there was a way to add file size to this output, it would be perfect for what 
I'm needing.


David Lang

Re: [RFC PATCH v2 1/5] branch: improve documentation and naming of certain parameters

On Fri, 2017-10-20 at 17:51 -0400, Eric Sunshine wrote:
> On Mon, Sep 25, 2017 at 1:20 AM, Kaartic Sivaraam
>  wrote:
> > Documentation for a certain function was incomplete as it didn't say
> > what certain parameters were used for. Further a parameter name wasn't
> > very communicative.
> > 
> > So, add missing documentation for the sake of completeness and easy
> > reference. Also, rename the concerned parameter to make it's name more
> 
> s/it's/its/
> 

Thanks!

-- 
Kaartic

Re: [RFC PATCH v2 1/5] branch: improve documentation and naming of certain parameters

On Fri, 2017-10-20 at 14:33 -0700, Stefan Beller wrote:
> Up to here ( including the subject line), I have no idea you're talking about
> 'create_branch'. Maybe
> 

That's because I didn't want to be explicit.

> branch: improve documentation and naming of parameters for create_branch
> 
> The documentation for 'create_branch' was incomplete ...
> 

Sounds good, will use it.

-- 
Kaartic

Re: Git for Windows v2.15.0-rc prerelease, was Re: [ANNOUNCE] Git v2.15.0-rc2

Bryan Turner  writes:

>> The Git for Windows equivalent is now available from
>>
>> 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_git-2Dfor-2Dwindows_git_releases_tag_v2.15.0-2Drc2.windows.1=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=3DdPEJGQe01zvIuHjX8rNURKuGEY_cPkUXvnur9xlNg=ZC45D6NoNiE4A8qs_F1ZDMJlGMdXcQ9DwDIpE1-whrU=
>>
>> Please test at your convenience,
>
> Thanks for publishing this, Johannes. I've run it through our Windows
> test matrix for Bitbucket Server (~1450 tests) and all pass. I've also
> added it to our CI build as a canary (pending the final 2.15.0
> release). I've done the same for 2.15.0-rc2 on Linux as well.

Thanks, both of you.

Re: [PATCH 4/4] fsmonitor: Delay updating state until after split index is merged

Ben Peart  writes:

>>> +   } else {
>>> +   trace_printf_key(_fsmonitor, "fsmonitor not enabled");
>>> +   }
>>> +
>
> I'd remove the trace statement above as it isn't always
> accurate. fsmonitor could be enabled but just hasn't written/read the
> extension yet.

I agree; when it is not enabled, we shouldn't be paying the penalty,
either.  I wonder if tweak_*() function can return early upfront if
we know fsmonitor is not enabled to make it even more obvious.

>>> +   if (ignore_fsmonitor)
>>> +   trace_printf_key(_fsmonitor, "Ignoring fsmonitor for %s", 
>>> ce->name);
>>
>> This is the code path I am fairly certain should not be penalized if
>> tracing is disabled.
>
> Definitely agree with the need to remove this tracing as it will get
> called a lot and we don't want to pay the perf penalty.

Yes.

Re: [PATCH 2/3] t5615: avoid re-using descriptor 4

Simon Ruderich  writes:

> On Fri, Oct 20, 2017 at 06:46:08PM -0400, Jeff King wrote:
>>> I agree. Maybe just stick with the original patch?
>>
>> OK. Why don't we live with that for now, then. The only advantage of the
>> "999" trickery is that it's less likely to come up again. If it doesn't,
>> then we're happy. If it does, then we can always switch then.
>
> I think switching the 4 to 9 (which you already brought up in
> this thread) is a good idea. It makes accidental conflicts less
> likely (it's rare to use so many file descriptors) and is easy to
> implement.

Yeah, I like the simplicity of implementation, and I more like the
fact that it is simpler to reason about its limitation.

Re: [PATCH v1] read_index_from(): Skip verification of the cache entry order to speed index loading

Stefan Beller  writes:

> There was a recent thread (which I assumed was the one I linked), that talked
> about security implications as soon as we loose the rather strict "git
> is to be used
> in a posix world", e.g. sharing your repo over NFS/Dropbox. The
> specific question
> that Peff asked was how the internal formats can be exploited. (Can a 
> malicious
> index file be crafted such that it is not just a segfault, but a
> 'remote' code execution,
> given that you deploy the maliciously crafted file via NFS. Removing checks 
> that
> we already have made me a bit suspicious that it *may* be helping an
> attacker here,
> though I have no hard data to show)
>
> Sorry for the confusion,

Thanks for an explanation, as I had the same reaction as Dscho
initially.  I'd assumed that the worst would be to create a wrong
state (e.g. the same path registered twice with different contents
in the index, a malformed tree written out of it, etc.), but that's
merely an assumption not the result of an audit.

Re: [PATCH] builtin/push.c: add push.pushOption config

Stefan Beller  writes:

>>> So in the config, we have to explicitly give an empty option to
>>> clear the previous options, but on the command line we do not need
>>> that, but instead we'd have to repeat any push options that we desire
>>> that were configured?
>>
>> It is not wrong per-se to phrase it like so, but I think that is
>> making it unnecessarily confusing by conflating two things.  (1)
>> configured values are overridden from the command line, just like
>> any other --option/config.variable pair
>
> because they are single value options (usually).
>
>> and (2) unlike usual single
>> value variables where "last one wins" rule is simple enough to
>> explain,, multi-value variables need a way to "forget everything we
>> said so far and start from scratch" syntax, especially when multiple
>> input files are involved.
>
> ok, my view of how that should be done is clashing once again
> with the projects established standards. Sorry for the noise.

I do not think it is a noise.  I was primarily focusing on "have to
explicitly" part, making it sound as if it was a flaw.  I do think
it is a good idea to explain a variable and/or an option is
multi-valued and how multiple instances of them interact with each
other.  I think the status quo is:

Both configuration and command line, these multi-valued
things accumulate.  The "command line can be used to
override things from the configuration" rule applies as any
other config/option pair.

In addition, in the configuration, there is a mechanism to
clear the values read so far with the "an empty value
clears" rule, because you may not have control over, or it
may be cumbersome to modify, lower-priority configuration
files.  There is no such thing for command line, as the
entirety of the command line for each invocation is under
your control.

If somebody has

[alias] mypush = push -ofoo

then the command line argument for one invocation of "git mypush"
may *not* be under your control and it might be also convenient if
there were a mechanism to clear what has been accumulated from the
command line.  It may be worth considering, but if we were going in
that direction, I suspect that it is probably a good idea to first
step back a bit and introduce a mechanism to easily define pairs of
option/config in a more sysmtematic way [*1*].  Once we have such a
mechanism, the "clear the previous" syntax for the command line
would be implemented uniformly in there.

[Footnote]

*1* E.g. right now, the fact that "push --push-option" and
"push.pushOption" are related, or that "status -u" and
"status.showUntrackedFiles" are related, is only known to the
code and the documentation; I am not sure if it is even a good
idea to add config to each and every option that exists, but for
the ones that do exist, it would be nicer if there were a more
uniform and systematic way for parse-options.c and config.c APIs
to help our code, instead of writing git_config() callback and
options[] array to give to parse_options(), making sure they
refer to the same internal variable, and the latter overrides
the former.

Re: [PATCH] builtin/push.c: add push.pushOption config

Marius Paliga  writes:

> Should we also mention that this option can take multiple values from
> the command line?
>
> -o ::
> --push-option=::
> Transmit the given string to the server, which passes them to
> the pre-receive as well as the post-receive hook. The given string
> must not contain a NUL or LF character.
> +   Multiple `--push-option=` are allowed on the command line.
>When no `--push-option=` is given from the command
> line, the values of configuration variable `push.pushOption`
> are used instead.

My first reaction was "Do we do that for other options that can be
given multiple times?  If not, perhaps this is not needed."

Then my second reaction was "Do we have that many options that can
be given multiple times in the first place?  If not, perhaps a
change like this to highlight these oddball options may not be a bad
idea."

And my third reaction was "Well, even if we have many such options,
and even if most of them are not explicitly marked as usable
multiple times in the current documentation, that's not a reason to
keep it that way---the readers ought to be able to find out which
ones can be used multiple times and how these multiple instances
interact with each other, because the usual rule for single-instance
options is 'the last one wins' (e.g. 'git status -uall -uno' should
be the same as 'git status -uno') but to these multi-value options
that rule does not hold".

So, yes, I think it is a good idea.

But it opens a tangent #leftoverbits.  Among the most commonly used
commands listed in "git --help", there indeed are some commands that
take multiple options of the same kind (even if we do not count an
obvious exception "--verbose", which everybody understands as the
number of times it is given indicates the verbosity level).
Somebody needs to go over their documentation and add "this can be
given multiple times from the command line, and here is what happens
to them".

In your suggestion for "push -o  -o ", "here is what
happens" is missing.  Perhaps

When multiple `--push-option=` are given, they are
all sent to the other side in the order listed on the
command line.

or something like that.

Thanks.

Re: [PATCH 2/3] t5615: avoid re-using descriptor 4

2017-10-20 Thread Simon Ruderich

On Fri, Oct 20, 2017 at 06:46:08PM -0400, Jeff King wrote:
>> I agree. Maybe just stick with the original patch?
>
> OK. Why don't we live with that for now, then. The only advantage of the
> "999" trickery is that it's less likely to come up again. If it doesn't,
> then we're happy. If it does, then we can always switch then.

I think switching the 4 to 9 (which you already brought up in
this thread) is a good idea. It makes accidental conflicts less
likely (it's rare to use so many file descriptors) and is easy to
implement.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [PATCH 4/3] t/Makefile: introduce TEST_SHELL_PATH

2017-10-20 Thread SZEDER Gábor


> On Thu, Oct 19, 2017 at 05:01:40PM -0400, Jeff King wrote:
> 
> > I sometimes run git's test suite as part of an automated testing
> > process. I was hoping to add "-x" support to get more details when a
> > test fails (since failures are sometimes hard to reproduce).

Would it make sense (or be feasible) to enable "-x" on Travis CI?


> diff --git a/Makefile b/Makefile
> index cd75985991..9baa3c4b50 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -425,6 +425,10 @@ all::
>  #
>  # to say "export LESS=FRX (and LV=-c) if the environment variable
>  # LESS (and LV) is not set, respectively".
> +#
> +# Define TEST_SHELL_PATH if you want to use a shell besides SHELL_PATH for
> +# running the test scripts (e.g., bash has better support for "set -x"
> +# tracing).
>  
>  GIT-VERSION-FILE: FORCE
>   @$(SHELL_PATH) ./GIT-VERSION-GEN
> @@ -727,6 +731,8 @@ endif
>  export PERL_PATH
>  export PYTHON_PATH
>  
> +TEST_SHELL_PATH = $(SHELL_PATH)
> +
>  LIB_FILE = libgit.a
>  XDIFF_LIB = xdiff/lib.a
>  VCSSVN_LIB = vcs-svn/lib.a
> @@ -1721,6 +1727,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
>  gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
>  
>  SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> +TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
>  PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
>  PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
>  TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
> @@ -2350,6 +2357,7 @@ GIT-LDFLAGS: FORCE
>  # and the first level quoting from the shell that runs "echo".
>  GIT-BUILD-OPTIONS: FORCE
>   @echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@+
> + @echo TEST_SHELL_PATH=\''$(subst ','\'',$(TEST_SHELL_PATH_SQ))'\' >$@+

This redirection overwrites, loosing the just written SHELL_PATH.  It
should append like the lines below.

>   @echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >>$@+
>   @echo DIFF=\''$(subst ','\'',$(subst ','\'',$(DIFF)))'\' >>$@+
>   @echo PYTHON_PATH=\''$(subst ','\'',$(PYTHON_PATH_SQ))'\' >>$@+


Gábor

Re: Git for Windows v2.15.0-rc prerelease, was Re: [ANNOUNCE] Git v2.15.0-rc2

2017-10-20 Thread Bryan Turner

On Fri, Oct 20, 2017 at 3:22 PM, Johannes Schindelin
 wrote:
> Hi team,
>
> [cutting linux-kernel]
>
> On Fri, 20 Oct 2017, Junio C Hamano wrote:
>
>> A release candidate Git v2.15.0-rc2 is now available for testing
>> at the usual places.
>
> The Git for Windows equivalent is now available from
>
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_git-2Dfor-2Dwindows_git_releases_tag_v2.15.0-2Drc2.windows.1=DwIBAg=wBUwXtM9sKhff6UeHOQgvw=uBedA6EFFVX1HiLgmpdrBrv8bIDAScKjk1yk9LOASBM=3DdPEJGQe01zvIuHjX8rNURKuGEY_cPkUXvnur9xlNg=ZC45D6NoNiE4A8qs_F1ZDMJlGMdXcQ9DwDIpE1-whrU=
>
> Please test at your convenience,

Thanks for publishing this, Johannes. I've run it through our Windows
test matrix for Bitbucket Server (~1450 tests) and all pass. I've also
added it to our CI build as a canary (pending the final 2.15.0
release). I've done the same for 2.15.0-rc2 on Linux as well.

Best regards,
Bryan Turner

I need your cooperation

2017-10-20 Thread Ahmed Hassan

Dear Friend,

I know that this mail will come to you as a surprise as we have never
met before, but need not to worry as I am contacting you independently
of my investigation and no one is informed of this communication. I
need your urgent assistance in transferring the sum of $11.3million
immediately to your private account.The money has been here in our
Bank lying dormant for years now without anybody coming for the claim
of it.

I want to release the money to you as the relative to our deceased
customer (the account owner) who died a long with his supposed NEXT OF
KIN since 16th October 2005. The Banking laws here does not allow such
money to stay more than 12 years, because the money will be recalled
to the Bank treasury account as unclaimed fund.

By indicating your interest I will send you the full details on how
the business will be executed.

Please respond urgently and delete if you are not interested.

Best Regards,
Mr. Ahmed Hassan.

[PATCH 4/3] t/Makefile: introduce TEST_SHELL_PATH

On Thu, Oct 19, 2017 at 05:01:40PM -0400, Jeff King wrote:

> I sometimes run git's test suite as part of an automated testing
> process. I was hoping to add "-x" support to get more details when a
> test fails (since failures are sometimes hard to reproduce). But I hit a
> few small snags:
> 
>   - you have to run with bash, since BASH_XTRACEFD is required to avoid
> failures in some tests when we capture the stderr of shell functions
> or subshells (which get polluted with the "set -x" outupt). This
> requirement isn't a big deal for me, but it showed some other
> issues.

Actually, this did lead me to one more fix.

I was building with SHELL_PATH=/bin/bash to make BASH_XTRACEFD work. But
that impacts not only the test scripts, but also the build itself.
I'd prefer to test something closer to my normal builds (which use
bin/sh). This patch lets me do:

  make \
TEST_SHELL_PATH=/bin/bash \
GIT_TEST_OPTS="--verbose-log -x" \
test

to run the whole test suite using /bin/sh for the build, for getting the
benefits of bash's "-x" tracing.

-- >8 --
Subject: [PATCH] t/Makefile: introduce TEST_SHELL_PATH

You may want to run the test suite with a different shell
than you use to build Git. For instance, you may build with
SHELL_PATH=/bin/sh (because it's faster, or it's what you
expect to exist on systems where the build will be used) but
want to run the test suite with bash (e.g., since that
allows using "-x" reliably across the whole test suite).
There's currently no good way to do this.

You might think that doing two separate make invocations,
like:

  make &&
  make -C t SHELL_PATH=/bin/bash

would work. And it _almost_ does. The second make will see
our bash SHELL_PATH, and we'll use that to run the
individual test scripts (or tell prove to use it to do so).
So far so good.

But this breaks down when "--tee" or "--verbose-log" is
used. Those options cause the test script to actually
re-exec itself using $SHELL_PATH. But wait, wouldn't our
second make invocation have set SHELL_PATH correctly in the
environment?

Yes, but test-lib.sh sources GIT-BUILD-OPTIONS, which we
built during the first "make". And that overrides the
environment, giving us the original SHELL_PATH again.

Let's introduce a new variable that lets you specify a
specific shell to be run for the test scripts. Note that we
have to touch both the main and t/ Makefiles, since we have
to record it in GIT-BUILD-OPTIONS in one, and use it in the
latter.

Signed-off-by: Jeff King 
---
 Makefile  | 8 
 t/Makefile| 6 --
 t/test-lib.sh | 2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index cd75985991..9baa3c4b50 100644
--- a/Makefile
+++ b/Makefile
@@ -425,6 +425,10 @@ all::
 #
 # to say "export LESS=FRX (and LV=-c) if the environment variable
 # LESS (and LV) is not set, respectively".
+#
+# Define TEST_SHELL_PATH if you want to use a shell besides SHELL_PATH for
+# running the test scripts (e.g., bash has better support for "set -x"
+# tracing).

 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -727,6 +731,8 @@ endif
 export PERL_PATH
 export PYTHON_PATH

+TEST_SHELL_PATH = $(SHELL_PATH)
+
 LIB_FILE = libgit.a
 XDIFF_LIB = xdiff/lib.a
 VCSSVN_LIB = vcs-svn/lib.a
@@ -1721,6 +1727,7 @@ prefix_SQ = $(subst ','\'',$(prefix))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))

 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 PYTHON_PATH_SQ = $(subst ','\'',$(PYTHON_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
@@ -2350,6 +2357,7 @@ GIT-LDFLAGS: FORCE
 # and the first level quoting from the shell that runs "echo".
 GIT-BUILD-OPTIONS: FORCE
@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@+
+   @echo TEST_SHELL_PATH=\''$(subst ','\'',$(TEST_SHELL_PATH_SQ))'\' >$@+
@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >>$@+
@echo DIFF=\''$(subst ','\'',$(subst ','\'',$(DIFF)))'\' >>$@+
@echo PYTHON_PATH=\''$(subst ','\'',$(PYTHON_PATH_SQ))'\' >>$@+
diff --git a/t/Makefile b/t/Makefile
index 1bb06c36f2..96317a35f4 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -8,6 +8,7 @@

 #GIT_TEST_OPTS = --verbose --debug
 SHELL_PATH ?= $(SHELL)
+TEST_SHELL_PATH ?= $(SHELL_PATH)
 PERL_PATH ?= /usr/bin/perl
 TAR ?= $(TAR)
 RM ?= rm -f
@@ -23,6 +24,7 @@ endif

 # Shell quote;
 SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))

@@ -42,11 +44,11 @@ failed:
test -z "$$failed" || $(MAKE) $$failed

 prove: pre-clean $(TEST_LINT)
-   @echo "*** prove ***"; $(PROVE) --exec '$(SHELL_PATH_SQ)' 
$(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+   @echo "*** prove ***"; $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' 
$(GIT_PROVE_OPTS) $(T)

Re: [PATCH 2/3] t5615: avoid re-using descriptor 4

On Fri, Oct 20, 2017 at 02:27:40PM -0700, Stefan Beller wrote:

> > So I dunno. It does solve the problem in a way that the individual test
> > scripts wouldn't have to care about. But it's a lot of eval trickery.
> 
> I agree. Maybe just stick with the original patch?

OK. Why don't we live with that for now, then. The only advantage of the
"999" trickery is that it's less likely to come up again. If it doesn't,
then we're happy. If it does, then we can always switch then.

I also noticed that our GIT_TRACE=4 trickery has the same problem (it
didn't trigger in the t5615 case because it doesn't actually run git
inside the subshell). If we end up doing a "999" descriptor eventually,
we should probably point GIT_TRACE at it, too.

-Peff

Re: [PATCH 4/5] diff: fix whitespace-skipping with --color-moved

On Fri, Oct 20, 2017 at 09:23:54AM +0200, Simon Ruderich wrote:

> On Thu, Oct 19, 2017 at 04:29:26PM -0400, Jeff King wrote:
> > [snip]
> >
> > --- a/t/t4015-diff-whitespace.sh
> > +++ b/t/t4015-diff-whitespace.sh
> > @@ -1463,6 +1463,73 @@ test_expect_success 'move detection ignoring 
> > whitespace changes' '
> > test_cmp expected actual
> >  '
> >
> > +test_expect_failure 'move detection ignoring whitespace at eol' '
> 
> Shouldn't this be test_expect_success? According to the commit
> message "and a new "--ignore-space-at-eol" shows off the breakage
> we're fixing.". I didn't actually run the code so I don't know if
> the test fails or not.

Thanks, good catch. I had originally added all the tests in a single
commit, with the intent of flipping this failure to success. But when I
shifted it to just get added here, I accidentally lost that flip.

-Peff

Git for Windows v2.15.0-rc prerelease, was Re: [ANNOUNCE] Git v2.15.0-rc2

Hi team,

[cutting linux-kernel]

On Fri, 20 Oct 2017, Junio C Hamano wrote:

> A release candidate Git v2.15.0-rc2 is now available for testing
> at the usual places.

The Git for Windows equivalent is now available from

https://github.com/git-for-windows/git/releases/tag/v2.15.0-rc2.windows.1

Please test at your convenience,
Johannes

Re: hot to get file sizes in git log output

2017-10-20 Thread Jacob Keller

On Fri, Oct 20, 2017 at 2:50 PM, Eric Sunshine  wrote:
> On Fri, Oct 20, 2017 at 5:43 PM, Jeff King  wrote:
>> On Fri, Oct 20, 2017 at 01:44:36PM -0700, Jacob Keller wrote:
>>> On Fri, Oct 20, 2017 at 11:12 AM, David Lang  wrote:
>>> > I'm needing to scan through git history looking for the file sizes 
>>> > (looking
>>> > for when a particular file shrunk drastically)
>>> >
>>> > I'm not seeing an option in git log or git whatchanged that gives me the
>>> > file size, am I overlooking something?
>>>
>>> I'm not exactly sure what you mean by size, but if you want to show
>>> how many lines were added and removed by a given commit for each file,
>>> you can use the "--stat" option to produce a diffstat. The "size" of
>>> the files in each commit isn't very meaningful to the commit itself,
>>> but a stat of how much was removed might be more accurate to what
>>> you're looking for.
>>
>> That's a good suggestion, and hopefully could help David answer his
>> original question.
>>
>> I took the request to mean "walk through history, and for each file that
>> a commit touches, show its size". Which is a bit harder to do, and I
>> think you need to script a little:
>
> David's mention of "a particular file", suggests to me that something
> "bad" happened to one file, and he wants to know in which commit that
> "badness" happened. If so, then it sounds like a job for git-bisect.

Yea, if you have a simple script which can tell when the file is
"bad", you could run it through git bisect run pretty easily and
rapidly find the right answer.

Thanks,
Jake

Re: hot to get file sizes in git log output

2017-10-20 Thread Eric Sunshine

On Fri, Oct 20, 2017 at 5:43 PM, Jeff King  wrote:
> On Fri, Oct 20, 2017 at 01:44:36PM -0700, Jacob Keller wrote:
>> On Fri, Oct 20, 2017 at 11:12 AM, David Lang  wrote:
>> > I'm needing to scan through git history looking for the file sizes (looking
>> > for when a particular file shrunk drastically)
>> >
>> > I'm not seeing an option in git log or git whatchanged that gives me the
>> > file size, am I overlooking something?
>>
>> I'm not exactly sure what you mean by size, but if you want to show
>> how many lines were added and removed by a given commit for each file,
>> you can use the "--stat" option to produce a diffstat. The "size" of
>> the files in each commit isn't very meaningful to the commit itself,
>> but a stat of how much was removed might be more accurate to what
>> you're looking for.
>
> That's a good suggestion, and hopefully could help David answer his
> original question.
>
> I took the request to mean "walk through history, and for each file that
> a commit touches, show its size". Which is a bit harder to do, and I
> think you need to script a little:

David's mention of "a particular file", suggests to me that something
"bad" happened to one file, and he wants to know in which commit that
"badness" happened. If so, then it sounds like a job for git-bisect.

Re: [RFC PATCH v2 1/5] branch: improve documentation and naming of certain parameters

2017-10-20 Thread Eric Sunshine

On Mon, Sep 25, 2017 at 1:20 AM, Kaartic Sivaraam
 wrote:
> Documentation for a certain function was incomplete as it didn't say
> what certain parameters were used for. Further a parameter name wasn't
> very communicative.
>
> So, add missing documentation for the sake of completeness and easy
> reference. Also, rename the concerned parameter to make it's name more

s/it's/its/

> communicative.
>
> Signed-off-by: Kaartic Sivaraam

Re: [RFC PATCH v2 2/5] branch: re-order function arguments to group related arguments

On Mon, Sep 25, 2017 at 1:20 AM, Kaartic Sivaraam
 wrote:
> The ad-hoc patches to add new arguments to a function when needed
> resulted in the related arguments not being close to each other.
> This misleads the person reading the code to believe that there isn't
> much relation between those arguments while it's not the case.
>
> So, re-order the arguments to keep the related arguments close to each
> other.

Thanks for taking a lot at the code quality in detail.

In my currently checked out version of Git, there are two occurrences
of create_branch in builtin/branch.c, this patch converts only one occurrence.

(So you'd need to convert that second one, too. Oh wait; it is converted
implicitly as the arguments are both '0':
create_branch(branch->name, new_upstream, 0, 0, 0, \
quiet, BRANCH_TRACK_OVERRIDE);
)

This leads to the generic problem of this patch: Assume there are other
contributors that write patches. They may introduce new calls to
`create_branch`, but using the order of parameters as they may
be unaware of this patch and they'd test without this patch.

As the signature of the function call doesn't change the compiler
doesn't assist us in finding such a potential race between patches.

I am not aware of any other patches in flight, so we *might* be fine
with this patch. But hope is rarely a good strategy.

Can we change the function signature (the name or another order
of arguments that also makes sense, or making one of the
parameters an enum) such that a potential race can be detected
easier?

Thanks,
Stefan

Re: [PATCH 4/4] fsmonitor: Delay updating state until after split index is merged

2017-10-20 Thread Ben Peart




On 10/20/2017 9:16 AM, Johannes Schindelin wrote:

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:


  extern struct index_state the_index;
diff --git a/fsmonitor.c b/fsmonitor.c
index 7c1540c05..4c2668f57 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, 
const void *data,
ewah_free(fsmonitor_dirty);
return error("failed to parse ewah bitmap reading fsmonitor index 
extension");
}
-
-   if (git_config_get_fsmonitor()) {
-   /* Mark all entries valid */
-   for (i = 0; i < istate->cache_nr; i++)
-   istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
-
-   /* Mark all previously saved entries as dirty */
-   ewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);
-
-   /* Now mark the untracked cache for fsmonitor usage */
-   if (istate->untracked)
-   istate->untracked->use_fsmonitor = 1;
-   }
-   ewah_free(fsmonitor_dirty);
+   istate->fsmonitor_dirty = fsmonitor_dirty;


 From the diff, it is not immediately clear that fsmonitor_dirty is not
leaked in any code path.

Could you clarify this in the commit message, please?


@@ -238,6 +225,29 @@ void remove_fsmonitor(struct index_state *istate)
  
  void tweak_fsmonitor(struct index_state *istate)

  {
+   int i;
+


Here we should test git_config_get_fsmonitor() (see the logic where you 
moved this from) as there is no reason to set up the fsmonitor state if 
we're about to remove the extension.  Save the results and use them in 
the case statement below.



+   if (istate->fsmonitor_dirty) {
+   /* Mark all entries valid */
+   trace_printf_key(_fsmonitor, "fsmonitor is enabled; cache is 
%d", istate->cache_nr);


Sadly, a call to trace_printf_key() is not really a noop when tracing is
disabled. The call to trace_printf_key() hands off to trace_vprintf_fl(),
which in turn calls prepare_trace_line() which asks trace_want() whether
we need to trace, which finally calls get_trace_fd(). This last function
initializes a trace key if needed, and this entire call stack takes time.

In this case, where we trace whether fsmonitor is enabled essentially once
during the life cycle of the current Git command invocation, it may be
okay, but later we perform a trace for every single ie_match_stat() call,
which I think should be guarded to avoid unnecessary code churn in
performance critical code paths (O(N) is pretty good until the constant
factor becomes large).


+   for (i = 0; i < istate->cache_nr; i++) {
+   istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
+   }
+   trace_printf_key(_fsmonitor, "marked all as valid");
+
+   /* Mark all previously saved entries as dirty */
+   trace_printf_key(_fsmonitor, "setting each bit on %p", 
istate->fsmonitor_dirty);
+   ewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, 
istate);
+
+   /* Now mark the untracked cache for fsmonitor usage */
+   trace_printf_key(_fsmonitor, "setting untracked state");
+   if (istate->untracked)
+   istate->untracked->use_fsmonitor = 1;
+   ewah_free(istate->fsmonitor_dirty);


At this point, please set istate->fsmonitor_dirty = NULL, as it is not
immediately obvious from this patch (or from the postimage of this diff)
that the array is not used later on.


+   } else {
+   trace_printf_key(_fsmonitor, "fsmonitor not enabled");
+   }
+


I'd remove the trace statement above as it isn't always accurate. 
fsmonitor could be enabled but just hasn't written/read the extension yet.



switch (git_config_get_fsmonitor()) {
case -1: /* keep: do nothing */
break;
diff --git a/read-cache.c b/read-cache.c
index c18e5e623..3b5cd00a2 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -330,6 +330,10 @@ int ie_match_stat(struct index_state *istate,
return 0;
if (!ignore_fsmonitor && (ce->ce_flags & CE_FSMONITOR_VALID))
return 0;
+   if (ce->ce_flags & CE_FSMONITOR_VALID)
+   trace_printf_key(_fsmonitor, "fsmon marked valid for %s", 
ce->name);
+   if (ignore_fsmonitor)
+   trace_printf_key(_fsmonitor, "Ignoring fsmonitor for %s", 
ce->name);


This is the code path I am fairly certain should not be penalized if
tracing is disabled.



Definitely agree with the need to remove this tracing as it will get 
called a lot and we don't want to pay the perf penalty.



Ciao,
Johannes



Thank you for detecting the issue with split index and even better, 
sending patches to fix it!

Re: hot to get file sizes in git log output

On Fri, Oct 20, 2017 at 01:44:36PM -0700, Jacob Keller wrote:

> On Fri, Oct 20, 2017 at 11:12 AM, David Lang  wrote:
> > I'm needing to scan through git history looking for the file sizes (looking
> > for when a particular file shrunk drastically)
> >
> > I'm not seeing an option in git log or git whatchanged that gives me the
> > file size, am I overlooking something?
> >
> > David Lang
> 
> I'm not exactly sure what you mean by size, but if you want to show
> how many lines were added and removed by a given commit for each file,
> you can use the "--stat" option to produce a diffstat. The "size" of
> the files in each commit isn't very meaningful to the commit itself,
> but a stat of how much was removed might be more accurate to what
> you're looking for.

That's a good suggestion, and hopefully could help David answer his
original question.

I took the request to mean "walk through history, and for each file that
a commit touches, show its size". Which is a bit harder to do, and I
think you need to script a little:

  git rev-list HEAD |
  git diff-tree --stdin -r |
  perl -lne '
# raw diff line, capture filename and post-image sha1
if (/^:\S+ \S+ \S+ (\S+) \S+\t(.*)/) {
  print "$1 $commit:$2"
}
# otherwise it is a commit sha1
else {
  $commit = $_;
}
  ' |
  git cat-file --batch-check='%(objectsize) %(rest)'

That should show the size of each file along with the "commit:path" in
which it was introduced.

-Peff

Re: [RFC PATCH v2 1/5] branch: improve documentation and naming of certain parameters

On Mon, Sep 25, 2017 at 1:20 AM, Kaartic Sivaraam
 wrote:
> Documentation for a certain function was incomplete as it didn't say
> what certain parameters were used for. Further a parameter name wasn't
> very communicative.
>
> So, add missing documentation for the sake of completeness and easy
> reference. Also, rename the concerned parameter to make it's name more
> communicative.
>
> Signed-off-by: Kaartic Sivaraam 

Up to here ( including the subject line), I have no idea you're talking about
'create_branch'. Maybe

branch: improve documentation and naming of parameters for create_branch

The documentation for 'create_branch' was incomplete ...

The patch itself looks great!

Thanks,
Stefan

Re: [PATCH 2/3] t5615: avoid re-using descriptor 4

On Thu, Oct 19, 2017 at 10:50 PM, Jeff King  wrote:
> On Thu, Oct 19, 2017 at 07:23:37PM -0400, Jeff King wrote:
>
>> So one trick is that we can't just set it to a higher number. We have to
>> also open and manage that descriptor. It might be enough to do:
>>
>>   if test -n "$BASH_VERSION"
>>   then
>>   exec 999>&4
>>   BASH_XTRACEFD=999
>>   fi
>> [...]
>> I think it might be workable, but I'm worried we're opening a can of
>> worms. Or continuing to dig into the can of worms from the original
>> BASH_XTRACEFD, if you prefer. :)
>
> So this is the best I came up with (on top of my other patches for
> ease of reading, though I'd re-work them if this is the route we're
> going to go):
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 14fac6d6f2..833ceaefd2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -377,7 +377,12 @@ fi
>  #
>  # Note also that we don't need or want to export it. The tracing is local to
>  # this shell, and we would not want to influence any shells we exec.
> -BASH_XTRACEFD=4
> +if test -n "$BASH_VERSION"
> +then
> +   exec 999>&4
> +   BASH_XTRACEFD=999
> +   silence_trace="999>/dev/null"
> +fi
>
>  test_failure=0
>  test_count=0
> @@ -627,14 +632,16 @@ test_eval_ () {
> # be _inside_ the block to avoid polluting the "set -x" output
> #
>
> -   test_eval_inner_ "$@" &3 2>&4
> -   {
> -   test_eval_ret_=$?
> -   if want_trace
> -   then
> -   set +x
> -   fi
> -   } 2>/dev/null 4>&2
> +   eval '
> +   test_eval_inner_ "$@" &3 2>&4
> +   {
> +   test_eval_ret_=$?
> +   if want_trace
> +   then
> +   set +x
> +   fi
> +   } 2>/dev/null '"$silence_trace"'
> +   '
>
> if test "$test_eval_ret_" != 0 && want_trace
> then
>
> I really hate that extra eval, since it adds an extra layer of "+" to
> the tracing output in bash.
..
> So I dunno. It does solve the problem in a way that the individual test
> scripts wouldn't have to care about. But it's a lot of eval trickery.
>

I agree. Maybe just stick with the original patch?

Thanks,
Stefan

Re: [RFE] Add minimal universal release management capabilities to GIT

On Fri, Oct 20, 2017 at 3:40 AM,   wrote:
> Hi,
>
> Git is a wonderful tool, which has transformed how software is created, and 
> made code sharing and reuse, a lot easier (both between human and software 
> tools).
>
> Unfortunately Git is so good more and more developers start to procrastinate 
> on any activity that happens outside of GIT, starting with cutting releases. 
> The meme "one only needs a git commit hash" is going strong, even infecting 
> institutions like lwn and glibc 
> (https://lwn.net/SubscriberLink/736429/e5a8ccc85cc8/)

For release you would want to include more than just "the code" into
the hash, such as compiler versions, environment variables, the phase
of the moon, what have you, that may impact the release build.

> However, the properties that make a hash commit terrific at the local 
> development level, also make it suboptimal as a release ID:
>
> – hashes are not ordered. A human can not guess the sequencing of two hashes, 
> nor can a tool, without access to Git history. Just try to handle "critical 
> security problem in project X, introduced with version Y and fixed in Z" when 
> all you have is some git hashes. hashing-only introduces severe frictions 
> when analysing deployment states.

It sounds to me as if you assume that if X, Y, Z were numbers (or
rather had some order), this can be easily deduced.
The output of git-describe ought to be sufficient for an ordering
scheme to rely on?
However the problem with deployments is that Y might be v1.8.0.1 and Z
might be v2.1.2.0 and X (that you are running) is v2.10.2.0.

> — hashes are not ranked. You can not guess, looking at a hash, if it 
> corresponds to a project stability point, or is in a middle of a refactoring 
> sequence, where things are expected to break. Evaluating every hash of every 
> project you use quickly becomes prohibitive, with the only possible strategy 
> being to just use the latest commit at a given time and pray (and if you are 
> lucky never never update afterwards unless you have lots of fixing and 
> testing time to waste).

That is up to the hash function. One could imagine a hash function
that generates bit patterns that you can use to obtain an order from.
SHA-1 that Git uses is not such a hash, but rather a supposedly secure
hash. One hash value looks like white noise, such that the entropy of
a SHA-1 object name can be estimated with 160 bits.

> – commit mixing is broken by design.

In Git terms a repository is the whole universe.
If you want relationships between different projects, you need to
include these projects e.g. via subtree or submodules.
It scales even up to linux distributions (e.g.
https://github.com/gittup/gittup, includes nethack!)

> One can not adapt the user of a piece of code to changes in this piece of 
> code before those changes are committed in the first place. There will always 
> be moments where the latest commit of a project, is incompatible with the 
> latest commit of downsteam users of this project. It is not a problem in 
> developer environments and automated testers, where you want things to break 
> early and be fixed early. It is a huge problem when you follow the same early 
> commit push strategy for actual production code, where failures are not just 
> a red light in a build farm dashboard, but have real-world consequences. And 
> the more interlinked git repositories you pile on one another, the higher the 
> probability is two commits won't work with one another with failures 
> cascading down

That is software engineering in general, I am not sure how Git relates
to this? Any change that you make (with or without utilizing Git) can
break the downstream world.

> – commits are too granular. Even assuming one could build an automated 
> regression farm powerful enough to build and test instantaneously every 
> commit, it is not possible to instantaneously push those rebuilds to every 
> instance where this code is deployed (even with infinite bandwidth, infinite 
> network reach and infinite network availability).

With infinite resources it would be possible, as the computers are
also infinitely fast. ;)

> Computers would be spending their time resetting to the latest build of one 
> component or another, with no real work being done. So there will always be a 
> distance, between the latest commit in a git repo, and what is actually 
> deployed. And we've seen bare hashes make evaluating this distance difficult
>
> – commits are a bad inter-project synchronisation point. There are too many 
> of them, they are not ranked, everyone is choosing a different commit to 
> deploy, that effectively kills the network effects that helped making 
> traditional releases solid (because distributors used the same release state, 
> and could share feedback and audit results).

There are different strategies. Relevant open source projects (kernel,
glibc, git) are pretty good at not breaking the downstream users with
every

Re: "Cannot fetch git.git" (worktrees at fault? or origin/HEAD) ?

On Thu, Oct 19, 2017 at 11:04 PM, Jeff King  wrote:
> On Thu, Oct 19, 2017 at 10:27:28PM -0700, Stefan Beller wrote:
>
>> > If my analysis above is correct, then it's already fixed. You just had
>> > leftover corruption.
>>
>> Well fetching yesterday worked and the commit in question is from
>> 8/23, the merge  8a044c7f1d56cef657be342e40de0795d688e882
>> occurred 9/18, so I suspect there is something else at play.
>> (I do not remember having a gc between yesterday and today.
>> Though maybe one in the background?)
>
> Even a gc between yesterday and today should have used the new code,
> which would have been safe. So yeah, maybe it is something else
> entirely.

Oh, yeah.

>
>> I am curious how you can have a worktree owned by multiple
>> repositories [1] (?).
>
> Sorry, I forgot my footnote. I saw this with my "ci" script:
>
>   https://github.com/peff/git/blob/7905ff395adecdd2bb7ab045a24223dfb103e0e9/ci
>
> I check out the contents of my "meta" branch as "Meta", and it contains
> that script. It basically just waits for ref updates, then walks over
> all the commits and runs "make test" on them in the background (caching
> the results, thanks to the git-test[1] script). So I kick off "Meta/ci"
> in a terminal and forget about it, and magically it builds my commits in
> the background as I work.
>
> It operates in a worktree inside the Meta directory (Meta/tmp-ci), so as
> not to disturb what I'm doing. So far so good.
>
> But I actually have _two_ clones of Git on my system. One on which I do
> most of my work, and then the other which has the fork we use in
> production at GitHub. I symlink the Meta directory from the first into
> the latter, which means they both see the same worktree directory. And
> somehow running "Meta/ci" in the second corrupted things.
>
> I can get some funniness now, but I think it's mostly caused by the
> script being confused about the worktree existing but not having access
> to our branches. That's not a corruption, just a confusion. I _think_ I
> had a bogus HEAD in the worktree at one point, but I may be
> mis-remembering. I can't seem to trigger it now.

Thanks for these insights.

I played around with Meta a bit, but I did not feel it would enhance
my workflow enough as I am not involved with any maintainance
of git using git.

The git-test from Michael sounds intriguing. Initially I put off using
it as I had my main working dir (or rather test dir) on a spinning
disk, still. Now I test in memory only, which is a lot faster, so I could
try if git-test can keep up with my local commit pace.

Thanks,
Stefan

Re: hot to get file sizes in git log output

2017-10-20 Thread Jacob Keller

On Fri, Oct 20, 2017 at 11:12 AM, David Lang  wrote:
> I'm needing to scan through git history looking for the file sizes (looking
> for when a particular file shrunk drastically)
>
> I'm not seeing an option in git log or git whatchanged that gives me the
> file size, am I overlooking something?
>
> David Lang

I'm not exactly sure what you mean by size, but if you want to show
how many lines were added and removed by a given commit for each file,
you can use the "--stat" option to produce a diffstat. The "size" of
the files in each commit isn't very meaningful to the commit itself,
but a stat of how much was removed might be more accurate to what
you're looking for.

Thanks,
Jake

Re: [PATCH] builtin/push.c: add push.pushOption config

On Thu, Oct 19, 2017 at 7:19 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>> @@ -161,6 +161,9 @@ already exists on the remote side.
>>> Transmit the given string to the server, which passes them to
>>> the pre-receive as well as the post-receive hook. The given string
>>> must not contain a NUL or LF character.
>>> +   When no `--push-option ` is given from the command
>>> +   line, the values of configuration variable `push.pushOption`
>>> +   are used instead.
>>
>> We'd also want to document how push.pushOption works in
>> Documentation/config.txt (that contains all the configs)
>
> Perhaps.
>
>> So in the config, we have to explicitly give an empty option to
>> clear the previous options, but on the command line we do not need
>> that, but instead we'd have to repeat any push options that we desire
>> that were configured?
>
> It is not wrong per-se to phrase it like so, but I think that is
> making it unnecessarily confusing by conflating two things.  (1)
> configured values are overridden from the command line, just like
> any other --option/config.variable pair

because they are single value options (usually).

> and (2) unlike usual single
> value variables where "last one wins" rule is simple enough to
> explain,, multi-value variables need a way to "forget everything we
> said so far and start from scratch" syntax, especially when multiple
> input files are involved.

ok, my view of how that should be done is clashing once again
with the projects established standards. Sorry for the noise.

>> Example:
>>
>>   /etc/gitconfig
>>   push.pushoption = a
>>   push.pushoption = b
>>
>>   ~/.gitconfig
>>   push.pushoption = c
>>
>>   repo/.git/config
>>   push.pushoption =
>>   push.pushoption = b
>>
>> will result in only b as a and c are
>> cleared.
>
> The above is correct, and it might be worth giving it as an example
> in the doc, because not just "give an empty entry to clear what has
> been accumulated so far" but a multi-valued option in general is a
> rather rare thing.
>
>> If I were to run
>>   git -c push.pushOption=d push ... (in repo)
>> it would be b and d, but
>>   git push --push-option=d
>> would be d only?
>
>>> @@ -584,12 +599,13 @@ int cmd_push(int argc, const char **argv, const char 
>>> *prefix)
>>> set_refspecs(argv + 1, argc - 1, repo);
>>> }
>>>
>>> -   for_each_string_list_item(item, _options)
>>> +   for_each_string_list_item(item, push_options)
>>
>> We have to do the same for _cmdline here, too?
>
> I do not think so.  The point of these lines that appear before this
> loop:
>
> git_config(git_push_config, );
> argc = parse_options(argc, argv, prefix, options, push_usage, 0);
> +   push_options = (push_options_cmdline.nr
> +   ? _options_cmdline
> +   : _options_config);
>
> is that the command line overrides configured values, just like any
> other configuration.  Adding _cmdline variant here is doubly wrong
> when command line options are given in that it (1) duplicates what
> was obtained from the command line, and (2) does not clear the
> configured values.

Oh, ok. Sorry for the noise once again.

Thanks,
Stefan

Re: [RFC PATCH v2 0/5] Give more useful error messages when renaming a branch

On Fri, Oct 20, 2017 at 12:09 AM, Kaartic Sivaraam
 wrote:
> What happened to the v2 of the patch? Has this not received attention
> or is there anything that's not done correctly?
>

I just checked origin/pu as well as the latest cooking email
https://public-inbox.org/git/xmqqr2tzith7@gitster.mtv.corp.google.com/
and could not find your series.

Sorry for dropping the ball as a reviewer, I'll take a look.

Thanks for pinging,
Stefan

Re: [PATCH v1] read_index_from(): Skip verification of the cache entry order to speed index loading

> Ben's original mail talks about integrity checks of the index file, and
> how expensive they get when you talk about any decent-sized index (read:
> *a lot* larger than Git or even Linux developers will see regularly).

I am quite aware of your situation.

> The text you quoted talks about our talking out of our rear ends when we
> talk about typical user schenarios because we simply have no telemetry or
> otherwise reliable statistics.
>
> Now, I fail to see any relationship between Jonathan's mail and either of
> Ben's statements.
>
> Care to enlighten me?

There was a recent thread (which I assumed was the one I linked), that talked
about security implications as soon as we loose the rather strict "git
is to be used
in a posix world", e.g. sharing your repo over NFS/Dropbox. The
specific question
that Peff asked was how the internal formats can be exploited. (Can a malicious
index file be crafted such that it is not just a segfault, but a
'remote' code execution,
given that you deploy the maliciously crafted file via NFS. Removing checks that
we already have made me a bit suspicious that it *may* be helping an
attacker here,
though I have no hard data to show)

Sorry for the confusion,

Thanks,
Stefan

hot to get file sizes in git log output

2017-10-20 Thread David Lang

I'm needing to scan through git history looking for the file sizes (looking for 
when a particular file shrunk drastically)


I'm not seeing an option in git log or git whatchanged that gives me the file 
size, am I overlooking something?


David Lang

[PATCH] l10n: de.po: fix typos

2017-10-20 Thread Ralf Thielow

From: Andre Hinrichs 

Signed-off-by: Andre Hinrichs 
Signed-off-by: Ralf Thielow 
---
 po/de.po | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/po/de.po b/po/de.po
index 0619c4988..a05aca5f3 100644
--- a/po/de.po
+++ b/po/de.po
@@ -2389,7 +2389,7 @@ msgstr "Sie haben Ihren Merge von Notizen nicht 
abgeschlossen (%s existiert)."
 #: notes-utils.c:42
 msgid "Cannot commit uninitialized/unreferenced notes tree"
 msgstr ""
-"Kann uninitialisiertes/unreferenzierte Notiz-Verzeichnis nicht committen."
+"Kann uninitialisiertes/unreferenziertes Notiz-Verzeichnis nicht committen."
 
 #: notes-utils.c:101
 #, c-format
@@ -4805,7 +4805,7 @@ msgstr "nichts zum Commit vorgemerkt, aber es gibt 
unversionierte Dateien\n"
 #, c-format
 msgid "nothing to commit (create/copy files and use \"git add\" to track)\n"
 msgstr ""
-"nichts zu committen (Erstellen/Kopieren Sie Dateien und benutzen\n"
+"nichts zu committen (erstellen/kopieren Sie Dateien und benutzen\n"
 "Sie \"git add\" zum Versionieren)\n"
 
 #: wt-status.c:1661 wt-status.c:1666
@@ -7141,7 +7141,7 @@ msgstr "Abstand zum linken Rand"
 
 #: builtin/column.c:32
 msgid "Padding space on right border"
-msgstr "Abstand zur rechten Rand"
+msgstr "Abstand zum rechten Rand"
 
 #: builtin/column.c:33
 msgid "Padding space between columns"
@@ -7573,7 +7573,7 @@ msgstr "Konnte neu erstellten Commit nicht nachschlagen."
 
 #: builtin/commit.c:1451
 msgid "could not parse newly created commit"
-msgstr "Konnte neulich erstellten Commit nicht analysieren."
+msgstr "Konnte neu erstellten Commit nicht analysieren."
 
 #: builtin/commit.c:1496
 msgid "detached HEAD"
@@ -8482,7 +8482,7 @@ msgstr "%s hat nicht alle erforderlichen Objekte 
gesendet\n"
 #, c-format
 msgid "reject %s because shallow roots are not allowed to be updated"
 msgstr ""
-"%s wurde zurückgewiesen, da Ursprungs-Commits von Repositoriesmit "
+"%s wurde zurückgewiesen, da Ursprungs-Commits von Repositories mit "
 "unvollständiger Historie (shallow) nicht aktualisiert werden dürfen."
 
 #: builtin/fetch.c:877 builtin/fetch.c:973
@@ -10539,7 +10539,7 @@ msgstr "--continue erwartet keine Argumente"
 
 #: builtin/merge.c:1192
 msgid "There is no merge in progress (MERGE_HEAD missing)."
-msgstr "Es ist keine Merge im Gange (MERGE_HEAD fehlt)."
+msgstr "Es ist kein Merge im Gange (MERGE_HEAD fehlt)."
 
 #: builtin/merge.c:1208
 msgid ""
@@ -10917,7 +10917,7 @@ msgstr "nur Tags verwenden, um die Commits zu benennen"
 
 #: builtin/name-rev.c:398
 msgid "only use refs matching "
-msgstr "nur Referenzen verwenden die  entsprechen"
+msgstr "nur Referenzen verwenden, die  entsprechen"
 
 #: builtin/name-rev.c:400
 msgid "ignore refs matching "
@@ -12395,7 +12395,7 @@ msgstr "alle Tags und verbundene Objekte beim Anfordern 
importieren"
 
 #: builtin/remote.c:165
 msgid "or do not fetch any tag at all (--no-tags)"
-msgstr "oder fordere gar keine Zweige an (--no-tags)"
+msgstr "oder fordere gar keine Tags an (--no-tags)"
 
 #: builtin/remote.c:167
 msgid "branch(es) to track"
@@ -12498,7 +12498,7 @@ msgstr[0] ""
 "gelöscht;\n"
 "um diesen zu löschen, benutzen Sie:"
 msgstr[1] ""
-"Hinweis: Einige Branches außer der refs/remotes/ Hierarchie wurden nicht "
+"Hinweis: Einige Branches außerhalb der refs/remotes/ Hierarchie wurden nicht "
 "entfernt;\n"
 "um diese zu entfernen, benutzen Sie:"
 
@@ -14247,7 +14247,7 @@ msgstr "Cache für unversionierte Dateien aktivieren 
oder deaktivieren"
 #: builtin/update-index.c:1008
 msgid "test if the filesystem supports untracked cache"
 msgstr ""
-"prüfen ob das Dateisystem einen Cache für unversionierte Dateien unterstützt"
+"prüfen, ob das Dateisystem einen Cache für unversionierte Dateien unterstützt"
 
 #: builtin/update-index.c:1010
 msgid "enable untracked cache without testing the filesystem"
@@ -14942,7 +14942,7 @@ msgid ""
 "Error: Your local changes to the following files would be overwritten by "
 "merge"
 msgstr ""
-"Fehler Ihre lokalen Änderungen in den folgenden Dateien würden durch den "
+"Fehler: Ihre lokalen Änderungen in den folgenden Dateien würden durch den "
 "Merge\n"
 "überschrieben werden"
 
@@ -15242,7 +15242,7 @@ msgstr "Konnte den Index nicht aktualisieren."
 
 #: git-stash.sh:568
 msgid "Cannot apply a stash in the middle of a merge"
-msgstr "Kann \"stash\" nicht anwenden, solang ein Merge im Gange ist"
+msgstr "Kann \"stash\" nicht anwenden, solange ein Merge im Gange ist"
 
 #: git-stash.sh:576
 msgid "Conflicts in index. Try without --index."
@@ -15268,7 +15268,7 @@ msgstr "Index wurde nicht aus dem Stash zurückgeladen."
 #: git-stash.sh:641
 msgid "The stash entry is kept in case you need it again."
 msgstr ""
-"Der Stash-Eintrag wird behalten, im Falle Sie benötigen diesen nochmal."
+"Der Stash-Eintrag wird für den Fall behalten, dass Sie diesen nochmal 
benötigen."
 
 #: git-stash.sh:650
 #, sh-format
@@ -16468,7 +16468,7 @@ msgstr

[RFC] protocol version 2

2017-10-20 Thread Brandon Williams

Objective
===

Replace Git's current wire protocol with a simpler, less wasteful
protocol that can evolve over time.

Background

Git's wire protocol is the language used to clone/fetch/push from/to a
remote git repository. A detailed explanation of the current protocol
spec can be found
[here](https://git.kernel.org/pub/scm/git/git.git/tree/Documentation/technical/pack-protocol.txt).
Some of the pain points with the current protocol spec are:

* The server's initial response is the ref advertisement. This
advertisement cannot be omitted and can become an issue due to the
sheer number of refs that can be sent with large repositories. For
example, when contacting the internal equivalent of
`https://android.googlesource.com/`, the server will send
approximately 1 million refs totaling 71MB. This is data that is
sent during each and every fetch and is not scalable.

* Capabilities were implemented as a hack and are hidden behind a NUL
byte after the first ref sent from the server during the ref
advertisement:

Since they are sent in the context of a pkt-line they are also subject
to the same length limitations (1k bytes with old clients). While we
may not be close to hitting this limitation with capabilities alone, it
has become a problem when trying to abuse capabilities for other
purposes (e.g.
[symrefs](https://public-inbox.org/git/20160816161838.klvjhhoxsftvkfmd@x/)).

* Various other technical debt (e.g. abusing capabilities to
communicate agent and symref data, service name set using a query
parameter).

Overview
==

This document presents a specification for a version 2 of Git's wire
protocol. Protocol v2 will improve upon v1 in the following ways:

* Instead of multiple service names, multiple commands will be
supported by a single service
* Easily extendable as capabilities are moved into their own section
of the protocol, no longer being hidden behind a NUL byte and
limited by the size of a pkt-line (as there will be a single
capability per pkt-line).
* Separate out other information hidden behind NUL bytes (e.g. agent
string as a capability and symrefs can be requested using 'ls-ref')
* Ref advertisement will be omitted unless explicitly requested
* ls-ref command to explicitly request some refs

Detailed Design
=

A client can request to speak protocol v2 by sending `version=2` in the
side-channel `GIT_PROTOCOL` in the initial request to the server.

In protocol v2 communication is command oriented. When first contacting
a server a list of capabilities will advertised. Some of these
capabilities will be commands which a client can request be executed.
Once a command has completed, a client can reuse the connection and
request that other commands be executed

Special Packets
-

In protocol v2 these special packets will have the following semantics:

* '' Flush Packet (flush-pkt) - indicates the end of a message
* '0001' End-of-List delimiter (delim-pkt) - indicates the end of a list

Capability Advertisement
--

A server which decides to communicate (based on a request from a client)
using protocol version 2, notifies the client by sending a version
string in its initial response followed by an advertisement of its
capabilities. Each capability is a key with an optional value. Clients
must ignore all unknown keys. Semantics of unknown values are left to
the definition of each key. Some capabilities will describe commands
which can be requested to be executed by the client.

capability-advertisement = protocol-version
capability-list
flush-pkt

protocol-version = PKT-LINE("version 2" LF)
capability-list = *capability
capability = PKT-LINE(key[=value] LF)

key = 1*CHAR
value = 1*CHAR
CHAR = 1*(ALPHA / DIGIT / "-" / "_")

A client then responds to select the command it wants with any
particular capabilities or arguments. There is then an optional section
where the client can provide any command specific parameters or queries.

command-request = command
capability-list
delim-pkt
(command specific parameters)
flush-pkt
command = PKT-LINE("command=" key LF)

The server will then acknowledge the command and requested capabilities
by echoing them back to the client and then launch into the command.

acknowledge-request = command
capability-list
delim-pkt
execute-command
execute-command =

A particular command can last for as many rounds as are required to
complete the service (multiple for negotiation during fetch and push or
no additional trips in the case of ls-refs).

Commands in v2

Services

Contact me for more informations.

2017-10-20 Thread HIMMED OMAR

Greetings

You are being contacted you  is to transfer an abandoned $26.7m
Thousand Dollars) to your account. As  the  Next Of Kin to deceased
customer who died in motor accident with his family living no body
behind to put claim on his deposit in our bank, you would be given
more information upon your response to my E-mail.
(himmedoma...@gmail.com )

Himmed Omar

Re: Updated to v2.14.2 on macOS; git add --patch broken

2017-10-20 Thread Jan Prachař

On Tue, 2017-10-03 at 05:10 -0400, Jeff King wrote:
> I dunno. Maybe I am wrong, because certainly I never set it. We've
> had
> two reports on the list since v2.14.2. The motivation for the first
> was
> "I have no idea why I set that, and I'll switch to auto". This is the
> second.
> 

I also came across this git add -p regression. The reason I added
color.ui =
always was that `git log --pretty="tformat:%Cgreen%h%Creset" | less`
stopped
having colored output with color.ui = auto. And git config man page
suggeseted
color.ui = always on the first place.

Re: [PATCH 3/4] fsmonitor: Document GIT_TRACE_FSMONITOR

2017-10-20 Thread Ben Peart




On 10/19/2017 9:11 PM, Alex Vandiver wrote:

Signed-off-by: Alex Vandiver 
---
  Documentation/git.txt | 4 
  1 file changed, 4 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 1fca63634..720db196e 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -594,6 +594,10 @@ into it.
  Unsetting the variable, or setting it to empty, "0" or
  "false" (case insensitive) disables trace messages.
  
+`GIT_TRACE_FSMONITOR`::

+   Enables trace messages for the filesystem monitor extension.
+   See `GIT_TRACE` for available trace output options.
+
  `GIT_TRACE_PACK_ACCESS`::
Enables trace messages for all accesses to any packs. For each
access, the pack file name and an offset in the pack is



Looks like a reasonable addition.  Thank you.

Re: [PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

2017-10-20 Thread Ben Peart




On 10/20/2017 9:00 AM, Johannes Schindelin wrote:

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:


This provides small performance savings.

diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
index 377edc7be..eba46c78b 100755
--- a/t/t7519/fsmonitor-watchman
+++ b/t/t7519/fsmonitor-watchman
@@ -51,7 +51,7 @@ launch_watchman();
  
  sub launch_watchman {
  
-	my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')

+   my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')


While I am very much infavor of this change (I was not aware of the
--no-pretty option), I would like to see some statistics on that. Could
you measure the impact, please, and include the results in the commit
message?

Ciao,
Johannes



I was also unaware of the --no-pretty option. I've tested this on 
Windows running version 4.9.0 of Watchman and verified that it does work 
correctly.  I'm also curious if it produces any measurable difference in 
performance.

Re: [PATCH 0/4] fsmonitor fixes

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:

> A few fixes found from playing around with the fsmonitor branch in
> next.

Thank you for having a look, and even better: for sending a couple of
improvements on top of Ben's & my work.

I sent a couple of suggestions and hope you will find the useful?

Ciao,
Johannes

Re: [PATCH 4/4] fsmonitor: Delay updating state until after split index is merged

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:

>  extern struct index_state the_index;
> diff --git a/fsmonitor.c b/fsmonitor.c
> index 7c1540c05..4c2668f57 100644
> --- a/fsmonitor.c
> +++ b/fsmonitor.c
> @@ -49,20 +49,7 @@ int read_fsmonitor_extension(struct index_state *istate, 
> const void *data,
>   ewah_free(fsmonitor_dirty);
>   return error("failed to parse ewah bitmap reading fsmonitor 
> index extension");
>   }
> -
> - if (git_config_get_fsmonitor()) {
> - /* Mark all entries valid */
> - for (i = 0; i < istate->cache_nr; i++)
> - istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
> -
> - /* Mark all previously saved entries as dirty */
> - ewah_each_bit(fsmonitor_dirty, fsmonitor_ewah_callback, istate);
> -
> - /* Now mark the untracked cache for fsmonitor usage */
> - if (istate->untracked)
> - istate->untracked->use_fsmonitor = 1;
> - }
> - ewah_free(fsmonitor_dirty);
> + istate->fsmonitor_dirty = fsmonitor_dirty;

>From the diff, it is not immediately clear that fsmonitor_dirty is not
leaked in any code path.

Could you clarify this in the commit message, please?

> @@ -238,6 +225,29 @@ void remove_fsmonitor(struct index_state *istate)
>  
>  void tweak_fsmonitor(struct index_state *istate)
>  {
> + int i;
> +
> + if (istate->fsmonitor_dirty) {
> + /* Mark all entries valid */
> + trace_printf_key(_fsmonitor, "fsmonitor is enabled; cache 
> is %d", istate->cache_nr);

Sadly, a call to trace_printf_key() is not really a noop when tracing is
disabled. The call to trace_printf_key() hands off to trace_vprintf_fl(),
which in turn calls prepare_trace_line() which asks trace_want() whether
we need to trace, which finally calls get_trace_fd(). This last function
initializes a trace key if needed, and this entire call stack takes time.

In this case, where we trace whether fsmonitor is enabled essentially once
during the life cycle of the current Git command invocation, it may be
okay, but later we perform a trace for every single ie_match_stat() call,
which I think should be guarded to avoid unnecessary code churn in
performance critical code paths (O(N) is pretty good until the constant
factor becomes large).

> + for (i = 0; i < istate->cache_nr; i++) {
> + istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID;
> + }
> + trace_printf_key(_fsmonitor, "marked all as valid");
> +
> + /* Mark all previously saved entries as dirty */
> + trace_printf_key(_fsmonitor, "setting each bit on %p", 
> istate->fsmonitor_dirty);
> + ewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, 
> istate);
> +
> + /* Now mark the untracked cache for fsmonitor usage */
> + trace_printf_key(_fsmonitor, "setting untracked state");
> + if (istate->untracked)
> + istate->untracked->use_fsmonitor = 1;
> + ewah_free(istate->fsmonitor_dirty);

At this point, please set istate->fsmonitor_dirty = NULL, as it is not
immediately obvious from this patch (or from the postimage of this diff)
that the array is not used later on.

> + } else {
> + trace_printf_key(_fsmonitor, "fsmonitor not enabled");
> + }
> +
>   switch (git_config_get_fsmonitor()) {
>   case -1: /* keep: do nothing */
>   break;
> diff --git a/read-cache.c b/read-cache.c
> index c18e5e623..3b5cd00a2 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -330,6 +330,10 @@ int ie_match_stat(struct index_state *istate,
>   return 0;
>   if (!ignore_fsmonitor && (ce->ce_flags & CE_FSMONITOR_VALID))
>   return 0;
> + if (ce->ce_flags & CE_FSMONITOR_VALID)
> + trace_printf_key(_fsmonitor, "fsmon marked valid for %s", 
> ce->name);
> + if (ignore_fsmonitor)
> + trace_printf_key(_fsmonitor, "Ignoring fsmonitor for %s", 
> ce->name);

This is the code path I am fairly certain should not be penalized if
tracing is disabled.

Ciao,
Johannes

[PATCH v4] commit: check result of resolve_ref_unsafe

2017-10-20 Thread Andrey Okoshkin

Add check of the resolved HEAD reference while printing of a commit summary.
resolve_ref_unsafe() may return NULL pointer if underlying calls of lstat() or
open() fail in files_read_raw_ref().
Such situation can be caused by race: file becomes inaccessible to this moment.

Signed-off-by: Andrey Okoshkin 
---
Hello,
I think this way is better for user experience:
* git doesn't crash;
* warning is shown;
* commit has been successfully created then it's safe to show a summary message
with already known information and without resolved HEAD.

Best regards,
Andrey

 builtin/commit.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1a0da71a4..647d6ab3e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1483,7 +1483,10 @@ static void print_summary(const char *prefix, const 
struct object_id *oid,
diff_setup_done();
 
head = resolve_ref_unsafe("HEAD", 0, junk_oid.hash, NULL);
-   if (!strcmp(head, "HEAD"))
+   if (!head) {
+   warning_errno(_("unable to resolve HEAD after creating 
commit"));
+   head = _("unresolvable HEAD");
+   } else if (!strcmp(head, "HEAD"))
head = _("detached HEAD");
else
skip_prefix(head, "refs/heads/", );
-- 
2.14.2

IMPORTANT INFORMATION,

2017-10-20 Thread Mr Thomas Alpha

Good Day,


Please accept my apologies for writing you a surprise letter.I am Mr
Thomas Alpha, account Manager with an investment bank here in Burkina
Faso.

I have a very important business I want to discuss with you.There is a
draft account opened in my firm by a long-time client of our bank.I
have the opportunity of transferring the left over fund (10.Million Us
Dollars)Ten Million United States of American Dollars of one of my
Bank clients who died at the collapsing of the world trade center at
the United States on September 11th 2001.

I want to invest this funds and introduce you to our bank for this
deal.All I require is your honest co-operation and I guarantee you
that this will be executed under a legitimate arrangement that will
protect us from any breach of the law.I agree that 40% of this money
will be for you as my foreign partner,50% for me while 10% is for
establishing of foundation for the less privileges in your country.If
you are really interested in my proposal return my email with the
below information and i will forward a text of application letter to
you which you are going to fill and send to the bank for claiming of
the fund.

Your Full Name...
Your Age and Sex
Your Private Telephone...
Your Occupation...
Your Country


Yours Sincerely,
Mr Thomas Alpha.

Re: [PATCH 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:

> This provides small performance savings.
> 
> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
> index 377edc7be..eba46c78b 100755
> --- a/t/t7519/fsmonitor-watchman
> +++ b/t/t7519/fsmonitor-watchman
> @@ -51,7 +51,7 @@ launch_watchman();
>  
>  sub launch_watchman {
>  
> - my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')
> + my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')

While I am very much infavor of this change (I was not aware of the
--no-pretty option), I would like to see some statistics on that. Could
you measure the impact, please, and include the results in the commit
message?

Ciao,
Johannes

Re: [PATCH 1/4] fsmonitor: Watch, and ask about, the top of the repo, not the CWD

Hi Alex,

On Thu, 19 Oct 2017, Alex Vandiver wrote:

> Signed-off-by: Alex Vandiver 
> ---
>  t/t7519/fsmonitor-watchman | 3 ++-
>  templates/hooks--fsmonitor-watchman.sample | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
> index a3e30bf54..377edc7be 100755
> --- a/t/t7519/fsmonitor-watchman
> +++ b/t/t7519/fsmonitor-watchman
> @@ -41,7 +41,8 @@ if ($system =~ m/^MSYS_NT/ || $system =~ m/^MINGW/) {
>   $git_work_tree =~ s/[\r\n]+//g;
>   $git_work_tree =~ s,\\,/,g;
>  } else {
> - $git_work_tree = $ENV{'PWD'};
> + $git_work_tree = `git rev-parse --show-toplevel`;
> + chomp $git_work_tree;

This is super expensive, as it means a full-blown new process instead of
just a simple environment variable expansion.

The idea behind using `PWD` instead was that Git will already have done
all of the work of figuring out the top-level directory and switched to
there before calling the fsmonitor hook.

Did you see any case where the script was *not* called from the top-level
directory?

Ciao,
Johannes

Re: [PATCH v1] read_index_from(): Skip verification of the cache entry order to speed index loading

Hi Stefan,

On Thu, 19 Oct 2017, Stefan Beller wrote:

> On Thu, Oct 19, 2017 at 8:12 AM, Ben Peart  wrote:
> 
> > If we are guarding against "git" writing out an invalid index, we can move
> > this into an assert so that only git developers pay the cost of validating
> > they haven't created a new bug.  I think this is better than just adding a
> > new test case as a new test case would not achieve the same coverage.  This
> > is my preferred solution.
> >
> > If we are guarding against "some other application" writing out an invalid
> > index, then everyone will have to pay the cost as we can't insert the test
> > into "some other applications."  Without user reports of it happening or any
> > telemetry saying it has happened I really have no idea if it every actually
> > happens in the wild anymore and whether the cost on every index load is
> > still justified.
> 
> How well does this play out in the security realm?, c.f.
> https://public-inbox.org/git/20171002234517.gv19...@aiede.mtv.corp.google.com/

That link talks about security implications from administrators accessing
Git repositories with maliciously crafted hooks/pagers.

Ben's original mail talks about integrity checks of the index file, and
how expensive they get when you talk about any decent-sized index (read:
*a lot* larger than Git or even Linux developers will see regularly).

The text you quoted talks about our talking out of our rear ends when we
talk about typical user schenarios because we simply have no telemetry or
otherwise reliable statistics.

Now, I fail to see any relationship between Jonathan's mail and either of
Ben's statements.

Care to enlighten me?

Ciao,
Dscho

[PATCH v3] commit: check result of resolve_ref_unsafe

2017-10-20 Thread Andrey Okoshkin

Add check of the resolved HEAD reference while printing of a commit summary.
resolve_ref_unsafe() may return NULL pointer if underlying calls of lstat() or
open() fail in files_read_raw_ref().
Such situation can be caused by race: file becomes inaccessible to this moment.

Signed-off-by: Andrey Okoshkin 
---
die_errno fits better here as resolve_ref_unsafe preserves errno value.

Changes since the previous patch:
* die is replaced with die_errno.
 builtin/commit.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1a0da71a4..b52829090 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1483,6 +1483,8 @@ static void print_summary(const char *prefix, const 
struct object_id *oid,
diff_setup_done();
 
head = resolve_ref_unsafe("HEAD", 0, junk_oid.hash, NULL);
+   if (!head)
+   die_errno(_("unable to resolve HEAD after creating commit"));
if (!strcmp(head, "HEAD"))
head = _("detached HEAD");
else
-- 
2.14.2

[RFE] Add minimal universal release management capabilities to GIT

2017-10-20 Thread nicolas . mailhot

Hi,

Git is a wonderful tool, which has transformed how software is created, and 
made code sharing and reuse, a lot easier (both between human and software 
tools).

Unfortunately Git is so good more and more developers start to procrastinate on 
any activity that happens outside of GIT, starting with cutting releases. The 
meme "one only needs a git commit hash" is going strong, even infecting 
institutions like lwn and glibc 
(https://lwn.net/SubscriberLink/736429/e5a8ccc85cc8/)

However, the properties that make a hash commit terrific at the local 
development level, also make it suboptimal as a release ID:

– hashes are not ordered. A human can not guess the sequencing of two hashes, 
nor can a tool, without access to Git history. Just try to handle "critical 
security problem in project X, introduced with version Y and fixed in Z" when 
all you have is some git hashes. hashing-only introduces severe frictions when 
analysing deployment states.

— hashes are not ranked. You can not guess, looking at a hash, if it 
corresponds to a project stability point, or is in a middle of a refactoring 
sequence, where things are expected to break. Evaluating every hash of every 
project you use quickly becomes prohibitive, with the only possible strategy 
being to just use the latest commit at a given time and pray (and if you are 
lucky never never update afterwards unless you have lots of fixing and testing 
time to waste).

– commit mixing is broken by design. One can not adapt the user of a piece of 
code to changes in this piece of code before those changes are committed in the 
first place. There will always be moments where the latest commit of a project, 
is incompatible with the latest commit of downsteam users of this project. It 
is not a problem in developer environments and automated testers, where you 
want things to break early and be fixed early. It is a huge problem when you 
follow the same early commit push strategy for actual production code, where 
failures are not just a red light in a build farm dashboard, but have 
real-world consequences. And the more interlinked git repositories you pile on 
one another, the higher the probability is two commits won't work with one 
another with failures cascading down

– commits are too granular. Even assuming one could build an automated 
regression farm powerful enough to build and test instantaneously every commit, 
it is not possible to instantaneously push those rebuilds to every instance 
where this code is deployed (even with infinite bandwidth, infinite network 
reach and infinite network availability). Computers would be spending their 
time resetting to the latest build of one component or another, with no real 
work being done. So there will always be a distance, between the latest commit 
in a git repo, and what is actually deployed. And we've seen bare hashes make 
evaluating this distance difficult

– commits are a bad inter-project synchronisation point. There are too many of 
them, they are not ranked, everyone is choosing a different commit to deploy, 
that effectively kills the network effects that helped making traditional 
releases solid (because distributors used the same release state, and could 
share feedback and audit results).

One could mitigate those problems in a Git management overlay (and, indeed, 
many try). The problem of those overlays is that they have variable maturity 
levels, make incompatible choices, cut corners, are not universal like Git, 
making building anything on top of them of dubious value, with quick fallback 
to commit hashes, which *are* universal among Git repos. Release handling and 
versioning really needs to happen in Git itself to be effective.

Please please please add release handling and versioning capabilities to Git 
itself. Without it some enthusiastic Git adopters are on a fast trajectory to 
unmanageable hash soup states, even if they are not realising it yet, because 
the deleterious side effects of giving up on releases only get clear with time.

Here is what such capabilities could look like (people on this list can 
probably invent something better, I don't care as long as something exists).

1. "release versions" are first class objects that can be attached to a commit 
(not just freestyle tags that look like versions, but may be something else 
entirely). Tools can identify release IDs reliably.

2. "release versions" have strong format constrains, that allow humans and 
tools to deduce their ordering without needing access to something else (full 
git history or project-specific conventions). The usual string of numbers 
separated by dots is probably simple and universal enough (if you start to 
allow letters people will try to use clever schemes like alpha or roman 
numerals, that break automation). There needs to be at least two numbers in the 
string to allow tracking patchlevels.

3. several such objects can be attached to a commit (a project may wish to 
promote a minor

Re: [PATCH v2] commit: check result of resolve_ref_unsafe

2017-10-20 Thread Andrey Okoshkin



19.10.2017 20:44, Jeff King wrote:

On Thu, Oct 19, 2017 at 12:36:50PM +0300, Andrey Okoshkin wrote:

Thanks, this looks good to me. One other possible minor improvement:


head = resolve_ref_unsafe("HEAD", 0, junk_oid.hash, NULL);
+   if (!head)
+   die(_("unable to resolve HEAD after creating commit"));


Should we use die_errno() here to report the value of errno? I think
resolve_ref_unsafe() should set it consistently (even an internal
problem, like an illegally-formatted refname, yields EINVAL).

Thanks, sounds relevant.
Also as an alternative solution it's possible to use warning_errno 
(instead of die_errno) and continue with 'head' set to something like

'unreadable|bad HEAD'.


I grepped the code base looking for other instances of the same problem,
and found four of them. Patches to follow.

Unlike this one, I ended up quietly returning an error in most cases.
The individual commit messages discuss the reasoning for each case, but
I do wonder if we ought to simply die() in each case out of an abundance
of caution (either the repo has a broken symref, or some weird
filesystem error occurred, but either way it may be best not to
continue). I dunno.

These are all independent, so can be applied in any order or combination
with respect to each other and to your patch.

   [1/4]: test-ref-store: avoid passing NULL to printf
   [2/4]: remote: handle broken symrefs
   [3/4]: log: handle broken HEAD in decoration check
   [4/4]: worktree: handle broken symrefs in find_shared_symref()

Good changes, it's better to apply.

--
Best regards,
Andrey

I Hope you have not forgotten me??

2017-10-20 Thread Steven Timoth

Hello Dear Friend,
I Hope you have not forgotten me??

It is my pleasure to reach you after our unsuccessful attempt on our
business transaction. Well, I just want to use this medium to thank
you very much for your earlier assistance to help me in receiving the
funds.

I am obliged to inform you that I have succeeded in receiving the
funds with the help of a new partner. In appreciation of your earlier
assistance to me in receiving the funds, I have decided to compensate
you with the sum of $1,200,000.00(One Million two hundred United
States Dollars) in a International Bank ATM Visa Card. This is from my
own share. I did this simply to show appreciation to you for your kind
support and assistance even though we couldn't succeed due to some
unforeseen circumstances. Presently, I am in South Korea for
investment with my own share under the advice of my partner.

In the light of the above, you are therefore Advice to contact my
personal secretary his name is MR:TOBE JAMES, and do send him your
contact address where you want the International Compensation Bank ATM
Visa Card to be send to you.
Full Name:
Mailing Address:
Telephone (Mobile/Home/Office)
Occupation and Your scanned ID

Stated below is the contact information of my secretary:
MR:TOBE JAMES.
E-mail address: ( bonb...@hotmail.com )
Mobile:(+226)-7493 0179

So feel free to get in touch with him to send your Compensation
International Bank ATM Visa Card to you without any delay ok.

With my best regards,
Dr.Steven Tim.

Re: [PATCH] builtin/push.c: add push.pushOption config

2017-10-20 Thread Marius Paliga

> --o::
> ---push-option::
> +-o ::
> +--push-option=::
> Transmit the given string to the server, which passes them to
> the pre-receive as well as the post-receive hook. The given string
> must not contain a NUL or LF character.
> -   When no `--push-option ` is given from the command
> +   When no `--push-option=` is given from the command
> line, the values of configuration variable `push.pushOption`
> are used instead.

Should we also mention that this option can take multiple values from
the command line?

-o ::
--push-option=::
Transmit the given string to the server, which passes them to
the pre-receive as well as the post-receive hook. The given string
must not contain a NUL or LF character.
+   Multiple `--push-option=` are allowed on the command line.
   When no `--push-option=` is given from the command
line, the values of configuration variable `push.pushOption`
are used instead.



2017-10-20 8:18 GMT+02:00 Junio C Hamano :
> Junio C Hamano  writes:
>
>> Junio C Hamano  writes:
>>
 We'd also want to document how push.pushOption works in
 Documentation/config.txt (that contains all the configs)
>>>
>>> Perhaps.
>>
>> Here is my attempt.
>
> Another thing I noticed while we are around the area is that unlike
> all other options in git-push.txt page, this one forgets to say it
> always takes mandatory string.  Here is a possible fix.
>
>  Documentation/git-push.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index aa78057dc5..a8504e0ae3 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -156,12 +156,12 @@ already exists on the remote side.
> Either all refs are updated, or on error, no refs are updated.
> If the server does not support atomic pushes the push will fail.
>
> --o::
> ---push-option::
> +-o ::
> +--push-option=::
> Transmit the given string to the server, which passes them to
> the pre-receive as well as the post-receive hook. The given string
> must not contain a NUL or LF character.
> -   When no `--push-option ` is given from the command
> +   When no `--push-option=` is given from the command
> line, the values of configuration variable `push.pushOption`
> are used instead.
>

Re: [PATCH 2/1] mention git stash push first in the man page

2017-10-20 Thread Robert P. J. Day

On Thu, 19 Oct 2017, Thomas Gummerer wrote:

> On 10/17, Jeff King wrote:
> > On Tue, Oct 17, 2017 at 10:45:15PM +0100, Thomas Gummerer wrote:
> >
> > > > Seems reasonable, though if we are deprecating "save" should we demote
> > > > it from being in the synopsis entirely?
> > >
> > > I saw that as a next step, with the "official" deprecation of "save".
> > > I thought we might want to advertise "push" a bit more before actually
> > > officially deprecating "save" and sending the deprecation notice out
> > > in the release notes.
> >
> > Right, my thinking was that it would still be documented in the manpage,
> > just lower down. And that would probably say something like "save is a
> > historical synonym for push, except that it differs in these ways...".
> >
> > > OTOH, dropping it from the synopsis in the man page probably wouldn't
> > > cause much issue, as "push" directly replaces it, and is easily
> > > visible.  Not sure how slow we want to take the deprecation?
> >
> > I don't think there's any reason to go slow in marking something as
> > deprecated. It's the part where we follow up and remove or change the
> > feature that must take a while.
>
> Makes sense, let me drop it from the synopsis then.

  what, exactly, is the oft-referred-to issue with how "git stash
save" works that is being addressed with the new syntax of "git stash
push"?

rday

-- 


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

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

Re: [PATCH 4/5] diff: fix whitespace-skipping with --color-moved

2017-10-20 Thread Simon Ruderich

On Thu, Oct 19, 2017 at 04:29:26PM -0400, Jeff King wrote:
> [snip]
>
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -1463,6 +1463,73 @@ test_expect_success 'move detection ignoring 
> whitespace changes' '
>   test_cmp expected actual
>  '
>
> +test_expect_failure 'move detection ignoring whitespace at eol' '

Shouldn't this be test_expect_success? According to the commit
message "and a new "--ignore-space-at-eol" shows off the breakage
we're fixing.". I didn't actually run the code so I don't know if
the test fails or not.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Re: [RFC PATCH v2 0/5] Give more useful error messages when renaming a branch

What happened to the v2 of the patch? Has this not received attention
or is there anything that's not done correctly?

-- 
Kaartic

[ANNOUNCE] Git v2.15.0-rc2

A release candidate Git v2.15.0-rc2 is now available for testing
at the usual places.  It is comprised of 737 non-merge commits
since v2.14.0, contributed by 75 people, 22 of which are new faces.

We had to back-track a bit wrt to the "git add -p" regression; for
now, we simply revert the changes that caused issues to users
without redefining 'color.ui=always' to mean 'color.ui=auto', which
may or may not happen in future releases.

The tarballs are found at:

https://www.kernel.org/pub/software/scm/git/testing/

The following public repositories all have a copy of the
'v2.15.0-rc2' tag and the 'master' branch that the tag points at:

  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.14.0 are as follows.
Welcome to the Git development community!

  Ann T Ropea, Daniel Watkins, Derrick Stolee, Dimitrios
  Christidis, Eric Rannaud, Evan Zacks, Hielke Christian Braun,
  Ian Campbell, Ilya Kantor, Jameson Miller, Job Snijders, Joel
  Teichroeb, joernchen, Łukasz Gryglicki, Manav Rathi, Martin
  Ågren, Michael Forney, Patryk Obara, Randall S. Becker, Ross
  Kabus, Taylor Blau, and Urs Thuermann.

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Adam Dinwoodie, Ævar Arnfjörð Bjarmason, Andreas Heiduk,
  Anthony Sottile, Ben Boeckel, Brandon Casey, Brandon Williams,
  brian m. carlson, Christian Couder, David Glasser, Eric Blake,
  Han-Wen Nienhuys, Heiko Voigt, Jean-Noel Avila, Jeff Hostetler,
  Jeff King, Johannes Schindelin, Johannes Sixt, Jonathan Nieder,
  Jonathan Tan, Junio C Hamano, Kaartic Sivaraam, Kevin Daudt,
  Kevin Willford, Lars Schneider, Martin Koegler, Matthieu Moy,
  Max Kirillov, Michael Haggerty, Michael J Gruber, Nguyễn Thái
  Ngọc Duy, Nicolas Morey-Chaisemartin, Øystein Walle, Paolo
  Bonzini, Pat Thoyts, Philip Oakley, Phillip Wood, Ralf Thielow,
  Raman Gupta, Ramsay Jones, René Scharfe, Sahil Dua, Santiago
  Torres, Stefan Beller, Stephan Beyer, Takashi Iwai, Thomas
  Braun, Thomas Gummerer, Todd Zullinger, Tom G. Christensen,
  Torsten Bögershausen, William Duclot, and W. Trevor King.



Git 2.15 Release Notes (draft)
==

Backward compatibility notes and other notable changes.

 * Use of an empty string as a pathspec element that is used for
   'everything matches' is still warned and Git asks users to use a
   more explicit '.' for that instead.  The hope is that existing
   users will not mind this change, and eventually the warning can be
   turned into a hard error, upgrading the deprecation into removal of
   this (mis)feature.  That is now scheduled to happen in Git v2.16,
   the next major release after this one.

 * Git now avoids blindly falling back to ".git" when the setup
   sequence said we are _not_ in Git repository.  A corner case that
   happens to work right now may be broken by a call to BUG().
   We've tried hard to locate such cases and fixed them, but there
   might still be cases that need to be addressed--bug reports are
   greatly appreciated.

 * "branch --set-upstream" that has been deprecated in Git 1.8 has
   finally been retired.


Updates since v2.14
---

UI, Workflows & Features

 * An example that is now obsolete has been removed from a sample hook,
   and an old example in it that added a sign-off manually has been
   improved to use the interpret-trailers command.

 * The advice message given when "git rebase" stops for conflicting
   changes has been improved.

 * The "rerere-train" script (in contrib/) learned the "--overwrite"
   option to allow overwriting existing recorded resolutions.

 * "git contacts" (in contrib/) now lists the address on the
   "Reported-by:" trailer to its output, in addition to those on
   S-o-b: and other trailers, to make it easier to notify (and thank)
   the original bug reporter.

 * "git rebase", especially when it is run by mistake and ends up
   trying to replay many changes, spent long time in silence.  The
   command has been taught to show progress report when it spends
   long time preparing these many changes to replay (which would give
   the user a chance to abort with ^C).

 * "git merge" learned a "--signoff" option to add the Signed-off-by:
   trailer with the committer's name.

 * "git diff" learned to optionally paint new lines that are the same
   as deleted lines elsewhere differently from genuinely new lines.

 * "git interpret-trailers" learned to take the trailer specifications
   from the command line that overrides the configured values.

 * "git interpret-trailers" has been taught a "--parse" and a few
   other options to make it easier for scripts to grab existing
   trailer lines from a commit log message.

 * The "--format=%(trailers)" option "git log" and its friends take
   learned to take the

Hi Dear,

2017-10-20 Thread Lisa Williams



Hi Dear,

how are you today I hope that everything is OK with you as it is my great 
pleasure to contact you in having communication with you starting from today, i 
was just going through the Internet search when i found your email address, I 
want to make a very new and special friend, so i decided to contact you to see 
how we can make it work if we can. Please i wish you will have the desire with 
me so that we can get to know each other better and see what happens in future.

My name is Lisa Williams, I am an American  presently I live in the UK, I will 
be very happy if you can write me through my private email 
address(lisa.williams...@yahoo.com) for easy communication so that we can know 
each other, I will give you my pictures and details about me.

bye
Lisa

Re: [PATCH] builtin/push.c: add push.pushOption config

Junio C Hamano  writes:

> Junio C Hamano  writes:
>
>>> We'd also want to document how push.pushOption works in
>>> Documentation/config.txt (that contains all the configs)
>>
>> Perhaps.
>
> Here is my attempt.

Another thing I noticed while we are around the area is that unlike
all other options in git-push.txt page, this one forgets to say it
always takes mandatory string.  Here is a possible fix.

 Documentation/git-push.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index aa78057dc5..a8504e0ae3 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -156,12 +156,12 @@ already exists on the remote side.
Either all refs are updated, or on error, no refs are updated.
If the server does not support atomic pushes the push will fail.
 
--o::
---push-option::
+-o ::
+--push-option=::
Transmit the given string to the server, which passes them to
the pre-receive as well as the post-receive hook. The given string
must not contain a NUL or LF character.
-   When no `--push-option ` is given from the command
+   When no `--push-option=` is given from the command
line, the values of configuration variable `push.pushOption`
are used instead.

Re: [PATCH] builtin/push.c: add push.pushOption config

Junio C Hamano  writes:

>> We'd also want to document how push.pushOption works in
>> Documentation/config.txt (that contains all the configs)
>
> Perhaps.

Here is my attempt.  I have a feeling that the way http.extraheaders
is described may be much easier to read and we may want to mimick
its style.  I dunno.

 Documentation/config.txt | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6adb..631ed1172e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2621,6 +2621,16 @@ push.gpgSign::
override a value from a lower-priority config file. An explicit
command-line flag always overrides this config option.
 
+push.pushOption::
+   When no `--push-option=` argument is given from the
+   command line, `git push` behaves as if each  of
+   this variable is given as `--push-option=`.
++
+This is a multi-valued variable, and an empty value can be used in a
+higher priority cofiguration file (e.g. `.git/config` in a
+repository) to clear the values inherited from a lower priority
+configuration files (e.g. `$HOME/.gitconfig`).
+
 push.recurseSubmodules::
Make sure all submodule commits used by the revisions to be pushed
are available on a remote-tracking branch. If the value is 'check'

Re: "Cannot fetch git.git" (worktrees at fault? or origin/HEAD) ?