[Market Info] Now is your chance to exhibit in Japan!

[Eiichi Hasegawa LIFESTYLE EXPO TOKYO Show Management]

Dear International Sales & Marketing Director
Zhejiang Wuchuan Industrial Co., Ltd,

Hello, this is Eiichi Hasegawa from LIFESTYLE EXPO TOKYO 2019 Show Management.

Today, I would like to share the information of the growing Japanese gift 
market, and the buyers that you can meet at our show.
Please take a look at the market information below, and consider exhibiting at 
LIFESTYLE EXPO TOKYO 2019 [January]!

-
< NOW is your time to tackle Japan >
-
1) Strong growth in the Japanese Gift Market.


  US$ 87 billion　→　US$ 93 billion
  That is over 6.8% growth rate! (*1)
 
2) Strong demand for Imported Gift Products in Japan.


　US$ 20 billion　→　US$ 21 billion
　That is a 3.4% growth rate, which is approximately more than 2 billion US 
dollars! (*1)

-
< Meet the visitors! >
-

At LIFESTYLE EXPO TOKYO, you can meet:
1) Importers/Distributors
   -- ANA TRADING, ITOCHU, MARUBENI INTEX, MITSUBISHI CORP. FASHION, etc.

2) Buyers
   -- Gift/Stationery Shops: ITO-YA, LOFT, MUJI, PLAZA, TOKYU HANDS, etc.
   -- Interior Shops/Design Stores: MOMA DESIGN STORE, FRANCFRANC, ACTUS, 
KEYUCA, etc.
   -- Concept Shops/Apparel Shops: BEAMS, FREAK’S STORE, SHIPS, URBAN RESEARCH, 
etc.
   -- Department Stores: DAIMARU MATSUZAKAYA, ISETAN MITSUKOSHI, TAKASHIMAYA, 
etc.

3) Key contacts for OEM or Contract Manufacturing 
   -- Major Japanese brands, Mass retailers, GMS, etc.

and more! 

* visitors are excerpts from the 2018 July show
-

For more information, please kindly fill in the REPLY FORM below.
We will be happy to get back to you for details.

We look forward to your reply.

== Reply Form 
mailto:lifestyle-...@reedexpo.co.jp
Company Name:
Contact Person:
Email:
TEL:
Main Products:

Your Request
(  ) Exhibiting Cost ( sqm)
(  ) Available Booth Locations
(  ) Other ( )
===

Please forward this message to the person responsible for marketing/export if 
needed.


Best regards,



Eiichi Hasegawa (Mr.), Chisato Miyawaki (Ms.), Mikako Shimada (Ms.)
Qu Jun (Mr.), Choi Ilyong (Mr.)
LIFESTYLE EXPO TOKYO Show Management
Reed Exhibitions Japan Ltd.
TEL: +81-3-3349-8505
mailto:lifestyle-...@reedexpo.co.jp

---
LIFESTYLE EXPO TOKYO 2019 [January] 
Jan. 30 (Wed.) - Feb. 1 (Fri.), 2019, Makuhari Messe, Japan
https://www.lifestyle-expo-spring.jp/en/
---

(*1: Figures from Yano Research Institute Ltd.)

ID:E36-G1402-0075
















This message is delivered to you to provide details of exhibitions and 
conferences organised, co-organised, or managed by Reed Exhibitions Japan Ltd.
If you would like to change your contact information, or prefer not to receive 
further information on this exhibition/conference, please follow the directions 
below.

 
Please click the URL below and follow the directions on the website to update 
your e-mail and other information.
https://contact.reedexpo.co.jp/expo/REED/?lg=en=ch=CHANGE


Please reply to this mail changing the subject to "REMOVE FROM LIST".
You will not receive any further information on this exhibition/conference.

[PATCH v4 1/3] repack: point out a bug handling stale shallow info

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin 

A `git fetch --prune` can turn previously-reachable objects unreachable,
even commits that are in the `shallow` list. A subsequent `git repack
-ad` will then unceremoniously drop those unreachable commits, and the
`shallow` list will become stale. This means that when we try to fetch
with a larger `--depth` the next time, we may end up with:

fatal: error in object: unshallow 

Reported by Alejandro Pauly.

Signed-off-by: Johannes Schindelin 
---
 t/t5537-fetch-shallow.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 7045685e2..686fdc928 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -186,6 +186,33 @@ EOF
test_cmp expect actual
 '
 
+test_expect_failure '.git/shallow is edited by repack' '
+   git init shallow-server &&
+   test_commit -C shallow-server A &&
+   test_commit -C shallow-server B &&
+   git -C shallow-server checkout -b branch &&
+   test_commit -C shallow-server C &&
+   test_commit -C shallow-server E &&
+   test_commit -C shallow-server D &&
+   d="$(git -C shallow-server rev-parse --verify D^0)" &&
+   git -C shallow-server checkout master &&
+
+   git clone --depth=1 --no-tags --no-single-branch \
+   "file://$PWD/shallow-server" shallow-client &&
+
+   : now remove the branch and fetch with prune &&
+   git -C shallow-server branch -D branch &&
+   git -C shallow-client fetch --prune --depth=1 \
+   origin "+refs/heads/*:refs/remotes/origin/*" &&
+   git -C shallow-client repack -adfl &&
+   test_must_fail git -C shallow-client rev-parse --verify $d^0 &&
+   ! grep $d shallow-client/.git/shallow &&
+
+   git -C shallow-server branch branch-orig $d &&
+   git -C shallow-client fetch --prune --depth=2 \
+   origin "+refs/heads/*:refs/remotes/origin/*"
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
gitgitgadget

Re: [PATCH v3 1/3] repack: point out a bug handling stale shallow info

[Johannes Schindelin]

Hi Junio,

me again. I was wrong.

On Wed, 24 Oct 2018, Johannes Schindelin wrote:

> On Wed, 24 Oct 2018, Junio C Hamano wrote:
> 
> > "Johannes Schindelin via GitGitGadget" 
> > writes:
> > 
> > > +...
> > > + d="$(git -C shallow-server rev-parse --verify D)" &&
> > > + git -C shallow-server checkout master &&
> > > +
> > > +...
> > > + ! grep $d shallow-client/.git/shallow &&
> > 
> > We know D (and $d) is not a tag,
> 
> Actually, it is... the `test_commit D` creates that tag, and that is what
> I use here.
> 
> > but perhaps the place that assigns to $d (way above) can do the
> > rev-parse of D^0, not D, to make it more clear what is going on,
> > especially given that...
> 
> ... that the `grep` really wants to test for the absence of the *commit*,
> not the *tag* in .git/shallow?
> 
> Yes, you are right ;-)
> 
> So why did my test do the right thing, if it looked at a tag, but did not
> dereference it via ^0? The answer is: the `test_commit` function creates
> light-weight tags, i.e. no tag objects. And therefore, the $d^0 you found
> below, that's just confusing.

What I was referring to was the call

test_must_fail git -C shallow-client rev-parse --verify $d^0

However, here we *have* to append ^0, otherwise `rev-parse --verify` will
(and quite confusingly so) *succeed*. I *repeatedly* fall into that trap
that `git rev-parse --verify `
will succeed. Why? Because that is a valid 40-digit hex string. Not
because the object exists. Because it does not.

So I managed to confuse myself again into believing that I only need to
append ^0 to the earlier rev-parse call, but can remove it from this one,
when in reality, I have to append it to both. In the first case, to avoid
having to think about dereferencing a tag, in the second case, to force
rev-parse to look for the object.

Ciao,
Dscho

> 
> I will change it so that the `rev-parse` call uses ^0 (even if it is
> technically not necessary), to avoid said confusion.
> 
> Thanks,
> Dscho
> 
> > 
> > > + git -C shallow-server branch branch-orig D^0 &&
> > 
> > ... this does that D^0 thing here to makes us wonder if D needs
> > unwrapping before using it as a commit (not commit-ish). 
> > 
> > If we did so, we could use $d here instead of D^0.
> > 
> > > + git -C shallow-client fetch --prune --depth=2 \
> > > + origin "+refs/heads/*:refs/remotes/origin/*"
> > > +'
> > > +
> > >  . "$TEST_DIRECTORY"/lib-httpd.sh
> > >  start_httpd
> > 
> > 
> 

Re: [PATCH v3 1/3] repack: point out a bug handling stale shallow info

[Johannes Schindelin]

Hi Junio,

On Wed, 24 Oct 2018, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" 
> writes:
> 
> > +...
> > +   d="$(git -C shallow-server rev-parse --verify D)" &&
> > +   git -C shallow-server checkout master &&
> > +
> > +...
> > +   ! grep $d shallow-client/.git/shallow &&
> 
> We know D (and $d) is not a tag,


Actually, it is... the `test_commit D` creates that tag, and that is what
I use here.

> but perhaps the place that assigns to $d (way above) can do the
> rev-parse of D^0, not D, to make it more clear what is going on,
> especially given that...

... that the `grep` really wants to test for the absence of the *commit*,
not the *tag* in .git/shallow?

Yes, you are right ;-)

So why did my test do the right thing, if it looked at a tag, but did not
dereference it via ^0? The answer is: the `test_commit` function creates
light-weight tags, i.e. no tag objects. And therefore, the $d^0 you found
below, that's just confusing.

I will change it so that the `rev-parse` call uses ^0 (even if it is
technically not necessary), to avoid said confusion.

Thanks,
Dscho

> 
> > +   git -C shallow-server branch branch-orig D^0 &&
> 
> ... this does that D^0 thing here to makes us wonder if D needs
> unwrapping before using it as a commit (not commit-ish). 
> 
> If we did so, we could use $d here instead of D^0.
> 
> > +   git -C shallow-client fetch --prune --depth=2 \
> > +   origin "+refs/heads/*:refs/remotes/origin/*"
> > +'
> > +
> >  . "$TEST_DIRECTORY"/lib-httpd.sh
> >  start_httpd
> 
> 

Re: [PATCH v3 1/3] repack: point out a bug handling stale shallow info

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" 
writes:

> From: Johannes Schindelin 
>
> A `git fetch --prune` can turn previously-reachable objects unreachable,
> even commits that are in the `shallow` list. A subsequent `git repack
> -ad` will then unceremoniously drop those unreachable commits, and the
> `shallow` list will become stale. This means that when we try to fetch
> with a larger `--depth` the next time, we may end up with:
>
>   fatal: error in object: unshallow 

Nicely analysed and described.  One minor thing nagged me in the
implementation but it is not a big issue.

> +...
> + d="$(git -C shallow-server rev-parse --verify D)" &&
> + git -C shallow-server checkout master &&
> +
> +...
> + ! grep $d shallow-client/.git/shallow &&

We know D (and $d) is not a tag, but perhaps the place that assigns
to $d (way above) can do the rev-parse of D^0, not D, to make it
more clear what is going on, especially given that...

> + git -C shallow-server branch branch-orig D^0 &&

... this does that D^0 thing here to makes us wonder if D needs
unwrapping before using it as a commit (not commit-ish). 

If we did so, we could use $d here instead of D^0.

> + git -C shallow-client fetch --prune --depth=2 \
> + origin "+refs/heads/*:refs/remotes/origin/*"
> +'
> +
>  . "$TEST_DIRECTORY"/lib-httpd.sh
>  start_httpd

