* [PATCH v2] git-p4: add support for large file systems
@ 2015-09-03 16:35 larsxschneider
2015-09-03 16:35 ` [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader larsxschneider
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: larsxschneider @ 2015-09-03 16:35 UTC (permalink / raw)
To: git; +Cc: luke, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
Diff to v1:
Fixed:
* simpler makedirs
* remove temp file if git-lfs command failed
* fix typos
* remove uneccessary sub shell for (git lfs help)
* make TC not dependable on hashes
Thanks Luke for the feedback!
Added:
* new gitConfigInt function
* support for generic large file systems based on gitattributes
* check compressed file size
Cheers,
Lars
Lars Schneider (4):
git-p4: add optional type specifier to gitConfig reader
git-p4: add gitConfigInt reader
git-p4: return an empty list if a list config has no values
git-p4: add support for large file systems
Documentation/git-p4.txt | 21 ++++
git-p4.py | 147 +++++++++++++++++++++++---
t/t9823-git-p4-lfs.sh | 263 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 418 insertions(+), 13 deletions(-)
create mode 100755 t/t9823-git-p4-lfs.sh
--
1.9.5 (Apple Git-50.3)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 16:35 [PATCH v2] git-p4: add support for large file systems larsxschneider
@ 2015-09-03 16:35 ` larsxschneider
2015-09-03 19:55 ` Luke Diamand
2015-09-03 16:35 ` [PATCH v2 2/4] git-p4: add gitConfigInt reader larsxschneider
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: larsxschneider @ 2015-09-03 16:35 UTC (permalink / raw)
To: git; +Cc: luke, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
git-p4.py | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 073f87b..c139cab 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -604,9 +604,12 @@ def gitBranchExists(branch):
_gitConfig = {}
-def gitConfig(key):
+def gitConfig(key, typeSpecifier=None):
if not _gitConfig.has_key(key):
- cmd = [ "git", "config", key ]
+ cmd = [ "git", "config" ]
+ if typeSpecifier:
+ cmd += [ typeSpecifier ]
+ cmd += [ key ]
s = read_pipe(cmd, ignore_error=True)
_gitConfig[key] = s.strip()
return _gitConfig[key]
@@ -617,10 +620,7 @@ def gitConfigBool(key):
in the config."""
if not _gitConfig.has_key(key):
- cmd = [ "git", "config", "--bool", key ]
- s = read_pipe(cmd, ignore_error=True)
- v = s.strip()
- _gitConfig[key] = v == "true"
+ _gitConfig[key] = gitConfig(key, '--bool') == "true"
return _gitConfig[key]
def gitConfigList(key):
--
1.9.5 (Apple Git-50.3)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/4] git-p4: add gitConfigInt reader
2015-09-03 16:35 [PATCH v2] git-p4: add support for large file systems larsxschneider
2015-09-03 16:35 ` [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader larsxschneider
@ 2015-09-03 16:35 ` larsxschneider
2015-09-03 19:57 ` Luke Diamand
2015-09-03 16:35 ` [PATCH v2 3/4] git-p4: return an empty list if a list config has no values larsxschneider
2015-09-03 16:35 ` [PATCH v2 4/4] git-p4: add support for large file systems larsxschneider
3 siblings, 1 reply; 17+ messages in thread
From: larsxschneider @ 2015-09-03 16:35 UTC (permalink / raw)
To: git; +Cc: luke, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
git-p4.py | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index c139cab..ae1a4d3 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -623,6 +623,17 @@ def gitConfigBool(key):
_gitConfig[key] = gitConfig(key, '--bool') == "true"
return _gitConfig[key]
+def gitConfigInt(key):
+ if not _gitConfig.has_key(key):
+ cmd = [ "git", "config", "--int", key ]
+ s = read_pipe(cmd, ignore_error=True)
+ v = s.strip()
+ try:
+ _gitConfig[key] = int(gitConfig(key, '--int'))
+ except Exception, e:
+ _gitConfig[key] = None
+ return _gitConfig[key]
+
def gitConfigList(key):
if not _gitConfig.has_key(key):
s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
--
1.9.5 (Apple Git-50.3)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/4] git-p4: return an empty list if a list config has no values
2015-09-03 16:35 [PATCH v2] git-p4: add support for large file systems larsxschneider
2015-09-03 16:35 ` [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader larsxschneider
2015-09-03 16:35 ` [PATCH v2 2/4] git-p4: add gitConfigInt reader larsxschneider
@ 2015-09-03 16:35 ` larsxschneider
2015-09-03 16:35 ` [PATCH v2 4/4] git-p4: add support for large file systems larsxschneider
3 siblings, 0 replies; 17+ messages in thread
From: larsxschneider @ 2015-09-03 16:35 UTC (permalink / raw)
To: git; +Cc: luke, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
git-p4.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index ae1a4d3..4d78e1c 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -638,6 +638,8 @@ def gitConfigList(key):
if not _gitConfig.has_key(key):
s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
_gitConfig[key] = s.strip().split(os.linesep)
+ if _gitConfig[key] == ['']:
+ _gitConfig[key] = []
return _gitConfig[key]
def p4BranchesInGit(branchesAreInRemotes=True):
--
1.9.5 (Apple Git-50.3)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/4] git-p4: add support for large file systems
2015-09-03 16:35 [PATCH v2] git-p4: add support for large file systems larsxschneider
` (2 preceding siblings ...)
2015-09-03 16:35 ` [PATCH v2 3/4] git-p4: return an empty list if a list config has no values larsxschneider
@ 2015-09-03 16:35 ` larsxschneider
2015-09-03 20:03 ` Luke Diamand
3 siblings, 1 reply; 17+ messages in thread
From: larsxschneider @ 2015-09-03 16:35 UTC (permalink / raw)
To: git; +Cc: luke, Lars Schneider
From: Lars Schneider <larsxschneider@gmail.com>
Perforce repositories can contain large (binary) files. Migrating these
repositories to Git generates very large local clones. External storage
systems such as LFS [1] or git-annex [2] try to address this problem.
Add a generic mechanism to detect large files based on extension,
uncompressed size, and/or compressed size. Add LFS as example
implementation.
[1] https://git-lfs.github.com/
[2] http://www.git-annex.org/
Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
Documentation/git-p4.txt | 21 ++++
git-p4.py | 126 +++++++++++++++++++++--
t/t9823-git-p4-lfs.sh | 263 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 401 insertions(+), 9 deletions(-)
create mode 100755 t/t9823-git-p4-lfs.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..eac5bad 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,27 @@ git-p4.useClientSpec::
option '--use-client-spec'. See the "CLIENT SPEC" section above.
This variable is a boolean, not the name of a p4 client.
+git-p4.largeFileSystem::
+ Specify the system that is used used for large (binary) files. Only
+ "LFS" [1] is supported right now. Download and install the Git LFS
+ command line extension to use this option.
+ [1] https://git-lfs.github.com/
+
+git-p4.largeFileExtensions::
+ All files matching a file extension in the list will be processed
+ by the large file system. Do not prefix the extensions with '.'.
+
+git-p4.largeFileThreshold::
+ All files with an uncompressed size exceeding the threshold will be
+ processed by the large file system. By default the threshold is
+ defined in bytes. Add the suffix k, m, or g to change the unit.
+
+git-p4.largeFileCompressedThreshold::
+ All files with an compressed size exceeding the threshold will be
+ processed by the large file system. This option might significantly
+ slow down your clone/sync process. By default the threshold is
+ defined in bytes. Add the suffix k, m, or g to change the unit.
+
Submit variables
~~~~~~~~~~~~~~~~
git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 4d78e1c..cde75a5 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -22,6 +22,8 @@ import platform
import re
import shutil
import stat
+import zipfile
+import zlib
try:
from subprocess import CalledProcessError
@@ -922,6 +924,51 @@ def wildcard_present(path):
m = re.search("[*#@%]", path)
return m is not None
+def largeFileSystem():
+ try:
+ return getattr(sys.modules[__name__], gitConfig('git-p4.largeFileSystem'))
+ except AttributeError as e:
+ die('Large file system not supported: %s' % gitConfig('git-p4.largeFileSystem'))
+
+class LFS:
+ @staticmethod
+ def description():
+ return 'LFS (see https://git-lfs.github.com/)'
+
+ @staticmethod
+ def attributeFilter():
+ return 'lfs'
+
+ @staticmethod
+ def generatePointer(cloneDestination, relPath, contents):
+ # Write P4 content to temp file
+ p4ContentTempFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
+ for d in contents:
+ p4ContentTempFile.write(d)
+ p4ContentTempFile.flush()
+
+ # Generate LFS pointer file based on P4 content
+ lfsProcess = subprocess.Popen(
+ ['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],
+ stdout=subprocess.PIPE
+ )
+ lfsPointerFile = lfsProcess.stdout.read()
+ if lfsProcess.wait():
+ os.remove(p4ContentTempFile.name)
+ die('git-lfs command failed. Did you install the extension?')
+ contents = [i+'\n' for i in lfsPointerFile.split('\n')[2:][:-1]]
+
+ # Write P4 content to LFS
+ oid = contents[1].split(' ')[1].split(':')[1][:-1]
+ oidPath = os.path.join(cloneDestination, '.git', 'lfs', 'objects', oid[:2], oid[2:4])
+ if not os.path.isdir(oidPath):
+ os.makedirs(oidPath)
+ shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))
+
+ # LFS Spec states that pointer files should not have the executable bit set.
+ gitMode = '100644'
+ return (gitMode, contents)
+
class Command:
def __init__(self):
self.usage = "usage: %prog [options]"
@@ -2038,6 +2085,7 @@ class P4Sync(Command, P4UserMap):
self.clientSpecDirs = None
self.tempBranches = []
self.tempBranchLocation = "git-p4-tmp"
+ self.largeFiles = []
if gitConfig("git-p4.syncFromOrigin") == "false":
self.syncWithOrigin = False
@@ -2158,6 +2206,59 @@ class P4Sync(Command, P4UserMap):
return branches
+ def writeToGitStream(self, gitMode, relPath, contents):
+ self.gitStream.write('M %s inline %s\n' % (gitMode, relPath))
+ self.gitStream.write('data %d\n' % sum(len(d) for d in contents))
+ for d in contents:
+ self.gitStream.write(d)
+ self.gitStream.write('\n')
+
+ def writeGitAttributesToStream(self):
+ self.writeToGitStream(
+ '100644',
+ '.gitattributes',
+ [
+ '\n',
+ '#\n',
+ '# %s\n' % largeFileSystem().description(),
+ '#\n',
+ ] +
+ ['*.' + f.replace(' ', '[:space:]') + ' filter=%s -text\n' % largeFileSystem().attributeFilter()
+ for f in gitConfigList("git-p4.largeFileExtensions")
+ ] +
+ ['/' + f.replace(' ', '[:space:]') + ' filter=%s -text\n' % largeFileSystem().attributeFilter()
+ for f in self.largeFiles if not self.hasLargeFileExtension(f)
+ ]
+ )
+
+ def hasLargeFileExtension(self, relPath):
+ return reduce(
+ lambda a, b: a or b,
+ [relPath.endswith('.' + e) for e in gitConfigList('git-p4.largeFileExtensions')],
+ False
+ )
+
+ def exceedsLargeFileThreshold(self, relPath, contents):
+ if gitConfigInt('git-p4.largeFileThreshold'):
+ contentsSize = sum(len(d) for d in contents)
+ if contentsSize > gitConfigInt('git-p4.largeFileThreshold'):
+ return True
+ if gitConfigInt('git-p4.largeFileCompressedThreshold'):
+ contentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
+ for d in contents:
+ contentFile.write(d)
+ contentFile.flush()
+ compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
+ zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
+ zf.write(contentFile.name, compress_type=zipfile.ZIP_DEFLATED)
+ zf.close()
+ compressedContentsSize = zf.infolist()[0].compress_size
+ os.remove(contentFile.name)
+ os.remove(compressedContentFile.name)
+ if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
+ return True
+ return False
+
# output one file from the P4 stream
# - helper for streamP4Files
@@ -2226,17 +2327,20 @@ class P4Sync(Command, P4UserMap):
text = regexp.sub(r'$\1$', text)
contents = [ text ]
- self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
+ if relPath == '.gitattributes':
+ die('.gitattributes already exists in P4.')
- # total length...
- length = 0
- for d in contents:
- length = length + len(d)
+ if (gitConfig('git-p4.largeFileSystem') and
+ ( self.exceedsLargeFileThreshold(relPath, contents) or
+ self.hasLargeFileExtension(relPath)
+ )):
+ (git_mode, contents) = largeFileSystem().generatePointer(self.cloneDestination, relPath, contents)
+ self.largeFiles.append(relPath)
+ self.writeGitAttributesToStream()
+ if verbose:
+ sys.stderr.write("%s added to large file system\n" % relPath)
- self.gitStream.write("data %d\n" % length)
- for d in contents:
- self.gitStream.write(d)
- self.gitStream.write("\n")
+ self.writeToGitStream(git_mode, relPath, contents)
def streamOneP4Deletion(self, file):
relPath = self.stripRepoPath(file['path'], self.branchPrefixes)
@@ -2244,6 +2348,10 @@ class P4Sync(Command, P4UserMap):
sys.stderr.write("delete %s\n" % relPath)
self.gitStream.write("D %s\n" % relPath)
+ if relPath in self.largeFiles:
+ self.largeFiles.remove(relPath)
+ self.writeGitAttributesToStream()
+
# handle another chunk of streaming data
def streamP4FilesCb(self, marshalled):
diff --git a/t/t9823-git-p4-lfs.sh b/t/t9823-git-p4-lfs.sh
new file mode 100755
index 0000000..a7587bf
--- /dev/null
+++ b/t/t9823-git-p4-lfs.sh
@@ -0,0 +1,263 @@
+#!/bin/sh
+
+test_description='Clone repositories and store files in LFS'
+
+git lfs help >/dev/null 2>&1 || {
+ skip_all='skipping git p4 LFS tests; no git lfs'
+ test_done
+}
+
+. ./lib-git-p4.sh
+
+test_file_in_lfs() {
+ FILE=$1
+ SIZE=$2
+ EXPECTED_CONTENT=$3
+ test_path_is_file $FILE &&
+ test_line_count = 3 $FILE &&
+ cat $FILE | grep "size $SIZE" &&
+ HASH=$(cat $FILE | grep "oid sha256:" | sed -e 's/oid sha256://g') &&
+ LFS_FILE=".git/lfs/objects/${HASH:0:2}/${HASH:2:2}/$HASH"
+ test_path_is_file $LFS_FILE &&
+ echo $EXPECTED_CONTENT >expect
+ test_cmp expect $LFS_FILE
+}
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'Create repo with binary files' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+
+ echo "txt 7b" >file1.txt &&
+ p4 add file1.txt &&
+ echo "bin 13 bytes" >file2.dat &&
+ p4 add file2.dat &&
+ p4 submit -d "Add text and binary file" &&
+
+ mkdir "path with spaces" &&
+ echo "bin 13 bytes" >"path with spaces/file3.bin" &&
+ p4 add "path with spaces/file3.bin" &&
+ p4 submit -d "Add another binary file with same content and spaces in path" &&
+
+ echo "bin 14 bytesx" >file4.bin &&
+ p4 add file4.bin &&
+ p4 submit -d "Add another binary file with different content"
+ )
+'
+
+test_expect_success 'Store files in LFS based on size (>12 bytes)' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.largeFileSystem LFS &&
+ git config git-p4.largeFileThreshold 12 &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+
+ test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
+ test_file_in_lfs "path with spaces/file3.bin" 13 "bin 13 bytes" &&
+ test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
+
+ find ".git/lfs/objects" -type f >actual &&
+ test_line_count = 2 actual &&
+
+ cat >expect <<-\EOF &&
+
+ #
+ # LFS (see https://git-lfs.github.com/)
+ #
+ /file2.dat filter=lfs -text
+ /path[:space:]with[:space:]spaces/file3.bin filter=lfs -text
+ /file4.bin filter=lfs -text
+ EOF
+ test_path_is_file .gitattributes &&
+ test_cmp expect .gitattributes
+ )
+'
+
+test_expect_success 'Store files in LFS based on size (>13 bytes)' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.largeFileSystem LFS &&
+ git config git-p4.largeFileThreshold 13 &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+
+ test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
+
+ find ".git/lfs/objects" -type f >actual &&
+ test_line_count = 1 actual &&
+
+ cat >expect <<-\EOF &&
+
+ #
+ # LFS (see https://git-lfs.github.com/)
+ #
+ /file4.bin filter=lfs -text
+ EOF
+ test_path_is_file .gitattributes &&
+ test_cmp expect .gitattributes
+ )
+'
+
+test_expect_success 'Store files in LFS based on extension (dat)' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.largeFileSystem LFS &&
+ git config git-p4.largeFileExtensions dat &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+
+ test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
+
+ find ".git/lfs/objects" -type f >actual &&
+ test_line_count = 1 actual &&
+
+ cat >expect <<-\EOF &&
+
+ #
+ # LFS (see https://git-lfs.github.com/)
+ #
+ *.dat filter=lfs -text
+ EOF
+ test_path_is_file .gitattributes &&
+ test_cmp expect .gitattributes
+ )
+'
+
+test_expect_success 'Store files in LFS based on size (>13 bytes) and extension (dat)' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.largeFileSystem LFS &&
+ git config git-p4.largeFileExtensions dat &&
+ git config git-p4.largeFileThreshold 13 &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+
+ test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
+ test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
+
+ find ".git/lfs/objects" -type f >actual &&
+ test_line_count = 2 actual &&
+
+ cat >expect <<-\EOF &&
+
+ #
+ # LFS (see https://git-lfs.github.com/)
+ #
+ *.dat filter=lfs -text
+ /file4.bin filter=lfs -text
+ EOF
+ test_path_is_file .gitattributes &&
+ test_cmp expect .gitattributes
+ )
+'
+
+test_expect_success 'Remove file from repo and store files in LFS based on size (>12 bytes)' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ p4 delete file4.bin &&
+ p4 submit -d "Remove file"
+ ) &&
+
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.largeFileSystem LFS &&
+ git config git-p4.largeFileThreshold 12 &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+
+ test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
+ test_file_in_lfs "path with spaces/file3.bin" 13 "bin 13 bytes" &&
+ ! test_path_is_file file4.bin &&
+
+ find ".git/lfs/objects" -type f >actual &&
+ # 3 is correct here as some commits in the tree still reference the deleted file
+ test_line_count = 3 actual
+
+ cat >expect <<-\EOF &&
+
+ #
+ # LFS (see https://git-lfs.github.com/)
+ #
+ /file2.dat filter=lfs -text
+ /path[:space:]with[:space:]spaces/file3.bin filter=lfs -text
+ EOF
+ test_path_is_file .gitattributes &&
+ test_cmp expect .gitattributes
+ )
+'
+
+test_expect_success 'Add big files to repo and store files in LFS based on compressed size (>19 bytes)' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+ echo "bin 24 bytesxxxxxxxxxxx" >file5.bin &&
+ p4 add file5.bin &&
+ p4 submit -d "Add another binary file with different content" &&
+
+ echo "bin 34 bytesxxxxxxxxxxxzzzzzzzzzz" >file6.bin &&
+ p4 add file6.bin &&
+ p4 submit -d "Add another binary file with different content"
+ ) &&
+
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.largeFileSystem LFS &&
+ git config git-p4.largeFileCompressedThreshold 19 &&
+ # We only import HEAD here ("@all" is missing!)
+ git p4 clone --use-client-spec --destination="$git" //depot &&
+
+ test_file_in_lfs file6.bin 13 "bin 34 bytesxxxxxxxxxxxzzzzzzzzzz" &&
+
+ find ".git/lfs/objects" -type f >actual &&
+ test_line_count = 1 actual
+
+ cat >expect <<-\EOF &&
+
+ #
+ # LFS (see https://git-lfs.github.com/)
+ #
+ /file6.bin filter=lfs -text
+ EOF
+ test_path_is_file .gitattributes &&
+ test_cmp expect .gitattributes
+ )
+'
+
+test_expect_success 'Clone repo with existing .gitattributes file' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+
+ echo "*.txt text" >.gitattributes &&
+ p4 add .gitattributes &&
+ p4 submit -d "Add .gitattributes"
+ ) &&
+
+ test_must_fail git p4 clone --use-client-spec --destination="$git" //depot 2>errs &&
+ grep ".gitattributes already exists in P4." errs
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.9.5 (Apple Git-50.3)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 16:35 ` [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader larsxschneider
@ 2015-09-03 19:55 ` Luke Diamand
2015-09-03 20:14 ` Lars Schneider
0 siblings, 1 reply; 17+ messages in thread
From: Luke Diamand @ 2015-09-03 19:55 UTC (permalink / raw)
To: larsxschneider, git
On 03/09/15 17:35, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
I think this commit may need some explanation!
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> git-p4.py | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index 073f87b..c139cab 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -604,9 +604,12 @@ def gitBranchExists(branch):
>
> _gitConfig = {}
>
> -def gitConfig(key):
> +def gitConfig(key, typeSpecifier=None):
> if not _gitConfig.has_key(key):
> - cmd = [ "git", "config", key ]
> + cmd = [ "git", "config" ]
> + if typeSpecifier:
> + cmd += [ typeSpecifier ]
> + cmd += [ key ]
> s = read_pipe(cmd, ignore_error=True)
> _gitConfig[key] = s.strip()
> return _gitConfig[key]
> @@ -617,10 +620,7 @@ def gitConfigBool(key):
> in the config."""
>
> if not _gitConfig.has_key(key):
> - cmd = [ "git", "config", "--bool", key ]
> - s = read_pipe(cmd, ignore_error=True)
> - v = s.strip()
> - _gitConfig[key] = v == "true"
> + _gitConfig[key] = gitConfig(key, '--bool') == "true"
> return _gitConfig[key]
>
> def gitConfigList(key):
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/4] git-p4: add gitConfigInt reader
2015-09-03 16:35 ` [PATCH v2 2/4] git-p4: add gitConfigInt reader larsxschneider
@ 2015-09-03 19:57 ` Luke Diamand
2015-09-03 20:17 ` Lars Schneider
0 siblings, 1 reply; 17+ messages in thread
From: Luke Diamand @ 2015-09-03 19:57 UTC (permalink / raw)
To: larsxschneider, git
On 03/09/15 17:35, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
Explanation?
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> git-p4.py | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/git-p4.py b/git-p4.py
> index c139cab..ae1a4d3 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -623,6 +623,17 @@ def gitConfigBool(key):
> _gitConfig[key] = gitConfig(key, '--bool') == "true"
> return _gitConfig[key]
>
> +def gitConfigInt(key):
> + if not _gitConfig.has_key(key):
> + cmd = [ "git", "config", "--int", key ]
> + s = read_pipe(cmd, ignore_error=True)
> + v = s.strip()
> + try:
> + _gitConfig[key] = int(gitConfig(key, '--int'))
> + except Exception, e:
Could do with specifying the actual type of the exception here (ValueError).
> + _gitConfig[key] = None
> + return _gitConfig[key]
> +
> def gitConfigList(key):
> if not _gitConfig.has_key(key):
> s = read_pipe(["git", "config", "--get-all", key], ignore_error=True)
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] git-p4: add support for large file systems
2015-09-03 16:35 ` [PATCH v2 4/4] git-p4: add support for large file systems larsxschneider
@ 2015-09-03 20:03 ` Luke Diamand
2015-09-03 20:49 ` Lars Schneider
0 siblings, 1 reply; 17+ messages in thread
From: Luke Diamand @ 2015-09-03 20:03 UTC (permalink / raw)
To: larsxschneider, git
On 03/09/15 17:35, larsxschneider@gmail.com wrote:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> Perforce repositories can contain large (binary) files. Migrating these
> repositories to Git generates very large local clones. External storage
> systems such as LFS [1] or git-annex [2] try to address this problem.
>
> Add a generic mechanism to detect large files based on extension,
> uncompressed size, and/or compressed size. Add LFS as example
> implementation.
Can you split this into "add mechanism for large file support" and then
"add LFS implementation".
It will make it easier to review if nothing else.
Some other comments inline.
Thanks!
Luke
>
> [1] https://git-lfs.github.com/
> [2] http://www.git-annex.org/
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
> Documentation/git-p4.txt | 21 ++++
> git-p4.py | 126 +++++++++++++++++++++--
> t/t9823-git-p4-lfs.sh | 263 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 401 insertions(+), 9 deletions(-)
> create mode 100755 t/t9823-git-p4-lfs.sh
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index 82aa5d6..eac5bad 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -510,6 +510,27 @@ git-p4.useClientSpec::
> option '--use-client-spec'. See the "CLIENT SPEC" section above.
> This variable is a boolean, not the name of a p4 client.
>
> +git-p4.largeFileSystem::
> + Specify the system that is used used for large (binary) files. Only
> + "LFS" [1] is supported right now. Download and install the Git LFS
> + command line extension to use this option.
> + [1] https://git-lfs.github.com/
> +
> +git-p4.largeFileExtensions::
> + All files matching a file extension in the list will be processed
> + by the large file system. Do not prefix the extensions with '.'.
> +
> +git-p4.largeFileThreshold::
> + All files with an uncompressed size exceeding the threshold will be
> + processed by the large file system. By default the threshold is
> + defined in bytes. Add the suffix k, m, or g to change the unit.
> +
> +git-p4.largeFileCompressedThreshold::
> + All files with an compressed size exceeding the threshold will be
s/an compressed/a compressed/
> + processed by the large file system. This option might significantly
> + slow down your clone/sync process. By default the threshold is
> + defined in bytes. Add the suffix k, m, or g to change the unit.
> +
> Submit variables
> ~~~~~~~~~~~~~~~~
> git-p4.detectRenames::
> diff --git a/git-p4.py b/git-p4.py
> index 4d78e1c..cde75a5 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -22,6 +22,8 @@ import platform
> import re
> import shutil
> import stat
> +import zipfile
> +import zlib
>
> try:
> from subprocess import CalledProcessError
> @@ -922,6 +924,51 @@ def wildcard_present(path):
> m = re.search("[*#@%]", path)
> return m is not None
>
> +def largeFileSystem():
> + try:
> + return getattr(sys.modules[__name__], gitConfig('git-p4.largeFileSystem'))
> + except AttributeError as e:
> + die('Large file system not supported: %s' % gitConfig('git-p4.largeFileSystem'))
> +
> +class LFS:
Docstrings?
> + @staticmethod
> + def description():
> + return 'LFS (see https://git-lfs.github.com/)'
> +
> + @staticmethod
> + def attributeFilter():
> + return 'lfs'
> +
> + @staticmethod
> + def generatePointer(cloneDestination, relPath, contents):
> + # Write P4 content to temp file
> + p4ContentTempFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
delete=False, doesn't that mean that if anything goes wrong, we leave
large files lying around? Perhaps that doesn't matter, I don't know.
> + for d in contents:
> + p4ContentTempFile.write(d)
> + p4ContentTempFile.flush()
These seems like behaviour that could live in the base class or the main
git-p4 code. It doesn't look LFS-specific.
> +
> + # Generate LFS pointer file based on P4 content
> + lfsProcess = subprocess.Popen(
> + ['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],
> + stdout=subprocess.PIPE
> + )
> + lfsPointerFile = lfsProcess.stdout.read()
> + if lfsProcess.wait():
> + os.remove(p4ContentTempFile.name)
> + die('git-lfs command failed. Did you install the extension?')
> + contents = [i+'\n' for i in lfsPointerFile.split('\n')[2:][:-1]]
> +
> + # Write P4 content to LFS
> + oid = contents[1].split(' ')[1].split(':')[1][:-1]
> + oidPath = os.path.join(cloneDestination, '.git', 'lfs', 'objects', oid[:2], oid[2:4])
> + if not os.path.isdir(oidPath):
> + os.makedirs(oidPath)
> + shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))
This also does not look LFS-specific.
> +
> + # LFS Spec states that pointer files should not have the executable bit set.
> + gitMode = '100644'
> + return (gitMode, contents)
> +
> class Command:
> def __init__(self):
> self.usage = "usage: %prog [options]"
> @@ -2038,6 +2085,7 @@ class P4Sync(Command, P4UserMap):
> self.clientSpecDirs = None
> self.tempBranches = []
> self.tempBranchLocation = "git-p4-tmp"
> + self.largeFiles = []
>
> if gitConfig("git-p4.syncFromOrigin") == "false":
> self.syncWithOrigin = False
> @@ -2158,6 +2206,59 @@ class P4Sync(Command, P4UserMap):
>
> return branches
>
> + def writeToGitStream(self, gitMode, relPath, contents):
> + self.gitStream.write('M %s inline %s\n' % (gitMode, relPath))
> + self.gitStream.write('data %d\n' % sum(len(d) for d in contents))
> + for d in contents:
> + self.gitStream.write(d)
> + self.gitStream.write('\n')
> +
> + def writeGitAttributesToStream(self):
> + self.writeToGitStream(
> + '100644',
> + '.gitattributes',
> + [
> + '\n',
> + '#\n',
> + '# %s\n' % largeFileSystem().description(),
> + '#\n',
> + ] +
> + ['*.' + f.replace(' ', '[:space:]') + ' filter=%s -text\n' % largeFileSystem().attributeFilter()
> + for f in gitConfigList("git-p4.largeFileExtensions")
> + ] +
> + ['/' + f.replace(' ', '[:space:]') + ' filter=%s -text\n' % largeFileSystem().attributeFilter()
> + for f in self.largeFiles if not self.hasLargeFileExtension(f)
> + ]
> + )
> +
> + def hasLargeFileExtension(self, relPath):
> + return reduce(
> + lambda a, b: a or b,
> + [relPath.endswith('.' + e) for e in gitConfigList('git-p4.largeFileExtensions')],
> + False
> + )
> +
> + def exceedsLargeFileThreshold(self, relPath, contents):
> + if gitConfigInt('git-p4.largeFileThreshold'):
> + contentsSize = sum(len(d) for d in contents)
> + if contentsSize > gitConfigInt('git-p4.largeFileThreshold'):
> + return True
> + if gitConfigInt('git-p4.largeFileCompressedThreshold'):
> + contentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
> + for d in contents:
> + contentFile.write(d)
> + contentFile.flush()
> + compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
> + zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
> + zf.write(contentFile.name, compress_type=zipfile.ZIP_DEFLATED)
> + zf.close()
> + compressedContentsSize = zf.infolist()[0].compress_size
> + os.remove(contentFile.name)
> + os.remove(compressedContentFile.name)
> + if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
> + return True
> + return False
> +
> # output one file from the P4 stream
> # - helper for streamP4Files
>
> @@ -2226,17 +2327,20 @@ class P4Sync(Command, P4UserMap):
> text = regexp.sub(r'$\1$', text)
> contents = [ text ]
>
> - self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
> + if relPath == '.gitattributes':
> + die('.gitattributes already exists in P4.')
>
> - # total length...
> - length = 0
> - for d in contents:
> - length = length + len(d)
> + if (gitConfig('git-p4.largeFileSystem') and
> + ( self.exceedsLargeFileThreshold(relPath, contents) or
> + self.hasLargeFileExtension(relPath)
> + )):
> + (git_mode, contents) = largeFileSystem().generatePointer(self.cloneDestination, relPath, contents)
> + self.largeFiles.append(relPath)
> + self.writeGitAttributesToStream()
> + if verbose:
> + sys.stderr.write("%s added to large file system\n" % relPath)
>
> - self.gitStream.write("data %d\n" % length)
> - for d in contents:
> - self.gitStream.write(d)
> - self.gitStream.write("\n")
> + self.writeToGitStream(git_mode, relPath, contents)
>
> def streamOneP4Deletion(self, file):
> relPath = self.stripRepoPath(file['path'], self.branchPrefixes)
> @@ -2244,6 +2348,10 @@ class P4Sync(Command, P4UserMap):
> sys.stderr.write("delete %s\n" % relPath)
> self.gitStream.write("D %s\n" % relPath)
>
> + if relPath in self.largeFiles:
> + self.largeFiles.remove(relPath)
> + self.writeGitAttributesToStream()
> +
> # handle another chunk of streaming data
> def streamP4FilesCb(self, marshalled):
>
> diff --git a/t/t9823-git-p4-lfs.sh b/t/t9823-git-p4-lfs.sh
> new file mode 100755
> index 0000000..a7587bf
> --- /dev/null
> +++ b/t/t9823-git-p4-lfs.sh
> @@ -0,0 +1,263 @@
> +#!/bin/sh
> +
> +test_description='Clone repositories and store files in LFS'
> +
> +git lfs help >/dev/null 2>&1 || {
> + skip_all='skipping git p4 LFS tests; no git lfs'
> + test_done
> +}
> +
> +. ./lib-git-p4.sh
> +
> +test_file_in_lfs() {
> + FILE=$1
> + SIZE=$2
> + EXPECTED_CONTENT=$3
> + test_path_is_file $FILE &&
> + test_line_count = 3 $FILE &&
> + cat $FILE | grep "size $SIZE" &&
> + HASH=$(cat $FILE | grep "oid sha256:" | sed -e 's/oid sha256://g') &&
> + LFS_FILE=".git/lfs/objects/${HASH:0:2}/${HASH:2:2}/$HASH"
> + test_path_is_file $LFS_FILE &&
> + echo $EXPECTED_CONTENT >expect
> + test_cmp expect $LFS_FILE
> +}
> +
> +test_expect_success 'start p4d' '
> + start_p4d
> +'
> +
> +test_expect_success 'Create repo with binary files' '
> + client_view "//depot/... //client/..." &&
> + (
> + cd "$cli" &&
> +
> + echo "txt 7b" >file1.txt &&
> + p4 add file1.txt &&
> + echo "bin 13 bytes" >file2.dat &&
> + p4 add file2.dat &&
> + p4 submit -d "Add text and binary file" &&
> +
> + mkdir "path with spaces" &&
> + echo "bin 13 bytes" >"path with spaces/file3.bin" &&
> + p4 add "path with spaces/file3.bin" &&
> + p4 submit -d "Add another binary file with same content and spaces in path" &&
> +
> + echo "bin 14 bytesx" >file4.bin &&
> + p4 add file4.bin &&
> + p4 submit -d "Add another binary file with different content"
> + )
> +'
> +
> +test_expect_success 'Store files in LFS based on size (>12 bytes)' '
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config git-p4.largeFileSystem LFS &&
> + git config git-p4.largeFileThreshold 12 &&
> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
> +
> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
> + test_file_in_lfs "path with spaces/file3.bin" 13 "bin 13 bytes" &&
> + test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
> +
> + find ".git/lfs/objects" -type f >actual &&
> + test_line_count = 2 actual &&
> +
> + cat >expect <<-\EOF &&
> +
> + #
> + # LFS (see https://git-lfs.github.com/)
> + #
> + /file2.dat filter=lfs -text
> + /path[:space:]with[:space:]spaces/file3.bin filter=lfs -text
> + /file4.bin filter=lfs -text
> + EOF
> + test_path_is_file .gitattributes &&
> + test_cmp expect .gitattributes
> + )
> +'
> +
> +test_expect_success 'Store files in LFS based on size (>13 bytes)' '
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config git-p4.largeFileSystem LFS &&
> + git config git-p4.largeFileThreshold 13 &&
> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
> +
> + test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
> +
> + find ".git/lfs/objects" -type f >actual &&
> + test_line_count = 1 actual &&
> +
> + cat >expect <<-\EOF &&
> +
> + #
> + # LFS (see https://git-lfs.github.com/)
> + #
> + /file4.bin filter=lfs -text
> + EOF
> + test_path_is_file .gitattributes &&
> + test_cmp expect .gitattributes
> + )
> +'
> +
> +test_expect_success 'Store files in LFS based on extension (dat)' '
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config git-p4.largeFileSystem LFS &&
> + git config git-p4.largeFileExtensions dat &&
> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
> +
> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
> +
> + find ".git/lfs/objects" -type f >actual &&
> + test_line_count = 1 actual &&
> +
> + cat >expect <<-\EOF &&
> +
> + #
> + # LFS (see https://git-lfs.github.com/)
> + #
> + *.dat filter=lfs -text
> + EOF
> + test_path_is_file .gitattributes &&
> + test_cmp expect .gitattributes
> + )
> +'
> +
> +test_expect_success 'Store files in LFS based on size (>13 bytes) and extension (dat)' '
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config git-p4.largeFileSystem LFS &&
> + git config git-p4.largeFileExtensions dat &&
> + git config git-p4.largeFileThreshold 13 &&
> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
> +
> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
> + test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
> +
> + find ".git/lfs/objects" -type f >actual &&
> + test_line_count = 2 actual &&
> +
> + cat >expect <<-\EOF &&
> +
> + #
> + # LFS (see https://git-lfs.github.com/)
> + #
> + *.dat filter=lfs -text
> + /file4.bin filter=lfs -text
> + EOF
> + test_path_is_file .gitattributes &&
> + test_cmp expect .gitattributes
> + )
> +'
> +
> +test_expect_success 'Remove file from repo and store files in LFS based on size (>12 bytes)' '
> + client_view "//depot/... //client/..." &&
> + (
> + cd "$cli" &&
> + p4 delete file4.bin &&
> + p4 submit -d "Remove file"
> + ) &&
> +
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config git-p4.largeFileSystem LFS &&
> + git config git-p4.largeFileThreshold 12 &&
> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
> +
> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
> + test_file_in_lfs "path with spaces/file3.bin" 13 "bin 13 bytes" &&
> + ! test_path_is_file file4.bin &&
> +
> + find ".git/lfs/objects" -type f >actual &&
> + # 3 is correct here as some commits in the tree still reference the deleted file
> + test_line_count = 3 actual
> +
> + cat >expect <<-\EOF &&
> +
> + #
> + # LFS (see https://git-lfs.github.com/)
> + #
> + /file2.dat filter=lfs -text
> + /path[:space:]with[:space:]spaces/file3.bin filter=lfs -text
> + EOF
> + test_path_is_file .gitattributes &&
> + test_cmp expect .gitattributes
> + )
> +'
> +
> +test_expect_success 'Add big files to repo and store files in LFS based on compressed size (>19 bytes)' '
> + client_view "//depot/... //client/..." &&
> + (
> + cd "$cli" &&
> + echo "bin 24 bytesxxxxxxxxxxx" >file5.bin &&
> + p4 add file5.bin &&
> + p4 submit -d "Add another binary file with different content" &&
> +
> + echo "bin 34 bytesxxxxxxxxxxxzzzzzzzzzz" >file6.bin &&
> + p4 add file6.bin &&
> + p4 submit -d "Add another binary file with different content"
> + ) &&
> +
> + client_view "//depot/... //client/..." &&
> + test_when_finished cleanup_git &&
> + (
> + cd "$git" &&
> + git init . &&
> + git config git-p4.largeFileSystem LFS &&
> + git config git-p4.largeFileCompressedThreshold 19 &&
> + # We only import HEAD here ("@all" is missing!)
> + git p4 clone --use-client-spec --destination="$git" //depot &&
> +
> + test_file_in_lfs file6.bin 13 "bin 34 bytesxxxxxxxxxxxzzzzzzzzzz" &&
> +
> + find ".git/lfs/objects" -type f >actual &&
> + test_line_count = 1 actual
> +
> + cat >expect <<-\EOF &&
> +
> + #
> + # LFS (see https://git-lfs.github.com/)
> + #
> + /file6.bin filter=lfs -text
> + EOF
> + test_path_is_file .gitattributes &&
> + test_cmp expect .gitattributes
> + )
> +'
> +
> +test_expect_success 'Clone repo with existing .gitattributes file' '
> + client_view "//depot/... //client/..." &&
> + (
> + cd "$cli" &&
> +
> + echo "*.txt text" >.gitattributes &&
> + p4 add .gitattributes &&
> + p4 submit -d "Add .gitattributes"
> + ) &&
> +
> + test_must_fail git p4 clone --use-client-spec --destination="$git" //depot 2>errs &&
> + grep ".gitattributes already exists in P4." errs
> +'
> +
> +test_expect_success 'kill p4d' '
> + kill_p4d
> +'
> +
> +test_done
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 19:55 ` Luke Diamand
@ 2015-09-03 20:14 ` Lars Schneider
2015-09-03 20:18 ` Junio C Hamano
0 siblings, 1 reply; 17+ messages in thread
From: Lars Schneider @ 2015-09-03 20:14 UTC (permalink / raw)
To: Luke Diamand; +Cc: git
On 03 Sep 2015, at 21:55, Luke Diamand <luke@diamand.org> wrote:
> On 03/09/15 17:35, larsxschneider@gmail.com wrote:
>> From: Lars Schneider <larsxschneider@gmail.com>
>>
>
> I think this commit may need some explanation!
The functions “gitConfig” and “gitConfigBool” are almost identical. Make “gitConfig” more generic by adding an optional type specifier. Use the type specifier “—bool” with “gitConfig” to implement “gitConfigBool. This prepares the implementation of other type specifiers such as “—int”.
OK?
Thank you,
Lars
>
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> git-p4.py | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index 073f87b..c139cab 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -604,9 +604,12 @@ def gitBranchExists(branch):
>>
>> _gitConfig = {}
>>
>> -def gitConfig(key):
>> +def gitConfig(key, typeSpecifier=None):
>> if not _gitConfig.has_key(key):
>> - cmd = [ "git", "config", key ]
>> + cmd = [ "git", "config" ]
>> + if typeSpecifier:
>> + cmd += [ typeSpecifier ]
>> + cmd += [ key ]
>> s = read_pipe(cmd, ignore_error=True)
>> _gitConfig[key] = s.strip()
>> return _gitConfig[key]
>> @@ -617,10 +620,7 @@ def gitConfigBool(key):
>> in the config."""
>>
>> if not _gitConfig.has_key(key):
>> - cmd = [ "git", "config", "--bool", key ]
>> - s = read_pipe(cmd, ignore_error=True)
>> - v = s.strip()
>> - _gitConfig[key] = v == "true"
>> + _gitConfig[key] = gitConfig(key, '--bool') == "true"
>> return _gitConfig[key]
>>
>> def gitConfigList(key):
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/4] git-p4: add gitConfigInt reader
2015-09-03 19:57 ` Luke Diamand
@ 2015-09-03 20:17 ` Lars Schneider
2015-09-03 20:20 ` Luke Diamand
0 siblings, 1 reply; 17+ messages in thread
From: Lars Schneider @ 2015-09-03 20:17 UTC (permalink / raw)
To: Luke Diamand; +Cc: git
On 03 Sep 2015, at 21:57, Luke Diamand <luke@diamand.org> wrote:
> On 03/09/15 17:35, larsxschneider@gmail.com wrote:
>> From: Lars Schneider <larsxschneider@gmail.com>
>>
>
> Explanation?
Add a git config reader for integer variables. Please note that the git config implementation automatically supports k, m, and g suffixes.
OK?
>
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> git-p4.py | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/git-p4.py b/git-p4.py
>> index c139cab..ae1a4d3 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -623,6 +623,17 @@ def gitConfigBool(key):
>> _gitConfig[key] = gitConfig(key, '--bool') == "true"
>> return _gitConfig[key]
>>
>> +def gitConfigInt(key):
>> + if not _gitConfig.has_key(key):
>> + cmd = [ "git", "config", "--int", key ]
>> + s = read_pipe(cmd, ignore_error=True)
>> + v = s.strip()
>> + try:
>> + _gitConfig[key] = int(gitConfig(key, '--int'))
>> + except Exception, e:
>
> Could do with specifying the actual type of the exception here (ValueError).
Will do!
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 20:14 ` Lars Schneider
@ 2015-09-03 20:18 ` Junio C Hamano
2015-09-03 20:53 ` Lars Schneider
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2015-09-03 20:18 UTC (permalink / raw)
To: Lars Schneider; +Cc: Luke Diamand, git
Lars Schneider <larsxschneider@gmail.com> writes:
> On 03 Sep 2015, at 21:55, Luke Diamand <luke@diamand.org> wrote:
>
>> On 03/09/15 17:35, larsxschneider@gmail.com wrote:
>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>
>>
>> I think this commit may need some explanation!
>
> The functions “gitConfig” and “gitConfigBool” are almost
> identical. Make “gitConfig” more generic by adding an optional type
> specifier. Use the type specifier “—bool” with “gitConfig” to
> implement “gitConfigBool. This prepares the implementation of other
> type specifiers such as “—int”.
OK.
> OK?
Not really ;-). The point of Luke's message is that all of the
above belong to the log message, I think.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/4] git-p4: add gitConfigInt reader
2015-09-03 20:17 ` Lars Schneider
@ 2015-09-03 20:20 ` Luke Diamand
0 siblings, 0 replies; 17+ messages in thread
From: Luke Diamand @ 2015-09-03 20:20 UTC (permalink / raw)
To: Lars Schneider; +Cc: git
On 03/09/15 21:17, Lars Schneider wrote:
>
> On 03 Sep 2015, at 21:57, Luke Diamand <luke@diamand.org> wrote:
>
>> On 03/09/15 17:35, larsxschneider@gmail.com wrote:
>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>
>>
>> Explanation?
> Add a git config reader for integer variables. Please note that the git config implementation automatically supports k, m, and g suffixes.
Sorry, I should have been clearer. The commit needs a comment to say
both what it's doing ("adding a git config int reader") and why.
c.f. Documentation/SubmittingChanges.
Luke
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] git-p4: add support for large file systems
2015-09-03 20:03 ` Luke Diamand
@ 2015-09-03 20:49 ` Lars Schneider
2015-09-04 8:58 ` Jeff King
0 siblings, 1 reply; 17+ messages in thread
From: Lars Schneider @ 2015-09-03 20:49 UTC (permalink / raw)
To: Luke Diamand; +Cc: Git List, peff
On 03 Sep 2015, at 22:03, Luke Diamand <luke@diamand.org> wrote:
> On 03/09/15 17:35, larsxschneider@gmail.com wrote:
>> From: Lars Schneider <larsxschneider@gmail.com>
>>
>> Perforce repositories can contain large (binary) files. Migrating these
>> repositories to Git generates very large local clones. External storage
>> systems such as LFS [1] or git-annex [2] try to address this problem.
>>
>> Add a generic mechanism to detect large files based on extension,
>> uncompressed size, and/or compressed size. Add LFS as example
>> implementation.
>
> Can you split this into "add mechanism for large file support" and then "add LFS implementation".
>
> It will make it easier to review if nothing else.
Fun fact: I had it that way first and then decided against it. I split it this way:
commit 1: generic impl + docs
commits 2: lfs impl + TC
OK?
>
> Some other comments inline.
>
> Thanks!
> Luke
>
>>
>> [1] https://git-lfs.github.com/
>> [2] http://www.git-annex.org/
>>
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> Documentation/git-p4.txt | 21 ++++
>> git-p4.py | 126 +++++++++++++++++++++--
>> t/t9823-git-p4-lfs.sh | 263 +++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 401 insertions(+), 9 deletions(-)
>> create mode 100755 t/t9823-git-p4-lfs.sh
>>
>> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
>> index 82aa5d6..eac5bad 100644
>> --- a/Documentation/git-p4.txt
>> +++ b/Documentation/git-p4.txt
>> @@ -510,6 +510,27 @@ git-p4.useClientSpec::
>> option '--use-client-spec'. See the "CLIENT SPEC" section above.
>> This variable is a boolean, not the name of a p4 client.
>>
>> +git-p4.largeFileSystem::
>> + Specify the system that is used used for large (binary) files. Only
>> + "LFS" [1] is supported right now. Download and install the Git LFS
>> + command line extension to use this option.
>> + [1] https://git-lfs.github.com/
>> +
>> +git-p4.largeFileExtensions::
>> + All files matching a file extension in the list will be processed
>> + by the large file system. Do not prefix the extensions with '.'.
>> +
>> +git-p4.largeFileThreshold::
>> + All files with an uncompressed size exceeding the threshold will be
>> + processed by the large file system. By default the threshold is
>> + defined in bytes. Add the suffix k, m, or g to change the unit.
>> +
>> +git-p4.largeFileCompressedThreshold::
>> + All files with an compressed size exceeding the threshold will be
>
> s/an compressed/a compressed/
ok
>
>> + processed by the large file system. This option might significantly
>> + slow down your clone/sync process. By default the threshold is
>> + defined in bytes. Add the suffix k, m, or g to change the unit.
>> +
>> Submit variables
>> ~~~~~~~~~~~~~~~~
>> git-p4.detectRenames::
>> diff --git a/git-p4.py b/git-p4.py
>> index 4d78e1c..cde75a5 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -22,6 +22,8 @@ import platform
>> import re
>> import shutil
>> import stat
>> +import zipfile
>> +import zlib
>>
>> try:
>> from subprocess import CalledProcessError
>> @@ -922,6 +924,51 @@ def wildcard_present(path):
>> m = re.search("[*#@%]", path)
>> return m is not None
>>
>> +def largeFileSystem():
>> + try:
>> + return getattr(sys.modules[__name__], gitConfig('git-p4.largeFileSystem'))
>> + except AttributeError as e:
>> + die('Large file system not supported: %s' % gitConfig('git-p4.largeFileSystem'))
>> +
>> +class LFS:
>
> Docstrings?
OK.
>
>
>> + @staticmethod
>> + def description():
>> + return 'LFS (see https://git-lfs.github.com/)'
>> +
>> + @staticmethod
>> + def attributeFilter():
>> + return 'lfs'
>> +
>> + @staticmethod
>> + def generatePointer(cloneDestination, relPath, contents):
>> + # Write P4 content to temp file
>> + p4ContentTempFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>
> delete=False, doesn't that mean that if anything goes wrong, we leave large files lying around? Perhaps that doesn't matter, I don't know.
delete=True means the file is deleted as soon as it is closed
source: https://docs.python.org/2/library/tempfile.html
I don’t close the file (sounds like a bug?!) so in theory delete=True would work. However, I am moving the file a few lines below.
>
>> + for d in contents:
>> + p4ContentTempFile.write(d)
>> + p4ContentTempFile.flush()
>
> These seems like behaviour that could live in the base class or the main git-p4 code. It doesn't look LFS-specific.
ok
>
>> +
>> + # Generate LFS pointer file based on P4 content
>> + lfsProcess = subprocess.Popen(
>> + ['git', 'lfs', 'pointer', '--file=' + p4ContentTempFile.name],
>> + stdout=subprocess.PIPE
>> + )
>> + lfsPointerFile = lfsProcess.stdout.read()
>> + if lfsProcess.wait():
>> + os.remove(p4ContentTempFile.name)
>> + die('git-lfs command failed. Did you install the extension?')
>> + contents = [i+'\n' for i in lfsPointerFile.split('\n')[2:][:-1]]
>> +
>> + # Write P4 content to LFS
>> + oid = contents[1].split(' ')[1].split(':')[1][:-1]
>> + oidPath = os.path.join(cloneDestination, '.git', 'lfs', 'objects', oid[:2], oid[2:4])
>> + if not os.path.isdir(oidPath):
>> + os.makedirs(oidPath)
>> + shutil.move(p4ContentTempFile.name, os.path.join(oidPath, oid))
>
> This also does not look LFS-specific.
ok
Thank you! One thing I missed to noticed in the commit message:
Right now the LFS uploads need to be executed manually. I will automate this with git-p4 in v3.
Do you want to test this feature against a real backend? In that case you would need a LFS enabled GitHub account. If you don’t have one, maybe _Jeff King_ can help?
Thanks,
Lars
>
>> +
>> + # LFS Spec states that pointer files should not have the executable bit set.
>> + gitMode = '100644'
>> + return (gitMode, contents)
>> +
>> class Command:
>> def __init__(self):
>> self.usage = "usage: %prog [options]"
>> @@ -2038,6 +2085,7 @@ class P4Sync(Command, P4UserMap):
>> self.clientSpecDirs = None
>> self.tempBranches = []
>> self.tempBranchLocation = "git-p4-tmp"
>> + self.largeFiles = []
>>
>> if gitConfig("git-p4.syncFromOrigin") == "false":
>> self.syncWithOrigin = False
>> @@ -2158,6 +2206,59 @@ class P4Sync(Command, P4UserMap):
>>
>> return branches
>>
>> + def writeToGitStream(self, gitMode, relPath, contents):
>> + self.gitStream.write('M %s inline %s\n' % (gitMode, relPath))
>> + self.gitStream.write('data %d\n' % sum(len(d) for d in contents))
>> + for d in contents:
>> + self.gitStream.write(d)
>> + self.gitStream.write('\n')
>> +
>> + def writeGitAttributesToStream(self):
>> + self.writeToGitStream(
>> + '100644',
>> + '.gitattributes',
>> + [
>> + '\n',
>> + '#\n',
>> + '# %s\n' % largeFileSystem().description(),
>> + '#\n',
>> + ] +
>> + ['*.' + f.replace(' ', '[:space:]') + ' filter=%s -text\n' % largeFileSystem().attributeFilter()
>> + for f in gitConfigList("git-p4.largeFileExtensions")
>> + ] +
>> + ['/' + f.replace(' ', '[:space:]') + ' filter=%s -text\n' % largeFileSystem().attributeFilter()
>> + for f in self.largeFiles if not self.hasLargeFileExtension(f)
>> + ]
>> + )
>> +
>> + def hasLargeFileExtension(self, relPath):
>> + return reduce(
>> + lambda a, b: a or b,
>> + [relPath.endswith('.' + e) for e in gitConfigList('git-p4.largeFileExtensions')],
>> + False
>> + )
>> +
>> + def exceedsLargeFileThreshold(self, relPath, contents):
>> + if gitConfigInt('git-p4.largeFileThreshold'):
>> + contentsSize = sum(len(d) for d in contents)
>> + if contentsSize > gitConfigInt('git-p4.largeFileThreshold'):
>> + return True
>> + if gitConfigInt('git-p4.largeFileCompressedThreshold'):
>> + contentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>> + for d in contents:
>> + contentFile.write(d)
>> + contentFile.flush()
>> + compressedContentFile = tempfile.NamedTemporaryFile(prefix='git-p4-large-file', delete=False)
>> + zf = zipfile.ZipFile(compressedContentFile.name, mode='w')
>> + zf.write(contentFile.name, compress_type=zipfile.ZIP_DEFLATED)
>> + zf.close()
>> + compressedContentsSize = zf.infolist()[0].compress_size
>> + os.remove(contentFile.name)
>> + os.remove(compressedContentFile.name)
>> + if compressedContentsSize > gitConfigInt('git-p4.largeFileCompressedThreshold'):
>> + return True
>> + return False
>> +
>> # output one file from the P4 stream
>> # - helper for streamP4Files
>>
>> @@ -2226,17 +2327,20 @@ class P4Sync(Command, P4UserMap):
>> text = regexp.sub(r'$\1$', text)
>> contents = [ text ]
>>
>> - self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))
>> + if relPath == '.gitattributes':
>> + die('.gitattributes already exists in P4.')
>>
>> - # total length...
>> - length = 0
>> - for d in contents:
>> - length = length + len(d)
>> + if (gitConfig('git-p4.largeFileSystem') and
>> + ( self.exceedsLargeFileThreshold(relPath, contents) or
>> + self.hasLargeFileExtension(relPath)
>> + )):
>> + (git_mode, contents) = largeFileSystem().generatePointer(self.cloneDestination, relPath, contents)
>> + self.largeFiles.append(relPath)
>> + self.writeGitAttributesToStream()
>> + if verbose:
>> + sys.stderr.write("%s added to large file system\n" % relPath)
>>
>> - self.gitStream.write("data %d\n" % length)
>> - for d in contents:
>> - self.gitStream.write(d)
>> - self.gitStream.write("\n")
>> + self.writeToGitStream(git_mode, relPath, contents)
>>
>> def streamOneP4Deletion(self, file):
>> relPath = self.stripRepoPath(file['path'], self.branchPrefixes)
>> @@ -2244,6 +2348,10 @@ class P4Sync(Command, P4UserMap):
>> sys.stderr.write("delete %s\n" % relPath)
>> self.gitStream.write("D %s\n" % relPath)
>>
>> + if relPath in self.largeFiles:
>> + self.largeFiles.remove(relPath)
>> + self.writeGitAttributesToStream()
>> +
>> # handle another chunk of streaming data
>> def streamP4FilesCb(self, marshalled):
>>
>> diff --git a/t/t9823-git-p4-lfs.sh b/t/t9823-git-p4-lfs.sh
>> new file mode 100755
>> index 0000000..a7587bf
>> --- /dev/null
>> +++ b/t/t9823-git-p4-lfs.sh
>> @@ -0,0 +1,263 @@
>> +#!/bin/sh
>> +
>> +test_description='Clone repositories and store files in LFS'
>> +
>> +git lfs help >/dev/null 2>&1 || {
>> + skip_all='skipping git p4 LFS tests; no git lfs'
>> + test_done
>> +}
>> +
>> +. ./lib-git-p4.sh
>> +
>> +test_file_in_lfs() {
>> + FILE=$1
>> + SIZE=$2
>> + EXPECTED_CONTENT=$3
>> + test_path_is_file $FILE &&
>> + test_line_count = 3 $FILE &&
>> + cat $FILE | grep "size $SIZE" &&
>> + HASH=$(cat $FILE | grep "oid sha256:" | sed -e 's/oid sha256://g') &&
>> + LFS_FILE=".git/lfs/objects/${HASH:0:2}/${HASH:2:2}/$HASH"
>> + test_path_is_file $LFS_FILE &&
>> + echo $EXPECTED_CONTENT >expect
>> + test_cmp expect $LFS_FILE
>> +}
>> +
>> +test_expect_success 'start p4d' '
>> + start_p4d
>> +'
>> +
>> +test_expect_success 'Create repo with binary files' '
>> + client_view "//depot/... //client/..." &&
>> + (
>> + cd "$cli" &&
>> +
>> + echo "txt 7b" >file1.txt &&
>> + p4 add file1.txt &&
>> + echo "bin 13 bytes" >file2.dat &&
>> + p4 add file2.dat &&
>> + p4 submit -d "Add text and binary file" &&
>> +
>> + mkdir "path with spaces" &&
>> + echo "bin 13 bytes" >"path with spaces/file3.bin" &&
>> + p4 add "path with spaces/file3.bin" &&
>> + p4 submit -d "Add another binary file with same content and spaces in path" &&
>> +
>> + echo "bin 14 bytesx" >file4.bin &&
>> + p4 add file4.bin &&
>> + p4 submit -d "Add another binary file with different content"
>> + )
>> +'
>> +
>> +test_expect_success 'Store files in LFS based on size (>12 bytes)' '
>> + client_view "//depot/... //client/..." &&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git" &&
>> + git init . &&
>> + git config git-p4.largeFileSystem LFS &&
>> + git config git-p4.largeFileThreshold 12 &&
>> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
>> +
>> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
>> + test_file_in_lfs "path with spaces/file3.bin" 13 "bin 13 bytes" &&
>> + test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
>> +
>> + find ".git/lfs/objects" -type f >actual &&
>> + test_line_count = 2 actual &&
>> +
>> + cat >expect <<-\EOF &&
>> +
>> + #
>> + # LFS (see https://git-lfs.github.com/)
>> + #
>> + /file2.dat filter=lfs -text
>> + /path[:space:]with[:space:]spaces/file3.bin filter=lfs -text
>> + /file4.bin filter=lfs -text
>> + EOF
>> + test_path_is_file .gitattributes &&
>> + test_cmp expect .gitattributes
>> + )
>> +'
>> +
>> +test_expect_success 'Store files in LFS based on size (>13 bytes)' '
>> + client_view "//depot/... //client/..." &&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git" &&
>> + git init . &&
>> + git config git-p4.largeFileSystem LFS &&
>> + git config git-p4.largeFileThreshold 13 &&
>> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
>> +
>> + test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
>> +
>> + find ".git/lfs/objects" -type f >actual &&
>> + test_line_count = 1 actual &&
>> +
>> + cat >expect <<-\EOF &&
>> +
>> + #
>> + # LFS (see https://git-lfs.github.com/)
>> + #
>> + /file4.bin filter=lfs -text
>> + EOF
>> + test_path_is_file .gitattributes &&
>> + test_cmp expect .gitattributes
>> + )
>> +'
>> +
>> +test_expect_success 'Store files in LFS based on extension (dat)' '
>> + client_view "//depot/... //client/..." &&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git" &&
>> + git init . &&
>> + git config git-p4.largeFileSystem LFS &&
>> + git config git-p4.largeFileExtensions dat &&
>> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
>> +
>> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
>> +
>> + find ".git/lfs/objects" -type f >actual &&
>> + test_line_count = 1 actual &&
>> +
>> + cat >expect <<-\EOF &&
>> +
>> + #
>> + # LFS (see https://git-lfs.github.com/)
>> + #
>> + *.dat filter=lfs -text
>> + EOF
>> + test_path_is_file .gitattributes &&
>> + test_cmp expect .gitattributes
>> + )
>> +'
>> +
>> +test_expect_success 'Store files in LFS based on size (>13 bytes) and extension (dat)' '
>> + client_view "//depot/... //client/..." &&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git" &&
>> + git init . &&
>> + git config git-p4.largeFileSystem LFS &&
>> + git config git-p4.largeFileExtensions dat &&
>> + git config git-p4.largeFileThreshold 13 &&
>> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
>> +
>> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
>> + test_file_in_lfs file4.bin 14 "bin 14 bytesx" &&
>> +
>> + find ".git/lfs/objects" -type f >actual &&
>> + test_line_count = 2 actual &&
>> +
>> + cat >expect <<-\EOF &&
>> +
>> + #
>> + # LFS (see https://git-lfs.github.com/)
>> + #
>> + *.dat filter=lfs -text
>> + /file4.bin filter=lfs -text
>> + EOF
>> + test_path_is_file .gitattributes &&
>> + test_cmp expect .gitattributes
>> + )
>> +'
>> +
>> +test_expect_success 'Remove file from repo and store files in LFS based on size (>12 bytes)' '
>> + client_view "//depot/... //client/..." &&
>> + (
>> + cd "$cli" &&
>> + p4 delete file4.bin &&
>> + p4 submit -d "Remove file"
>> + ) &&
>> +
>> + client_view "//depot/... //client/..." &&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git" &&
>> + git init . &&
>> + git config git-p4.largeFileSystem LFS &&
>> + git config git-p4.largeFileThreshold 12 &&
>> + git p4 clone --use-client-spec --destination="$git" //depot@all &&
>> +
>> + test_file_in_lfs file2.dat 13 "bin 13 bytes" &&
>> + test_file_in_lfs "path with spaces/file3.bin" 13 "bin 13 bytes" &&
>> + ! test_path_is_file file4.bin &&
>> +
>> + find ".git/lfs/objects" -type f >actual &&
>> + # 3 is correct here as some commits in the tree still reference the deleted file
>> + test_line_count = 3 actual
>> +
>> + cat >expect <<-\EOF &&
>> +
>> + #
>> + # LFS (see https://git-lfs.github.com/)
>> + #
>> + /file2.dat filter=lfs -text
>> + /path[:space:]with[:space:]spaces/file3.bin filter=lfs -text
>> + EOF
>> + test_path_is_file .gitattributes &&
>> + test_cmp expect .gitattributes
>> + )
>> +'
>> +
>> +test_expect_success 'Add big files to repo and store files in LFS based on compressed size (>19 bytes)' '
>> + client_view "//depot/... //client/..." &&
>> + (
>> + cd "$cli" &&
>> + echo "bin 24 bytesxxxxxxxxxxx" >file5.bin &&
>> + p4 add file5.bin &&
>> + p4 submit -d "Add another binary file with different content" &&
>> +
>> + echo "bin 34 bytesxxxxxxxxxxxzzzzzzzzzz" >file6.bin &&
>> + p4 add file6.bin &&
>> + p4 submit -d "Add another binary file with different content"
>> + ) &&
>> +
>> + client_view "//depot/... //client/..." &&
>> + test_when_finished cleanup_git &&
>> + (
>> + cd "$git" &&
>> + git init . &&
>> + git config git-p4.largeFileSystem LFS &&
>> + git config git-p4.largeFileCompressedThreshold 19 &&
>> + # We only import HEAD here ("@all" is missing!)
>> + git p4 clone --use-client-spec --destination="$git" //depot &&
>> +
>> + test_file_in_lfs file6.bin 13 "bin 34 bytesxxxxxxxxxxxzzzzzzzzzz" &&
>> +
>> + find ".git/lfs/objects" -type f >actual &&
>> + test_line_count = 1 actual
>> +
>> + cat >expect <<-\EOF &&
>> +
>> + #
>> + # LFS (see https://git-lfs.github.com/)
>> + #
>> + /file6.bin filter=lfs -text
>> + EOF
>> + test_path_is_file .gitattributes &&
>> + test_cmp expect .gitattributes
>> + )
>> +'
>> +
>> +test_expect_success 'Clone repo with existing .gitattributes file' '
>> + client_view "//depot/... //client/..." &&
>> + (
>> + cd "$cli" &&
>> +
>> + echo "*.txt text" >.gitattributes &&
>> + p4 add .gitattributes &&
>> + p4 submit -d "Add .gitattributes"
>> + ) &&
>> +
>> + test_must_fail git p4 clone --use-client-spec --destination="$git" //depot 2>errs &&
>> + grep ".gitattributes already exists in P4." errs
>> +'
>> +
>> +test_expect_success 'kill p4d' '
>> + kill_p4d
>> +'
>> +
>> +test_done
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 20:18 ` Junio C Hamano
@ 2015-09-03 20:53 ` Lars Schneider
2015-09-03 21:31 ` Junio C Hamano
0 siblings, 1 reply; 17+ messages in thread
From: Lars Schneider @ 2015-09-03 20:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Luke Diamand, git
On 03 Sep 2015, at 22:18, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Schneider <larsxschneider@gmail.com> writes:
>
>> On 03 Sep 2015, at 21:55, Luke Diamand <luke@diamand.org> wrote:
>>
>>> On 03/09/15 17:35, larsxschneider@gmail.com wrote:
>>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>>
>>>
>>> I think this commit may need some explanation!
>>
>> The functions “gitConfig” and “gitConfigBool” are almost
>> identical. Make “gitConfig” more generic by adding an optional type
>> specifier. Use the type specifier “—bool” with “gitConfig” to
>> implement “gitConfigBool. This prepares the implementation of other
>> type specifiers such as “—int”.
>
> OK.
>
>> OK?
>
> Not really ;-). The point of Luke's message is that all of the
> above belong to the log message, I think.
Right, it is my intention to add this as commit message. I just wanted to check upfront if the message is what he expects.
That leads me to a general question:
In case I agree with a reviewer. What is the more appropriate action? A response like the one above or a new role that includes the change right away? I don’t want to spam the list with lots of tiny changes…
Thanks!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 20:53 ` Lars Schneider
@ 2015-09-03 21:31 ` Junio C Hamano
2015-09-04 7:49 ` Lars Schneider
0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2015-09-03 21:31 UTC (permalink / raw)
To: Lars Schneider; +Cc: Luke Diamand, git
Lars Schneider <larsxschneider@gmail.com> writes:
> In case I agree with a reviewer. What is the more appropriate action?
> A response like the one above or a new role that includes the change
> right away? I don’t want to spam the list with lots of tiny changes…
Responding to review comment like you did with "will do" is
perfectly fine.
When you do think you will (eventually) want to send an updated
patch, there are things other than "will do" that you can say. "I
understand what you said, but I am not sure how exactly to make this
better. Perhaps lend me a help?" is good, too.
An explanation, followed by "ok?", in response to "it is unclear
what you are doing here" (commenting on code) or to "I cannot
understand what this is trying to say" (commenting on log message),
is problematic because your intention becomes ambiguous.
The reviewers are rarely saying "I do not understand; educate me."
but "I do not understand, and it is likely many others don't, too.
Make it more easily understandable." is what they mean.
An explanation with "ok?" can be taken as a sign that you mistook
the review comment as "educate me".
What I meant was .... Do you think commenting the code here
with the above description is good enough? Or do you think
of a way to restructure the code itself to be more self
evident?
or something like that may be a way to avoid the ambiguity.
Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader
2015-09-03 21:31 ` Junio C Hamano
@ 2015-09-04 7:49 ` Lars Schneider
0 siblings, 0 replies; 17+ messages in thread
From: Lars Schneider @ 2015-09-04 7:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Luke Diamand, git
On 03 Sep 2015, at 23:31, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Schneider <larsxschneider@gmail.com> writes:
>
>> In case I agree with a reviewer. What is the more appropriate action?
>> A response like the one above or a new role that includes the change
>> right away? I don’t want to spam the list with lots of tiny changes…
>
> Responding to review comment like you did with "will do" is
> perfectly fine.
>
> When you do think you will (eventually) want to send an updated
> patch, there are things other than "will do" that you can say. "I
> understand what you said, but I am not sure how exactly to make this
> better. Perhaps lend me a help?" is good, too.
>
> An explanation, followed by "ok?", in response to "it is unclear
> what you are doing here" (commenting on code) or to "I cannot
> understand what this is trying to say" (commenting on log message),
> is problematic because your intention becomes ambiguous.
>
> The reviewers are rarely saying "I do not understand; educate me."
> but "I do not understand, and it is likely many others don't, too.
> Make it more easily understandable." is what they mean.
>
> An explanation with "ok?" can be taken as a sign that you mistook
> the review comment as "educate me".
>
> What I meant was .... Do you think commenting the code here
> with the above description is good enough? Or do you think
> of a way to restructure the code itself to be more self
> evident?
>
> or something like that may be a way to avoid the ambiguity.
>
> Thanks.
Thank you for the explanation! :-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/4] git-p4: add support for large file systems
2015-09-03 20:49 ` Lars Schneider
@ 2015-09-04 8:58 ` Jeff King
0 siblings, 0 replies; 17+ messages in thread
From: Jeff King @ 2015-09-04 8:58 UTC (permalink / raw)
To: Lars Schneider; +Cc: Luke Diamand, Git List
On Thu, Sep 03, 2015 at 10:49:36PM +0200, Lars Schneider wrote:
> Do you want to test this feature against a real backend? In that case
> you would need a LFS enabled GitHub account. If you don’t have one,
> maybe _Jeff King_ can help?
You can sign up here:
https://github.com/early_access/git-lfs
I don't know what the turnaround time is like, but if you (or anybody
doing development work around git-lfs) needs it expedited, email me
off-list and I can look into it.
-Peff
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-09-04 8:58 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-03 16:35 [PATCH v2] git-p4: add support for large file systems larsxschneider
2015-09-03 16:35 ` [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader larsxschneider
2015-09-03 19:55 ` Luke Diamand
2015-09-03 20:14 ` Lars Schneider
2015-09-03 20:18 ` Junio C Hamano
2015-09-03 20:53 ` Lars Schneider
2015-09-03 21:31 ` Junio C Hamano
2015-09-04 7:49 ` Lars Schneider
2015-09-03 16:35 ` [PATCH v2 2/4] git-p4: add gitConfigInt reader larsxschneider
2015-09-03 19:57 ` Luke Diamand
2015-09-03 20:17 ` Lars Schneider
2015-09-03 20:20 ` Luke Diamand
2015-09-03 16:35 ` [PATCH v2 3/4] git-p4: return an empty list if a list config has no values larsxschneider
2015-09-03 16:35 ` [PATCH v2 4/4] git-p4: add support for large file systems larsxschneider
2015-09-03 20:03 ` Luke Diamand
2015-09-03 20:49 ` Lars Schneider
2015-09-04 8:58 ` Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).