From: Pete Wyckoff <pw@padd.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 03/11] git p4: work around p4 bug that causes empty symlinks
Date: Tue, 21 Jan 2014 18:16:40 -0500 [thread overview]
Message-ID: <1390346208-9207-4-git-send-email-pw@padd.com> (raw)
In-Reply-To: <1390346208-9207-1-git-send-email-pw@padd.com>
Damien Gérard highlights an interesting problem. Some p4
repositories end up with symlinks that have an empty target. It
is not possible to create this with current p4, but they do
indeed exist.
The effect in git p4 is that "p4 print" on the symlink returns an
empty string, confusing the curret symlink-handling code.
Such broken repositories cause problems in p4 as well, even with
no git involved. In p4, syncing to a change that includes a
bogus symlink causes errors:
//depot/empty-symlink - updating /home/me/p4/empty-symlink
rename: /home/me/p4/empty-symlink: No such file or directory
and leaves no symlink.
In git, replicate the p4 behavior by ignoring these bad symlinks.
If, in a later p4 revision, the symlink happens to point to
something non-null, the symlink will be replaced properly.
Add a big test for all this too.
This happens to be a regression introduced by 1292df1 (git-p4:
Fix occasional truncation of symlink contents., 2013-08-08) and
appeared first in 1.8.5. But it only shows up only in p4
repositories of dubious character, so can wait for a proper
release.
Tested-by: Damien Gérard <damien@iwi.me>
Signed-off-by: Pete Wyckoff <pw@padd.com>
---
git-p4.py | 9 ++++++-
t/t9802-git-p4-filetype.sh | 66 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/git-p4.py b/git-p4.py
index 5ea8bb8..e798ecf 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2075,7 +2075,14 @@ class P4Sync(Command, P4UserMap):
# p4 print on a symlink sometimes contains "target\n";
# if it does, remove the newline
data = ''.join(contents)
- if data[-1] == '\n':
+ if not data:
+ # Some version of p4 allowed creating a symlink that pointed
+ # to nothing. This causes p4 errors when checking out such
+ # a change, and errors here too. Work around it by ignoring
+ # the bad symlink; hopefully a future change fixes it.
+ print "\nIgnoring empty symlink in %s" % file['depotFile']
+ return
+ elif data[-1] == '\n':
contents = [data[:-1]]
else:
contents = [data]
diff --git a/t/t9802-git-p4-filetype.sh b/t/t9802-git-p4-filetype.sh
index 94d7be9..66d3fc9 100755
--- a/t/t9802-git-p4-filetype.sh
+++ b/t/t9802-git-p4-filetype.sh
@@ -267,6 +267,72 @@ test_expect_success SYMLINKS 'ensure p4 symlink parsed correctly' '
)
'
+test_expect_success SYMLINKS 'empty symlink target' '
+ (
+ # first create the file as a file
+ cd "$cli" &&
+ >empty-symlink &&
+ p4 add empty-symlink &&
+ p4 submit -d "add empty-symlink as a file"
+ ) &&
+ (
+ # now change it to be a symlink to "target1"
+ cd "$cli" &&
+ p4 edit empty-symlink &&
+ p4 reopen -t symlink empty-symlink &&
+ rm empty-symlink &&
+ ln -s target1 empty-symlink &&
+ p4 add empty-symlink &&
+ p4 submit -d "make empty-symlink point to target1"
+ ) &&
+ (
+ # Hack the p4 depot to make the symlink point to nothing;
+ # this should not happen in reality, but shows up
+ # in p4 repos in the wild.
+ #
+ # The sed expression changes this:
+ # @@
+ # text
+ # @target1
+ # @
+ # to this:
+ # @@
+ # text
+ # @@
+ #
+ cd "$db/depot" &&
+ sed "/@target1/{; s/target1/@/; n; d; }" \
+ empty-symlink,v >empty-symlink,v.tmp &&
+ mv empty-symlink,v.tmp empty-symlink,v
+ ) &&
+ (
+ # Make sure symlink really is empty. Asking
+ # p4 to sync here will make it generate errors.
+ cd "$cli" &&
+ p4 print -q //depot/empty-symlink#2 >out &&
+ test ! -s out
+ ) &&
+ test_when_finished cleanup_git &&
+
+ # make sure git p4 handles it without error
+ git p4 clone --dest="$git" //depot@all &&
+
+ # fix the symlink, make it point to "target2"
+ (
+ cd "$cli" &&
+ p4 open empty-symlink &&
+ rm empty-symlink &&
+ ln -s target2 empty-symlink &&
+ p4 submit -d "make empty-symlink point to target2"
+ ) &&
+ cleanup_git &&
+ git p4 clone --dest="$git" //depot@all &&
+ (
+ cd "$git" &&
+ test $(readlink empty-symlink) = target2
+ )
+'
+
test_expect_success 'kill p4d' '
kill_p4d
'
--
1.8.5.2.320.g99957e5
next prev parent reply other threads:[~2014-01-21 23:17 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-21 23:16 [PATCH 00/11] git p4 tests and a few bug fixes Pete Wyckoff
2014-01-21 23:16 ` [PATCH 01/11] git p4 test: wildcards are supported Pete Wyckoff
2014-01-21 23:16 ` [PATCH 02/11] git p4 test: ensure p4 symlink parsing works Pete Wyckoff
2014-01-21 23:16 ` Pete Wyckoff [this message]
2014-01-22 1:26 ` [PATCH 03/11] git p4: work around p4 bug that causes empty symlinks Eric Sunshine
2014-01-21 23:16 ` [PATCH 04/11] git p4 test: explicitly check p4 wildcard delete Pete Wyckoff
2014-01-21 23:16 ` [PATCH 05/11] git p4 test: is_cli_file_writeable succeeds Pete Wyckoff
2014-01-21 23:16 ` [PATCH 06/11] git p4 test: run as user "author" Pete Wyckoff
2014-01-22 1:26 ` Eric Sunshine
2014-01-21 23:16 ` [PATCH 07/11] git p4 test: do not pollute /tmp Pete Wyckoff
2014-01-21 23:16 ` [PATCH 08/11] git p4: handle files with wildcards when doing RCS scrubbing Pete Wyckoff
2014-01-21 23:16 ` [PATCH 09/11] git p4: fix an error message when "p4 where" fails Pete Wyckoff
2014-01-21 23:16 ` [PATCH 10/11] git p4 test: examine behavior with locked (+l) files Pete Wyckoff
2014-01-21 23:16 ` [PATCH 11/11] git p4 doc: use two-line style for options with multiple spellings Pete Wyckoff
2014-01-22 0:03 ` [PATCH 00/11] git p4 tests and a few bug fixes Junio C Hamano
2014-01-22 22:44 ` Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 01/11] git p4 test: wildcards are supported Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 02/11] git p4 test: ensure p4 symlink parsing works Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 03/11] git p4: work around p4 bug that causes empty symlinks Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 04/11] git p4 test: explicitly check p4 wildcard delete Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 05/11] git p4 test: is_cli_file_writeable succeeds Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 06/11] git p4 test: run as user "author" Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 07/11] git p4 test: do not pollute /tmp Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 08/11] git p4: handle files with wildcards when doing RCS scrubbing Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 09/11] git p4: fix an error message when "p4 where" fails Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 10/11] git p4 test: examine behavior with locked (+l) files Pete Wyckoff
2014-01-22 22:47 ` [PATCHv2 11/11] git p4 doc: use two-line style for options with multiple spellings Pete Wyckoff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1390346208-9207-4-git-send-email-pw@padd.com \
--to=pw@padd.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).