Re: [PATCH] git-p4 shouldn't attempt to store symlinks in LFS
From: Junio C Hamano <hidden>
Date: 2023-10-18 19:30:09
Possibly related (same subject, not in this thread)
- 2023-10-18 · Re: [PATCH] git-p4 shouldn't attempt to store symlinks in LFS · Junio C Hamano <hidden>
- 2023-10-18 · [PATCH] git-p4 shouldn't attempt to store symlinks in LFS · Matthew McClain <hidden>
Matthew McClain [off-list ref] writes:
If a symlink in your Perforce repository matches git-p4.largeFileExtensions, git-p4.py will attempt to put the symlink in LFS but will blow up when it passes a string to generateTempFile. Git LFS normal behavior does not stash symlinks in LFS.
I am not a "git p4" (or "git lfs") user, but if nobody uses LFS to store symbolic links (which is very much understandable given what it was invented for), then that alone is a good enough reason to avoid the regular blob codepath, and that is true even if generateTempFile accepted a str and silently converted it to bytes to help callers, no? So "but will blow up ..." and the stacktrace below are more or less irrelevant details and do not help convince readers why this change is desirable. I'd suggest removing it (and perhaps place more stress on explaining why storing symbolic links in LFS is a bad practice nobody uses, but if it is so obvious perhaps the single sentence explanation you have above would be sufficient). I know I can ask brian to take a look at this change from LFS angle, but who's authoritatively reviewing "git p4" related changes these days (Matthew, this is not a question for you, but to the list)? Thanks.
quoted hunk
Importing revision 42889 (100%)Traceback (most recent call last): File "./git/git-p4.py", line 4621, in <module> main() File "./git/git-p4.py", line 4615, in main if not cmd.run(args): File "./git/git-p4.py", line 4225, in run self.importRevisions(args, branch_arg_given) File "./git/git-p4.py", line 4002, in importRevisions self.importChanges(changes) File "./git/git-p4.py", line 3876, in importChanges self.initialParent) File "./git/git-p4.py", line 3496, in commit self.streamP4Files(files) File "./git/git-p4.py", line 3336, in streamP4Files cb=streamP4FilesCbSelf) File "./git/git-p4.py", line 910, in p4CmdList cb(entry) File "./git/git-p4.py", line 3321, in streamP4FilesCbSelf self.streamP4FilesCb(entry) File "./git/git-p4.py", line 3266, in streamP4FilesCb self.streamOneP4File(self.stream_file, self.stream_contents) File "./git/git-p4.py", line 3217, in streamOneP4File git_mode, contents = self.largeFileSystem.processContent(git_mode, relPath, contents) File "./git/git-p4.py", line 1656, in processContent return LargeFileSystem.processContent(self, git_mode, relPath, contents) File "./git/git-p4.py", line 1526, in processContent contentTempFile = self.generateTempFile(contents) File "./git/git-p4.py", line 1488, in generateTempFile contentFile.write(d) File "/usr/lib64/python3.6/tempfile.py", line 485, in func_wrapper return func(*args, **kwargs) TypeError: a bytes-like object is required, not 'str' Signed-off-by: Matthew McClain <redacted> --- git-p4.py | 4 ++++ 1 file changed, 4 insertions(+)diff --git a/git-p4.py b/git-p4.py index d26a980e5a..f5fda2a3dc 100755 --- a/git-p4.py +++ b/git-p4.py@@ -1522,6 +1522,10 @@ def processContent(self, git_mode, relPath, contents): file is stored in the large file system and handles all necessary steps. """ + # no need to store symlinks in LFS (generateTempFile wants bytes) + if git_mode == "120000": + return (git_mode, contents) + if self.exceedsLargeFileThreshold(relPath, contents) or self.hasLargeFileExtension(relPath): contentTempFile = self.generateTempFile(contents) pointer_git_mode, contents, localLargeFile = self.generatePointer(contentTempFile)