Re: git interpret-trailers with multiple keys

2016-04-11 Thread Matthieu Moy
"Michael S. Tsirkin"  writes:

> On Sun, Apr 10, 2016 at 06:57:53PM +0200, Christian Couder wrote:
>> What I meant is that we could create new options called maybe
>> trailer.autocommands and trailer..autocommands that default to
>> 'true' and if 'false' the command would not be run automatically and
>> the corresponding trailer would not be added.
>
> I don't think it has to do with commands.
> For example, if we add "value" it should behave the same.
>
> So I think a better name is "ifnotlisted", with values "add"
> and "donothing".

Having a negation in the variable name feels wrong. When the token is
listed on the command-line and ifnotlisted=donothing, I have to apply a
double-negation to guess what would happen (=> "if listed then do
something").

I agree that having such variable would be a good thing. It would solve
your issue (i.e. "How to I configure a token for quick use from the
command-line without applying it unconditionally"), and allow full
backward compatibility.

I'd call the option "apply" or perhaps "run", with values 1/true/always
= default = current behavior, or "auto" = "apply when asked from the
command-line". I'm wondering whether other values could make sense (not
to implement it right now, but to keep the design open to further
extensions): perhaps apply=ifauthor could mean "apply this trailer to
patches I'm the author of" for example.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git interpret-trailers with multiple keys

2016-04-11 Thread Michael S. Tsirkin
On Mon, Apr 11, 2016 at 09:09:48AM +0200, Matthieu Moy wrote:
> "Michael S. Tsirkin"  writes:
> 
> > On Sun, Apr 10, 2016 at 06:57:53PM +0200, Christian Couder wrote:
> >> What I meant is that we could create new options called maybe
> >> trailer.autocommands and trailer..autocommands that default to
> >> 'true' and if 'false' the command would not be run automatically and
> >> the corresponding trailer would not be added.
> >
> > I don't think it has to do with commands.
> > For example, if we add "value" it should behave the same.
> >
> > So I think a better name is "ifnotlisted", with values "add"
> > and "donothing".
> 
> Having a negation in the variable name feels wrong. When the token is
> listed on the command-line and ifnotlisted=donothing, I have to apply a
> double-negation to guess what would happen (=> "if listed then do
> something").

Isn't this similar to ifmissing?

> I agree that having such variable would be a good thing. It would solve
> your issue (i.e. "How to I configure a token for quick use from the
> command-line without applying it unconditionally"), and allow full
> backward compatibility.
> 
> I'd call the option "apply" or perhaps "run", with values 1/true/always
> = default = current behavior, or "auto" = "apply when asked from the
> command-line". I'm wondering whether other values could make sense (not
> to implement it right now, but to keep the design open to further
> extensions): perhaps apply=ifauthor could mean "apply this trailer to
> patches I'm the author of" for example.
> 
> -- 
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] fetch-pack: Add missing line-feed character when sending depth-request packet line

2016-04-11 Thread Stan Hu
The pkt-line format mandates: "a sender should include a LF, but the
receive MUST NOT complain if it is not present." This patch
is not absolutely necessary since receivers handle the missing the LF,
but this patch adds it for good measure.

Signed-off-by: Stan Hu 
---
 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index f96f6df..77299d9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -330,7 +330,7 @@ static int find_common(struct fetch_pack_args *args,
if (is_repository_shallow())
write_shallow_commits(&req_buf, 1, NULL);
if (args->depth > 0)
-   packet_buf_write(&req_buf, "deepen %d", args->depth);
+   packet_buf_write(&req_buf, "deepen %d\n", args->depth);
packet_buf_flush(&req_buf);
state_len = req_buf.len;
 
-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 4/7] i18n: builtin/pull.c: mark strings for translation

2016-04-11 Thread Vasco Almeida
Às 18:01 de 10-04-2016, Junio C Hamano escreveu:
> Vasco Almeida  writes:
> 
>> Some translations might also translate "" and "".
> 
> This offers an interesting observation that I didn't think of while
> reviewing the first round of this series.
> 
> Do translations want to translate "remote" and "branch" without the
> ""?  Or is it better to allow translations to adjust
> the "quote around a placeholder" in a locale dependent way?
> 
I think they want to translate without brackets.
>From what I can see, translator are using the same quoting.
The use of this kind of placeholders should be a new thing to languages,
and might be limited to some backgrounds or situations. So, translator
might adopt the English way, since there are no better or equivalent way
in their languages. Or I may be be assuming to much?

>> @@ -458,13 +458,13 @@ static void NORETURN die_no_merge_candidates(const 
>> char *repo, const char **refs
>>  fprintf_ln(stderr, _("Please specify which branch you 
>> want to merge with."));
>>  fprintf_ln(stderr, _("See git-pull(1) for details."));
>>  fprintf(stderr, "\n");
>> -fprintf_ln(stderr, "git pull  ");
>> +fprintf_ln(stderr, "git pull <%s> <%s>", _("remote"), 
>> _("branch"));
> 
> I know this hunk follows I suggested, i.e. "quotes around a
> placeholder is universal and locale independent".  However, ...
> 
>>  fprintf(stderr, "\n");
>>  } else if (!curr_branch->merge_nr) {
>>  const char *remote_name = NULL;
>>  
>>  if (for_each_remote(get_only_remote, &remote_name) || 
>> !remote_name)
>> -remote_name = "";
>> +remote_name = _("");
>>  
> ... this does not.  It allows to translate the "quote around a
> placeholder".  And where this phony "remote_name" string is used,
> there is also this reference to :
> 
> fprintf_ln(stderr, _("If you wish to set tracking information for ..."
> "\n"
> "git branch --set-upstream-to=%s/ %s\n"),
> remote_name, curr_branch->name);
> 
> which also does.
> 
> Perhaps the first hunk at around ll.458 would want to do
> 
>> +fprintf_ln(stderr, "git pull %s %s", _(""), _(""));
> 
> to be consistent and more flexible for the translator's needs?  The
> quoting convention may be locale dependent after all.
>
If we think placeholders quoting is important for localization, we could
mark them for translation in usage_argh() function in parse-options.c:

s = literal ? " %s" : _(" <%s>");

and do the same for other cases of <%s> in that function. Plus, add a
comment for translator, perhaps.

Otherwise, something like the following just to be consistent:

diff --git a/builtin/pull.c b/builtin/pull.c
index 617893c..317c3ce 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -463,8 +463,7 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
} else if (!curr_branch->merge_nr) {
const char *remote_name = NULL;
 
-   if (for_each_remote(get_only_remote, &remote_name) || 
!remote_name)
-   remote_name = _("");
+   for_each_remote(get_only_remote, &remote_name);
 
fprintf_ln(stderr, _("There is no tracking information for the 
current branch."));
if (opt_rebase)
@@ -477,8 +476,11 @@ static void NORETURN die_no_merge_candidates(const char 
*repo, const char **refs
fprintf(stderr, "\n");
fprintf_ln(stderr, _("If you wish to set tracking information 
for this branch you can do so with:"));
fprintf(stderr, "\n");
-   fprintf_ln(stderr, "git branch --set-upstream-to=%s/<%s> 
%s\n",
-   remote_name, _("branch"), curr_branch->name);
+   fprintf_ln(stderr, remote_name
+  ? "git branch --set-upstream-to=%s/<%s> 
%s\n"
+  : "git branch 
--set-upstream-to=<%s>/<%s> %s\n",
+   remote_name ? remote_name : _("remote"),
+   _("branch"), curr_branch->name);
} else
fprintf_ln(stderr, _("Your configuration specifies to merge 
with the ref '%s'\n"
"from the remote, but no such ref was fetched."),

I think there is no need to internationalize the bracket. If we did so,
they would end up just in the way without adding much value for
translators.
What do you think?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Johannes Schindelin
Hi Michael,

On Sun, 10 Apr 2016, Michael S. Tsirkin wrote:

> This implements a new ack! action for git rebase -i
> It is essentially a middle ground between fixup! and squash!:
> - commits are squashed silently without editor being started
> - commit logs are concatenated (with action line being discarded)
> - because of the above, empty commits aren't discarded,
>   their log is also included.
> 
> I am using it as follows:
>   git am -s < mailbox #creates first commit
>   hack ...
>   get mail with Ack
>   git commit --allow-empty -m `cat <<-EOF
>   ack! first
> 
>   Acked-by: maintainer
>   EOF`
>   repeat cycle
>   git rebase --autosquash -i origin/master
>   before public branch push
> 
> The "cat" command above is actually a script that
> parses the Ack mail to create the empty commit -
> to be submitted separately.

This looks awfully complicated, still, and not very generic.

How about making it easier to use, and much, much more generic, like this?

1. introducing an `--add-footer` flag to `git commit` that you could use
like this:

git commit --amend --add-footer "Acked-by: Bugs Bunny"

2. introducing an `--exec-after` flag to `git commit` that would be a new
sibling of `--fixup` and `--squash` and would work like this:

git commit --exec-after HEAD~5 \
'git commit --amend --add-footer "Acked-by: Bugs Bunny"'

(it should imply `--allow-empty`, of course, and probably even fail if
anything was staged for commit at that point.) The commit message would
then look something like

exec-after! Fix broken breakage

git commit --amend --add-footer "Acked-by: Bugs Bunny"

This way would obviously benefit a lot more users. For example, you could
easily say (and alias)

git commit --amend --add-footer 'Reviewed-by: Arrested Developer"

i.e. support all kind of use cases where developers need to slap on
footers in a quick & easy way.

And the --exec-after option would obviously have *a lot* more use cases
than just squashing in ACKs.

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Michael S. Tsirkin
On Mon, Apr 11, 2016 at 01:02:07PM +0200, Johannes Schindelin wrote:
> Hi Michael,
> 
> On Sun, 10 Apr 2016, Michael S. Tsirkin wrote:
> 
> > This implements a new ack! action for git rebase -i
> > It is essentially a middle ground between fixup! and squash!:
> > - commits are squashed silently without editor being started
> > - commit logs are concatenated (with action line being discarded)
> > - because of the above, empty commits aren't discarded,
> >   their log is also included.
> > 
> > I am using it as follows:
> > git am -s < mailbox #creates first commit
> > hack ...
> > get mail with Ack
> > git commit --allow-empty -m `cat <<-EOF
> > ack! first
> > 
> > Acked-by: maintainer
> > EOF`
> > repeat cycle
> > git rebase --autosquash -i origin/master
> > before public branch push
> > 
> > The "cat" command above is actually a script that
> > parses the Ack mail to create the empty commit -
> > to be submitted separately.
> 
> This looks awfully complicated, still, and not very generic.
> 
> How about making it easier to use, and much, much more generic, like this?

I can look at using a different syntax but the below does not
support the workflow I described, which is a standard
email based one: get email, handle it.

> 1. introducing an `--add-footer` flag to `git commit` that you could use
> like this:
> 
>   git commit --amend --add-footer "Acked-by: Bugs Bunny"
> 2. introducing an `--exec-after` flag to `git commit` that would be a new
> sibling of `--fixup` and `--squash` and would work like this:
> 
>   git commit --exec-after HEAD~5 \
>   'git commit --amend --add-footer "Acked-by: Bugs Bunny"'

But it wouldn't address my use-case where I get an ack
by email. If I have to dig up the relevant commit(s) by hand
anyway, then what was the point?

> 
> (it should imply `--allow-empty`, of course, and probably even fail if
> anything was staged for commit at that point.) The commit message would
> then look something like
> 
>   exec-after! Fix broken breakage
> 
>   git commit --amend --add-footer "Acked-by: Bugs Bunny"

So if I happen to fetch a branch from someone
and rebase it, stuff gets auto-executed on my local system?
That looks scary. 

> This way would obviously benefit a lot more users.

It might benefit others who have the commit handy but it does not look
like it helps email based workflow.

> For example, you could
> easily say (and alias)
> 
>   git commit --amend --add-footer 'Reviewed-by: Arrested Developer"
> 
> i.e. support all kind of use cases where developers need to slap on
> footers in a quick & easy way.
>
> And the --exec-after option would obviously have *a lot* more use cases
> than just squashing in ACKs.
> 
> Ciao,
> Johannes

So far I only see examples of adding footers. If that's all we can think
up, why code in all this genericity?  All these small scripts scattered
around just make things hard to use, and add security issues.


-- 
MSR
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Christian Neukirchen
Johannes Schindelin  writes:

> How about making it easier to use, and much, much more generic, like this?
>
> 1. introducing an `--add-footer` flag to `git commit` that you could use
> like this:
>
>   git commit --amend --add-footer "Acked-by: Bugs Bunny"

I have a script where I currently do this ala:

GIT_EDITOR="git -c trailer.closes.ifExists=replace interpret-trailers \
--trailer 'Closes: #$PR [via git-merge-pr]' --in-place" \
git commit --quiet --amend

But I think it could be a good addition to porcelain.
(interpret-trailers still feels hard to drive by a script...)

-- 
Christian Neukirchenhttp://chneukirchen.org

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin

2016-04-11 Thread Jeff King
On Sun, Apr 10, 2016 at 11:57:31PM +0100, Ramsay Jones wrote:

> So, the approach taken by patch #1 is to forget about UNIX_PATH_MAX and
> simply use sizeof(address.sun_path) instead!

That's what the existing code in unix-socket.c does. Which makes me
wonder why the index-helper code is not simply calling that.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin

2016-04-11 Thread Ramsay Jones


On 11/04/16 05:20, Torsten Bögershausen wrote:
> On 04/11/2016 12:59 AM, Ramsay Jones wrote:
> The header mentions cygwin, but changes it for linux, BSD and Mac OS as well.
> Is this intentional ?

Yes. I only compile on 32/64-bit linux and 64-bit cygwin these days, so I only
noticed the warnings (yes I wrote error, but it should be warning) on cygwin.
Other systems may suffer the same warnings, of course.

>> Signed-off-by: Ramsay Jones 
>> ---
>>   git-compat-util.h | 17 -
>>   index-helper.c|  4 ++--
>>   read-cache.c  |  2 +-
>>   3 files changed, 3 insertions(+), 20 deletions(-)
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 0e35c13..c90c8c6 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -1043,21 +1043,4 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
>>   #define getc_unlocked(fh) getc(fh)
>>   #endif
>>   -#ifdef __linux__
>> -#define UNIX_PATH_MAX 108
>> -#elif defined(__APPLE__) || defined(BSD)
>> -#define UNIX_PATH_MAX 104
>> -#else
> Can we use a #elif __CYGWIN__ here to fix the re-definition just for cygwi ?

As I said in the cover letter, that is an option - but why bother with
UNIX_PATH_MAX at all?

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Johannes Schindelin
Hi Michael,

On Mon, 11 Apr 2016, Michael S. Tsirkin wrote:

> So far I only see examples of adding footers. If that's all we can think
> up, why code in all this genericity?

Because as far as I can see, the only benefitor of your patches would be
you.

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Default authentication over https?

2016-04-11 Thread Isaac Levy
Hi all,

I use a git server which requires authentication over https. Git seems
determined to always try an unauthenticated request first, slowing
down operations by a couple seconds.

Is there a way to configure git to default to authenticated requests?  Thanks!

Regards,
Isaac Levy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[no subject]

2016-04-11 Thread Michael S. Tsirkin
Cc junio
Bcc: 
Subject: Re: [PATCH 1/4] rebase -i: add ack action
Message-ID: <20160411184535-mutt-send-email-...@redhat.com>
Reply-To: 
In-Reply-To: 

On Mon, Apr 11, 2016 at 05:36:45PM +0200, Johannes Schindelin wrote:
> Hi Michael,
> 
> On Mon, 11 Apr 2016, Michael S. Tsirkin wrote:
> 
> > So far I only see examples of adding footers. If that's all we can think
> > up, why code in all this genericity?
> 
> Because as far as I can see, the only benefitor of your patches would be
> you.
> 
> Ciao,
> Johannes

This seems unlikely.  Just merging the patches won't benefit me directly
- I have maintained them in my tree for a couple of years now with very
little effort.  For sure, I could benefit if they get merged and then
someone improves them further - that was the point of posting them - but
then I'm not the only benefitor.

The workflow including getting acks for patches by email is not handled
well by upstream git right now.  It would surprise me if no one uses it
if it's upstream, as you seem to suggest.  But maybe most people moved
on and just do pull requests instead.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Documentation: clarify signature verification

2016-04-11 Thread Junio C Hamano
KellerFuchs  writes:

> On Sun, Apr 10, 2016 at 11:46:10AM -0700, Junio C Hamano wrote:
>> > --- a/Documentation/merge-options.txt
>> > +++ b/Documentation/merge-options.txt
>> > @@ -89,8 +89,10 @@ option can be used to override --squash.
>> >  
>> >  --verify-signatures::
>> >  --no-verify-signatures::
>> > -  Verify that the commits being merged have good and trusted GPG 
>> > signatures
>> > +  Verify that the commits being merged have good and valid GPG signatures
>> >and abort the merge in case they do not.
>> > +  For instance, when running `git merge --verify-signature remote/branch`,
>> > +  only the head commit on `remote/branch` needs to be signed.
>> 
>> The first part of this change and all other changes are of dubious
>> value, but the last two lines is truly an improvement--it adds
>> missing information people who use the feature may care about.
>
> The reason for the first edit is that “trusted” and “valid” are OpenPGP
>   concepts: a key is trusted if the user set a trust level for it,
>   and a uid is valid if it has been signed by a trusted key [0].

OK, so it is wrong to talk about "trusted" and/or "valid" "GPG
signatures" like the original one.  We should say "... have GPG
signatures that were signed by valid key" (not "trusted" key)?

> Most of my confusion came from this, since it sounded like the signature
>   would only be accepted if it came from a key with a non-zero ownertrust.

Thanks for clarification.  The distinction between trusted and valid
should at least be in the log message and possibly (if we can find a
good way to flow it into the description) added to the documentation.

Perhaps like this?

Verify that the tip commit of the side branch being merged is
signed with a valid key (i.e. a key that is signed by a key that
the user set the trust level as trusted), and abort the merge if
it is not.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Michael S. Tsirkin
Repost, sorry about the noise.

On Mon, Apr 11, 2016 at 05:36:45PM +0200, Johannes Schindelin wrote:
> Hi Michael,
> 
> On Mon, 11 Apr 2016, Michael S. Tsirkin wrote:
> 
> > So far I only see examples of adding footers. If that's all we can think
> > up, why code in all this genericity?
> 
> Because as far as I can see, the only benefitor of your patches would be
> you.
> 
> Ciao,
> Johannes

This seems unlikely.  Just merging the patches won't benefit me directly
- I have maintained them in my tree for a couple of years now with very
little effort.  For sure, I could benefit if they get merged and then
someone improves them further - that was the point of posting them - but
then I'm not the only benefitor.

The workflow including getting acks for patches by email is not handled
well by upstream git right now.  It would surprise me if no one uses it
if it's upstream, as you seem to suggest.  But maybe most people moved
on and just do pull requests instead.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Hardcoded #!/bin/sh in t5532 causes problems on Solaris

2016-04-11 Thread Junio C Hamano
Eric Sunshine  writes:

>> with "printf".  The output from the latter is compared with an
>> expected output, again prepared with "printf" hance lacking the
>
> s/hance/hence/

Thanks
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


meaning of SP on ls-tree docs

2016-04-11 Thread pedro rijo
Hey,

On https://git-scm.com/docs/git-ls-tree#_output_format, the format is
presented as  SP  SP  TAB 

But what is SP? Couldn't find the meaning. Space? System separator?

-- 
Thanks,

Pedro Rijo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Hardcoded #!/bin/sh in t5532 causes problems on Solaris

2016-04-11 Thread Jeff King
On Sun, Apr 10, 2016 at 12:01:30PM -0700, Junio C Hamano wrote:

> > diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
> > index 8e22b03..6dedb1c 100755
> > --- a/t/t1020-subdirectory.sh
> > +++ b/t/t1020-subdirectory.sh
> > @@ -142,9 +142,9 @@ test_expect_success 'GIT_PREFIX for built-ins' '
> > # Use GIT_EXTERNAL_DIFF to test that the "diff" built-in
> > # receives the GIT_PREFIX variable.
> > printf "dir/" >expect &&
> > -   printf "#!/bin/sh\n" >diff &&
> > -   printf "printf \"\$GIT_PREFIX\"" >>diff &&
> > -   chmod +x diff &&
> > +   write_script diff <<-\EOF &&
> > +   printf "%s" "$GIT_PREFIX"
> > +   EOF
> > (
> > cd dir &&
> > printf "change" >two &&
> 
> Regarding this one, I notice that "expect" and "actual" (produced
> later in this script by executing "diff" script) are eventually
> compared by test_cmp, which runs "diff" to show the actual
> differences.  If we are doing this modernization to use write_script
> more, we probably should make "expect" and "actual" text files that
> end with a complete line.

Yeah I wondered about that. And also the fact that the shell script
itself doesn't end in newline. But I think that is just an accident, and
no shell happened to complain (not that I would expect them to, but we
come across enough weirdness around final newlines with tools like sed
and tr, I wouldn't have been surprised).

> -- >8 --
> Subject: t1020: do not overuse printf and use write_script
> 
> The test prepares a sample file "dir/two" with a single incomplete
> line in it with "printf", and also prepares a small helper script
> "diff" to create a file with a single incomplete line in it, again
> with "printf".  The output from the latter is compared with an
> expected output, again prepared with "printf" hance lacking the
> final LF.  There is no reason for this test to be using files with
> an incomplete line at the end, and these look more like a mistake
> of not using
> 
>   printf "%s\n" "string to be written"
> 
> and using
> 
>   printf "string to be written"
> 
> Depending on what would be in $GIT_PREFIX, using the latter form
> could be a bug waiting to happen.  Correct them.
> 
> Also, the test uses hardcoded #!/bin/sh to create a small helper
> script.  For a small task like what the generated script does, it
> does not matter too much in that what appears as /bin/sh would not
> be _so_ broken, but while we are at it, use write_script instead,
> which happens to make the result easier to read by reducing need
> of one level of quoting.

Looks good to me. I suspect you could actually just use:

  echo "$GIT_PREFIX"

in the helper script. That is also not completely safe against arbitrary
bytes in $GIT_PREFIX (due to unportable backslash escapes), though I
suspect it would be fine for the purposes of the test script. Using a
proper printf isn't that many more bytes, though.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: meaning of SP on ls-tree docs

2016-04-11 Thread pedro rijo
ups :) Thanks

2016-04-11 18:15 GMT+01:00 Konstantin Khomoutov :
> On Mon, 11 Apr 2016 18:04:42 +0100
> pedro rijo  wrote:
>
>> On https://git-scm.com/docs/git-ls-tree#_output_format, the format is
>> presented as  SP  SP  TAB 
>>
>> But what is SP? Couldn't find the meaning. Space? System separator?
>
> A single space character, code 0x20 in HEX or 32 in decimal.
> TAB is a single TAB character, code 9.



-- 
Obrigado,

Pedro Rijo
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Hardcoded #!/bin/sh in t5532 causes problems on Solaris

2016-04-11 Thread Jeff King
On Sat, Apr 09, 2016 at 05:37:43PM -0700, Junio C Hamano wrote:

> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index b79f442..d96d0e4 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -555,10 +555,9 @@ test_expect_success 'rebase a detached HEAD' '
>  test_expect_success 'rebase a commit violating pre-commit' '
>  
>   mkdir -p .git/hooks &&
> - PRE_COMMIT=.git/hooks/pre-commit &&
> - echo "#!/bin/sh" > $PRE_COMMIT &&
> - echo "test -z \"\$(git diff --cached --check)\"" >> $PRE_COMMIT &&
> - chmod a+x $PRE_COMMIT &&
> + write_script .git/hooks/pre-commit <<-\EOF &&
> + test -z "$(git diff --cached --check)"
> + EOF

Looks good and is the minimal change. I kind of wonder if the example
would be more clear, though, as just:

  write_script .git/hooks/pre-commit <<-\EOF &&
  exit 1
  EOF
  echo whatever >file1 &&
  ...

I don't think we ever actually need the pre-commit check to pass, as we
simply override it with --no-verify. But I dunno. Maybe people find it
easier to read with a pseudo-realistic example (it took me a minute to
realize the trailing whitespace in the content was important).

It could also stand to clean up its hook with test_when_finished. The
next test resorts to "rm -rf" on the hooks directory at the beginning.
Yuck.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[no subject]

2016-04-11 Thread miwilliams

From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
From: Mike Williams 
Date: Mon, 11 Apr 2016 14:18:39 -0400
Subject: [PATCH 1/1] wt-status: Remove '!!' from  
wt_status_collect_changed_cb


The wt_status_collect_changed_cb function uses an extraneous double  
negation (!!)

when determining whether or not a submodule has new commits.

Signed-off-by: Mike Williams 
---
 wt-status.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index ef74864..b955179 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct  
diff_queue_struct *q,

d->worktree_status = p->status;
d->dirty_submodule = p->two->dirty_submodule;
if (S_ISGITLINK(p->two->mode))
-   d->new_submodule_commits = !!hashcmp(p->one->sha1, 
p->two->sha1);
+   d->new_submodule_commits = hashcmp(p->one->sha1, 
p->two->sha1);
}
 }

--
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: your mail

2016-04-11 Thread Jeff King
On Mon, Apr 11, 2016 at 07:04:23PM +, miwilli...@google.com wrote:

> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams 
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb

These bits (minus the initial "From ..." line) should go into your
actual email headers. As it is, your email has no subject line.

> The wt_status_collect_changed_cb function uses an extraneous double
> negation (!!) when determining whether or not a submodule has new
> commits.
> [...]
> - d->new_submodule_commits = !!hashcmp(p->one->sha1, 
> p->two->sha1);
> + d->new_submodule_commits = hashcmp(p->one->sha1, 
> p->two->sha1);

It's not extraneous. hashcmp() returns 0 for equality, but an arbitrary
positive or negative value depending on how the two arguments differ.
The assigned "new_submodule_commits" is a bitfield of size 1. So the
"!!" is normalizing the value into "0" or "1".

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: none

2016-04-11 Thread Matthieu Moy
miwilli...@google.com writes:

> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams 
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb
>
> The wt_status_collect_changed_cb function uses an extraneous double
> negation (!!)
> when determining whether or not a submodule has new commits.

It's not just a double negation, it's a way to ensure that the value is
0 or 1 (it's a relatively common idiom in C at least in Git's codebase).

new_submodule_commits is a 1-bit bitfield, and you don't want to assign
anything other than 1 or 0 (or you'll get modulo 2^n semantics, with
n==1).

So the old code is correct and your patch would introduce a bug.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Central Bank Governor

2016-04-11 Thread Godwin Emefiele
Dear Sir,

We had a meeting with the President Rtd General Muhamad Buhari and he has 
ordered that all outstanding foreign payment be paid to all contractors and 
beneficiaries as agreed by the office of the President and National Assembly 
and also to make sure that all the contractors and beneficiaries received their 
payment within seven (7) working days and your name appeared among the 
beneficiaries of this second quarter pay for the year 2016, So you are 
therefore require to forward the following to this office:-

Your Names and Full Address.
Your Telephone number.
Your international passport or your ID.
Your bank details.

As soon as we received these information we will commence with the transfer of 
your fund to your account as instructed by the President of the Federal 
republic of Nigeria.

Mr. Godwin Emefiele
Central Bank Governor (CBN)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Junio C Hamano
"Michael S. Tsirkin"  writes:

> Repost, sorry about the noise.
>
> On Mon, Apr 11, 2016 at 05:36:45PM +0200, Johannes Schindelin wrote:
>> Hi Michael,
>> 
>> On Mon, 11 Apr 2016, Michael S. Tsirkin wrote:
>> 
>> > So far I only see examples of adding footers. If that's all we can think
>> > up, why code in all this genericity?
>> 
>> Because as far as I can see, the only benefitor of your patches would be
>> you.
>> 
>> Ciao,
>> Johannes
>
> This seems unlikely.  Just merging the patches won't benefit me directly
> - I have maintained them in my tree for a couple of years now with very
> little effort.  For sure, I could benefit if they get merged and then
> someone improves them further - that was the point of posting them - but
> then I'm not the only benefitor.
>
> The workflow including getting acks for patches by email is not handled
> well by upstream git right now.  It would surprise me if no one uses it
> if it's upstream, as you seem to suggest.  But maybe most people moved
> on and just do pull requests instead.

I doubt I would use this in its current form myself.

Patch series I receive are all queued on their own separate topic
branches, and having to switch branches only to create a fake empty
commit to record received Acked-by and Reviewed-by is a chore that
serves only half of what needs to be done.  Once I decide to switch
back to the topic branch after receiving Acked-by and Reviewed-by,
I'd rather "rebase -i" to directly record them at that point, with
"reword".

If the "trailers" stuff is packaged into an easier-to-use format to
use with "git commit --amend", I may use that together with "exec"
to automatically add these while doing so, but again, I do not see
any need for fake empty commits out of received e-mails in the
resulting workflow.

That does not at all mean nobody other than Michael would use it,
though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Michael S. Tsirkin
On Mon, Apr 11, 2016 at 12:48:22PM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin"  writes:
> 
> > Repost, sorry about the noise.
> >
> > On Mon, Apr 11, 2016 at 05:36:45PM +0200, Johannes Schindelin wrote:
> >> Hi Michael,
> >> 
> >> On Mon, 11 Apr 2016, Michael S. Tsirkin wrote:
> >> 
> >> > So far I only see examples of adding footers. If that's all we can think
> >> > up, why code in all this genericity?
> >> 
> >> Because as far as I can see, the only benefitor of your patches would be
> >> you.
> >> 
> >> Ciao,
> >> Johannes
> >
> > This seems unlikely.  Just merging the patches won't benefit me directly
> > - I have maintained them in my tree for a couple of years now with very
> > little effort.  For sure, I could benefit if they get merged and then
> > someone improves them further - that was the point of posting them - but
> > then I'm not the only benefitor.
> >
> > The workflow including getting acks for patches by email is not handled
> > well by upstream git right now.  It would surprise me if no one uses it
> > if it's upstream, as you seem to suggest.  But maybe most people moved
> > on and just do pull requests instead.
> 
> I doubt I would use this in its current form myself.
> 
> Patch series I receive are all queued on their own separate topic
> branches, and having to switch branches only to create a fake empty
> commit to record received Acked-by and Reviewed-by is a chore that
> serves only half of what needs to be done.

Interesting. An empty commit would be rather easy to create on any
branch, not just the current one, using git-commit-tree.
Does it sounds interesting if I teach
git ack to get an active branch as a parameter?


> Once I decide to switch
> back to the topic branch after receiving Acked-by and Reviewed-by,
> I'd rather "rebase -i" to directly record them at that point, with
> "reword".
> 
> If the "trailers" stuff is packaged into an easier-to-use format to
> use with "git commit --amend", I may use that together with "exec"
> to automatically add these while doing so, but again, I do not see
> any need for fake empty commits out of received e-mails in the
> resulting workflow.
> 
> That does not at all mean nobody other than Michael would use it,
> though.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] rebase -i: add ack action

2016-04-11 Thread Matthieu Moy
"Michael S. Tsirkin"  writes:

> On Mon, Apr 11, 2016 at 12:48:22PM -0700, Junio C Hamano wrote:
>> "Michael S. Tsirkin"  writes:
>> 
>> > Repost, sorry about the noise.
>> >
>> > On Mon, Apr 11, 2016 at 05:36:45PM +0200, Johannes Schindelin wrote:
>> >> Hi Michael,
>> >> 
>> >> On Mon, 11 Apr 2016, Michael S. Tsirkin wrote:
>> >> 
>> >> > So far I only see examples of adding footers. If that's all we can think
>> >> > up, why code in all this genericity?
>> >> 
>> >> Because as far as I can see, the only benefitor of your patches would be
>> >> you.
>> >> 
>> >> Ciao,
>> >> Johannes
>> >
>> > This seems unlikely.  Just merging the patches won't benefit me directly
>> > - I have maintained them in my tree for a couple of years now with very
>> > little effort.  For sure, I could benefit if they get merged and then
>> > someone improves them further - that was the point of posting them - but
>> > then I'm not the only benefitor.
>> >
>> > The workflow including getting acks for patches by email is not handled
>> > well by upstream git right now.  It would surprise me if no one uses it
>> > if it's upstream, as you seem to suggest.  But maybe most people moved
>> > on and just do pull requests instead.
>> 
>> I doubt I would use this in its current form myself.
>> 
>> Patch series I receive are all queued on their own separate topic
>> branches, and having to switch branches only to create a fake empty
>> commit to record received Acked-by and Reviewed-by is a chore that
>> serves only half of what needs to be done.
>
> Interesting. An empty commit would be rather easy to create on any
> branch, not just the current one, using git-commit-tree.

This "modify a branch without checking-it out" makes me think of "git
notes". It may make sense to teach "git rebase -i" to look for notes in
rebased commits and append them to the commit message when applying.
Just an idea, not necessarily a good one ;-).

> Does it sounds interesting if I teach
> git ack to get an active branch as a parameter?

I think "ack" is not a good name for this feature: you use it to append
"Acked-by", but it can be used to append any trailer (for example,
Reviewed-by: would make complete sense too). I think using a better name
would help the discussion (to remove the "it's my use-case" biais).
Perhaps "append"?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin

2016-04-11 Thread David Turner
On Mon, 2016-04-11 at 09:33 -0400, Jeff King wrote:
> On Sun, Apr 10, 2016 at 11:57:31PM +0100, Ramsay Jones wrote:
> 
> > So, the approach taken by patch #1 is to forget about UNIX_PATH_MAX
> > and
> > simply use sizeof(address.sun_path) instead!
> 
> That's what the existing code in unix-socket.c does. Which makes me
> wonder why the index-helper code is not simply calling that.

Because I didn't notice it at the time.  Duy pointed it out on another
comment on this series; I'll fix it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


clone hang prevention / timeout?

2016-04-11 Thread Jason Vas Dias
It appears GIT has no way of specifying a timeout for a clone operation -
if the server decides not to complete a get request, the clone can
hang forever -
is this correct ?
This appears to be what I am seeing, in a script that is attempting to do many
successive clone operations, eg. of
git://anongit.freedesktop.org/xorg/* , the script
occasionally hangs in a clone - I can see with netstat + strace that the TCP
connection is open and GIT is trying to read .
Is there any option I can specify to get the clone to timeout, or do I manually
have to strace the git process and send it a signal after a hang is detected?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] index-helper: take extra care with readlink

2016-04-11 Thread David Turner
On Mon, 2016-04-11 at 00:03 +0100, Ramsay Jones wrote:
> It took me a few minutes to convince myself that, if index-helper is
> the only writer for the symlink, that the call to readlink would
> result in a properly NULL terminated string. This relies on the
> initialization of the address variable setting each byte of the
> 'struct sockaddr_un' to zero and the contents of the symlink being
> no more than 'sizeof(address.sun_path) - 1' bytes long.
> 
> In order to make the call easier to reason about, introduce a wrapper
> function, read_link(), which copes with overlong symlinks and ensures
> correct NULL termination.
> 
> Signed-off-by: Ramsay Jones 
> ---
>  read-cache.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/read-cache.c b/read-cache.c
> index c7053d8..39330a1 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1722,12 +1722,23 @@ static void post_read_index_from(struct
> index_state *istate)
>   tweak_untracked_cache(istate);
>  }
>  
> +static int read_link(const char *path, char *buf, size_t bufsize)
> +{
> + int len;
> +
> + len = readlink(path, buf, bufsize);
> + if (len < 0 || len >= bufsize)
> + return -1;
> + buf[len] = '\0';
> + return 0;
> +}
> +
>  int connect_to_index_helper(void)
>  {
>   struct sockaddr_un address = {0};
>   int fd;
>  
> - if (readlink(git_path("index-helper.path"),
> address.sun_path,
> + if (read_link(git_path("index-helper.path"),
> address.sun_path,
>sizeof(address.sun_path)) < 0)
>   return -1;
>  


Will squash.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin

2016-04-11 Thread David Turner
On Mon, 2016-04-11 at 14:30 +0100, Ramsay Jones wrote:
> 
> On 11/04/16 05:20, Torsten Bögershausen wrote:
> > On 04/11/2016 12:59 AM, Ramsay Jones wrote:
> > The header mentions cygwin, but changes it for linux, BSD and Mac
> > OS as well.
> > Is this intentional ?
> 
> Yes. I only compile on 32/64-bit linux and 64-bit cygwin these days,
> so I only
> noticed the warnings (yes I wrote error, but it should be warning) on
> cygwin.
> Other systems may suffer the same warnings, of course.
> 
> > > Signed-off-by: Ramsay Jones 
> > > ---
> > >   git-compat-util.h | 17 -
> > >   index-helper.c|  4 ++--
> > >   read-cache.c  |  2 +-
> > >   3 files changed, 3 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/git-compat-util.h b/git-compat-util.h
> > > index 0e35c13..c90c8c6 100644
> > > --- a/git-compat-util.h
> > > +++ b/git-compat-util.h
> > > @@ -1043,21 +1043,4 @@ struct tm *git_gmtime_r(const time_t *,
> > > struct tm *);
> > >   #define getc_unlocked(fh) getc(fh)
> > >   #endif
> > >   -#ifdef __linux__
> > > -#define UNIX_PATH_MAX 108
> > > -#elif defined(__APPLE__) || defined(BSD)
> > > -#define UNIX_PATH_MAX 104
> > > -#else
> > Can we use a #elif __CYGWIN__ here to fix the re-definition just
> > for cygwi ?
> 
> As I said in the cover letter, that is an option - but why bother
> with
> UNIX_PATH_MAX at all?

Will remove UNIX_PATH_MAX in favor of sizeof().

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] index-helper: convert strncpy to memcpy

2016-04-11 Thread David Turner
On Mon, 2016-04-11 at 00:02 +0100, Ramsay Jones wrote:
> see commit eddda371 ("convert strncpy to memcpy", 24-09-2015).
> 
> Signed-off-by: Ramsay Jones 
> ---
>  index-helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/index-helper.c b/index-helper.c
> index 4a8e2ae..00f286a 100644
> --- a/index-helper.c
> +++ b/index-helper.c
> @@ -317,7 +317,7 @@ static int setup_socket(const char *socket_path)
>   return -1;
>  
>   address.sun_family = AF_UNIX;
> - strncpy(address.sun_path, socket_path,
> sizeof(address.sun_path));
> + memcpy(address.sun_path, socket_path, len+1); /* include
> '\0' */
>  
>   if (bind(fd, (struct sockaddr *) &address, sizeof(address)))
>   die_errno(_("failed to bind to socket %s"),
> socket_path);

Thanks. 

I'm actually going to drop this patch because I'm switching to the unix
-socket.c helpers, so the affected code no longer exists.  But I
appreciate the thought anyway.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 03/16] index-helper: new daemon for caching index and related stuff

2016-04-11 Thread David Turner
On Fri, 2016-04-08 at 18:16 -0400, David Turner wrote:
> And SHM on Macs works a bit differently than on Linux in at least two
> irritating ways. 
> 
> So, uh, new version to come once I actually make it work on Mac.
> Probably Monday.

I was chatting with a friend about this and he mentioned that SHM does
not really fit well into the Unix "everything is a file" model.  It
lives in a separate namespace, and still requires most of the file-like
operations just with funny names and a separate namespace: shm_open,
shm_unlink.  This weirdness is something I noticed in my porting work:
on OS X, a shm name can only be 32 bytes long, requiring weird hacks.
And on OSX, fstat on a shm fd is rounded up to the page size (!). 
 There may also be other portability issues that I have not yet
discovered.

Instead, my friend suggests that we should just use files.  For
instance, we could do $TMPDIR/$index_helper_pid/shm-index.$sha.  

(I'm proposing $TMPDIR because it's cleaned up on reboot so we don't
need any manual intervention or complicated gc schemes)

What do folks think of this?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Documentation: clarify signature verification

2016-04-11 Thread KellerFuchs
On Mon, Apr 11, 2016 at 09:41:22AM -0700, Junio C Hamano wrote:
> KellerFuchs  writes:
> > The reason for the first edit is that “trusted” and “valid” are OpenPGP
> >   concepts: a key is trusted if the user set a trust level for it,
> >   and a uid is valid if it has been signed by a trusted key [0].
> 
> OK, so it is wrong to talk about "trusted" and/or "valid" "GPG
> signatures" like the original one.  We should say "... have GPG
> signatures that were signed by valid key" (not "trusted" key)?

Well, the GnuPG documentation also talks of valid signatures,
  and it is a convenient short-hand:

  https://www.gnupg.org/documentation/manuals/gpgme/Verify.html
  
On the other hand, being more explicit here cannot hurt.

> Thanks for clarification.  The distinction between trusted and valid
> should at least be in the log message and possibly (if we can find a
> good way to flow it into the description) added to the documentation.

Ok.  I will have a second go at the patch (with the split you requested,
  a more explicit description and an explanation in the commit msg).

What is the prefered way to send a second version of a patchset here?
Just git-email-ing it here In-Reply-To the first mail?

> Verify that the tip commit of the side branch being merged is
> signed with a valid key (i.e. a key that is signed by a key that
> the user set the trust level as trusted), and abort the merge if
> it is not.

I would rather see something like

> Verify that the tip commit of the side branch being merged is
> signed with a valid key, i.e. a key that has a valid uid: in the
> default trust model, this means it has been signed by a trusted key.
> If the tip commit of the side branch is not signed with a valid key,
> the merge is aborted.

It's unfortunately more verbose, but I don't want to make promises
  about GnuPG's behaviour that depends on the user's configuration.


Best,

  kf
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] index-helper: fix UNIX_PATH_MAX redefinition error on cygwin

2016-04-11 Thread Jeff King
On Mon, Apr 11, 2016 at 05:29:22PM -0400, David Turner wrote:

> On Mon, 2016-04-11 at 09:33 -0400, Jeff King wrote:
> > On Sun, Apr 10, 2016 at 11:57:31PM +0100, Ramsay Jones wrote:
> > 
> > > So, the approach taken by patch #1 is to forget about UNIX_PATH_MAX
> > > and
> > > simply use sizeof(address.sun_path) instead!
> > 
> > That's what the existing code in unix-socket.c does. Which makes me
> > wonder why the index-helper code is not simply calling that.
> 
> Because I didn't notice it at the time.  Duy pointed it out on another
> comment on this series; I'll fix it.

Thanks. I _think_ it should do everything you need already, but I'm
happy to review any tweaks you need to make to it.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-stash: Don't GPG sign when stashing changes

2016-04-11 Thread Daurnimator
On 7 April 2016 at 18:19, Johannes Schindelin
 wrote:
> Hi,
>
> you dropped the Cc: list. So most likely Cameron won't get your mail nor
> any response to your mail.

I originally replied via the gmane web interface, apparently it
doesn't CC the original sender.
CCd now.

> On Thu, 7 Apr 2016, daurnimator wrote:
>
>> Cameron Currie  cameroncurrie.net> writes:
>> > This is helpful for folks with commit.gpgsign = true in their .gitconfig.
>>
>> > https://github.com/git/git/pull/186
>>
>> I too would like this.
>> Bumping due to lack of attention.
>
> It lacks a Sign-off, our convention is to continue in lower-case after the
> colon in the commit's subject, and I think that it would be good to write
> so much as one paragraph in the commit message.
>
> Ciao,
> Johannes

Cameron, able you able to complete this?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 0 bot for Git

2016-04-11 Thread Stefan Beller
Resending as plain text. (I need to tame my mobile)

On Mon, Apr 11, 2016 at 7:51 AM, Stefan Beller  wrote:
> Hi Greg,
>
> Thanks for your talk at the Git Merge 2016!
> The Git community uses the same workflow as the kernel. So we may be
> interested in the 0 bot which could compile and test each patch on the list.
> Could you put us in touch with the authors/maintainers of said tool?
>
> Unlike the kernel we would not need hardware testing and we're low traffic
> compared to the kernel, which would make it easier to set it up.
>
> A healthier Git would help the kernel long term as well.
>
> Thanks,
> Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re:

2016-04-11 Thread Stefan Beller
On Mon, Apr 11, 2016 at 12:04 PM,   wrote:
> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams 
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb
>
> The wt_status_collect_changed_cb function uses an extraneous double negation
> (!!)

How is an !! errornous?

It serves the purpose to map an integer value(-1,0,1,2,3,4)
to a boolean (0,1, or a real bit in a bit field).

> when determining whether or not a submodule has new commits.
>
> Signed-off-by: Mike Williams 
> ---
>  wt-status.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index ef74864..b955179 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct
> diff_queue_struct *q,
> d->worktree_status = p->status;
> d->dirty_submodule = p->two->dirty_submodule;
> if (S_ISGITLINK(p->two->mode))
> -   d->new_submodule_commits = !!hashcmp(p->one->sha1,
> p->two->sha1);
> +   d->new_submodule_commits = hashcmp(p->one->sha1,
> p->two->sha1);
> }
>  }
>
> --
> 2.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] upload-pack: Exit when server finishes sending shallow-update in stateless RPC mode

2016-04-11 Thread Stan Hu
In the stateless RPC case, the server should respond to the client's depth
request with the set of commits which are no deeper than the desired
depth. Once this finishes, the server should terminate and receive the reply
in another POST request.

Previously the server would sit idle and die when it detected the client
closed the connection.

Signed-off-by: Stan Hu 
---
 upload-pack.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/upload-pack.c b/upload-pack.c
index b3f6653..4fb1e60 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -676,6 +676,8 @@ static void receive_needs(void)
register_shallow(object->oid.hash);
}
packet_flush(1);
+   if (stateless_rpc)
+   exit(0);
} else
if (shallows.nr > 0) {
int i;
-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] upload-pack: Exit when server finishes sending shallow-update in stateless RPC mode

