{"thread":{"id":"40206","subject":"[RFC PATCH] git-p4: add option to store files in Git LFS on import","startedAt":"2015-08-28T12:24:50Z","lastAt":"2015-09-03T10:12:39Z","messageCount":9,"participants":["larsxschneider@gmail.com","Luke Diamand","Lars Schneider","John Keeping"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"268837","messageId":"1440764691-62254-1-git-send-email-larsxschneider@gmail.com","threadId":"40206","inReplyTo":null,"subject":"[RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-08-28T12:24:50Z","receivedAt":"2015-08-28T12:24:50Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nI am migrating huge Perforce repositories including history to Git. Some of them contain large files that would blow up the resulting Git repositories. This patch adds an option to store these files in Git LFS [1] on git-p4 clone.\n\nIn order to run the unit tests you need to install the Git LFS extension [2].\n\nKnown limitations:\nThe option \"use-lfs-if-size-exceeds\" looks at the uncompressed file size. Sometimes huge XML files are tiny if compressed. I wonder if there is an easy way to learn about the size of a file in a git pack file. I assume compressing it is the only way to know.\n\nFeedback is highly appreciated.\n\nThank you,\nLars\n\n\n[1] https://git-lfs.github.com/\n[2] https://github.com/github/git-lfs/releases/\n\nLars Schneider (1):\n  git-p4: add option to store files in Git LFS on import\n\n Documentation/git-p4.txt |  12 ++\n git-p4.py                |  94 ++++++++++++++--\n t/t9822-git-p4-lfs.sh    | 277 +++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 374 insertions(+), 9 deletions(-)\n create mode 100755 t/t9822-git-p4-lfs.sh\n\n--\n1.9.5 (Apple Git-50.3)\n"},{"id":"268838","messageId":"1440764691-62254-2-git-send-email-larsxschneider@gmail.com","threadId":"40206","inReplyTo":"1440764691-62254-1-git-send-email-larsxschneider@gmail.com","subject":"[RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-08-28T12:24:51Z","receivedAt":"2015-08-28T12:24:51Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider <larsxschneider@gmail.com>\n\nSigned-off-by: Lars Schneider <larsxschneider@gmail.com>\n---\n Documentation/git-p4.txt |  12 ++\n git-p4.py                |  94 ++++++++++++++--\n t/t9822-git-p4-lfs.sh    | 277 +++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 374 insertions(+), 9 deletions(-)\n create mode 100755 t/t9822-git-p4-lfs.sh\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex 82aa5d6..a188840 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -252,6 +252,18 @@ Git repository:\n \tUse a client spec to find the list of interesting files in p4.\n \tSee the \"CLIENT SPEC\" section below.\n \n+--use-lfs-if-size-exceeds <n>::\n+\tStore files that have an uncompressed size exceeding 'n' bytes in \n+\tGit LFS. Download and install the Git LFS command line extension to\n+\tuse that option.\n+\tMore info here: https://git-lfs.github.com/\n+\n+--use-lfs-for-extension <extension>::\n+\tStore files with 'extension' in Git LFS. Do not prefix the extensions\n+\twith a '.'. You can use this option multiple times. Download and \n+\tinstall the Git LFS command line extension to use that option.\n+\tMore info here: https://git-lfs.github.com/\n+\n -/ <path>::\n \tExclude selected depot paths when cloning or syncing.\n \ndiff --git a/git-p4.py b/git-p4.py\nindex 073f87b..e031021 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -22,6 +22,7 @@ import platform\n import re\n import shutil\n import stat\n+import errno\n \n try:\n     from subprocess import CalledProcessError\n@@ -104,6 +105,16 @@ def chdir(path, is_client_path=False):\n         path = os.getcwd()\n     os.environ['PWD'] = path\n \n+def mkdir_p(path):\n+    # Copied from http://stackoverflow.com/questions/600268/mkdir-p-functionality-in-python\n+    try:\n+        os.makedirs(path)\n+    except OSError as exc: # Python >2.5\n+        if exc.errno == errno.EEXIST and os.path.isdir(path):\n+            pass\n+        else:\n+            raise\n+\n def die(msg):\n     if verbose:\n         raise Exception(msg)\n@@ -1994,6 +2005,11 @@ class P4Sync(Command, P4UserMap):\n                 optparse.make_option(\"-/\", dest=\"cloneExclude\",\n                                      action=\"append\", type=\"string\",\n                                      help=\"exclude depot path\"),\n+                optparse.make_option(\"--use-lfs-if-size-exceeds\", dest=\"lfsMinimumFileSize\", type=\"int\",\n+                                     help=\"Use LFS to store files bigger than the given threshold in bytes.\"),\n+                optparse.make_option(\"--use-lfs-for-extension\", dest=\"lfsFileExtensions\",\n+                                     action=\"append\", type=\"string\",\n+                                     help=\"Use LFS to store files with the given file extension(s).\"),\n         ]\n         self.description = \"\"\"Imports from Perforce into a git repository.\\n\n     example:\n@@ -2025,6 +2041,9 @@ class P4Sync(Command, P4UserMap):\n         self.clientSpecDirs = None\n         self.tempBranches = []\n         self.tempBranchLocation = \"git-p4-tmp\"\n+        self.lfsFiles = []\n+        self.lfsMinimumFileSize = None\n+        self.lfsFileExtensions = []\n \n         if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n             self.syncWithOrigin = False\n@@ -2145,6 +2164,63 @@ class P4Sync(Command, P4UserMap):\n \n         return branches\n \n+    def writeToGitStream(self, gitMode, relPath, contents):\n+        self.gitStream.write('M %s inline %s\\n' % (gitMode, relPath))\n+        self.gitStream.write('data %d\\n' % sum(len(d) for d in contents))\n+        for d in contents:\n+            self.gitStream.write(d)\n+        self.gitStream.write('\\n')\n+\n+    def writeGitAttributesToStream(self):\n+        gitAttributes = [f + ' filter=lfs -text\\n' for f in self.lfsFiles if not self.hasFileLFSExtension(f)]\n+        self.writeToGitStream(\n+            '100644',\n+            '.gitattributes',\n+            ['*.' + f + ' filter=lfs -text\\n' for f in self.lfsFileExtensions] +\n+            [f + ' filter=lfs -text\\n' for f in self.lfsFiles if not self.hasFileLFSExtension(f)]\n+        )\n+\n+    def hasFileLFSExtension(self, relPath):\n+        return reduce(\n+            lambda a, b: a or b,\n+            [relPath.endswith('.' + e) for e in self.lfsFileExtensions],\n+            False\n+        )\n+\n+    def isFileLargerThanLFSTreshold(self, relPath, contents):\n+        return self.lfsMinimumFileSize and sum(len(d) for d in contents) >= self.lfsMinimumFileSize\n+\n+    def generateLFSPointerFile(self, relPath, contents):\n+        # Write P4 content to temp file\n+        p4ContentTempFile = tempfile.NamedTemporaryFile(prefix='git-lfs', delete=False)\n+        for d in contents:\n+            p4ContentTempFile.write(d)\n+        p4ContentTempFile.flush()\n+\n+        # Generate LFS pointer file based on P4 content\n+        lfsProcess = subprocess.Popen(\n+            ['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],\n+            stdout=subprocess.PIPE\n+        )\n+        lfsPointerFile = lfsProcess.stdout.read()\n+        if lfsProcess.wait():\n+            die('git-lfs command failed. Did you install the extension?')\n+        contents = [i+'\\n' for i in lfsPointerFile.split('\\n')[2:][:-1]]\n+\n+        # Write P4 content to LFS\n+        oid = contents[1].split(' ')[1].split(':')[1][:-1]\n+        oidPath = os.path.join(self.cloneDestination, '.git', 'lfs', 'objects', oid[:2], oid[2:4])\n+        mkdir_p(oidPath)\n+        shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))\n+\n+        # Update Git attributes\n+        self.lfsFiles.append(relPath)\n+        self.writeGitAttributesToStream()\n+\n+        # LFS Spec states that pointer files should not have the executable bit set.\n+        gitMode = '100644'\n+        return (gitMode, contents)\n+\n     # output one file from the P4 stream\n     # - helper for streamP4Files\n \n@@ -2213,17 +2289,13 @@ class P4Sync(Command, P4UserMap):\n             text = regexp.sub(r'$\\1$', text)\n             contents = [ text ]\n \n-        self.gitStream.write(\"M %s inline %s\\n\" % (git_mode, relPath))\n+        if relPath == '.gitattributes':\n+            die('.gitattributes already exists in P4.')\n \n-        # total length...\n-        length = 0\n-        for d in contents:\n-            length = length + len(d)\n+        if self.isFileLargerThanLFSTreshold(relPath, contents) or self.hasFileLFSExtension(relPath):\n+            (git_mode, contents) = self.generateLFSPointerFile(relPath, contents)\n \n-        self.gitStream.write(\"data %d\\n\" % length)\n-        for d in contents:\n-            self.gitStream.write(d)\n-        self.gitStream.write(\"\\n\")\n+        self.writeToGitStream(git_mode, relPath, contents)\n \n     def streamOneP4Deletion(self, file):\n         relPath = self.stripRepoPath(file['path'], self.branchPrefixes)\n@@ -2231,6 +2303,10 @@ class P4Sync(Command, P4UserMap):\n             sys.stderr.write(\"delete %s\\n\" % relPath)\n         self.gitStream.write(\"D %s\\n\" % relPath)\n \n+        if relPath in self.lfsFiles:\n+            self.lfsFiles.remove(relPath)\n+            self.writeGitAttributesToStream()\n+\n     # handle another chunk of streaming data\n     def streamP4FilesCb(self, marshalled):\n \ndiff --git a/t/t9822-git-p4-lfs.sh b/t/t9822-git-p4-lfs.sh\nnew file mode 100755\nindex 0000000..b27bf29\n--- /dev/null\n+++ b/t/t9822-git-p4-lfs.sh\n@@ -0,0 +1,277 @@\n+#!/bin/sh\n+\n+test_description='Clone repositories and store files in LFS'\n+\n+( git lfs help ) >/dev/null 2>&1 || {\n+\tskip_all='skipping git p4 LFS tests; no git lfs'\n+\ttest_done\n+}\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'Create repo with binary files' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo \"text\" >file.txt &&\n+\t\techo \"bin 13 bytes\" >file.dat &&\n+\t\tp4 add file.txt &&\n+\t\tp4 add file.dat &&\n+\t\tp4 submit -d \"Add text and binary file\" &&\n+\t\techo \"bin 13 bytes\" >file2.bin &&\n+\t\tp4 add file2.bin &&\n+\t\tp4 submit -d \"Add another binary file with same content\"\n+\t\techo \"bin 14 bytess\" >file3.bin &&\n+\t\tp4 add file3.bin &&\n+\t\tp4 submit -d \"Add another binary file with different content\"\n+\t)\n+'\n+\n+test_expect_success 'Store files in LFS based on size (10 bytes)' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\tgit p4 clone --use-client-spec --use-lfs-if-size-exceeds=10 --destination=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\t.git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\t.git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tEOF\n+\t\tfind \".git/lfs/objects\" -type f >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tsize 13\n+\t\tEOF\n+\t\tcat file.dat >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tsize 13\n+\t\tEOF\n+\t\tcat file2.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tsize 14\n+\t\tEOF\n+\t\tcat file3.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tfile.dat filter=lfs -text\n+\t\tfile2.bin filter=lfs -text\n+\t\tfile3.bin filter=lfs -text\n+\t\tEOF\n+\t\tcat .gitattributes >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Store files in LFS based on size (14 bytes)' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\tgit p4 clone --use-client-spec --use-lfs-if-size-exceeds=14 --destination=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\t.git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tEOF\n+\t\tfind \".git/lfs/objects\" -type f >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tbin 13 bytes\n+\t\tEOF\n+\t\tcat file.dat >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tbin 13 bytes\n+\t\tEOF\n+\t\tcat file2.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tsize 14\n+\t\tEOF\n+\t\tcat file3.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tfile3.bin filter=lfs -text\n+\t\tEOF\n+\t\tcat .gitattributes >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Store files in LFS based on extension (dat)' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\tgit p4 clone --use-client-spec --use-lfs-for-extension=dat --destination=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\t.git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tEOF\n+\t\tfind \".git/lfs/objects\" -type f >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tsize 13\n+\t\tEOF\n+\t\tcat file.dat >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tbin 13 bytes\n+\t\tEOF\n+\t\tcat file2.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tbin 14 bytess\n+\t\tEOF\n+\t\tcat file3.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\t*.dat filter=lfs -text\n+\t\tEOF\n+\t\tcat .gitattributes >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Store files in LFS based on size (14 bytes) and extension (dat)' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\tgit p4 clone \\\n+\t\t--use-client-spec \\\n+\t\t--use-lfs-if-size-exceeds=14 \\\n+\t\t--use-lfs-for-extension=dat \\\n+\t\t--destination=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\t.git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\t.git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tEOF\n+\t\tfind \".git/lfs/objects\" -type f >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tsize 13\n+\t\tEOF\n+\t\tcat file.dat >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tbin 13 bytes\n+\t\tEOF\n+\t\tcat file2.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tsize 14\n+\t\tEOF\n+\t\tcat file3.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\t*.dat filter=lfs -text\n+\t\tfile3.bin filter=lfs -text\n+\t\tEOF\n+\t\tcat .gitattributes >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Remove file from repo and store files in LFS based on size (10 bytes)' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\tp4 delete file3.bin &&\n+\t\tp4 submit -d \"Remove file\"\n+\t) &&\n+\n+\tgit p4 clone --use-client-spec --use-lfs-if-size-exceeds=10 --destination=\"$git\" //depot@all &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\n+\t\t# Note that file3 remains here as it referenced in the history\n+\t\tcat >expect <<-\\EOF &&\n+\t\t.git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\t.git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n+\t\tEOF\n+\t\tfind \".git/lfs/objects\" -type f >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tsize 13\n+\t\tEOF\n+\t\tcat file.dat >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tversion https://git-lfs.github.com/spec/v1\n+\t\toid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n+\t\tsize 13\n+\t\tEOF\n+\t\tcat file2.bin >actual &&\n+\t\ttest_cmp expect actual &&\n+\n+\t\tcat >expect <<-\\EOF &&\n+\t\tfile.dat filter=lfs -text\n+\t\tfile2.bin filter=lfs -text\n+\t\tEOF\n+\t\tcat .gitattributes >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'Clone repo with existing .gitattributes file' '\n+\tclient_view \"//depot/... //client/...\" &&\n+\t(\n+\t\tcd \"$cli\" &&\n+\n+\t\techo \"*.txt text\" >.gitattributes &&\n+\t\tp4 add .gitattributes &&\n+\t\tp4 submit -d \"Add .gitattributes\"\n+\t) &&\n+\n+\ttest_must_fail git p4 clone --use-client-spec --destination=\"$git\" //depot 2>errs &&\n+\tgrep \".gitattributes already exists in P4.\" errs\n+'\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.9.5 (Apple Git-50.3)\n"},{"id":"268927","messageId":"CAE5ih7-TfLqwye8YpmZz90AVjYGQ8m0sF6GLKnG9pjvV8cTSjw@mail.gmail.com","threadId":"40206","inReplyTo":"1440764691-62254-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-08-30T08:49:22Z","receivedAt":"2015-08-30T08:49:22Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 28 August 2015 at 13:24,  <larsxschneider@gmail.com> wrote:\n> From: Lars Schneider <larsxschneider@gmail.com>\n>\n> I am migrating huge Perforce repositories including history to Git. Some of them contain large files that would blow up the resulting Git repositories. This patch adds an option to store these files in Git LFS [1] on git-p4 clone.\n\nI'm a bit worried by this. LFS isn't the only way to handle large\nfiles in git - there's also git annex (which I've used in a similar\nsituation) and obviously random homebrew solutions. We're going to end\nup with git-p4 sprouting ever increasing numbers of\n--use-XXX-if-size-exceeds options. On the other hand, having it\nintegrated into git-p4 is quite nice as it saves a lot of messing\naround.\n\nWould it be possible as a start to have (within git-p4) a generic\nspot-big-files-and-handle-them-differently patch, and a second patch\nto add specific LFS support? That then means that other schemes would\nbe a lot easier to add in future.\n\nSome other comments inline.\n\n>\n> In order to run the unit tests you need to install the Git LFS extension [2].\n>\n> Known limitations:\n> The option \"use-lfs-if-size-exceeds\" looks at the uncompressed file size. Sometimes huge XML files are tiny if compressed. I wonder if there is an easy way to learn about the size of a file in a git pack file. I assume compressing it is the only way to know.\n>\n> Feedback is highly appreciated.\n>\n> Thank you,\n> Lars\n>\n>\n> [1] https://git-lfs.github.com/\n> [2] https://github.com/github/git-lfs/releases/\n>\n> Lars Schneider (1):\n>   git-p4: add option to store files in Git LFS on import\n>\n>  Documentation/git-p4.txt |  12 ++\n>  git-p4.py                |  94 ++++++++++++++--\n>  t/t9822-git-p4-lfs.sh    | 277 +++++++++++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 374 insertions(+), 9 deletions(-)\n>  create mode 100755 t/t9822-git-p4-lfs.sh\n>\n> --\n> 1.9.5 (Apple Git-50.3)\n\nCan you switch to a newer git - this one's quite old now so if there\nare regressions introduced later, you won't know about them!\n\n>\n"},{"id":"268928","messageId":"CAE5ih7_yvzEgij9XEvoDnMMvkCeB77SUfnkf7=xr6r8eN6SCog@mail.gmail.com","threadId":"40206","inReplyTo":"1440764691-62254-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-08-30T09:08:10Z","receivedAt":"2015-08-30T09:08:10Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"Comments below.\n\n>\n> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\n> index 82aa5d6..a188840 100644\n> --- a/Documentation/git-p4.txt\n> +++ b/Documentation/git-p4.txt\n> @@ -252,6 +252,18 @@ Git repository:\n>         Use a client spec to find the list of interesting files in p4.\n>         See the \"CLIENT SPEC\" section below.\n>\n\n<snip>\n\n>\n> +def mkdir_p(path):\n> +    # Copied from http://stackoverflow.com/questions/600268/mkdir-p-functionality-in-python\n> +    try:\n> +        os.makedirs(path)\n> +    except OSError as exc: # Python >2.5\n> +        if exc.errno == errno.EEXIST and os.path.isdir(path):\n> +            pass\n> +        else:\n> +            raise\n\nSigh. We need to upgrade to Python 3....\n\nCoujld also just do:\n\n   if not path.isdir(path) os.makedirs(path)\n\n(Although there is a tiny race hazard if something else creates the\nsame path between isdir and makedir, but the way you're using it,\nseems unlikely.\n\n> +\n>  def die(msg):\n>      if verbose:\n>          raise Exception(msg)\n> @@ -1994,6 +2005,11 @@ class P4Sync(Command, P4UserMap):\n>                  optparse.make_option(\"-/\", dest=\"cloneExclude\",\n>                                       action=\"append\", type=\"string\",\n>                                       help=\"exclude depot path\"),\n> +                optparse.make_option(\"--use-lfs-if-size-exceeds\", dest=\"lfsMinimumFileSize\", type=\"int\",\n> +                                     help=\"Use LFS to store files bigger than the given threshold in bytes.\"),\n> +                optparse.make_option(\"--use-lfs-for-extension\", dest=\"lfsFileExtensions\",\n> +                                     action=\"append\", type=\"string\",\n> +                                     help=\"Use LFS to store files with the given file extension(s).\"),\n>          ]\n>          self.description = \"\"\"Imports from Perforce into a git repository.\\n\n>      example:\n> @@ -2025,6 +2041,9 @@ class P4Sync(Command, P4UserMap):\n>          self.clientSpecDirs = None\n>          self.tempBranches = []\n>          self.tempBranchLocation = \"git-p4-tmp\"\n> +        self.lfsFiles = []\n> +        self.lfsMinimumFileSize = None\n> +        self.lfsFileExtensions = []\n>\n>          if gitConfig(\"git-p4.syncFromOrigin\") == \"false\":\n>              self.syncWithOrigin = False\n> @@ -2145,6 +2164,63 @@ class P4Sync(Command, P4UserMap):\n>\n>          return branches\n>\n> +    def writeToGitStream(self, gitMode, relPath, contents):\n> +        self.gitStream.write('M %s inline %s\\n' % (gitMode, relPath))\n> +        self.gitStream.write('data %d\\n' % sum(len(d) for d in contents))\n> +        for d in contents:\n> +            self.gitStream.write(d)\n> +        self.gitStream.write('\\n')\n> +\n> +    def writeGitAttributesToStream(self):\n> +        gitAttributes = [f + ' filter=lfs -text\\n' for f in self.lfsFiles if not self.hasFileLFSExtension(f)]\n> +        self.writeToGitStream(\n> +            '100644',\n> +            '.gitattributes',\n> +            ['*.' + f + ' filter=lfs -text\\n' for f in self.lfsFileExtensions] +\n> +            [f + ' filter=lfs -text\\n' for f in self.lfsFiles if not self.hasFileLFSExtension(f)]\n> +        )\n> +\n> +    def hasFileLFSExtension(self, relPath):\n> +        return reduce(\n> +            lambda a, b: a or b,\n> +            [relPath.endswith('.' + e) for e in self.lfsFileExtensions],\n> +            False\n> +        )\n> +\n> +    def isFileLargerThanLFSTreshold(self, relPath, contents):\n> +        return self.lfsMinimumFileSize and sum(len(d) for d in contents) >= self.lfsMinimumFileSize\n\nCould have a command-line option \"--try-compress-first\" (or some such)\nwhich compresses the file, and it it's very compressible, leaves it\nalone. It would trade speed of cloning with not-using-LFS-needlessly.\n\n> +\n> +    def generateLFSPointerFile(self, relPath, contents):\n> +        # Write P4 content to temp file\n> +        p4ContentTempFile = tempfile.NamedTemporaryFile(prefix='git-lfs', delete=False)\n> +        for d in contents:\n> +            p4ContentTempFile.write(d)\n> +        p4ContentTempFile.flush()\n> +\n> +        # Generate LFS pointer file based on P4 content\n> +        lfsProcess = subprocess.Popen(\n> +            ['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],\n> +            stdout=subprocess.PIPE\n> +        )\n> +        lfsPointerFile = lfsProcess.stdout.read()\n> +        if lfsProcess.wait():\n> +            die('git-lfs command failed. Did you install the extension?')\n\nWe're going to leave the P4 content file lying around undeleted here;\nis there any way to avoid that and cleanup nicely?\n\n> +        contents = [i+'\\n' for i in lfsPointerFile.split('\\n')[2:][:-1]]\n> +\n> +        # Write P4 content to LFS\n> +        oid = contents[1].split(' ')[1].split(':')[1][:-1]\n> +        oidPath = os.path.join(self.cloneDestination, '.git', 'lfs', 'objects', oid[:2], oid[2:4])\n> +        mkdir_p(oidPath)\n> +        shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))\n> +\n> +        # Update Git attributes\n> +        self.lfsFiles.append(relPath)\n> +        self.writeGitAttributesToStream()\n> +\n> +        # LFS Spec states that pointer files should not have the executable bit set.\n> +        gitMode = '100644'\n> +        return (gitMode, contents)\n> +\n>      # output one file from the P4 stream\n>      # - helper for streamP4Files\n>\n> @@ -2213,17 +2289,13 @@ class P4Sync(Command, P4UserMap):\n>              text = regexp.sub(r'$\\1$', text)\n>              contents = [ text ]\n>\n> -        self.gitStream.write(\"M %s inline %s\\n\" % (git_mode, relPath))\n> +        if relPath == '.gitattributes':\n> +            die('.gitattributes already exists in P4.')\n>\n> -        # total length...\n> -        length = 0\n> -        for d in contents:\n> -            length = length + len(d)\n> +        if self.isFileLargerThanLFSTreshold(relPath, contents) or self.hasFileLFSExtension(relPath):\n\ns/Treshold/Threshold/g\n\n\n> +            (git_mode, contents) = self.generateLFSPointerFile(relPath, contents)\n>\n> -        self.gitStream.write(\"data %d\\n\" % length)\n> -        for d in contents:\n> -            self.gitStream.write(d)\n> -        self.gitStream.write(\"\\n\")\n> +        self.writeToGitStream(git_mode, relPath, contents)\n>\n>      def streamOneP4Deletion(self, file):\n>          relPath = self.stripRepoPath(file['path'], self.branchPrefixes)\n> @@ -2231,6 +2303,10 @@ class P4Sync(Command, P4UserMap):\n>              sys.stderr.write(\"delete %s\\n\" % relPath)\n>          self.gitStream.write(\"D %s\\n\" % relPath)\n>\n> +        if relPath in self.lfsFiles:\n> +            self.lfsFiles.remove(relPath)\n> +            self.writeGitAttributesToStream()\n> +\n>      # handle another chunk of streaming data\n>      def streamP4FilesCb(self, marshalled):\n>\n> diff --git a/t/t9822-git-p4-lfs.sh b/t/t9822-git-p4-lfs.sh\n> new file mode 100755\n> index 0000000..b27bf29\n> --- /dev/null\n> +++ b/t/t9822-git-p4-lfs.sh\n> @@ -0,0 +1,277 @@\n> +#!/bin/sh\n> +\n> +test_description='Clone repositories and store files in LFS'\n> +\n> +( git lfs help ) >/dev/null 2>&1 || {\n\nDoes this need to be in a subshell?\n\n> +       skip_all='skipping git p4 LFS tests; no git lfs'\n> +       test_done\n> +}\n> +\n> +. ./lib-git-p4.sh\n> +\n> +test_expect_success 'start p4d' '\n> +       start_p4d\n> +'\n> +\n> +test_expect_success 'Create repo with binary files' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       (\n> +               cd \"$cli\" &&\n> +               echo \"text\" >file.txt &&\n> +               echo \"bin 13 bytes\" >file.dat &&\n> +               p4 add file.txt &&\n> +               p4 add file.dat &&\n> +               p4 submit -d \"Add text and binary file\" &&\n> +               echo \"bin 13 bytes\" >file2.bin &&\n> +               p4 add file2.bin &&\n> +               p4 submit -d \"Add another binary file with same content\"\n> +               echo \"bin 14 bytess\" >file3.bin &&\n> +               p4 add file3.bin &&\n> +               p4 submit -d \"Add another binary file with different content\"\n> +       )\n> +'\n> +\n> +test_expect_success 'Store files in LFS based on size (10 bytes)' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       git p4 clone --use-client-spec --use-lfs-if-size-exceeds=10 --destination=\"$git\" //depot@all &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               .git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               .git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n\nThis feels like it could be very fragile. Every time a new file gets\nadded to the tests we'll end up having to mess around with SHA1\ndigests. Surely all we care about is that LFS can recreate the files\nwe gave it originally?\n\nPlus it makes it quite hard to understand what's going on!\n\n> +               EOF\n> +               find \".git/lfs/objects\" -type f >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               size 13\n> +               EOF\n> +               cat file.dat >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               size 13\n> +               EOF\n> +               cat file2.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n> +               size 14\n> +               EOF\n> +               cat file3.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               file.dat filter=lfs -text\n> +               file2.bin filter=lfs -text\n> +               file3.bin filter=lfs -text\n> +               EOF\n> +               cat .gitattributes >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Store files in LFS based on size (14 bytes)' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       git p4 clone --use-client-spec --use-lfs-if-size-exceeds=14 --destination=\"$git\" //depot@all &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               .git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n> +               EOF\n> +               find \".git/lfs/objects\" -type f >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               bin 13 bytes\n> +               EOF\n> +               cat file.dat >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               bin 13 bytes\n> +               EOF\n> +               cat file2.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n> +               size 14\n> +               EOF\n> +               cat file3.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               file3.bin filter=lfs -text\n> +               EOF\n> +               cat .gitattributes >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Store files in LFS based on extension (dat)' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       git p4 clone --use-client-spec --use-lfs-for-extension=dat --destination=\"$git\" //depot@all &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               .git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               EOF\n> +               find \".git/lfs/objects\" -type f >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               size 13\n> +               EOF\n> +               cat file.dat >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               bin 13 bytes\n> +               EOF\n> +               cat file2.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               bin 14 bytess\n> +               EOF\n> +               cat file3.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               *.dat filter=lfs -text\n> +               EOF\n> +               cat .gitattributes >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Store files in LFS based on size (14 bytes) and extension (dat)' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       git p4 clone \\\n> +               --use-client-spec \\\n> +               --use-lfs-if-size-exceeds=14 \\\n> +               --use-lfs-for-extension=dat \\\n> +               --destination=\"$git\" //depot@all &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               .git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               .git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n> +               EOF\n> +               find \".git/lfs/objects\" -type f >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               size 13\n> +               EOF\n> +               cat file.dat >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               bin 13 bytes\n> +               EOF\n> +               cat file2.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n> +               size 14\n> +               EOF\n> +               cat file3.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               *.dat filter=lfs -text\n> +               file3.bin filter=lfs -text\n> +               EOF\n> +               cat .gitattributes >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Remove file from repo and store files in LFS based on size (10 bytes)' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       (\n> +               cd \"$cli\" &&\n> +               p4 delete file3.bin &&\n> +               p4 submit -d \"Remove file\"\n> +       ) &&\n> +\n> +       git p4 clone --use-client-spec --use-lfs-if-size-exceeds=10 --destination=\"$git\" //depot@all &&\n> +       test_when_finished cleanup_git &&\n> +       (\n> +               cd \"$git\" &&\n> +\n> +               # Note that file3 remains here as it referenced in the history\n> +               cat >expect <<-\\EOF &&\n> +               .git/lfs/objects/d4/43/d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               .git/lfs/objects/e5/fe/e5fec48503cd7b85eb9ffaea3311cde2fe9542078b9640369032b26bb5403fff\n> +               EOF\n> +               find \".git/lfs/objects\" -type f >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               size 13\n> +               EOF\n> +               cat file.dat >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               version https://git-lfs.github.com/spec/v1\n> +               oid sha256:d443795c1aa3ff7e62afd89d6a86bb84ceba0305f6c22151aa8ee95077a39101\n> +               size 13\n> +               EOF\n> +               cat file2.bin >actual &&\n> +               test_cmp expect actual &&\n> +\n> +               cat >expect <<-\\EOF &&\n> +               file.dat filter=lfs -text\n> +               file2.bin filter=lfs -text\n> +               EOF\n> +               cat .gitattributes >actual &&\n> +               test_cmp expect actual\n> +       )\n> +'\n> +\n> +test_expect_success 'Clone repo with existing .gitattributes file' '\n> +       client_view \"//depot/... //client/...\" &&\n> +       (\n> +               cd \"$cli\" &&\n> +\n> +               echo \"*.txt text\" >.gitattributes &&\n> +               p4 add .gitattributes &&\n> +               p4 submit -d \"Add .gitattributes\"\n> +       ) &&\n> +\n> +       test_must_fail git p4 clone --use-client-spec --destination=\"$git\" //depot 2>errs &&\n> +       grep \".gitattributes already exists in P4.\" errs\n> +'\n> +\n> +test_expect_success 'kill p4d' '\n> +       kill_p4d\n> +'\n> +\n> +test_done\n> --\n> 1.9.5 (Apple Git-50.3)\n>\n"},{"id":"268930","messageId":"9951C805-DFBF-47AC-B215-B9C657D4A852@gmail.com","threadId":"40206","inReplyTo":"CAE5ih7-TfLqwye8YpmZz90AVjYGQ8m0sF6GLKnG9pjvV8cTSjw@mail.gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-08-30T10:18:29Z","receivedAt":"2015-08-30T10:18:29Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Thanks for your feedback! \n\nI like the “handle big files” plugin kind of idea. However, I wonder if it makes sense to put more and more stuff into git-p4.py (>3000 LOC already). What do you think about splitting git-p4 into multiple files?\n\nRegarding Python 3: \nWould you drop Python 2 support or do you want to support Python 2/3 in parallel? I would prefer the former…\n\n- Lars\n\nOn 30 Aug 2015, at 10:49, Luke Diamand <luke@diamand.org> wrote:\n\n> On 28 August 2015 at 13:24,  <larsxschneider@gmail.com> wrote:\n>> From: Lars Schneider <larsxschneider@gmail.com>\n>> \n>> I am migrating huge Perforce repositories including history to Git. Some of them contain large files that would blow up the resulting Git repositories. This patch adds an option to store these files in Git LFS [1] on git-p4 clone.\n> \n> I'm a bit worried by this. LFS isn't the only way to handle large\n> files in git - there's also git annex (which I've used in a similar\n> situation) and obviously random homebrew solutions. We're going to end\n> up with git-p4 sprouting ever increasing numbers of\n> --use-XXX-if-size-exceeds options. On the other hand, having it\n> integrated into git-p4 is quite nice as it saves a lot of messing\n> around.\n> \n> Would it be possible as a start to have (within git-p4) a generic\n> spot-big-files-and-handle-them-differently patch, and a second patch\n> to add specific LFS support? That then means that other schemes would\n> be a lot easier to add in future.\n> \n> Some other comments inline.\n> \n>> \n>> In order to run the unit tests you need to install the Git LFS extension [2].\n>> \n>> Known limitations:\n>> The option \"use-lfs-if-size-exceeds\" looks at the uncompressed file size. Sometimes huge XML files are tiny if compressed. I wonder if there is an easy way to learn about the size of a file in a git pack file. I assume compressing it is the only way to know.\n>> \n>> Feedback is highly appreciated.\n>> \n>> Thank you,\n>> Lars\n>> \n>> \n>> [1] https://git-lfs.github.com/\n>> [2] https://github.com/github/git-lfs/releases/\n>> \n>> Lars Schneider (1):\n>>  git-p4: add option to store files in Git LFS on import\n>> \n>> Documentation/git-p4.txt |  12 ++\n>> git-p4.py                |  94 ++++++++++++++--\n>> t/t9822-git-p4-lfs.sh    | 277 +++++++++++++++++++++++++++++++++++++++++++++++\n>> 3 files changed, 374 insertions(+), 9 deletions(-)\n>> create mode 100755 t/t9822-git-p4-lfs.sh\n>> \n>> --\n>> 1.9.5 (Apple Git-50.3)\n> \n> Can you switch to a newer git - this one's quite old now so if there\n> are regressions introduced later, you won't know about them!\n"},{"id":"268935","messageId":"CAE5ih7-w5ZjO=QUE45yBvwyqdqKCADczrLt7=4W8X6BR+cy4PA@mail.gmail.com","threadId":"40206","inReplyTo":"9951C805-DFBF-47AC-B215-B9C657D4A852@gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-08-30T16:36:42Z","receivedAt":"2015-08-30T16:36:42Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 30 August 2015 at 11:18, Lars Schneider <larsxschneider@gmail.com> wrote:\n> Thanks for your feedback!\n>\n> I like the “handle big files” plugin kind of idea. However, I wonder if it makes sense to put more and more stuff into git-p4.py (>3000 LOC already). What do you think about splitting git-p4 into multiple files?\n\nI was wondering about that. I think for now, the simplicity of keeping\neverything in one file is worth the slight extra pain. I don't imagine\nthat the big-file-handler code would be very large.\n\n>\n> Regarding Python 3:\n> Would you drop Python 2 support or do you want to support Python 2/3 in parallel? I would prefer the former…\n\nFor quite some time we would need to support both; we can't just have\na release of git that one day breaks git-p4 for people stuck on Python\n2. But it might not be that hard to support both (though converting\nall those print statements could be quite tiresome).\n"},{"id":"269304","messageId":"1D65DD68-EE3F-4C87-9B5F-E436BA13704E@gmail.com","threadId":"40206","inReplyTo":"CAE5ih7-w5ZjO=QUE45yBvwyqdqKCADczrLt7=4W8X6BR+cy4PA@mail.gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2015-09-03T09:40:20Z","receivedAt":"2015-09-03T09:40:20Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 30 Aug 2015, at 18:36, Luke Diamand <luke@diamand.org> wrote:\n\n> On 30 August 2015 at 11:18, Lars Schneider <larsxschneider@gmail.com> wrote:\n>> Thanks for your feedback!\n>> \n>> I like the “handle big files” plugin kind of idea. However, I wonder if it makes sense to put more and more stuff into git-p4.py (>3000 LOC already). What do you think about splitting git-p4 into multiple files?\n> \n> I was wondering about that. I think for now, the simplicity of keeping\n> everything in one file is worth the slight extra pain. I don't imagine\n> that the big-file-handler code would be very large.\nOK.\n\n> \n>> \n>> Regarding Python 3:\n>> Would you drop Python 2 support or do you want to support Python 2/3 in parallel? I would prefer the former…\n> \n> For quite some time we would need to support both; we can't just have\n> a release of git that one day breaks git-p4 for people stuck on Python\n> 2. But it might not be that hard to support both (though converting\n> all those print statements could be quite tiresome).\nAgreed. However supporting both versions increases code complexity as well as testing effort. Would a compromise like the following work? We fork “git-p4.py” to “git-p4-python2.py” and just apply important bug fixes to that file. All new development happens on a Python 3 only git-p4.py. \n\nCheers,\nLars"},{"id":"269307","messageId":"CAE5ih79Zjg7Rk0OQ0VkTc+KGJ26dEk_iyOWWhT2qSGePRTDqog@mail.gmail.com","threadId":"40206","inReplyTo":"1D65DD68-EE3F-4C87-9B5F-E436BA13704E@gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2015-09-03T10:07:19Z","receivedAt":"2015-09-03T10:07:19Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":">>>\n>>> Regarding Python 3:\n>>> Would you drop Python 2 support or do you want to support Python 2/3 in parallel? I would prefer the former…\n>>\n>> For quite some time we would need to support both; we can't just have\n>> a release of git that one day breaks git-p4 for people stuck on Python\n>> 2. But it might not be that hard to support both (though converting\n>> all those print statements could be quite tiresome).\n> Agreed. However supporting both versions increases code complexity as well as testing effort. Would a compromise like the following work? We fork “git-p4.py” to “git-p4-python2.py” and just apply important bug fixes to that file. All new development happens on a Python 3 only git-p4.py.\n\nI'm not a python expert, but I think we're quite a way from that point\nanyway. I think we'd want to run 2to3 on it and make it work - at that\npoint it should work on both python 2.7 (and earlier? I don't know)\nand python 3.x. By the time that's done, we may well find that we\n_can_ just drop python2 support, or fork, as you suggest.\n\nRunning 2to3 also includes adding test cases for all the code that is\nin there that's not currently covered so that end-users don't find out\nthe hard way that we've missed bits. That's why I think it's a fairly\nlong-term goal.\n\nRegardless, I think we'd want to have a wider discussion about the\nbest way forward, and there doesn't seem much point having that\ndiscussion now when there's no actual code!\n"},{"id":"269308","messageId":"20150903101239.GP30659@serenity.lan","threadId":"40206","inReplyTo":"1D65DD68-EE3F-4C87-9B5F-E436BA13704E@gmail.com","subject":"Re: [RFC PATCH] git-p4: add option to store files in Git LFS on import","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2015-09-03T10:12:39Z","receivedAt":"2015-09-03T10:12:39Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Sep 03, 2015 at 11:40:20AM +0200, Lars Schneider wrote:\n> \n> On 30 Aug 2015, at 18:36, Luke Diamand <luke@diamand.org> wrote:\n> \n> > On 30 August 2015 at 11:18, Lars Schneider <larsxschneider@gmail.com> wrote:\n> >> Thanks for your feedback!\n> >> \n> >> I like the “handle big files” plugin kind of idea. However, I\n> >> wonder if it makes sense to put more and more stuff into git-p4.py\n> >> (>3000 LOC already). What do you think about splitting git-p4 into\n> >> multiple files?\n> > \n> > I was wondering about that. I think for now, the simplicity of keeping\n> > everything in one file is worth the slight extra pain. I don't imagine\n> > that the big-file-handler code would be very large.\n> OK.\n> \n> > \n> >> \n> >> Regarding Python 3:\n> >> Would you drop Python 2 support or do you want to support Python\n> >> 2/3 in parallel? I would prefer the former…\n> > \n> > For quite some time we would need to support both; we can't just have\n> > a release of git that one day breaks git-p4 for people stuck on Python\n> > 2. But it might not be that hard to support both (though converting\n> > all those print statements could be quite tiresome).\n> Agreed. However supporting both versions increases code complexity as\n> well as testing effort. Would a compromise like the following work? We\n> fork “git-p4.py” to “git-p4-python2.py” and just apply important bug\n> fixes to that file. All new development happens on a Python 3 only\n> git-p4.py. \n\nDocumentation/CodingGuidelines currently says:\n\n - As a minimum, we aim to be compatible with Python 2.6 and 2.7.\n\n - Where required libraries do not restrict us to Python 2, we try to\n   also be compatible with Python 3.1 and later.\n\nThat was added in commit 9ef43dd (CodingGuidelines: add Python coding\nguidelines, 2013-01-30), which gives the following rationale in the\ncommit message:\n\n - Advocating Python 3 support in all scripts is currently unrealistic\n   because:\n\n     - 'p4 -G' provides output in a format that is very hard to use with\n       Python 3 (and its documentation claims Python 3 is unsupported).\n\nHas that changed?\n\nI also found a message describing why the output is hard to use with\nPython 3:\n\n \thttp://permalink.gmane.org/gmane.comp.version-control.git/213316\n\nIf that problem can be solved, I don't think it would be difficult to\nsupport 2.6+ and 3.x with a single file.\n"}]}