{"thread":{"id":"42131","subject":"[PATCH v3 0/3] git-p4: fix Git LFS pointer parsing","startedAt":"2016-04-24T18:58:09Z","lastAt":"2016-04-25T07:35:48Z","messageCount":9,"participants":["larsxschneider@gmail.com","Sebastian Schuberth","Eric Sunshine","Lars Schneider"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"284348","messageId":"1461524292-20490-1-git-send-email-larsxschneider@gmail.com","threadId":"42131","inReplyTo":null,"subject":"[PATCH v3 0/3] git-p4: fix Git LFS pointer parsing","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-24T18:58:09Z","receivedAt":"2016-04-24T18:58:09Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nv1: $gmane/291917\nv2: $gmane/291991\n\ndiff to v2:\n * use regex to parse \"oid\" instead of a complicated split logic\n ($gmane/291995, thanks Sebastian)\n * add a note to the commit message emphasizing the change from array\n to string in the second return parameter ($gmane/292077, thanks Junio)\n * detect the Git-LFS preamble by the constant first part of the string\n ($gmane/292079, thanks Ben)\n * change the name of the Git-LFS and Perforce version variables to\n express that they are only used in the Linux build\n\nThanks,\nLars\n\nLars Schneider (3):\n travis-ci: update Git-LFS and P4 to the latest version\n travis-ci: express Linux/OS X dependency versions more clearly\n git-p4: fix Git LFS pointer parsing\n\n .travis.yml | 17 ++++++++++-------\n git-p4.py | 13 ++++++++++---\n t/t9824-git-p4-git-lfs.sh | 4 ++++\n 3 files changed, 24 insertions(+), 10 deletions(-)\n\n--\n2.5.1\n"},{"id":"284349","messageId":"1461524292-20490-2-git-send-email-larsxschneider@gmail.com","threadId":"42131","inReplyTo":"1461524292-20490-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 1/3] travis-ci: update Git-LFS and P4 to the latest version","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-24T18:58:10Z","receivedAt":"2016-04-24T18:58:10Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nSigned-off-by: Lars Schneider \n---\n .travis.yml | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/.travis.yml b/.travis.yml\nindex 78e433b..4acf617 100644\n--- a/.travis.yml\n+++ b/.travis.yml\n@@ -22,8 +22,8 @@ addons:\n env:\n global:\n - DEVELOPER=1\n- - P4_VERSION=\"15.2\"\n- - GIT_LFS_VERSION=\"1.1.0\"\n+ - P4_VERSION=\"16.1\"\n+ - GIT_LFS_VERSION=\"1.2.0\"\n - DEFAULT_TEST_TARGET=prove\n - GIT_PROVE_OPTS=\"--timer --jobs 3 --state=failed,slow,save\"\n - GIT_TEST_OPTS=\"--verbose --tee\"\n-- \n2.5.1\n"},{"id":"284350","messageId":"1461524292-20490-3-git-send-email-larsxschneider@gmail.com","threadId":"42131","inReplyTo":"1461524292-20490-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 2/3] travis-ci: express Linux/OS X dependency versions more clearly","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-24T18:58:11Z","receivedAt":"2016-04-24T18:58:11Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nThe Git Travis CI OSX build always installs the latest versions of Git LFS and\nPerforce via brew and the Linux build installs fixed versions. Consequently new\nLFS/Perforce versions can brake the OS X build even if there is no change in\nGit.\n\nSigned-off-by: Lars Schneider \n---\n .travis.yml | 17 ++++++++++-------\n 1 file changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/.travis.yml b/.travis.yml\nindex 4acf617..1fdcec8 100644\n--- a/.travis.yml\n+++ b/.travis.yml\n@@ -22,8 +22,11 @@ addons:\n env:\n global:\n - DEVELOPER=1\n- - P4_VERSION=\"16.1\"\n- - GIT_LFS_VERSION=\"1.2.0\"\n+ # The Linux build installs the defined dependency versions below.\n+ # The OS X build installs the latest available versions. Keep that\n+ # in mind when you encounter a broken OS X build!\n+ - LINUX_P4_VERSION=\"16.1\"\n+ - LINUX_GIT_LFS_VERSION=\"1.2.0\"\n - DEFAULT_TEST_TARGET=prove\n - GIT_PROVE_OPTS=\"--timer --jobs 3 --state=failed,slow,save\"\n - GIT_TEST_OPTS=\"--verbose --tee\"\n@@ -38,17 +41,17 @@ before_install:\n linux)\n mkdir --parents custom/p4\n pushd custom/p4\n- wget --quiet http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4d\n- wget --quiet http://filehost.perforce.com/perforce/r$P4_VERSION/bin.linux26x86_64/p4\n+ wget --quiet http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION/bin.linux26x86_64/p4d\n+ wget --quiet http://filehost.perforce.com/perforce/r$LINUX_P4_VERSION/bin.linux26x86_64/p4\n chmod u+x p4d\n chmod u+x p4\n export PATH=\"$(pwd):$PATH\"\n popd\n mkdir --parents custom/git-lfs\n pushd custom/git-lfs\n- wget --quiet https://github.com/github/git-lfs/releases/download/v$GIT_LFS_VERSION/git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz\n- tar --extract --gunzip --file \"git-lfs-linux-amd64-$GIT_LFS_VERSION.tar.gz\"\n- cp git-lfs-$GIT_LFS_VERSION/git-lfs .\n+ wget --quiet https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz\n+ tar --extract --gunzip --file \"git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz\"\n+ cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .\n export PATH=\"$(pwd):$PATH\"\n popd\n ;;\n-- \n2.5.1\n"},{"id":"284351","messageId":"1461524292-20490-4-git-send-email-larsxschneider@gmail.com","threadId":"42131","inReplyTo":"1461524292-20490-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v3 3/3] git-p4: fix Git LFS pointer parsing","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-24T18:58:12Z","receivedAt":"2016-04-24T18:58:12Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nGit LFS 1.2.0 removed a preamble from the output of the 'git lfs pointer'\ncommand [1] which broke the parsing of this output. Adjust the parser\nto support the old and the new format.\n\nPlease note that this patch slightly changes the second return parameter\nfrom a list of LF terminated strings to a single string that contains\na number of LF characters.\n\n[1] https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43\n\nSigned-off-by: Lars Schneider \nHelped-by: Sebastian Schuberth \nHelped-by: Ben Woosley \n---\n git-p4.py | 13 ++++++++++---\n t/t9824-git-p4-git-lfs.sh | 4 ++++\n 2 files changed, 14 insertions(+), 3 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 527d44b..f92a4bc 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -1064,8 +1064,15 @@ class GitLFS(LargeFileSystem):\n if pointerProcess.wait():\n os.remove(contentFile)\n die('git-lfs pointer command failed. Did you install the extension?')\n- pointerContents = [i+'\\n' for i in pointerFile.split('\\n')[2:][:-1]]\n- oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]\n+\n+ # Git LFS removed the preamble in the output of the 'pointer' command\n+ # starting from version 1.2.0. Check for the preamble here to support\n+ # earlier versions.\n+ # c.f. https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43\n+ if pointerFile.startswith('Git LFS pointer for'):\n+ re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n+\n+ oid = re.search(r'^oid \\w+:(\\w+)', pointerFile, re.MULTILINE).group(1)\n localLargeFile = os.path.join(\n os.getcwd(),\n '.git', 'lfs', 'objects', oid[:2], oid[2:4],\n@@ -1073,7 +1080,7 @@ class GitLFS(LargeFileSystem):\n )\n # LFS Spec states that pointer files should not have the executable bit set.\n gitMode = '100644'\n- return (gitMode, pointerContents, localLargeFile)\n+ return (gitMode, pointerFile, localLargeFile)\n \n def pushFile(self, localLargeFile):\n uploadProcess = subprocess.Popen(\ndiff --git a/t/t9824-git-p4-git-lfs.sh b/t/t9824-git-p4-git-lfs.sh\nindex 0b664a3..ca93ac8 100755\n--- a/t/t9824-git-p4-git-lfs.sh\n+++ b/t/t9824-git-p4-git-lfs.sh\n@@ -13,6 +13,10 @@ test_file_in_lfs () {\n \tFILE=\"$1\" &&\n \tSIZE=\"$2\" &&\n \tEXPECTED_CONTENT=\"$3\" &&\n+\tsed -n '1,1 p' \"$FILE\" | grep \"^version \" &&\n+\tsed -n '2,2 p' \"$FILE\" | grep \"^oid \" &&\n+\tsed -n '3,3 p' \"$FILE\" | grep \"^size \" &&\n+\ttest_line_count = 3 \"$FILE\" &&\n \tcat \"$FILE\" | grep \"size $SIZE\" &&\n \tHASH=$(cat \"$FILE\" | grep \"oid sha256:\" | sed -e \"s/oid sha256://g\") &&\n \tLFS_FILE=\".git/lfs/objects/$(echo \"$HASH\" | cut -c1-2)/$(echo \"$HASH\" | cut -c3-4)/$HASH\" &&\n-- \n2.5.1\n"},{"id":"284354","messageId":"CAHGBnuMs6h478FXGA_x2nP1D6BJCxGzhumR_xq5JFmNgzM6Sdw@mail.gmail.com","threadId":"42131","inReplyTo":"1461524292-20490-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-04-24T19:16:48Z","receivedAt":"2016-04-24T19:16:48Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Sun, Apr 24, 2016 at 8:58 PM, wrote:\n\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -1064,8 +1064,15 @@ class GitLFS(LargeFileSystem):\n> if pointerProcess.wait():\n> os.remove(contentFile)\n> die('git-lfs pointer command failed. Did you install the extension?')\n> - pointerContents = [i+'\\n' for i in pointerFile.split('\\n')[2:][:-1]]\n> - oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]\n> +\n> + # Git LFS removed the preamble in the output of the 'pointer' command\n> + # starting from version 1.2.0. Check for the preamble here to support\n> + # earlier versions.\n> + # c.f. https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43\n> + if pointerFile.startswith('Git LFS pointer for'):\n> + re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n\nI liked the code from v2 better. I know Ben said \"there could be\nexpansions or other modifications applied by git-lfs between input and\noutput\", but I believe it's better to be too strict than too lenient\nif you're omitting lines from the output. Also, the regex matches\nagainst the whole multi-line string. That is, if the file for some\nreason was ending in '\\n\\n' instead of just '\\n', the '.*' would match\nalmost all content of the pointer file, not just the remains of the\npreamble. One way to fix this would be to use\n\nre.sub(r'Git LFS pointer for [^\\n]+\\n\\n', '', pointerFile)\n\ninstead.\n\nRegards,\nSebastian\n"},{"id":"284372","messageId":"CAPig+cSoWqZhQY8asE0akgSKqaRbU0pd87_mjA5kBdEueUZBrQ@mail.gmail.com","threadId":"42131","inReplyTo":"1461524292-20490-3-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v3 2/3] travis-ci: express Linux/OS X dependency versions more clearly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-25T04:40:12Z","receivedAt":"2016-04-25T04:40:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sunday, April 24, 2016, wrote:\n> The Git Travis CI OSX build always installs the latest versions of Git LFS and\n> Perforce via brew and the Linux build installs fixed versions. Consequently new\n> LFS/Perforce versions can brake the OS X build even if there is no change in\n\ns/brake/break/\n\n> Git.\n>\n> Signed-off-by: Lars Schneider \n"},{"id":"284374","messageId":"EAB93C5E-56E1-4361-8987-E9DFB6C2F428@gmail.com","threadId":"42131","inReplyTo":"CAPig+cSoWqZhQY8asE0akgSKqaRbU0pd87_mjA5kBdEueUZBrQ@mail.gmail.com","subject":"Re: [PATCH v3 2/3] travis-ci: express Linux/OS X dependency versions more clearly","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-25T07:05:27Z","receivedAt":"2016-04-25T07:05:27Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 25 Apr 2016, at 06:40, Eric Sunshine wrote:\n\n> On Sunday, April 24, 2016, wrote:\n>> The Git Travis CI OSX build always installs the latest versions of Git LFS and\n>> Perforce via brew and the Linux build installs fixed versions. Consequently new\n>> LFS/Perforce versions can brake the OS X build even if there is no change in\n> \n> s/brake/break/\n\nThank you!\n\n> \n>> Git.\n>> \n>> Signed-off-by: Lars Schneider \n"},{"id":"284376","messageId":"8340C6E5-C774-46EE-A1F9-1C4C7DF9CEB9@gmail.com","threadId":"42131","inReplyTo":"CAHGBnuMs6h478FXGA_x2nP1D6BJCxGzhumR_xq5JFmNgzM6Sdw@mail.gmail.com","subject":"Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-25T07:33:01Z","receivedAt":"2016-04-25T07:33:01Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 24 Apr 2016, at 21:16, Sebastian Schuberth wrote:\n\n> On Sun, Apr 24, 2016 at 8:58 PM, wrote:\n> \n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -1064,8 +1064,15 @@ class GitLFS(LargeFileSystem):\n>> if pointerProcess.wait():\n>> os.remove(contentFile)\n>> die('git-lfs pointer command failed. Did you install the extension?')\n>> - pointerContents = [i+'\\n' for i in pointerFile.split('\\n')[2:][:-1]]\n>> - oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]\n>> +\n>> + # Git LFS removed the preamble in the output of the 'pointer' command\n>> + # starting from version 1.2.0. Check for the preamble here to support\n>> + # earlier versions.\n>> + # c.f. https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43\n>> + if pointerFile.startswith('Git LFS pointer for'):\n>> + re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n> \n> I liked the code from v2 better. I know Ben said \"there could be\n> expansions or other modifications applied by git-lfs between input and\n> output\", but I believe it's better to be too strict than too lenient\n> if you're omitting lines from the output. Also, the regex matches\n> against the whole multi-line string. That is, if the file for some\n> reason was ending in '\\n\\n' instead of just '\\n', the '.*' would match\n> almost all content of the pointer file, not just the remains of the\n> preamble. One way to fix this would be to use\n> \n> re.sub(r'Git LFS pointer for [^\\n]+\\n\\n', '', pointerFile)\n> \n> instead.\n\nIn general you are right as \"*\" is greedy. However, in Python \".\" matches any \ncharacter except a newline [1]. Therefore I think the regex is correct.\n\nNevertheless... thanks for making me read the line again. I forgot to\nassign the pointerFile variable in the version I sent around :-(\n\nThis is how it should be:\n\npointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n\nThanks,\nLars\n\n\n[1] https://docs.python.org/2/library/re.html\n"},{"id":"284377","messageId":"CAHGBnuO16Y2b3G5pyuG59b+vojRGe_wySmUcAUka9eT2opABdA@mail.gmail.com","threadId":"42131","inReplyTo":"8340C6E5-C774-46EE-A1F9-1C4C7DF9CEB9@gmail.com","subject":"Re: [PATCH v3 3/3] git-p4: fix Git LFS pointer parsing","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-04-25T07:35:48Z","receivedAt":"2016-04-25T07:35:48Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Mon, Apr 25, 2016 at 9:33 AM, Lars Schneider\n wrote:\n\n>> if you're omitting lines from the output. Also, the regex matches\n>> against the whole multi-line string. That is, if the file for some\n>> reason was ending in '\\n\\n' instead of just '\\n', the '.*' would match\n>> almost all content of the pointer file, not just the remains of the\n>> preamble. One way to fix this would be to use\n>>\n>> re.sub(r'Git LFS pointer for [^\\n]+\\n\\n', '', pointerFile)\n>>\n>> instead.\n>\n> In general you are right as \"*\" is greedy. However, in Python \".\" matches any\n> character except a newline [1]. Therefore I think the regex is correct.\n\nAh, thanks for pointing that out. Looks ok to me then.\n\n> Nevertheless... thanks for making me read the line again. I forgot to\n> assign the pointerFile variable in the version I sent around :-(\n>\n> This is how it should be:\n>\n> pointerFile = re.sub(r'Git LFS pointer for.*\\n\\n', '', pointerFile)\n\nRight. Good you've catched that!\n\n-- \nSebastian Schuberth\n"}]}