On 19/10/15 19:43, larsxschneider@xxxxxxxxx wrote:
From: Lars Schneider <larsxschneider@xxxxxxxxx>
A changelist that contains only excluded files (e.g. via client spec or
branch prefix) will be imported as empty commit. Add option
"git-p4.ignoreEmptyCommits" to ignore these commits.
Signed-off-by: Lars Schneider <larsxschneider@xxxxxxxxx>
---
Documentation/git-p4.txt | 5 ++
git-p4.py | 41 ++++++++-----
t/t9826-git-p4-ignore-empty-commits.sh | 103 +++++++++++++++++++++++++++++++++
3 files changed, 133 insertions(+), 16 deletions(-)
create mode 100755 t/t9826-git-p4-ignore-empty-commits.sh
diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index 82aa5d6..f096a6a 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -510,6 +510,11 @@ 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.ignoreEmptyCommits::
+ A changelist that contains only excluded files will be imported
+ as empty commit. To ignore these commits set this boolean option
+ to 'true'.
s/as empty/as an empty/
Possibly putting 'true' in quotes could be confusing.
+
Submit variables
~~~~~~~~~~~~~~~~
git-p4.detectRenames::
diff --git a/git-p4.py b/git-p4.py
index 0093fa3..6c50c74 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2288,12 +2288,6 @@ class P4Sync(Command, P4UserMap):
filesToDelete = []
for f in files:
- # if using a client spec, only add the files that have
- # a path in the client
- if self.clientSpecDirs:
- if self.clientSpecDirs.map_in_client(f['path']) == "":
- continue
-
filesForCommit.append(f)
if f['action'] in self.delete_actions:
filesToDelete.append(f)
@@ -2368,18 +2362,33 @@ class P4Sync(Command, P4UserMap):
if self.verbose:
print "commit into %s" % branch
- # start with reading files; if that fails, we should not
- # create a commit.
- new_files = []
- for f in files:
- if [p for p in self.branchPrefixes if p4PathStartsWith(f['path'], p)]:
- new_files.append (f)
- else:
- sys.stderr.write("Ignoring file outside of prefix: %s\n" % f['path'])
-
if self.clientSpecDirs:
self.clientSpecDirs.update_client_spec_path_cache(files)
+ def inClientSpec(path):
This seems to be adding a new function in the middle of an existing
function. Is that right?
+ if not self.clientSpecDirs:
+ return True
+ inClientSpec = self.clientSpecDirs.map_in_client(path)
+ if not inClientSpec and self.verbose:
+ print '\n Ignoring file outside of client spec' % path
+ return inClientSpec
Any particular reason for putting a \n at the start of the message?
Also, could you use python3 style print stmnts, print("whatever") ?
+
+ def hasBranchPrefix(path):
+ if not self.branchPrefixes:
+ return True
+ hasPrefix = [p for p in self.branchPrefixes
+ if p4PathStartsWith(path, p)]
+ if hasPrefix and self.verbose:
+ print '\n Ignoring file outside of prefix: %s' % path
+ return hasPrefix
+
+ files = [f for f in files
+ if inClientSpec(f['path']) and hasBranchPrefix(f['path'])]
+
+ if not files and gitConfigBool('git-p4.ignoreEmptyCommits'):
+ print '\n Ignoring change %s as it would produce an empty commit.'
+ return
As with Junio's comment elsewhere, I worry about deletion here.
+
self.gitStream.write("commit %s\n" % branch)
# gitStream.write("mark :%s\n" % details["change"])
self.committedChanges.add(int(details["change"]))
@@ -2403,7 +2412,7 @@ class P4Sync(Command, P4UserMap):
print "parent %s" % parent
self.gitStream.write("from %s\n" % parent)
- self.streamP4Files(new_files)
+ self.streamP4Files(files)
self.gitStream.write("\n")
change = int(details["change"])
diff --git a/t/t9826-git-p4-ignore-empty-commits.sh b/t/t9826-git-p4-ignore-empty-commits.sh
new file mode 100755
index 0000000..5ddccde
--- /dev/null
+++ b/t/t9826-git-p4-ignore-empty-commits.sh
@@ -0,0 +1,103 @@
+#!/bin/sh
+
+test_description='Clone repositories and ignore empty commits'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'Create a repo' '
+ client_view "//depot/... //client/..." &&
+ (
+ cd "$cli" &&
+
+ mkdir -p subdir &&
+
+ >subdir/file1.txt &&
+ p4 add subdir/file1.txt &&
+ p4 submit -d "Add file 1" &&
+
+ >file2.txt &&
+ p4 add file2.txt &&
+ p4 submit -d "Add file 2" &&
+
+ >subdir/file3.txt &&
+ p4 add subdir/file3.txt &&
+ p4 submit -d "Add file 3"
+ )
+'
+
+test_expect_success 'Clone repo root path with all history' '
+ client_view "//depot/... //client/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+ cat >expect <<-\EOF &&
+Add file 3
+[git-p4: depot-paths = "//depot/": change = 3]
+
+Add file 2
+[git-p4: depot-paths = "//depot/": change = 2]
+
+Add file 1
+[git-p4: depot-paths = "//depot/": change = 1]
Could you not just test for existence of these files?
If the format of the magic comments that git-p4 ever changes, this will
break.
(There's a patch out there that gets git-p4 to use git notes, so it's
not as far-fetched as it sounds).
+
+ EOF
+ git log --format=%B >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'Clone repo subdir with all history' '
+ client_view "//depot/subdir/... //client/subdir/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+ cat >expect <<-\EOF &&
+Add file 3
+[git-p4: depot-paths = "//depot/": change = 3]
+
+Add file 2
+[git-p4: depot-paths = "//depot/": change = 2]
+
+Add file 1
+[git-p4: depot-paths = "//depot/": change = 1]
+
+ EOF
+ git log --format=%B >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'Clone repo subdir with all history but ignore empty commits' '
+ client_view "//depot/subdir/... //client/subdir/..." &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git init . &&
+ git config git-p4.ignoreEmptyCommits true &&
+ git p4 clone --use-client-spec --destination="$git" //depot@all &&
+ cat >expect <<-\EOF &&
+Add file 3
+[git-p4: depot-paths = "//depot/": change = 3]
+
+Add file 1
+[git-p4: depot-paths = "//depot/": change = 1]
+
+ EOF
+ git log --format=%B >actual &&
A deletion test would make me feel more comfortable!
Thanks!
Luke
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html