Re: [PATCH v3] git-p4: recover from inconsistent perforce history
- From
Luke Diamand <luke@diamand.org>
- Date
- May 10, 2020, 12:03 UTC
- Message-ID
- <CAE5ih793qKyOSE-hkOw7+nFmM3XTRxxrXv0FD2+WWXjGbVHkoQ@mail.gmail.com>
- In-Reply-To
- <20200510101650.50583-1-andrew@adoakley.name>
On Sun, 10 May 2020 at 11:17, Andrew Oakley <andrew@adoakley.name> wrote:
Show 7 quoted lines
> > 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.
Looks good to me.
Perforce changed their server to reject this kind of thing in the 2017.1 version:
Bugs fixed in 2017.1
#1489051 (Job #2170) **
Submitting a file with the same name as an existing depot
directory path (or vice versa) will now be rejected.(Of course people will still have damaged repos even today).
I tried your test with both the 2015.1 and the 2020.1 versions, and it worked in both cases - shouldn't it be impossible to get into the state that git-p4 now recovers from with a newer p4d?
Luke
Show 152 quoted lines
>
>
> Signed-off-by: Andrew Oakley <andrew@adoakley.name>
> ---
> git-p4.py | 43 ++++++++++++++++++++-
> t/t9834-git-p4-file-dir-bug.sh | 70 ++++++++++++++++++++++++++++++++++
> 2 files changed, 111 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 b8b2a1679e..d551efb0dd 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -3214,6 +3214,42 @@ 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 = decode_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 = ['%s@%s' % (wildcard_encode(p), change) for p in to_check
> + if self.hasBranchPrefix(p)]
> + if to_check:
> + stat_result = p4CmdList(["-x", "-", "fstat", "-T",
> + "depotFile,headAction,headRev,headType"], stdin=to_check)
> + for record in stat_result:
> + if record['code'] != 'stat':
> + continue
> + if record['headAction'] in self.delete_actions:
> + 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"]
> @@ -3222,11 +3258,14 @@ 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(decode_path(f['path']))]
> + self.findShadowedFiles(files, details['change'])
> +
> if self.clientSpecDirs:
> self.clientSpecDirs.update_client_spec_path_cache(files)
>
> - files = [f for (f, path) in ((f, decode_path(f['path'])) for f in files)
> - if self.inClientSpec(path) and self.hasBranchPrefix(path)]
> + files = [f for f in files if self.inClientSpec(decode_path(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..031e1f8668
> --- /dev/null
> +++ b/t/t9834-git-p4-file-dir-bug.sh
> @@ -0,0 +1,70 @@
> +#!/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_might_fail p4 configure set submit.collision.check=0
> +'
> +
> +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" &&
> +
> + p4 delete -k "add_file_add_dir_del_file/file" &&
> + p4 delete -k "add_file_add_dir_del_dir" &&
> + p4 delete -k "add_dir_add_file_del_file/file" &&
> + p4 delete -k "add_dir_add_file_del_dir" &&
> + p4 submit -d "delete remaining"
> + )
> +'
> +
> +test_expect_success 'clone with git-p4' '
> + git p4 clone --dest="$git" //depot/@1,3
> +'
> +
> +test_expect_success 'check 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 'rebase and check empty' '
> + git -C "$git" p4 rebase &&
> +
> + test_path_is_missing "$git/add_file_add_dir_del_file" &&
> + test_path_is_missing "$git/add_file_add_dir_del_dir" &&
> + test_path_is_missing "$git/add_dir_add_file_del_file" &&
> + test_path_is_missing "$git/add_dir_add_file_del_dir"
> +'
> +
> +test_done
> --
> 2.24.1
>