Re: [PATCH 00/12] object_id part 9

2017-07-04 Thread Junio C Hamano 
"brian m. carlson"  writes:

> It is possible there will be some other conflicts with in flight topics,
> as get_sha1 is commonly used in the codebase.  This is unavoidable to
> some extent, but should be kept in mind.  My experience is that usually
> the required changes for conversion are minimal.

Thanks. 

It did have a few conflicts in submodule area and sequencer, but
they were (hopefully) trivial to resolve.  The result is queued at
the tip of 'pu'.  It seems to pass the tests locally and also at
Travis.

Re: [PATCH v3 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

2017-07-04 Thread Junio C Hamano 
Stefan Beller  writes:

> On Tue, Jul 4, 2017 at 3:50 PM, Ævar Arnfjörð Bjarmason
>  wrote:
>
>>
>> I think some invocation of "git submodule update ???" will do the same,
>> but I can't from the docs see what that is right now.
>
> '--remote' is what you are looking for.
>
> When we have the branch field configured, the workflow for *creating*
> the patch sent
> to Junio might be different than it currently is. Currently, you would
> send a patch that is
> produced as:
>
>   git -C sha1collisiondetection pull origin master
>   git add sha1collisiondetection
>   git commit -m "serious reasoning in the commit message"
>
> This could be 'simplified' to
>
>   git submodule update --remote
>   git add sha1collisiondetection
>   git commit -m "serious reasoning in the commit message"
>
> but as we had different branches in the submodule field,
> I wonder how much of an ease this is.
>
> For Junio the workflow stays the same, as he would just
> apply the patch, so I understand why he might see the
> branch field as pollution.

My reaction was more about "the rest of us", not me or those who
choose to bind a new/different commit in the submodule to the
superproject.

I was recalling a wish by submodule users in a distant past that
lets "submodule update" to detach to the tip of the named branch in
the submodule, regardless of what commit the superproject points at
with its gitlink.  

When those merely following along with this project did "pull &&
submodule update", I do not want the submodule directory to check
out the commit that happens to be at the tip of their 'master'
branch.  If "submodule update" requires an explicit "--remote"
option to work in that way, then my worry is allayed, greatly.

Thanks.

Re: Truncating HEAD reflog on branch move

2017-07-04 Thread Junio C Hamano 
Jeff King  writes:

> On Tue, Jul 04, 2017 at 07:58:06PM +, brian m. carlson wrote:
>
>> > And here's one more patch on top of those that's necessary to get the
>> > tests to pass (I don't expect anybody to necessarily be applying this
>> > slow string of patches; it's just to show the direction I'm looking in).
>> 
>> I've looked at your original patch, which modified reflog-walk.c, and it
>> does fix the issue.  I'm happy to send in a patch with that and a test
>> (provided you're okay with me adding your sign-off), or if you wanted to
>> send in something a bit more complete, like the series of patches here,
>> that's fine, too.
>
> I've been on vacation for the past week, but wrapping this up is on my
> todo. I'll try to get to it tonight.

;-)  Welcome back.  

Hopefully I haven't advanced topics to 'next' that are sub-par
during your absence.

Re: "git intepret-trailers" vs. "sed script" to add the signature

2017-07-04 Thread Junio C Hamano 
Kaartic Sivaraam  writes:

> On Mon, 2017-07-03 at 09:58 -0700, Junio C Hamano wrote:
>> Kaartic Sivaraam  writes:
>> 
>> > I'll send a typical patch that uses "git interpret-headers" as a
>> > follow-up.
>> 
>> When you say a "typical" patch, what do you exactly mean?  Does
>> anybody else send typical patches (or atypical ones for that matter)
>> to the list?
>> 
> I apologise for the inconsistent wordings. I try to mean a patch which
> I'm not sure is acceptable (or) not. I guess that's [PATCH/RFC] in the
> language of this list. I'm not acquainted to the wordings as I'm an
> "off-list" person trying to help and get help! I'll try to use
> consistent wordings as far as I could. Once again, please excuse my
> ignorance.

Oh no need to apologise for "ignorance"; that goes both ways. I
wasn't familiar with a phrase you used "a typical patch" (which I
suspected that is something you may be used to from your involvement
in other development community) and showing _my_ ignorance and
asking for help to clarify, so that both of us can understand each
other better.

My reading of your response is that it is a normal patch that
proposes a change, as opposed to the final version of a patch meant
for inclusion, after it has been discussed here and everybody
supports---let me know if I am still not reading you correctly.

Thanks.

Re: [PATCH v3 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

2017-07-04 Thread Stefan Beller 
On Tue, Jul 4, 2017 at 3:50 PM, Ævar Arnfjörð Bjarmason
 wrote:

>
> I think some invocation of "git submodule update ???" will do the same,
> but I can't from the docs see what that is right now.

'--remote' is what you are looking for.

When we have the branch field configured, the workflow for *creating*
the patch sent
to Junio might be different than it currently is. Currently, you would
send a patch that is
produced as:

  git -C sha1collisiondetection pull origin master
  git add sha1collisiondetection
  git commit -m "serious reasoning in the commit message"

This could be 'simplified' to

  git submodule update --remote
  git add sha1collisiondetection
  git commit -m "serious reasoning in the commit message"

but as we had different branches in the submodule field,
I wonder how much of an ease this is.

For Junio the workflow stays the same, as he would just
apply the patch, so I understand why he might see the
branch field as pollution.

Re: What's cooking in git.git (Jun 2017, #09; Fri, 30)

2017-07-04 Thread Stefan Beller 
On Mon, Jul 3, 2017 at 11:02 PM, Junio C Hamano  wrote:
> "Philip Oakley"  writes:
>
>> From: "Junio C Hamano" 
>>
>>> I am not sure what you are asking.  Is this the command you are
>>> looking for?
>>>
>>> $ grep -e "^[[*]" whats-cooking.txt
>>>
>> Ah, thanks.
>>
>> It does presume that one has extracted the email to the text file,
>> which is easier on some systems and mail clients than others ;-)
>
> Perhaps you would want to find an e-mail client that can handle text
> files better, then? ;-)
>
> If you are cloning and following along my tree from one of these
> places:
>
> git://repo.or.cz/alt-git.git/
> git://git.kernel.org/pub/scm/git/git.git/
> git://github.com/git/git.git/
>
> the 'todo' branch in these repositories keeps track of the
> whats-cooking.txt report (among other things), so you can also:
>
> $ git cat-file -p origin/todo:whats-cooking.txt |
>   grep -e "^[[*]"
>
> to get the same effect.
>