Re: [PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence

[Junio C Hamano]

"brian m. carlson"  writes:

> On Mon, Oct 22, 2018 at 06:38:19PM +0200, Michał Górny wrote:
>> Replace the logic used to determine whether key and signer information
>> is present to use explicit flags in sigcheck_gpg_status[] array.  This
>> is more future-proof, since it makes it possible to add additional
>> statuses without having to explicitly update the conditions.
>
> This series looks good to me.  I was going to ask after patch 2 whether
> you were printing the subkey or primary key fingerprint, and then you
> answered my question in patch 3.  Thanks for including both.

Yeah, this looks good to me too.  Thanks, both.

Re: [PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence

[brian m. carlson]

On Mon, Oct 22, 2018 at 06:38:19PM +0200, Michał Górny wrote:
> Replace the logic used to determine whether key and signer information
> is present to use explicit flags in sigcheck_gpg_status[] array.  This
> is more future-proof, since it makes it possible to add additional
> statuses without having to explicitly update the conditions.

This series looks good to me.  I was going to ask after patch 2 whether
you were printing the subkey or primary key fingerprint, and then you
answered my question in patch 3.  Thanks for including both.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

[PATCH v3 1/3] repack: point out a bug handling stale shallow info

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin 

A `git fetch --prune` can turn previously-reachable objects unreachable,
even commits that are in the `shallow` list. A subsequent `git repack
-ad` will then unceremoniously drop those unreachable commits, and the
`shallow` list will become stale. This means that when we try to fetch
with a larger `--depth` the next time, we may end up with:

fatal: error in object: unshallow 

Reported by Alejandro Pauly.

Signed-off-by: Johannes Schindelin 
---
 t/t5537-fetch-shallow.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 7045685e2..2d0031703 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -186,6 +186,33 @@ EOF
test_cmp expect actual
 '
 
+test_expect_failure '.git/shallow is edited by repack' '
+   git init shallow-server &&
+   test_commit -C shallow-server A &&
+   test_commit -C shallow-server B &&
+   git -C shallow-server checkout -b branch &&
+   test_commit -C shallow-server C &&
+   test_commit -C shallow-server E &&
+   test_commit -C shallow-server D &&
+   d="$(git -C shallow-server rev-parse --verify D)" &&
+   git -C shallow-server checkout master &&
+
+   git clone --depth=1 --no-tags --no-single-branch \
+   "file://$PWD/shallow-server" shallow-client &&
+
+   : now remove the branch and fetch with prune &&
+   git -C shallow-server branch -D branch &&
+   git -C shallow-client fetch --prune --depth=1 \
+   origin "+refs/heads/*:refs/remotes/origin/*" &&
+   git -C shallow-client repack -adfl &&
+   test_must_fail git -C shallow-client rev-parse --verify $d^0 &&
+   ! grep $d shallow-client/.git/shallow &&
+
+   git -C shallow-server branch branch-orig D^0 &&
+   git -C shallow-client fetch --prune --depth=2 \
+   origin "+refs/heads/*:refs/remotes/origin/*"
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
 
-- 
gitgitgadget

[PATCH 1/3] gpg-interface.c: use flags to determine key/signer info presence

[Michał Górny]

Replace the logic used to determine whether key and signer information
is present to use explicit flags in sigcheck_gpg_status[] array.  This
is more future-proof, since it makes it possible to add additional
statuses without having to explicitly update the conditions.

Signed-off-by: Michał Górny 
---
 gpg-interface.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index d72a43b77..c7cd24ec0 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -77,20 +77,27 @@ void signature_check_clear(struct signature_check *sigc)
 
 /* An exclusive status -- only one of them can appear in output */
 #define GPG_STATUS_EXCLUSIVE   (1<<0)
+/* The status includes key identifier */
+#define GPG_STATUS_KEYID   (1<<1)
+/* The status includes user identifier */
+#define GPG_STATUS_UID (1<<2)
+
+/* Short-hand for standard exclusive *SIG status with keyid & UID */
+#define GPG_STATUS_STDSIG  
(GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID|GPG_STATUS_UID)
 
 static struct {
char result;
const char *check;
unsigned int flags;
 } sigcheck_gpg_status[] = {
-   { 'G', "GOODSIG ", GPG_STATUS_EXCLUSIVE },
-   { 'B', "BADSIG ", GPG_STATUS_EXCLUSIVE },
+   { 'G', "GOODSIG ", GPG_STATUS_STDSIG },
+   { 'B', "BADSIG ", GPG_STATUS_STDSIG },
{ 'U', "TRUST_NEVER", 0 },
{ 'U', "TRUST_UNDEFINED", 0 },
-   { 'E', "ERRSIG ", GPG_STATUS_EXCLUSIVE },
-   { 'X', "EXPSIG ", GPG_STATUS_EXCLUSIVE },
-   { 'Y', "EXPKEYSIG ", GPG_STATUS_EXCLUSIVE },
-   { 'R', "REVKEYSIG ", GPG_STATUS_EXCLUSIVE },
+   { 'E', "ERRSIG ", GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID },
+   { 'X', "EXPSIG ", GPG_STATUS_STDSIG },
+   { 'Y', "EXPKEYSIG ", GPG_STATUS_STDSIG },
+   { 'R', "REVKEYSIG ", GPG_STATUS_STDSIG },
 };
 
 static void parse_gpg_output(struct signature_check *sigc)
@@ -117,13 +124,13 @@ static void parse_gpg_output(struct signature_check *sigc)
}
 
sigc->result = sigcheck_gpg_status[i].result;
-   /* The trust messages are not followed by 
key/signer information */
-   if (sigc->result != 'U') {
+   /* Do we have key information? */
+   if (sigcheck_gpg_status[i].flags & 
GPG_STATUS_KEYID) {
next = strchrnul(line, ' ');
free(sigc->key);
sigc->key = xmemdupz(line, next - line);
-   /* The ERRSIG message is not followed 
by signer information */
-   if (*next && sigc->result != 'E') {
+   /* Do we have signer information? */
+   if (*next && 
(sigcheck_gpg_status[i].flags & GPG_STATUS_UID)) {
line = next + 1;
next = strchrnul(line, '\n');
free(sigc->signer);
-- 
2.19.1

wrong info from 'git remote show ' about 'git push' configuration

[alexander barakin (aka sash-kan)]

"git remote show " shows wrong information
about not yet configured for 'git push' local ref.

steps to reproduce:

$ git init
$ git remote add origin https://github.com/git/git # for example
$ git pull origin master
$ git remote show origin
...
  Local ref configured for 'git push':
master pushes to master (up to date)
$

but the local branch "master" is not yet configured to push to remote "master":
1. there is no section '[branch "master"]' (with required content) in the 
.git/config
2. attempt to push results in error (and that's right):
$ git push
fatal: The current branch master has no upstream branch.

[PATCH v4 2/3] git.c: handle_alias: prepend alias info when first argument is -h

[Rasmus Villemoes]

Most git commands respond to -h anywhere in the command line, or at
least as a first and lone argument, by printing the usage
information. For aliases, we can provide a little more information that
might be useful in interpreting/understanding the following output by
prepending a line telling that the command is an alias, and for what.

When one invokes a simple alias, such as "cp = cherry-pick"
with -h, this results in

$ git cp -h
'cp' is aliased to 'cherry-pick'
usage: git cherry-pick [] ...
...

When the alias consists of more than one word, this provides the
additional benefit of informing the user which options are implicit in
using the alias, e.g. with "cp = cherry-pick -n":

$ git cp -h
'cp' is aliased to 'cherry-pick -n'
usage: git cherry-pick [] ...
...

For shell commands, we cannot know how it responds to -h, but printing
this line to stderr should not hurt, and can help in figuring out what
is happening in a case like

$ git sc -h
'sc' is aliased to '!somecommand'
somecommand: invalid option '-h'

Suggested-by: Jeff King 
Signed-off-by: Rasmus Villemoes 
---
 git.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git.c b/git.c
index a6f4b44af5..0211c2d4c0 100644
--- a/git.c
+++ b/git.c
@@ -318,6 +318,9 @@ static int handle_alias(int *argcp, const char ***argv)
alias_command = (*argv)[0];
alias_string = alias_lookup(alias_command);
if (alias_string) {
+   if (*argcp > 1 && !strcmp((*argv)[1], "-h"))
+   fprintf_ln(stderr, _("'%s' is aliased to '%s'"),
+  alias_command, alias_string);
if (alias_string[0] == '!') {
struct child_process child = CHILD_PROCESS_INIT;
int nongit_ok;
-- 
2.19.1.4.g721af0fda3

Info

[SYARIKAT MISTI JAYA SDN BHD]

Let's work together to execute this Business ,contact ( dongeh...@hotmail.com) 
for more details

[PATCH v3 2/3] git.c: handle_alias: prepend alias info when first argument is -h

[Rasmus Villemoes]

Most git commands respond to -h anywhere in the command line, or at
least as a first and lone argument, by printing the usage
information. For aliases, we can provide a little more information that
might be useful in interpreting/understanding the following output by
prepending a line telling that the command is an alias, and for what.

When one invokes a simple alias, such as "cp = cherry-pick"
with -h, this results in

$ git cp -h
'cp' is aliased to 'cherry-pick'
usage: git cherry-pick [] ...
...

When the alias consists of more than one word, this provides the
additional benefit of informing the user which options are implicit in
using the alias, e.g. with "cp = cherry-pick -n":

$ git cp -h
'cp' is aliased to 'cherry-pick -n'
usage: git cherry-pick [] ...
...

For shell commands, we cannot know how it responds to -h, but printing
this line to stderr should not hurt, and can help in figuring out what
is happening in a case like

$ git sc -h
'sc' is aliased to '!somecommand'
somecommand: invalid option '-h'

Suggested-by: Jeff King 
Signed-off-by: Rasmus Villemoes 
---
 git.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git.c b/git.c
index a6f4b44af5..0211c2d4c0 100644
--- a/git.c
+++ b/git.c
@@ -318,6 +318,9 @@ static int handle_alias(int *argcp, const char ***argv)
alias_command = (*argv)[0];
alias_string = alias_lookup(alias_command);
if (alias_string) {
+   if (*argcp > 1 && !strcmp((*argv)[1], "-h"))
+   fprintf_ln(stderr, _("'%s' is aliased to '%s'"),
+  alias_command, alias_string);
if (alias_string[0] == '!') {
struct child_process child = CHILD_PROCESS_INIT;
int nongit_ok;
-- 
2.19.0

Re: [PATCH v2 2/3] git.c: handle_alias: prepend alias info when first argument is -h

[Jeff King]

On Mon, Oct 01, 2018 at 01:21:06PM +0200, Rasmus Villemoes wrote:

> Most git commands respond to -h anywhere in the command line, or at
> least as a first and lone argument, by printing the usage
> information. For aliases, we can provide a little more information that
> might be useful in interpreting/understanding the following output by
> prepending a line telling that the command is an alias, and for what.
> 
> When one invokes a simple alias, such as "cp = cherry-pick"
> with -h, this results in
> 
> $ git cp -h
> 'cp' is aliased to 'cherry-pick'
> usage: git cherry-pick [] ...
> ...
> 
> When the alias consists of more than one word, this provides the
> additional benefit of informing the user which options are implicit in
> using the alias, e.g. with "cp = cherry-pick -n":
> 
> $ git cp -h
> 'cp' is aliased to 'cherry-pick -n'
> usage: git cherry-pick [] ...
> ...
> 
> For shell commands, we cannot know how it responds to -h, but printing
> this line to stderr should not hurt, and can help in figuring out what
> is happening in a case like
> 
> $ git sc -h
> 'sc' is aliased to '!somecommand'
> somecommand: invalid option '-h'

Nicely explained.

> diff --git a/git.c b/git.c
> index a6f4b44af5..0211c2d4c0 100644
> --- a/git.c
> +++ b/git.c
> @@ -318,6 +318,9 @@ static int handle_alias(int *argcp, const char ***argv)
>   alias_command = (*argv)[0];
>   alias_string = alias_lookup(alias_command);
>   if (alias_string) {
> + if (*argcp > 1 && !strcmp((*argv)[1], "-h"))
> + fprintf_ln(stderr, _("'%s' is aliased to '%s'"),
> +alias_command, alias_string);
>   if (alias_string[0] == '!') {
>   struct child_process child = CHILD_PROCESS_INIT;
>   int nongit_ok;

And the implementation makes sense.

-Peff

[PATCH v2 2/3] git.c: handle_alias: prepend alias info when first argument is -h

[Rasmus Villemoes]

Most git commands respond to -h anywhere in the command line, or at
least as a first and lone argument, by printing the usage
information. For aliases, we can provide a little more information that
might be useful in interpreting/understanding the following output by
prepending a line telling that the command is an alias, and for what.

When one invokes a simple alias, such as "cp = cherry-pick"
with -h, this results in

$ git cp -h
'cp' is aliased to 'cherry-pick'
usage: git cherry-pick [] ...
...

When the alias consists of more than one word, this provides the
additional benefit of informing the user which options are implicit in
using the alias, e.g. with "cp = cherry-pick -n":

$ git cp -h
'cp' is aliased to 'cherry-pick -n'
usage: git cherry-pick [] ...
...

For shell commands, we cannot know how it responds to -h, but printing
this line to stderr should not hurt, and can help in figuring out what
is happening in a case like

$ git sc -h
'sc' is aliased to '!somecommand'
somecommand: invalid option '-h'

Suggested-by: Jeff King 
Signed-off-by: Rasmus Villemoes 
---
 git.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git.c b/git.c
index a6f4b44af5..0211c2d4c0 100644
--- a/git.c
+++ b/git.c
@@ -318,6 +318,9 @@ static int handle_alias(int *argcp, const char ***argv)
alias_command = (*argv)[0];
alias_string = alias_lookup(alias_command);
if (alias_string) {
+   if (*argcp > 1 && !strcmp((*argv)[1], "-h"))
+   fprintf_ln(stderr, _("'%s' is aliased to '%s'"),
+  alias_command, alias_string);
if (alias_string[0] == '!') {
struct child_process child = CHILD_PROCESS_INIT;
int nongit_ok;
-- 
2.19.0

Contact this Email for your payment; info...@consultant.com

[UBA CONSULTANT]

   This is to officially inform you that we have verified your contract 
file presently on my desk, and I found out that you have not received 
your payment due to your lack of co-operation and not fulfilling the 
obligations giving to you in respect to your contract payment.Secondly, 
you are hereby advised to stop dealing with some non-officials in the 
bank as this is an illegal act and will have to stop if you so wish to 
receive your payment after the board of director's meeting held in 
Burkina Faso, we have resolved in finding a solution to your problem.

 We have arranged your payment through our atm card payment center in 
europe, america, africa and asia pacific; this is part of an 
instruction/mandate passed by the senate in respect to overseas 
contract payment and debt re-scheduling. We will send you an atm card 
which you will use to withdraw your money via atm machine in any part 
of the world, and the maximum daily limit is two thousand united states 
dollars($2,000.00) valued sum of five million united states dollars 
($5m), if you desire to receive your fund this way, kindly re-confirm 
your info:


Thanks for your understanding.

Mrs Selin UBA Manager
Contact this Email for your payment; info...@consultant.com

[PATCH 1/9] multi-pack-index: provide more helpful usage info

[Derrick Stolee]

The multi-pack-index builtin has a very simple command-line
interface. Instead of simply reporting usage, give the user a
hint to why the arguments failed.

Reported-by: Eric Sunshine 
Signed-off-by: Derrick Stolee 
---
 builtin/multi-pack-index.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 6a7aa00cf2..2633efd95d 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -32,16 +32,16 @@ int cmd_multi_pack_index(int argc, const char **argv,
opts.object_dir = get_object_directory();
 
if (argc == 0)
-   goto usage;
+   usage_with_options(builtin_multi_pack_index_usage,
+  builtin_multi_pack_index_options);
 
-   if (!strcmp(argv[0], "write")) {
-   if (argc > 1)
-   goto usage;
+   if (argc > 1) {
+   die(_("too many arguments"));
+   return 1;
+   }
 
+   if (!strcmp(argv[0], "write"))
return write_midx_file(opts.object_dir);
-   }
 
-usage:
-   usage_with_options(builtin_multi_pack_index_usage,
-  builtin_multi_pack_index_options);
+   die(_("unrecognized verb: %s"), argv[0]);
 }
-- 
2.18.0.118.gd4f65b8d14

Unable to build Info pages using "make info"

[Kaushal Modi]

Hello,

I have updated to the latest master and trying to build git plus its
Info manuals.

I am on RHEL 6.8. It does not have docbook2x-texi, but I was able to
install docbook2texi from https://sourceforge.net/p/docbook2x.

The whole git build goes fine except when it reaches the "make info"
step. I get these errors:

MAKEINFO git.info
user-manual.texi:15: warning: empty menu entry name in `* : idp4751360.'
user-manual.texi:141: warning: @unnumbered missing argument
user-manual.texi:3204: warning: @ref cross-reference name should not contain `:'
DB2TEXI gitman.texi
MAKEINFO gitman.info
novalidate not a possible customization in Texinfo::Parser::parser
gitman.texi:1610: @ref reference to nonexistent node `ATTRIBUTES'
gitman.texi:3165: @pxref reference to nonexistent node `CO1-1'
gitman.texi:3181: @pxref reference to nonexistent node `CO2-1'
gitman.texi:3186: @pxref reference to nonexistent node `CO2-2'
gitman.texi:5070: @pxref reference to nonexistent node `CO1-1'
gitman.texi:5075: @pxref reference to nonexistent node `CO1-2'
gitman.texi:5079: @pxref reference to nonexistent node `CO1-3'
gitman.texi:5129: @pxref reference to nonexistent node `CO2-1'
gitman.texi:5132: @pxref reference to nonexistent node `CO2-2'
gitman.texi:5135: @pxref reference to nonexistent node `CO2-3'
gitman.texi:5475: @pxref reference to nonexistent node `CO1-1'
gitman.texi:5481: @pxref reference to nonexistent node `CO1-2'
gitman.texi:5484: @pxref reference to nonexistent node `CO1-3'
gitman.texi:5489: @pxref reference to nonexistent node `CO1-4'
gitman.texi:7466: @ref reference to nonexistent node `EXAMPLES'
gitman.texi:7476: @ref reference to nonexistent node `FILES'
gitman.texi:7559: @ref reference to nonexistent node `FILES'
gitman.texi:7569: @ref reference to nonexistent node `FILES'
gitman.texi:7578: @ref reference to nonexistent node `FILES'
gitman.texi:7785: @ref reference to nonexistent node `FILES'
gitman.texi:12524: @ref reference to nonexistent node `FILES'
gitman.texi:12630: @pxref reference to nonexistent node `INPUT/OUTPUT FORMAT'
gitman.texi:12938: @pxref reference to nonexistent node `ISSUES'
gitman.texi:13353: @pxref reference to nonexistent node `DATABASE BACKEND'
gitman.texi:13488: @pxref reference to nonexistent node `configaccessmethod'
gitman.texi:19956: @pxref reference to nonexistent node `CO1-1'
gitman.texi:19959: @pxref reference to nonexistent node `CO1-2'
gitman.texi:19963: @pxref reference to nonexistent node `CO1-3'
gitman.texi:19978: @pxref reference to nonexistent node `CO2-1'
gitman.texi:19982: @pxref reference to nonexistent node `CO2-2'
gitman.texi:19987: @pxref reference to nonexistent node `CO2-3'
gitman.texi:20001: @pxref reference to nonexistent node `CO3-1'
gitman.texi:20004: @pxref reference to nonexistent node `CO3-2'
gitman.texi:20007: @pxref reference to nonexistent node `CO3-3'
gitman.texi:20022: @pxref reference to nonexistent node `CO4-1'
gitman.texi:20026: @pxref reference to nonexistent node `CO4-2'
gitman.texi:20030: @pxref reference to nonexistent node `CO4-3'
gitman.texi:20043: @pxref reference to nonexistent node `CO5-1'
gitman.texi:20047: @pxref reference to nonexistent node `CO5-2'
gitman.texi:22570: @pxref reference to nonexistent node `[CRTB]'
gitman.texi:22935: @pxref reference to nonexistent node `[CRTB]'
gitman.texi:23278: @ref reference to nonexistent node `Remap to ancestor'
gitman.texi:23394: @ref reference to nonexistent node `Remap to ancestor'
gitman.texi:27608: @pxref reference to nonexistent node `CO1-1'
gitman.texi:27611: @pxref reference to nonexistent node `CO1-2'
gitman.texi:27614: @pxref reference to nonexistent node `CO1-3'
gitman.texi:37177: @pxref reference to nonexistent node `[OPTIONS]'
gitman.texi:41101: @pxref reference to nonexistent node `CO1-1'
gitman.texi:41107: @pxref reference to nonexistent node `CO1-2'
gitman.texi:41110: @pxref reference to nonexistent node `CO1-3'
gitman.texi:41117: @pxref reference to nonexistent node `CO1-4'
gitman.texi:41133: @pxref reference to nonexistent node `CO2-1'
gitman.texi:41138: @pxref reference to nonexistent node `CO2-2'
gitman.texi:41141: @pxref reference to nonexistent node `CO2-3'
gitman.texi:41159: @pxref reference to nonexistent node `CO3-1'
gitman.texi:41165: @pxref reference to nonexistent node `CO3-2'

I have searched online in general and also in the archive of this
mailing list, but have't seen this issue before. So something in my
build process is not right.

Can someone point out a known issue on old RHEL 6.8 systems that can
cause this, and also the workaroud?

Here's my build script: https://ptpb.pw/Rcco/bash .

Thanks! And please CC me on all the replies.

Kaushal

Re: [PATCH] update-index: there no longer is `apply --index-info`

[Jeff King]

On Wed, Aug 08, 2018 at 02:35:18PM -0700, Junio C Hamano wrote:

> Back when we removed `git apply --index-info` in 2007, we forgot to
> adjust the documentation for update-index that reads its output.
> 
> Let's reorder the description of three formats to present the other
> two formats that are still generated by git commands before this
> format, and stop mentioning `git apply --index-info`.

Yep, this makes sense.

> ---
>  Documentation/git-update-index.txt | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)

For fun, I tried out my "doc-diff" on it, and it looks good. :)

-Peff

[PATCH] update-index: there no longer is `apply --index-info`

[Junio C Hamano]

Back when we removed `git apply --index-info` in 2007, we forgot to
adjust the documentation for update-index that reads its output.

Let's reorder the description of three formats to present the other
two formats that are still generated by git commands before this
format, and stop mentioning `git apply --index-info`.

Signed-off-by: Junio C Hamano 
---
 Documentation/git-update-index.txt | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-update-index.txt 
b/Documentation/git-update-index.txt
index 4e8e762e68..c62a683648 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -268,23 +268,20 @@ USING --INDEX-INFO
 multiple entry definitions from the standard input, and designed
 specifically for scripts.  It can take inputs of three formats:
 
-. mode SP sha1  TAB path
-+
-The first format is what "git-apply --index-info"
-reports, and used to reconstruct a partial tree
-that is used for phony merge base tree when falling
-back on 3-way merge.
-
 . mode SP type SP sha1  TAB path
 +
-The second format is to stuff 'git ls-tree' output
-into the index file.
+This format is to stuff `git ls-tree` output into the index.
 
 . mode SP sha1 SP stage TAB path
 +
 This format is to put higher order stages into the
 index file and matches 'git ls-files --stage' output.
 
+. mode SP sha1  TAB path
++
+This format is no longer produced by any Git command, but is
+and will continue to be supported by `update-index --index-info`.
+
 To place a higher stage entry to the index, the path should
 first be removed by feeding a mode=0 entry for the path, and
 then feeding necessary input lines in the third format.

[PATCH v2 1/2] repack: point out a bug handling stale shallow info

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin 

A `git fetch --prune` can turn previously-reachable objects unreachable,
even commits that are in the `shallow` list. A subsequent `git repack
-ad` will then unceremoniously drop those unreachable commits, and the
`shallow` list will become stale. This means that when we try to fetch
with a larger `--depth` the next time, we may end up with:

fatal: error in object: unshallow 

Reported by Alejandro Pauly.

Signed-off-by: Johannes Schindelin 
---
 t/t5537-fetch-shallow.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 943231af9..561485d31 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -186,4 +186,31 @@ EOF
test_cmp expect actual
 '
 
+test_expect_failure '.git/shallow is edited by repack' '
+   git init shallow-server &&
+   test_commit -C shallow-server A &&
+   test_commit -C shallow-server B &&
+   git -C shallow-server checkout -b branch &&
+   test_commit -C shallow-server C &&
+   test_commit -C shallow-server E &&
+   test_commit -C shallow-server D &&
+   d="$(git -C shallow-server rev-parse --verify D)" &&
+   git -C shallow-server checkout master &&
+
+   git clone --depth=1 --no-tags --no-single-branch \
+   "file://$PWD/shallow-server" shallow-client &&
+
+   : now remove the branch and fetch with prune &&
+   git -C shallow-server branch -D branch &&
+   git -C shallow-client fetch --prune --depth=1 \
+   origin "+refs/heads/*:refs/remotes/origin/*" &&
+   git -C shallow-client repack -adfl &&
+   test_must_fail git -C shallow-client rev-parse --verify $d^0 &&
+   ! grep $d shallow-client/.git/shallow &&
+
+   git -C shallow-server branch branch-orig D^0 &&
+   git -C shallow-client fetch --prune --depth=2 \
+   origin "+refs/heads/*:refs/remotes/origin/*"
+'
+
 test_done
-- 
gitgitgadget

Re: [PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

[Junio C Hamano]

Jeff King  writes:

> I think you missed it. In the paragraph above the one you
> quoted, I said:
>
>The cgit repository, for example, has a file named
>.gitmodules from a pre-submodule attempt at sharing code,
>but does not actually have any gitlinks.

Indeed.

> So I'm curious if you found the argument in my commit
> message compelling. :)
> ...
>  - I think your main concern was that there be a way for the
>user to loosen/tighten, which there is.

Yeah, I think the solution looks good.

Re: [PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

[Jeff King]

On Mon, Jul 16, 2018 at 11:04:04AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> >site's support). And the broken .gitmodules may be too
> >far back in history for rewriting to be feasible (again,
> >this is an issue for cgit).
> 
> "again" but this is the first mention that hints cgit has some
> problem (but not exactly what problem).  Is that the "cgit has a
> file called .gitmodules that predates the submodule support on our
> side?" thing?

I think you missed it. In the paragraph above the one you
quoted, I said:

   The cgit repository, for example, has a file named
   .gitmodules from a pre-submodule attempt at sharing code,
   but does not actually have any gitlinks.

> > So we're being unnecessarily restrictive without actually
> > improving the security in a meaningful way. It would be more
> > convenient to downgrade this check to "info", which means
> > we'd still comment on it, but not reject a push. Site admins
> > can already do this via config, but we should ship sensible
> > defaults.
> > ...
> > Considering both sets of arguments, it makes sense to loosen
> > this check for now.
> >
> > Note that we have to tweak the test in t7415 since fsck will
> > no longer consider this a fatal error. But we still check
> > that it reports the warning, and that we don't get the
> > spurious error from the config code.
> >
> > Signed-off-by: Jeff King 
> > ---
> 
> Thanks.

So I'm curious if you found the argument in my commit
message compelling. :)

My recollection from the earlier discussion was that you
were more in favor of keeping things tight. E.g.,:

  https://public-inbox.org/git/xmqqh8lgrz5c@gitster-ct.c.googlers.com/

but reading it again:

 - there we were talking about non-blob objects as
   .gitmodules

 - I think your main concern was that there be a way for the
   user to loosen/tighten, which there is.

-Peff

Re: [PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

[Junio C Hamano]

Jeff King  writes:

>site's support). And the broken .gitmodules may be too
>far back in history for rewriting to be feasible (again,
>this is an issue for cgit).

"again" but this is the first mention that hints cgit has some
problem (but not exactly what problem).  Is that the "cgit has a
file called .gitmodules that predates the submodule support on our
side?" thing?

> So we're being unnecessarily restrictive without actually
> improving the security in a meaningful way. It would be more
> convenient to downgrade this check to "info", which means
> we'd still comment on it, but not reject a push. Site admins
> can already do this via config, but we should ship sensible
> defaults.
> ...
> Considering both sets of arguments, it makes sense to loosen
> this check for now.
>
> Note that we have to tweak the test in t7415 since fsck will
> no longer consider this a fatal error. But we still check
> that it reports the warning, and that we don't get the
> spurious error from the config code.
>
> Signed-off-by: Jeff King 
> ---

Thanks.

>  fsck.c | 2 +-
>  t/t7415-submodule-names.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index 4129935d86..69ea8d5321 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -62,7 +62,6 @@ static struct oidset gitmodules_done = OIDSET_INIT;
>   FUNC(ZERO_PADDED_DATE, ERROR) \
>   FUNC(GITMODULES_MISSING, ERROR) \
>   FUNC(GITMODULES_BLOB, ERROR) \
> - FUNC(GITMODULES_PARSE, ERROR) \
>   FUNC(GITMODULES_LARGE, ERROR) \
>   FUNC(GITMODULES_NAME, ERROR) \
>   FUNC(GITMODULES_SYMLINK, ERROR) \
> @@ -77,6 +76,7 @@ static struct oidset gitmodules_done = OIDSET_INIT;
>   FUNC(ZERO_PADDED_FILEMODE, WARN) \
>   FUNC(NUL_IN_COMMIT, WARN) \
>   /* infos (reported as warnings, but ignored by default) */ \
> + FUNC(GITMODULES_PARSE, INFO) \
>   FUNC(BAD_TAG_NAME, INFO) \
>   FUNC(MISSING_TAGGER_ENTRY, INFO)
>  
> diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
> index ba8af785a5..293e2e1963 100755
> --- a/t/t7415-submodule-names.sh
> +++ b/t/t7415-submodule-names.sh
> @@ -185,7 +185,7 @@ test_expect_success 'fsck detects corrupt .gitmodules' '
>   git add .gitmodules &&
>   git commit -m "broken gitmodules" &&
>  
> - test_must_fail git fsck 2>output &&
> + git fsck 2>output &&
>   grep gitmodulesParse output &&
>   test_i18ngrep ! "bad config" output
>   )

Re: [PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

[Stefan Beller]

> Considering both sets of arguments, it makes sense to loosen
> this check for now.
>

I agree with this reasoning,

>
> Signed-off-by: Jeff King 

This and the previous patch are
Reviewed-by: Stefan Beller 

[PATCH 1/2] repack: point out a bug handling stale shallow info

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin 

A `git fetch --prune` can turn previously-reachable objects unreachable,
even commits that are in the `shallow` list. A subsequent `git repack
-ad` will then unceremoniously drop those unreachable commits, and the
`shallow` list will become stale. This means that when we try to fetch
with a larger `--depth` the next time, we may end up with:

fatal: error in object: unshallow 

Reported by Alejandro Pauly.

Signed-off-by: Johannes Schindelin 
---
 t/t5537-fetch-shallow.sh | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 943231af9..561485d31 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -186,4 +186,31 @@ EOF
test_cmp expect actual
 '
 
+test_expect_failure '.git/shallow is edited by repack' '
+   git init shallow-server &&
+   test_commit -C shallow-server A &&
+   test_commit -C shallow-server B &&
+   git -C shallow-server checkout -b branch &&
+   test_commit -C shallow-server C &&
+   test_commit -C shallow-server E &&
+   test_commit -C shallow-server D &&
+   d="$(git -C shallow-server rev-parse --verify D)" &&
+   git -C shallow-server checkout master &&
+
+   git clone --depth=1 --no-tags --no-single-branch \
+   "file://$PWD/shallow-server" shallow-client &&
+
+   : now remove the branch and fetch with prune &&
+   git -C shallow-server branch -D branch &&
+   git -C shallow-client fetch --prune --depth=1 \
+   origin "+refs/heads/*:refs/remotes/origin/*" &&
+   git -C shallow-client repack -adfl &&
+   test_must_fail git -C shallow-client rev-parse --verify $d^0 &&
+   ! grep $d shallow-client/.git/shallow &&
+
+   git -C shallow-server branch branch-orig D^0 &&
+   git -C shallow-client fetch --prune --depth=2 \
+   origin "+refs/heads/*:refs/remotes/origin/*"
+'
+
 test_done
-- 
gitgitgadget

[PATCH 2/2] fsck: downgrade gitmodulesParse default to "info"

[Jeff King]

We added an fsck check in ed8b10f631 (fsck: check
.gitmodules content, 2018-05-02) as a defense against the
vulnerability from 0383bbb901 (submodule-config: verify
submodule names as paths, 2018-04-30). With the idea that
up-to-date hosting sites could protect downstream unpatched
clients that fetch from them.

As part of that defense, we reject any ".gitmodules" entry
that is not syntactically valid. The theory is that if we
cannot even parse the file, we cannot accurately check it
for vulnerabilities. And anybody with a broken .gitmodules
file would eventually want to know anyway.

But there are a few reasons this is a bad tradeoff in
practice:

 - for this particular vulnerability, the client has to be
   able to parse the file. So you cannot sneak an attack
   through using a broken file, assuming the config parsers
   for the process running fsck and the eventual victim are
   functionally equivalent.

 - a broken .gitmodules file is not necessarily a problem.
   Our fsck check detects .gitmodules in _any_ tree, not
   just at the root. And the presence of a .gitmodules file
   does not necessarily mean it will be used; you'd have to
   also have gitlinks in the tree. The cgit repository, for
   example, has a file named .gitmodules from a
   pre-submodule attempt at sharing code, but does not
   actually have any gitlinks.

 - when the fsck check is used to reject a push, it's often
   hard to work around. The pusher may not have full control
   over the destination repository (e.g., if it's on a
   hosting server, they may need to contact the hosting
   site's support). And the broken .gitmodules may be too
   far back in history for rewriting to be feasible (again,
   this is an issue for cgit).

So we're being unnecessarily restrictive without actually
improving the security in a meaningful way. It would be more
convenient to downgrade this check to "info", which means
we'd still comment on it, but not reject a push. Site admins
can already do this via config, but we should ship sensible
defaults.

There are a few counterpoints to consider in favor of
keeping the check as an error:

 - the first point above assumes that the config parsers for
   the victim and the fsck process are equivalent. This is
   pretty true now, but as time goes on will become less so.
   Hosting sites are likely to upgrade their version of Git,
   whereas vulnerable clients will be stagnant (if they did
   upgrade, they'd cease to be vulnerable!). So in theory we
   may see drift over time between what two config parsers
   will accept.

   In practice, this is probably OK. The config format is
   pretty established at this point and shouldn't change a
   lot. And the farther we get from the announcement of the
   vulnerability, the less interesting this extra layer of
   protection becomes. I.e., it was _most_ valuable on day
   0, when everybody's client was still vulnerable and
   hosting sites could protect people. But as time goes on
   and people upgrade, the population of vulnerable clients
   becomes smaller and smaller.

 - In theory this could protect us from other
   vulnerabilities in the future. E.g., .gitmodules are the
   only way for a malicious repository to feed data to the
   config parser, so this check could similarly protect
   clients from a future (to-be-found) bug there.

   But that's trading a hypothetical case for real-world
   pain today. If we do find such a bug, the hosting site
   would need to be updated to fix it, too. At which point
   we could figure out whether it's possible to detect
   _just_ the malicious case without hurting existing
   broken-but-not-evil cases.

 - Until recently, we hadn't made any restrictions on
   .gitmodules content. So now in tightening that we're
   hitting cases where certain things used to work, but
   don't anymore. There's some moderate pain now. But as
   time goes on, we'll see more (and more varied) cases that
   will make tightening harder in the future. So there's
   some argument for putting rules in place _now_, before
   users grow more cases that violate them.

   Again, this is trading pain now for hypothetical benefit
   in the future. And if we try hard in the future to keep
   our tightening to a minimum (i.e., rejecting true
   maliciousness without hurting broken-but-not-evil repos),
   then that reduces even the hypothetical benefit.

Considering both sets of arguments, it makes sense to loosen
this check for now.

Note that we have to tweak the test in t7415 since fsck will
no longer consider this a fatal error. But we still check
that it reports the warning, and that we don't get the
spurious error from the config code.

Signed-off-by: Jeff King 
---
 fsck.c | 2 +-
 t/t7415-submodule-names.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fsck.c b/fsck.c
index 4129935d86..69ea8d5321 100644
--- a/fsck.c
+++ b/fsck.c
@@ -62,7 +62,6 @@ static struct oidset gitmodule

info!!

[Lee Morrow]

Top of the day to you, this is in respect of a very beneficial transaction 
which you would not want to let go reply for more details,
Regards,
Lee 

Re: [PATCH 2/3] ls-tree: update usage info

[Eric Sunshine]

On Mon, Jul 2, 2018 at 11:58 PM Joshua Nelson  wrote:
> show [tree-ish] and [--] as optional
> ---
> diff --git builtin/ls-tree.c builtin/ls-tree.c
> @@ -26,7 +26,7 @@ static int chomp_prefix;
>  static const  char * const ls_tree_usage[] = {
> -   N_("git ls-tree []  [...]"),
> +   N_("git ls-tree [] [tree-ish] [--] [...]"),

You lost the '<' and '>'. This should be typeset as:

git ls-tree [] [] [--] [...]

And, as Elijah noted, Documentation/git-ls-tree.txt needs an update.

Re: [PATCH 2/3] ls-tree: update usage info

[Elijah Newren]

Hi Joshua,

On Mon, Jul 2, 2018 at 8:58 PM, Joshua Nelson  wrote:
> show [tree-ish] and [--] as optional

You're missing a Signed-off-by tag here.

> ---
>  builtin/ls-tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git builtin/ls-tree.c builtin/ls-tree.c

No corresponding update to Documentation/git-ls-tree.txt?

> index 14102b052..c5649b09c 100644
> --- builtin/ls-tree.c
> +++ builtin/ls-tree.c
> @@ -26,7 +26,7 @@ static int chomp_prefix;
>  static const char *ls_tree_prefix;
>
>  static const  char * const ls_tree_usage[] = {
> -   N_("git ls-tree []  [...]"),
> +   N_("git ls-tree [] [tree-ish] [--] [...]"),
> NULL
>  };
>
> --
> 2.18.GIT

[PATCH 2/3] ls-tree: update usage info

[Joshua Nelson]

show [tree-ish] and [--] as optional
---
 builtin/ls-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git builtin/ls-tree.c builtin/ls-tree.c
index 14102b052..c5649b09c 100644
--- builtin/ls-tree.c
+++ builtin/ls-tree.c
@@ -26,7 +26,7 @@ static int chomp_prefix;
 static const char *ls_tree_prefix;
 
 static const  char * const ls_tree_usage[] = {
-   N_("git ls-tree []  [...]"),
+   N_("git ls-tree [] [tree-ish] [--] [...]"),
NULL
 };
 
-- 
2.18.GIT

[PATCH v6 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched.  Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.

This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.

Signed-off-by: Brandon Williams 
---
 builtin/clone.c  |  4 ++--
 builtin/fetch.c  | 28 
 fetch-object.c   |  2 +-
 fetch-pack.c | 15 ---
 transport-helper.c   |  6 --
 transport-internal.h |  9 -
 transport.c  | 34 --
 transport.h  |  3 ++-
 8 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
}
 
if (!is_local && !complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
update_remote_refs(refs, mapped_refs, remote_head_points_at,
   branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bda00e826..0347cf016 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
 {
int ret = quickfetch(ref_map);
if (ret)
-   ret = transport_fetch_refs(transport, ref_map);
+   ret = transport_fetch_refs(transport, ref_map,
+  updated_remote_refs);
if (!ret)
/*
 * Keep the new pack's ".keep" file around to allow the caller
@@ -1112,7 +1114,7 @@ static void backfill_tags(struct transport *transport, 
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-   if (!fetch_refs(transport, ref_map))
+   if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
 
if (gsecondary) {
@@ -1128,6 +1130,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+   struct ref *updated_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
if (tags == TAGS_DEFAULT) {
@@ -1178,7 +1181,24 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
{
+
+   if (fetch_refs(transport, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (updated_remote_refs) {
+   /*
+* Regenerate ref_map using the updated remote refs.  This is
+* to account for additional information which may be provided
+* by the transport (e.g. shallow info).
+*/
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, updated_remote_refs, 
rs,
+ tags, );
+   free_refs(updated_remote_refs);
+   }
+   if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref 
*ref)
 
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");

Re: [PATCH v5 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

On 06/26, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > Expand the transport fetch method signature, by adding an output
> > parameter, to allow transports to return information about the refs they
> > have fetched.  Then communicate shallow status information through this
> > mechanism instead of by modifying the input list of refs.
> 
> Makes sense.  Would this mechanism also allow us to be more explicit
> about the "tag following"?
> 

Yes most likely.  We could change it so that when a packfile is sent the
result of tag following could be sent along too (the actual tag refs
themselves that is) instead of having the client rely on the ref
advertisement for tag following.

-- 
Brandon Williams

Re: [PATCH v5 7/8] fetch-pack: put shallow info in output parameter

[Junio C Hamano]

Brandon Williams  writes:

> Expand the transport fetch method signature, by adding an output
> parameter, to allow transports to return information about the refs they
> have fetched.  Then communicate shallow status information through this
> mechanism instead of by modifying the input list of refs.

Makes sense.  Would this mechanism also allow us to be more explicit
about the "tag following"?

[PATCH v5 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched.  Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.

This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.

Signed-off-by: Brandon Williams 
---
 builtin/clone.c  |  4 ++--
 builtin/fetch.c  | 28 
 fetch-object.c   |  2 +-
 fetch-pack.c | 15 ---
 transport-helper.c   |  6 --
 transport-internal.h |  9 -
 transport.c  | 34 --
 transport.h  |  3 ++-
 8 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
}
 
if (!is_local && !complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
update_remote_refs(refs, mapped_refs, remote_head_points_at,
   branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bda00e826..0347cf016 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
 {
int ret = quickfetch(ref_map);
if (ret)
-   ret = transport_fetch_refs(transport, ref_map);
+   ret = transport_fetch_refs(transport, ref_map,
+  updated_remote_refs);
if (!ret)
/*
 * Keep the new pack's ".keep" file around to allow the caller
@@ -1112,7 +1114,7 @@ static void backfill_tags(struct transport *transport, 
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-   if (!fetch_refs(transport, ref_map))
+   if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
 
if (gsecondary) {
@@ -1128,6 +1130,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+   struct ref *updated_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
if (tags == TAGS_DEFAULT) {
@@ -1178,7 +1181,24 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
{
+
+   if (fetch_refs(transport, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (updated_remote_refs) {
+   /*
+* Regenerate ref_map using the updated remote refs.  This is
+* to account for additional information which may be provided
+* by the transport (e.g. shallow info).
+*/
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, updated_remote_refs, 
rs,
+ tags, );
+   free_refs(updated_remote_refs);
+   }
+   if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref 
*ref)
 
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");

[PATCH v4 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched.  Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.

This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.

Signed-off-by: Brandon Williams 
---
 builtin/clone.c  |  4 ++--
 builtin/fetch.c  | 28 
 fetch-object.c   |  2 +-
 fetch-pack.c | 15 ---
 transport-helper.c   |  6 --
 transport-internal.h |  9 -
 transport.c  | 34 --
 transport.h  |  3 ++-
 8 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
}
 
if (!is_local && !complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
update_remote_refs(refs, mapped_refs, remote_head_points_at,
   branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bda00e826..0347cf016 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
 {
int ret = quickfetch(ref_map);
if (ret)
-   ret = transport_fetch_refs(transport, ref_map);
+   ret = transport_fetch_refs(transport, ref_map,
+  updated_remote_refs);
if (!ret)
/*
 * Keep the new pack's ".keep" file around to allow the caller
@@ -1112,7 +1114,7 @@ static void backfill_tags(struct transport *transport, 
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-   if (!fetch_refs(transport, ref_map))
+   if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
 
if (gsecondary) {
@@ -1128,6 +1130,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+   struct ref *updated_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
if (tags == TAGS_DEFAULT) {
@@ -1178,7 +1181,24 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
{
+
+   if (fetch_refs(transport, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (updated_remote_refs) {
+   /*
+* Regenerate ref_map using the updated remote refs.  This is
+* to account for additional information which may be provided
+* by the transport (e.g. shallow info).
+*/
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, updated_remote_refs, 
rs,
+ tags, );
+   free_refs(updated_remote_refs);
+   }
+   if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref 
*ref)
 
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");

Re: [PATCH v3 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

On 06/25, Jonathan Tan wrote:
> >  static void update_shallow(struct fetch_pack_args *args,
> > -  struct ref **sought, int nr_sought,
> > +  struct ref *refs,
> 
> update_shallow() now takes in a linked list of refs instead of an array.
> I see that the translation of this function is straightforward -
> occasionally, we need to iterate through the linked list and count up
> from 0 at the same time, but that is not a problem.
> 
> >struct shallow_info *si)
> >  {
> > struct oid_array ref = OID_ARRAY_INIT;
> > int *status;
> > -   int i;
> > +   int i = 0;
> 
> Remove the " = 0" - I've verified that it does not need to be there, and
> it might inhibit useful "unintialized variable" warnings if others were
> to change the code later.
> 
> Optional: I would also remove this declaration and declare "int i;" in
> each of the blocks that need it.
> 
> >  static int fetch_refs_via_pack(struct transport *transport,
> > -  int nr_heads, struct ref **to_fetch)
> > +  int nr_heads, struct ref **to_fetch,
> > +  struct ref **fetched_refs)
> >  {
> > int ret = 0;
> > struct git_transport_data *data = transport->data;
> > @@ -354,8 +356,12 @@ static int fetch_refs_via_pack(struct transport 
> > *transport,
> > if (report_unmatched_refs(to_fetch, nr_heads))
> > ret = -1;
> >  
> > +   if (fetched_refs)
> > +   *fetched_refs = refs;
> > +   else
> > +   free_refs(refs);
> > +
> > free_refs(refs_tmp);
> > -   free_refs(refs);
> > free(dest);
> > return ret;
> >  }
> 
> Instead of just freeing the linked list, we return it if requested by
> the client. This makes sense.
> 
> > -int transport_fetch_refs(struct transport *transport, struct ref *refs)
> > +int transport_fetch_refs(struct transport *transport, struct ref *refs,
> > +struct ref **fetched_refs)
> >  {
> > int rc;
> > int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
> > struct ref **heads = NULL;
> > +   struct ref *nop_head = NULL, **nop_tail = _head;
> > struct ref *rm;
> >  
> > for (rm = refs; rm; rm = rm->next) {
> > nr_refs++;
> > if (rm->peer_ref &&
> > !is_null_oid(>old_oid) &&
> > -   !oidcmp(>peer_ref->old_oid, >old_oid))
> > +   !oidcmp(>peer_ref->old_oid, >old_oid)) {
> > +   /*
> > +* These need to be reported as fetched, but we don't
> > +* actually need to fetch them.
> > +*/
> > +   if (fetched_refs) {
> > +   struct ref *nop_ref = copy_ref(rm);
> > +   *nop_tail = nop_ref;
> > +   nop_tail = _ref->next;
> > +   }
> > continue;
> > +   }
> > ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
> > heads[nr_heads++] = rm;
> > }
> > @@ -1245,7 +1263,11 @@ int transport_fetch_refs(struct transport 
> > *transport, struct ref *refs)
> > heads[nr_heads++] = rm;
> > }
> >  
> > -   rc = transport->vtable->fetch(transport, nr_heads, heads);
> > +   rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs);
> > +   if (fetched_refs && nop_head) {
> > +   *nop_tail = *fetched_refs;
> > +   *fetched_refs = nop_head;
> > +   }
> >  
> > free(heads);
> > return rc;
> 
> And sometimes, even if we are merely simulating the fetching of refs, we
> still need to report those refs in fetched_refs. This is correct.
> 
> I also see that t5703 now passes.
> 
> Besides enabling the writing of subsequent patches, I see that this also
> makes the API clearer in that the input refs to transport_fetch_refs()
> are not overloaded to output shallow information. Other than the " = 0"
> change above, this patch looks good to me.

Perfect, I'll just drop the " = 0" part (making the diff slightly
smaller)

-- 
Brandon Williams

Re: [PATCH v3 7/8] fetch-pack: put shallow info in output parameter

[Jonathan Tan]

>  static void update_shallow(struct fetch_pack_args *args,
> -struct ref **sought, int nr_sought,
> +struct ref *refs,

update_shallow() now takes in a linked list of refs instead of an array.
I see that the translation of this function is straightforward -
occasionally, we need to iterate through the linked list and count up
from 0 at the same time, but that is not a problem.

>  struct shallow_info *si)
>  {
>   struct oid_array ref = OID_ARRAY_INIT;
>   int *status;
> - int i;
> + int i = 0;

Remove the " = 0" - I've verified that it does not need to be there, and
it might inhibit useful "unintialized variable" warnings if others were
to change the code later.

Optional: I would also remove this declaration and declare "int i;" in
each of the blocks that need it.

>  static int fetch_refs_via_pack(struct transport *transport,
> -int nr_heads, struct ref **to_fetch)
> +int nr_heads, struct ref **to_fetch,
> +struct ref **fetched_refs)
>  {
>   int ret = 0;
>   struct git_transport_data *data = transport->data;
> @@ -354,8 +356,12 @@ static int fetch_refs_via_pack(struct transport 
> *transport,
>   if (report_unmatched_refs(to_fetch, nr_heads))
>   ret = -1;
>  
> + if (fetched_refs)
> + *fetched_refs = refs;
> + else
> + free_refs(refs);
> +
>   free_refs(refs_tmp);
> - free_refs(refs);
>   free(dest);
>   return ret;
>  }

Instead of just freeing the linked list, we return it if requested by
the client. This makes sense.

> -int transport_fetch_refs(struct transport *transport, struct ref *refs)
> +int transport_fetch_refs(struct transport *transport, struct ref *refs,
> +  struct ref **fetched_refs)
>  {
>   int rc;
>   int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
>   struct ref **heads = NULL;
> + struct ref *nop_head = NULL, **nop_tail = _head;
>   struct ref *rm;
>  
>   for (rm = refs; rm; rm = rm->next) {
>   nr_refs++;
>   if (rm->peer_ref &&
>   !is_null_oid(>old_oid) &&
> - !oidcmp(>peer_ref->old_oid, >old_oid))
> + !oidcmp(>peer_ref->old_oid, >old_oid)) {
> + /*
> +  * These need to be reported as fetched, but we don't
> +  * actually need to fetch them.
> +  */
> + if (fetched_refs) {
> + struct ref *nop_ref = copy_ref(rm);
> + *nop_tail = nop_ref;
> + nop_tail = _ref->next;
> + }
>   continue;
> + }
>   ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
>   heads[nr_heads++] = rm;
>   }
> @@ -1245,7 +1263,11 @@ int transport_fetch_refs(struct transport *transport, 
> struct ref *refs)
>   heads[nr_heads++] = rm;
>   }
>  
> - rc = transport->vtable->fetch(transport, nr_heads, heads);
> + rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs);
> + if (fetched_refs && nop_head) {
> + *nop_tail = *fetched_refs;
> + *fetched_refs = nop_head;
> + }
>  
>   free(heads);
>   return rc;

And sometimes, even if we are merely simulating the fetching of refs, we
still need to report those refs in fetched_refs. This is correct.

I also see that t5703 now passes.

Besides enabling the writing of subsequent patches, I see that this also
makes the API clearer in that the input refs to transport_fetch_refs()
are not overloaded to output shallow information. Other than the " = 0"
change above, this patch looks good to me.

[PATCH v3 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched.  Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.

This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.

Signed-off-by: Brandon Williams 
---
 builtin/clone.c  |  4 ++--
 builtin/fetch.c  | 28 
 fetch-object.c   |  2 +-
 fetch-pack.c | 17 +
 transport-helper.c   |  6 --
 transport-internal.h |  9 -
 transport.c  | 34 --
 transport.h  |  3 ++-
 8 files changed, 78 insertions(+), 25 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
}
 
if (!is_local && !complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
update_remote_refs(refs, mapped_refs, remote_head_points_at,
   branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b600e1f10..8362a97a2 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
 {
int ret = quickfetch(ref_map);
if (ret)
-   ret = transport_fetch_refs(transport, ref_map);
+   ret = transport_fetch_refs(transport, ref_map,
+  updated_remote_refs);
if (ret)
transport_unlock_pack(transport);
return ret;
@@ -1106,7 +1108,7 @@ static void backfill_tags(struct transport *transport, 
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-   if (!fetch_refs(transport, ref_map))
+   if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
 
if (gsecondary) {
@@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+   struct ref *updated_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
if (tags == TAGS_DEFAULT) {
@@ -1172,7 +1175,24 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
{
+
+   if (fetch_refs(transport, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (updated_remote_refs) {
+   /*
+* Regenerate ref_map using the updated remote refs.  This is
+* to account for additional information which may be provided
+* by the transport (e.g. shallow info).
+*/
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, updated_remote_refs, 
rs,
+ tags, );
+   free_refs(updated_remote_refs);
+   }
+   if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref 
*ref)
 
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
transport_set_option(transport, TRANS_OPT_NO_DEP

Re: [PATCH v2 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

On 06/14, Jonathan Tan wrote:
> > @@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
> > int autotags = (transport->remote->fetch_tags == 1);
> > int retcode = 0;
> > const struct ref *remote_refs;
> > +   struct ref *new_remote_refs = NULL;
> 
> Above, you use the name "updated_remote_refs" - it's probably better to
> standardize on one. I think "updated" is better.

Good catch I'll update the variable name.

> 
> (The transport calling it "fetched_refs" is fine, because that's what
> they are from the perspective of the transport. From the perspective of
> fetch-pack, it is indeed a new or updated set of remote refs.)
> 
> > -   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
> > {
> > +
> > +   if (fetch_refs(transport, ref_map, _remote_refs)) {
> > +   free_refs(ref_map);
> > +   retcode = 1;
> > +   goto cleanup;
> > +   }
> > +   if (new_remote_refs) {
> > +   free_refs(ref_map);
> > +   ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
> > + tags, );
> > +   free_refs(new_remote_refs);
> > +   }
> > +   if (consume_refs(transport, ref_map)) {
> > free_refs(ref_map);
> > retcode = 1;
> > goto cleanup;
> 
> Here, if we got updated remote refs, we need to regenerate ref_map,
> since it is the source of truth.
> 
> Maybe add a comment in the "if (new_remote_refs)" block explaining this
> - something like: Regenerate ref_map using the updated remote refs,
> because the transport would place shallow (and other) information
> there.

That's probably a good idea to give future readers more context into why
this is happening.

> 
> > -   for (i = 0; i < nr_sought; i++)
> > +   for (r = refs; r; r = r->next, i++)
> > if (status[i])
> > -   sought[i]->status = REF_STATUS_REJECT_SHALLOW;
> > +   r->status = REF_STATUS_REJECT_SHALLOW;
> 
> You use i here without initializing it to 0. t5703 also fails with this
> patch - probably related to this, but I didn't check.

Oh yeah that's definitely a bug, thanks for catching that.

> 
> If you initialize i here, I don't think you need to initialize it to 0
> at the top of this function.

-- 
Brandon Williams

Re: [PATCH v2 7/8] fetch-pack: put shallow info in output parameter

[Jonathan Tan]

> @@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
>   int autotags = (transport->remote->fetch_tags == 1);
>   int retcode = 0;
>   const struct ref *remote_refs;
> + struct ref *new_remote_refs = NULL;

Above, you use the name "updated_remote_refs" - it's probably better to
standardize on one. I think "updated" is better.

(The transport calling it "fetched_refs" is fine, because that's what
they are from the perspective of the transport. From the perspective of
fetch-pack, it is indeed a new or updated set of remote refs.)

> - if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
> {
> +
> + if (fetch_refs(transport, ref_map, _remote_refs)) {
> + free_refs(ref_map);
> + retcode = 1;
> + goto cleanup;
> + }
> + if (new_remote_refs) {
> + free_refs(ref_map);
> + ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
> +   tags, );
> + free_refs(new_remote_refs);
> + }
> + if (consume_refs(transport, ref_map)) {
>   free_refs(ref_map);
>   retcode = 1;
>   goto cleanup;

Here, if we got updated remote refs, we need to regenerate ref_map,
since it is the source of truth.

Maybe add a comment in the "if (new_remote_refs)" block explaining this
- something like: Regenerate ref_map using the updated remote refs,
because the transport would place shallow (and other) information
there.

> - for (i = 0; i < nr_sought; i++)
> + for (r = refs; r; r = r->next, i++)
>   if (status[i])
> - sought[i]->status = REF_STATUS_REJECT_SHALLOW;
> + r->status = REF_STATUS_REJECT_SHALLOW;

You use i here without initializing it to 0. t5703 also fails with this
patch - probably related to this, but I didn't check.

If you initialize i here, I don't think you need to initialize it to 0
at the top of this function.

Re: [PATCH v2 7/8] fetch-pack: put shallow info in output parameter

[Stefan Beller]

> +   !oidcmp(>peer_ref->old_oid, >old_oid)) {
> +   /*
> +* These need to be reported as fetched, but we don 
> not

do not or don't; there is no middle way. ;)

[PATCH v2 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched.  Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.

This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.

Signed-off-by: Brandon Williams 
---
 builtin/clone.c  |  4 ++--
 builtin/fetch.c  | 23 +++
 fetch-object.c   |  2 +-
 fetch-pack.c | 17 +
 transport-helper.c   |  6 --
 transport-internal.h |  9 -
 transport.c  | 34 --
 transport.h  |  3 ++-
 8 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
}
 
if (!is_local && !complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
update_remote_refs(refs, mapped_refs, remote_head_points_at,
   branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b600e1f10..ddf44ba1a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
 {
int ret = quickfetch(ref_map);
if (ret)
-   ret = transport_fetch_refs(transport, ref_map);
+   ret = transport_fetch_refs(transport, ref_map,
+  updated_remote_refs);
if (ret)
transport_unlock_pack(transport);
return ret;
@@ -1106,7 +1108,7 @@ static void backfill_tags(struct transport *transport, 
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-   if (!fetch_refs(transport, ref_map))
+   if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
 
if (gsecondary) {
@@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+   struct ref *new_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
if (tags == TAGS_DEFAULT) {
@@ -1172,7 +1175,19 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
{
+
+   if (fetch_refs(transport, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (new_remote_refs) {
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
+ tags, );
+   free_refs(new_remote_refs);
+   }
+   if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref 
*ref)
 
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1");
-   transport_fetch_refs(transport, ref);
+   transport_fetch_refs(transport, ref, NULL);
fetch_if_missing = original_fetch_if_missing;
 }
 
diff --git a/fetch-pack.c b/fetch-pack.c
index a320ce987..7799ee2cd 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1470,12 +1470,13 @@ static int 

[PATCH 7/8] fetch-pack: put shallow info in output parameter

[Brandon Williams]

Expand the transport fetch method signature, by adding an output
parameter, to allow transports to return information about the refs they
have fetched.  Then communicate shallow status information through this
mechanism instead of by modifying the input list of refs.

This does require clients to sometimes generate the ref map twice: once
from the list of refs provided by the remote (as is currently done) and
potentially once from the new list of refs that the fetch mechanism
provides.

Signed-off-by: Brandon Williams 
---
 builtin/clone.c  |  4 ++--
 builtin/fetch.c  | 23 +++
 fetch-object.c   |  2 +-
 fetch-pack.c | 17 +
 transport-helper.c   |  6 --
 transport-internal.h |  9 -
 transport.c  | 34 --
 transport.h  |  3 ++-
 8 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 99e73dae8..8f86d99c5 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1155,7 +1155,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
}
 
if (!is_local && !complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1197,7 +1197,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
if (is_local)
clone_local(path, git_dir);
else if (refs && complete_refs_before_fetch)
-   transport_fetch_refs(transport, mapped_refs);
+   transport_fetch_refs(transport, mapped_refs, NULL);
 
update_remote_refs(refs, mapped_refs, remote_head_points_at,
   branch_top.buf, reflog_msg.buf, transport,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b600e1f10..ddf44ba1a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -946,11 +946,13 @@ static int quickfetch(struct ref *ref_map)
return check_connected(iterate_ref_map, , );
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map)
+static int fetch_refs(struct transport *transport, struct ref *ref_map,
+ struct ref **updated_remote_refs)
 {
int ret = quickfetch(ref_map);
if (ret)
-   ret = transport_fetch_refs(transport, ref_map);
+   ret = transport_fetch_refs(transport, ref_map,
+  updated_remote_refs);
if (ret)
transport_unlock_pack(transport);
return ret;
@@ -1106,7 +1108,7 @@ static void backfill_tags(struct transport *transport, 
struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-   if (!fetch_refs(transport, ref_map))
+   if (!fetch_refs(transport, ref_map, NULL))
consume_refs(transport, ref_map);
 
if (gsecondary) {
@@ -1122,6 +1124,7 @@ static int do_fetch(struct transport *transport,
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
const struct ref *remote_refs;
+   struct ref *new_remote_refs = NULL;
struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
if (tags == TAGS_DEFAULT) {
@@ -1172,7 +1175,19 @@ static int do_fetch(struct transport *transport,
   transport->url);
}
}
-   if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) 
{
+
+   if (fetch_refs(transport, ref_map, _remote_refs)) {
+   free_refs(ref_map);
+   retcode = 1;
+   goto cleanup;
+   }
+   if (new_remote_refs) {
+   free_refs(ref_map);
+   ref_map = get_ref_map(transport->remote, new_remote_refs, rs,
+ tags, );
+   free_refs(new_remote_refs);
+   }
+   if (consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
diff --git a/fetch-object.c b/fetch-object.c
index 853624f81..48fe63dd6 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref 
*ref)
 
transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1");
-   transport_fetch_refs(transport, ref);
+   transport_fetch_refs(transport, ref, NULL);
fetch_if_missing = original_fetch_if_missing;
 }
 
diff --git a/fetch-pack.c b/fetch-pack.c
index a320ce987..7799ee2cd 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1470,12 +1470,13 @@ static int 

Re: format-patch: no 'prerequisite-patch-id' info when specifying commit range

[Ye Xiaolong]

On 06/04, Junio C Hamano wrote:
>Ye Xiaolong  writes:
>
>> I narrowed down the problem to revision walk, if users specify the commit 
>> range
>> via "Z..C" pattern, the first prepare_revision_walk function called in
>> cmd_format_patch would mark all parents (ancestors) of Z to be uninteresting,
>> thus the next revision walk in prepare_bases wouldn't be able to reach
>> prerequisite patches, one quick solution I can think of is to clear
>> UNINTERESTING flag in reset_revision_walk, like below:
>>
>> void reset_revision_walk(void)
>> {
>>  clear_object_flags(SEEN | ADDED | SHOWN| UNINTERESTING);
>> }
>
>When you are done with objects that are UNINTERESTING in your
>application (i.e. only when "format-patch" is told to compute list
>of prereq patches by doing an extra revision walk), your application
>can call clear_object_flags() on the flags you are done with, I
>would think.
>
>But the current callers of reset_revision_walk() do not expect any
>flags other than the ones that are used to keep track of the
>traversal state, so it is likely you will break them if you suddenly
>started to clear flags randomly.

Got it, I'll try to call clear_object_flags in format-patch related codepatch
only, not to touch the global reset_revision_walk.

Thanks,
Xiaolong

Re: format-patch: no 'prerequisite-patch-id' info when specifying commit range

[Junio C Hamano]

Ye Xiaolong  writes:

> I narrowed down the problem to revision walk, if users specify the commit 
> range
> via "Z..C" pattern, the first prepare_revision_walk function called in
> cmd_format_patch would mark all parents (ancestors) of Z to be uninteresting,
> thus the next revision walk in prepare_bases wouldn't be able to reach
> prerequisite patches, one quick solution I can think of is to clear
> UNINTERESTING flag in reset_revision_walk, like below:
>
> void reset_revision_walk(void)
> {
>   clear_object_flags(SEEN | ADDED | SHOWN| UNINTERESTING);
> }

When you are done with objects that are UNINTERESTING in your
application (i.e. only when "format-patch" is told to compute list
of prereq patches by doing an extra revision walk), your application
can call clear_object_flags() on the flags you are done with, I
would think.

But the current callers of reset_revision_walk() do not expect any
flags other than the ones that are used to keep track of the
traversal state, so it is likely you will break them if you suddenly
started to clear flags randomly.

Re: format-patch: no 'prerequisite-patch-id' info when specifying commit range

[Ye Xiaolong]

Hi, Junio

On 05/29, Eduardo Habkost wrote:
>Hi,
>
>I'm trying to use git-format-patch --base to generate the list of
>prerequisite patches for a series, but the behavior of git
>doesn't seem to match the documentation:
>
>When using a commit count (e.g.: "-2"), git-format-patch generates the
>prerequisite-patch-id lines as expected.  But when using a commit range like
>"Z..C", the prerequisite-patch-id lines are missing.
>

I narrowed down the problem to revision walk, if users specify the commit range
via "Z..C" pattern, the first prepare_revision_walk function called in
cmd_format_patch would mark all parents (ancestors) of Z to be uninteresting,
thus the next revision walk in prepare_bases wouldn't be able to reach
prerequisite patches, one quick solution I can think of is to clear
UNINTERESTING flag in reset_revision_walk, like below:

void reset_revision_walk(void)
{
clear_object_flags(SEEN | ADDED | SHOWN| UNINTERESTING);
}

Though I'm not sure whether it has some side effects, or whether it would impact
behavior of other reference of reset_revision_walk. If you think it's a sensible
solution, I'll submit a patch.

Thanks,
Xiaolong


>Is this intentional, or it is a bug?
>
>Example using git.git commits:
>
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 -2 
> b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  prerequisite-patch-id: 080ac2faf21a6a7f9b23cb68286866d026a92930
>  prerequisite-patch-id: e3ee77500c9aa70248e7ee814662d01f79d0dcdb
>  prerequisite-patch-id: 6d831e23e33075681e6b74553151a32b73092013
>  (ehabkost@localhost:~/rh/proj/git (ok) 1j)
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 
> b7b1fca17~2..b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  $ git --version
>  git version 2.17.1
>  $ git log --graph --pretty=oneline -6 b7b1fca17
>  * b7b1fca175f1ed7933f361028c631b9ac86d868d fsck: complain when .gitmodules 
> is a symlink
>  * 73c3f0f704a91b6792e0199a3f3ab6e3a1971675 index-pack: check .gitmodules 
> files with --strict
>  * 6e328d6caef218db320978e3e251009135d87d0e unpack-objects: call 
> fsck_finish() after fscking objects
>  * 1995b5e03e1cc97116be58cdc0502d4a23547856 fsck: call fsck_finish() after 
> fscking objects
>  * ed8b10f631c9a71df3351d46187bf7f3fa4f9b7e fsck: check .gitmodules content
>  * 2738744426c161a98c2ec494d41241a4c5eef9ef fsck: handle promisor objects in 
> .gitmodules check
>  $ 
>
>If I understand the documentation correctly, both "-3 C" or "Z..C" were
>supposed to be equivalent:
>
>> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
>> `--cover-letter` or using `Z..C` instead of `-3 C` to specify the
>> range), the base tree information block is shown at the end of the
>> first message the command outputs (either the first patch, or the
>> cover letter), like this:
>> 
>> 
>> base-commit: P
>> prerequisite-patch-id: X
>> prerequisite-patch-id: Y
>> prerequisite-patch-id: Z
>> 
>
>-- 
>Eduardo

Re: format-patch: no 'prerequisite-patch-id' info when specifying commit range

[Ye Xiaolong]

Hi, Eduardo

On 05/29, Eduardo Habkost wrote:
>Hi,
>
>I'm trying to use git-format-patch --base to generate the list of
>prerequisite patches for a series, but the behavior of git
>doesn't seem to match the documentation:
>
>When using a commit count (e.g.: "-2"), git-format-patch generates the
>prerequisite-patch-id lines as expected.  But when using a commit range like
>"Z..C", the prerequisite-patch-id lines are missing.
>
>Is this intentional, or it is a bug?

Thanks for reporting, it seems an unexpected behavior, I'll look into it.

Thanks,
Xiaolong

>
>Example using git.git commits:
>
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 -2 
> b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  prerequisite-patch-id: 080ac2faf21a6a7f9b23cb68286866d026a92930
>  prerequisite-patch-id: e3ee77500c9aa70248e7ee814662d01f79d0dcdb
>  prerequisite-patch-id: 6d831e23e33075681e6b74553151a32b73092013
>  (ehabkost@localhost:~/rh/proj/git (ok) 1j)
>  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 
> b7b1fca17~2..b7b1fca17 | egrep 'base-commit|prereq'
>  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
>  $ git --version
>  git version 2.17.1
>  $ git log --graph --pretty=oneline -6 b7b1fca17
>  * b7b1fca175f1ed7933f361028c631b9ac86d868d fsck: complain when .gitmodules 
> is a symlink
>  * 73c3f0f704a91b6792e0199a3f3ab6e3a1971675 index-pack: check .gitmodules 
> files with --strict
>  * 6e328d6caef218db320978e3e251009135d87d0e unpack-objects: call 
> fsck_finish() after fscking objects
>  * 1995b5e03e1cc97116be58cdc0502d4a23547856 fsck: call fsck_finish() after 
> fscking objects
>  * ed8b10f631c9a71df3351d46187bf7f3fa4f9b7e fsck: check .gitmodules content
>  * 2738744426c161a98c2ec494d41241a4c5eef9ef fsck: handle promisor objects in 
> .gitmodules check
>  $ 
>
>If I understand the documentation correctly, both "-3 C" or "Z..C" were
>supposed to be equivalent:
>
>> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
>> `--cover-letter` or using `Z..C` instead of `-3 C` to specify the
>> range), the base tree information block is shown at the end of the
>> first message the command outputs (either the first patch, or the
>> cover letter), like this:
>> 
>> 
>> base-commit: P
>> prerequisite-patch-id: X
>> prerequisite-patch-id: Y
>> prerequisite-patch-id: Z
>> 
>
>-- 
>Eduardo

format-patch: no 'prerequisite-patch-id' info when specifying commit range

[Eduardo Habkost]

Hi,

I'm trying to use git-format-patch --base to generate the list of
prerequisite patches for a series, but the behavior of git
doesn't seem to match the documentation:

When using a commit count (e.g.: "-2"), git-format-patch generates the
prerequisite-patch-id lines as expected.  But when using a commit range like
"Z..C", the prerequisite-patch-id lines are missing.

Is this intentional, or it is a bug?

Example using git.git commits:

  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 -2 
b7b1fca17 | egrep 'base-commit|prereq'
  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
  prerequisite-patch-id: 080ac2faf21a6a7f9b23cb68286866d026a92930
  prerequisite-patch-id: e3ee77500c9aa70248e7ee814662d01f79d0dcdb
  prerequisite-patch-id: 6d831e23e33075681e6b74553151a32b73092013
  (ehabkost@localhost:~/rh/proj/git (ok) 1j)
  $ git format-patch --stdout --cover-letter --stdout --base b7b1fca17~5 
b7b1fca17~2..b7b1fca17 | egrep 'base-commit|prereq'
  base-commit: 2738744426c161a98c2ec494d41241a4c5eef9ef
  $ git --version
  git version 2.17.1
  $ git log --graph --pretty=oneline -6 b7b1fca17
  * b7b1fca175f1ed7933f361028c631b9ac86d868d fsck: complain when .gitmodules is 
a symlink
  * 73c3f0f704a91b6792e0199a3f3ab6e3a1971675 index-pack: check .gitmodules 
files with --strict
  * 6e328d6caef218db320978e3e251009135d87d0e unpack-objects: call fsck_finish() 
after fscking objects
  * 1995b5e03e1cc97116be58cdc0502d4a23547856 fsck: call fsck_finish() after 
fscking objects
  * ed8b10f631c9a71df3351d46187bf7f3fa4f9b7e fsck: check .gitmodules content
  * 2738744426c161a98c2ec494d41241a4c5eef9ef fsck: handle promisor objects in 
.gitmodules check
  $ 

If I understand the documentation correctly, both "-3 C" or "Z..C" were
supposed to be equivalent:

> With `git format-patch --base=P -3 C` (or variants thereof, e.g. with
> `--cover-letter` or using `Z..C` instead of `-3 C` to specify the
> range), the base tree information block is shown at the end of the
> first message the command outputs (either the first patch, or the
> cover letter), like this:
> 
> 
> base-commit: P
> prerequisite-patch-id: X
> prerequisite-patch-id: Y
> prerequisite-patch-id: Z
> 

-- 
Eduardo

Re: [GSoC] Info: Week 02 Blog Post

[Pratik Karki]

On Thu, May 10, 2018 at 11:35 PM, Stefan Beller  wrote:
> Hi Pratik,

Hi Stefan,

> On Wed, May 9, 2018 at 8:07 PM, Pratik Karki  wrote:
>> Hi,
>>
>> The week 02 blog post[1] is live. This post is part I out of II and this
>> time it will be biweekly. The part II of will come in 2-3 days which
>> will describe the current `git-rebase.sh`.
>
> Cool post!

Thanks!

>> Do give me feedback.
>
> In "and you’re choice of DVCS is Git.", s/you're/your/

Fixed it! Thanks for pointing that out.

Re: [GSoC] Info: Week 02 Blog Post

[Stefan Beller]

Hi Pratik,

On Wed, May 9, 2018 at 8:07 PM, Pratik Karki  wrote:
> Hi,
>
> The week 02 blog post[1] is live. This post is part I out of II and this
> time it will be biweekly. The part II of will come in 2-3 days which
> will describe the current `git-rebase.sh`.

Cool post!

> Do give me feedback.

In "and you’re choice of DVCS is Git.", s/you're/your/

[PATCH v7 00/13] Keep all info in command-list.txt in git binary

[Nguyễn Thái Ngọc Duy]

v7 is mostly code cleanup plus one more change:

git-completion.bash now always checks completion.commands config key.
This makes it much more convenient to customize the command complete
list. You only have to fall back to setting $GIT_COMPLETION_CMD_GROUPS
when you have very special needs.

Interdiff

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2659153cb3..91f7eaed7b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1343,6 +1343,16 @@ credential..*::
 credentialCache.ignoreSIGHUP::
Tell git-credential-cache--daemon to ignore SIGHUP, instead of quitting.
 
+completion.commands::
+   This is only used by git-completion.bash to add or remove
+   commands from the complete list. Normally only porcelain
+   commands and a few select others are in the complete list. You
+   can add more commands, separated by space, in this
+   variable. Prefixing a command with '-' will remove it from
+   the existing list.
++
+This variable should not be per-repository.
+
 include::diff-config.txt[]
 
 difftool..path::
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 0fd29803d5..f7ca9a4d4f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -50,10 +50,10 @@
 #
 # GIT_COMPLETION_CMD_GROUPS=main,others
 #
-# Or you could go with defaults add some extra commands specified
-# in the configuration variable completion.commands [1] with
+# Or you could go with main porcelain only and extra commands in
+# the configuration variable completion.commands with
 #
-# GIT_COMPLETION_CMD_GROUPS=mainporcelain,others,list-complete,config
+# GIT_COMPLETION_CMD_GROUPS=mainporcelain,config
 #
 # Or go completely custom group with
 #
@@ -61,15 +61,6 @@
 #
 # Or you could even play with other command categories found in
 # command-list.txt.
-#
-# [1] Note that completion.commands should not be per-repository
-# since the command list is generated once and cached.
-#
-# completion.commands could be used to exclude commands as
-# well.  If a command in this list begins with '-', then it
-# will be excluded from the list of commands gathered by the
-# groups specified before "config" in
-# $GIT_COMPLETION_CMD_GROUPS.
 
 case "$COMP_WORDBREAKS" in
 *:*) : great ;;
@@ -876,7 +867,7 @@ __git_commands () {
then
git --list-cmds="$GIT_COMPLETION_CMD_GROUPS"
else
-   git --list-cmds=list-mainporcelain,others,list-complete
+   git 
--list-cmds=list-mainporcelain,others,list-complete,config
fi
;;
all)
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index bfd8ef0671..8d6d8b45ce 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -9,7 +9,7 @@ command_list () {
grep -v '^#' "$1"
 }
 
-get_categories() {
+get_categories () {
tr ' ' '\n'|
grep -v '^$' |
sort |
@@ -32,7 +32,7 @@ get_synopsis () {
}' "Documentation/$1.txt"
 }
 
-define_categories() {
+define_categories () {
echo
echo "/* Command categories */"
bit=0
@@ -45,7 +45,7 @@ define_categories() {
test "$bit" -gt 32 && die "Urgh.. too many categories?"
 }
 
-define_category_names() {
+define_category_names () {
echo
echo "/* Category names */"
echo "static const char *category_names[] = {"
@@ -60,7 +60,7 @@ define_category_names() {
echo "};"
 }
 
-print_command_list() {
+print_command_list () {
echo "static struct cmdname_help command_list[] = {"
 
command_list "$1" |
diff --git a/git.c b/git.c
index fd08911e11..ea4feedd0b 100644
--- a/git.c
+++ b/git.c
@@ -38,6 +38,13 @@ static int use_pager = -1;
 
 static void list_builtins(struct string_list *list, unsigned int 
exclude_option);
 
+static int match_token(const char *spec, int len, const char *token)
+{
+   int token_len = strlen(token);
+
+   return len == token_len && !strncmp(spec, token, token_len);
+}
+
 static int list_cmds(const char *spec)
 {
struct string_list list = STRING_LIST_INIT_DUP;
@@ -47,13 +54,13 @@ static int list_cmds(const char *spec)
const char *sep = strchrnul(spec, ',');
int len = sep - spec;
 
-   if (len == 8 && !strncmp(spec, "builtins", 8))
+   if (match_token(spec, len, "builtins"))
list_builtins(, 0);
-   else if (len == 4 && !strncmp(spec, "main", 4))
+   else if (match_token(spec, len, "main"))
list_all_main_cmds();
-   else if (len == 6 && !strncmp(spec, "others", 6))
+   else if (match_token(spec, len, "others"))
list_all_other_cmds();
-   else if (len == 6 && 

[GSoC] Info: Week 02 Blog Post

[Pratik Karki]

Hi,

The week 02 blog post[1] is live. This post is part I out of II and this
time it will be biweekly. The part II of will come in 2-3 days which
will describe the current `git-rebase.sh`.

Do give me feedback.

Thanks to all the awesome Git contributors for this awesome tool. :-)

[1]: https://prertik.github.io/categories/git/

Cheers,
Pratik Karki

[PATCH v6 00/13] Keep all info in command-list.txt in git binary

[Nguyễn Thái Ngọc Duy]

v6 is now "feature complete". v6 adds

- documentation in command-list.txt so people know how to update it
- support for config key completion.commands, which consists
  of extra commands. It could also be used for excluding some
  commands.

Interdiff

diff --git a/command-list.txt b/command-list.txt
index 40776b9587..3e21ddfcfb 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -1,3 +1,47 @@
+# Command classification list
+# ---
+# All supported commands, builtin or external, must be described in
+# here. This info is used to list commands in various places. Each
+# command is on one line followed by one or more attributes.
+#
+# The first attribute group is mandatory and indicates the command
+# type. This group includes:
+#
+#   mainporcelain
+#   ancillarymanipulators
+#   ancillaryinterrogators
+#   foreignscminterface
+#   plumbingmanipulators
+#   plumbinginterrogators
+#   synchingrepositories
+#   synchelpers
+#   purehelpers
+#
+# The type names are self explanatory. But if you want to see what
+# command belongs to what group to get a better picture, have a look
+# at "git" man page, "GIT COMMANDS" section.
+#
+# Commands of type mainporcelain can also optionally have one of these
+# attributes:
+#
+#   init
+#   worktree
+#   info
+#   history
+#   remote
+#
+# These commands are considered "common" and will show up in "git
+# help" output in groups. Uncommon porcelain commands must not
+# specify any of these attributes.
+#
+# "complete" attribute is used to mark that the command should be
+# completable by git-completion.bash. Note that by default,
+# mainporcelain commands are completable so you don't need this
+# attribute.
+#
+# While not true commands, guides are also specified here, which can
+# only have "guide" attribute and nothing else.
+#
 ### command list (do not change this line, also do not change alignment)
 # command name  category [category] [category]
 git-add mainporcelain   worktree
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 908692ea52..0fd29803d5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -38,6 +38,38 @@
 #
 # When set to "1", do not include "DWIM" suggestions in git-checkout
 # completion (e.g., completing "foo" when "origin/foo" exists).
+#
+#   GIT_COMPLETION_CMD_GROUPS
+#
+# When set, "git --list-cmds=$GIT_COMPLETION_CMD_GROUPS" will be
+# used to get the list of completable commands. The default is
+# "mainporcelain,others,list-complete" (in English: all porcelain
+# commands and external ones are included. Certain non-porcelain
+# commands are also marked for completion in command-list.txt).
+# You could for example complete all commands with
+#
+# GIT_COMPLETION_CMD_GROUPS=main,others
+#
+# Or you could go with defaults add some extra commands specified
+# in the configuration variable completion.commands [1] with
+#
+# GIT_COMPLETION_CMD_GROUPS=mainporcelain,others,list-complete,config
+#
+# Or go completely custom group with
+#
+# GIT_COMPLETION_CMD_GROUPS=config
+#
+# Or you could even play with other command categories found in
+# command-list.txt.
+#
+# [1] Note that completion.commands should not be per-repository
+# since the command list is generated once and cached.
+#
+# completion.commands could be used to exclude commands as
+# well.  If a command in this list begins with '-', then it
+# will be excluded from the list of commands gathered by the
+# groups specified before "config" in
+# $GIT_COMPLETION_CMD_GROUPS.
 
 case "$COMP_WORDBREAKS" in
 *:*) : great ;;
@@ -840,6 +872,9 @@ __git_commands () {
if test -n "$GIT_TESTING_PORCELAIN_COMMAND_LIST"
then
printf "%s" "$GIT_TESTING_PORCELAIN_COMMAND_LIST"
+   elif test -n "$GIT_COMPLETION_CMD_GROUPS"
+   then
+   git --list-cmds="$GIT_COMPLETION_CMD_GROUPS"
else
git --list-cmds=list-mainporcelain,others,list-complete
fi
diff --git a/git.c b/git.c
index 1c8b0c93e1..fd08911e11 100644
--- a/git.c
+++ b/git.c
@@ -36,27 +36,30 @@ const char git_more_info_string[] =
 
 static int use_pager = -1;
 
-static void list_builtins(unsigned int exclude_option, char sep);
+static void list_builtins(struct string_list *list, unsigned int 
exclude_option);
 
 static int list_cmds(const char *spec)
 {
+   struct string_list list = STRING_LIST_INIT_DUP;
+   int i;
+
while (*spec) {
const char *sep = strchr

Re: [GSoC] Info: new blog series of my work on Git GSoC '18

[Pratik Karki]

On Thu, May 3, 2018 at 6:31 AM, Andrew Ardill  wrote:
> On 2 May 2018 at 17:12, Johannes Schindelin  
> wrote:
>> Hi Pratik,
>>
>> On Wed, 2 May 2018, Pratik Karki wrote:
>>
>>> As promised in my proposal, I've started
>>> to write a blog series of GSoC '18 with Git. The initial blog is up.
>>> You can find it here[1]. The initial one is just to get started and
>>> from next iterations, I'll start detailing of my work towards converting
>>> rebase to builtin.
>>>
>>> [1]: https://prertik.github.io/categories/git/
>>
>> This is awesome! Thanks for doing this,
>> Dscho
>
> Agreed, was fun to read.
>
> I'd encourage you to post to the list when you blog, or perhaps
> include a link to the blog as part of any regular updates you give on
> your project progress.
>
> Would also make for an interesting addition to the newsletter.
>
> I know it can be difficult to sit down and write about what you're
> doing, especially when it feels like you could be focusing on 'real
> work'. Hopefully you will find the process rewarding; I'm looking
> forward to reading about what you find easy and hard, how you learn
> the git developer processes, and the challenges you find in converting
> shell scripts to a built-in. I'm sure other people are too, and I'll
> bet the ones who have been there before will have feedback for you as
> well.
>
> I'd find it interesting even if it was a 5-line bullet list of what's
> going through your mind with respect to the project! Looking forward
> to following along.
>
> Regards,
>
> Andrew Ardill

Thank you. Feedbacks mean a lot to me.
I will be writing a weekly blog of the progress
I made during the week. I will try my best to detail
things I did and intend to do. I will surely notify
everyone as soon as I publish a post.

Cheers,
Pratik Karki

Re: [GSoC] Info: new blog series of my work on Git GSoC '18

[Andrew Ardill]

On 2 May 2018 at 17:12, Johannes Schindelin  wrote:
> Hi Pratik,
>
> On Wed, 2 May 2018, Pratik Karki wrote:
>
>> As promised in my proposal, I've started
>> to write a blog series of GSoC '18 with Git. The initial blog is up.
>> You can find it here[1]. The initial one is just to get started and
>> from next iterations, I'll start detailing of my work towards converting
>> rebase to builtin.
>>
>> [1]: https://prertik.github.io/categories/git/
>
> This is awesome! Thanks for doing this,
> Dscho

Agreed, was fun to read.

I'd encourage you to post to the list when you blog, or perhaps
include a link to the blog as part of any regular updates you give on
your project progress.

Would also make for an interesting addition to the newsletter.

I know it can be difficult to sit down and write about what you're
doing, especially when it feels like you could be focusing on 'real
work'. Hopefully you will find the process rewarding; I'm looking
forward to reading about what you find easy and hard, how you learn
the git developer processes, and the challenges you find in converting
shell scripts to a built-in. I'm sure other people are too, and I'll
bet the ones who have been there before will have feedback for you as
well.

I'd find it interesting even if it was a 5-line bullet list of what's
going through your mind with respect to the project! Looking forward
to following along.

Regards,

Andrew Ardill

Re: [GSoC] Info: new blog series of my work on Git GSoC '18

[Johannes Schindelin]

Hi Pratik,

On Wed, 2 May 2018, Pratik Karki wrote:

> As promised in my proposal, I've started
> to write a blog series of GSoC '18 with Git. The initial blog is up.
> You can find it here[1]. The initial one is just to get started and
> from next iterations, I'll start detailing of my work towards converting
> rebase to builtin.
> 
> [1]: https://prertik.github.io/categories/git/

This is awesome! Thanks for doing this,
Dscho

[GSoC] Info: new blog series of my work on Git GSoC '18

[Pratik Karki]

Hello mentors,

As promised in my proposal, I've started
to write a blog series of GSoC '18 with Git. The initial blog is up.
You can find it here[1]. The initial one is just to get started and
from next iterations, I'll start detailing of my work towards converting
rebase to builtin.

[1]: https://prertik.github.io/categories/git/

Cheers,
Pratik Karki

[PATCH v2 02/42] server-info: remove unused members from struct pack_info

[brian m. carlson]

The head member of struct pack_info is completely unused and the
nr_heads member is used only in one place, which is an assignment.  This
member was last usefully used in 3e15c67c90 (server-info: throw away T
computation as well, 2005-12-04).

Since this structure member is not useful, remove it.

Signed-off-by: brian m. carlson <sand...@crustytoothpaste.net>
---
 server-info.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/server-info.c b/server-info.c
index 83460ec0d6..7ce6dcd67b 100644
--- a/server-info.c
+++ b/server-info.c
@@ -92,8 +92,6 @@ static struct pack_info {
int old_num;
int new_num;
int nr_alloc;
-   int nr_heads;
-   unsigned char (*head)[20];
 } **info;
 static int num_pack;
 static const char *objdir;
@@ -225,12 +223,9 @@ static void init_pack_info(const char *infofile, int force)
else
stale = 1;
 
-   for (i = 0; i < num_pack; i++) {
-   if (stale) {
+   for (i = 0; i < num_pack; i++)
+   if (stale)
info[i]->old_num = -1;
-   info[i]->nr_heads = 0;
-   }
-   }
 
/* renumber them */
QSORT(info, num_pack, compare_info);

Re: [PATCH 02/41] server-info: remove unused members from struct pack_info

[Duy Nguyen]

On Mon, Apr 23, 2018 at 11:39:12PM +, brian m. carlson wrote:
> The head member of struct pack_info is completely unused and the
> nr_heads member is used only in one place, which is an assignment.
> Since these structure members are not useful, remove them.

If you reroll, you could add that their last use was in 3e15c67c90
(server-info: throw away T computation as well. - 2005-12-04)
--
Duy

Re: [PATCH v5 00/10] Keep all info in command-list.txt in git binary

[Duy Nguyen]

This is probably scope creep for this series, but do you guys think we
should do the same for config variables completion? We currently
maintain a giant list at the end of _git_config(). Extracting the list
from Documentation/config.txt to keep it in a C array does not look
super hard. There will be some special handling for advice.* or
color but overall I still think it's a net gain.

Listing all recognizable config variables from "git config" (or "git
help") would be lovely, but I don't think it helps unless we could
print a short summary line each variable, but this info is not
available anywhere.
--
Duy

[PATCH v5 00/10] Keep all info in command-list.txt in git binary

[Nguyễn Thái Ngọc Duy]

I think v5 is getting close to what I wanted to achieve from the RFC
version (I skip v4 since I sent out v4/wip and another v4 may confuse
people).

Interdiff is too large to be helpful, but the summary of changes
compared to v3 is:

- common-cmds.h is renamed to command-list.h
- the common group description is moved from command-list.txt to
  help.c to simplify command-list.txt format
- generate-cmds.sh supports multiple categories per command, a new one
  "complete" is added to aid git-completion.bash
- multiple --list-cmds options is replaced with
  --list-cmds=[,...]. This allows easier group
  customization in git-completion.bash (not happens yet)
- __git_list_all_commands() and __git_list_porcelain_commands() for
  backward compatibility
- "git help " completion also makes use of guide list in
  command-list.txt
- better tests from Ramsay

There is one sticky point yet about the guides. I'll pull Phillip in
and explain more in the relevant patch.

Nguyễn Thái Ngọc Duy (10):
  generate-cmds.sh: factor out synopsis extract code
  generate-cmds.sh: export all commands to command-list.h
  help: use command-list.h for common command list
  Remove common-cmds.h
  git.c: convert --list-*builtins to --list-cmds=*
  completion: implement and use --list-cmds=main,others
  git: support --list-cmds=list-
  help: add "-a --verbose" to list all commands with synopsis
  help: use command-list.txt for the source of guides
  completion: let git provide the completable command list

 .gitignore |   2 +-
 Documentation/git-help.txt |   4 +-
 Documentation/gitattributes.txt|   2 +-
 Documentation/gitmodules.txt   |   2 +-
 Documentation/gitrevisions.txt |   2 +-
 Makefile   |  16 +-
 builtin/help.c |  39 +
 command-list.txt   |  67 
 contrib/completion/git-completion.bash | 134 +---
 generate-cmdlist.sh| 126 ++-
 git.c  |  38 -
 help.c | 209 +
 help.h |   5 +
 t/t0012-help.sh|  26 ++-
 t/t9902-completion.sh  |   5 +-
 15 files changed, 426 insertions(+), 251 deletions(-)

-- 
2.17.0.664.g8924eee37a

[Re-send PATCH v7 00/12] Deprecate .git/info/grafts

[Johannes Schindelin]

[Resent the cover letter, as I did not see it on public-inbox...]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft 
[...]`.

Changes since v6:

- Made --convert-graft-file issue a mere warning (instead of an error) when
  a graft leaves the parents unchanged, and is hence unnecessary.

- Modified the regression test for --convert-graft-file to succeed even when
  the GPG prerequisite is unmet (thanks, Gábor!).

- Reassured by Stefan's feedback, I refrained from reinstating the contrib/
  script.


Johannes Schindelin (12):
  argv_array: offer to split a string by whitespace
  commit: Let the callback of for_each_mergetag return on error
  replace: avoid using die() to indicate a bug
  replace: "libify" create_graft() and callees
  replace: prepare create_graft() for converting graft files wholesale
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: stop referring to grafts
  technical/shallow: describe why shallow cannot use replace refs
  Remove obsolete script to convert grafts to replace refs

 Documentation/git-filter-branch.txt   |   2 +-
 Documentation/git-replace.txt |  11 +-
 Documentation/technical/shallow.txt   |  20 +-
 advice.c  |   2 +
 advice.h  |   1 +
 argv-array.c  |  20 ++
 argv-array.h  |   2 +
 builtin/replace.c | 234 --
 commit.c  |  18 +-
 commit.h  |   4 +-
 contrib/convert-grafts-to-replace-refs.sh |  28 ---
 log-tree.c|  13 +-
 t/t6001-rev-list-graft.sh |   9 +
 t/t6050-replace.sh|  28 +++
 14 files changed, 274 insertions(+), 118 deletions(-)
 delete mode 100755 contrib/convert-grafts-to-replace-refs.sh


base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v7
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v7

Interdiff vs v6:
 diff --git a/builtin/replace.c b/builtin/replace.c
 index 35394ec1874..a87fca045be 100644
 --- a/builtin/replace.c
 +++ b/builtin/replace.c
 @@ -429,7 +429,7 @@ static int check_mergetags(struct commit *commit, int 
argc, const char **argv)
return for_each_mergetag(check_one_mergetag, commit, _data);
  }
  
 -static int create_graft(int argc, const char **argv, int force)
 +static int create_graft(int argc, const char **argv, int force, int gentle)
  {
struct object_id old_oid, new_oid;
const char *old_ref = argv[0];
 @@ -471,8 +471,13 @@ static int create_graft(int argc, const char **argv, int 
force)
  
strbuf_release();
  
 -  if (!oidcmp(_oid, _oid))
 +  if (!oidcmp(_oid, _oid)) {
 +  if (gentle) {
 +  warning("graft for '%s' unnecessary", 
oid_to_hex(_oid));
 +  return 0;
 +  }
return error("new commit is the same as the old one: '%s'", 
oid_to_hex(_oid));
 +  }
  
return replace_object_oid(old_ref, _oid, "replacement", _oid, 
force);
  }
 @@ -492,7 +497,7 @@ static int convert_graft_file(int force)
continue;
  
argv_array_split(, buf.buf);
 -  if (args.argc && create_graft(args.argc, args.argv, force))
 +  if (args.argc && create_graft(args.argc, args.argv, force, 1))
strbuf_addf(, "\n\t%s", buf.buf);
argv_array_clear();
}
 @@ -583,7 +588,7 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
if (argc < 1)
usage_msg_opt("-g needs at least one argument",
  git_replace_usage, options);
 -  return create_graft(argc, argv, force);
 +  return create_graft(argc, argv, force, 0);
  
case MODE_CONVERT_GRAFT_FILE:
if (argc != 0)
 diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
 index bed86a0af3d..d174bfed309 100755
 --- a/t/t6050-replace.sh
 +++ b/t/t6050-replace.sh
 @@ -445,6 +445,14 @@ test_expect_success GPG '--graft on a commit with a 
mergetag' '
  '
  
  test_expect_success '--convert-graft-file' '
 +  git checkout -b with-graft-file &&
 +  test_commit root2 &&
 +  git reset --hard root2^ &&
 +  test_commit root1 &&
 +  test_com

[PATCH v7 08/12] Deprecate support for .git/info/grafts

[Johannes Schindelin]

The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.

Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.

For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.

Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.

The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.

Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Reviewed-by: Stefan Beller <sbel...@google.com>
---
 advice.c  |  2 ++
 advice.h  |  1 +
 commit.c  | 10 ++
 t/t6001-rev-list-graft.sh |  9 +
 4 files changed, 22 insertions(+)

diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
 
 static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", _add_embedded_repo },
{ "ignoredhook", _ignored_hook },
{ "waitingforeditor", _waiting_for_editor },
+   { "graftfiledeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", _push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index 2952ec987c5..451d3ce8dfe 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
 #include "prio-queue.h"
 #include "sha1-lookup.h"
 #include "wt-status.h"
+#include "advice.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+   if (advice_graft_file_deprecated)
+   advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
 
 done
+
+test_expect_success 'show advice that grafts are deprecated' '
+   git show HEAD 2>err &&
+   test_i18ngrep "git replace" err &&
+   test_config advice.graftFileDeprecated false &&
+   git show HEAD 2>err &&
+   test_i18ngrep ! "git replace" err
+'
+
 test_done
-- 
2.17.0.windows.1.36.gdf4ca5fb72a

[PATCH v6 07/11] Deprecate support for .git/info/grafts

[Johannes Schindelin]

The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.

Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.

For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.

Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.

The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.

Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Reviewed-by: Stefan Beller <sbel...@google.com>
---
 advice.c  |  2 ++
 advice.h  |  1 +
 commit.c  | 10 ++
 t/t6001-rev-list-graft.sh |  9 +
 4 files changed, 22 insertions(+)

diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
 
 static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", _add_embedded_repo },
{ "ignoredhook", _ignored_hook },
{ "waitingforeditor", _waiting_for_editor },
+   { "graftfiledeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", _push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index 2952ec987c5..451d3ce8dfe 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
 #include "prio-queue.h"
 #include "sha1-lookup.h"
 #include "wt-status.h"
+#include "advice.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+   if (advice_graft_file_deprecated)
+   advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
 
 done
+
+test_expect_success 'show advice that grafts are deprecated' '
+   git show HEAD 2>err &&
+   test_i18ngrep "git replace" err &&
+   test_config advice.graftFileDeprecated false &&
+   git show HEAD 2>err &&
+   test_i18ngrep ! "git replace" err
+'
+
 test_done
-- 
2.17.0.windows.1.33.gfcbb1fa0445

[PATCH v6 00/11] Deprecate .git/info/grafts

[Johannes Schindelin]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft 
[...]`.

Changes since v5:

- Disentangled the lumped-together conditional blocks in
  edit_and_replace() again.

- Moved fixup (a superfluous argv_array_clear()) from the patch that
  adds a test for --convert-graft-file back to the patch that actually
  introduces that option.


Johannes Schindelin (11):
  argv_array: offer to split a string by whitespace
  commit: Let the callback of for_each_mergetag return on error
  replace: avoid using die() to indicate a bug
  replace: "libify" create_graft() and callees
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: stop referring to grafts
  technical/shallow: describe why shallow cannot use replace refs
  Remove obsolete script to convert grafts to replace refs

 Documentation/git-filter-branch.txt   |   2 +-
 Documentation/git-replace.txt |  11 +-
 Documentation/technical/shallow.txt   |  20 +-
 advice.c  |   2 +
 advice.h  |   1 +
 argv-array.c  |  20 ++
 argv-array.h  |   2 +
 builtin/replace.c | 223 --
 commit.c  |  18 +-
 commit.h  |   4 +-
 contrib/convert-grafts-to-replace-refs.sh |  28 ---
 log-tree.c|  13 +-
 t/t6001-rev-list-graft.sh |   9 +
 t/t6050-replace.sh|  20 ++
 14 files changed, 258 insertions(+), 115 deletions(-)
 delete mode 100755 contrib/convert-grafts-to-replace-refs.sh


base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v6
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v6

Interdiff vs v5:
 diff --git a/builtin/replace.c b/builtin/replace.c
 index acd30e3d824..35394ec1874 100644
 --- a/builtin/replace.c
 +++ b/builtin/replace.c
 @@ -326,10 +326,15 @@ static int edit_and_replace(const char *object_ref, int 
force, int raw)
strbuf_release();
  
tmpfile = git_pathdup("REPLACE_EDITOBJ");
 -  if (export_object(_oid, type, raw, tmpfile) ||
 -  (launch_editor(tmpfile, NULL, NULL) < 0 &&
 -   error("editing object file failed")) ||
 -  import_object(_oid, type, raw, tmpfile)) {
 +  if (export_object(_oid, type, raw, tmpfile)) {
 +  free(tmpfile);
 +  return -1;
 +  }
 +  if (launch_editor(tmpfile, NULL, NULL) < 0) {
 +  free(tmpfile);
 +  return error("editing object file failed");
 +  }
 +  if (import_object(_oid, type, raw, tmpfile)) {
free(tmpfile);
return -1;
}
-- 
2.17.0.windows.1.33.gfcbb1fa0445

Re: [PATCH v5 00/11] Deprecate .git/info/grafts

[Johannes Schindelin]

Hi Junio,

On Thu, 26 Apr 2018, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> >  -  if (export_object(_oid, type, raw, tmpfile))
> >  -  return -1;
> >  -  if (launch_editor(tmpfile, NULL, NULL) < 0)
> >  -  return error("editing object file failed");
> >  -  if (import_object(_oid, type, raw, tmpfile))
> >  +  tmpfile = git_pathdup("REPLACE_EDITOBJ");
> >  +  if (export_object(_oid, type, raw, tmpfile) ||
> >  +  (launch_editor(tmpfile, NULL, NULL) < 0 &&
> >  +   error("editing object file failed")) ||
> >  +  import_object(_oid, type, raw, tmpfile)) {
> >  +  free(tmpfile);
> > return -1;
> >  -
> >  +  }
> 
> I know the above is to avoid leaking tmpfile, but a single if ()
> condition that makes multiple calls to functions primarily for their
> side effects is too ugly to live.

I changed it back to individual conditional blocks, with every single one
of them having their own free(tmpfile). That is at least clearer.

Ciao,
Dscho

Re: [PATCH v5 00/11] Deprecate .git/info/grafts

[Junio C Hamano]

Johannes Schindelin  writes:

>  -if (export_object(_oid, type, raw, tmpfile))
>  -return -1;
>  -if (launch_editor(tmpfile, NULL, NULL) < 0)
>  -return error("editing object file failed");
>  -if (import_object(_oid, type, raw, tmpfile))
>  +tmpfile = git_pathdup("REPLACE_EDITOBJ");
>  +if (export_object(_oid, type, raw, tmpfile) ||
>  +(launch_editor(tmpfile, NULL, NULL) < 0 &&
>  + error("editing object file failed")) ||
>  +import_object(_oid, type, raw, tmpfile)) {
>  +free(tmpfile);
>   return -1;
>  -
>  +}

I know the above is to avoid leaking tmpfile, but a single if ()
condition that makes multiple calls to functions primarily for their
side effects is too ugly to live.  Perhaps something like

if (export_object(...))
goto clean_fail;
if (launch_editor(...)) {
error("editing object file failed");
goto clean_fail;
}
if (import_object(...)) {
clean_fail:
free(tmpfile);
return -1;
}

would keep the ease-of-reading of the original while plugging the
leak.  It would even be cleaner to move the body of clean_fail:
completely out of line, instead of having it in the last of three
steps like the above.

Other than that, looked cleanly done.  Thanks for a pleasant read.

Will queue.

[PATCH v4/wip 00/12] Keep all info in command-list.txt in git binary

[Nguyễn Thái Ngọc Duy]

This is not exactly v4 and likely broken. But I've made several
debatable changes and would like your opinions before making even more
changes in this direction.

In 03/12 I made a format change in command-list.txt. The group
description is no longer in this file but in help.c instead. This
simplifies the format. However it may be harder for people to know
what category means what (but then it's already so for a lot more
categories).

In 11/12 I added the new "complete" category to command-list.txt so
that we could replace the giant list in git-completion.bash. This is
really questionable because several commands will NOT be completable
anymore. These are listed there so we can discuss.

Another thing I wonder is, whether I should tag "nocomplete" instead
of "complete" in command-list.txt, similar to parseopt's nocomplete
strategy: commands are completable by default then we just exclude
some of them. The "complete" tag goes the opposite direction, we only
show ones that are either external, "complete" or mainporcelain.

I think we would go with whitelist than blacklist though to avoid
helper commands showing up by default...

Also in 02/12 I moved to fixed column format. Strictly speaking I
do not need that (but it makes the code slightly more complex). If you
are really against fixed column format, speak up now.

Nguyễn Thái Ngọc Duy (12):
  generate-cmds.sh: factor out synopsis extract code
  generate-cmds.sh: export all commands to command-list.h
  help: use command-list.h for common command list
  Remove common-cmds.h
  git.c: convert --list-*builtins to --list-cmds=*
  git: accept multiple --list-cmds options
  completion: implement and use --list-cmds=all
  git: support --list-cmds=
  help: add "-a --verbose" to list all commands with synopsis
  help: use command-list.txt for the source of guides
  command-list.txt: add new category "complete"
  completion: let git provide the completable command list

 .gitignore |   2 +-
 Documentation/git-help.txt |   4 +-
 Documentation/gitattributes.txt|   2 +-
 Documentation/gitmodules.txt   |   2 +-
 Documentation/gitrevisions.txt |   2 +-
 Makefile   |  16 +--
 builtin/help.c |  39 +-
 command-list.txt   |  55 
 contrib/completion/git-completion.bash | 121 +++-
 generate-cmdlist.sh| 128 +++--
 git.c  |  19 ++-
 help.c | 186 +
 help.h |   4 +
 t/t0012-help.sh|  11 +-
 t/t9902-completion.sh  |   5 +-
 15 files changed, 344 insertions(+), 252 deletions(-)

-- 
2.17.0.519.gb89679a4aa

[PATCH v5 07/11] Deprecate support for .git/info/grafts

[Johannes Schindelin]

The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.

Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.

For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.

Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.

The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.

Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Reviewed-by: Stefan Beller <sbel...@google.com>
---
 advice.c  |  2 ++
 advice.h  |  1 +
 commit.c  | 10 ++
 t/t6001-rev-list-graft.sh |  9 +
 4 files changed, 22 insertions(+)

diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
 
 static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", _add_embedded_repo },
{ "ignoredhook", _ignored_hook },
{ "waitingforeditor", _waiting_for_editor },
+   { "graftfiledeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", _push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index 2952ec987c5..451d3ce8dfe 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
 #include "prio-queue.h"
 #include "sha1-lookup.h"
 #include "wt-status.h"
+#include "advice.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+   if (advice_graft_file_deprecated)
+   advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
 
 done
+
+test_expect_success 'show advice that grafts are deprecated' '
+   git show HEAD 2>err &&
+   test_i18ngrep "git replace" err &&
+   test_config advice.graftFileDeprecated false &&
+   git show HEAD 2>err &&
+   test_i18ngrep ! "git replace" err
+'
+
 test_done
-- 
2.17.0.windows.1.33.gfcbb1fa0445

[PATCH v5 00/11] Deprecate .git/info/grafts

[Johannes Schindelin]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft 
[...]`.

Changes since v4:

- Fixed resource leaks (missing free() and strbuf_release() calls when
  returning an error).

- Avoided error_errno() mistakenly using close()'s errno.

- Actually close the handle to the graft file after converting it.

- Changed the description of Documentation/technical/shallow to not even
  bother to describe how it might be construed to be similar to replace
  refs.


Johannes Schindelin (11):
  argv_array: offer to split a string by whitespace
  commit: Let the callback of for_each_mergetag return on error
  replace: avoid using die() to indicate a bug
  replace: "libify" create_graft() and callees
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: stop referring to grafts
  technical/shallow: describe why shallow cannot use replace refs
  Remove obsolete script to convert grafts to replace refs

 Documentation/git-filter-branch.txt   |   2 +-
 Documentation/git-replace.txt |  11 +-
 Documentation/technical/shallow.txt   |  20 +-
 advice.c  |   2 +
 advice.h  |   1 +
 argv-array.c  |  20 ++
 argv-array.h  |   2 +
 builtin/replace.c | 218 +++---
 commit.c  |  18 +-
 commit.h  |   4 +-
 contrib/convert-grafts-to-replace-refs.sh |  28 ---
 log-tree.c|  13 +-
 t/t6001-rev-list-graft.sh |   9 +
 t/t6050-replace.sh|  20 ++
 14 files changed, 253 insertions(+), 115 deletions(-)
 delete mode 100755 contrib/convert-grafts-to-replace-refs.sh


base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v5
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v5

Interdiff vs v4:
 diff --git a/Documentation/technical/shallow.txt 
b/Documentation/technical/shallow.txt
 index cb79181c2bb..01dedfe9ffe 100644
 --- a/Documentation/technical/shallow.txt
 +++ b/Documentation/technical/shallow.txt
 @@ -8,18 +8,10 @@ repo, and therefore grafts are introduced pretending that
  these commits have no parents.
  *
  
 -The basic idea is to write the SHA-1s of shallow commits into
 -$GIT_DIR/shallow, and handle its contents similar to replace
 -refs (with the difference that shallow does not actually
 -create those replace refs) and very much like the deprecated
 -graft file (with the difference that shallow commits will
 -always have their parents grafted away, not replaced by
 -different parents).
 -
 -This information is stored in a special-purpose file because the
 -user should not touch that file at all (even throughout
 -development of the shallow clone, it was never manually
 -edited!).
 +$GIT_DIR/shallow lists commit object names and tells Git to
 +pretend as if they are root commits (e.g. "git log" traversal
 +stops after showing them; "git fsck" does not complain saying
 +the commits listed on their "parent" lines do not exist).
  
  Each line contains exactly one SHA-1. When read, a commit_graft
  will be constructed, which has nr_parent < 0 to make it easier
 diff --git a/builtin/replace.c b/builtin/replace.c
 index 6b3e0301e90..acd30e3d824 100644
 --- a/builtin/replace.c
 +++ b/builtin/replace.c
 @@ -175,8 +175,10 @@ static int replace_object_oid(const char *object_ref,
 object_ref, type_name(obj_type),
 replace_ref, type_name(repl_type));
  
 -  if (check_ref_valid(object, , , force))
 +  if (check_ref_valid(object, , , force)) {
 +  strbuf_release();
return -1;
 +  }
  
transaction = ref_transaction_begin();
if (!transaction ||
 @@ -264,9 +266,10 @@ static int import_object(struct object_id *oid, enum 
object_type type,
}
  
if (strbuf_read(, cmd.out, 41) < 0) {
 +  error_errno("unable to read from mktree");
close(fd);
close(cmd.out);
 -  return error_errno("unable to read from mktree");
 +  return -1;
}
close(cmd.out);
  
 @@ -285,8 +288,9 @@ static int import_object(struct object_id *oid, enum 
object_type type,
int flags = H

Re: [PATCH 02/41] server-info: remove unused members from struct pack_info

[Martin Ågren]

On 24 April 2018 at 01:39, brian m. carlson
<sand...@crustytoothpaste.net> wrote:
> The head member of struct pack_info is completely unused and the
> nr_heads member is used only in one place, which is an assignment.
> Since these structure members are not useful, remove them.

Good catch.

> @@ -228,7 +226,6 @@ static void init_pack_info(const char *infofile, int 
> force)
> for (i = 0; i < num_pack; i++) {
> if (stale) {
> info[i]->old_num = -1;
> -   info[i]->nr_heads = 0;
> }
> }

Minor nits: The braces could go. Not something you're introducing, but
the nesting of the `for` and the `if` looks odd. There used to be more
inside this loop, which explains this.

Martin

[PATCH 02/41] server-info: remove unused members from struct pack_info

[brian m. carlson]

The head member of struct pack_info is completely unused and the
nr_heads member is used only in one place, which is an assignment.
Since these structure members are not useful, remove them.

Signed-off-by: brian m. carlson <sand...@crustytoothpaste.net>
---
 server-info.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/server-info.c b/server-info.c
index 83460ec0d6..828ec5e538 100644
--- a/server-info.c
+++ b/server-info.c
@@ -92,8 +92,6 @@ static struct pack_info {
int old_num;
int new_num;
int nr_alloc;
-   int nr_heads;
-   unsigned char (*head)[20];
 } **info;
 static int num_pack;
 static const char *objdir;
@@ -228,7 +226,6 @@ static void init_pack_info(const char *infofile, int force)
for (i = 0; i < num_pack; i++) {
if (stale) {
    info[i]->old_num = -1;
-   info[i]->nr_heads = 0;
}
}
 

Re: [PATCH v4 00/11] Deprecate .git/info/grafts

[Stefan Beller]

On Sat, Apr 21, 2018 at 2:43 AM, Johannes Schindelin
 wrote:
> It is fragile, as there is no way for the revision machinery to say "but
> now I want to traverse the graph ignoring the graft file" e.g. when
> pushing commits to a remote repository (which, as a consequence, can
> miss commits).
>
> And we already have a better solution with `git replace --graft 
> [...]`.
>

Apart from some technical questions on patch 4 [1]
this series looks good to me,.

Thanks,
Stefan

[1] 
https://public-inbox.org/git/CAGZ79ka=BLGCCTOw848m0SE9O+ZKhQfiW9RUz99W4=gdg+7...@mail.gmail.com/

Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Ramsay Jones]

On 22/04/18 17:12, Duy Nguyen wrote:
> On Sun, Apr 22, 2018 at 5:58 PM, Ramsay Jones
>  wrote:
 I think you need to try a little harder than this! ;-)
>>>
>>> Yeah. I did think about grepping the output but decided not to because
>>> of gettext poison stuff and column output in "git help". If we do want
>>> to test this, how about I extend --list-cmds= option to take a few
>>> more parameters? --list-cmds=common would output all common commands,
>>> --list-cmds= does the same for other command category. This
>>> way we can verify without worrying about text formatting, paging or
>>> translation.
>>
>> Hmm, my immediate reaction would be to prefer my simple tests.
>> Yes, they are not exactly rigorous and they would be affected
>> by changing the help formatting, but they are effective. ;-)
>>
>> [I don't think the formatting would change that often, or at
>> all - whoever submits that patch would get to update the tests!]
> 
> Hmm.. for non-column output that's true. "git help" with column output
> should probably fine as well because even though we add more and more
> commands every month, these are not marked common (and unlikely so).
> So yeah I agree.
> 
>> What did you think about adding the BUG() checks?
> 
> I was thinking if there was a way to fail the build after running
> ./generate-cmds.sh and generating empty output but it does not sound
> easy to do. So yeah, BUG() checks sound good.

Yeah, failing the build would be preferable, but I didn't get
that far. ;-)

[BTW, I just applied your patch series to the 'next' branch
(I just couldn't be bothered to revert your last series from
the 'pu' branch, etc.) and, as expected, everything built OK,
passed the test-suite and 'git help' is working just fine.]

Thanks!

ATB,
Ramsay Jones

Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Duy Nguyen]

On Sun, Apr 22, 2018 at 5:58 PM, Ramsay Jones
 wrote:
>>> I think you need to try a little harder than this! ;-)
>>
>> Yeah. I did think about grepping the output but decided not to because
>> of gettext poison stuff and column output in "git help". If we do want
>> to test this, how about I extend --list-cmds= option to take a few
>> more parameters? --list-cmds=common would output all common commands,
>> --list-cmds= does the same for other command category. This
>> way we can verify without worrying about text formatting, paging or
>> translation.
>
> Hmm, my immediate reaction would be to prefer my simple tests.
> Yes, they are not exactly rigorous and they would be affected
> by changing the help formatting, but they are effective. ;-)
>
> [I don't think the formatting would change that often, or at
> all - whoever submits that patch would get to update the tests!]

Hmm.. for non-column output that's true. "git help" with column output
should probably fine as well because even though we add more and more
commands every month, these are not marked common (and unlikely so).
So yeah I agree.

> What did you think about adding the BUG() checks?

I was thinking if there was a way to fail the build after running
./generate-cmds.sh and generating empty output but it does not sound
easy to do. So yeah, BUG() checks sound good.

-- 
Duy

Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Ramsay Jones]

On 22/04/18 16:22, Duy Nguyen wrote:
> On Sun, Apr 22, 2018 at 4:45 PM, Ramsay Jones
>  wrote:
>>
>>
>> On 21/04/18 17:56, Duy Nguyen wrote:
>>> On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
 Changes:

 - remove the deprecated column in command-list.txt. My change break it
   anyway if anyone uses it.
 - fix up failed tests that I marked in the RFC and kinda forgot about it.
 - fix bashisms in generate-cmdlist.sh
 - fix segfaul in "git help"
>>>
>>> Sorry I forgot the interdiff
>>>
>> [snip]
>>
>>> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
>>> index fd2a7f27dc..53208ab20e 100755
>>> --- a/t/t0012-help.sh
>>> +++ b/t/t0012-help.sh
>>> @@ -25,6 +25,15 @@ test_expect_success "setup" '
>>>   EOF
>>>  '
>>>
>>> +# make sure to exercise these code paths, the output is a bit tricky
>>> +# to verify
>>> +test_expect_success 'basic help commands' '
>>> + git help >/dev/null &&
>>> + git help -a >/dev/null &&
>>> + git help -g >/dev/null &&
>>> + git help -av >/dev/null
>>> +'
>>> +
>> I think you need to try a little harder than this! ;-)
> 
> Yeah. I did think about grepping the output but decided not to because
> of gettext poison stuff and column output in "git help". If we do want
> to test this, how about I extend --list-cmds= option to take a few
> more parameters? --list-cmds=common would output all common commands,
> --list-cmds= does the same for other command category. This
> way we can verify without worrying about text formatting, paging or
> translation.

Hmm, my immediate reaction would be to prefer my simple tests.
Yes, they are not exactly rigorous and they would be affected 
by changing the help formatting, but they are effective. ;-)

[I don't think the formatting would change that often, or at
all - whoever submits that patch would get to update the tests!]

What did you think about adding the BUG() checks?

ATB,
Ramsay Jones

Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Duy Nguyen]

On Sun, Apr 22, 2018 at 4:45 PM, Ramsay Jones
 wrote:
>
>
> On 21/04/18 17:56, Duy Nguyen wrote:
>> On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
>>> Changes:
>>>
>>> - remove the deprecated column in command-list.txt. My change break it
>>>   anyway if anyone uses it.
>>> - fix up failed tests that I marked in the RFC and kinda forgot about it.
>>> - fix bashisms in generate-cmdlist.sh
>>> - fix segfaul in "git help"
>>
>> Sorry I forgot the interdiff
>>
> [snip]
>
>> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
>> index fd2a7f27dc..53208ab20e 100755
>> --- a/t/t0012-help.sh
>> +++ b/t/t0012-help.sh
>> @@ -25,6 +25,15 @@ test_expect_success "setup" '
>>   EOF
>>  '
>>
>> +# make sure to exercise these code paths, the output is a bit tricky
>> +# to verify
>> +test_expect_success 'basic help commands' '
>> + git help >/dev/null &&
>> + git help -a >/dev/null &&
>> + git help -g >/dev/null &&
>> + git help -av >/dev/null
>> +'
>> +
> I think you need to try a little harder than this! ;-)

Yeah. I did think about grepping the output but decided not to because
of gettext poison stuff and column output in "git help". If we do want
to test this, how about I extend --list-cmds= option to take a few
more parameters? --list-cmds=common would output all common commands,
--list-cmds= does the same for other command category. This
way we can verify without worrying about text formatting, paging or
translation.
-- 
Duy

Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Ramsay Jones]

On 21/04/18 17:56, Duy Nguyen wrote:
> On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
>> Changes:
>>
>> - remove the deprecated column in command-list.txt. My change break it
>>   anyway if anyone uses it.
>> - fix up failed tests that I marked in the RFC and kinda forgot about it.
>> - fix bashisms in generate-cmdlist.sh
>> - fix segfaul in "git help"
> 
> Sorry I forgot the interdiff
> 
[snip]

> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
> index fd2a7f27dc..53208ab20e 100755
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -25,6 +25,15 @@ test_expect_success "setup" '
>   EOF
>  '
>  
> +# make sure to exercise these code paths, the output is a bit tricky
> +# to verify
> +test_expect_success 'basic help commands' '
> + git help >/dev/null &&
> + git help -a >/dev/null &&
> + git help -g >/dev/null &&
> + git help -av >/dev/null
> +'
> +
I think you need to try a little harder than this! ;-)

This test would not have noticed the recent failure (what annoyed me was
that git build without error and then the test-suite sailed through with
nary a squeak of complaint). Viz:

  $ ./git help
  usage: git [--version] [--help] [-C ] [-c =]
 [--exec-path[=]] [--html-path] [--man-path] [--info-path]
 [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]
 [--git-dir=] [--work-tree=] [--namespace=]
  []
  
  These are common Git commands used in various situations:
  
  'git help -a' and 'git help -g' list available subcommands and some
  concept guides. See 'git help ' or 'git help '
  to read about a specific subcommand or concept.
  $ echo $?
  0
  $ 

  $ ./git help -g
  The common Git guides are:
  
  
  'git help -a' and 'git help -g' list available subcommands and some
  concept guides. See 'git help ' or 'git help '
  to read about a specific subcommand or concept.
  $ echo $?
  0
  $ 

  $ ./git help -av
  Main Porcelain Commands
  
  
  Ancillary Commands / Manipulators
  
  
  Ancillary Commands / Interrogators
  
  
  Interacting with Others
  
  
  Low-level Commands / Manipulators
  
  
  Low-level Commands / Interrogators
  
  
  Low-level Commands / Synching Repositories
  
  
  Low-level Commands / Internal Helpers
  
  $ echo $?
  0
  $ 
 
I started to add some tests, like so:

  diff --git a/t/t0012-help.sh b/t/t0012-help.sh
  index fd2a7f27d..7e10c2862 100755
  --- a/t/t0012-help.sh
  +++ b/t/t0012-help.sh
  @@ -49,6 +49,22 @@ test_expect_success "--help does not work for guides" "
  test_i18ncmp expect actual
   "
   
  +test_expect_success 'git help' '
  +   git help >help.output &&
  +   test_i18ngrep "^   clone  " help.output &&
  +   test_i18ngrep "^   add" help.output &&
  +   test_i18ngrep "^   log" help.output &&
  +   test_i18ngrep "^   commit " help.output &&
  +   test_i18ngrep "^   fetch  " help.output
  +'
  +
  +test_expect_success 'git help -g' '
  +   git help -g >help.output &&
  +   test_i18ngrep "^   attributes " help.output &&
  +   test_i18ngrep "^   everyday   " help.output &&
  +   test_i18ngrep "^   tutorial   " help.output
  +'
  +
   test_expect_success 'generate builtin list' '
  git --list-cmds=builtins >builtins
   '

These tests, while not rigorous, did at least correctly catch the bad
build for me. I was about to add the obvious one for 'git help -av',
when I decided to catch the problem 'at source', viz:

  diff --git a/help.c b/help.c
  index a47f7e2c4..13790bb54 100644
  --- a/help.c
  +++ b/help.c
  @@ -196,6 +196,9 @@ static void extract_common_cmds(struct cmdname_help 
**p_common_cmds,
int i, nr = 0;
struct cmdname_help *common_cmds;
   
  + if (ARRAY_SIZE(command_list) == 0)
  + BUG("empty command_list");
  +
ALLOC_ARRAY(common_cmds, ARRAY_SIZE(command_list));
   
for (i = 0; i < ARRAY_SIZE(command_list); i++) {
  @@ -279,6 +282,9 @@ void list_porcelain_cmds(void)
int i, nr = ARRAY_SIZE(command_list);
struct cmdname_help *cmds = command_list;
   
  + if (nr == 0)
  + BUG("empty command_list");
  +
for (i = 0; i < nr; i++) {
if (cmds[i].category != CAT_mainporcelain)
continue;
  @@ -321,6 +327,9 @@ void list_common_guides_help(void)
int nr = ARRAY_SIZE(command_list);
struct cmdname_help *cmds = command_list;
   
  + if (nr == 0)
  + BUG("empty command_list");
  +
QSORT(cmds, nr, cmd_category_cmp);
   
for (i = 0; i < nr; i++) {
  @@ -343,6 +352,9

Re: [PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Duy Nguyen]

On Sat, Apr 21, 2018 at 06:54:08PM +0200, Nguyễn Thái Ngọc Duy wrote:
> Changes:
> 
> - remove the deprecated column in command-list.txt. My change break it
>   anyway if anyone uses it.
> - fix up failed tests that I marked in the RFC and kinda forgot about it.
> - fix bashisms in generate-cmdlist.sh
> - fix segfaul in "git help"

Sorry I forgot the interdiff

diff --git a/command-list.txt b/command-list.txt
index 0809a19184..1835f1a928 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -9,7 +9,7 @@ history  grow, mark and tweak your common history
 remote   collaborate (see also: git help workflows)
 
 ### command list (do not change this line)
-# command name  category [deprecated] [common]
+# command name  category[common]
 git-add mainporcelain   worktree
 git-am  mainporcelain
 git-annotateancillaryinterrogators
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 9f17703aa7..7d17ca23f6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -835,19 +835,23 @@ __git_complete_strategy ()
 }
 
 __git_commands () {
-   if test -n "${GIT_TESTING_COMMAND_COMPLETION:-}"
+   if test -n "$GIT_TESTING_COMPLETION"
then
-   printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}"
+   case "$1" in
+   porcelain)
+   printf "%s" "$GIT_TESTING_PORCELAIN_COMMAND_LIST";;
+   all)
+   printf "%s" "$GIT_TESTING_ALL_COMMAND_LIST";;
+   esac
else
git --list-cmds=$1
fi
 }
 
-__git_list_all_commands ()
+__git_list_commands ()
 {
local i IFS=" "$'\n'
-   local category=${1-all}
-   for i in $(__git_commands $category)
+   for i in $(__git_commands $1)
do
case $i in
*--*) : helper pattern;;
@@ -860,14 +864,14 @@ __git_all_commands=
 __git_compute_all_commands ()
 {
test -n "$__git_all_commands" ||
-   __git_all_commands=$(__git_list_all_commands)
+   __git_all_commands=$(__git_list_commands all)
 }
 
 __git_porcelain_commands=
 __git_compute_porcelain_commands ()
 {
test -n "$__git_porcelain_commands" ||
-   __git_porcelain_commands=$(__git_list_all_commands porcelain)
+   __git_porcelain_commands=$(__git_list_commands porcelain)
 }
 
 # Lists all set config variables starting with the given section prefix,
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index e35f3e357b..86d59419b3 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -36,7 +36,7 @@ sed -n '
' "$1"
 printf '};\n\n'
 
-echo "#define GROUP_NONE 0xff /* no common group */"
+echo "#define GROUP_NONE 0xff"
 n=0
 while read grp
 do
@@ -45,15 +45,6 @@ do
 done <"$grps"
 echo
 
-echo '/*'
-printf 'static const char *cmd_categories[] = {\n'
-category_list "$1" |
-while read category; do
-   printf '\t\"'$category'\",\n'
-done
-printf '\tNULL\n};\n\n'
-echo '*/'
-
 n=0
 category_list "$1" |
 while read category; do
@@ -68,10 +59,11 @@ sort |
 while read cmd category tags
 do
if [ "$category" = guide ]; then
-   name=${cmd/git}
+   prefix=git
else
-   name=${cmd/git-}
+   prefix=git-
fi
+   name=$(echo $cmd | sed "s/^${prefix}//")
sed -n '
/^NAME/,/'"$cmd"'/H
${
diff --git a/help.c b/help.c
index a44f4a113e..88127fdd6f 100644
--- a/help.c
+++ b/help.c
@@ -201,7 +201,8 @@ static void extract_common_cmds(struct cmdname_help 
**p_common_cmds,
for (i = 0; i < ARRAY_SIZE(command_list); i++) {
const struct cmdname_help *cmd = command_list + i;
 
-   if (cmd->category != CAT_mainporcelain)
+   if (cmd->category != CAT_mainporcelain ||
+   cmd->group == GROUP_NONE)
continue;
 
common_cmds[nr++] = *cmd;
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index fd2a7f27dc..53208ab20e 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -25,6 +25,15 @@ test_expect_success "setup" '
EOF
 '
 
+# make sure to exercise these code paths, the output is a bit tricky
+# to verify
+test_expect_success 'basic help commands' '
+   git help >/dev/null &&
+   git help -a >/dev/null &&
+   git help -g >/dev/null &&
+   git help -av >/dev/null
+'
+
 test_expect_success "works for commands and guides by default" '
configure_help &&
git help status &&
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4bfd26ddf9..5a23a46fcf 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -13,7 +13,7 @@ complete ()
return 0
 }
 
-# Be careful when updating this list:
+# Be careful when 

[PATCH v3 0/6] Keep all info in command-list.txt in git binary

[Nguyễn Thái Ngọc Duy]

Changes:

- remove the deprecated column in command-list.txt. My change break it
  anyway if anyone uses it.
- fix up failed tests that I marked in the RFC and kinda forgot about it.
- fix bashisms in generate-cmdlist.sh
- fix segfaul in "git help"

Nguyễn Thái Ngọc Duy (6):
  git.c: convert --list-*builtins to --list-cmds=*
  git.c: implement --list-cmds=all and use it in git-completion.bash
  generate-cmdlist.sh: keep all information in common-cmds.h
  git.c: implement --list-cmds=porcelain
  help: add "-a --verbose" to list all commands with synopsis
  help: use command-list.txt for the source of guides

 Documentation/git-help.txt |   4 +-
 Documentation/gitattributes.txt|   2 +-
 Documentation/gitmodules.txt   |   2 +-
 Documentation/gitrevisions.txt |   2 +-
 builtin/help.c |  39 ++
 command-list.txt   |  10 +-
 contrib/completion/git-completion.bash | 108 ++--
 generate-cmdlist.sh|  57 ++---
 git.c  |  16 ++-
 help.c | 164 -
 help.h |   4 +
 t/t0012-help.sh|  11 +-
 t/t9902-completion.sh  |   6 +-
 13 files changed, 263 insertions(+), 162 deletions(-)

-- 
2.17.0.367.g5dd2e386c3

[PATCH v4 07/11] Deprecate support for .git/info/grafts

[Johannes Schindelin]

The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.

Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.

For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.

Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.

The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.

Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Reviewed-by: Stefan Beller <sbel...@google.com>
---
 advice.c  |  2 ++
 advice.h  |  1 +
 commit.c  | 10 ++
 t/t6001-rev-list-graft.sh |  9 +
 4 files changed, 22 insertions(+)

diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
 
 static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", _add_embedded_repo },
{ "ignoredhook", _ignored_hook },
{ "waitingforeditor", _waiting_for_editor },
+   { "graftfiledeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", _push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index 2952ec987c5..451d3ce8dfe 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
 #include "prio-queue.h"
 #include "sha1-lookup.h"
 #include "wt-status.h"
+#include "advice.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+   if (advice_graft_file_deprecated)
+   advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
 
 done
+
+test_expect_success 'show advice that grafts are deprecated' '
+   git show HEAD 2>err &&
+   test_i18ngrep "git replace" err &&
+   test_config advice.graftFileDeprecated false &&
+   git show HEAD 2>err &&
+   test_i18ngrep ! "git replace" err
+'
+
 test_done
-- 
2.17.0.windows.1.15.gaa56ade3205

[PATCH v4 00/11] Deprecate .git/info/grafts

[Johannes Schindelin]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft 
[...]`.

Changes since v3:

- The argv_array_split() declaration now has a clear comment indicating
  that it does not perform any "un-quoting" but goes purely by
  whitespace.

- Fixed t6050 under GETTEXT_POISON.


Johannes Schindelin (11):
  argv_array: offer to split a string by whitespace
  commit: Let the callback of for_each_mergetag return on error
  replace: avoid using die() to indicate a bug
  replace: "libify" create_graft() and callees
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: describe the relationship with replace refs
  technical/shallow: describe why shallow cannot use replace refs
  Remove obsolete script to convert grafts to replace refs

 Documentation/git-filter-branch.txt   |   2 +-
 Documentation/git-replace.txt |  11 +-
 Documentation/technical/shallow.txt   |  24 ++-
 advice.c  |   2 +
 advice.h  |   1 +
 argv-array.c  |  20 +++
 argv-array.h  |   2 +
 builtin/replace.c | 189 +++---
 commit.c  |  18 ++-
 commit.h  |   4 +-
 contrib/convert-grafts-to-replace-refs.sh |  28 
 log-tree.c|  13 +-
 t/t6001-rev-list-graft.sh |   9 ++
 t/t6050-replace.sh|  20 +++
 14 files changed, 236 insertions(+), 107 deletions(-)
 delete mode 100755 contrib/convert-grafts-to-replace-refs.sh


base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v4
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v4

Interdiff vs v3:
 diff --git a/argv-array.h b/argv-array.h
 index c7c397695df..750c30d2f2c 100644
 --- a/argv-array.h
 +++ b/argv-array.h
 @@ -19,6 +19,7 @@ LAST_ARG_MUST_BE_NULL
  void argv_array_pushl(struct argv_array *, ...);
  void argv_array_pushv(struct argv_array *, const char **);
  void argv_array_pop(struct argv_array *);
 +/* Splits by whitespace; does not handle quoted arguments! */
  void argv_array_split(struct argv_array *, const char *);
  void argv_array_clear(struct argv_array *);
  const char **argv_array_detach(struct argv_array *);
 diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
 index 8a3ee7c3db9..bed86a0af3d 100755
 --- a/t/t6050-replace.sh
 +++ b/t/t6050-replace.sh
 @@ -460,8 +460,8 @@ test_expect_success '--convert-graft-file' '
test_when_finished "rm -f .git/info/grafts" &&
echo $EMPTY_BLOB $EMPTY_TREE >.git/info/grafts &&
test_must_fail git replace --convert-graft-file 2>err &&
 -  grep "$EMPTY_BLOB $EMPTY_TREE" err &&
 -  grep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts
 +  test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" err &&
 +  test_i18ngrep "$EMPTY_BLOB $EMPTY_TREE" .git/info/grafts
  '
  
  test_done
-- 
2.17.0.windows.1.15.gaa56ade3205

[PATCH v3 07/11] Deprecate support for .git/info/grafts

[Johannes Schindelin]

The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.

Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.

For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.

Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.

The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.

Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Reviewed-by: Stefan Beller <sbel...@google.com>
---
 advice.c  |  2 ++
 advice.h  |  1 +
 commit.c  | 10 ++
 t/t6001-rev-list-graft.sh |  9 +
 4 files changed, 22 insertions(+)

diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
 
 static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", _add_embedded_repo },
{ "ignoredhook", _ignored_hook },
{ "waitingforeditor", _waiting_for_editor },
+   { "graftfiledeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", _push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index 2952ec987c5..451d3ce8dfe 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
 #include "prio-queue.h"
 #include "sha1-lookup.h"
 #include "wt-status.h"
+#include "advice.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+   if (advice_graft_file_deprecated)
+   advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
 
 done
+
+test_expect_success 'show advice that grafts are deprecated' '
+   git show HEAD 2>err &&
+   test_i18ngrep "git replace" err &&
+   test_config advice.graftFileDeprecated false &&
+   git show HEAD 2>err &&
+   test_i18ngrep ! "git replace" err
+'
+
 test_done
-- 
2.17.0.windows.1.15.gaa56ade3205

[PATCH v3 00/11] Deprecate .git/info/grafts

[Johannes Schindelin]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft 
[...]`.

Changes since v2:

- Fixed tyop in the description of --graft where it suggests
  --convert-graft-file instead.

- Dropped the rant from the "libify create_graft()" commit.

- Changed a die("BUG: ...") call to BUG() (instead of libifying it).

- Truly libified create_graft() by replacing every single instance where die()
  (or lookup_commit_or_die()) was called with a proper error(), adding resource
  handling to avoid leaks, so that that rant that was dropped from the commit
  message would now truly be merited.

- Graft files with commented lines are now handled correctly.

- Graft files with empty lines are now handled correctly, too.

- The graft file parser is now based on a shiny new argv_array_split().

- The script in contrib/ whose functionality is superseded by `git replace
  --convert-graft-file` was removed.


Johannes Schindelin (11):
  argv_array: offer to split a string by whitespace
  commit: Let the callback of for_each_mergetag return on error
  replace: avoid using die() to indicate a bug
  replace: "libify" create_graft() and callees
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: describe the relationship with replace refs
  technical/shallow: describe why shallow cannot use replace refs
  Remove obsolete script to convert grafts to replace refs

 Documentation/git-filter-branch.txt   |   2 +-
 Documentation/git-replace.txt |  11 +-
 Documentation/technical/shallow.txt   |  24 ++-
 advice.c  |   2 +
 advice.h  |   1 +
 argv-array.c  |  20 +++
 argv-array.h  |   1 +
 builtin/replace.c | 189 +++---
 commit.c  |  18 ++-
 commit.h  |   4 +-
 contrib/convert-grafts-to-replace-refs.sh |  28 
 log-tree.c|  13 +-
 t/t6001-rev-list-graft.sh |   9 ++
 t/t6050-replace.sh|  20 +++
 14 files changed, 235 insertions(+), 107 deletions(-)
 delete mode 100755 contrib/convert-grafts-to-replace-refs.sh


base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v3
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v3

Interdiff vs v2:
 diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
 index 4dc0686f7d6..246dc9943c2 100644
 --- a/Documentation/git-replace.txt
 +++ b/Documentation/git-replace.txt
 @@ -89,7 +89,7 @@ OPTIONS
[...] instead of 's parents. A replacement ref
is then created to replace  with the newly created
commit. Use `--convert-graft-file` to convert a
 -  `$GIT_DIR/info/grafts` file use replace refs instead.
 +  `$GIT_DIR/info/grafts` file and use replace refs instead.
  
  --convert-graft-file::
Creates graft commits for all entries in `$GIT_DIR/info/grafts`
 diff --git a/argv-array.c b/argv-array.c
 index 5d370fa3366..cb5bcd2c064 100644
 --- a/argv-array.c
 +++ b/argv-array.c
 @@ -64,6 +64,26 @@ void argv_array_pop(struct argv_array *array)
array->argc--;
  }
  
 +void argv_array_split(struct argv_array *array, const char *to_split)
 +{
 +  while (isspace(*to_split))
 +  to_split++;
 +  for (;;) {
 +  const char *p = to_split;
 +
 +  if (!*p)
 +  break;
 +
 +  while (*p && !isspace(*p))
 +  p++;
 +  argv_array_push_nodup(array, xstrndup(to_split, p - to_split));
 +
 +  while (isspace(*p))
 +  p++;
 +  to_split = p;
 +  }
 +}
 +
  void argv_array_clear(struct argv_array *array)
  {
if (array->argv != empty_argv) {
 diff --git a/argv-array.h b/argv-array.h
 index 29056e49a12..c7c397695df 100644
 --- a/argv-array.h
 +++ b/argv-array.h
 @@ -19,6 +19,7 @@ LAST_ARG_MUST_BE_NULL
  void argv_array_pushl(struct argv_array *, ...);
  void argv_array_pushv(struct argv_array *, const char **);
  void argv_array_pop(struct argv_array *);
 +void argv_array_split(struct argv_array *, const char *);
  void argv_array_clear(struct argv_array *);
  const char **argv_array_detach(struct argv_array *);
  
 diff --git a/builtin/replace.c b/builtin/replace.c
 index 4cdc00a96df..6b3e0301e90 100644
 --- a/builtin/replace.c
 +++ b/builtin/replace.c
 @@ -80,9 +80,9 @@ static

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Simon Ruderich]

On Thu, Apr 19, 2018 at 01:26:18PM +0200, SZEDER Gábor wrote:
> On Thu, Apr 19, 2018 at 12:37 PM, Simon Ruderich wrote:
>> This doesn't occur on a non-parallel build.
>
> It does occur in non-parallel builds, too.
>
> See:
>
>   
> https://public-inbox.org/git/cam0vkjkns+asvymse2fxzt8a8oqydnx3qo8mnw2juogfc7l...@mail.gmail.com/

Thanks for the correction. I noticed it at the beginning of make
-j run and incorrectly assummed it would occur quickly in the
non-parallel run as well.

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

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[SZEDER Gábor]

On Thu, Apr 19, 2018 at 12:37 PM, Simon Ruderich  wrote:
> When running make -j$(nproc) with the current pu f9f8c1f765
> (Merge branch 'hn/bisect-first-parent' into pu) I see the
> following error:
>
> GIT_VERSION = 2.17.0.732.gf9f8c1f765
> * new build flags
> * new prefix flags
> GEN common-cmds.h
> * new link flags
> CC ident.o
> CC hex.o
> CC json-writer.o
> ./generate-cmdlist.sh: 73: ./generate-cmdlist.sh: Bad substitution
>
> This doesn't occur on a non-parallel build.

It does occur in non-parallel builds, too.

See:

  
https://public-inbox.org/git/cam0vkjkns+asvymse2fxzt8a8oqydnx3qo8mnw2juogfc7l...@mail.gmail.com/

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Simon Ruderich]

Hello,

When running make -j$(nproc) with the current pu f9f8c1f765
(Merge branch 'hn/bisect-first-parent' into pu) I see the
following error:

GIT_VERSION = 2.17.0.732.gf9f8c1f765
* new build flags
* new prefix flags
GEN common-cmds.h
* new link flags
CC ident.o
CC hex.o
CC json-writer.o
./generate-cmdlist.sh: 73: ./generate-cmdlist.sh: Bad substitution

This doesn't occur on a non-parallel build.

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

[PATCH v2 4/7] Deprecate support for .git/info/grafts

[Johannes Schindelin]

The grafts feature was a convenient way to "stitch together" ancient
history to the fresh start of linux.git.

Its implementation is, however, not up to Git's standards, as there are
too many ways where it can lead to surprising and unwelcome behavior.

For example, when pushing from a repository with active grafts, it is
possible to miss commits that have been "grafted out", resulting in a
broken state on the other side.

Also, the grafts feature is limited to "rewriting" commits' list of
parents, it cannot replace anything else.

The much younger feature implemented as `git replace` set out to remedy
those limitations and dangerous bugs.

Seeing as `git replace` is pretty mature by now (since 4228e8bc98
(replace: add --graft option, 2014-07-19) it can perform the graft
file's duties), it is time to deprecate support for the graft file, and
to retire it eventually.

Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
Reviewed-by: Stefan Beller <sbel...@google.com>
---
 advice.c  |  2 ++
 advice.h  |  1 +
 commit.c  | 10 ++
 t/t6001-rev-list-graft.sh |  9 +
 4 files changed, 22 insertions(+)

diff --git a/advice.c b/advice.c
index 406efc183ba..4411704fd45 100644
--- a/advice.c
+++ b/advice.c
@@ -19,6 +19,7 @@ int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
+int advice_graft_file_deprecated = 1;
 
 static struct {
const char *name;
@@ -42,6 +43,7 @@ static struct {
{ "addembeddedrepo", _add_embedded_repo },
{ "ignoredhook", _ignored_hook },
{ "waitingforeditor", _waiting_for_editor },
+   { "graftfiledeprecated", _graft_file_deprecated },
 
/* make this an alias for backward compatibility */
{ "pushnonfastforward", _push_update_rejected }
diff --git a/advice.h b/advice.h
index 70568fa7922..9f5064e82a8 100644
--- a/advice.h
+++ b/advice.h
@@ -21,6 +21,7 @@ extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
+extern int advice_graft_file_deprecated;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/commit.c b/commit.c
index ca474a7c112..1a5e8777617 100644
--- a/commit.c
+++ b/commit.c
@@ -12,6 +12,7 @@
 #include "prio-queue.h"
 #include "sha1-lookup.h"
 #include "wt-status.h"
+#include "advice.h"
 
 static struct commit_extra_header *read_commit_extra_header_lines(const char 
*buf, size_t len, const char **);
 
@@ -176,6 +177,15 @@ static int read_graft_file(const char *graft_file)
struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
+   if (advice_graft_file_deprecated)
+   advise(_("Support for /info/grafts is deprecated\n"
+"and will be removed in a future Git version.\n"
+"\n"
+"Please use \"git replace --convert-graft-file\"\n"
+"to convert the grafts into replace refs.\n"
+"\n"
+"Turn this message off by running\n"
+"\"git config advice.graftFileDeprecated false\""));
while (!strbuf_getwholeline(, fp, '\n')) {
/* The format is just "Commit Parent1 Parent2 ...\n" */
struct commit_graft *graft = read_graft_line();
diff --git a/t/t6001-rev-list-graft.sh b/t/t6001-rev-list-graft.sh
index 05ddc69cf2a..7504ba47511 100755
--- a/t/t6001-rev-list-graft.sh
+++ b/t/t6001-rev-list-graft.sh
@@ -110,4 +110,13 @@ do
"
 
 done
+
+test_expect_success 'show advice that grafts are deprecated' '
+   git show HEAD 2>err &&
+   test_i18ngrep "git replace" err &&
+   test_config advice.graftFileDeprecated false &&
+   git show HEAD 2>err &&
+   test_i18ngrep ! "git replace" err
+'
+
 test_done
-- 
2.17.0.windows.1.4.g7e4058d72e3

[PATCH v2 0/7] Deprecate .git/info/grafts

[Johannes Schindelin]

It is fragile, as there is no way for the revision machinery to say "but
now I want to traverse the graph ignoring the graft file" e.g. when
pushing commits to a remote repository (which, as a consequence, can
miss commits).

And we already have a better solution with `git replace --graft 
[...]`.

Changes since v1:

- Fixed typo (stich -> stitch).

- Added reference in the commit message to the patch where `git replace`
  reached feature parity with the graft file.

- Added the --convert-graft-file option to `git replace` (with tests).

- Adjusted the advice to suggest --convert-graft-file instead.

- Adjusted the documentation of filter-branch and technical/shallow to
  reflect the fact that grafts are deprecated.


Johannes Schindelin (7):
  replace: "libify" create_graft()
  replace: introduce --convert-graft-file
  Add a test for `git replace --convert-graft-file`
  Deprecate support for .git/info/grafts
  filter-branch: stop suggesting to use grafts
  technical/shallow: describe the relationship with replace refs
  technical/shallow: describe why shallow cannot use replace refs

 Documentation/git-filter-branch.txt |  2 +-
 Documentation/git-replace.txt   | 11 +++--
 Documentation/technical/shallow.txt | 24 +++
 advice.c|  2 +
 advice.h|  1 +
 builtin/replace.c   | 63 -
 commit.c| 10 +
 t/t6001-rev-list-graft.sh   |  9 +
 t/t6050-replace.sh  | 20 +
 9 files changed, 129 insertions(+), 13 deletions(-)


base-commit: fe0a9eaf31dd0c349ae4308498c33a5c3794b293
Published-As: https://github.com/dscho/git/releases/tag/deprecate-grafts-v2
Fetch-It-Via: git fetch https://github.com/dscho/git deprecate-grafts-v2

Interdiff vs v1:
 diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
 index b634043183b..1d4d2f86045 100644
 --- a/Documentation/git-filter-branch.txt
 +++ b/Documentation/git-filter-branch.txt
 @@ -288,7 +288,7 @@ git filter-branch --parent-filter \
  or even simpler:
  
  ---
 -echo "$commit-id $graft-id" >> .git/info/grafts
 +git replace --graft $commit-id $graft-id
  git filter-branch $graft-id..HEAD
  ---
  
 diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
 index e5c57ae6ef4..4dc0686f7d6 100644
 --- a/Documentation/git-replace.txt
 +++ b/Documentation/git-replace.txt
 @@ -11,6 +11,7 @@ SYNOPSIS
  'git replace' [-f]  
  'git replace' [-f] --edit 
  'git replace' [-f] --graft  [...]
 +'git replace' [-f] --convert-graft-file
  'git replace' -d ...
  'git replace' [--format=] [-l []]
  
 @@ -87,9 +88,13 @@ OPTIONS
content as  except that its parents will be
[...] instead of 's parents. A replacement ref
is then created to replace  with the newly created
 -  commit. See contrib/convert-grafts-to-replace-refs.sh for an
 -  example script based on this option that can convert grafts to
 -  replace refs.
 +  commit. Use `--convert-graft-file` to convert a
 +  `$GIT_DIR/info/grafts` file use replace refs instead.
 +
 +--convert-graft-file::
 +  Creates graft commits for all entries in `$GIT_DIR/info/grafts`
 +  and deletes that file upon success. The purpose is to help users
 +  with transitioning off of the now-deprecated graft file.
  
  -l ::
  --list ::
 diff --git a/Documentation/technical/shallow.txt 
b/Documentation/technical/shallow.txt
 index 5183b154229..cb79181c2bb 100644
 --- a/Documentation/technical/shallow.txt
 +++ b/Documentation/technical/shallow.txt
 @@ -9,19 +9,29 @@ these commits have no parents.
  *
  
  The basic idea is to write the SHA-1s of shallow commits into
 -$GIT_DIR/shallow, and handle its contents like the contents
 -of $GIT_DIR/info/grafts (with the difference that shallow
 -cannot contain parent information).
 +$GIT_DIR/shallow, and handle its contents similar to replace
 +refs (with the difference that shallow does not actually
 +create those replace refs) and very much like the deprecated
 +graft file (with the difference that shallow commits will
 +always have their parents grafted away, not replaced by
 +different parents).
  
 -This information is stored in a new file instead of grafts, or
 -even the config, since the user should not touch that file
 -at all (even throughout development of the shallow clone, it
 -was never manually edited!).
 +This information is stored in a special-purpose file because the
 +user should not touch that file at all (even throughout
 +development of the shallow clone, it was never manually
 +edited!).
  
  Each line contains exactly one SHA-1. When read, a commit_graft
  will be constructed, which has nr_parent < 0 to make it easier
  to discern from user p

Re: [PATCH] Deprecate support for .git/info/grafts

[Johannes Schindelin]

Hi Stefan,

On Fri, 13 Apr 2018, Stefan Beller wrote:

> On Fri, Apr 13, 2018 at 3:35 PM, Johannes Schindelin
>  wrote:
> 
> >> I wonder if we want to offer a migration tool or just leave it at
> >> this hint.
> >
> > There is contrib/convert-grafts-to-replace-refs.sh.
> 
> Oh cool! I wonder if we want to expose it more such that people
> discover it.

Nah.

> > I wonder whether we have to care enough to implement a `git replace
> > --convert-graft-file`...
> 
> I don't think so.

After reflecting about this in the back of my mind, I actually do. So I
implemented it.

Ciao,
Dscho

Re: [PATCH] Deprecate support for .git/info/grafts

[Johannes Schindelin]

Hi Eric,

On Fri, 13 Apr 2018, Eric Sunshine wrote:

> On Fri, Apr 13, 2018 at 7:11 AM, Johannes Schindelin
> <johannes.schinde...@gmx.de> wrote:
> > The grafts feature was a convenient way to "stich together" ancient
> > history to the fresh start of linux.git.
> > [...]
> > The much younger feature implemented as `git replace` set out to remedy
> > those limitations and dangerous bugs.
> >
> > Seeing as `git replace` is pretty mature by now, it is time to deprecate
> > support for the graft file, and to retire it eventually.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
> > ---
> >  advice.c  | 2 ++
> >  advice.h  | 1 +
> >  commit.c  | 9 +
> >  t/t6001-rev-list-graft.sh | 9 +
> >  4 files changed, 21 insertions(+)
> 
> Perhaps, as part of this deprecation, the example in
> Documentation/git-filter-branch.txt should be updated to suggest
> git-replace instead of .git/info/grafts.

Good point!

> Maybe, also, Documentation/shallow.txt should talk about replace-refs
> rather than .git/info/grafts.

Sure. Internally, of course, "shallow" is still handled very much like the
graft file. In that sense, it is probably a good thing to keep referring
to the graft file in technical/shallow *in addition* to replace refs.

The reason why the graft file should be deprecated does not apply to
the shallow file at all, either: the entire set of problems with grafts
comes from the fact that you can *replace* the parents *with other
parents*. That allows for pushing corrupt history to public repositories
IIRC. The same problems do not arise with the shallow feature, where you
can *cut* history, but not replace it.

There is a notable difference between shallow commits and replace refs
which I did not think about earlier (it actually only came up in testing):
we currently disallow completely to replace merge commits when they
contain mergetags, i.e. entries in the commit header that record the hash
of the *tag* that was merged (if any). That includes the case where you
want to replace the commit with a root commit, as would be needed for
shallow commits.

However, there are more reasons not to conflate the shallow commits with
replaced commits: by nature, the "shallow" attribute is a lot more
volatile than the "replaced" one, as we want to keep it easy to deepen the
history (or to "unshallow" it).

Ciao,
Dscho

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Philip Oakley]

From: "Duy Nguyen" 

On Wed, Apr 18, 2018 at 12:47 AM, Philip Oakley 
wrote:

> Is that something I should add to my todo to add a 'guide' category >
> etc.?

I added it too [1]. Not sure if you want anything more on top though.



What I've seen is looking good - I've not had as much time as I'd like..

I'm not sure of the status of the git/generate-cmdlist.sh though. Should
that also be updated, or did I miss that?


Yes it's updated by other patches in the same thread.
--

Thanks. Hopefully I'll have some time this weekend/coming week as my wife is
away
Philip

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Duy Nguyen]

On Wed, Apr 18, 2018 at 12:47 AM, Philip Oakley  wrote:
>>> > Is that something I should add to my todo to add a 'guide' category >
>>> > etc.?
>>>
>>> I added it too [1]. Not sure if you want anything more on top though.
>
>
> What I've seen is looking good - I've not had as much time as I'd like..
>
> I'm not sure of the status of the git/generate-cmdlist.sh though. Should
> that also be updated, or did I miss that?

Yes it's updated by other patches in the same thread.
-- 
Duy

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Philip Oakley]

From: "Philip Oakley"  : Tuesday, April 17, 2018 11:47
PM

From: "Duy Nguyen"  : Tuesday, April 17, 2018 5:48 PM

On Tue, Apr 17, 2018 at 06:24:41PM +0200, Duy Nguyen wrote:

On Sun, Apr 15, 2018 at 11:21 PM, Philip Oakley 
wrote:
> From: "Duy Nguyen"  : Saturday, April 14, 2018 4:44
> PM
>
>> On Thu, Apr 12, 2018 at 12:06 AM, Philip Oakley
>> 
>> wrote:
>>>
>>> I'm only just catching up, but does/can this series also capture the
>>> non-command guides that are available in git so that the 'git
>>> help -g'
>>> can
>>> begin to list them all?
>>
>>
>> It currently does not. But I don't see why it should not. This should
>> allow git.txt to list all the guides too, for people who skip "git
>> help" and go hard core mode with "man git". Thanks for bringing this
>> up.
>> --
>> Duy
>>
> Is that something I should add to my todo to add a 'guide' category
> etc.?

I added it too [1]. Not sure if you want anything more on top though.


What I've seen is looking good - I've not had as much time as I'd like..

I'm not sure of the status of the git/generate-cmdlist.sh though. Should
that also be updated, or did I miss that?
--
Philip


I may be miss-remembering the order that the `git help` determines the list
of commands and guides. There was at least one place where the list of
commands was generated programatically that I may be confused with (I've not
had time to delve into the code :-(
--






The "anything more" that at least I had in mind was something like
this. Though I'm not sure if it's a good thing to replace a hand
crafted section with an automatedly generated one. This patch on top
combines the "SEE ALSO" and "FURTHER DOCUMENT" into one with most of
documents/guides are extracted from command-list.txt

-- 8< --
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 6232143cb9..3e0ecd2e11 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -292,6 +292,7 @@ doc.dep : $(docdep_prereqs) $(wildcard *.txt)
build-docdep.perl

cmds_txt = cmds-ancillaryinterrogators.txt \
 cmds-ancillarymanipulators.txt \
+ cmds-guide.txt \
 cmds-mainporcelain.txt \
 cmds-plumbinginterrogators.txt \
 cmds-plumbingmanipulators.txt \
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 5aa73cfe45..e158bd9b96 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -54,6 +54,7 @@ for (sort <>) {

for my $cat (qw(ancillaryinterrogators
 ancillarymanipulators
+ guide
 mainporcelain
 plumbinginterrogators
 plumbingmanipulators
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 4767860e72..d60d2ae0c7 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -808,29 +808,6 @@ The index is also capable of storing multiple
entries (called "stages")
for a given pathname.  These stages are used to hold the various
unmerged version of a file when a merge is in progress.

-FURTHER DOCUMENTATION
--
-
-See the references in the "description" section to get started
-using Git.  The following is probably more detail than necessary
-for a first-time user.
-
-The link:user-manual.html#git-concepts[Git concepts chapter of the
-user-manual] and linkgit:gitcore-tutorial[7] both provide
-introductions to the underlying Git architecture.
-
-See linkgit:gitworkflows[7] for an overview of recommended workflows.
-
-See also the link:howto-index.html[howto] documents for some useful
-examples.
-
-The internals are documented in the
-link:technical/api-index.html[Git API documentation].
-
-Users migrating from CVS may also want to
-read linkgit:gitcvs-migration[7].
-
-
Authors
---
Git was started by Linus Torvalds, and is currently maintained by Junio
@@ -854,11 +831,16 @@ the Git Security mailing list
.

SEE ALSO

-linkgit:gittutorial[7], linkgit:gittutorial-2[7],
-linkgit:giteveryday[7], linkgit:gitcvs-migration[7],
-linkgit:gitglossary[7], linkgit:gitcore-tutorial[7],
-linkgit:gitcli[7], link:user-manual.html[The Git User's Manual],
-linkgit:gitworkflows[7]
+
+See the references in the "description" section to get started
+using Git.  The following is probably more detail than necessary
+for a first-time user.
+
+include::cmds-guide.txt[]
+
+See also the link:howto-index.html[howto] documents for some useful
+examples. The internals are documented in the
+link:technical/api-index.html[Git API documentation].

GIT
---
diff --git a/command-list.txt b/command-list.txt
index 1835f1a928..f26b8acd52 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -150,10 +150,14 @@ git-whatchanged ancillaryinterrogators
git-worktreemainporcelain
git-write-tree  plumbingmanipulators
gitattributes   guide
+gitcvs-migrationguide
+gitcli  guide
+gitcore-tutorial  

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Philip Oakley]

From: "Duy Nguyen"  : Tuesday, April 17, 2018 5:48 PM

On Tue, Apr 17, 2018 at 06:24:41PM +0200, Duy Nguyen wrote:
On Sun, Apr 15, 2018 at 11:21 PM, Philip Oakley  
wrote:
> From: "Duy Nguyen"  : Saturday, April 14, 2018 4:44 
> PM

>
>> On Thu, Apr 12, 2018 at 12:06 AM, Philip Oakley 
>> wrote:
>>>
>>> I'm only just catching up, but does/can this series also capture the
>>> non-command guides that are available in git so that the 'git 
>>> help -g'

>>> can
>>> begin to list them all?
>>
>>
>> It currently does not. But I don't see why it should not. This should
>> allow git.txt to list all the guides too, for people who skip "git
>> help" and go hard core mode with "man git". Thanks for bringing this
>> up.
>> --
>> Duy
>>
> Is that something I should add to my todo to add a 'guide' category 
> etc.?


I added it too [1]. Not sure if you want anything more on top though.


What I've seen is looking good - I've not had as much time as I'd like..

I'm not sure of the status of the git/generate-cmdlist.sh though. Should 
that also be updated, or did I miss that?

--
Philip



The "anything more" that at least I had in mind was something like
this. Though I'm not sure if it's a good thing to replace a hand
crafted section with an automatedly generated one. This patch on top
combines the "SEE ALSO" and "FURTHER DOCUMENT" into one with most of
documents/guides are extracted from command-list.txt

-- 8< --
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 6232143cb9..3e0ecd2e11 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -292,6 +292,7 @@ doc.dep : $(docdep_prereqs) $(wildcard *.txt) 
build-docdep.perl


cmds_txt = cmds-ancillaryinterrogators.txt \
 cmds-ancillarymanipulators.txt \
+ cmds-guide.txt \
 cmds-mainporcelain.txt \
 cmds-plumbinginterrogators.txt \
 cmds-plumbingmanipulators.txt \
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 5aa73cfe45..e158bd9b96 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -54,6 +54,7 @@ for (sort <>) {

for my $cat (qw(ancillaryinterrogators
 ancillarymanipulators
+ guide
 mainporcelain
 plumbinginterrogators
 plumbingmanipulators
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 4767860e72..d60d2ae0c7 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -808,29 +808,6 @@ The index is also capable of storing multiple entries 
(called "stages")

for a given pathname.  These stages are used to hold the various
unmerged version of a file when a merge is in progress.

-FURTHER DOCUMENTATION
--
-
-See the references in the "description" section to get started
-using Git.  The following is probably more detail than necessary
-for a first-time user.
-
-The link:user-manual.html#git-concepts[Git concepts chapter of the
-user-manual] and linkgit:gitcore-tutorial[7] both provide
-introductions to the underlying Git architecture.
-
-See linkgit:gitworkflows[7] for an overview of recommended workflows.
-
-See also the link:howto-index.html[howto] documents for some useful
-examples.
-
-The internals are documented in the
-link:technical/api-index.html[Git API documentation].
-
-Users migrating from CVS may also want to
-read linkgit:gitcvs-migration[7].
-
-
Authors
---
Git was started by Linus Torvalds, and is currently maintained by Junio
@@ -854,11 +831,16 @@ the Git Security mailing list 
.


SEE ALSO

-linkgit:gittutorial[7], linkgit:gittutorial-2[7],
-linkgit:giteveryday[7], linkgit:gitcvs-migration[7],
-linkgit:gitglossary[7], linkgit:gitcore-tutorial[7],
-linkgit:gitcli[7], link:user-manual.html[The Git User's Manual],
-linkgit:gitworkflows[7]
+
+See the references in the "description" section to get started
+using Git.  The following is probably more detail than necessary
+for a first-time user.
+
+include::cmds-guide.txt[]
+
+See also the link:howto-index.html[howto] documents for some useful
+examples. The internals are documented in the
+link:technical/api-index.html[Git API documentation].

GIT
---
diff --git a/command-list.txt b/command-list.txt
index 1835f1a928..f26b8acd52 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -150,10 +150,14 @@ git-whatchanged 
ancillaryinterrogators

git-worktreemainporcelain
git-write-tree  plumbingmanipulators
gitattributes   guide
+gitcvs-migrationguide
+gitcli  guide
+gitcore-tutorialguide
giteveryday guide
gitglossary guide
gitignore   guide
gitmodules  guide
gitrevisionsguide
gittutorial guide
+gittutorial-2   guide

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Duy Nguyen]

On Tue, Apr 17, 2018 at 06:24:41PM +0200, Duy Nguyen wrote:
> On Sun, Apr 15, 2018 at 11:21 PM, Philip Oakley  wrote:
> > From: "Duy Nguyen"  : Saturday, April 14, 2018 4:44 PM
> >
> >> On Thu, Apr 12, 2018 at 12:06 AM, Philip Oakley 
> >> wrote:
> >>>
> >>> I'm only just catching up, but does/can this series also capture the
> >>> non-command guides that are available in git so that the 'git help -g'
> >>> can
> >>> begin to list them all?
> >>
> >>
> >> It currently does not. But I don't see why it should not. This should
> >> allow git.txt to list all the guides too, for people who skip "git
> >> help" and go hard core mode with "man git". Thanks for bringing this
> >> up.
> >> --
> >> Duy
> >>
> > Is that something I should add to my todo to add a 'guide' category etc.?
> 
> I added it too [1]. Not sure if you want anything more on top though.

The "anything more" that at least I had in mind was something like
this. Though I'm not sure if it's a good thing to replace a hand
crafted section with an automatedly generated one. This patch on top
combines the "SEE ALSO" and "FURTHER DOCUMENT" into one with most of
documents/guides are extracted from command-list.txt

-- 8< --
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 6232143cb9..3e0ecd2e11 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -292,6 +292,7 @@ doc.dep : $(docdep_prereqs) $(wildcard *.txt) 
build-docdep.perl
 
 cmds_txt = cmds-ancillaryinterrogators.txt \
cmds-ancillarymanipulators.txt \
+   cmds-guide.txt \
cmds-mainporcelain.txt \
cmds-plumbinginterrogators.txt \
cmds-plumbingmanipulators.txt \
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 5aa73cfe45..e158bd9b96 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -54,6 +54,7 @@ for (sort <>) {
 
 for my $cat (qw(ancillaryinterrogators
ancillarymanipulators
+   guide
mainporcelain
plumbinginterrogators
plumbingmanipulators
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 4767860e72..d60d2ae0c7 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -808,29 +808,6 @@ The index is also capable of storing multiple entries 
(called "stages")
 for a given pathname.  These stages are used to hold the various
 unmerged version of a file when a merge is in progress.
 
-FURTHER DOCUMENTATION
--
-
-See the references in the "description" section to get started
-using Git.  The following is probably more detail than necessary
-for a first-time user.
-
-The link:user-manual.html#git-concepts[Git concepts chapter of the
-user-manual] and linkgit:gitcore-tutorial[7] both provide
-introductions to the underlying Git architecture.
-
-See linkgit:gitworkflows[7] for an overview of recommended workflows.
-
-See also the link:howto-index.html[howto] documents for some useful
-examples.
-
-The internals are documented in the
-link:technical/api-index.html[Git API documentation].
-
-Users migrating from CVS may also want to
-read linkgit:gitcvs-migration[7].
-
-
 Authors
 ---
 Git was started by Linus Torvalds, and is currently maintained by Junio
@@ -854,11 +831,16 @@ the Git Security mailing list 
.
 
 SEE ALSO
 
-linkgit:gittutorial[7], linkgit:gittutorial-2[7],
-linkgit:giteveryday[7], linkgit:gitcvs-migration[7],
-linkgit:gitglossary[7], linkgit:gitcore-tutorial[7],
-linkgit:gitcli[7], link:user-manual.html[The Git User's Manual],
-linkgit:gitworkflows[7]
+
+See the references in the "description" section to get started
+using Git.  The following is probably more detail than necessary
+for a first-time user.
+
+include::cmds-guide.txt[]
+
+See also the link:howto-index.html[howto] documents for some useful
+examples. The internals are documented in the
+link:technical/api-index.html[Git API documentation].
 
 GIT
 ---
diff --git a/command-list.txt b/command-list.txt
index 1835f1a928..f26b8acd52 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -150,10 +150,14 @@ git-whatchanged 
ancillaryinterrogators
 git-worktreemainporcelain
 git-write-tree  plumbingmanipulators
 gitattributes   guide
+gitcvs-migrationguide
+gitcli  guide
+gitcore-tutorialguide
 giteveryday guide
 gitglossary guide
 gitignore   guide
 gitmodules  guide
 gitrevisionsguide
 gittutorial guide
+gittutorial-2   guide
 gitworkflowsguide
-- 8< --

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Duy Nguyen]

On Sun, Apr 15, 2018 at 11:21 PM, Philip Oakley  wrote:
> From: "Duy Nguyen"  : Saturday, April 14, 2018 4:44 PM
>
>> On Thu, Apr 12, 2018 at 12:06 AM, Philip Oakley 
>> wrote:
>>>
>>> I'm only just catching up, but does/can this series also capture the
>>> non-command guides that are available in git so that the 'git help -g'
>>> can
>>> begin to list them all?
>>
>>
>> It currently does not. But I don't see why it should not. This should
>> allow git.txt to list all the guides too, for people who skip "git
>> help" and go hard core mode with "man git". Thanks for bringing this
>> up.
>> --
>> Duy
>>
> Is that something I should add to my todo to add a 'guide' category etc.?

I added it too [1]. Not sure if you want anything more on top though.

[1] https://public-inbox.org/git/20180415164238.9107-7-pclo...@gmail.com/T/#u
-- 
Duy

Re: [PATCH/RFC 0/5] Keep all info in command-list.txt in git binary

[Philip Oakley]

From: "Duy Nguyen"  : Saturday, April 14, 2018 4:44 PM

On Thu, Apr 12, 2018 at 12:06 AM, Philip Oakley 
wrote:

I'm only just catching up, but does/can this series also capture the
non-command guides that are available in git so that the 'git help -g'
can
begin to list them all?


It currently does not. But I don't see why it should not. This should
allow git.txt to list all the guides too, for people who skip "git
help" and go hard core mode with "man git". Thanks for bringing this
up.
--
Duy


Is that something I should add to my todo to add a 'guide' category etc.?

A quick search of public-inbox suggests
https://public-inbox.org/git/1361660761-1932-1-git-send-email-philipoak...@iee.org/
as being where I first made the suggestions, but it got trimmed back to not
update (be embedded in) the command-list.txt

Philip

[PATCH v2 0/6] Keep all info in command-list.txt in git binary

[Nguyễn Thái Ngọc Duy]

v2 changes

- bug fixes spotted by Eric
- keep 'git help -av' layout close to what's in git.txt
- enable pager for 'git help -av' because it's usually long
- move guide list (aka 'help -g') to command-list.txt

Nguyễn Thái Ngọc Duy (6):
  git.c: convert --list-builtins to --list-cmds=builtins
  git.c: implement --list-cmds=all and use it in git-completion.bash
  generate-cmdlist.sh: keep all information in common-cmds.h
  git.c: implement --list-cmds=porcelain
  help: add "-a --verbose" to list all commands with synopsis
  help: use command-list.txt for the source of guides

 Documentation/git-help.txt |   4 +-
 Documentation/gitattributes.txt|   2 +-
 Documentation/gitmodules.txt   |   2 +-
 Documentation/gitrevisions.txt |   2 +-
 builtin/help.c |  39 ++
 command-list.txt   |   8 ++
 contrib/completion/git-completion.bash |  96 +--
 generate-cmdlist.sh|  65 +++---
 git.c  |  16 ++-
 help.c | 163 -
 help.h |   4 +
 t/t0012-help.sh|   2 +-
 t/t9902-completion.sh  |   4 +-
 13 files changed, 249 insertions(+), 158 deletions(-)

-- 
2.17.0.367.g5dd2e386c3