Thread (3 messages) 3 messages, 3 authors, 2023-10-19

Re: [PATCH] git-p4 shouldn't attempt to store symlinks in LFS

From: brian m. carlson <hidden>
Date: 2023-10-18 22:26:56

Possibly related (same subject, not in this thread)

On 2023-10-18 at 19:30:02, Junio C Hamano wrote:
Matthew McClain [off-list ref] writes:
quoted
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?
Git LFS doesn't store symlinks because smudge/clean filters don't handle
symlinks.  They never get passed to the filter process nor the
smudge/clean filters, nor could that occur without a change to the
protocol or command-line interface.  Unless Git learned how to send them
to the filters, Git LFS would have a hard time using them in any useful
way.

Also, for Git LFS, whose goal is to move large files out of the
repository history, symlinks are typically not an interesting thing to
process because they are functionally limited to 4 KiB or a similar size
on most systems.
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).
Hopefully my explanation above can be part of that commit message.
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)?
It looks fine to me from an LFS angle, but I know nothing about Perforce
or git-p4.

Also, as a minor request, it would be great if you could continue to
email me at my personal address, since that's the address with which I
read the list.  My work address appearing on series is more of a
reflection that I'm more frequently finding time to work on things on
work time (hence the work address) versus personal time (where I'd be
using my personal email for patches).  I've fixed it up above.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help