* [PATCH 1/1] git-p4: recover from inconsistent perforce history
@ 2019-02-06 19:42 andrew
2019-02-07 8:04 ` Luke Diamand
0 siblings, 1 reply; 5+ messages in thread
From: andrew @ 2019-02-06 19:42 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Luke Diamand, Andrew Oakley
From: Andrew Oakley <andrew@adoakley.name>
Perforce allows you commit files and directories with the same name, so
you could have files //depot/foo and //depot/foo/bar both checked in. A
p4 sync of a repository in this state fails. Deleting one of the files
recovers the repository.
When this happens we want git-p4 to recover in the same way as perforce.
Signed-off-by: Andrew Oakley <andrew@adoakley.name>
---
git-p4.py | 41 ++++++++++++++++++++++--
t/t9834-git-p4-file-dir-bug.sh | 58 ++++++++++++++++++++++++++++++++++
2 files changed, 97 insertions(+), 2 deletions(-)
create mode 100755 t/t9834-git-p4-file-dir-bug.sh
diff --git a/git-p4.py b/git-p4.py
index 3e12774f96..6bf2bbbcec 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3012,6 +3012,43 @@ def hasBranchPrefix(self, path):
print('Ignoring file outside of prefix: {0}'.format(path))
return hasPrefix
+ def isIncluded(self, path):
+ return self.inClientSpec(path) and self.hasBranchPrefix(path)
+
+ def findShadowedFiles(self, files, change):
+ # Perforce allows you commit files and directories with the same name,
+ # so you could have files //depot/foo and //depot/foo/bar both checked
+ # in. A p4 sync of a repository in this state fails. Deleting one of
+ # the files recovers the repository.
+ #
+ # Git will not allow the broken state to exist and only the most recent
+ # of the conflicting names is left in the repository. When one of the
+ # conflicting files is deleted we need to re-add the other one to make
+ # sure the git repository recovers in the same way as perforce.
+ deleted = [f for f in files if f['action'] in self.delete_actions]
+ to_check = set()
+ for f in deleted:
+ path = f['path']
+ to_check.add(path + '/...')
+ while True:
+ path = path.rsplit("/", 1)[0]
+ if path == "/" or path in to_check:
+ break
+ to_check.add(path)
+ to_check = [p for p in to_check if self.isIncluded(p)]
+ if to_check:
+ stat_result = p4CmdList(
+ ["fstat", "-T", "depotFile,headRev,headType"] +
+ ["%s@%s" % (p, change) for p in to_check])
+ for record in stat_result:
+ if record['code'] != 'stat':
+ continue
+ files.append({
+ 'action': 'add',
+ 'path': record['depotFile'],
+ 'rev': record['headRev'],
+ 'type': record['headType']})
+
def commit(self, details, files, branch, parent = "", allow_empty=False):
epoch = details["time"]
author = details["user"]
@@ -3023,8 +3060,8 @@ def commit(self, details, files, branch, parent = "", allow_empty=False):
if self.clientSpecDirs:
self.clientSpecDirs.update_client_spec_path_cache(files)
- files = [f for f in files
- if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]
+ files = [f for f in files if self.isIncluded(f['path'])]
+ self.findShadowedFiles(files, details["change"])
if gitConfigBool('git-p4.keepEmptyCommits'):
allow_empty = True
diff --git a/t/t9834-git-p4-file-dir-bug.sh b/t/t9834-git-p4-file-dir-bug.sh
new file mode 100755
index 0000000000..9839a3d2bb
--- /dev/null
+++ b/t/t9834-git-p4-file-dir-bug.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='git p4 directory/file bug handling
+
+This test creates files and directories with the same name in perforce and
+checks that git-p4 recovers from the error at the same time as the perforce
+repository.'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'init depot' '
+ (
+ cd "$cli" &&
+
+ touch add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ p4 add add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ mkdir add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ touch add_dir_add_file_del_file/file add_dir_add_file_del_dir/file &&
+ p4 add add_dir_add_file_del_file/file add_dir_add_file_del_dir/file &&
+ p4 submit -d "add initial" &&
+
+ rm -f add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ mkdir add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ touch add_file_add_dir_del_file/file add_file_add_dir_del_dir/file &&
+ p4 add add_file_add_dir_del_file/file add_file_add_dir_del_dir/file &&
+ rm -rf add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ touch add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ p4 add add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ p4 submit -d "add conflicting" &&
+
+ p4 delete -k add_file_add_dir_del_file &&
+ p4 delete -k add_file_add_dir_del_dir/file &&
+ p4 delete -k add_dir_add_file_del_file &&
+ p4 delete -k add_dir_add_file_del_dir/file &&
+ p4 submit -d "delete conflicting"
+ )
+'
+
+test_expect_success 'clone with git-p4' '
+ git p4 clone --dest="$git" //depot/@all
+'
+
+test_expect_success 'check final contents' '
+ test_path_is_dir "$git/add_file_add_dir_del_file" &&
+ test_path_is_file "$git/add_file_add_dir_del_dir" &&
+ test_path_is_dir "$git/add_dir_add_file_del_file" &&
+ test_path_is_file "$git/add_dir_add_file_del_dir"
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
2.19.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] git-p4: recover from inconsistent perforce history
2019-02-06 19:42 [PATCH 1/1] git-p4: recover from inconsistent perforce history andrew
@ 2019-02-07 8:04 ` Luke Diamand
2019-02-08 9:07 ` Andrew Oakley
0 siblings, 1 reply; 5+ messages in thread
From: Luke Diamand @ 2019-02-07 8:04 UTC (permalink / raw)
To: andrew; +Cc: git, Junio C Hamano
On Wed, 6 Feb 2019 19:42:19 +0000
andrew@adoakley.name wrote:
> From: Andrew Oakley <andrew@adoakley.name>
>
> Perforce allows you commit files and directories with the same name, so
> you could have files //depot/foo and //depot/foo/bar both checked in. A
> p4 sync of a repository in this state fails. Deleting one of the files
> recovers the repository.
>
> When this happens we want git-p4 to recover in the same way as perforce.
I'm finding the test fails for me on a clean git repo, although I can't see any obvious reason why.
Having the ability to detect when Perforce users submit a change which creates a file-inside-a-file will be really very useful.
Luke
>
> Signed-off-by: Andrew Oakley <andrew@adoakley.name>
> ---
> git-p4.py | 41 ++++++++++++++++++++++--
> t/t9834-git-p4-file-dir-bug.sh | 58 ++++++++++++++++++++++++++++++++++
> 2 files changed, 97 insertions(+), 2 deletions(-)
> create mode 100755 t/t9834-git-p4-file-dir-bug.sh
>
> diff --git a/git-p4.py b/git-p4.py
> index 3e12774f96..6bf2bbbcec 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -3012,6 +3012,43 @@ def hasBranchPrefix(self, path):
> print('Ignoring file outside of prefix: {0}'.format(path))
> return hasPrefix
>
> + def isIncluded(self, path):
> + return self.inClientSpec(path) and self.hasBranchPrefix(path)
> +
> + def findShadowedFiles(self, files, change):
> + # Perforce allows you commit files and directories with the same name,
> + # so you could have files //depot/foo and //depot/foo/bar both checked
> + # in. A p4 sync of a repository in this state fails. Deleting one of
> + # the files recovers the repository.
> + #
> + # Git will not allow the broken state to exist and only the most recent
> + # of the conflicting names is left in the repository. When one of the
> + # conflicting files is deleted we need to re-add the other one to make
> + # sure the git repository recovers in the same way as perforce.
> + deleted = [f for f in files if f['action'] in self.delete_actions]
> + to_check = set()
> + for f in deleted:
> + path = f['path']
> + to_check.add(path + '/...')
> + while True:
> + path = path.rsplit("/", 1)[0]
> + if path == "/" or path in to_check:
> + break
> + to_check.add(path)
> + to_check = [p for p in to_check if self.isIncluded(p)]
> + if to_check:
> + stat_result = p4CmdList(
> + ["fstat", "-T", "depotFile,headRev,headType"] +
> + ["%s@%s" % (p, change) for p in to_check])
> + for record in stat_result:
> + if record['code'] != 'stat':
> + continue
> + files.append({
> + 'action': 'add',
> + 'path': record['depotFile'],
> + 'rev': record['headRev'],
> + 'type': record['headType']})
> +
> def commit(self, details, files, branch, parent = "", allow_empty=False):
> epoch = details["time"]
> author = details["user"]
> @@ -3023,8 +3060,8 @@ def commit(self, details, files, branch, parent = "", allow_empty=False):
> if self.clientSpecDirs:
> self.clientSpecDirs.update_client_spec_path_cache(files)
>
> - files = [f for f in files
> - if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]
> + files = [f for f in files if self.isIncluded(f['path'])]
> + self.findShadowedFiles(files, details["change"])
>
> if gitConfigBool('git-p4.keepEmptyCommits'):
> allow_empty = True
> diff --git a/t/t9834-git-p4-file-dir-bug.sh b/t/t9834-git-p4-file-dir-bug.sh
> new file mode 100755
> index 0000000000..9839a3d2bb
> --- /dev/null
> +++ b/t/t9834-git-p4-file-dir-bug.sh
> @@ -0,0 +1,58 @@
> +#!/bin/sh
> +
> +test_description='git p4 directory/file bug handling
> +
> +This test creates files and directories with the same name in perforce and
> +checks that git-p4 recovers from the error at the same time as the perforce
> +repository.'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d' '
> + start_p4d
> +'
> +
> +test_expect_success 'init depot' '
> + (
> + cd "$cli" &&
> +
> + touch add_file_add_dir_del_file add_file_add_dir_del_dir &&
> + p4 add add_file_add_dir_del_file add_file_add_dir_del_dir &&
> + mkdir add_dir_add_file_del_file add_dir_add_file_del_dir &&
> + touch add_dir_add_file_del_file/file add_dir_add_file_del_dir/file &&
> + p4 add add_dir_add_file_del_file/file add_dir_add_file_del_dir/file &&
> + p4 submit -d "add initial" &&
> +
> + rm -f add_file_add_dir_del_file add_file_add_dir_del_dir &&
> + mkdir add_file_add_dir_del_file add_file_add_dir_del_dir &&
> + touch add_file_add_dir_del_file/file add_file_add_dir_del_dir/file &&
> + p4 add add_file_add_dir_del_file/file add_file_add_dir_del_dir/file &&
> + rm -rf add_dir_add_file_del_file add_dir_add_file_del_dir &&
> + touch add_dir_add_file_del_file add_dir_add_file_del_dir &&
> + p4 add add_dir_add_file_del_file add_dir_add_file_del_dir &&
> + p4 submit -d "add conflicting" &&
> +
> + p4 delete -k add_file_add_dir_del_file &&
> + p4 delete -k add_file_add_dir_del_dir/file &&
> + p4 delete -k add_dir_add_file_del_file &&
> + p4 delete -k add_dir_add_file_del_dir/file &&
> + p4 submit -d "delete conflicting"
> + )
> +'
> +
> +test_expect_success 'clone with git-p4' '
> + git p4 clone --dest="$git" //depot/@all
> +'
> +
> +test_expect_success 'check final contents' '
> + test_path_is_dir "$git/add_file_add_dir_del_file" &&
> + test_path_is_file "$git/add_file_add_dir_del_dir" &&
> + test_path_is_dir "$git/add_dir_add_file_del_file" &&
> + test_path_is_file "$git/add_dir_add_file_del_dir"
> +'
> +
> +test_expect_success 'kill p4d' '
> + kill_p4d
> +'
> +
> +test_done
> --
> 2.19.2
>
--
Luke Diamand <luke@diamand.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] git-p4: recover from inconsistent perforce history
2019-02-07 8:04 ` Luke Diamand
@ 2019-02-08 9:07 ` Andrew Oakley
2019-02-08 9:10 ` [PATCH v2] " andrew
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Oakley @ 2019-02-08 9:07 UTC (permalink / raw)
To: Luke Diamand; +Cc: git, Junio C Hamano
On Thu, 7 Feb 2019 08:04:20 +0000
Luke Diamand <luke@diamand.org> wrote:
> On Wed, 6 Feb 2019 19:42:19 +0000
> andrew@adoakley.name wrote:
>
> > From: Andrew Oakley <andrew@adoakley.name>
> >
> > Perforce allows you commit files and directories with the same
> > name, so you could have files //depot/foo and //depot/foo/bar both
> > checked in. A p4 sync of a repository in this state fails.
> > Deleting one of the files recovers the repository.
> >
> > When this happens we want git-p4 to recover in the same way as
> > perforce.
>
> I'm finding the test fails for me on a clean git repo, although I
> can't see any obvious reason why.
This introduced a failure when client specs are being used. I wasn't
populating the cache (update_client_spec_path_cache) before checking if
a file should be included (inClientSpec). I can rearrange the code to
fix this.
Thanks
--
Andrew Oakley
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] git-p4: recover from inconsistent perforce history
2019-02-08 9:07 ` Andrew Oakley
@ 2019-02-08 9:10 ` andrew
2019-02-11 8:54 ` Andrew Oakley
0 siblings, 1 reply; 5+ messages in thread
From: andrew @ 2019-02-08 9:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Luke Diamand, Andrew Oakley
From: Andrew Oakley <andrew@adoakley.name>
Perforce allows you commit files and directories with the same name, so
you could have files //depot/foo and //depot/foo/bar both checked in. A
p4 sync of a repository in this state fails. Deleting one of the files
recovers the repository.
When this happens we want git-p4 to recover in the same way as perforce.
Signed-off-by: Andrew Oakley <andrew@adoakley.name>
---
git-p4.py | 41 ++++++++++++++++++++++--
t/t9834-git-p4-file-dir-bug.sh | 58 ++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 3 deletions(-)
create mode 100755 t/t9834-git-p4-file-dir-bug.sh
diff --git a/git-p4.py b/git-p4.py
index 3e12774f96..42f6805641 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -3012,6 +3012,40 @@ def hasBranchPrefix(self, path):
print('Ignoring file outside of prefix: {0}'.format(path))
return hasPrefix
+ def findShadowedFiles(self, files, change):
+ # Perforce allows you commit files and directories with the same name,
+ # so you could have files //depot/foo and //depot/foo/bar both checked
+ # in. A p4 sync of a repository in this state fails. Deleting one of
+ # the files recovers the repository.
+ #
+ # Git will not allow the broken state to exist and only the most recent
+ # of the conflicting names is left in the repository. When one of the
+ # conflicting files is deleted we need to re-add the other one to make
+ # sure the git repository recovers in the same way as perforce.
+ deleted = [f for f in files if f['action'] in self.delete_actions]
+ to_check = set()
+ for f in deleted:
+ path = f['path']
+ to_check.add(path + '/...')
+ while True:
+ path = path.rsplit("/", 1)[0]
+ if path == "/" or path in to_check:
+ break
+ to_check.add(path)
+ to_check = [p for p in to_check if self.hasBranchPrefix(p)]
+ if to_check:
+ stat_result = p4CmdList(
+ ["fstat", "-T", "depotFile,headRev,headType"] +
+ ["%s@%s" % (p, change) for p in to_check])
+ for record in stat_result:
+ if record['code'] != 'stat':
+ continue
+ files.append({
+ 'action': 'add',
+ 'path': record['depotFile'],
+ 'rev': record['headRev'],
+ 'type': record['headType']})
+
def commit(self, details, files, branch, parent = "", allow_empty=False):
epoch = details["time"]
author = details["user"]
@@ -3020,11 +3054,12 @@ def commit(self, details, files, branch, parent = "", allow_empty=False):
if self.verbose:
print('commit into {0}'.format(branch))
+ files = [f for f in files if self.hasBranchPrefix(f['path'])]
+ self.findShadowedFiles(files, details["change"])
+
if self.clientSpecDirs:
self.clientSpecDirs.update_client_spec_path_cache(files)
-
- files = [f for f in files
- if self.inClientSpec(f['path']) and self.hasBranchPrefix(f['path'])]
+ files = [f for f in files if self.inClientSpec(f['path'])]
if gitConfigBool('git-p4.keepEmptyCommits'):
allow_empty = True
diff --git a/t/t9834-git-p4-file-dir-bug.sh b/t/t9834-git-p4-file-dir-bug.sh
new file mode 100755
index 0000000000..9839a3d2bb
--- /dev/null
+++ b/t/t9834-git-p4-file-dir-bug.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='git p4 directory/file bug handling
+
+This test creates files and directories with the same name in perforce and
+checks that git-p4 recovers from the error at the same time as the perforce
+repository.'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'init depot' '
+ (
+ cd "$cli" &&
+
+ touch add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ p4 add add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ mkdir add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ touch add_dir_add_file_del_file/file add_dir_add_file_del_dir/file &&
+ p4 add add_dir_add_file_del_file/file add_dir_add_file_del_dir/file &&
+ p4 submit -d "add initial" &&
+
+ rm -f add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ mkdir add_file_add_dir_del_file add_file_add_dir_del_dir &&
+ touch add_file_add_dir_del_file/file add_file_add_dir_del_dir/file &&
+ p4 add add_file_add_dir_del_file/file add_file_add_dir_del_dir/file &&
+ rm -rf add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ touch add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ p4 add add_dir_add_file_del_file add_dir_add_file_del_dir &&
+ p4 submit -d "add conflicting" &&
+
+ p4 delete -k add_file_add_dir_del_file &&
+ p4 delete -k add_file_add_dir_del_dir/file &&
+ p4 delete -k add_dir_add_file_del_file &&
+ p4 delete -k add_dir_add_file_del_dir/file &&
+ p4 submit -d "delete conflicting"
+ )
+'
+
+test_expect_success 'clone with git-p4' '
+ git p4 clone --dest="$git" //depot/@all
+'
+
+test_expect_success 'check final contents' '
+ test_path_is_dir "$git/add_file_add_dir_del_file" &&
+ test_path_is_file "$git/add_file_add_dir_del_dir" &&
+ test_path_is_dir "$git/add_dir_add_file_del_file" &&
+ test_path_is_file "$git/add_dir_add_file_del_dir"
+'
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
2.19.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] git-p4: recover from inconsistent perforce history
2019-02-08 9:10 ` [PATCH v2] " andrew
@ 2019-02-11 8:54 ` Andrew Oakley
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Oakley @ 2019-02-11 8:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Luke Diamand
On Fri, 8 Feb 2019 09:10:33 +0000
andrew@adoakley.name wrote:
> From: Andrew Oakley <andrew@adoakley.name>
>
> Perforce allows you commit files and directories with the same name,
> so you could have files //depot/foo and //depot/foo/bar both checked
> in. A p4 sync of a repository in this state fails. Deleting one of
> the files recovers the repository.
>
> When this happens we want git-p4 to recover in the same way as
> perforce.
Unfortunately this still has a few issues:
- newer p4d versions don't allow this by default so the tests fail
- deleted files get recreated in some cases
- command line too long when there are lots of deletions
These are all easily fixable, but I'll test more on a real repository
with before sharing another patch.
--
Andrew Oakley
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-11 8:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 19:42 [PATCH 1/1] git-p4: recover from inconsistent perforce history andrew
2019-02-07 8:04 ` Luke Diamand
2019-02-08 9:07 ` Andrew Oakley
2019-02-08 9:10 ` [PATCH v2] " andrew
2019-02-11 8:54 ` Andrew Oakley
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).