{"thread":{"id":"61104","subject":"[PATCH 1/2] Teach send-pack & push to --send-up-to-date refs","startedAt":"2024-03-12T21:55:14Z","lastAt":"2024-03-15T08:18:54Z","messageCount":3,"participants":["Christopher Lindee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"490514","messageId":"SA1PR14MB469144B8903909F65493323A8D2B2@SA1PR14MB4691.namprd14.prod.outlook.com","threadId":"61104","inReplyTo":null,"subject":"[PATCH 1/2] Teach send-pack & push to --send-up-to-date refs","fromName":"Christopher Lindee","fromEmail":"christopher.lindee@webpros.com","sentAt":"2024-03-12T21:55:12Z","receivedAt":"2024-03-12T21:55:14Z","isPatch":true,"sender":{"key":"christopher.lindee@webpros.com","avatar":null},"body":"Provide an option that forces the local Git transport to transmit a local\nref even when the receiving ref would not change (i.e. the local & remote\nrefs point to the same object).  This is not done normally, as no changes\nwould take place on the server in a vanilla Git setup.  However, some Git\nservers support push options and those push options decide which branches\nto operate on based on the refs that are transmitted alongside the option.\n\nThis option ensures the refs listed on the command line are always sent.\n\nSigned-off-by: Christopher Lindee <christopher.lindee@webpros.com>\n---\n Documentation/git-push.txt      | 8 +++++++-\n Documentation/git-send-pack.txt | 7 +++++++\n builtin/push.c                  | 1 +\n builtin/send-pack.c             | 4 ++++\n send-pack.c                     | 2 +-\n send-pack.h                     | 3 ++-\n transport-helper.c              | 7 ++++++-\n transport.c                     | 1 +\n transport.h                     | 1 +\n 9 files changed, 30 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 9b7cfbc5c1..44d1e04931 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n [verse]\n 'git push' [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]\n \t   [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-q | --quiet] [-v | --verbose]\n-\t   [-u | --set-upstream] [-o <string> | --push-option=<string>]\n+\t   [-u | --set-upstream] [-o <string> | --push-option=<string>] [--send-up-to-date]\n \t   [--[no-]signed|--signed=(true|false|if-asked)]\n \t   [--force-with-lease[=<refname>[:<expect>]] [--force-if-includes]]\n \t   [--no-verify] [<repository> [<refspec>...]]\n@@ -225,6 +225,12 @@ already exists on the remote side.\n \tline, the values of configuration variable `push.pushOption`\n \tare used instead.\n \n+--send-up-to-date::\n+\tUsually, the command will not send a local ref when the remote ref\n+\talready matches, as no change would occur if it did.  This flag\n+\tdisables that check.  This allows push options to be sent alongside\n+\tup-to-date references on the remote.\n+\n --receive-pack=<git-receive-pack>::\n --exec=<git-receive-pack>::\n \tPath to the 'git-receive-pack' program on the remote\ndiff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt\nindex b9e73f2e77..2517d226d0 100644\n--- a/Documentation/git-send-pack.txt\n+++ b/Documentation/git-send-pack.txt\n@@ -13,6 +13,7 @@ SYNOPSIS\n \t\t[--receive-pack=<git-receive-pack>]\n \t\t[--verbose] [--thin] [--atomic]\n \t\t[--[no-]signed | --signed=(true|false|if-asked)]\n+\t\t[--send-up-to-date]\n \t\t[<host>:]<directory> (--all | <ref>...)\n \n DESCRIPTION\n@@ -88,6 +89,12 @@ be in a separate packet, and the list must end with a flush packet.\n \toptions, error out.  See linkgit:git-push[1] and\n \tlinkgit:githooks[5] for details.\n \n+--send-up-to-date::\n+\tUsually, the command will not send a local ref when the remote ref\n+\talready matches, as no change would occur if it did.  This flag\n+\tdisables that check.  This allows push options to be sent alongside\n+\tup-to-date references on the remote.\n+\n <host>::\n \tA remote host to house the repository.  When this\n \tpart is specified, 'git-receive-pack' is invoked via\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2fbb31c3ad..555e8b53fe 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -617,6 +617,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"no-verify\", &flags, N_(\"bypass pre-push hook\"), TRANSPORT_PUSH_NO_HOOK),\n \t\tOPT_BIT(0, \"follow-tags\", &flags, N_(\"push missing but relevant tags\"),\n \t\t\tTRANSPORT_PUSH_FOLLOW_TAGS),\n+\t\tOPT_BIT(0, \"send-up-to-date\", &flags, N_(\"send refs when there is no change\"), TRANSPORT_PUSH_SEND_UPTODATE),\n \t\tOPT_CALLBACK_F(0, \"signed\", &push_cert, \"(yes|no|if-asked)\", N_(\"GPG sign the push\"),\n \t\t\t\tPARSE_OPT_OPTARG, option_parse_push_signed),\n \t\tOPT_BIT(0, \"atomic\", &flags, N_(\"request atomic transaction on remote side\"), TRANSPORT_PUSH_ATOMIC),\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 3df9eaad09..da6630fa12 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -19,6 +19,7 @@ static const char * const send_pack_usage[] = {\n \t   \"              [--receive-pack=<git-receive-pack>]\\n\"\n \t   \"              [--verbose] [--thin] [--atomic]\\n\"\n \t   \"              [--[no-]signed | --signed=(true|false|if-asked)]\\n\"\n+\t   \"              [--send-up-to-date]\\n\"\n \t   \"              [<host>:]<directory> (--all | <ref>...)\"),\n \tNULL,\n };\n@@ -165,6 +166,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tconst char *receivepack = \"git-receive-pack\";\n \tunsigned dry_run = 0;\n \tunsigned send_mirror = 0;\n+\tunsigned send_uptodate = 0;\n \tunsigned force_update = 0;\n \tunsigned quiet = 0;\n \tint push_cert = 0;\n@@ -200,6 +202,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"stateless-rpc\", &stateless_rpc, N_(\"use stateless RPC protocol\")),\n \t\tOPT_BOOL(0, \"stdin\", &from_stdin, N_(\"read refs from stdin\")),\n \t\tOPT_BOOL(0, \"helper-status\", &helper_status, N_(\"print status from remote helper\")),\n+\t\tOPT_BOOL(0, \"send-up-to-date\", &send_uptodate, N_(\"send refs when there is no change\")),\n \t\tOPT_CALLBACK_F(0, \"force-with-lease\", &cas, N_(\"<refname>:<expect>\"),\n \t\t  N_(\"require old value of ref to be at this value\"),\n \t\t  PARSE_OPT_OPTARG, parseopt_push_cas_option),\n@@ -221,6 +224,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \targs.verbose = verbose;\n \targs.dry_run = dry_run;\n \targs.send_mirror = send_mirror;\n+\targs.send_uptodate = send_uptodate;\n \targs.force_update = force_update;\n \targs.quiet = quiet;\n \targs.push_cert = push_cert;\ndiff --git a/send-pack.c b/send-pack.c\nindex 37f59d4f66..30b0f2f276 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -313,7 +313,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar\n \tcase REF_STATUS_REJECT_NODELETE:\n \t\treturn CHECK_REF_STATUS_REJECTED;\n \tcase REF_STATUS_UPTODATE:\n-\t\treturn CHECK_REF_UPTODATE;\n+\t\treturn args->send_uptodate ? 0 : CHECK_REF_UPTODATE;\n \n \tdefault:\n \tcase REF_STATUS_EXPECTING_REPORT:\ndiff --git a/send-pack.h b/send-pack.h\nindex 7edb80596c..241b4278bc 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -27,7 +27,8 @@ struct send_pack_args {\n \t\tpush_cert:2,\n \t\tstateless_rpc:1,\n \t\tatomic:1,\n-\t\tdisable_bitmaps:1;\n+\t\tdisable_bitmaps:1,\n+\t\tsend_uptodate:1;\n \tconst struct string_list *push_options;\n };\n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex dd6002b393..115f46f6b8 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -983,6 +983,7 @@ static int push_refs_with_push(struct transport *transport,\n \tint force_all = flags & TRANSPORT_PUSH_FORCE;\n \tint mirror = flags & TRANSPORT_PUSH_MIRROR;\n \tint atomic = flags & TRANSPORT_PUSH_ATOMIC;\n+\tint do_nop = flags & TRANSPORT_PUSH_SEND_UPTODATE;\n \tstruct helper_data *data = transport->data;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct ref *ref;\n@@ -1010,7 +1011,11 @@ static int push_refs_with_push(struct transport *transport,\n \t\t\t} else\n \t\t\t\tcontinue;\n \t\tcase REF_STATUS_UPTODATE:\n-\t\t\tcontinue;\n+\t\t\tif (do_nop) {\n+\t\t\t\t; /* do nothing */\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tcontinue;\n \t\tdefault:\n \t\t\t; /* do nothing */\n \t\t}\ndiff --git a/transport.c b/transport.c\nindex df518ead70..84deadd2b6 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -867,6 +867,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re\n \n \tmemset(&args, 0, sizeof(args));\n \targs.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);\n+\targs.send_uptodate = !!(flags & TRANSPORT_PUSH_SEND_UPTODATE);\n \targs.force_update = !!(flags & TRANSPORT_PUSH_FORCE);\n \targs.use_thin_pack = data->options.thin;\n \targs.verbose = (transport->verbose > 0);\ndiff --git a/transport.h b/transport.h\nindex 6393cd9823..61a20e84f0 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -158,6 +158,7 @@ struct transport {\n #define TRANSPORT_RECURSE_SUBMODULES_ONLY\t(1<<15)\n #define TRANSPORT_PUSH_FORCE_IF_INCLUDES\t(1<<16)\n #define TRANSPORT_PUSH_AUTO_UPSTREAM\t\t(1<<17)\n+#define TRANSPORT_PUSH_SEND_UPTODATE\t\t(1<<18)\n \n int transport_summary_width(const struct ref *refs);\n \n-- \n2.38.1\n"},{"id":"490595","messageId":"xmqqzfv14s8d.fsf@gitster.g","threadId":"61104","inReplyTo":"SA1PR14MB469144B8903909F65493323A8D2B2@SA1PR14MB4691.namprd14.prod.outlook.com","subject":"Re: [PATCH 1/2] Teach send-pack & push to --send-up-to-date refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-13T23:38:58Z","receivedAt":"2024-03-13T23:39:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christopher Lindee <christopher.lindee@webpros.com> writes:\n\n> Subject: Re: [PATCH 1/2] Teach send-pack & push to --send-up-to-date refs\n\ncf. Documentation/SubmittingPatches:describe-changes.\n\nPerhaps\n\n\t[PATCH 1/2] push: allow pushing a no-op update to refs\n\nor something?\n\n> Provide an option that forces the local Git transport to transmit a local\n> ref even when the receiving ref would not change (i.e. the local & remote\n> refs point to the same object).  This is not done normally, as no changes\n> would take place on the server in a vanilla Git setup.  However, some Git\n> servers support push options and those push options decide which branches\n> to operate on based on the refs that are transmitted alongside the option.\n> This option ensures the refs listed on the command line are always sent.\n\nFlip the order of description to give your observation of the\ncurrent behaviour first, your perceived problem in it next, and then\nfinally your solution.  That way, those who are totally unfamiliar\nwith the problem area can just read from start to end straight, and\nthose who know what we do currently can skip and start from your\nproblem description.\n\nAlso, it would be nice if you throw in the issue about missing ref\nin the push certificate, which I mentioned in my response to the\ncover letter.\n\n>  Documentation/git-push.txt      | 8 +++++++-\n>  Documentation/git-send-pack.txt | 7 +++++++\n>  builtin/push.c                  | 1 +\n>  builtin/send-pack.c             | 4 ++++\n>  send-pack.c                     | 2 +-\n>  send-pack.h                     | 3 ++-\n>  transport-helper.c              | 7 ++++++-\n>  transport.c                     | 1 +\n>  transport.h                     | 1 +\n>  9 files changed, 30 insertions(+), 4 deletions(-)\n\nNo tests?\n\n>  'git push' [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]\n>  \t   [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-q | --quiet] [-v | --verbose]\n> -\t   [-u | --set-upstream] [-o <string> | --push-option=<string>]\n> +\t   [-u | --set-upstream] [-o <string> | --push-option=<string>] [--send-up-to-date]\n\nI do not know if the new option is a name that is easy to\nunderstand.  I am not great at naming, either but how does\n\"--force-no-op\" sound?\n\n> +\tUsually, the command will not send a local ref when the remote ref\n> +\talready matches, as no change would occur if it did.  This flag\n\nIn the context of \"push\", 'match' is a verb that is used in\ndifferent contexts, like \"'git push master' finds a ref that matches\n'master' and updates refs/heads/master\".  You would want to avoid it\nwhen you can.\n\n    Usually the command omits instructing the receiving end to\n    update a ref to point at an object, if the target ref points at\n    the exact object already, as no change ...\n\n> +\tdisables that check.  This allows push options to be sent alongside\n> +\tup-to-date references on the remote.\n\nAside from options and push certificates, there may be other side\neffects from this change.  I am not sure if we want to make sure we\nenumerate the benefit all like this.  Perhaps drop \"This allows ...\"\naltogether?\n\n> diff --git a/send-pack.c b/send-pack.c\n> index 37f59d4f66..30b0f2f276 100644\n> --- a/send-pack.c\n> +++ b/send-pack.c\n> @@ -313,7 +313,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar\n>  \tcase REF_STATUS_REJECT_NODELETE:\n>  \t\treturn CHECK_REF_STATUS_REJECTED;\n>  \tcase REF_STATUS_UPTODATE:\n> -\t\treturn CHECK_REF_UPTODATE;\n> +\t\treturn args->send_uptodate ? 0 : CHECK_REF_UPTODATE;\n>  \n>  \tdefault:\n>  \tcase REF_STATUS_EXPECTING_REPORT:\n\nGiven the existing flow of this code, I would prefer to see it\nwritten more like so:\n\ndiff --git i/send-pack.c w/send-pack.c\nindex 37f59d4f66..97d01726bb 100644\n--- i/send-pack.c\n+++ w/send-pack.c\n@@ -313,8 +313,9 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar\n \tcase REF_STATUS_REJECT_NODELETE:\n \t\treturn CHECK_REF_STATUS_REJECTED;\n \tcase REF_STATUS_UPTODATE:\n-\t\treturn CHECK_REF_UPTODATE;\n-\n+\t\tif (!args->send_uptodate)\n+\t\t\treturn CHECK_REF_UPTODATE;\n+\t\t/* fallthru */\n \tdefault:\n \tcase REF_STATUS_EXPECTING_REPORT:\n \t\t/* already passed checks on the local side */\n\nto make it clear that the caller gets 0 (\"go ahead and do it\")\nunless it is one of the cases explicitly listed abouve the \"default\"\nlabel.\n\n> +\tint do_nop = flags & TRANSPORT_PUSH_SEND_UPTODATE;\n\nHere we do call it \"do_nop\", showing that at least to you, \"nop\" is\na much more fitting word than \"uptodate\" for what we are trying to\nachieve in this topic.  It would give us one piece of good input to\ndecide what the end-user facing name should be.  In fact it is where\nI took inspiration from for \"force-noop\" I mentioned earlier.\n\n> @@ -1010,7 +1011,11 @@ static int push_refs_with_push(struct transport *transport,\n>  \t\t\t} else\n>  \t\t\t\tcontinue;\n>  \t\tcase REF_STATUS_UPTODATE:\n> -\t\t\tcontinue;\n> +\t\t\tif (do_nop) {\n> +\t\t\t\t; /* do nothing */\n> +\t\t\t}\n> +\t\t\telse\n> +\t\t\t\tcontinue;\n\nDrop needless braces around a single-statement block.\nAlternatively, we could write it like so:\n\n\t\t\tif (!do_nop)\n\t\t\t\tcontinue;\n\t\t\t/* fallthru */\n\nbut I think what you wrote, modulo the unnecessary braces, makes the\nintent a bit more clear.\n\n>  \t\tdefault:\n>  \t\t\t; /* do nothing */\n>  \t\t}\n"},{"id":"490680","messageId":"SA1PR14MB46916697B71E56C4AE9262358D282@SA1PR14MB4691.namprd14.prod.outlook.com","threadId":"61104","inReplyTo":"xmqqzfv14s8d.fsf@gitster.g","subject":"Re: [PATCH 1/2] Teach send-pack & push to --send-up-to-date refs","fromName":"Christopher Lindee","fromEmail":"christopher.lindee@webpros.com","sentAt":"2024-03-15T08:18:52Z","receivedAt":"2024-03-15T08:18:54Z","isPatch":true,"sender":{"key":"christopher.lindee@webpros.com","avatar":null},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Christopher Lindee <christopher.lindee@webpros.com> writes:\n> \n> > Subject: Re: [PATCH 1/2] Teach send-pack & push to --send-up-to-date refs\n> \n> cf. Documentation/SubmittingPatches:describe-changes.\n> \n> Perhaps\n> \n>         [PATCH 1/2] push: allow pushing a no-op update to refs\n> \n> or something?\n\nThanks!  I will change the commit message.\n\n> > Provide an option that forces the local Git transport to transmit a local\n> > ref even when the receiving ref would not change (i.e. the local & remote\n> > refs point to the same object).  This is not done normally, as no changes\n> > would take place on the server in a vanilla Git setup.  However, some Git\n> > servers support push options and those push options decide which branches\n> > to operate on based on the refs that are transmitted alongside the option.\n> > This option ensures the refs listed on the command line are always sent.\n> \n> Flip the order of description to give your observation of the\n> current behaviour first, your perceived problem in it next, and then\n> finally your solution.  That way, those who are totally unfamiliar\n> with the problem area can just read from start to end straight, and\n> those who know what we do currently can skip and start from your\n> problem description.\n\nCan do.  I wonder if the new message will come out justified too.\n\n> Also, it would be nice if you throw in the issue about missing ref\n> in the push certificate, which I mentioned in my response to the\n> cover letter.\n\nI'd like to get a better understanding of the feature before I comment\non push certificates.  I'll read up and see if I can include it.\n\n> >  Documentation/git-push.txt      | 8 +++++++-\n> >  Documentation/git-send-pack.txt | 7 +++++++\n> >  builtin/push.c                  | 1 +\n> >  builtin/send-pack.c             | 4 ++++\n> >  send-pack.c                     | 2 +-\n> >  send-pack.h                     | 3 ++-\n> >  transport-helper.c              | 7 ++++++-\n> >  transport.c                     | 1 +\n> >  transport.h                     | 1 +\n> >  9 files changed, 30 insertions(+), 4 deletions(-)\n> \n> No tests?\n\nOnly manual.  I'm happy to add automated tests; can you recommend a\nt####?\n\nI was also hesitant since I did not know what to expect and thought a\nwholesale redesign might be required; v2 will have less uncertainty.\n\n> >  'git push' [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]\n> >          [--repo=<repository>] [-f | --force] [-d | --delete] [--prune] [-q | --quiet] [-v | --verbose]\n> > -        [-u | --set-upstream] [-o <string> | --push-option=<string>]\n> > +        [-u | --set-upstream] [-o <string> | --push-option=<string>] [--send-up-to-date]\n> \n> I do not know if the new option is a name that is easy to\n> understand.  I am not great at naming, either but how does\n> \"--force-no-op\" sound?\n\nI already gave a response in a reply to the cover letter, but I also\nhave some new thoughts:\n\nIs this something that can be easy to understand?  It seems to require\nsome knowledge of transports and server behavior.  Moreover, with the\nserver-side implications mentioned by brian, we should encourage a\ndeeper understanding before people use this option.  In that case,\nperhaps a simple name is not as valuable.\n\nHow do people feel about --aggressive? ;)\n\nTo discourage unconsidered use, we could go with a scary option name,\nlike --disable-noop-optimizations, though that particular name has the\nunfortunate American vs. British spelling issue.  It does exactly what\nit says on the tin® though, which is nice.\n\nSee the cover letter reply for other suggestions.\n\n> > +     Usually, the command will not send a local ref when the remote ref\n> > +     already matches, as no change would occur if it did.  This flag\n> \n> In the context of \"push\", 'match' is a verb that is used in\n> different contexts, like \"'git push master' finds a ref that matches\n> 'master' and updates refs/heads/master\".  You would want to avoid it\n> when you can.\n> \n>     Usually the command omits instructing the receiving end to\n>     update a ref to point at an object, if the target ref points at\n>     the exact object already, as no change ...\n\nGood to know.  I will avoid overloading the term.\n\n> > +     disables that check.  This allows push options to be sent alongside\n> > +     up-to-date references on the remote.\n> \n> Aside from options and push certificates, there may be other side\n> effects from this change.  I am not sure if we want to make sure we\n> enumerate the benefit all like this.  Perhaps drop \"This allows ...\"\n> altogether?\n\nI think we both agree this is a option that is difficult to convey, so\nmuch so that I think we need examples.  How obvious would it be that I\nwanted to use this for push options if I didn't explicitly mention it?\n\nBut I also think you're right.  There are a decent number of things to\nconsider before using this.  Perhaps a fuller discussion on the option\n- with examples - is warranted in a more deep-dive document and we can\nrefer the user to that document here.  If so, would any existing pages\nbe appropriate?\n\n> > diff --git a/send-pack.c b/send-pack.c\n> > index 37f59d4f66..30b0f2f276 100644\n> > --- a/send-pack.c\n> > +++ b/send-pack.c\n> > @@ -313,7 +313,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar\n> >       case REF_STATUS_REJECT_NODELETE:\n> >               return CHECK_REF_STATUS_REJECTED;\n> >       case REF_STATUS_UPTODATE:\n> > -             return CHECK_REF_UPTODATE;\n> > +             return args->send_uptodate ? 0 : CHECK_REF_UPTODATE;\n> >\n> >       default:\n> >       case REF_STATUS_EXPECTING_REPORT:\n> \n> Given the existing flow of this code, I would prefer to see it\n> written more like so:\n> \n> diff --git i/send-pack.c w/send-pack.c\n> index 37f59d4f66..97d01726bb 100644\n> --- i/send-pack.c\n> +++ w/send-pack.c\n> @@ -313,8 +313,9 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar\n>         case REF_STATUS_REJECT_NODELETE:\n>                 return CHECK_REF_STATUS_REJECTED;\n>         case REF_STATUS_UPTODATE:\n> -               return CHECK_REF_UPTODATE;\n> -\n> +               if (!args->send_uptodate)\n> +                       return CHECK_REF_UPTODATE;\n> +               /* fallthru */\n>         default:\n>         case REF_STATUS_EXPECTING_REPORT:\n>                 /* already passed checks on the local side */\n> \n> to make it clear that the caller gets 0 (\"go ahead and do it\")\n> unless it is one of the cases explicitly listed abouve the \"default\"\n> label.\n\nNice; will do.\n\n> > +     int do_nop = flags & TRANSPORT_PUSH_SEND_UPTODATE;\n> \n> Here we do call it \"do_nop\", showing that at least to you, \"nop\" is\n> a much more fitting word than \"uptodate\" for what we are trying to\n> achieve in this topic.  It would give us one piece of good input to\n> decide what the end-user facing name should be.  In fact it is where\n> I took inspiration from for \"force-noop\" I mentioned earlier.\n>\n> > @@ -1010,7 +1011,11 @@ static int push_refs_with_push(struct transport *transport,\n> >                       } else\n> >                               continue;\n> >               case REF_STATUS_UPTODATE:\n> > -                     continue;\n> > +                     if (do_nop) {\n> > +                             ; /* do nothing */\n> > +                     }\n> > +                     else\n> > +                             continue;\n> \n> Drop needless braces around a single-statement block.\n> Alternatively, we could write it like so:\n> \n>                         if (!do_nop)\n>                                 continue;\n>                         /* fallthru */\n> \n> but I think what you wrote, modulo the unnecessary braces, makes the\n> intent a bit more clear.\n> \n> >               default:\n> >                       ; /* do nothing */\n> >               }\n\nI was having trouble coming up with a good name when developing this,\nso I went with something that was 6 characters long.  I'm open to\nidentifier suggestions that might be easier to read.\n\nThanks,\nChris."}]}