{"thread":{"id":"44736","subject":"[PATCH 0/3] push only submodules","startedAt":"2016-12-19T18:26:42Z","lastAt":"2016-12-20T19:26:20Z","messageCount":7,"participants":["Brandon Williams","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"308073","messageId":"1482171933-180601-1-git-send-email-bmwill@google.com","threadId":"44736","inReplyTo":null,"subject":"[PATCH 0/3] push only submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-19T18:25:30Z","receivedAt":"2016-12-19T18:26:42Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"This series teaches 'git push' to be able to only push submodules while leaving\na superproject unpushed.\n\nThis is a desirable feature in a scenario where updates to the\nsuperproject are handled automatically by some other means, perhaps a\ncode review tool.  In this scenario a developer could make a change\nwhich spans multiple submodules and then push their commits for code\nreview.  Upon completion of the code review, their commits can be\naccepted and applied to their respective submodules while the code\nreview tool can then automatically update the superproject to the most\nrecent SHA1 of each submodule.  This would eliminate the merge conflicts\nin the superproject that could occur if multiple people are contributing\nto the same submodule.\n\nBrandon Williams (3):\n  transport: refactor flag #defines to be more readable\n  submodules: add RECURSE_SUBMODULES_ONLY value\n  push: add option to push only submodules\n\n builtin/push.c                 |  2 ++\n submodule-config.c             |  2 ++\n submodule.h                    |  1 +\n t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++\n transport.c                    | 15 +++++++++++----\n transport.h                    | 31 ++++++++++++++++---------------\n 6 files changed, 53 insertions(+), 19 deletions(-)\n\n-- \n2.8.0.rc3.226.g39d4020\n\n"},{"id":"308074","messageId":"1482171933-180601-2-git-send-email-bmwill@google.com","threadId":"44736","inReplyTo":"1482171933-180601-1-git-send-email-bmwill@google.com","subject":"[PATCH 1/3] transport: refactor flag #defines to be more readable","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-19T18:25:31Z","receivedAt":"2016-12-19T18:26:46Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"All of the #defines for the TRANSPORT_* flags are hardcoded to be powers\nof two.  This can be error prone when adding a new flag and is difficult\nto read.  This patch refactors these defines to instead use a shift\noperation to generate the flags.  This makes adding an additional flag\nsimpilar and makes the defines easier to read.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n transport.h | 30 +++++++++++++++---------------\n 1 file changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/transport.h b/transport.h\nindex b8e4ee8..1b65458 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -131,21 +131,21 @@ struct transport {\n \tenum transport_family family;\n };\n \n-#define TRANSPORT_PUSH_ALL 1\n-#define TRANSPORT_PUSH_FORCE 2\n-#define TRANSPORT_PUSH_DRY_RUN 4\n-#define TRANSPORT_PUSH_MIRROR 8\n-#define TRANSPORT_PUSH_PORCELAIN 16\n-#define TRANSPORT_PUSH_SET_UPSTREAM 32\n-#define TRANSPORT_RECURSE_SUBMODULES_CHECK 64\n-#define TRANSPORT_PUSH_PRUNE 128\n-#define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n-#define TRANSPORT_PUSH_NO_HOOK 512\n-#define TRANSPORT_PUSH_FOLLOW_TAGS 1024\n-#define TRANSPORT_PUSH_CERT_ALWAYS 2048\n-#define TRANSPORT_PUSH_CERT_IF_ASKED 4096\n-#define TRANSPORT_PUSH_ATOMIC 8192\n-#define TRANSPORT_PUSH_OPTIONS 16384\n+#define TRANSPORT_PUSH_ALL\t\t\t(1<<0)\n+#define TRANSPORT_PUSH_FORCE\t\t\t(1<<1)\n+#define TRANSPORT_PUSH_DRY_RUN\t\t\t(1<<2)\n+#define TRANSPORT_PUSH_MIRROR\t\t\t(1<<3)\n+#define TRANSPORT_PUSH_PORCELAIN\t\t(1<<4)\n+#define TRANSPORT_PUSH_SET_UPSTREAM\t\t(1<<5)\n+#define TRANSPORT_RECURSE_SUBMODULES_CHECK\t(1<<6)\n+#define TRANSPORT_PUSH_PRUNE\t\t\t(1<<7)\n+#define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND\t(1<<8)\n+#define TRANSPORT_PUSH_NO_HOOK\t\t\t(1<<9)\n+#define TRANSPORT_PUSH_FOLLOW_TAGS\t\t(1<<10)\n+#define TRANSPORT_PUSH_CERT_ALWAYS\t\t(1<<11)\n+#define TRANSPORT_PUSH_CERT_IF_ASKED\t\t(1<<12)\n+#define TRANSPORT_PUSH_ATOMIC\t\t\t(1<<13)\n+#define TRANSPORT_PUSH_OPTIONS\t\t\t(1<<14)\n \n extern int transport_summary_width(const struct ref *refs);\n \n-- \n2.8.0.rc3.226.g39d4020\n\n"},{"id":"308075","messageId":"1482171933-180601-3-git-send-email-bmwill@google.com","threadId":"44736","inReplyTo":"1482171933-180601-1-git-send-email-bmwill@google.com","subject":"[PATCH 2/3] submodules: add RECURSE_SUBMODULES_ONLY value","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-19T18:25:32Z","receivedAt":"2016-12-19T18:27:37Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Add the `RECURSE_SUBMODULES_ONLY` enum value to submodule.h.  This enum\nvalue will be used in a later patch to push to indicate that only\nsubmodules should be pushed, while the superproject should remain\nunpushed.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n submodule-config.c | 2 ++\n submodule.h        | 1 +\n 2 files changed, 3 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 098085b..33eb62d 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -251,6 +251,8 @@ static int parse_push_recurse(const char *opt, const char *arg,\n \t\t\treturn RECURSE_SUBMODULES_ON_DEMAND;\n \t\telse if (!strcmp(arg, \"check\"))\n \t\t\treturn RECURSE_SUBMODULES_CHECK;\n+\t\telse if (!strcmp(arg, \"only\"))\n+\t\t\treturn RECURSE_SUBMODULES_ONLY;\n \t\telse if (die_on_error)\n \t\t\tdie(\"bad %s argument: %s\", opt, arg);\n \t\telse\ndiff --git a/submodule.h b/submodule.h\nindex 23d7668..4a83a4c 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -6,6 +6,7 @@ struct argv_array;\n struct sha1_array;\n \n enum {\n+\tRECURSE_SUBMODULES_ONLY = -5,\n \tRECURSE_SUBMODULES_CHECK = -4,\n \tRECURSE_SUBMODULES_ERROR = -3,\n \tRECURSE_SUBMODULES_NONE = -2,\n-- \n2.8.0.rc3.226.g39d4020\n\n"},{"id":"308076","messageId":"1482171933-180601-4-git-send-email-bmwill@google.com","threadId":"44736","inReplyTo":"1482171933-180601-1-git-send-email-bmwill@google.com","subject":"[PATCH 3/3] push: add option to push only submodules","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-19T18:25:33Z","receivedAt":"2016-12-19T18:27:40Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"Teach push the --recurse-submodules=only option.  This enables push to\nrecursively push all unpushed submodules while leaving the superproject\nunpushed.\n\nThis is a desirable feature in a scenario where updates to the\nsuperproject are handled automatically by some other means, perhaps a\ncode review tool.  In this scenario a developer could make a change\nwhich spans multiple submodules and then push their commits for code\nreview.  Upon completion of the code review, their commits can be\naccepted and applied to their respective submodules while the code\nreview tool can then automatically update the superproject to the most\nrecent SHA1 of each submodule.  This would eliminate the merge conflicts\nin the superproject that could occur if multiple people are contributing\nto the same submodule.\n\nSigned-off-by: Brandon Williams <bmwill@google.com>\n---\n builtin/push.c                 |  2 ++\n t/t5531-deep-submodule-push.sh | 21 +++++++++++++++++++++\n transport.c                    | 15 +++++++++++----\n transport.h                    |  1 +\n 4 files changed, 35 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 3bb9d6b..9433797 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -565,6 +565,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tflags |= TRANSPORT_RECURSE_SUBMODULES_CHECK;\n \telse if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)\n \t\tflags |= TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND;\n+\telse if (recurse_submodules == RECURSE_SUBMODULES_ONLY)\n+\t\tflags |= TRANSPORT_RECURSE_SUBMODULES_ONLY;\n \n \tif (tags)\n \t\tadd_refspec(\"refs/tags/*\");\ndiff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh\nindex 1524ff5..676d686 100755\n--- a/t/t5531-deep-submodule-push.sh\n+++ b/t/t5531-deep-submodule-push.sh\n@@ -454,4 +454,25 @@ test_expect_success 'push --dry-run does not recursively update submodules' '\n \ttest_cmp expected_submodule actual_submodule\n '\n \n+test_expect_success 'push --dry-run does not recursively update submodules' '\n+\tgit -C work push --dry-run --recurse-submodules=only ../pub.git master &&\n+\n+\tgit -C submodule.git rev-parse master >actual_submodule &&\n+\tgit -C pub.git rev-parse master >actual_pub &&\n+\ttest_cmp expected_pub actual_pub &&\n+\ttest_cmp expected_submodule actual_submodule\n+'\n+\n+test_expect_success 'push only unpushed submodules recursively' '\n+\tgit -C pub.git rev-parse master >expected_pub &&\n+\tgit -C work/gar/bage rev-parse master >expected_submodule &&\n+\n+\tgit -C work push --recurse-submodules=only ../pub.git master &&\n+\n+\tgit -C submodule.git rev-parse master >actual_submodule &&\n+\tgit -C pub.git rev-parse master >actual_pub &&\n+\ttest_cmp expected_submodule actual_submodule &&\n+\ttest_cmp expected_pub actual_pub\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex 04e5d66..20ebee8 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -947,7 +947,9 @@ int transport_push(struct transport *transport,\n \t\t\tif (run_pre_push_hook(transport, remote_refs))\n \t\t\t\treturn -1;\n \n-\t\tif ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {\n+\t\tif ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |\n+\t\t\t      TRANSPORT_RECURSE_SUBMODULES_ONLY)) &&\n+\t\t    !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n \t\t\tstruct sha1_array commits = SHA1_ARRAY_INIT;\n \n@@ -965,7 +967,8 @@ int transport_push(struct transport *transport,\n \t\t}\n \n \t\tif (((flags & TRANSPORT_RECURSE_SUBMODULES_CHECK) ||\n-\t\t     ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) &&\n+\t\t     ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |\n+\t\t\t\tTRANSPORT_RECURSE_SUBMODULES_ONLY)) &&\n \t\t      !pretend)) && !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n \t\t\tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n@@ -984,7 +987,10 @@ int transport_push(struct transport *transport,\n \t\t\tsha1_array_clear(&commits);\n \t\t}\n \n-\t\tpush_ret = transport->push_refs(transport, remote_refs, flags);\n+\t\tif (!(flags & TRANSPORT_RECURSE_SUBMODULES_ONLY))\n+\t\t\tpush_ret = transport->push_refs(transport, remote_refs, flags);\n+\t\telse\n+\t\t\tpush_ret = 0;\n \t\terr = push_had_errors(remote_refs);\n \t\tret = push_ret | err;\n \n@@ -996,7 +1002,8 @@ int transport_push(struct transport *transport,\n \t\tif (flags & TRANSPORT_PUSH_SET_UPSTREAM)\n \t\t\tset_upstreams(transport, remote_refs, pretend);\n \n-\t\tif (!(flags & TRANSPORT_PUSH_DRY_RUN)) {\n+\t\tif (!(flags & (TRANSPORT_PUSH_DRY_RUN |\n+\t\t\t       TRANSPORT_RECURSE_SUBMODULES_ONLY))) {\n \t\t\tstruct ref *ref;\n \t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n \t\t\t\ttransport_update_tracking_ref(transport->remote, ref, verbose);\ndiff --git a/transport.h b/transport.h\nindex 1b65458..efd5fb6 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -146,6 +146,7 @@ struct transport {\n #define TRANSPORT_PUSH_CERT_IF_ASKED\t\t(1<<12)\n #define TRANSPORT_PUSH_ATOMIC\t\t\t(1<<13)\n #define TRANSPORT_PUSH_OPTIONS\t\t\t(1<<14)\n+#define TRANSPORT_RECURSE_SUBMODULES_ONLY\t(1<<15)\n \n extern int transport_summary_width(const struct ref *refs);\n \n-- \n2.8.0.rc3.226.g39d4020\n\n"},{"id":"308078","messageId":"xmqqd1gnpxr7.fsf@gitster.mtv.corp.google.com","threadId":"44736","inReplyTo":"1482171933-180601-1-git-send-email-bmwill@google.com","subject":"Re: [PATCH 0/3] push only submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-19T18:41:32Z","receivedAt":"2016-12-19T18:42:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams <bmwill@google.com> writes:\n\n> This series teaches 'git push' to be able to only push submodules\n> while leaving a superproject unpushed.\n> ...\n>  builtin/push.c                 |  2 ++\n\nMy knee-jerk reaction is \"why is this even part of 'git push' if it\ndoes not push?\" \n\nI think \"git submodule foreach git push\" is probably a mouthful to\nsay, but I am not sure \"git push --recurse-submodule=only\" is a\nshort-hand that is way better than that.  \n\nMaybe I'll find why the latter is better after reading the patches\nthrough ;-)\n\n\n\n\n"},{"id":"308081","messageId":"CAGZ79kbc_o-B4nX8T1WN4z2PrpSHmrFBskeRA1rbdkzdnv2okw@mail.gmail.com","threadId":"44736","inReplyTo":"1482171933-180601-4-git-send-email-bmwill@google.com","subject":"Re: [PATCH 3/3] push: add option to push only submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-19T18:56:14Z","receivedAt":"2016-12-19T18:57:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Dec 19, 2016 at 10:25 AM, Brandon Williams <bmwill@google.com> wrote:\n> Teach push the --recurse-submodules=only option.  This enables push to\n> recursively push all unpushed submodules while leaving the superproject\n> unpushed.\n>\n> This is a desirable feature in a scenario where updates to the\n> superproject are handled automatically by some other means, perhaps a\n\ne.g. Gerrit. (No need to be shy about our code review tool)\n\n>\n> code review tool.  In this scenario a developer could make a change\n> which spans multiple submodules and then push their commits for code\n> review.  Upon completion of the code review, their commits can be\n> accepted and applied to their respective submodules while the code\n> review tool can then automatically update the superproject to the most\n> recent SHA1 of each submodule.  This would eliminate the merge conflicts\n> in the superproject that could occur if multiple people are contributing\n> to the same submodule.\n\nCode and tests look good to me, I think the commit message\nis good enough, but let's hear Junio on this one.\n\nThanks,\nStefan\n"},{"id":"308164","messageId":"xmqqr352jtc0.fsf@gitster.mtv.corp.google.com","threadId":"44736","inReplyTo":"1482171933-180601-1-git-send-email-bmwill@google.com","subject":"Re: [PATCH 0/3] push only submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-20T19:25:51Z","receivedAt":"2016-12-20T19:26:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Williams <bmwill@google.com> writes:\n\n> This series teaches 'git push' to be able to only push submodules\n> while leaving a superproject unpushed.\n\nIt somehow feels a bit strange to single out the top-level as\nspecial like this one (iow, shouldn't it be equally as easy to push\nout the superproject and two submodules leaving one submodule alone,\nas to push out these three submodules leaving the superproject\nalone?), but I will queue these for now with tweaks to address a few\nminor points.\n\n * What is done with 1/3 I do not think is \"refactor\".\n\n * \"perhaps a code review tool\" -> \"perhaps a tool like Gerrit code\n   review\" as suggested by Stefan.\n\n * Swap the order expected_pub and expected_submodule are generated\n   to match the order they are used for final verification in the\n   test.\n\n\n\n"}]}