Re: [RFC PATCH] cherry: teach git cherry to respect pathspec

[Rasmus Villemoes]

On 16 February 2018 at 02:15, Rasmus Villemoes  wrote:
> This is a very naive attempt at teaching git cherry to restrict
> attention to a given pathspec. Very much RFC, hence no documentation
> update or test cases for now.
>

Ping, any comments on this?

Re: Git Merge contributor summit notes

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> On Sat, Mar 10 2018, Alex Vandiver jotted:
>
>> It was great to meet some of you in person!  Some notes from the
>> Contributor Summit at Git Merge are below.  Taken in haste, so
>> my apologies if there are any mis-statements.
>
> Thanks a lot for taking these notes. I've read them over and they're all
> accurate per my wetware recollection. Adding some things I remember
> about various discussions below where I think it may help to clarify
> things a bit.
>
>>  - Alex

Thanks, both, for sharing.

Re: git rerere to remember half-merged progress - valid use?

[Junio C Hamano]

Ilya Kantor  writes:

> Let's say I'm merging a branch with many conflicts.
> I resolved some of them, but then can't proceed or need to switch elsewhere.
>
> Will it be a good practice to call `git rerere` to remember resolved
> conflicts, so that in the future
> when I re-merge, I get my half-done merge back?

As many of real world issues, the answer is "it depends".  

If you are absolutely sure about what you have resolved so far, then
recording the resolutions for files that you have finished while
there are still other files with conflicts to be resolved before
switching away to another task is a very sensible thing to do.

On the other hand, recording resolutions you are not sure about
means more work for you when you come back and attempt the merge
again, as you would need to check resolutions of which files need
to be nuked and redone (using "checkout -m" and "rerere forget").

My personal strategy is to avoid abandoning conflict resolution in
the middle in the first place ;-), but in many cases I can be quite
confident that I resolved conflicts in a file correctly without
looking at conflicts in other files, so I do not think it is a bad
practice to record resolution of files you resolved so far by
calling "rerere" manually.

Dear friend,

[Baari Abdul]

Dear friend,

 

I  Mr.  Baari Abdul, Head of Operation at Bank of Africa. I want invite into a 
business overture which involves an amount of $ 22.3 million. At your 
acceptance, this amount will be transferred to your name as a foreign partner.

 

I need your help to get this fund to be transfer out from here to your account, 
and we share at a ratio of 50% each. You will receive this amount by bank 
transfer.

Please send your full name...

You’re directly phone numbers...

Address

 

I will detail you more about this transaction but i need above data to make 
some vital changes in the transit account which will make your name appear as 
the true beneficiary of the fund. You have to contact me through my private 
e-mail at (baariabdul...@gmail.com) your prompt reply will be highly 
appreciated.

Sincerely,

Mr.  Baari Abdul  .







.

[PATCH v3] git{,-blame}.el: remove old bitrotting Emacs code

[Ævar Arnfjörð Bjarmason]

The git-blame.el mode has been superseded by Emacs's own
vc-annotate (invoked by C-x v g). Users of the git.el mode are now
much better off using either Magit or the Git backend for Emacs's own
VC mode.

These modes were added over 10 years ago when Emacs's own Git support
was much less mature, and there weren't other mature modes in the wild
or shipped with Emacs itself.

These days these modes have few if any users, and users of git aren't
well served by us shipping these (some OS's install them alongside git
by default, which is confusing and leads users astray).

So let's remove these per Alexandre Julliard's message to the
ML[1]. If someone still wants these for some reason they're better
served by hosting these elsewhere (e.g. on ELPA), instead of us
distributing them with git.

1. "Re: [PATCH] git.el: handle default excludesfile
   properly" (87muzlwhb0@winehq.org) --
   https://public-inbox.org/git/87muzlwhb0@winehq.org/

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

On Sat, Mar 10 2018, Martin Ågren jotted:
> [...]

Thanks. Here's a v3 which incorporates these suggestions, and also
Kyle's suggestion upthread which I somehow missed when re-rolling
this. tbdiff to v2 below:

  1: ad00297143 ! 1: 5bc3d3848d git{,-blame}.el: remove old bitrotting Emacs 
code
  @@ -11,7 +11,7 @@
   was much less mature, and there weren't other mature modes in the 
wild
   or shipped with Emacs itself.
   
  -These days these modes have very few if users, and users of git 
aren't
  +These days these modes have few if any users, and users of git aren't
   well served by us shipping these (some OS's install them alongside 
git
   by default, which is confusing and leads users astray).
   
  @@ -70,10 +70,10 @@
   -To make the modules available to Emacs, you should add this directory
   -to your load-path, and then require the modules you want. This can be
   -done by adding to your .emacs something like this:
  -+These were added shortly after Git was first released, since then
  ++These were added shortly after Git was first released. Since then
   +Emacs's own support for Git got better than what was offered by these
  -+modules, or was superseded by popular 3rd-party Git modes such as
  -+Magit.
  ++modes. There are also popular 3rd-party Git modes such as Magit which
  ++offer replacements for these.

   -  (add-to-list 'load-path ".../git/contrib/emacs")
   -  (require 'git)
  @@ -92,7 +92,7 @@
   -  the pcl-cvs mode. It can be started with `M-x git-status'.
   +  Wrapper for "git status" that provided access to other git commands.
   +
  -+  Modern alternatives to this are Magit, or the VC mode that ships
  ++  Modern alternatives to this include Magit, and VC mode that ships
   +  with Emacs.

* git-blame.el:

 contrib/emacs/.gitignore   |1 -
 contrib/emacs/Makefile |   21 -
 contrib/emacs/README   |   32 +-
 contrib/emacs/git-blame.el |  483 -
 contrib/emacs/git.el   | 1704 
 5 files changed, 13 insertions(+), 2228 deletions(-)
 delete mode 100644 contrib/emacs/.gitignore
 delete mode 100644 contrib/emacs/Makefile
 delete mode 100644 contrib/emacs/git-blame.el
 delete mode 100644 contrib/emacs/git.el

diff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore
deleted file mode 100644
index c531d9867f..00
--- a/contrib/emacs/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-*.elc
diff --git a/contrib/emacs/Makefile b/contrib/emacs/Makefile
deleted file mode 100644
index 24d9312941..00
--- a/contrib/emacs/Makefile
+++ /dev/null
@@ -1,21 +0,0 @@
-## Build and install stuff
-
-EMACS = emacs
-
-ELC = git.elc git-blame.elc
-INSTALL ?= install
-INSTALL_ELC = $(INSTALL) -m 644
-prefix ?= $(HOME)
-emacsdir = $(prefix)/share/emacs/site-lisp
-RM ?= rm -f
-
-all: $(ELC)
-
-install: all
-   $(INSTALL) -d $(DESTDIR)$(emacsdir)
-   $(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
-
-%.elc: %.el
-   $(EMACS) -batch -f batch-byte-compile $<
-
-clean:; $(RM) $(ELC)
diff --git a/contrib/emacs/README b/contrib/emacs/README
index 82368bdbff..977a16f1e3 100644
--- a/contrib/emacs/README
+++ b/contrib/emacs/README
@@ -1,30 +1,24 @@
-This directory contains various modules for Emacs support.
+This directory used to contain various modules for Emacs support.
 
-To make the modules available to Emacs, you should add this directory
-to your load-path, and then require the modules you want. This can be
-done by adding to your .emacs something like this:
+These were added shortly after Git was first released. Since then
+Emacs's own support for Git got better than what was offered by these
+modes. There are also popular 3rd-party Git modes such as Magit which
+offer replacements for these.
 
-  (add-to-list 'load-path ".../git/contrib/emacs")

Re: [PATCH v2] git{,-blame}.el: remove old bitrotting Emacs code

[Martin Ågren]

On 10 March 2018 at 13:30, Ævar Arnfjörð Bjarmason  wrote:
> diff --git a/contrib/emacs/README b/contrib/emacs/README
> index 82368bdbff..5a63109458 100644
> --- a/contrib/emacs/README
> +++ b/contrib/emacs/README
> @@ -1,30 +1,24 @@
> -This directory contains various modules for Emacs support.
> +This directory used to contain various modules for Emacs support.
>
> -To make the modules available to Emacs, you should add this directory
> -to your load-path, and then require the modules you want. This can be
> -done by adding to your .emacs something like this:
> +These were added shortly after Git was first released, since then

s/, since/. Since/ ?

> +Emacs's own support for Git got better than what was offered by these
> +modules, or was superseded by popular 3rd-party Git modes such as
> +Magit.

This somehow reads like "Emacs's own support ... was superseded ...".
Maybe that's what you mean, but i'm not sure. Perhaps s/, was superseded
by/. There are also/.

>  * git.el:
>
> -  Status manager that displays the state of all the files of the
> -  project, and provides easy access to the most frequently used git
> -  commands. The user interface is as far as possible compatible with
> -  the pcl-cvs mode. It can be started with `M-x git-status'.
> +  Wrapper for "git status" that provided access to other git commands.
> +
> +  Modern alternatives to this are Magit, or the VC mode that ships
> +  with Emacs.

s/, or/ and/ ? My thinking: "A and B are modern alternatives", not "A or
B are modern alternatives.".

>  * git-blame.el:
>
> -  Emacs implementation of incremental git-blame.  When you turn it on
> -  while viewing a file, the editor buffer will be updated by setting
> -  the background of individual lines to a color that reflects which
> -  commit it comes from.  And when you move around the buffer, a
> -  one-line summary will be shown in the echo area.
> +  A wrapper for "git blame" written before Emacs's own vc-annotate
> +  mode learned to invoke git-blame, which can be done via C-x v g.

Thanks for giving constructive hints. :-)

Martin

[PATCH v4 0/3] give more useful error messages while renaming branch (reboot)

[Kaartic Sivaraam]

It's been a long time since the v3 of the patch. So, it's worth restating
the reason behind this patch.

>From v1 of this patch,

 In builtin/branch, the error messages weren't handled directly by the 
branch
 renaming function and was left to the other function. Though this avoids
 redundancy this gave unclear error messages in some cases. So, make
 builtin/branch give more useful error messages.

Changes since v3:

 - Handled more error related to old branch name.

 - Incorporated changes suggested in v3 which include using ';' as a sentence
   connector instead 'and'.

 - Error messages use the interpreted branch names (without the (refs/heads/ 
part).

The unrelated cleanup patches which were in the previous versions have
since been submitted as a separate series and have been merged into
the codebase.

The first two patches are related to the topic of this patch. The 3rd one
is a little typo fix that I noticed on the way.

This patch was based off 'master' and has been rebased to incorporate
the new changes to 'master'. So, it generally should apply cleanly on
'master'. Let me know if it doesn't.

The sample input/output cases for this patch are as follows,

$ git branch
* master
  foo
  bar

Before patch,

# Case 1: Trying to rename non-existent branch
$ git branch -m hypothet no_such_branch
error: refname refs/heads/hypothet not found
fatal: Branch rename failed

# Case 2: Trying to rename non-existent branch to an existing one
$ git branch -m hypothet master
fatal: A branch named 'master' already exists.

# Case 3: Trying to force update current branch
$ git branch -M foo master
fatal: Cannot force update the current branch.

# Case 4: Trying to force rename an in-existent branch with an invalid 
name
$ git branch -M hypothet ?123
fatal: '?123' is not a valid branch name.

After patch,

# Case 1: Trying to rename non-existent branch
$ git branch -m hypothet no_such_branch
fatal: branch 'hypothet' doesn't exist

# Case 2: Trying to rename non-existent branch to an existing one
$ git branch -m hypothet master
fatal: branch 'hypothet' doesn't exist; branch 'master' already exists

# Case 3: Trying to force update current branch
$ git branch -M foo master
fatal: cannot force update the current branch

# Case 4: Trying to force rename an in-existent branch with an invalid 
name
$ git branch -M hypothet ?123
fatal: branch 'hypothet' doesn't exist; new branch name '?123' is 
invalid


Note: Thanks to the strbuf API that made it possible to easily
construct the composite error message strings!

Kaartic Sivaraam (3):
  branch: introduce dont_fail parameter for branchname validation
  builtin/branch: give more useful error messages when renaming
  t/t3200: fix a typo in a test description

 branch.c   |  59 +---
 branch.h   |  61 -
 builtin/branch.c   | 111 ++---
 builtin/checkout.c |   5 +-
 t/t3200-branch.sh  |   2 +-
 5 files changed, 181 insertions(+), 57 deletions(-)

-- 
2.16.1.291.g4437f3f13

[PATCH v4 1/3] branch: introduce dont_fail parameter for branchname validation

[Kaartic Sivaraam]

This parameter allows the branchname validation functions to
optionally return a flag specifying the reason for failure, when
requested. This allows the caller to know why it was about to die.
This allows more useful error messages to be given to the user when
trying to rename a branch.

The flags are specified in the form of an enum and values for success
flags have been assigned explicitly to clearly express that certain
callers rely on those values and they cannot be arbitrary.

Only the logic has been added but no caller has been made to use
it, yet. So, no functional changes.

Signed-off-by: Kaartic Sivaraam 
---
 branch.c   | 59 
 branch.h   | 61 +-
 builtin/branch.c   |  4 +--
 builtin/checkout.c |  5 ++--
 4 files changed, 88 insertions(+), 41 deletions(-)

diff --git a/branch.c b/branch.c
index 2672054f0..0eeb8403e 100644
--- a/branch.c
+++ b/branch.c
@@ -178,41 +178,48 @@ int read_branch_desc(struct strbuf *buf, const char 
*branch_name)
return 0;
 }
 
-/*
- * Check if 'name' can be a valid name for a branch; die otherwise.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-int validate_branchname(const char *name, struct strbuf *ref)
+enum branch_validation_result validate_branchname(const char *name, struct 
strbuf *ref, unsigned gently)
 {
-   if (strbuf_check_branch_ref(ref, name))
-   die(_("'%s' is not a valid branch name."), name);
+   if (strbuf_check_branch_ref(ref, name)) {
+   if (gently)
+   return VALIDATION_FATAL_INVALID_BRANCH_NAME;
+   else
+   die(_("'%s' is not a valid branch name."), name);
+   }
 
-   return ref_exists(ref->buf);
+   return ref_exists(ref->buf) ? VALIDATION_PASS_BRANCH_EXISTS : 
VALIDATION_PASS_BRANCH_DOESNT_EXIST;
 }
 
-/*
- * Check if a branch 'name' can be created as a new branch; die otherwise.
- * 'force' can be used when it is OK for the named branch already exists.
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+enum branch_validation_result validate_new_branchname(const char *name, struct 
strbuf *ref, int force, unsigned gently)
 {
const char *head;
 
-   if (!validate_branchname(name, ref))
-   return 0;
+   if (gently) {
+   enum branch_validation_result res = validate_branchname(name, 
ref, 1);
+   if (res == VALIDATION_FATAL_INVALID_BRANCH_NAME || res == 
VALIDATION_PASS_BRANCH_DOESNT_EXIST)
+   return res;
+   } else {
+   if (validate_branchname(name, ref, 0) == 
VALIDATION_PASS_BRANCH_DOESNT_EXIST)
+   return VALIDATION_PASS_BRANCH_DOESNT_EXIST;
+   }
 
-   if (!force)
-   die(_("A branch named '%s' already exists."),
-   ref->buf + strlen("refs/heads/"));
+   if (!force) {
+   if (gently)
+   return VALIDATION_FATAL_BRANCH_EXISTS_NO_FORCE;
+   else
+   die(_("A branch named '%s' already exists."),
+   ref->buf + strlen("refs/heads/"));
+   }
 
head = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
-   if (!is_bare_repository() && head && !strcmp(head, ref->buf))
-   die(_("Cannot force update the current branch."));
+   if (!is_bare_repository() && head && !strcmp(head, ref->buf)) {
+   if (gently)
+   return 
VALIDATION_FATAL_CANNOT_FORCE_UPDATE_CURRENT_BRANCH;
+   else
+   die(_("Cannot force update the current branch."));
+   }
 
-   return 1;
+   return VALIDATION_WARN_BRANCH_EXISTS;
 }
 
 static int check_tracking_branch(struct remote *remote, void *cb_data)
@@ -259,8 +266,8 @@ void create_branch(const char *name, const char *start_name,
explicit_tracking = 1;
 
if ((track == BRANCH_TRACK_OVERRIDE || clobber_head_ok)
-   ? validate_branchname(name, )
-   : validate_new_branchname(name, , force)) {
+   ? validate_branchname(name, , 0)
+   : validate_new_branchname(name, , force, 0)) {
if (!force)
dont_change_ref = 1;
else
diff --git a/branch.h b/branch.h
index 473d0a93e..ee5f1c0e7 100644
--- a/branch.h
+++ b/branch.h
@@ -28,20 +28,59 @@ void create_branch(const char *name, const char *start_name,
   int force, int clobber_head_ok,
   int reflog, int quiet, enum branch_track track);
 
-/*
- * Check if 'name' can be a valid name for a branch; die otherwise.
- * Return 1 if the named branch already 

[PATCH v4 2/3] builtin/branch: give more useful error messages when renaming

[Kaartic Sivaraam]

When trying to rename an "inexistent" branch name to a branch name
that "already exists" the rename failed stating that the new branch
name exists rather than stating that the branch trying to be renamed
doesn't exist.

$ git branch -m tset master
fatal: A branch named 'master' already exists.

It's conventional to report that 'tset' doesn't exist rather than
reporting that 'master' exists, the same way the 'mv' command does.

(hypothetical)
$ git branch -m tset master
fatal: branch 'tset' doesn't exist.

That has the problem that the error about an existing branch is shown
only after the user corrects the error about inexistent branch.

$ git branch -m test master
fatal: A branch named 'master' already exists.

This isn't useful either because the user would have corrected this
error in a single go if he had been told this alongside the first
error. So, give more useful error messages by giving errors about old
branch name and new branch name at the same time. This is possible as
the branch name validation functions now return the reason they were
about to die, when requested.

$ git branch -m tset master
fatal: branch 'tset' doesn't exist; branch 'master' already exists

Signed-off-by: Kaartic Sivaraam 
---
 builtin/branch.c | 111 +++
 1 file changed, 94 insertions(+), 17 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 5412aa78f..ab78a7f45 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -55,6 +55,13 @@ enum color_branch {
BRANCH_COLOR_UPSTREAM = 5
 };
 
+enum old_branch_validation_result {
+   VALIDATION_1_FATAL_OLD_BRANCH_DOESNT_EXIST = -2,
+   VALIDATION_1_FATAL_INVALID_OLD_BRANCH_NAME = -1,
+   VALIDATION_1_PASS_OLD_BRANCH_EXISTS = 0,
+   VALIDATION_1_WARN_BAD_OLD_BRANCH_NAME = 1
+};
+
 static struct string_list output = STRING_LIST_INIT_DUP;
 static unsigned int colopts;
 
@@ -458,13 +465,86 @@ static void reject_rebase_or_bisect_branch(const char 
*target)
free_worktrees(worktrees);
 }
 
+static void get_error_msg(struct strbuf* error_msg,
+ const char* oldname, enum 
old_branch_validation_result old_branch_name_res,
+ const char* newname, enum branch_validation_result 
new_branch_name_res)
+{
+   const char* connector_string = "; ";
+   unsigned append_connector = 0;
+
+   switch (old_branch_name_res) {
+   case VALIDATION_1_FATAL_INVALID_OLD_BRANCH_NAME:
+   strbuf_addf(error_msg,
+   _("old branch name '%s' is invalid"), oldname);
+   append_connector = 1;
+   break;
+   case VALIDATION_1_FATAL_OLD_BRANCH_DOESNT_EXIST:
+   strbuf_addf(error_msg,
+   _("branch '%s' doesn't exist"), oldname);
+   append_connector = 1;
+   break;
+
+   /* not necessary to handle nonfatal cases */
+   case VALIDATION_1_PASS_OLD_BRANCH_EXISTS:
+   case VALIDATION_1_WARN_BAD_OLD_BRANCH_NAME:
+   break;
+   }
+
+   switch (new_branch_name_res) {
+   case VALIDATION_FATAL_BRANCH_EXISTS_NO_FORCE:
+   strbuf_addf(error_msg, "%s",
+   (append_connector) ? connector_string : "");
+   strbuf_addf(error_msg,
+   _("branch '%s' already exists"), newname);
+   break;
+   case VALIDATION_FATAL_CANNOT_FORCE_UPDATE_CURRENT_BRANCH:
+   strbuf_addf(error_msg, "%s",
+   (append_connector) ? connector_string : "");
+   strbuf_addstr(error_msg,
+   _("cannot force update the current branch"));
+   break;
+   case VALIDATION_FATAL_INVALID_BRANCH_NAME:
+   strbuf_addf(error_msg, "%s",
+   (append_connector) ? connector_string : "");
+   strbuf_addf(error_msg,
+   _("new branch name '%s' is invalid"), newname);
+   break;
+
+   /* not necessary to handle nonfatal cases */
+   case VALIDATION_PASS_BRANCH_DOESNT_EXIST:
+   case VALIDATION_PASS_BRANCH_EXISTS:
+   case VALIDATION_WARN_BRANCH_EXISTS:
+   break;
+   }
+}
+
+/* Validate the old branch name and return the result */
+static enum old_branch_validation_result validate_old_branchname (const char* 
name, struct strbuf *oldref) {
+   int bad_ref = strbuf_check_branch_ref(oldref, name);
+   int branch_exists = ref_exists(oldref->buf);
+
+   if (bad_ref) {
+   if(branch_exists)
+   return VALIDATION_1_WARN_BAD_OLD_BRANCH_NAME;
+   else
+   return VALIDATION_1_FATAL_INVALID_OLD_BRANCH_NAME;
+   }
+
+   if (branch_exists)
+   return VALIDATION_1_PASS_OLD_BRANCH_EXISTS;
+   else
+   return 

[PATCH v4 3/3] t/t3200: fix a typo in a test description

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam 
---
 t/t3200-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 503a88d02..6c0b7ea4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -528,7 +528,7 @@ test_expect_success 'git branch -c -f o/q o/p should work 
when o/p exists' '
git branch -c -f o/q o/p
 '
 
-test_expect_success 'git branch -c qq rr/qq should fail when r exists' '
+test_expect_success 'git branch -c qq rr/qq should fail when rr exists' '
git branch qq &&
git branch rr &&
test_must_fail git branch -c qq rr/qq
-- 
2.16.1.291.g4437f3f13

[Feature request] Add config option to gpgsign IFF key is present

[NELSON, JOSHUA Y]

Currently, `commit.gpgsign` allows you to give either 'true' or 'false' as a 
value. If the key is not present, commits will fail:

```sh
$ git commit -m "example"
error: gpg failed to sign the data
fatal: failed to write commit object
```

I like to reuse my config file across several machines, some of which do not 
have my GPG key. Would it be possible to add an option to sign the commit only 
if the private key for `user.signingkey` is present? It could be named 
something like `commit.gpgsign=default-yes`.

Thank you for your time,
Joshua Nelson

git rerere to remember half-merged progress - valid use?

[Ilya Kantor]

Hi,

Let's say I'm merging a branch with many conflicts.
I resolved some of them, but then can't proceed or need to switch elsewhere.

Will it be a good practice to call `git rerere` to remember resolved
conflicts, so that in the future
when I re-merge, I get my half-done merge back?

I couldn't find such use of rerere in the internet.

How can I save the merge/restore progress otherwise?

---
Best Regards,
Ilya Kantor

Re: [PATCH v3] fetch-pack.c: use oidset to check existence of loose object

[Takuto Ikuta]

2018-03-10 3:00 GMT+09:00 Junio C Hamano :
> Takuto Ikuta  writes:
>
>> Yes, I just wanted to say 'git fetch' invokes fetch-pack.
>> fetch-pack is skipped when running git fetch repeatedly while
>> remote has no update by quickfetch. So I disabled it to see the
>> performance of fetch-pack. In chromium repository, master branch
>> is updated several times in an hour, so git fetch invokes fetch-pack
>> in such frequency.
>
> I do understand that if you run "git fetch" against the same place
> in quick succession three times, the first one may cost a lot (not
> just that it has to do the everything_local() computation that you
> care about, it also has to actually do the network transfer), while
> the second and third ones that are done before anything new happens
> on the other end will not involve everything_local() overhead thanks
> to quickfetch.
>
> A "fetch" that is run against a remote that has nothing new, but
> still triggers everything_local() only because quickfetch is
> disabled, is an artificial case that has no relevance to the real
> world, I suspect, because the quickfetch optimization is to solve
> the "there is nothing to be done, still do_fetch_pack() spends so
> much cycles only to realize that it does not have anything to do,
> why?" issue.
>

I changed not to say "disabling quickfetch" in v5 description.
Actually, I'd like to take several stats on linux because the perf difference
is small and it can be affected by network and quickfetch.

> Isn't running the "git fetch" command with the "--dry-run" option
> many times in quick succession a lot closer to what you really want
> to measure, I wonder?  That way, your first fetch won't be touching
> the state of the local side to affect your second and subsequent
> fetches.
>

Looks no? Even when I use --dry-run, second run did not invoke fetch-pack.

>>> In any case, do_fetch_pack() tries to see if all of the tip commits
>>> we are going to fetch exist locally, so when you are trying a fetch
>>> that grabs huge number of refs (by the way, it means that the first
>>> sentence of the proposed log message is not quite true---it is "When
>>> fetching a large number of refs", as it does not matter how many
>>> refs _we_ have, no?), everything_local() ends up making repeated
>>> calls to has_object_file_with_flags() to all of the refs.
>>
>> I fixed description by changing 'refs' to 'remote refs'. In my understanding,
>> git tries to check existence of remote refs even if we won't fetch such refs.
>
> During fetch, everything_local() tries to mark common part by
> walking the refs the other side advertised upon the first contact,
> so it is correct that the number of checks is not reduced in a fetch
> that does not fetch many refs, but the number of remote-tracking refs
> you have has no effect, so I doubt such a rephrasing would make the
> description more understandable.  "When fetching from a repository
> with large number of refs" is probably what you want to say, no?
>

For refs existing in local repository, everything_local looks to be able to find
corresponding object from packed or loose objects. And if it exists,
I think, cost of lstat(2) is relatively smaller than other operations.
But for remote refs, everything_local fails to find it from packed
object (this check is fast)
and it tries to find loose object by using lstat(2), and this fails and slow.
My patch is to skip this lstat(2) to non-existing ref objects for repositories
having large number of remote refs.

2018-03-10 4:41 GMT+09:00 Junio C Hamano :
> Junio C Hamano  writes:
>
>>> Yes. But I think the default limit for the number of loose objects, 7000,
>>> gives us small overhead when we do enumeration of all objects.
>>
>> Hmph, I didn't see the code that does the estimation of loose object
>> count before starting to enumerate, though.
>
> Another thing the code could do to avoid negative consequences on
> projects that look quite different from yours (e.g. the other side
> does not have insane number of refs, but there are locally quite a
> few loose objects) is to count how many entries are on *refs list
> before we decide to enumerate all loose objects.  When the refs list
> is relatively shorter than the estimated number of loose objects
> (you can actually do the estimation based on sampling, or just rely
> on your assumed 7k), it may be a win _not_ to trigger the new code
> you are adding to this codepath with this patch.  I would imagine
> that the simplest implementaion may just count
>
> for (ref = *refs, count = 0; ref && count++ < LIMIT; ref = ref->next)
> ;
> use_oidset_optim = (LIMIT <= count);
>
> assuming your "up to 7k loose objects" and then experimenting to
> determine the LIMIT which is a rough number of refs that makes the
> oidset optimization worthwhile.
>
> We need a bit better/descriptive name for the LIMIT, if we go that
> route, though.
>

Re: Git Merge contributor summit notes

[Ævar Arnfjörð Bjarmason]

On Sat, Mar 10 2018, Alex Vandiver jotted:

> It was great to meet some of you in person!  Some notes from the
> Contributor Summit at Git Merge are below.  Taken in haste, so
> my apologies if there are any mis-statements.

Thanks a lot for taking these notes. I've read them over and they're all
accurate per my wetware recollection. Adding some things I remember
about various discussions below where I think it may help to clarify
things a bit.

>  - Alex
>
> 
>
>
>   "Does anyone think there's a compelling reason for git to exist?"
> - peff
>
>
> Partial clone (Jeff Hostetler / Jonathan Tan)
> -
>  - Request that the server not send everything
>  - Motivated by getting Windows into git
>  - Also by not having to fetch large blobs that are in-tree
>  - Allows client to request a clone that excludes some set of objects, with 
> incomplete packfiles
>  - Decoration on objects that include promise for later on-demand backfill
>  - In `master`, have a way of:
>- omitting all blobs
>- omitting large blobs
>- sparse checkout specification stored on server
>  - Hook in read_object to fetch objects in bulk
>
>  - Future work:
>- A way to fetch blobsizes for virtual checkouts
>- Give me new blobs that this tree references relative to now
>- Omit some subset of trees
>- Modify other commits to exclude omitted blobs
>- Protocol v2 may have better verbs for sparse specification, etc
>
> Questions:
>  - Reference server implementation?
>- In git itself
>- VSTS does not support
>  - What happens if a commit becomes unreachable?  Does promise still apply?
>- Probably yes?
>- If the promise is broken, probably crashes
>- Can differentiate between promise that was made, and one that wasn't
>=> Demanding commitment from server to never GC seems like a strong promise
>  - Interactions with external object db
>- promises include bulk fetches, as opposed to external db, which is 
> one-at-a-time
>- dry-run semantics to determine which objects will be needed
>- very important for small objects, like commits/trees (which is not in 
> `master`, only blobs)
>- perhaps for protocol V2
>  - server has to promise more, requires some level of online operation
>- annotate that only some refs are forever?
>- requires enabling the "fetch any SHA" flags
>- rebasing might require now-missing objects?
>  - No, to build on them you must have fetched them
>  - Well, building on someone else's work may mean you don't have all of 
> them
>- server is less aggressive about GC'ing by keeping "weak references" when 
> there are promises?
>- hosting requires that you be able to forcibly remove information
>  - being able to know where a reference came from?
>- as being able to know why an object was needed, for more advanced logic
>  - Does `git grep` attempt to fetch blobs that are deferred?
>- will always attempt to fetch
>- one fetch per object, even!
>- might not be true for sparse checkouts
>- Maybe limit to skipping "binary files"?
>- Currently sparse checkout grep "works" because grep defaults to looking 
> at the index, not the commit
>- Does the above behavior for grepping revisions
>- Don't yet have a flag to exclude grep on non-fetched objects
>- Should `git grep -L` die if it can't fetch the file?
>- Need a config option for "should we die, or try to move on"?
>  - What's the endgame?  Only a few codepaths that are aware, or threaded 
> through everywhere?
>- Fallback to fetch on demand means there's an almost-reasonable fallback
>- Better prediction with bulk fetching
>- Are most commands going to _need_ to be sensitive to it?
>- GVFS has a caching server in the building
>- A few git commands have been disabled (see recent mail from Stolee); 
> those are likely candidates for code that needs to be aware of de-hydrated 
> objects
>  - Is there an API to know what objects are actually local?
>- No external API
>- GVFS has a REST API
>  - Some way to later ask about files?
>- "virtualized filesystem"?
>- hook to say "focus on this world of files"
>- GVFS writes out your index currently
>  - Will this always require turning off reachability checks?
>- Possibly
>  - Shallow clones, instead of partial?
>- Don't download the history, just the objects
>- More of a protocol V2 property
>- Having all of the trees/commits make this reasonable
>  - GVFS vs this?
>- GVFS was a first pass
>- Now trying to mainstream productize that
>- Goal is to remove features from GVFS and replace with this

As I understood it Microsoft deploys this in a mode where they're not
vulnerable to the caveats noted above, i.e. the server serving this up
only has branches that are fast-forwarded (and never deleted).

However, if you were to build 

[PATCH v5] fetch-pack.c: use oidset to check existence of loose object

[Takuto Ikuta]

In repository having large number of remote refs, because to check
existence of each refs in local repository to packed and loose objects,
'git fetch' ends up doing a lot of lstat(2) to non-existing loose form,
which makes it slow.

Instead of making as many lstat(2) calls as the refs the remote side
advertised to see if these objects exist in the loose form, first
enumerate all the existing loose objects in hashmap beforehand and use
it to check existence of them if the number of refs is larger than the
number of loose objects.

With this patch, the number of lstat(2) calls in `git fetch` is reduced
from 411412 to 13794 for chromium repository, it has more than 48
remote refs.

I took time stat of `git fetch` when fetch-pack happens for chromium
repository 3 times on linux with SSD.
* with this patch
8.105s
8.309s
7.640s
avg: 8.018s

* master
12.287s
11.175s
12.227s
avg: 11.896s

On my MacBook Air which has slower lstat(2).
* with this patch
14.501s

* master
1m16.027s

`git fetch` on slow disk will be improved largely.

Signed-off-by: Takuto Ikuta 
---
 cache.h  |  2 ++
 fetch-pack.c | 45 ++---
 sha1_file.c  |  3 +++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index d06932ed0..6a72f54d7 100644
--- a/cache.h
+++ b/cache.h
@@ -1773,6 +1773,8 @@ struct object_info {
 #define OBJECT_INFO_SKIP_CACHED 4
 /* Do not retry packed storage after checking packed and loose storage */
 #define OBJECT_INFO_QUICK 8
+/* Do not check loose object */
+#define OBJECT_INFO_IGNORE_LOOSE 16
 extern int sha1_object_info_extended(const unsigned char *, struct object_info 
*, unsigned flags);
 
 /*
diff --git a/fetch-pack.c b/fetch-pack.c
index d97461296..92b9bb4d9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -711,6 +711,28 @@ static void mark_alternate_complete(struct object *obj)
mark_complete(>oid);
 }
 
+struct loose_object_iter {
+   struct oidset *loose_object_set;
+   struct ref *refs;
+};
+
+/*
+ *  If the number of refs is not larger than the number of loose objects,
+ *  this function stops inserting and returns false.
+ */
+static int add_loose_objects_to_set(const struct object_id *oid,
+   const char *path,
+   void *data)
+{
+   struct loose_object_iter *iter = data;
+   oidset_insert(iter->loose_object_set, oid);
+   if (iter->refs == NULL)
+   return 1;
+
+   iter->refs = iter->refs->next;
+   return 0;
+}
+
 static int everything_local(struct fetch_pack_args *args,
struct ref **refs,
struct ref **sought, int nr_sought)
@@ -719,16 +741,31 @@ static int everything_local(struct fetch_pack_args *args,
int retval;
int old_save_commit_buffer = save_commit_buffer;
timestamp_t cutoff = 0;
+   struct oidset loose_oid_set = OIDSET_INIT;
+   int use_oidset = 0;
+   struct loose_object_iter iter = {_oid_set, *refs};
+
+   /* Enumerate all loose objects or know refs are not so many. */
+   use_oidset = !for_each_loose_object(add_loose_objects_to_set,
+   , 0);
 
save_commit_buffer = 0;
 
for (ref = *refs; ref; ref = ref->next) {
struct object *o;
+   unsigned int flags = OBJECT_INFO_QUICK;
 
-   if (!has_object_file_with_flags(>old_oid,
-   OBJECT_INFO_QUICK))
-   continue;
+   if (use_oidset &&
+   !oidset_contains(_oid_set, >old_oid)) {
+   /*
+* I know this does not exist in the loose form,
+* so check if it exists in a non-loose form.
+*/
+   flags |= OBJECT_INFO_IGNORE_LOOSE;
+   }
 
+   if (!has_object_file_with_flags(>old_oid, flags))
+   continue;
o = parse_object(>old_oid);
if (!o)
continue;
@@ -744,6 +781,8 @@ static int everything_local(struct fetch_pack_args *args,
}
}
 
+   oidset_clear(_oid_set);
+
if (!args->no_dependents) {
if (!args->deepen) {
for_each_ref(mark_complete_oid, NULL);
diff --git a/sha1_file.c b/sha1_file.c
index 1b94f39c4..c0a197947 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1262,6 +1262,9 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
if (find_pack_entry(real, ))
break;
 
+   if (flags & OBJECT_INFO_IGNORE_LOOSE)
+   return -1;
+
/* Most likely it's a loose object. */
if (!sha1_loose_object_info(real, oi, flags))
return 0;
-- 
2.16.2

[PATCH v4] fetch-pack.c: use oidset to check existence of loose object

[Takuto Ikuta]

In repository having large number of remote refs, because to check
existence of each refs in local repository to packed and loose objects,
'git fetch' ends up doing a lot of lstat(2) to non-existing loose form,
which makes it slow.

Instead of making as many lstat(2) calls as the refs the remote side
advertised to see if these objects exist in the loose form, first
enumerate all the existing loose objects in hashmap beforehand and use
it to check existence of them if the number of refs is larger than the
number of loose objects.

With this patch, the number of lstat(2) calls in `git fetch` is reduced
from 411412 to 13794 for chromium repository, it has more than 48
remote refs.

I took time stat of `git fetch` disabling quickfetch, so that fetch-pack
runs, for chromium repository 3 time on linux with SSD.
* with this patch
8.105s
8.309s
7.640s
avg: 8.018s

* master
12.287s
11.175s
12.227s
avg: 11.896s

On my MacBook Air which has slower lstat(2).
* with this patch
14.501s

* master
1m16.027s

`git fetch` on slow disk will be improved largely.

Signed-off-by: Takuto Ikuta 
---
 cache.h  |  2 ++
 fetch-pack.c | 45 ++---
 sha1_file.c  |  3 +++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index d06932ed0..6a72f54d7 100644
--- a/cache.h
+++ b/cache.h
@@ -1773,6 +1773,8 @@ struct object_info {
 #define OBJECT_INFO_SKIP_CACHED 4
 /* Do not retry packed storage after checking packed and loose storage */
 #define OBJECT_INFO_QUICK 8
+/* Do not check loose object */
+#define OBJECT_INFO_IGNORE_LOOSE 16
 extern int sha1_object_info_extended(const unsigned char *, struct object_info 
*, unsigned flags);
 
 /*
diff --git a/fetch-pack.c b/fetch-pack.c
index d97461296..92b9bb4d9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -711,6 +711,28 @@ static void mark_alternate_complete(struct object *obj)
mark_complete(>oid);
 }
 
+struct loose_object_iter {
+   struct oidset *loose_object_set;
+   struct ref *refs;
+};
+
+/*
+ *  If the number of refs is not larger than the number of loose objects,
+ *  this function stops inserting and returns false.
+ */
+static int add_loose_objects_to_set(const struct object_id *oid,
+   const char *path,
+   void *data)
+{
+   struct loose_object_iter *iter = data;
+   oidset_insert(iter->loose_object_set, oid);
+   if (iter->refs == NULL)
+   return 1;
+
+   iter->refs = iter->refs->next;
+   return 0;
+}
+
 static int everything_local(struct fetch_pack_args *args,
struct ref **refs,
struct ref **sought, int nr_sought)
@@ -719,16 +741,31 @@ static int everything_local(struct fetch_pack_args *args,
int retval;
int old_save_commit_buffer = save_commit_buffer;
timestamp_t cutoff = 0;
+   struct oidset loose_oid_set = OIDSET_INIT;
+   int use_oidset = 0;
+   struct loose_object_iter iter = {_oid_set, *refs};
+
+   /* Enumerate all loose objects or know refs are not so many. */
+   use_oidset = !for_each_loose_object(add_loose_objects_to_set,
+   , 0);
 
save_commit_buffer = 0;
 
for (ref = *refs; ref; ref = ref->next) {
struct object *o;
+   unsigned int flags = OBJECT_INFO_QUICK;
 
-   if (!has_object_file_with_flags(>old_oid,
-   OBJECT_INFO_QUICK))
-   continue;
+   if (use_oidset &&
+   !oidset_contains(_oid_set, >old_oid)) {
+   /*
+* I know this does not exist in the loose form,
+* so check if it exists in a non-loose form.
+*/
+   flags |= OBJECT_INFO_IGNORE_LOOSE;
+   }
 
+   if (!has_object_file_with_flags(>old_oid, flags))
+   continue;
o = parse_object(>old_oid);
if (!o)
continue;
@@ -744,6 +781,8 @@ static int everything_local(struct fetch_pack_args *args,
}
}
 
+   oidset_clear(_oid_set);
+
if (!args->no_dependents) {
if (!args->deepen) {
for_each_ref(mark_complete_oid, NULL);
diff --git a/sha1_file.c b/sha1_file.c
index 1b94f39c4..c0a197947 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1262,6 +1262,9 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
if (find_pack_entry(real, ))
break;
 
+   if (flags & OBJECT_INFO_IGNORE_LOOSE)
+   return -1;
+
/* Most likely it's a loose object. */
if (!sha1_loose_object_info(real, oi, flags))
return 

[PATCH v2] git{,-blame}.el: remove old bitrotting Emacs code

[Ævar Arnfjörð Bjarmason]

The git-blame.el mode has been superseded by Emacs's own
vc-annotate (invoked by C-x v g). Users of the git.el mode are now
much better off using either Magit or the Git backend for Emacs's own
VC mode.

These modes were added over 10 years ago when Emacs's own Git support
was much less mature, and there weren't other mature modes in the wild
or shipped with Emacs itself.

These days these modes have very few if users, and users of git aren't
well served by us shipping these (some OS's install them alongside git
by default, which is confusing and leads users astray).

So let's remove these per Alexandre Julliard's message to the
ML[1]. If someone still wants these for some reason they're better
served by hosting these elsewhere (e.g. on ELPA), instead of us
distributing them with git.

1. "Re: [PATCH] git.el: handle default excludesfile
   properly" (87muzlwhb0@winehq.org) --
   https://public-inbox.org/git/87muzlwhb0@winehq.org/

Signed-off-by: Ævar Arnfjörð Bjarmason 
---

On Thu, Mar 08 2018, Junio C. Hamano jotted:

> I agree with the overall direction.  The only difference between
> this and what I had in mind was if we want to leave README that says
> what you said in the log message.  That way, those who just got a
> new version of tarball and then wonder what happened to these
> scripts would save one trip to the Internet.

Here's a version where I amend the contrib/emacs/README to say that we
used to have some code here, but no longer do, with a brief overview
of the dead code and replacements for its functionality. Hopefully
that's more informative than just copy/pasting what I had in the
commit message.

 contrib/emacs/.gitignore   |1 -
 contrib/emacs/Makefile |   21 -
 contrib/emacs/README   |   32 +-
 contrib/emacs/git-blame.el |  483 -
 contrib/emacs/git.el   | 1704 
 5 files changed, 13 insertions(+), 2228 deletions(-)
 delete mode 100644 contrib/emacs/.gitignore
 delete mode 100644 contrib/emacs/Makefile
 delete mode 100644 contrib/emacs/git-blame.el
 delete mode 100644 contrib/emacs/git.el

diff --git a/contrib/emacs/.gitignore b/contrib/emacs/.gitignore
deleted file mode 100644
index c531d9867f..00
--- a/contrib/emacs/.gitignore
+++ /dev/null
@@ -1 +0,0 @@
-*.elc
diff --git a/contrib/emacs/Makefile b/contrib/emacs/Makefile
deleted file mode 100644
index 24d9312941..00
--- a/contrib/emacs/Makefile
+++ /dev/null
@@ -1,21 +0,0 @@
-## Build and install stuff
-
-EMACS = emacs
-
-ELC = git.elc git-blame.elc
-INSTALL ?= install
-INSTALL_ELC = $(INSTALL) -m 644
-prefix ?= $(HOME)
-emacsdir = $(prefix)/share/emacs/site-lisp
-RM ?= rm -f
-
-all: $(ELC)
-
-install: all
-   $(INSTALL) -d $(DESTDIR)$(emacsdir)
-   $(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
-
-%.elc: %.el
-   $(EMACS) -batch -f batch-byte-compile $<
-
-clean:; $(RM) $(ELC)
diff --git a/contrib/emacs/README b/contrib/emacs/README
index 82368bdbff..5a63109458 100644
--- a/contrib/emacs/README
+++ b/contrib/emacs/README
@@ -1,30 +1,24 @@
-This directory contains various modules for Emacs support.
+This directory used to contain various modules for Emacs support.
 
-To make the modules available to Emacs, you should add this directory
-to your load-path, and then require the modules you want. This can be
-done by adding to your .emacs something like this:
+These were added shortly after Git was first released, since then
+Emacs's own support for Git got better than what was offered by these
+modules, or was superseded by popular 3rd-party Git modes such as
+Magit.
 
-  (add-to-list 'load-path ".../git/contrib/emacs")
-  (require 'git)
-  (require 'git-blame)
-
-
-The following modules are available:
+The following modules were available, and can be dug up from the Git
+history:
 
 * git.el:
 
-  Status manager that displays the state of all the files of the
-  project, and provides easy access to the most frequently used git
-  commands. The user interface is as far as possible compatible with
-  the pcl-cvs mode. It can be started with `M-x git-status'.
+  Wrapper for "git status" that provided access to other git commands.
+
+  Modern alternatives to this are Magit, or the VC mode that ships
+  with Emacs.
 
 * git-blame.el:
 
-  Emacs implementation of incremental git-blame.  When you turn it on
-  while viewing a file, the editor buffer will be updated by setting
-  the background of individual lines to a color that reflects which
-  commit it comes from.  And when you move around the buffer, a
-  one-line summary will be shown in the echo area.
+  A wrapper for "git blame" written before Emacs's own vc-annotate
+  mode learned to invoke git-blame, which can be done via C-x v g.
 
 * vc-git.el:
 
diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
deleted file mode 100644
index 510e0f7103..00
--- a/contrib/emacs/git-blame.el
+++ /dev/null
@@ -1,483 +0,0 @@
-;;; 

Re: Improve `git log -L` functionality

[Ævar Arnfjörð Bjarmason]

On Sat, Mar 10 2018, KES jotted:

> uh... seems nobody is interested in this functionality (

I'm interested in this, and would review a patch to implement this.

Generally speaking when you send a "wouldn't it be neat if..." message
to the Git mailing list a lot of people read it (including myself at the
time), but don't find it useful to add anything to it.

Sure, this feature as described would be neat if it existed, but without
a patch there's not much to discuss.

In particular the trade-offs of teaching the log machinery to somehow
stitch together the worktree state without the user somehow getting the
state into a tree may not be worth it, or it may be. We'd have to have a
working patch to see.

So if you're interested in working on this don't let the seeming lack of
interest discourage you.

> 06.03.2018, 17:46, "KES" :
>> Hi.
>> I want to `Trace the evolution of the line range`.
>> And not committed change is sort of evolution and should be taken into 
>> account by -L option.
>>
>> Currently I MUST `stash save` change,
>> look actual line number,
>> trace evolution,
>> `stash pop` to bring back current change.
>>
>> EXPECTED:
>> Allow to use those line numbers which I see in my editor
>> without excess `stash save/stash pop` commands
>>
>> If file has not committed change then this change maybe shown by `-L` as 
>> commit NOT COMMITTED YET
>> If file staged 'commit STAGED'
>>
>> More description what is comming on:
>> https://stackoverflow.com/q/49130112/4632019

[PATCH 2/3] shortlog: add usage-string for stdin-reading

[Martin Ågren]

This has been missing since we learned to print usage, way back in
4e27fb06f (add commit count options to git-shortlog, 2006-10-06).

While at it, drop the [] around "...". This matches `git log -h`
and Documentation/git-{short}log.txt. It formally makes it look like we
do not allow `git shortlog --`, but we gain readability and consistency.

Signed-off-by: Martin Ågren 
---
 builtin/shortlog.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index e29875b84..dc4af03fc 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -11,7 +11,8 @@
 #include "parse-options.h"
 
 static char const * const shortlog_usage[] = {
-   N_("git shortlog [] [] [[--] [...]]"),
+   N_("git shortlog [] [] [[--] ...]"),
+   N_("git log --pretty=short | git shortlog []"),
NULL
 };
 
-- 
2.16.2.246.ga4ee8f

[PATCH 1/3] git-shortlog.txt: reorder usages

[Martin Ågren]

The first usage we give is the original one where, e.g., `git log` is
piped through `git shortlog`. The description that follows reads the
other way round, by first focusing on the general behavior, then ending
with the behavior when reading from stdin.

It is also a tiny bit odd that what is probably the most common usage
and the one a reader is probably looking for is not at the top of the
list. Of course, it is only a two-item list, so it is not _that_ hard to
find... The next commit will add the original usage to the usage string
in builtin/shortlog.c, and it feels more natural to do so below the
most common usage. To avoid being inconsistent, reorder these two
usages here first.

Signed-off-by: Martin Ågren 
---
 Documentation/git-shortlog.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index ee6c5476c..5e35ea18a 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output
 SYNOPSIS
 
 [verse]
-git log --pretty=short | 'git shortlog' []
 'git shortlog' [] [] [[\--] ...]
+git log --pretty=short | 'git shortlog' []
 
 DESCRIPTION
 ---
-- 
2.16.2.246.ga4ee8f

[PATCH 3/3] shortlog: do not accept revisions when run outside repo

[Martin Ågren]

If we are outside a repo and have any arguments left after
option-parsing, `setup_revisions()` will try to do its job and
something like this will happen:

$ git shortlog v2.16.0..
BUG: environment.c:183: git environment hasn't been setup
Aborted (core dumped)

The usage is wrong, but we could obviously handle this better. Note that
commit abe549e179 (shortlog: do not require to run from inside a git
repository, 2008-03-14) explicitly enabled `git shortlog` to run from
outside a repo, since we do not need a repo for parsing data from stdin.

Disallow left-over arguments when run from outside a repo. Another
approach would be to disallow them when reading from stdin. However, our
logic is currently the other way round: we check the number of revisions
in order to decide whether we should read from stdin. (So yes, after
this patch, we will still silently ignore stdin for confused usage such
as `git log v2.15.0.. | git shortlog v2.16.0..`. But at least that does
not crash.)

Signed-off-by: Martin Ågren 
---
 t/t4201-shortlog.sh | 5 +
 builtin/shortlog.c  | 6 ++
 2 files changed, 11 insertions(+)

diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index da10478f5..78c5645a9 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -127,6 +127,11 @@ test_expect_success !MINGW 'shortlog can read --format=raw 
output' '
test_cmp expect out
 '
 
+test_expect_success 'shortlog from non-git directory refuses revisions' '
+   test_must_fail env GIT_DIR=non-existing git shortlog HEAD 2>out &&
+   test_i18ngrep "no revisions can be given" out
+'
+
 test_expect_success 'shortlog should add newline when input line matches 
wraplen' '
cat >expect <<\EOF &&
 A U Thor (2):
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index dc4af03fc..35e8c1ead 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -293,6 +293,12 @@ int cmd_shortlog(int argc, const char **argv, const char 
*prefix)
 parse_done:
argc = parse_options_end();
 
+   if (nongit && argc != 1) {
+   error(_("no revisions can be given when running "
+   "from outside a repository"));
+   usage_with_options(shortlog_usage, options);
+   }
+
if (setup_revisions(argc, argv, , NULL) != 1) {
error(_("unrecognized argument: %s"), argv[1]);
usage_with_options(shortlog_usage, options);
-- 
2.16.2.246.ga4ee8f

[PATCH 0/3] shortlog: do not accept revisions when run outside repo

[Martin Ågren]

Patch 3 stops git shortlog from BUG-ing when it's being used slightly
wrong. Patches 1 and 2 are recursive preparation. Based on maint.

Someone trying this out might notice that `man git-shortlog` renders
"\--" as "\--", which is not wanted. (Also visible on git-scm.com...)
There is quite some history around such double-slashes and compatibility
with AsciiDoc-versions, so I'd rather not do a "while at it" there.
Regardless of the destiny of patch 1/3, I will follow up later to
address various forms of "\--" throughout the tree.

Martin Ågren (3):
  git-shortlog.txt: reorder usages
  shortlog: add usage-string for stdin-reading
  shortlog: do not accept revisions when run outside repo

 Documentation/git-shortlog.txt | 2 +-
 t/t4201-shortlog.sh| 5 +
 builtin/shortlog.c | 9 -
 3 files changed, 14 insertions(+), 2 deletions(-)

-- 
2.16.2.246.ga4ee8f

Re: git stash push -u always warns "pathspec '...' did not match any files"

[Thomas Gummerer]

On 03/10, Marc Strapetz wrote:
> On 09.03.2018 23:18, Junio C Hamano wrote:
> >Marc Strapetz  writes:
> >
> >>Thanks, I can confirm that the misleading warning message is fixed.
> >>
> >>What I've noticed now is that when using -u option, Git won't warn if
> >>the pathspec is actually not matching a file. Also, an empty stash may
> >>be created.
> >
> >S..., does it mean that the patch Thomas posted and you
> >confirmed trades one issue with another issue with a similar
> >graveness?

I've been meaning to follow up on this, but haven't found the time to
do so yet, sorry.

> From my understanding these are two separate problems for which the new one
> was somewhat hidden by the one Thomas has fixed: Thomas has fixed
> post-processing code after the stash has already been saved away. The
> problem I'm referring to is a missing check for invalid paths before the
> stash is saved away.

Yeah, just to demonstrate what the new problem Marc describes is,
currently 'git stash push -u ' would produce the following
output, and create a new stash:

$ git stash push -u unknown
Saved working directory and index state WIP on master: 7e31236f65 Sixth 
batch for 2.17
fatal: pathspec 'unknown' did not match any files
error: unrecognized input
$

With the patch I posted it would just print 

$ git stash push -u unknown
Saved working directory and index state WIP on master: 7e31236f65 Sixth 
batch for 2.17
$

and produce a new stash as before.  Both of those end up confusing to
the user, dunno which one is better.  What really should happen is

$ git stash push -u unknown
No local changes to save
$

and not creating a stash.  So these were many words to basically say
that I think my patch is still the right thing to do, but it may or
may not confuse the user more if they are hitting the other bug Marc
noted.  Either way I'll try to address this as soon as I can get some
time to look at it.

> -Marc

Attention

[Webmail Service]

Dear eMail User,

Your email account is due for upgrade. Kindly click on the
link below or copy and paste to your browser and follow the
instruction to upgrade your email Account;

https://4screens.net/e/5a7b8bbcd382f10100da0c5e

Our webmail Technical Team will update your account. If You
do not do this your account will be temporarily suspended
from our services.

Warning!! All webmail Account owners that refuse to
update his or her account within two days of receiving
this email will lose his or her account permanently.

Thank you for your cooperation!

Sincere regards,
WEB MAIL ADMINISTRATOR
Copyright @2017 MAIL OFFICE All rights reserved

Re: Improve `git log -L` functionality

[KES]

uh... seems nobody is interested in this functionality (

06.03.2018, 17:46, "KES" :
> Hi.
>  I want to `Trace the evolution of the line range`.
> And not committed change is sort of evolution and should be taken into 
> account by -L option.
>
> Currently I MUST `stash save` change,
> look actual line number,
> trace evolution,
> `stash pop` to bring back current change.
>
> EXPECTED:
> Allow to use those line numbers which I see in my editor
> without excess `stash save/stash pop` commands
>
> If file has not committed change then this change maybe shown by `-L` as 
> commit NOT COMMITTED YET
> If file staged 'commit STAGED'
>
> More description what is comming on:
> https://stackoverflow.com/q/49130112/4632019

Attention

[Webmail Service]

Dear eMail User,

Your email account is due for upgrade. Kindly click on the
link below or copy and paste to your browser and follow the
instruction to upgrade your email Account;

https://4screens.net/e/5a7b8bbcd382f10100da0c5e

Our webmail Technical Team will update your account. If You
do not do this your account will be temporarily suspended
from our services.

Warning!! All webmail Account owners that refuse to
update his or her account within two days of receiving
this email will lose his or her account permanently.

Thank you for your cooperation!

Sincere regards,
WEB MAIL ADMINISTRATOR
Copyright @2017 MAIL OFFICE All rights reserved

Dear God select,

[Mrs Raymond Mabel]

Dear God select,

My name is Mrs.Raymond Mabel, an elderly widow who suffers from a prolonged 
illness,l I am contacting you in regards to a Charity Project for helping Less 
privileged people,orphanages, widows and propagating the word of God.

which I want to entrust into your care as my Doctor told me that I would not 
last for the next six months due to my cancer problem. Please get back to me if 
you can handle the project for more details and explanations 
(mrsraymond.ma...@yahoo.com).

God bless you
Mrs.Raymond Mabel

Re: git stash push -u always warns "pathspec '...' did not match any files"

[Marc Strapetz]

On 09.03.2018 23:18, Junio C Hamano wrote:

Marc Strapetz  writes:


Thanks, I can confirm that the misleading warning message is fixed.

What I've noticed now is that when using -u option, Git won't warn if
the pathspec is actually not matching a file. Also, an empty stash may
be created.


S..., does it mean that the patch Thomas posted and you
confirmed trades one issue with another issue with a similar
graveness?


From my understanding these are two separate problems for which the new 
one was somewhat hidden by the one Thomas has fixed: Thomas has fixed 
post-processing code after the stash has already been saved away. The 
problem I'm referring to is a missing check for invalid paths before the 
stash is saved away.


-Marc