Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate: properly escape blob filenames #4683

Merged
merged 1 commit into from Oct 14, 2021
Merged

migrate: properly escape blob filenames #4683

merged 1 commit into from Oct 14, 2021

Conversation

pyckle
Copy link
Contributor

@pyckle pyckle commented Oct 13, 2021

Please consider this minor PR to fix the below bug. I added a unit test. I hope this is straightforward.

Describe the bug
When files have a combination of spaces and parentheses/dashes, git lfs migrate import produces invalid filenames in .gitattributes

To Reproduce
Steps to reproduce the behavior:

#!/bin/bash -ex
mkdir testrepo
cd testrepo
git init .
dd if=/dev/urandom of='./test - special.bin' bs=1M count=1
dd if=/dev/urandom of='./test (test2) special.bin' bs=1M count=1
dd if=/dev/urandom of='./test * ** special.bin' bs=1M count=1
git add .
git commit -am 'my first commit'
git lfs migrate import --everything --above 512KB
git add .gitattributes
cat .gitattributes

Expected behavior
.gitattributes should escape the filenames. Ideally they should be in double quotes (c-style escaping), but pattern escaping also should be OK.

System environment
Debian 10.10 and git lfs 3.0.1

Output of git lfs env
The output of running git lfs env as a code block.

$ git lfs env
git-lfs/3.0.1 (GitHub; linux amd64; go 1.17.1)
git version 2.33.0

LocalWorkingDir=/home/andrewp/testing/testrepo
LocalGitDir=/home/andrewp/testing/testrepo/.git
LocalGitStorageDir=/home/andrewp/testing/testrepo/.git
LocalMediaDir=/home/andrewp/testing/testrepo/.git/lfs/objects
LocalReferenceDirs=
TempDir=/home/andrewp/testing/testrepo/.git/lfs/tmp
ConcurrentTransfers=8
TusTransfers=false
BasicTransfersOnly=false
SkipDownloadErrors=false
FetchRecentAlways=false
FetchRecentRefsDays=7
FetchRecentCommitsDays=0
FetchRecentRefsIncludeRemotes=true
PruneOffsetDays=3
PruneVerifyRemoteAlways=false
PruneRemoteName=origin
LfsStorageDir=/home/andrewp/testing/testrepo/.git/lfs
AccessDownload=none
AccessUpload=none
DownloadTransfers=basic,lfs-standalone-file,ssh
UploadTransfers=basic,lfs-standalone-file,ssh
GIT_EXEC_PATH=/home/andrewp/local/libexec/git-core
git config filter.lfs.process = "git-lfs filter-process"
git config filter.lfs.smudge = "git-lfs smudge -- %f"
git config filter.lfs.clean = "git-lfs clean -- %f"

@pyckle pyckle force-pushed the main branch 2 times, most recently from c12a8cb to 84821aa Compare October 14, 2021 09:07
@pyckle
Copy link
Contributor Author

pyckle commented Oct 14, 2021

Please rerun the MacOS build - I think it was a spurious failure related to #4674

Copy link
Member

@bk2204 bk2204 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,

Thanks for the patch, and welcome to Git LFS! This looks good, and once CI goes green here, I'll go ahead and merge it.

@pyckle
Copy link
Contributor Author

pyckle commented Oct 14, 2021

Sounds good. Thanks @bk2204 👍

@bk2204 bk2204 merged commit 4d0427e into git-lfs:main Oct 14, 2021
bk2204 added a commit to bk2204/git-lfs that referenced this pull request Oct 25, 2021
migrate: properly escape blob filenames
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants