{"thread":{"id":"61105","subject":"[PATCH 2/2] Add transport message for up-to-date references","startedAt":"2024-03-12T21:55:19Z","lastAt":"2024-03-15T16:54:44Z","messageCount":9,"participants":["Christopher Lindee","Junio C Hamano","Martin Ågren"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"490515","messageId":"SA1PR14MB4691A8AC86E290A3539BE1398D2B2@SA1PR14MB4691.namprd14.prod.outlook.com","threadId":"61105","inReplyTo":null,"subject":"[PATCH 2/2] Add transport message for up-to-date references","fromName":"Christopher Lindee","fromEmail":"christopher.lindee@webpros.com","sentAt":"2024-03-12T21:55:18Z","receivedAt":"2024-03-12T21:55:19Z","isPatch":true,"sender":{"key":"christopher.lindee@webpros.com","avatar":null},"body":"If the `--send-up-to-date` option in the previous commit is used, the\n\"Everything up-to-date!\" message will never appear, even if all of the\nrefs are up to date. Moreover, the output `deadbeef..deadbeef` appears\nsuspicious, almost as if a collision occurred. To clarify that the hash\nis, in fact, identical & to allow grepping for the phrase \"up-to-date\",\nadd a message to the output when the ref is transmitted, but no change\noccurred.\n\nSigned-off-by: Christopher Lindee \n---\n transport.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/transport.c b/transport.c\nindex 84deadd2b6..89802452ea 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -670,6 +670,8 @@ static void print_ok_ref_status(struct ref *ref,\n \t\t\tstrbuf_addstr(&quickref, \"..\");\n \t\t\ttype = ' ';\n \t\t\tmsg = NULL;\n+\t\t\tif (oideq(old_oid, new_oid))\n+\t\t\t\tmsg = \"up-to-date\";\n \t\t}\n \t\tstrbuf_add_unique_abbrev(&quickref, new_oid,\n \t\t\t\t\t DEFAULT_ABBREV);\n-- \n2.38.1"},{"id":"490597","messageId":"xmqqttl94qxg.fsf@gitster.g","threadId":"61105","inReplyTo":"SA1PR14MB4691A8AC86E290A3539BE1398D2B2@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-14T00:07:07Z","receivedAt":"2024-03-14T00:07:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Lindee writes:\n\n> Subject: Re: [PATCH 2/2] Add transport message for up-to-date references\n\nSame comment as [1/2]. Perhaps\n\n push: mark forced no-op pushes differently in the report\n\nor something?\n\n> If the `--send-up-to-date` option in the previous commit is used, the\n> \"Everything up-to-date!\" message will never appear, even if all of the\n> refs are up to date. Moreover, the output `deadbeef..deadbeef` appears\n> suspicious, almost as if a collision occurred. To clarify that the hash\n> is, in fact, identical & to allow grepping for the phrase \"up-to-date\",\n> add a message to the output when the ref is transmitted, but no change\n> occurred.\n>\n> Signed-off-by: Christopher Lindee \n> ---\n> transport.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/transport.c b/transport.c\n> index 84deadd2b6..89802452ea 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -670,6 +670,8 @@ static void print_ok_ref_status(struct ref *ref,\n> \t\t\tstrbuf_addstr(&quickref, \"..\");\n> \t\t\ttype = ' ';\n> \t\t\tmsg = NULL;\n> +\t\t\tif (oideq(old_oid, new_oid))\n> +\t\t\t\tmsg = \"up-to-date\";\n> \t\t}\n> \t\tstrbuf_add_unique_abbrev(&quickref, new_oid,\n> \t\t\t\t\t DEFAULT_ABBREV);\n\nThis code is in an if/else cascade of this shape:\n\n\tif we are deleting\n\t\tprint [deleted]\n\telse if they did not have any\n\t\tprint [new]\n\telse {\n\t\tif we forced\n\t\tthen\n\t\t\tprepare to say forced\n\t\telse\n\t\t\tprepare to say an unforced update\n\t\tsay \"updated\" in a certain way\n\t}\n\nThe above addition looks somewhat out of place. I would understand\nif the addition happened like so instead:\n\ndiff --git i/transport.c w/transport.c\nindex df518ead70..bacef94b33 100644\n--- i/transport.c\n+++ w/transport.c\n@@ -666,6 +666,8 @@ static void print_ok_ref_status(struct ref *ref,\n \t\t\tstrbuf_addstr(&quickref, \"...\");\n \t\t\ttype = '+';\n \t\t\tmsg = \"forced update\";\n+\t\t} else if (oideq(old_oid, new_oid)) {\n+\t\t\t... prepare to say \"same but forced no-op\" ...\n \t\t} else {\n \t\t\tstrbuf_addstr(&quickref, \"..\");\n \t\t\ttype = ' ';\n\n\nto make the new logic flow more like so:\n\n\t\tif we forced\n\t\tthen\n\t\t\tprepare to say forced\n\t\telse if we were forced to tell no-op push\n\t\t\tprepare to say we did no-op\n\t\telse\n\t\t\tprepare to say an unforced update\n\t\tsay \"updated\" in a certain way\n"},{"id":"490645","messageId":"CAN0heSp2A-sx_WA__90WGhP1yD3EBBtR7_N7s5ZPWqe559xpKA@mail.gmail.com","threadId":"61105","inReplyTo":"SA1PR14MB4691A8AC86E290A3539BE1398D2B2@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2024-03-14T21:44:26Z","receivedAt":"2024-03-14T21:44:38Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Tue, 12 Mar 2024 at 22:55, Christopher Lindee\n wrote:\n>\n> If the `--send-up-to-date` option in the previous commit is used, the\n> \"Everything up-to-date!\" message will never appear, even if all of the\n> refs are up to date. Moreover, the output `deadbeef..deadbeef` appears\n> suspicious, almost as if a collision occurred. To clarify that the hash\n> is, in fact, identical & to allow grepping for the phrase \"up-to-date\",\n> add a message to the output when the ref is transmitted, but no change\n> occurred.\n\n> + if (oideq(old_oid, new_oid))\n> + msg = \"up-to-date\";\n\nFrom a grammar point of view, I think this should be \"up to date\". (A\nbranch can be up to date, in which case it is an up-to-date branch.)\n\nThere is quite some history exactly around this phrase, though, see,\ne.g., 80bdaba894 (\"messages: mark some strings with \"up-to-date\" not to\ntouch\", 2024-01-12). If we really want \"up-to-date\" here, I think we\nshould add a comment similar to 80bdaba894, although it does feel\nawkward to knowingly introduce a new instance. So maybe better, go for\n\"up-to-date ref\"?\n\nIf we worry about how \"Everything up-to-date\" disappeared in the\nprevious commit, maybe that commit should instead detect that all refs\nwere such no-ops and emit that \"Everything up-to-date\"?\n\n(Part of me wonders what kind of existing scripts [1] would be helped by\nus sticking to the dashed form. If they go \"grep '^Everything\nup-to-date$\", they won't notice this new output. If they use a more\nrelaxed \"grep up-to-date\", they might get confused, possibly much more\noften than they could already be by a branch called \"up-to-date\".)\n\nMartin\n\n[1] They don't even provide this new option, but let's assume we\neventually grow a config knob.\n"},{"id":"490659","messageId":"SA1PR14MB469168B6AAB246344190D9038D282@SA1PR14MB4691.namprd14.prod.outlook.com","threadId":"61105","inReplyTo":"CAN0heSp2A-sx_WA__90WGhP1yD3EBBtR7_N7s5ZPWqe559xpKA@mail.gmail.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Christopher Lindee","fromEmail":"christopher.lindee@webpros.com","sentAt":"2024-03-15T03:09:53Z","receivedAt":"2024-03-15T03:09:55Z","isPatch":true,"sender":{"key":"christopher.lindee@webpros.com","avatar":null},"body":"Martin Ågren writes:\n> On Tue, 12 Mar 2024 at 22:55, Christopher Lindee\n> wrote:\n> >\n> > If the `--send-up-to-date` option in the previous commit is used, the\n> > \"Everything up-to-date!\" message will never appear, even if all of the\n> > refs are up to date.  Moreover, the output `deadbeef..deadbeef` appears\n> > suspicious, almost as if a collision occurred.  To clarify that the hash\n> > is, in fact, identical & to allow grepping for the phrase \"up-to-date\",\n> > add a message to the output when the ref is transmitted, but no change\n> > occurred.\n>\n> > +                       if (oideq(old_oid, new_oid))\n> > +                               msg = \"up-to-date\";\n>\n> From a grammar point of view, I think this should be \"up to date\". (A\n> branch can be up to date, in which case it is an up-to-date branch.)\n\nI wondered about that; I will fix.\n\n> There is quite some history exactly around this phrase, though, see,\n> e.g., 80bdaba894 (\"messages: mark some strings with \"up-to-date\" not to\n> touch\", 2024-01-12). If we really want \"up-to-date\" here, I think we\n> should add a comment similar to 80bdaba894, although it does feel\n> awkward to knowingly introduce a new instance. So maybe better, go for\n> \"up-to-date ref\"?\n>\n> If we worry about how \"Everything up-to-date\" disappeared in the\n> previous commit, maybe that commit should instead detect that all refs\n> were such no-ops and emit that \"Everything up-to-date\"?\n>\n> (Part of me wonders what kind of existing scripts [1] would be helped by\n> us sticking to the dashed form. If they go \"grep '^Everything\n> up-to-date$\", they won't notice this new output. If they use a more\n> relaxed \"grep up-to-date\", they might get confused, possibly much more\n> often than they could already be by a branch called \"up-to-date\".)\n\nThis raises a good point: if we default to the old behavior and only\nsend up-to-date refs when the new option is present (as is currently \nthe case with this patchset), then existing scripts will continue to\nwork. Moreover, scripts using the new option will consistently have\n\"up to date\", so they can change the grep to use that (since it will \nnever show \"Everything up-to-date!\").\n\nThanks,\nChris."},{"id":"490668","messageId":"SA1PR14MB46913A0B1CB3F3D39ADA6A5F8D282@SA1PR14MB4691.namprd14.prod.outlook.com","threadId":"61105","inReplyTo":"xmqqttl94qxg.fsf@gitster.g","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Christopher Lindee","fromEmail":"christopher.lindee@webpros.com","sentAt":"2024-03-15T06:09:02Z","receivedAt":"2024-03-15T06:09:05Z","isPatch":true,"sender":{"key":"christopher.lindee@webpros.com","avatar":null},"body":"Junio C Hamano writes:\n\n> Christopher Lindee writes:\n> \n> > Subject: Re: [PATCH 2/2] Add transport message for up-to-date references\n> \n> Same comment as [1/2]. Perhaps\n> \n> push: mark forced no-op pushes differently in the report\n> \n> or something?\n\nThanks! I will correct this.\n\n> > If the `--send-up-to-date` option in the previous commit is used, the\n> > \"Everything up-to-date!\" message will never appear, even if all of the\n> > refs are up to date. Moreover, the output `deadbeef..deadbeef` appears\n> > suspicious, almost as if a collision occurred. To clarify that the hash\n> > is, in fact, identical & to allow grepping for the phrase \"up-to-date\",\n> > add a message to the output when the ref is transmitted, but no change\n> > occurred.\n> >\n> > Signed-off-by: Christopher Lindee \n> > ---\n> > transport.c | 2 ++\n> > 1 file changed, 2 insertions(+)\n> >\n> > diff --git a/transport.c b/transport.c\n> > index 84deadd2b6..89802452ea 100644\n> > --- a/transport.c\n> > +++ b/transport.c\n> > @@ -670,6 +670,8 @@ static void print_ok_ref_status(struct ref *ref,\n> > strbuf_addstr(&quickref, \"..\");\n> > type = ' ';\n> > msg = NULL;\n> > + if (oideq(old_oid, new_oid))\n> > + msg = \"up-to-date\";\n> > }\n> > strbuf_add_unique_abbrev(&quickref, new_oid,\n> > DEFAULT_ABBREV);\n> \n> This code is in an if/else cascade of this shape:\n> \n> if we are deleting\n> print [deleted]\n> else if they did not have any\n> print [new]\n> else {\n> if we forced\n> then\n> prepare to say forced\n> else\n> prepare to say an unforced update\n> say \"updated\" in a certain way\n> }\n> \n> The above addition looks somewhat out of place. I would understand\n> if the addition happened like so instead:\n> \n> diff --git i/transport.c w/transport.c\n> index df518ead70..bacef94b33 100644\n> --- i/transport.c\n> +++ w/transport.c\n> @@ -666,6 +666,8 @@ static void print_ok_ref_status(struct ref *ref,\n> strbuf_addstr(&quickref, \"...\");\n> type = '+';\n> msg = \"forced update\";\n> + } else if (oideq(old_oid, new_oid)) {\n> + ... prepare to say \"same but forced no-op\" ...\n> } else {\n> strbuf_addstr(&quickref, \"..\");\n> type = ' ';\n> \n> \n> to make the new logic flow more like so:\n> \n> if we forced\n> then\n> prepare to say forced\n> else if we were forced to tell no-op push\n> prepare to say we did no-op\n> else\n> prepare to say an unforced update\n> say \"updated\" in a certain way\n\nThe shoehorned `if` was to avoid duplicating the `strbuf_addstr` and\n`type` statements. It sounds like code duplication is not a concern,\nso I can make that change. However, with this new logic flow, maybe\nit would be better to have wholly unique values for the display, for\nexample:\n\n deadbeef...deadbeef main -> main\n -deadbeef..deadbeef main -> main\n =deadbeef..deadbeef main -> main\n =deadbeef main -> main\n deadbeef^! main -> main\n ^deadbeef deadbeef main -> main\n deadbeef==deadbeef main -> main\n deadbeef..deadbeef main == main\n\nThere's a fair amount of room for creativity here. Of course, using\nrevisions is useful, but the existing output contains `+` for forced\nupdates, which is not valid in a revision, so there is clearly space\nfor novelty.\n\nWe may also want to consider what happens when both --force and this\nnew option are used at the same time. When testing, the message was\nalways \"up-to-date\", but I realize now that a server might report it\nas a forced update - it would be odd, but would it be impossible? I\ndon't know enough about the push certificate records discussion from\nthe cover page to say that we wouldn't force an update on a noop.\n\nRegards,\nChris."},{"id":"490669","messageId":"SA1PR14MB469129D76D1C65832FC10EA58D282@SA1PR14MB4691.namprd14.prod.outlook.com","threadId":"61105","inReplyTo":"SA1PR14MB46913A0B1CB3F3D39ADA6A5F8D282@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Christopher Lindee","fromEmail":"christopher.lindee@webpros.com","sentAt":"2024-03-15T06:47:28Z","receivedAt":"2024-03-15T06:47:30Z","isPatch":true,"sender":{"key":"christopher.lindee@webpros.com","avatar":null},"body":"Christopher Lindee writes:\n\n> Junio C Hamano writes:\n> \n> > Christopher Lindee writes:\n> > \n> > > Subject: Re: [PATCH 2/2] Add transport message for up-to-date references\n> > >\n> > > If the `--send-up-to-date` option in the previous commit is used, the\n> > > \"Everything up-to-date!\" message will never appear, even if all of the\n> > > refs are up to date. Moreover, the output `deadbeef..deadbeef` appears\n> > > suspicious, almost as if a collision occurred. To clarify that the hash\n> > > is, in fact, identical & to allow grepping for the phrase \"up-to-date\",\n> > > add a message to the output when the ref is transmitted, but no change\n> > > occurred.\n> > >\n> > > Signed-off-by: Christopher Lindee \n> > > ---\n> > > transport.c | 2 ++\n> > > 1 file changed, 2 insertions(+)\n> > >\n> > > diff --git a/transport.c b/transport.c\n> > > index 84deadd2b6..89802452ea 100644\n> > > --- a/transport.c\n> > > +++ b/transport.c\n> > > @@ -670,6 +670,8 @@ static void print_ok_ref_status(struct ref *ref,\n> > > strbuf_addstr(&quickref, \"..\");\n> > > type = ' ';\n> > > msg = NULL;\n> > > + if (oideq(old_oid, new_oid))\n> > > + msg = \"up-to-date\";\n> > > }\n> > > strbuf_add_unique_abbrev(&quickref, new_oid,\n> > > DEFAULT_ABBREV);\n> > \n> > This code is in an if/else cascade of this shape:\n> > \n> > if we are deleting\n> > print [deleted]\n> > else if they did not have any\n> > print [new]\n> > else {\n> > if we forced\n> > then\n> > prepare to say forced\n> > else\n> > prepare to say an unforced update\n> > say \"updated\" in a certain way\n> > }\n> > \n> > The above addition looks somewhat out of place. I would understand\n> > if the addition happened like so instead:\n> > \n> > diff --git i/transport.c w/transport.c\n> > index df518ead70..bacef94b33 100644\n> > --- i/transport.c\n> > +++ w/transport.c\n> > @@ -666,6 +666,8 @@ static void print_ok_ref_status(struct ref *ref,\n> > strbuf_addstr(&quickref, \"...\");\n> > type = '+';\n> > msg = \"forced update\";\n> > + } else if (oideq(old_oid, new_oid)) {\n> > + ... prepare to say \"same but forced no-op\" ...\n> > } else {\n> > strbuf_addstr(&quickref, \"..\");\n> > type = ' ';\n> > \n> > \n> > to make the new logic flow more like so:\n> > \n> > if we forced\n> > then\n> > prepare to say forced\n> > else if we were forced to tell no-op push\n> > prepare to say we did no-op\n> > else\n> > prepare to say an unforced update\n> > say \"updated\" in a certain way\n> \n> The shoehorned `if` was to avoid duplicating the `strbuf_addstr` and\n> `type` statements. It sounds like code duplication is not a concern,\n> so I can make that change. However, with this new logic flow, maybe\n> it would be better to have wholly unique values for the display, for\n> example:\n> \n> deadbeef...deadbeef main -> main\n> -deadbeef..deadbeef main -> main\n> =deadbeef..deadbeef main -> main\n> =deadbeef main -> main\n> deadbeef^! main -> main\n> ^deadbeef deadbeef main -> main\n> deadbeef==deadbeef main -> main\n> deadbeef..deadbeef main == main\n> \n> There's a fair amount of room for creativity here. Of course, using\n> revisions is useful, but the existing output contains `+` for forced\n> updates, which is not valid in a revision, so there is clearly space\n> for novelty.\n\nIt seems I just needed to look one function below to find precedent:\n\n static int print_one_push_report(struct ref *ref, const char *dest, int count,\n struct ref_push_report *report,\n int porcelain, int summary_width)\n { \n ...\n case REF_STATUS_UPTODATE:\n print_ref_status('=', \"[up to date]\", ref,\n ref->peer_ref, NULL,\n report, porcelain, summary_width);\n break;\n\nIs this a precedent we should follow, or is print_one_push_report()\nspecial in some way?\n\nThanks,\nChris."},{"id":"490695","messageId":"xmqqil1nsay5.fsf@gitster.g","threadId":"61105","inReplyTo":"SA1PR14MB46913A0B1CB3F3D39ADA6A5F8D282@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T16:42:58Z","receivedAt":"2024-03-15T16:43:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Lindee writes:\n\n> The shoehorned `if` was to avoid duplicating the `strbuf_addstr` and\n> `type` statements. It sounds like code duplication is not a concern,\n> so I can make that change. However, with this new logic flow, maybe\n> it would be better to have wholly unique values for the display,...\n\nI wasn't avoiding duplication at all.\n\n20% of the reason why I pointed it out was because the third member\n\"forced no-op update\" that is added to the existing two, \"forced\nupdate\" and \"non-forced fast-forwarding update\", looked as if it\nwere made a part of the \"non-forced update\", where the three at\nleast to me looked more like equals. And the rest 80% is exactly\nbecause the \"three equals\" arrangement gives us a more freedom to\nexpress how we show the update.\n\n> deadbeef==deadbeef main -> main\n> deadbeef..deadbeef main == main\n>\n> There's a fair amount of room for creativity here. Of course, using\n> revisions is useful, but the existing output contains `+` for forced\n> updates, which is not valid in a revision, so there is clearly space\n> for novelty.\n\nSorry, but I do not understand the remark about '+' at all. In the\nexisting output, the flag char like '+' comes at the beginning and\nfollowed by a whitespace before and ref correspondence\nis reported, no?\n\nI can buy the idea of using \"summary\" that is different from the\nexisting A..B or A...B form to make it stand out, but the \"->\"\nbetween the two \"main\" in your example must stay as is. That part\nof the output shows the correspondence between our ref and their\nref, and has nothing to do with what object their ref originally was\npointing at and what object their ref points at now. There is no\nchoice of the sign there, but even if there were a choice [*], it\nshould not be influenced by how the pair of objects\ninvolved in the update are related.\n\n\tSide note: we could imagine using a different sign other\n\tthan \"->\" if you update a branch from a non-ref\n\t(e.g. pushing the freshly made commit on detached HEAD state\n\tto update a branch over there), or perhaps updating a ref\n\twith a ref from different hierarchies (e.g. push the tip of\n\tthe current branch to update their refs/tags/v1.2.3).\n\nPerhaps use \"%\" as the type and show just a single object name in\nsummary? I.e. something like the first one in this example:\n\n % 4f9b731bde\t\t master -> main\n 1203cff8ae..6e790dbe36 next -> next\n + b7485789d7...3e580ca942 seen -> seen\n\n> We may also want to consider what happens when both --force and this\n> new option are used at the same time. When testing, the message was\n> always \"up-to-date\", but I realize now that a server might report it\n> as a forced update - it would be odd, but would it be impossible?\n\nIf I recall correctly what happens at the protocol level (without\nlooking at the code---look for comment \"Finally.*tell the other end\"\nto find it out where it is), I do not think the receiving end can\neven tell, because the only thing they see for each ref is the old\nand the new object name and the refname. There are intricate\nmechanism among the sender (push/send-pack), the receiver\n(receive-pack), and the transport helpers, to carry necessary info\naround to produce a report that says \"this ref update went OK\" and\n\"this ref update was NG, due to such and such reason\", but I do not\nthink the receiving end does not compute the forcedness for\nreporting. It only runs in_merge_bases() between old and new\nobjects to decide if it is a non-fast-forward, when configured to\nreject such updates, and it may report the failure was due to\nnon-fast-forward condition, but I do not think there is more than\none kind of successful updates reported (they just say \"ok\").\n\nSpeaking of what happens on the receiving end, we may have to\nconsider what, if anything, we need to do to the hooks that run\nthere, which gets \"the ref X was updated from object A to object B\".\nThey never saw \"object A to object A\" update and may get confused.\n"},{"id":"490696","messageId":"xmqqbk7fsanz.fsf@gitster.g","threadId":"61105","inReplyTo":"SA1PR14MB469129D76D1C65832FC10EA58D282@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T16:49:04Z","receivedAt":"2024-03-15T16:49:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Lindee writes:\n\n> It seems I just needed to look one function below to find precedent:\n>\n> static int print_one_push_report(struct ref *ref, const char *dest, int count,\n> struct ref_push_report *report,\n> int porcelain, int summary_width)\n\nIt is \"static int\" meaning it is a file-local function. You should\nbe able to find out how it is used without running around all over\nthe codebase but just the single file it appears in.\n\n> { \n> ...\n> case REF_STATUS_UPTODATE:\n> print_ref_status('=', \"[up to date]\", ref,\n> ref->peer_ref, NULL,\n> report, porcelain, summary_width);\n> break;\n>\n> Is this a precedent we should follow, or is print_one_push_report()\n> special in some way?\n\nThe code path this function is called is triggerd when you go\nverbose:\n\n $ git push ../victim-00 v2.43.0-rc0~56:master\n Everything up-to-date\n $ git push -v ../victim-00 v2.43.0-rc0~56:master\n Pushing to ../victim-00\n To ../victim-00\n = [up to date]\t\tv2.43.0-rc0~56 -> master\n\nDo we want to make it possible to differentiate between the forced\nno-op and omitted no-op from \"git push -v\" output, or should users\nrefrain from using \"-v\" when they want to be able to tell?\n\n"},{"id":"490697","messageId":"xmqq7ci3saep.fsf@gitster.g","threadId":"61105","inReplyTo":"SA1PR14MB469168B6AAB246344190D9038D282@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 2/2] Add transport message for up-to-date references","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-15T16:54:38Z","receivedAt":"2024-03-15T16:54:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Lindee writes:\n\n> This raises a good point: if we default to the old behavior and only\n> send up-to-date refs when the new option is present (as is currently \n> the case with this patchset), then existing scripts will continue to\n> work. Moreover, scripts using the new option will consistently have\n> \"up to date\", so they can change the grep to use that (since it will \n> never show \"Everything up-to-date!\").\n\nScripts should never be parsing the output meant for humans. You\nshould worry about \"git push --porcelain\" output first.\n\nI think we report \"= srcref:dstref [up to date]\" there and that is\nwhat the scripts are expecting for an up-to-date case. If you want\nto allow the scripts to react to a forced no-op update differently\nfrom the normal \"we didn't even tell them about this ref\" case, then\nwe'd need a different type letter or \"[up to date]\" string (or\nboth).\n\n\n"}]}