{"thread":{"id":"29700","subject":"[PATCH 2/4] remote: reorganize check_pattern_match()","startedAt":"2012-02-22T22:43:37Z","lastAt":"2012-02-23T02:30:10Z","messageCount":9,"participants":["Felipe Contreras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"185218","messageId":"1329950621-21165-1-git-send-email-felipe.contreras@gmail.com","threadId":"29700","inReplyTo":null,"subject":"[PATCH 0/4] push: add 'prune' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-22T22:43:37Z","receivedAt":"2012-02-22T22:43:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nAs mentioned in a previous thread[1], git is lacking some functionality to\nsynchronize completely with remote repositories.\n\nAs an example I put my use-case; I want to backup *all* my local branches to a\npersonal repository, and I want to remove branches that I have removed from my\nlocal repository. git push personal 'refs/heads/*' mostly does the job, but it\ndoesn't remove anything, and that's where 'prune' comes from.\n\nDo not confuse the remote branches with the upstream ones; you could have two\nrepositories where you want to synchronize branches to:\n% git push --prune --force backup1 'refs/heads/*'\n% git push --prune --force backup2 'refs/heads/*'\n\nI still think a 'git remote sync' would be tremendously useful, but it can\nactually use this --prune option.\n\nCheers.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/184990\n\nSince RFC:\n\n - Most of comments addressed\n - Documentation and tests added\n\nFelipe Contreras (4):\n  remote: use a local variable in match_push_refs()\n  remote: reorganize check_pattern_match()\n  remote: refactor code into alloc_delete_ref()\n  push: add 'prune' option\n\n Documentation/git-push.txt |    9 +++-\n builtin/push.c             |    2 +\n remote.c                   |  107 ++++++++++++++++++++++++++++----------------\n remote.h                   |    3 +-\n t/t5516-fetch-push.sh      |   16 +++++++\n transport.c                |    2 +\n transport.h                |    1 +\n 7 files changed, 100 insertions(+), 40 deletions(-)\n\n-- \n1.7.9.1\n"},{"id":"185221","messageId":"1329950621-21165-2-git-send-email-felipe.contreras@gmail.com","threadId":"29700","inReplyTo":"1329950621-21165-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/4] remote: use a local variable in match_push_refs()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-22T22:43:38Z","receivedAt":"2012-02-22T22:43:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"So that we can reuse src later on. No functional changes.\n\nWill be useful in next patches.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n remote.c |   19 ++++++++++---------\n 1 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 73a3809..55d68d1 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1157,7 +1157,7 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \tint send_mirror = flags & MATCH_REFS_MIRROR;\n \tint errs;\n \tstatic const char *default_refspec[] = { \":\", NULL };\n-\tstruct ref **dst_tail = tail_ref(dst);\n+\tstruct ref *ref, **dst_tail = tail_ref(dst);\n \n \tif (!nr_refspec) {\n \t\tnr_refspec = 1;\n@@ -1167,14 +1167,14 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \terrs = match_explicit_refs(src, *dst, &dst_tail, rs, nr_refspec);\n \n \t/* pick the remainder */\n-\tfor ( ; src; src = src->next) {\n+\tfor (ref = src; ref; ref = ref->next) {\n \t\tstruct ref *dst_peer;\n \t\tconst struct refspec *pat = NULL;\n \t\tchar *dst_name;\n-\t\tif (src->peer_ref)\n+\t\tif (ref->peer_ref)\n \t\t\tcontinue;\n \n-\t\tpat = check_pattern_match(rs, nr_refspec, src);\n+\t\tpat = check_pattern_match(rs, nr_refspec, ref);\n \t\tif (!pat)\n \t\t\tcontinue;\n \n@@ -1184,13 +1184,14 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \t\t\t * including refs outside refs/heads/ hierarchy, but\n \t\t\t * that does not make much sense these days.\n \t\t\t */\n-\t\t\tif (!send_mirror && prefixcmp(src->name, \"refs/heads/\"))\n+\t\t\tif (!send_mirror && prefixcmp(ref->name, \"refs/heads/\"))\n \t\t\t\tcontinue;\n-\t\t\tdst_name = xstrdup(src->name);\n+\t\t\tdst_name = xstrdup(ref->name);\n+\n \n \t\t} else {\n \t\t\tconst char *dst_side = pat->dst ? pat->dst : pat->src;\n-\t\t\tif (!match_name_with_pattern(pat->src, src->name,\n+\t\t\tif (!match_name_with_pattern(pat->src, ref->name,\n \t\t\t\t\t\t     dst_side, &dst_name))\n \t\t\t\tdie(\"Didn't think it matches any more\");\n \t\t}\n@@ -1211,9 +1212,9 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \n \t\t\t/* Create a new one and link it */\n \t\t\tdst_peer = make_linked_ref(dst_name, &dst_tail);\n-\t\t\thashcpy(dst_peer->new_sha1, src->new_sha1);\n+\t\t\thashcpy(dst_peer->new_sha1, ref->new_sha1);\n \t\t}\n-\t\tdst_peer->peer_ref = copy_ref(src);\n+\t\tdst_peer->peer_ref = copy_ref(ref);\n \t\tdst_peer->force = pat->force;\n \tfree_name:\n \t\tfree(dst_name);\n-- \n1.7.9.1\n"},{"id":"185217","messageId":"1329950621-21165-3-git-send-email-felipe.contreras@gmail.com","threadId":"29700","inReplyTo":"1329950621-21165-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/4] remote: reorganize check_pattern_match()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-22T22:43:39Z","receivedAt":"2012-02-22T22:43:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's a lot of code that can be consolidated there, and will be useful\nfor next patches.\n\nRight now match_name_with_pattern() is called twice, the first one\nwithout value and result, and the second one with them. Since\ncheck_pattern_match() is only used in one place, we can just reorganize\nit to make a single call and fetch the values at the same time.\n\nSince this is a semantic change, also rename the function to\nget_ref_match() which actually describes more closely what it's actually\ndoing now.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n remote.c |   59 ++++++++++++++++++++++++++++++-----------------------------\n 1 files changed, 30 insertions(+), 29 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 55d68d1..9d29acc 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1110,10 +1110,11 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,\n \treturn errs;\n }\n \n-static const struct refspec *check_pattern_match(const struct refspec *rs,\n-\t\t\t\t\t\t int rs_nr,\n-\t\t\t\t\t\t const struct ref *src)\n+static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref *ref,\n+\t\tint send_mirror, const struct refspec **ret_pat)\n {\n+\tconst struct refspec *pat;\n+\tchar *name;\n \tint i;\n \tint matching_refs = -1;\n \tfor (i = 0; i < rs_nr; i++) {\n@@ -1123,14 +1124,31 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (rs[i].pattern && match_name_with_pattern(rs[i].src, src->name,\n-\t\t\t\t\t\t\t     NULL, NULL))\n-\t\t\treturn rs + i;\n+\t\tif (rs[i].pattern) {\n+\t\t\tconst char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;\n+\t\t\tif (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {\n+\t\t\t\tmatching_refs = i;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n \t}\n-\tif (matching_refs != -1)\n-\t\treturn rs + matching_refs;\n-\telse\n+\tif (matching_refs == -1)\n \t\treturn NULL;\n+\n+\tpat = rs + matching_refs;\n+\tif (pat->matching) {\n+\t\t/*\n+\t\t * \"matching refs\"; traditionally we pushed everything\n+\t\t * including refs outside refs/heads/ hierarchy, but\n+\t\t * that does not make much sense these days.\n+\t\t */\n+\t\tif (!send_mirror && prefixcmp(ref->name, \"refs/heads/\"))\n+\t\t\treturn NULL;\n+\t\tname = xstrdup(ref->name);\n+\t}\n+\tif (ret_pat)\n+\t\t*ret_pat = pat;\n+\treturn name;\n }\n \n static struct ref **tail_ref(struct ref **head)\n@@ -1171,36 +1189,19 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \t\tstruct ref *dst_peer;\n \t\tconst struct refspec *pat = NULL;\n \t\tchar *dst_name;\n+\n \t\tif (ref->peer_ref)\n \t\t\tcontinue;\n \n-\t\tpat = check_pattern_match(rs, nr_refspec, ref);\n-\t\tif (!pat)\n+\t\tdst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, &pat);\n+\t\tif (!dst_name)\n \t\t\tcontinue;\n \n-\t\tif (pat->matching) {\n-\t\t\t/*\n-\t\t\t * \"matching refs\"; traditionally we pushed everything\n-\t\t\t * including refs outside refs/heads/ hierarchy, but\n-\t\t\t * that does not make much sense these days.\n-\t\t\t */\n-\t\t\tif (!send_mirror && prefixcmp(ref->name, \"refs/heads/\"))\n-\t\t\t\tcontinue;\n-\t\t\tdst_name = xstrdup(ref->name);\n-\n-\n-\t\t} else {\n-\t\t\tconst char *dst_side = pat->dst ? pat->dst : pat->src;\n-\t\t\tif (!match_name_with_pattern(pat->src, ref->name,\n-\t\t\t\t\t\t     dst_side, &dst_name))\n-\t\t\t\tdie(\"Didn't think it matches any more\");\n-\t\t}\n \t\tdst_peer = find_ref_by_name(*dst, dst_name);\n \t\tif (dst_peer) {\n \t\t\tif (dst_peer->peer_ref)\n \t\t\t\t/* We're already sending something to this ref. */\n \t\t\t\tgoto free_name;\n-\n \t\t} else {\n \t\t\tif (pat->matching && !(send_all || send_mirror))\n \t\t\t\t/*\n-- \n1.7.9.1\n"},{"id":"185219","messageId":"1329950621-21165-4-git-send-email-felipe.contreras@gmail.com","threadId":"29700","inReplyTo":"1329950621-21165-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 3/4] remote: refactor code into alloc_delete_ref()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-22T22:43:40Z","receivedAt":"2012-02-22T22:43:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Will be useful in next patches. No functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n remote.c |   14 +++++++++-----\n 1 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 9d29acc..4709e20 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -978,16 +978,20 @@ static void tail_link_ref(struct ref *ref, struct ref ***tail)\n \t*tail = &ref->next;\n }\n \n+static struct ref *alloc_delete_ref(void)\n+{\n+\tstruct ref *ref = alloc_ref(\"(delete)\");\n+\thashclr(ref->new_sha1);\n+\treturn ref;\n+}\n+\n static struct ref *try_explicit_object_name(const char *name)\n {\n \tunsigned char sha1[20];\n \tstruct ref *ref;\n \n-\tif (!*name) {\n-\t\tref = alloc_ref(\"(delete)\");\n-\t\thashclr(ref->new_sha1);\n-\t\treturn ref;\n-\t}\n+\tif (!*name)\n+\t\treturn alloc_delete_ref();\n \tif (get_sha1(name, sha1))\n \t\treturn NULL;\n \tref = alloc_ref(name);\n-- \n1.7.9.1\n"},{"id":"185220","messageId":"1329950621-21165-5-git-send-email-felipe.contreras@gmail.com","threadId":"29700","inReplyTo":"1329950621-21165-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 4/4] push: add 'prune' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-22T22:43:41Z","receivedAt":"2012-02-22T22:43:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This will allow us to remove refs from the remote that have been removed\nlocally.\n\nIt's useful to conveniently synchronize all the local branches to\ncertain remote.\n\nUnfortunately, if we want to handle src:dst refspecs properly, some\nextra complexity is needed, so check_pattern_match() needs to be\nreorganized (previous patch).\n\nThis ensures that the following works:\n\n $ git push --prune remote refs/heads/*:refs/tmp/*\n\nSo that local branches are not only pushed as custom refs in the remote,\nbut also refs/tmp/foo would be removed if there's no corresponding local\nrefs/heads/foo.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-push.txt |    9 ++++++++-\n builtin/push.c             |    2 ++\n remote.c                   |   31 ++++++++++++++++++++++++++++---\n remote.h                   |    3 ++-\n t/t5516-fetch-push.sh      |   16 ++++++++++++++++\n transport.c                |    2 ++\n transport.h                |    1 +\n 7 files changed, 59 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex aede488..204f36d 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git push' [--all | --mirror | --tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]\n-\t   [--repo=<repository>] [-f | --force] [-v | --verbose] [-u | --set-upstream]\n+\t   [--repo=<repository>] [-f | --force] [--prune] [-v | --verbose] [-u | --set-upstream]\n \t   [<repository> [<refspec>...]]\n \n DESCRIPTION\n@@ -71,6 +71,13 @@ nor in any Push line of the corresponding remotes file---see below).\n \tInstead of naming each ref to push, specifies that all\n \trefs under `refs/heads/` be pushed.\n \n+--prune::\n+\tRemove remote branches that don't have a local counterpart. For example\n+\ta remote branch `tmp` will be removed if a local branch with the same\n+\tname doesn't exist any more. This also respects refspecs, e.g.\n+\t`refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`\n+\twill be removed if `refs/heads/foo` doesn't exist.\n+\n --mirror::\n \tInstead of naming each ref to push, specifies that all\n \trefs under `refs/` (which includes but is not\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 35cce53..fdfb2c4 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -261,6 +261,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('u', \"set-upstream\", &flags, \"set upstream for git pull/status\",\n \t\t\tTRANSPORT_PUSH_SET_UPSTREAM),\n \t\tOPT_BOOLEAN(0, \"progress\", &progress, \"force progress reporting\"),\n+\t\tOPT_BIT(0, \"prune\", &flags, \"prune locally removed refs\",\n+\t\t\tTRANSPORT_PUSH_PRUNE),\n \t\tOPT_END()\n \t};\n \ndiff --git a/remote.c b/remote.c\nindex 4709e20..9c590ae 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -8,6 +8,8 @@\n #include \"tag.h\"\n #include \"string-list.h\"\n \n+enum map_direction { FROM_SRC, FROM_DST };\n+\n static struct refspec s_tag_refspec = {\n \t0,\n \t1,\n@@ -1115,7 +1117,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,\n }\n \n static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref *ref,\n-\t\tint send_mirror, const struct refspec **ret_pat)\n+\t\tint send_mirror, int direction, const struct refspec **ret_pat)\n {\n \tconst struct refspec *pat;\n \tchar *name;\n@@ -1130,7 +1132,12 @@ static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref\n \n \t\tif (rs[i].pattern) {\n \t\t\tconst char *dst_side = rs[i].dst ? rs[i].dst : rs[i].src;\n-\t\t\tif (match_name_with_pattern(rs[i].src, ref->name, dst_side, &name)) {\n+\t\t\tint match;\n+\t\t\tif (direction == FROM_SRC)\n+\t\t\t\tmatch = match_name_with_pattern(rs[i].src, ref->name, dst_side, &name);\n+\t\t\telse\n+\t\t\t\tmatch = match_name_with_pattern(dst_side, ref->name, rs[i].src, &name);\n+\t\t\tif (match) {\n \t\t\t\tmatching_refs = i;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -1177,6 +1184,7 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \tstruct refspec *rs;\n \tint send_all = flags & MATCH_REFS_ALL;\n \tint send_mirror = flags & MATCH_REFS_MIRROR;\n+\tint send_prune = flags & MATCH_REFS_PRUNE;\n \tint errs;\n \tstatic const char *default_refspec[] = { \":\", NULL };\n \tstruct ref *ref, **dst_tail = tail_ref(dst);\n@@ -1197,7 +1205,7 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \t\tif (ref->peer_ref)\n \t\t\tcontinue;\n \n-\t\tdst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, &pat);\n+\t\tdst_name = get_ref_match(rs, nr_refspec, ref, send_mirror, FROM_SRC, &pat);\n \t\tif (!dst_name)\n \t\t\tcontinue;\n \n@@ -1224,6 +1232,23 @@ int match_push_refs(struct ref *src, struct ref **dst,\n \tfree_name:\n \t\tfree(dst_name);\n \t}\n+\tif (send_prune) {\n+\t\t/* check for missing refs on the remote */\n+\t\tfor (ref = *dst; ref; ref = ref->next) {\n+\t\t\tchar *src_name;\n+\n+\t\t\tif (ref->peer_ref)\n+\t\t\t\t/* We're already sending something to this ref. */\n+\t\t\t\tcontinue;\n+\n+\t\t\tsrc_name = get_ref_match(rs, nr_refspec, ref, send_mirror, FROM_DST, NULL);\n+\t\t\tif (src_name) {\n+\t\t\t\tif (!find_ref_by_name(src, src_name))\n+\t\t\t\t\tref->peer_ref = alloc_delete_ref();\n+\t\t\t\tfree(src_name);\n+\t\t\t}\n+\t\t}\n+\t}\n \tif (errs)\n \t\treturn -1;\n \treturn 0;\ndiff --git a/remote.h b/remote.h\nindex b395598..341142c 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);\n enum match_refs_flags {\n \tMATCH_REFS_NONE\t\t= 0,\n \tMATCH_REFS_ALL \t\t= (1 << 0),\n-\tMATCH_REFS_MIRROR\t= (1 << 1)\n+\tMATCH_REFS_MIRROR\t= (1 << 1),\n+\tMATCH_REFS_PRUNE\t= (1 << 2),\n };\n \n /* Reporting of tracking info */\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex b69cf57..b5417cc 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -979,4 +979,20 @@ test_expect_success 'push --porcelain --dry-run rejected' '\n \ttest_cmp .git/foo .git/bar\n '\n \n+test_expect_success 'push --prune' '\n+\tmk_test heads/master heads/second heads/foo heads/bar &&\n+\tgit push --prune testrepo &&\n+\tcheck_push_result $the_commit heads/master &&\n+\tcheck_push_result $the_first_commit heads/second &&\n+\t! check_push_result $the_first_commit heads/foo heads/bar\n+'\n+\n+test_expect_success 'push --prune refspec' '\n+\tmk_test tmp/master tmp/second tmp/foo tmp/bar &&\n+\tgit push --prune testrepo \"refs/heads/*:refs/tmp/*\" &&\n+\tcheck_push_result $the_commit tmp/master &&\n+\tcheck_push_result $the_first_commit tmp/second &&\n+\t! check_push_result $the_first_commit tmp/foo tmp/bar\n+'\n+\n test_done\ndiff --git a/transport.c b/transport.c\nindex cac0c06..c20267c 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1028,6 +1028,8 @@ int transport_push(struct transport *transport,\n \t\t\tmatch_flags |= MATCH_REFS_ALL;\n \t\tif (flags & TRANSPORT_PUSH_MIRROR)\n \t\t\tmatch_flags |= MATCH_REFS_MIRROR;\n+\t\tif (flags & TRANSPORT_PUSH_PRUNE)\n+\t\t\tmatch_flags |= MATCH_REFS_PRUNE;\n \n \t\tif (match_push_refs(local_refs, &remote_refs,\n \t\t\t\t    refspec_nr, refspec, match_flags)) {\ndiff --git a/transport.h b/transport.h\nindex 059b330..ce99ef8 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -102,6 +102,7 @@ struct transport {\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 \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n \n-- \n1.7.9.1\n"},{"id":"185229","messageId":"7v1upm8jnq.fsf@alter.siamese.dyndns.org","threadId":"29700","inReplyTo":"1329950621-21165-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 4/4] push: add 'prune' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-23T00:42:33Z","receivedAt":"2012-02-23T00:42:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> +--prune::\n> +\tRemove remote branches that don't have a local counterpart. For example\n> +\ta remote branch `tmp` will be removed if a local branch with the same\n> +\tname doesn't exist any more. This also respects refspecs, e.g.\n> +\t`refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`\n> +\twill be removed if `refs/heads/foo` doesn't exist.\n\nI do not think it adds much useful information to mention `tmp` only once\nover what is already said by the first sentence.  Also, the first sentence\nof the example does not make it clear that it is assuming a same-for-same\nmapping.\n\nComing up with a precise technical description is easy, but it is hard to\nexplain to the end user in easy terms, and I commend you for attempting to\nadd an example in a short single sentence, though.\n\nPerhaps spelling out the underlying assumption the example makes is the\nbest we could do here without going too technical.\n\n        ... For example, if you are pushing all your local branches to\n        update the local branches of the remote, `tmp` branch will be\n        removed from the remote if you removed your `tmp` branch locally.\n\n        If you are pushing all your local branches on your laptop to a\n        repository on your desktop machine under `refs/remotes/laptop/`\n        hierarchy to back them up, `refs/remotes/laptop/tmp` is removed\n        from the remote if you no longer have the branch `tmp` on your\n        laptop.\n\n\nWill queue with a slight fix-ups, including this bit:\n\n> diff --git a/remote.h b/remote.h\n> index b395598..341142c 100644\n> --- a/remote.h\n> +++ b/remote.h\n> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);\n>  enum match_refs_flags {\n>  \tMATCH_REFS_NONE\t\t= 0,\n>  \tMATCH_REFS_ALL \t\t= (1 << 0),\n> -\tMATCH_REFS_MIRROR\t= (1 << 1)\n> +\tMATCH_REFS_MIRROR\t= (1 << 1),\n> +\tMATCH_REFS_PRUNE\t= (1 << 2),\n>  };\n\nLose the ',' at the end, for the same reason why deleted line did not have\none.\n\nThanks.\n"},{"id":"185233","messageId":"CAMP44s1s_NeXeUpxDR0cLoLGjSaf-0E_62MzAxeiHS=-6c045A@mail.gmail.com","threadId":"29700","inReplyTo":"7v1upm8jnq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] push: add 'prune' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-23T01:09:00Z","receivedAt":"2012-02-23T01:09:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> +--prune::\n>> +     Remove remote branches that don't have a local counterpart. For example\n>> +     a remote branch `tmp` will be removed if a local branch with the same\n>> +     name doesn't exist any more. This also respects refspecs, e.g.\n>> +     `refs/heads/*:refs/tmp/*` would make sure that remote `refs/tmp/foo`\n>> +     will be removed if `refs/heads/foo` doesn't exist.\n>\n> I do not think it adds much useful information to mention `tmp` only once\n> over what is already said by the first sentence.  Also, the first sentence\n> of the example does not make it clear that it is assuming a same-for-same\n> mapping.\n\nSure, the first sentence doesn't make it clear, but it would be a\nvalid and obvious assumption. The second sentence makes it clear, and\nthe name `tmp` immediately evokes a branch that will probably be\nremoved.\n\n> Coming up with a precise technical description is easy, but it is hard to\n> explain to the end user in easy terms, and I commend you for attempting to\n> add an example in a short single sentence, though.\n>\n> Perhaps spelling out the underlying assumption the example makes is the\n> best we could do here without going too technical.\n>\n>        ... For example, if you are pushing all your local branches to\n>        update the local branches of the remote,\n\nYeah, but that's 'git push --all', and that's not the common\noperation--'git push' is. So that's what I presumed the reader would\nassume, and it really doesn't make a difference as to what will\nfollow:\n\n>        `tmp` branch will be\n>        removed from the remote if you removed your `tmp` branch locally.\n\nThis reuses the name `tmp`, which seems to be your objective, but it\ndoesn't explain _why_ it would remove `tmp`; is it because `tmp` is\nthe upstream branch, or is it because it has the same name?\n\n>        If you are pushing all your local branches on your laptop to a\n>        repository on your desktop machine under `refs/remotes/laptop/`\n>        hierarchy to back them up, `refs/remotes/laptop/tmp` is removed\n>        from the remote if you no longer have the branch `tmp` on your\n>        laptop.\n\nUnfortunately, I as a reader have trouble understanding this. More\nspecifically I have trouble understanding where `refs/remotes/laptop/`\nis coming from, and what it is meaning. I have always pictured\n`refs/remotes` as something that 'git fetch' updates, and always from\nthe relevant repository. While eventually I could understand what this\nthing is doing, it took me more than one read, and I had to read\nslowly, and even then it seems completely non-standard to me.\n\nI think a synthetic example, like `refs/heads/*:refs/tmp/*`, is much\neasier to understand because it doesn't mess up with any established\nrefs, and also has the advantage that it shows the relevant refspec,\nwhich is useful for people that are not familiar with refspecs, and in\nfact, people could try it out without messing with their current refs.\n\n>> diff --git a/remote.h b/remote.h\n>> index b395598..341142c 100644\n>> --- a/remote.h\n>> +++ b/remote.h\n>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);\n>>  enum match_refs_flags {\n>>       MATCH_REFS_NONE         = 0,\n>>       MATCH_REFS_ALL          = (1 << 0),\n>> -     MATCH_REFS_MIRROR       = (1 << 1)\n>> +     MATCH_REFS_MIRROR       = (1 << 1),\n>> +     MATCH_REFS_PRUNE        = (1 << 2),\n>>  };\n>\n> Lose the ',' at the end, for the same reason why deleted line did not have\n> one.\n\nAnd why is that? Isn't git c99? That comma would only ensure that the\nnext patch that touches this would be two lines instead of one.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"185235","messageId":"7vlinu72ui.fsf@alter.siamese.dyndns.org","threadId":"29700","inReplyTo":"CAMP44s1s_NeXeUpxDR0cLoLGjSaf-0E_62MzAxeiHS=-6c045A@mail.gmail.com","subject":"Re: [PATCH 4/4] push: add 'prune' option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-23T01:31:01Z","receivedAt":"2012-02-23T01:31:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Yeah, but that's 'git push --all', and that's not the common\n> operation--'git push' is.\n\n\"git push\" is common but that does not give you a solid base to guess what\nthe reader's assumption would be.  Are you assuming \"matching\" semantics?\n\n> So that's what I presumed the reader would\n> assume,...\n\nI do not want let us guess what the reader assumes, as many people seem to\nsuggest setting push.default to different values and that would change\nwhat the reader would assume.  That was the whole reason that I suggested\nto spell the assumption out, so that the reader's assumption does not have\nto get into the picture.\n\n> This reuses the name `tmp`, which seems to be your objective, but it\n> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is\n> the upstream branch, or is it because it has the same name?\n\nThe example is to clarify \"local counterpart\" in the main text.  I\nactually would prefer to get rid of `tmp` but I left it as-is as you\nwrote.  The exact name used in the example does not matter, whether it is\n`tmp` or `xyzzy`.\n\n> Unfortunately, I as a reader have trouble understanding this. More\n> specifically I have trouble understanding where `refs/remotes/laptop/`\n> is coming from, and what it is meaning. I have always pictured\n> `refs/remotes` as something that 'git fetch' updates, and always from\n> the relevant repository.\n\nThe layout is the recommended set-up to emulate a fetch with a push in the\nreverse direction, which I thought anybody should notice.  It is a failure\nin our documentation that even an expert didn't.\n\n>>> diff --git a/remote.h b/remote.h\n>>> index b395598..341142c 100644\n>>> --- a/remote.h\n>>> +++ b/remote.h\n>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);\n>>>  enum match_refs_flags {\n>>>       MATCH_REFS_NONE         = 0,\n>>>       MATCH_REFS_ALL          = (1 << 0),\n>>> -     MATCH_REFS_MIRROR       = (1 << 1)\n>>> +     MATCH_REFS_MIRROR       = (1 << 1),\n>>> +     MATCH_REFS_PRUNE        = (1 << 2),\n>>>  };\n>>\n>> Lose the ',' at the end, for the same reason why deleted line did not have\n>> one.\n>\n> And why is that?\n\nBecause I told you so ;-).\n\nMore seriously, we have had patches to accomodate other people's compilers\nby dropping the last comma in enum {}.  See c9b6782 (enums: omit trailing\ncomma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma\nfor portability, 2010-05-14) for examples.\n"},{"id":"185236","messageId":"CAMP44s1i6LaXgfbMfx+hhE20NbaBZ+npAZXNSUb79nRkKzpVWg@mail.gmail.com","threadId":"29700","inReplyTo":"7vlinu72ui.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] push: add 'prune' option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-02-23T02:30:10Z","receivedAt":"2012-02-23T02:30:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Feb 23, 2012 at 3:31 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Thu, Feb 23, 2012 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Yeah, but that's 'git push --all', and that's not the common\n>> operation--'git push' is.\n>\n> \"git push\" is common but that does not give you a solid base to guess what\n> the reader's assumption would be.  Are you assuming \"matching\" semantics?\n\nBut if you want to spell the assumption, why not spell the most common one?\n\n>> So that's what I presumed the reader would\n>> assume,...\n>\n> I do not want let us guess what the reader assumes, as many people seem to\n> suggest setting push.default to different values and that would change\n> what the reader would assume.\n\nNo, it wouldn't. Almost all readers are familiar about what 'git push'\ndoes by default--I would even adventure to say all--the ones that set\na custom push.default would do it because they _know_ they don't want\nthe default.\n\nAnd in fact, as I said, it doesn't matter what the reader has set in\npush.default, or if the reader is assuming 'git push --all', or 'git\npush :', or 'git push --tags', or 'git push refs/heads/*', the end\nresult is the same.\n\n> That was the whole reason that I suggested\n> to spell the assumption out, so that the reader's assumption does not have\n> to get into the picture.\n\nI don't think it matters what the reader assumes, because the end\nresult is the same, so there's no reason to spell any assumption.\n\n>> This reuses the name `tmp`, which seems to be your objective, but it\n>> doesn't explain _why_ it would remove `tmp`; is it because `tmp` is\n>> the upstream branch, or is it because it has the same name?\n>\n> The example is to clarify \"local counterpart\" in the main text.  I\n> actually would prefer to get rid of `tmp` but I left it as-is as you\n> wrote.  The exact name used in the example does not matter, whether it is\n> `tmp` or `xyzzy`.\n\nI believe `tmp` is more useful than `xyzzy`, or even more than\n`master`. I already explained why.\n\n>> Unfortunately, I as a reader have trouble understanding this. More\n>> specifically I have trouble understanding where `refs/remotes/laptop/`\n>> is coming from, and what it is meaning. I have always pictured\n>> `refs/remotes` as something that 'git fetch' updates, and always from\n>> the relevant repository.\n>\n> The layout is the recommended set-up to emulate a fetch with a push in the\n> reverse direction, which I thought anybody should notice.  It is a failure\n> in our documentation that even an expert didn't.\n\nThe problem is not figuring out the `refs/remotes/foo`, but picturing\nthe fact that there are two repositories, the user ran on one of them\n'git remote add laptop', and then 'git remote add pc', and then 'git\npush --prune refs/heads*:refs/remotes/laptop/*'. Lots of things to\nkeep in mind for this paragraph. And, as you say, it's the reverse\ndirection of a fetch, so it's immediately weird.\n\nMoreover, I find it strange that you want to rely on the assumption\nthat the reader is familiar with the pattern `refs/remotes/foo`, and\nthe refspec syntax, which is pretty deep plumbing, and not on the\ndefault action of 'git push', which is high-level porcelain, and as I\nsaid, not even needed to understand my example.\n\n>>>> diff --git a/remote.h b/remote.h\n>>>> index b395598..341142c 100644\n>>>> --- a/remote.h\n>>>> +++ b/remote.h\n>>>> @@ -145,7 +145,8 @@ int branch_merge_matches(struct branch *, int n, const char *);\n>>>>  enum match_refs_flags {\n>>>>       MATCH_REFS_NONE         = 0,\n>>>>       MATCH_REFS_ALL          = (1 << 0),\n>>>> -     MATCH_REFS_MIRROR       = (1 << 1)\n>>>> +     MATCH_REFS_MIRROR       = (1 << 1),\n>>>> +     MATCH_REFS_PRUNE        = (1 << 2),\n>>>>  };\n>>>\n>>> Lose the ',' at the end, for the same reason why deleted line did not have\n>>> one.\n>>\n>> And why is that?\n>\n> Because I told you so ;-).\n\nYou didn't tell me anything about the previous line :)\n\n> More seriously, we have had patches to accomodate other people's compilers\n> by dropping the last comma in enum {}.  See c9b6782 (enums: omit trailing\n> comma for portability, 2011-03-16), 4b05548 (enums: omit trailing comma\n> for portability, 2010-05-14) for examples.\n\nYeah, I remembered those patches after sending the mail. Shame. Maybe\nnext decade =/\n\nCheers.\n-- \nFelipe Contreras\n"}]}