2016-04-11 Thread Torsten Bögershausen

On 04/12/2016 06:55 AM, Stan Hu wrote:

In the stateless RPC case, the server should respond to the client's depth
request with the set of commits which are no deeper than the desired
depth. Once this finishes, the server should terminate and receive the reply
in another POST request.

Previously the server would sit idle and die when it detected the client
closed the connection.

Some loose thoughts about the wording(s):
the server did not terminate (it's a machine, a hardware), a process 
terminated.

And it did not "die" either, it terminates gracefully.
How about something like this:

In the stateless RPC case, the server responds to the client's depth
request. Once this finishes, the server should terminate.
The reply is and received in another POST request.

Previously the server would sit idle and terminates when it detected the client
closed the connection.

---

But, but,
there is may be more.
What happens, when the server process calls exit(0) ?
Is there a guarantee that the data is send out on the socket?
Or is the exit(0) ripping out all the data in the TCP send buffers,
and the client may, or not may, receive the complete packets.

Beside that, a close() of a socket means that the socket is going into
time_wait() state.
Is this desirable for a server to have many sockets in time_wait ?
(This is out of my head, I may be wrong)

Could it be that the client is broken ?
Could you elaborate this a little bit more ?



Signed-off-by: Stan Hu 
---
  upload-pack.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/upload-pack.c b/upload-pack.c
index b3f6653..4fb1e60 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -676,6 +676,8 @@ static void receive_needs(void)
register_shallow(object->oid.hash);
}
packet_flush(1);
+   if (stateless_rpc)
+   exit(0);
} else
if (shallows.nr > 0) {
int i;


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 0 bot for Git

2016-04-11 Thread Greg KH
On Mon, Apr 11, 2016 at 09:29:59PM -0700, Stefan Beller wrote:
> Resending as plain text. (I need to tame my mobile)
> 
> On Mon, Apr 11, 2016 at 7:51 AM, Stefan Beller  wrote:
> > Hi Greg,
> >
> > Thanks for your talk at the Git Merge 2016!
> > The Git community uses the same workflow as the kernel. So we may be
> > interested in the 0 bot which could compile and test each patch on the list.
> > Could you put us in touch with the authors/maintainers of said tool?
> >
> > Unlike the kernel we would not need hardware testing and we're low traffic
> > compared to the kernel, which would make it easier to set it up.

We don't get much, if any, real hardware testing from the 0-day bot,
it's just lots and lots of builds and static testing tools.

You can reach the developers of it at:
kbuild test robot 

Hope this helps,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html