{"thread":{"id":"35125","subject":"[PATCH v4 1/2] version-gen: cleanup","startedAt":"2013-10-12T07:07:06Z","lastAt":"2013-10-14T05:29:23Z","messageCount":7,"participants":["Felipe Contreras","David Aguilar","Jonathan Nieder"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"228864","messageId":"1381561628-20665-1-git-send-email-felipe.contreras@gmail.com","threadId":"35125","inReplyTo":null,"subject":"[PATCH v4 0/2] Version fixes and cleanups","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-12T07:07:06Z","receivedAt":"2013-10-12T07:07:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Chances since v3:\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex 19902e9..c04c4de 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -26,8 +26,10 @@ describe () {\n if test -f version\n then\n        VN=$(cat version) || VN=\"$DEF_VER\"\n-elif test ! -d ${GIT_DIR:-.git} -a ! -f .git || ! describe\n+elif test -d ${GIT_DIR:-.git} -o -f .git && describe\n then\n+       VN=$(echo \"$VN\" | sed -e 's/-/~/g')\n+else\n        VN=\"$DEF_VER\"\n fi\n \ndiff --git a/Makefile b/Makefile\nindex 7a8bee7..3588ca1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2426,7 +2426,7 @@ quick-install-html:\n ### Maintainer's dist rules\n \n git.spec: git.spec.in GIT-VERSION-FILE\n-       sed -e 's/@@VERSION@@/$(subst -,~,$(GIT_VERSION))/g' < $< > $@+\n+       sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+\n        mv $@+ $@\n \n GIT_TARNAME = git-$(GIT_VERSION)\n\nFelipe Contreras (2):\n  version-gen: cleanup\n  version-gen: fix versions\n\n GIT-VERSION-GEN | 36 ++++++++++++++++++++----------------\n 1 file changed, 20 insertions(+), 16 deletions(-)\n\n-- \n1.8.4-fc\n"},{"id":"228838","messageId":"1381561628-20665-2-git-send-email-felipe.contreras@gmail.com","threadId":"35125","inReplyTo":"1381561628-20665-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 1/2] version-gen: cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-12T07:07:07Z","receivedAt":"2013-10-12T07:07:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n GIT-VERSION-GEN | 36 ++++++++++++++++++++----------------\n 1 file changed, 20 insertions(+), 16 deletions(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex 06026ea..e96538d 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -6,22 +6,29 @@ DEF_VER=v1.8.4\n LF='\n '\n \n-# First see if there is a version file (included in release tarballs),\n-# then try git-describe, then default.\n-if test -f version\n-then\n-\tVN=$(cat version) || VN=\"$DEF_VER\"\n-elif test -d ${GIT_DIR:-.git} -o -f .git &&\n-\tVN=$(git describe --match \"v[0-9]*\" --abbrev=7 HEAD 2>/dev/null) &&\n+describe () {\n+\tVN=$(git describe --match \"v[0-9]*\" --abbrev=7 HEAD 2>/dev/null) || return 1\n \tcase \"$VN\" in\n-\t*$LF*) (exit 1) ;;\n+\t*$LF*)\n+\t\treturn 1\n+\t\t;;\n \tv[0-9]*)\n \t\tgit update-index -q --refresh\n \t\ttest -z \"$(git diff-index --name-only HEAD --)\" ||\n-\t\tVN=\"$VN-dirty\" ;;\n+\t\tVN=\"$VN-dirty\"\n+\t\treturn 0\n+\t\t;;\n \tesac\n+}\n+\n+# First see if there is a version file (included in release tarballs),\n+# then try 'git describe', then default.\n+if test -f version\n+then\n+\tVN=$(cat version) || VN=\"$DEF_VER\"\n+elif test -d ${GIT_DIR:-.git} -o -f .git && describe\n then\n-\tVN=$(echo \"$VN\" | sed -e 's/-/./g');\n+\tVN=$(echo \"$VN\" | sed -e 's/-/./g')\n else\n \tVN=\"$DEF_VER\"\n fi\n@@ -34,9 +41,6 @@ then\n else\n \tVC=unset\n fi\n-test \"$VN\" = \"$VC\" || {\n-\techo >&2 \"GIT_VERSION = $VN\"\n-\techo \"GIT_VERSION = $VN\" >$GVF\n-}\n-\n-\n+test \"$VN\" = \"$VC\" && exit\n+echo >&2 \"GIT_VERSION = $VN\"\n+echo \"GIT_VERSION = $VN\" >$GVF\n-- \n1.8.4-fc\n"},{"id":"228839","messageId":"1381561628-20665-3-git-send-email-felipe.contreras@gmail.com","threadId":"35125","inReplyTo":"1381561628-20665-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v4 2/2] version-gen: fix versions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-12T07:07:08Z","receivedAt":"2013-10-12T07:07:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Virtually all packaging guidelines would prefer 1.8.4~rc1, over\n1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.\n\nIn particular, the only packaging we provide, git.spec, generates a\nwrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes\nthe problem as it's considered newer.\n\nThe same happens in dpkg.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n GIT-VERSION-GEN | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex e96538d..c04c4de 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -28,7 +28,7 @@ then\n \tVN=$(cat version) || VN=\"$DEF_VER\"\n elif test -d ${GIT_DIR:-.git} -o -f .git && describe\n then\n-\tVN=$(echo \"$VN\" | sed -e 's/-/./g')\n+\tVN=$(echo \"$VN\" | sed -e 's/-/~/g')\n else\n \tVN=\"$DEF_VER\"\n fi\n-- \n1.8.4-fc\n"},{"id":"228911","messageId":"CAJDDKr5K0UjcbhUpAjHjGcEk6=E2+cAVC8-RpxY+C1atSVj-Tg@mail.gmail.com","threadId":"35125","inReplyTo":"1381561628-20665-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v4 2/2] version-gen: fix versions","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-10-13T21:56:36Z","receivedAt":"2013-10-13T21:56:36Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, Oct 12, 2013 at 12:07 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> Virtually all packaging guidelines would prefer 1.8.4~rc1, over\n> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.\n>\n> In particular, the only packaging we provide, git.spec, generates a\n> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes\n> the problem as it's considered newer.\n>\n> The same happens in dpkg.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  GIT-VERSION-GEN | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> index e96538d..c04c4de 100755\n> --- a/GIT-VERSION-GEN\n> +++ b/GIT-VERSION-GEN\n> @@ -28,7 +28,7 @@ then\n>         VN=$(cat version) || VN=\"$DEF_VER\"\n>  elif test -d ${GIT_DIR:-.git} -o -f .git && describe\n>  then\n> -       VN=$(echo \"$VN\" | sed -e 's/-/./g')\n> +       VN=$(echo \"$VN\" | sed -e 's/-/~/g')\n>  else\n>         VN=\"$DEF_VER\"\n>  fi\n> --\n\nThis seems related:\n\nhttp://lintian.debian.org/tags/rc-version-greater-than-expected-version.html\n\nShould the RC tags in the Git repo be named v1.2.3~rc4 (tilde-rc#)\ninstead of dash-rc#, or does that not matter?\n\nIf so, would that change anything about this patch, or is it better to\nnormalize it all here?\n\nThe input is subtly different sometimes so I'm curious whether whether\n\"~\" is preferred in all cases (particularly, by all package managers).\n e.g.\n\n$ git describe v1.5.0^\nv1.5.0-rc4-372-g26cfcfb\n\n$ git describe v1.5.0.1^\nv1.5.0-27-g38eb932\n-- \nDavid\n"},{"id":"228918","messageId":"CAMP44s2o5O=zKBef5Jn486yYkYYvuLSZRQokZAUQVN+EE=3+3Q@mail.gmail.com","threadId":"35125","inReplyTo":"CAJDDKr5K0UjcbhUpAjHjGcEk6=E2+cAVC8-RpxY+C1atSVj-Tg@mail.gmail.com","subject":"Re: [PATCH v4 2/2] version-gen: fix versions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-14T02:42:22Z","receivedAt":"2013-10-14T02:42:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, Oct 13, 2013 at 4:56 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Sat, Oct 12, 2013 at 12:07 AM, Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n>> Virtually all packaging guidelines would prefer 1.8.4~rc1, over\n>> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.\n>>\n>> In particular, the only packaging we provide, git.spec, generates a\n>> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes\n>> the problem as it's considered newer.\n>>\n>> The same happens in dpkg.\n>>\n>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n>> ---\n>>  GIT-VERSION-GEN | 2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n>> index e96538d..c04c4de 100755\n>> --- a/GIT-VERSION-GEN\n>> +++ b/GIT-VERSION-GEN\n>> @@ -28,7 +28,7 @@ then\n>>         VN=$(cat version) || VN=\"$DEF_VER\"\n>>  elif test -d ${GIT_DIR:-.git} -o -f .git && describe\n>>  then\n>> -       VN=$(echo \"$VN\" | sed -e 's/-/./g')\n>> +       VN=$(echo \"$VN\" | sed -e 's/-/~/g')\n>>  else\n>>         VN=\"$DEF_VER\"\n>>  fi\n>> --\n>\n> This seems related:\n>\n> http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html\n>\n> Should the RC tags in the Git repo be named v1.2.3~rc4 (tilde-rc#)\n> instead of dash-rc#, or does that not matter?\n\nI thought so first, but then I realized ~ is not allowed in a ref.\n\n> If so, would that change anything about this patch, or is it better to\n> normalize it all here?\n>\n> The input is subtly different sometimes so I'm curious whether whether\n> \"~\" is preferred in all cases (particularly, by all package managers).\n>  e.g.\n\nAll package managers I investigated do handle ~ specially, and thus\nrecommend it for rc versioning, except pacman. So in pacman,\nv1.5.0~rc4 would remain newer than v1.5.0, but that's not different\nfrom the current situation, and there isn't much we can do about that.\n\n> $ git describe v1.5.0^\n> v1.5.0-rc4-372-g26cfcfb\n>\n> $ git describe v1.5.0.1^\n> v1.5.0-27-g38eb932\n\nAt least both in RPM and dpkg, 1.5.0~27 is newer than 1.5.0~rc4.\n\n-- \nFelipe Contreras\n"},{"id":"228919","messageId":"20131014050104.GA25344@google.com","threadId":"35125","inReplyTo":"CAJDDKr5K0UjcbhUpAjHjGcEk6=E2+cAVC8-RpxY+C1atSVj-Tg@mail.gmail.com","subject":"Re: [PATCH v4 2/2] version-gen: fix versions","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-14T05:01:04Z","receivedAt":"2013-10-14T05:01:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nDavid Aguilar wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> wrote:\n\n>> Virtually all packaging guidelines would prefer 1.8.4~rc1, over\n>> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.\n>>\n>> In particular, the only packaging we provide, git.spec, generates a\n>> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes\n>> the problem as it's considered newer.\n\nA more conservative fix would be to tweak the .spec generation in the\nMakefile to follow whatever the appropriate Red Hat convention is.\nFor example, something like this:\n\n-- >8 --\ndiff --git i/Makefile w/Makefile\nindex 0f931a2..73bd89d 100644\n--- i/Makefile\n+++ w/Makefile\n@@ -2385,8 +2385,9 @@ quick-install-html:\n \n ### Maintainer's dist rules\n \n+GIT_VERSION_RPM = $(subst -rc,~rc,$(GIT_VERSION))\n git.spec: git.spec.in GIT-VERSION-FILE\n-\tsed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+\n+\tsed -e 's/@@VERSION@@/$(GIT_VERSION_RPM)/g' < $< > $@+\n \tmv $@+ $@\n \n GIT_TARNAME = git-$(GIT_VERSION)\n-- 8< --\n\nThat way, programs that parse the git version by splitting at '.'\n(there are more than a few, unfortunately) would continue to work, but\nthe packaging system would get the benefit of the proposed versioning\nstyle change.\n\n>> The same happens in dpkg.\n\nHave you tested this?  I thought the Debian packaging did not use the\nGIT-VERSION-GEN generated version in this way.\n\n[...]\n> This seems related:\n>\n> http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html\n\nIf I understand correctly, that page has an exhaustive list of affected\npackages in the Debian archive and doesn't include git.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"228922","messageId":"CAMP44s3PfRVV5V4=iddR2AOdnqhOt5oi8Wx9nS=2x=z_kN2wjw@mail.gmail.com","threadId":"35125","inReplyTo":"20131014050104.GA25344@google.com","subject":"Re: [PATCH v4 2/2] version-gen: fix versions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-14T05:29:23Z","receivedAt":"2013-10-14T05:29:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Oct 14, 2013 at 12:01 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Hi,\n>\n> David Aguilar wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> wrote:\n>\n>>> Virtually all packaging guidelines would prefer 1.8.4~rc1, over\n>>> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.\n>>>\n>>> In particular, the only packaging we provide, git.spec, generates a\n>>> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes\n>>> the problem as it's considered newer.\n>\n> A more conservative fix would be to tweak the .spec generation in the\n> Makefile to follow whatever the appropriate Red Hat convention is.\n> For example, something like this:\n\nIt's not Red Hat's convention, it's RPM, and dpkg, so basically every\npackage manager that most distributions out there use.\n\nAnd I already sent a patch for that which was ignored:\n\nhttp://article.gmane.org/gmane.comp.version-control.git/234794\n\n> -- >8 --\n> diff --git i/Makefile w/Makefile\n> index 0f931a2..73bd89d 100644\n> --- i/Makefile\n> +++ w/Makefile\n> @@ -2385,8 +2385,9 @@ quick-install-html:\n>\n>  ### Maintainer's dist rules\n>\n> +GIT_VERSION_RPM = $(subst -rc,~rc,$(GIT_VERSION))\n\nThat wouldn't work; VERSION doesn't have '-rc', it has '.rc'. and\nwhat's the point of creating a new variable? It's a was of space.\n\n>  git.spec: git.spec.in GIT-VERSION-FILE\n> -       sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+\n> +       sed -e 's/@@VERSION@@/$(GIT_VERSION_RPM)/g' < $< > $@+\n>         mv $@+ $@\n>\n>  GIT_TARNAME = git-$(GIT_VERSION)\n> -- 8< --\n>\n> That way, programs that parse the git version by splitting at '.'\n> (there are more than a few, unfortunately) would continue to work,\n\nDo you have any evidence that such programs exist? Specifically,\nprograms that work with 1.8.4.rc1, but not 1.8.4-rc1 or 1.8.4~rc1. I\nfind that very very very unlikely.\n\nAnyway, in the very very very unlikely scenario that somebody's script\ndoes break they can report that, and we can revert. What's the\nproblem?\n\n> but\n> the packaging system would get the benefit of the proposed versioning\n> style change.\n>\n>>> The same happens in dpkg.\n>\n> Have you tested this?\n\n% dpkg --compare-versions 1.8.4 gt 1.8.4~rc1 && echo yes || echo no\nyes\n % dpkg --compare-versions 1.8.4 gt 1.8.4-rc1 && echo yes || echo no\nno\n\n> I thought the Debian packaging did not use the\n> GIT-VERSION-GEN generated version in this way.\n\nIt doesn't matter. The name of the package would be git-1.8.4~rc1, and\n'git --version' would return 1.8.4.rc1, that's inconsistent.\n\nWhy be inconsistent when we can be consistent?\n\n> [...]\n>> This seems related:\n>>\n>> http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html\n>\n> If I understand correctly, that page has an exhaustive list of affected\n> packages in the Debian archive and doesn't include git.\n\nBecause a) they don't package release candidates, and b) they use a\ndifferent version (s/\\./~/).\n\n-- \nFelipe Contreras\n"}]}