{"thread":{"id":"23035","subject":"t5505-remote.29 not working correctly","startedAt":"2010-03-16T01:12:36Z","lastAt":"2010-03-16T07:40:17Z","messageCount":4,"participants":["Brandon Casey","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"136911","messageId":"OGVB8d37MB-s4H3VOECmd2htUvpq3B4NginojN-XskLyvHmnbfGA4Q@cipher.nrlssc.navy.mil","threadId":"23035","inReplyTo":null,"subject":"t5505-remote.29 not working correctly","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-03-16T01:12:36Z","receivedAt":"2010-03-16T01:12:36Z","isPatch":false,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"\nJunio,\n\nI ran across this flaw while investigating something else.\n\nThe test titled 'remote prune to cause a dangling symref' is\nnot linked together with &&'s. When the &&'s are added, it\ndoes not complete successfully.\n\nHere's the copy&pasted diff which adds '&&' in two places:\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex a82c5ff..a3406dd 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -510,12 +510,12 @@ test_expect_success 'remote prune to cause a dangling symr\n ) 2>err &&\n grep \"has become dangling\" err &&\n \n- : And the dangling symref will not cause other annoying errors\n+ : And the dangling symref will not cause other annoying errors &&\n (\n cd seven &&\n git branch -a\n ) 2>err &&\n- ! grep \"points nowhere\" err\n+ ! grep \"points nowhere\" err &&\n (\n cd seven &&\n test_must_fail git branch nomore origin\n\n\nThe first grep causes the failure:\n\n ...\n (\n \tcd seven &&\n \tgit remote prune origin\n ) 2>err &&\n grep \"has become dangling\" err &&\n ...\n\n'git remote prune origin' is not printing \"has become dangling\".\n\n-brandon\n"},{"id":"136916","messageId":"7vwrxdueug.fsf@alter.siamese.dyndns.org","threadId":"23035","inReplyTo":"OGVB8d37MB-s4H3VOECmd2htUvpq3B4NginojN-XskLyvHmnbfGA4Q@cipher.nrlssc.navy.mil","subject":"Re: t5505-remote.29 not working correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-16T03:26:15Z","receivedAt":"2010-03-16T03:26:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> The test titled 'remote prune to cause a dangling symref' is\n> not linked together with &&'s. When the &&'s are added, it\n> does not complete successfully.\n\nHmm, it looks like f8948e2 (remote prune: warn dangling symrefs,\n2009-02-08) is internally inconsistent. This is a fix directly on top of\nthat commit.\n\nI haven't tried merging the result to a more recent codebase, though.\n\n-- >8 --\nSubject: [PATCH] warn_dangling_symref(): send the warning to standard error stream\n\nBrandon Casey noticed tht t5505 had accidentally broken its && chain,\nhiding inconsistency between the code that writes the warning to the\nstandard output and the test that expects to see the warning on the\nstandard error.\n\nSigned-off-by: Junio C Hamano \n---\n builtin-remote.c | 2 +-\n refs.c | 7 ++++---\n refs.h | 2 +-\n t/t5505-remote.sh | 2 +-\n 4 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex ac69d37..e67221d 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -789,7 +789,7 @@ static int prune(int argc, const char **argv)\n \n \t\t\tprintf(\" * [%s] %s\\n\", dry_run ? \"would prune\" : \"pruned\",\n \t\t\t abbrev_ref(refname, \"refs/remotes/\"));\n-\t\t\twarn_dangling_symref(dangling_msg, refname);\n+\t\t\twarn_dangling_symref(stderr, dangling_msg, refname);\n \t\t}\n \n \t\t/* NEEDSWORK: free remote */\ndiff --git a/refs.c b/refs.c\nindex 6eb5f53..a0e7da7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -286,6 +286,7 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)\n }\n \n struct warn_if_dangling_data {\n+\tFILE *fp;\n \tconst char *refname;\n \tconst char *msg_fmt;\n };\n@@ -304,13 +305,13 @@ static int warn_if_dangling_symref(const char *refname, const unsigned char *sha\n \tif (!resolves_to || strcmp(resolves_to, d->refname))\n \t\treturn 0;\n \n-\tprintf(d->msg_fmt, refname);\n+\tfprintf(d->fp, d->msg_fmt, refname);\n \treturn 0;\n }\n \n-void warn_dangling_symref(const char *msg_fmt, const char *refname)\n+void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)\n {\n-\tstruct warn_if_dangling_data data = { refname, msg_fmt };\n+\tstruct warn_if_dangling_data data = { fp, refname, msg_fmt };\n \tfor_each_rawref(warn_if_dangling_symref, &data);\n }\n \ndiff --git a/refs.h b/refs.h\nindex 29bdcec..64a8e62 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -27,7 +27,7 @@ extern int for_each_remote_ref(each_ref_fn, void *);\n /* can be used to learn about broken ref and symref */\n extern int for_each_rawref(each_ref_fn, void *);\n \n-extern void warn_dangling_symref(const char *msg_fmt, const char *refname);\n+extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);\n \n /*\n * Extra refs will be listed by for_each_ref() before any actual refs\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 2067dc5..2cad416 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -415,7 +415,7 @@ test_expect_success 'remote prune to cause a dangling symref' '\n \t) 2>err &&\n \tgrep \"has become dangling\" err &&\n \n-\t: And the dangling symref will not cause other annoying errors\n+\t: And the dangling symref will not cause other annoying errors &&\n \t(\n \t\tcd seven &&\n \t\tgit branch -a\n-- \n1.7.0.2.445.g0ae494\n"},{"id":"136922","messageId":"7v7hpcvobt.fsf@alter.siamese.dyndns.org","threadId":"23035","inReplyTo":"7vwrxdueug.fsf@alter.siamese.dyndns.org","subject":"Re: t5505-remote.29 not working correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-16T05:16:06Z","receivedAt":"2010-03-16T05:16:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Brandon Casey writes:\n>\n>> The test titled 'remote prune to cause a dangling symref' is\n>> not linked together with &&'s. When the &&'s are added, it\n>> does not complete successfully.\n>\n> Hmm, it looks like f8948e2 (remote prune: warn dangling symrefs,\n> 2009-02-08) is internally inconsistent. This is a fix directly on top of\n> that commit.\n\nActually this is a bit deeper than that. We don't get _any_ output to\neither standard output nor standard error with the recent code.\n\nIf we change the test to:\n\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex a82c5ff..ca88e29 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -507,8 +507,8 @@ test_expect_success 'remote prune to cause a dangling symref' '\n \t(\n \t\tcd seven &&\n \t\tgit remote prune origin\n-\t) 2>err &&\n-\tgrep \"has become dangling\" err &&\n+\t) >err 2>&1 &&\n+\tgrep \"has become dangling\" err || exit\n \n \t: And the dangling symref will not cause other annoying errors\n \t(\n\nso it does not matter whether the output is set to standard output or\nstandard error, we can bisect this, which points at eafb452 (do_one_ref():\nnull_sha1 check is not about broken ref, 2009-07-22).\n\nv1.6.3 and onwards seem to be broken.\n"},{"id":"136956","messageId":"7vsk80k93y.fsf_-_@alter.siamese.dyndns.org","threadId":"23035","inReplyTo":"7v7hpcvobt.fsf@alter.siamese.dyndns.org","subject":"[PATCH] refs: ref entry with NULL sha1 is can be a dangling symref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-16T07:40:17Z","receivedAt":"2010-03-16T07:40:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey noticed that t5505 had accidentally broken its && chain,\nhiding inconsistency between the code that writes the warning to the\nstandard output and the test that expects to see the warning on the\nstandard error, which was introduced by f8948e2 (remote prune: warn\ndangling symrefs, 2009-02-08).\n\nIt turns out that the issue is deeper than that. After f8948e2, a symref\nthat is dangling is marked with a NULL sha1, and the idea of using NULL\nsha1 to mean a deleted ref was scrapped, but somehow a follow-up eafb452\n(do_one_ref(): null_sha1 check is not about broken ref, 2009-07-22)\nincorrectly reorganized do_one_ref(), still thinking NULL sha1 is never\nused in the code.\n\nFix this by:\n\n - adopt Brandon's fix to t5505 test;\n\n - introduce REF_BROKEN flag to mark a ref that fails to resolve (dangling\n symref);\n\n - move the check for broken ref back inside the \"if we are skipping\n dangling refs\" code block.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I deliberately left hashclr() because other codepaths are likely to be\n using it as a signal that the ref is invalid. They can be lazily\n converted to pay attention to REF_BROKEN bit instead of doing 20-byte\n comparison to null SHA-1.\n\n refs.c | 11 +++++++----\n t/t5505-remote.sh | 6 +++---\n 2 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 63e30d7..a7518b6 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -6,6 +6,7 @@\n \n /* ISSYMREF=01 and ISPACKED=02 are public interfaces */\n #define REF_KNOWS_PEELED 04\n+#define REF_BROKEN 010\n \n struct ref_list {\n \tstruct ref_list *next;\n@@ -275,8 +276,10 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)\n \t\t\t\tlist = get_ref_dir(ref, list);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!resolve_ref(ref, sha1, 1, &flag))\n+\t\t\tif (!resolve_ref(ref, sha1, 1, &flag)) {\n \t\t\t\thashclr(sha1);\n+\t\t\t\tflag |= REF_BROKEN;\n+\t\t\t}\n \t\t\tlist = add_ref(ref, sha1, flag, list, NULL);\n \t\t}\n \t\tfree(ref);\n@@ -539,10 +542,10 @@ static int do_one_ref(const char *base, each_ref_fn fn, int trim,\n {\n \tif (strncmp(base, entry->name, trim))\n \t\treturn 0;\n-\t/* Is this a \"negative ref\" that represents a deleted ref? */\n-\tif (is_null_sha1(entry->sha1))\n-\t\treturn 0;\n+\n \tif (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) {\n+\t\tif (entry->flag & REF_BROKEN)\n+\t\t\treturn 0; /* ignore dangling symref */\n \t\tif (!has_sha1_file(entry->sha1)) {\n \t\t\terror(\"%s does not point to a valid object!\", entry->name);\n \t\t\treturn 0;\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex a82c5ff..2692050 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -507,15 +507,15 @@ test_expect_success 'remote prune to cause a dangling symref' '\n \t(\n \t\tcd seven &&\n \t\tgit remote prune origin\n-\t) 2>err &&\n+\t) >err 2>&1 &&\n \tgrep \"has become dangling\" err &&\n \n-\t: And the dangling symref will not cause other annoying errors\n+\t: And the dangling symref will not cause other annoying errors &&\n \t(\n \t\tcd seven &&\n \t\tgit branch -a\n \t) 2>err &&\n-\t! grep \"points nowhere\" err\n+\t! grep \"points nowhere\" err &&\n \t(\n \t\tcd seven &&\n \t\ttest_must_fail git branch nomore origin\n"}]}