FWIW:
I have a script in my git dir,

cat git/cookreport.sh
git -C Meta pull origin todo
Meta/cook -w

with Meta being a repo tracking one of the URLs above.

I might add  a | grep -e "^[[*]" or add some scripting to verbose
report my own topics.

Thanks,
Stefan

Re: [PATCH v3 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

2017-07-04 Thread Ævar Arnfjörð Bjarmason 

On Tue, Jul 04 2017, Junio C. Hamano jotted:

> Ævar Arnfjörð Bjarmason  writes:
>
 diff --git a/.gitmodules b/.gitmodules
 new file mode 100644
 index 00..cbeebdab7a
 --- /dev/null
 +++ b/.gitmodules
 @@ -0,0 +1,4 @@
 +[submodule "sha1collisiondetection"]
 +  path = sha1collisiondetection
 +  url = https://github.com/cr-marcstevens/sha1collisiondetection.git
 +  branch = master
>>>
>>> Do we need to say this "branch" bit?
>>
>> Yes, it's to make future updates easier, see b928922727 ("submodule add:
>> If --branch is given, record it in .gitmodules", 2012-12-19).
>
> Why?  It's not like we want to _follow_ the 'master' branch of that
> sha1collisiondetection repository.  We declare that a specific
> commit from the (sub)module is suited for our project, and do not
> really care to automatically update from whatever happens to be at
> the tip of 'master' there.

I'm honestly at a bit of a loss as to to what the confusion is here. So
to try to unravel that let's start from square one, explaining some
things you surely know already, but hopefully clearing this up.

> It's not like we want to _follow_ the 'master' branch of that
> sha1collisiondetection repository.

Git has no support at all for submodules that somehow follow an upstream
branch in the SVN sense of remotes, so no, that's not what putting
"branch" into the .gitmodules config means at all.

> We declare that a specific commit from the (sub)module is suited for
> our project, and do not really care to automatically update from
> whatever happens to be at the tip of 'master' there.

There is no automatic updating involved. The "master" branch here is
just metadata. If and when we bump the sha1collisiondetection submodule
that's going to be from the master branch, so by recording it we save
ourselves one step (in theory) by issuing some "pull updates from the
branch we always update from" command, rather than being at a loss as to
where we should go from the currently detached ref to N potential
upstream branches to update from.

Now, it seems git-submodule's tooling for doing this is still rather
crappy, but I think that's the idea, maybe I'm just holding it wrong.

Before git ever got this "branch" key in .gitmodules I'd added it to my
own aliases (and they're happily compatible with git-submodule). It
still (for me) works better than what git-submodule does:

$ git config alias.sm-mainbranch
!git config --file ../.gitmodules submodule.$NAME.branch || git describe 
--all --always | sed 's!^heads/!!'
$ git config alias.sm-pull-all
!git submodule foreach 'git checkout $(NAME=$name git sm-mainbranch) && git 
pull'

So in a repo with submodules I can simply run "git sm-pull-all" and
it'll update them all to the branch they're tracking, and at this point
I can "git add" them and review the updates.

I think some invocation of "git submodule update ???" will do the same,
but I can't from the docs see what that is right now.

In any case, if and when I/others figure that out the metadata will be
there, saving us one step in updating this in the future.

>>
>>> Other than that looks good to me.
>>>
>>> Thanks.

Re: Truncating HEAD reflog on branch move

2017-07-04 Thread Jeff King 
On Tue, Jul 04, 2017 at 09:27:09PM +, brian m. carlson wrote:

> On Tue, Jul 04, 2017 at 05:24:08PM -0400, Jeff King wrote:
> > On Tue, Jul 04, 2017 at 07:58:06PM +, brian m. carlson wrote:
> > > I've looked at your original patch, which modified reflog-walk.c, and it
> > > does fix the issue.  I'm happy to send in a patch with that and a test
> > > (provided you're okay with me adding your sign-off), or if you wanted to
> > > send in something a bit more complete, like the series of patches here,
> > > that's fine, too.
> > 
> > I've been on vacation for the past week, but wrapping this up is on my
> > todo. I'll try to get to it tonight.
> 
> Okay, if it's on your radar, that's fine.  Certainly you should enjoy
> your vacation and not feel obligated to work on Git unless you want to.

Heh, no, "tonight" is the first work day after the vacation ends. Once
the fireworks are done, it's back to work. ;)

-Peff

Re: Truncating HEAD reflog on branch move

2017-07-04 Thread brian m. carlson 
On Tue, Jul 04, 2017 at 05:24:08PM -0400, Jeff King wrote:
> On Tue, Jul 04, 2017 at 07:58:06PM +, brian m. carlson wrote:
> > I've looked at your original patch, which modified reflog-walk.c, and it
> > does fix the issue.  I'm happy to send in a patch with that and a test
> > (provided you're okay with me adding your sign-off), or if you wanted to
> > send in something a bit more complete, like the series of patches here,
> > that's fine, too.
> 
> I've been on vacation for the past week, but wrapping this up is on my
> todo. I'll try to get to it tonight.

Okay, if it's on your radar, that's fine.  Certainly you should enjoy
your vacation and not feel obligated to work on Git unless you want to.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: Truncating HEAD reflog on branch move

2017-07-04 Thread Jeff King 
On Tue, Jul 04, 2017 at 07:58:06PM +, brian m. carlson wrote:

> > And here's one more patch on top of those that's necessary to get the
> > tests to pass (I don't expect anybody to necessarily be applying this
> > slow string of patches; it's just to show the direction I'm looking in).
> 
> I've looked at your original patch, which modified reflog-walk.c, and it
> does fix the issue.  I'm happy to send in a patch with that and a test
> (provided you're okay with me adding your sign-off), or if you wanted to
> send in something a bit more complete, like the series of patches here,
> that's fine, too.

I've been on vacation for the past week, but wrapping this up is on my
todo. I'll try to get to it tonight.

-Peff

Re: Why doesn't merge fail if message has only sign-off?

2017-07-04 Thread Kaartic Sivaraam 
On Mon, 2017-07-03 at 10:21 -0700, Junio C Hamano wrote:
> I think that it is not by design that it doesn't fail.  It's not
> like we decided to allow s-o-b only merge because we found a reason
> why it is a good idea to do so.
> 
> So I do not think anybody minds too deeply if somebody came up a
> patch to "fix" it.  It's just that nobody tried to create such a
> silly merge in real life so far (I do not think you did, either--you
> found this out by playing around trying to find corner cases, no?)
> 
Yes and no. I found this out while playing around with the "insert
notes in the commit template" patch I sent previously. I wasn't trying
to find corner cases, though.

-- 
Kaartic

Re: Help needed for solving a few issues with building git

2017-07-04 Thread Kaartic Sivaraam 
On Mon, 2017-07-03 at 11:13 -0700, Junio C Hamano wrote:
> Adding HTTPS support
> > > 
> > > I tried to add HTTP/HTTPS support to the custom built version
> > > for which
> > > AFAIK 'git' depends on 'curl'. I tried providing the location
> > > of the
> > > curl source in the Makefile using the following line after
> > > reading the
> > > instructions in the Makefile.
> > > 
> > > CURLDIR=/path/to/curl/source
> > 
>  Shouldn't this point at an installed location (iow, we do not build
>  curl from the source while building Git)?
>  
>  # Define CURLDIR=/foo/bar if your curl header and library files
>  are in
>  # /foo/bar/include and /foo/bar/lib directories.
I tried pointing it to the installed location, it doesn't seem to be
working. To elaborate a little on what I did,

* I installed the "libcurl4-openssl-dev" package b
* I found that the 'include' directory to be present  at
'/usr/include/x86_64-linux-gnu/curl'. I wasn't sure if
'/usr/lib/x86_64-linux-gnu/' is the corresponding library
directory. 
* I took the common parent of both '/usr' and ran the following 
  commands to build 'git'

$ make CURLDIR=/usr prefix=/custom/location
$ make CURLDIR=/usr install prefix=/custom/location

* The build did succeed but I get an error that "'https' helper
is not found"

Was anything I did, wrong?

>  This is probably because you are trying to run without installing?
Nope. I'm *installing* git not using the binary wrappers.

>  Ask the "git" you built what its --exec-path is, and run "ls" on
>  that directory to see if you have git-remote-https installed?
>  
Obviously, I don't see any 'git-remote-https' binary in the folder to
which I built git.

>  Trying a freshly built Git binaries without installing is done by
>  setting GIT_EXEC_PATH to point at bin-wrappers/ directory at the
>  top-level of your build tree (that is how our tests can run on an
>  otherwise virgin box with no Git installed).
> 


On Mon, 2017-07-03 at 13:11 -0700, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> $ make NO_GETTEXT=1 NO_MSGFMT=1
> 
> may help.
> 
Ok, I seem to have crapped a little. It seems following the intructions
in the Makefile blindly led to this issue. Reading the instruction
"Define NO_GETTEXT if you don't want Git output to be translated.", I
defined NO_GETTEXT=1 in the Makefile itself! (as specified in the
previous thread)

I'm able to build git without localization support by using the
following command,

make NO_GETTEXT=1 prefix=/custom/location

> NO_GETTEXT is "My build environment may or may not be capable of
> doing the gettext things, but I choose not to use it in my build
> result" but NO_MSGFMT is simply "I do not have the msgfmt tool".
> 
> Having to specify both is rather unfortunate and we may want to
> streamline this.
I guess it's not required!

-- 
Kaartic

Re: Truncating HEAD reflog on branch move

2017-07-04 Thread brian m. carlson 
On Thu, Jun 22, 2017 at 11:13:15PM -0400, Jeff King wrote:
> On Thu, Jun 22, 2017 at 06:25:46PM -0400, Jeff King wrote:
> 
> > So here's a patch on top of what I posted before that pushes the reflog
> > check into the loop (it just decides whether to pull from the reflogs or
> > from the commit queue at the top of the loop).
> > 
> > I was surprised to find, though, that simplify_commit() does not
> > actually do the pathspec limiting! It's done by
> > try_to_simplify_commit(), which is part of add_parents_to_list(). I
> > hacked around it in the later part of the loop by calling
> > try_to_simplify myself and checking the TREESAME flag. But I have a
> > feeling I'm missing something about how the traversal is supposed to
> > work.
> > 
> > This does behave sensibly with "--no-merges" and "--merges", as well as
> > pathspec limiting.
> 
> And here's one more patch on top of those that's necessary to get the
> tests to pass (I don't expect anybody to necessarily be applying this
> slow string of patches; it's just to show the direction I'm looking in).

I've looked at your original patch, which modified reflog-walk.c, and it
does fix the issue.  I'm happy to send in a patch with that and a test
(provided you're okay with me adding your sign-off), or if you wanted to
send in something a bit more complete, like the series of patches here,
that's fine, too.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: "git intepret-trailers" vs. "sed script" to add the signature

2017-07-04 Thread Kaartic Sivaraam 
On Mon, 2017-07-03 at 09:58 -0700, Junio C Hamano wrote:
> Kaartic Sivaraam  writes:
> 
> > So, it seems that excepting for 'commit' it has quite a nice
> > spacing. I
> > guess we could add something like the following to fix that,
> > 
> > # Add new line after SOB in case of "git commit"
> > NEW_LINE='\
> > '
> > if [ -z "$2" ]
> > then
> >   sed -i "1i$NEW_LINE" "$1"
> > fi
> 
> Isn't "sed -i" GNUism that is not portable?
> 
It does seem to be the case. Then the alternative would be the
following,

if [ -z "$2" ]
then
  sed -e "1i$NEW_LINE" "$1" >"sed-output.temp"
  mv "sed-output.temp" "$1"
fi

Actually the GNU's sed documentation tricked me into believing '-i'
wasn't a GNU extension. The '-i' option works with the '--posix' option
of GNU sed which made me believe it isn't an extension.

> > I'll send a typical patch that uses "git interpret-headers" as a
> > follow-up.
> 
> When you say a "typical" patch, what do you exactly mean?  Does
> anybody else send typical patches (or atypical ones for that matter)
> to the list?
> 
I apologise for the inconsistent wordings. I try to mean a patch which
I'm not sure is acceptable (or) not. I guess that's [PATCH/RFC] in the
language of this list. I'm not acquainted to the wordings as I'm an
"off-list" person trying to help and get help! I'll try to use
consistent wordings as far as I could. Once again, please excuse my
ignorance.

-- 
Kaartic

Re: [PATCH] push: add config option to --force-with-lease by default.

2017-07-04 Thread Junio C Hamano 
Francesco Mazzoli  writes:

> The flag can be overridden with `--no-force-with-lease`, or by
> passing the config via the command line.
>
> Signed-off-by: Francesco Mazzoli 
> ---
>  Documentation/config.txt | 5 +
>  builtin/push.c   | 3 +++
>  cache.h  | 1 +
>  config.c | 4 
>  environment.c| 1 +
>  5 files changed, 14 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 06898a7..36fe882 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2537,6 +2537,11 @@ push.default::
>   specific workflows; for instance, in a purely central workflow
>   (i.e. the fetch source is equal to the push destination),
>   `upstream` is probably what you want.  Possible values are:
> +
> +push.alwaysforcewithlease::
> + When true, `--force-with-lease` is the default behavior when
> + using `push --force`. Explicit invocations of `--force-with-lease`
> + or `--no-force-with-lease` if present, take precedence.
>  +
>  --

I suspect this may be going in a wrong direction.  

People have been burned by the lazy "--force-with-lease" that does
not say what object to expect there and forces the command to DWIM
incorrectly what the remote's ref ought to be pointing at.  This
change encourages its use without the user being painfully aware of
that danger.  Whenever you say "push --force", you'd be using the
dangerous "--force-with-lease" that does not specify what the
expected current state of the remote is.  The end result gives an
illusion of being safer than a simple "--force", without being
not really safer.

I'd understand more if there were two new (and orthogonal) options,
though:

 - disable the use of "--force" option, telling the user to use
   "--force-with-lease=" instead.

 - disable the DWIM based on the remote-tracking branches when
   "--force-with-lease[=]" is used, i.e. error out when the
   option is used without a specific object to expect.

Re: [PATCH v2] add --signoff flag to `git-merge` command.

2017-07-04 Thread Junio C Hamano 
Ævar Arnfjörð Bjarmason  writes:

> On Tue, Jul 04 2017, Łukasz Gryglicki jotted:
>
>> add --signoff flag to `git-merge` command.
>
> We'd usually say this as:
>
> merge: add a --signoff flag
>
> Or something like that.

I thought I gave a fairly complete example that can be imitated, but
apparently it didn't go through X-<.

>> Some projects require every commit to be signed off.
>> Our workflow is to create feature branches and require every commit to
>> be signed off. When feature is finally approved we need to merge it into
>> master. Merge itself is usually trivial and is done by
>> `git merge origin/master`. Unfortunatelly this command have no --signoff
>> flag, so we need to either add signoff line manually or use
>> `git commit --amend -s` after the merge. First solution is not ideal
>> because not all developers are familiar with exact sign-off syntax.
>> The second solution works, but is obviously tedious.
>> This patch adds --signoff support to git-merge command. It works just
>> like --signoff in `git-commit` command.
>
> It would be nice to split this into a at least a couple of paragraphs,
> and more closely follow the format suggested by
> Documentation/SubmittingPatches.

Good suggestion.

>> More details here:
>> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u
>
> These more details include my outstanding question in
> 87fueferd4@gmail.com which hasn't been answered yet.

I have an opinion on that topic, but I'd prefer to hear from others
first before I speak out.

>> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
>> new file mode 100755
>> index 0..f542f136f5dda
>> --- /dev/null
>> +++ b/t/t9904-git-merge-signoff.sh
>
> The convention for adding new tests is not to add a new one after
> whatever name sorts the highest, see "Naming Tests" in t/README.

Correct.

> I.e. this should be somewhere in t[6-7]* with the other merge tests.

Yeah.  While most of t[67]??? series are about the contents of the
merge, i.e. resulting trees and what happens in the working tree,
there are some tests about the merge messages in there.  t7608 is
exactly about how the command prepares the messages before giving
them to human to edit, and I think "merge can be told to optionally
add sign-off" fits there just fine.  All existing tests there are
only interested about the title, but that does not mean there must
not be tests that care more than the title in the script.

Also, as you suggest, these will become a lot shorter when the
standard test helper shell functions are used.  I do not think we
necessarily want a brand new test script to test only three or so
combinations (i.e. last one wins when --option and --no-option is
given, --option has an effect, --no-option does not have an effect).

Re: [PATCH v3 2/3] sha1dc: optionally use sha1collisiondetection as a submodule

2017-07-04 Thread Junio C Hamano 
Ævar Arnfjörð Bjarmason  writes:

>>> diff --git a/.gitmodules b/.gitmodules
>>> new file mode 100644
>>> index 00..cbeebdab7a
>>> --- /dev/null
>>> +++ b/.gitmodules
>>> @@ -0,0 +1,4 @@
>>> +[submodule "sha1collisiondetection"]
>>> +   path = sha1collisiondetection
>>> +   url = https://github.com/cr-marcstevens/sha1collisiondetection.git
>>> +   branch = master
>>
>> Do we need to say this "branch" bit?
>
> Yes, it's to make future updates easier, see b928922727 ("submodule add:
> If --branch is given, record it in .gitmodules", 2012-12-19).

Why?  It's not like we want to _follow_ the 'master' branch of that
sha1collisiondetection repository.  We declare that a specific
commit from the (sub)module is suited for our project, and do not
really care to automatically update from whatever happens to be at
the tip of 'master' there.

>
>> Other than that looks good to me.
>>
>> Thanks.

Re: [PATCH 06/12] builtin/update_ref: convert to struct object_id

2017-07-04 Thread brian m. carlson 
On Mon, Jul 03, 2017 at 10:49:39PM +0200, Ævar Arnfjörð Bjarmason wrote:
> On Mon, Jul 03 2017, brian m. carlson jotted:
> > [...]
> >  1 file changed, 34 insertions(+), 35 deletions(-)
> > [...]
> > struct strbuf err = STRBUF_INIT;
> > char *refname;
> > -   unsigned char new_sha1[20];
> > -   unsigned char old_sha1[20];
> > +   struct object_id new_oid, old_oid;
> > [...]
> 
> It's easier to skim these when you leave changes in the number of lines
> to separate commits which do more than just rename boilerplate code,
> e.g. as in 05/12 where `const char *p` is introduced.

I can do that in the future.  I've received feedback in the past that I
could coalesce them into one line instead of needing multiple lines, but
since I don't have a preference either way, I'm happy to do whatever
makes review easier.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: Flurries of 'git reflog expire'

2017-07-04 Thread Ævar Arnfjörð Bjarmason 

On Tue, Jul 04 2017, Andreas Krey jotted:

> Hi everyone,
>
> how is 'git reflog expire' triggered? We're occasionally seeing a lot
> of the running in parallel on a single of our repos (atlassian bitbucket),
> and this usually means that the machine is not very responsive for
> twenty minutes, the repo being as big as it is.

Assuming Linux, what does 'ps auxf' look like when this happens? Is the
parent a 'git gc --auto'?

> The server is still on git 2.6.2 (and bitbucket 4.14.5).

You might want to upgrade, we've had a bunch of changes since then,
maybe some of this fixes it:

git log --reverse -p -L'/^static.*lock_repo_for/,/^}/:builtin/gc.c'

> Questions:
>
> What can be done about this? Cronjob 'git reflog expire' at midnight,
> so the heuristic don't trigger during the day? (The relnotes don't
> mention anything after 2.4.0, so I suppose a git upgrade won't help.)
>
> What is the actual cause? Bad heuristics in git itself, or does
> bitbucket run them too often (improbable)?

You can set gc.auto=0 in the repo to disable auto-gc, and play with
e.g. the reflog expire values, see the git-gc manpage.

But then you need to run your own gc, which is not a bad idea anyway
with a dedicated git server.

But it would be good to get to the bottom of this, we shouldn't be
running these concurrently.

[PATCH v3] merge: add a --signoff flag.

2017-07-04 Thread Łukasz Gryglicki 
Some projects require every commit to be signed off.
Our workflow is to create feature branches and require every commit to
be signed off. When feature is finally approved we need to merge it into
master. Merge itself is usually trivial and is done by
`git merge origin/master`.

Unfortunatelly `merge` command have no --signoff
flag, so we need to either add signoff line manually or use
`git commit --amend -s` after the merge.

First solution is not ideal because not all developers are familiar with
exact sign-off syntax. The second solution works, but is obviously tedious.

This patch adds --signoff support to `git-merge` command.
It works just like --signoff in `git-commit` command.

More details can be found here:
https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u

Signed-off-by: lukaszgryglicki 
---
 Documentation/git-merge.txt |  8 ++
 builtin/merge.c |  4 +++
 t/t7614-merge-signoff.sh| 69 +
 3 files changed, 81 insertions(+)
 create mode 100755 t/t7614-merge-signoff.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 04fdd8cf086db..6b308ab6d0b52 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,6 +64,14 @@ OPTIONS
 ---
 include::merge-options.txt[]
 
+--signoff::
+   Add Signed-off-by line by the committer at the end of the commit
+   log message.  The meaning of a signoff depends on the project,
+   but it typically certifies that committer has
+   the rights to submit this work under the same license and
+   agrees to a Developer Certificate of Origin
+   (see http://developercertificate.org/ for more information).
+
 -S[]::
 --gpg-sign[=]::
GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb45d0b..78c36e9bf353b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -70,6 +70,7 @@ static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
+static int signoff;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
{ OPTION_STRING, 'S', "gpg-sign", _commit, N_("key-id"),
  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_BOOL(0, "overwrite-ignore", _ignore, N_("update ignored 
files (default)")),
+   OPT_BOOL(0, "signoff", , N_("add Signed-off-by:")),
OPT_END()
 };
 
@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list 
*remoteheads)
strbuf_addch(, '\n');
if (0 < option_edit)
strbuf_commented_addf(, _(merge_editor_comment), 
comment_line_char);
+   if (signoff)
+   append_signoff(, ignore_non_trailer(msg.buf, msg.len), 0);
write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
if (run_commit_hook(0 < option_edit, get_index_file(), 
"prepare-commit-msg",
git_path_merge_msg(), "merge", NULL))
diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
new file mode 100755
index 0..c1b8446f491dc
--- /dev/null
+++ b/t/t7614-merge-signoff.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='git merge --signoff
+
+This test runs git merge --signoff and makes sure that it works.
+'
+
+. ./test-lib.sh
+
+# Setup test files
+test_setup() {
+   # Expected commit message after merge --signoff
+   cat >expected-signed <.*/>/")
+EOF
+
+   # Expected commit message after merge without --signoff (or with 
--no-signoff)
+   cat >expected-unsigned actual &&
+   test_cmp expected-unsigned actual
+'
+
+# Test for --no-signoff flag
+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
+   git checkout master &&
+   test_commit master-branch-4 file4 4 &&
+   git checkout other-branch &&
+   git merge master --no-edit --signoff --no-signoff &&
+   git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+   test_cmp expected-unsigned actual
+'
+
+test_done

--
https://github.com/git/git/pull/383

LOAN

2017-07-04 Thread LOAN CAPITAL FINANCE 

UNSECURED BUSINESS/PERSONAL LOAN BY LOAN CAPITAL FINANCE

- NO COLLATERAL

- MINIMUM DOCUMENTATION

- BUSINESS LOAN UP TO FIVE(5) MILLION US DOLLARS

CONTACT US TODAY VIA EMAIL: loancapi...@outlook.in

Re: [PATCH v2] add --signoff flag to `git-merge` command.

2017-07-04 Thread Ævar Arnfjörð Bjarmason 

On Tue, Jul 04 2017, Łukasz Gryglicki jotted:

> add --signoff flag to `git-merge` command.

We'd usually say this as:

merge: add a --signoff flag

Or something like that.

> Some projects require every commit to be signed off.
> Our workflow is to create feature branches and require every commit to
> be signed off. When feature is finally approved we need to merge it into
> master. Merge itself is usually trivial and is done by
> `git merge origin/master`. Unfortunatelly this command have no --signoff
> flag, so we need to either add signoff line manually or use
> `git commit --amend -s` after the merge. First solution is not ideal
> because not all developers are familiar with exact sign-off syntax.
> The second solution works, but is obviously tedious.
> This patch adds --signoff support to git-merge command. It works just
> like --signoff in `git-commit` command.

It would be nice to split this into a at least a couple of paragraphs,
and more closely follow the format suggested by
Documentation/SubmittingPatches.

> More details here:
> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u

These more details include my outstanding question in
87fueferd4@gmail.com which hasn't been answered yet.

> Signed-off-by: lukaszgryglicki 
> ---
>  Documentation/git-merge.txt  |  8 +
>  builtin/merge.c  |  4 +++
>  t/t9904-git-merge-signoff.sh | 75 
> 
>
>  3 files changed, 87 insertions(+)
>  create mode 100755 t/t9904-git-merge-signoff.sh
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 04fdd8cf086db..6b308ab6d0b52 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -64,6 +64,14 @@ OPTIONS
>  ---
>  include::merge-options.txt[]
>
> +--signoff::
> + Add Signed-off-by line by the committer at the end of the commit
> + log message.  The meaning of a signoff depends on the project,
> + but it typically certifies that committer has
> + the rights to submit this work under the same license and
> + agrees to a Developer Certificate of Origin
> + (see http://developercertificate.org/ for more information).
> +
>  -S[]::
>  --gpg-sign[=]::
>   GPG-sign the resulting merge commit. The `keyid` argument is
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 900bafdb45d0b..78c36e9bf353b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -70,6 +70,7 @@ static int continue_current_merge;
>  static int allow_unrelated_histories;
>  static int show_progress = -1;
>  static int default_to_upstream = 1;
> +static int signoff;
>  static const char *sign_commit;
>
>  static struct strategy all_strategy[] = {
> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
>   { OPTION_STRING, 'S', "gpg-sign", _commit, N_("key-id"),
> N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
>   OPT_BOOL(0, "overwrite-ignore", _ignore, N_("update ignored 
> files (default)")),
> + OPT_BOOL(0, "signoff", , N_("add Signed-off-by:")),
>   OPT_END()
>  };
>
> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list 
> *remoteheads)
>   strbuf_addch(, '\n');
>   if (0 < option_edit)
>   strbuf_commented_addf(, _(merge_editor_comment), 
> comment_line_char);
> + if (signoff)
> + append_signoff(, ignore_non_trailer(msg.buf, msg.len), 0);
>   write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
>   if (run_commit_hook(0 < option_edit, get_index_file(), 
> "prepare-commit-msg",
>   git_path_merge_msg(), "merge", NULL))
> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
> new file mode 100755
> index 0..f542f136f5dda
> --- /dev/null
> +++ b/t/t9904-git-merge-signoff.sh

The convention for adding new tests is not to add a new one after
whatever name sorts the highest, see "Naming Tests" in t/README.

I.e. this should be somewhere in t[6-7]* with the other merge tests.

> @@ -0,0 +1,75 @@
> +#!/bin/sh
> +
> +test_description='git merge --signoff
> +
> +This test runs git merge --signoff and makes sure that it works.
> +'
> +
> +. ./test-lib.sh
> +
> +# Setup test files
> +test_setup() {
> + # A simples files to commit
> + echo "1" >file1
> + echo "2" >file2
> + echo "3" >file3
> + echo "4" >file4
> +
> + # Expected commit message after merge --signoff
> + cat >expected-signed < +Merge branch 'master' into other-branch
> +
> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> +EOF
> +
> + # Expected commit message after merge without --signoff (or with 
> --no-signoff)
> + cat >expected-unsigned < +Merge branch 'master' into other-branch
> +EOF
> +
> + # Initial commit and feature branch to merge master into it.
> + git commit --allow-empty -m "Initial empty commit"
> + git checkout

Re: Should "head" also work for "HEAD" on case-insensitive FS?

2017-07-04 Thread Ævar Arnfjörð Bjarmason 

On Tue, Jul 04 2017, Konstantin Khomoutov jotted:

> On Tue, Jul 04, 2017 at 12:00:49AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> I don't have a OSX box, but was helping a co-worker over Jabber the
>> other day, and he pasted something like:
>>
>> $ git merge-base github/master head
>>
>> Which didn't work for me, and I thought he had a local "head" branch
>> until realizing that of course we were just resolving HEAD on the FS.
>>
>> Has this come up before? I think it makes sense to warn/error about
>> these magic /HEAD/ revisions if they're not upper-case.
>>
>> This is likely unintentional and purely some emergent effect of how it's
>> implemented, and leads to unportable git invocations.
>
> JFTR this is one common case of confusion on Windows as well.
> To the point that I saw people purposedly using "head" on StackOverflow
> questions.  That is, they appear to think (for some reason) that
> branches in Git have case-insensitive names and prefer to spell "head"
> since it (supposedly) easier to type.
>
> I don't know what to do about it.
> Ideally we'd just have a way to perform a final check on the file into
> which a ref name was resolved to see its "real" name but I don't know
> whether all popular filesystems are case preserving (HFS+ and NTFS are,
> IIRC) and even if they are, whether the appropriate platform-specific
> APIs exists to perform such a check.

I think there's no easy way do this in the general case with the current
ref backend, because we rely on the FS to store the refs.

But I'm thinking of the more specific case where you specify
{HEAD,FETCH_HEAD,ORIG_HEAD,MERGE_HEAD,CHERRY_PICK_HEAD} as non-upper
case, and we resolve it from .git/$NAME.

So the detection would not be checking whether the file on-disk has the
same casing, but knowing that if we resolve anything from .git/$NAME
then the string provided on the command-line must be upper-case.

Although there is this:

$ git rev-parse HEAD
051ee1e7dd2c7b8bdc20f237eea3c7d5b1314280
$ git rev-parse WHATEVER
WHATEVER
fatal: ambiguous argument 'WHATEVER': unknown revision or path not in the 
working tree.
$ cp .git/{HEAD,WHATEVER}
$ git rev-parse WHATEVER
051ee1e7dd2c7b8bdc20f237eea3c7d5b1314280

I.e. we allow any arbitrary ref sitting in .git/, but presumably we
could just record the original string the user provided so that this
dies on OSX/Windows too:

$ cp .git/{HEAD,Whatever}
$ git rev-parse wHATEVER
wHATEVER
fatal: ambiguous argument 'wHATEVER': unknown revision or path not in the 
working tree.

But this may be a much deeper rabbit hole than I initially thought, I
was fishing to see if someone knew of a place in the code or WIP patch
that dealt with these special refs, but between the low-level machinery
& sha1_name.c (and others) there may be no easy one place to do this...

Flurries of 'git reflog expire'

2017-07-04 Thread Andreas Krey 
Hi everyone,

how is 'git reflog expire' triggered? We're occasionally seeing a lot
of the running in parallel on a single of our repos (atlassian bitbucket),
and this usually means that the machine is not very responsive for
twenty minutes, the repo being as big as it is.

The server is still on git 2.6.2 (and bitbucket 4.14.5).

Questions:

What can be done about this? Cronjob 'git reflog expire' at midnight,
so the heuristic don't trigger during the day? (The relnotes don't
mention anything after 2.4.0, so I suppose a git upgrade won't help.)

What is the actual cause? Bad heuristics in git itself, or does
bitbucket run them too often (improbable)?

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds 
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: Should "head" also work for "HEAD" on case-insensitive FS?

2017-07-04 Thread Konstantin Khomoutov 
On Tue, Jul 04, 2017 at 12:00:49AM +0200, Ævar Arnfjörð Bjarmason wrote:

> I don't have a OSX box, but was helping a co-worker over Jabber the
> other day, and he pasted something like:
> 
> $ git merge-base github/master head
> 
> Which didn't work for me, and I thought he had a local "head" branch
> until realizing that of course we were just resolving HEAD on the FS.
> 
> Has this come up before? I think it makes sense to warn/error about
> these magic /HEAD/ revisions if they're not upper-case.
> 
> This is likely unintentional and purely some emergent effect of how it's
> implemented, and leads to unportable git invocations.

JFTR this is one common case of confusion on Windows as well.
To the point that I saw people purposedly using "head" on StackOverflow
questions.  That is, they appear to think (for some reason) that
branches in Git have case-insensitive names and prefer to spell "head"
since it (supposedly) easier to type.

I don't know what to do about it.
Ideally we'd just have a way to perform a final check on the file into
which a ref name was resolved to see its "real" name but I don't know
whether all popular filesystems are case preserving (HFS+ and NTFS are,
IIRC) and even if they are, whether the appropriate platform-specific
APIs exists to perform such a check.

[PATCH v2] add --signoff flag to `git-merge` command.

2017-07-04 Thread Łukasz Gryglicki 
Some projects require every commit to be signed off.
Our workflow is to create feature branches and require every commit to
be signed off. When feature is finally approved we need to merge it into
master. Merge itself is usually trivial and is done by
`git merge origin/master`. Unfortunatelly this command have no --signoff
flag, so we need to either add signoff line manually or use
`git commit --amend -s` after the merge. First solution is not ideal
because not all developers are familiar with exact sign-off syntax.
The second solution works, but is obviously tedious.
This patch adds --signoff support to git-merge command. It works just
like --signoff in `git-commit` command.
More details here:
https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-dy9h3kt4feymgv__p2gzznr0wua...@mail.gmail.com/T/#u

Signed-off-by: lukaszgryglicki 
---
 Documentation/git-merge.txt  |  8 +
 builtin/merge.c  |  4 +++
 t/t9904-git-merge-signoff.sh | 75 
 3 files changed, 87 insertions(+)
 create mode 100755 t/t9904-git-merge-signoff.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 04fdd8cf086db..6b308ab6d0b52 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,6 +64,14 @@ OPTIONS
 ---
 include::merge-options.txt[]
 
+--signoff::
+   Add Signed-off-by line by the committer at the end of the commit
+   log message.  The meaning of a signoff depends on the project,
+   but it typically certifies that committer has
+   the rights to submit this work under the same license and
+   agrees to a Developer Certificate of Origin
+   (see http://developercertificate.org/ for more information).
+
 -S[]::
 --gpg-sign[=]::
GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb45d0b..78c36e9bf353b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -70,6 +70,7 @@ static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
+static int signoff;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
{ OPTION_STRING, 'S', "gpg-sign", _commit, N_("key-id"),
  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
OPT_BOOL(0, "overwrite-ignore", _ignore, N_("update ignored 
files (default)")),
+   OPT_BOOL(0, "signoff", , N_("add Signed-off-by:")),
OPT_END()
 };
 
@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list 
*remoteheads)
strbuf_addch(, '\n');
if (0 < option_edit)
strbuf_commented_addf(, _(merge_editor_comment), 
comment_line_char);
+   if (signoff)
+   append_signoff(, ignore_non_trailer(msg.buf, msg.len), 0);
write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
if (run_commit_hook(0 < option_edit, get_index_file(), 
"prepare-commit-msg",
git_path_merge_msg(), "merge", NULL))
diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
new file mode 100755
index 0..f542f136f5dda
--- /dev/null
+++ b/t/t9904-git-merge-signoff.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='git merge --signoff
+
+This test runs git merge --signoff and makes sure that it works.
+'
+
+. ./test-lib.sh
+
+# Setup test files
+test_setup() {
+   # A simples files to commit
+   echo "1" >file1
+   echo "2" >file2
+   echo "3" >file3
+   echo "4" >file4
+
+   # Expected commit message after merge --signoff
+   cat >expected-signed <.*/>/")
+EOF
+
+   # Expected commit message after merge without --signoff (or with 
--no-signoff)
+   cat >expected-unsigned actual &&
+   test_cmp expected-unsigned actual
+'
+
+# Test for --no-signoff flag
+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
+   git checkout master &&
+   git add file4 &&
+   git commit -m master-branch-3 &&
+   git checkout other-branch &&
+   git merge master --no-edit --signoff --no-signoff &&
+   git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+   test_cmp expected-unsigned actual
+'
+
+test_done

--
https://github.com/git/git/pull/382

Re: What's cooking in git.git (Jun 2017, #09; Fri, 30)

2017-07-04 Thread Junio C Hamano 
"Philip Oakley"  writes:

> From: "Junio C Hamano" 
>
>> I am not sure what you are asking.  Is this the command you are
>> looking for?
>>
>> $ grep -e "^[[*]" whats-cooking.txt
>>
> Ah, thanks.
>
> It does presume that one has extracted the email to the text file,
> which is easier on some systems and mail clients than others ;-)

Perhaps you would want to find an e-mail client that can handle text
files better, then? ;-)

If you are cloning and following along my tree from one of these
places:

git://repo.or.cz/alt-git.git/
git://git.kernel.org/pub/scm/git/git.git/
git://github.com/git/git.git/

the 'todo' branch in these repositories keeps track of the
whats-cooking.txt report (among other things), so you can also:

$ git cat-file -p origin/todo:whats-cooking.txt |
  grep -e "^[[*]"

to get the same effect.