[Market Info] Now is your chance to exhibit in Japan!
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
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
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
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
"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
"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
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
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
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
"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
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
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
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
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
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
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
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"
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`
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`
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
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"
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"
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"
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"
> 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
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"
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!!
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
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
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
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
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
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
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
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
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
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
> 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
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
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
> @@ -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
> + !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
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
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
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
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
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
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
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
On Thu, May 10, 2018 at 11:35 PM, Stefan Bellerwrote: > 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
Hi Pratik, On Wed, May 9, 2018 at 8:07 PM, Pratik Karkiwrote: > 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
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
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
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
On Thu, May 3, 2018 at 6:31 AM, Andrew Ardillwrote: > 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
On 2 May 2018 at 17:12, Johannes Schindelinwrote: > 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
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
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
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
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
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
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
[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
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
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
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
Hi Junio, On Thu, 26 Apr 2018, Junio C Hamano wrote: > Johannes Schindelinwrites: > > > - 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
Johannes Schindelinwrites: > -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
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
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
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
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
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
On Sat, Apr 21, 2018 at 2:43 AM, Johannes Schindelinwrote: > 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
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
On Sun, Apr 22, 2018 at 5:58 PM, Ramsay Joneswrote: >>> 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
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
On Sun, Apr 22, 2018 at 4:45 PM, Ramsay Joneswrote: > > > 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
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
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
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
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
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
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
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
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
On Thu, Apr 19, 2018 at 12:37 PM, Simon Ruderichwrote: > 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
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
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
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
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
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
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
On Wed, Apr 18, 2018 at 12:47 AM, Philip Oakleywrote: >>> > 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
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
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
On Tue, Apr 17, 2018 at 06:24:41PM +0200, Duy Nguyen wrote: > On Sun, Apr 15, 2018 at 11:21 PM, Philip Oakleywrote: > > 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
On Sun, Apr 15, 2018 at 11:21 PM, Philip Oakleywrote: > 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
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
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