-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Handle files starting with colons in WalkGitLog #22935
Conversation
Surprise .... According to git document: https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefpathspecapathspec :
If I understand correctly, the |
And one more thing, I think we should process the file list from framework level (the git module), there could be a lot of functions need to pass the file list (path list) |
Yes, that also handles the other edge cases much better (i.e. |
Thanks for the suggestion!! I checked it and it does work, would you suggest adding the
Sure, I'll try to touch only the places where paths are being passed, and not oid's/etc I have a concern with setting this on the git-module level, because now if some command say specifically gets only jpegs by doing |
Actually I am not sure whether ps: as a bug fix, only process the paths for the |
I tried changing |
Since the EDIT: Tried doing it, and it still breaks the release creation tests, because the tag name is passed to |
// Use the :(literal) pathspec magic to handle edge cases with files named like ":file.txt" or "*.jpg" | ||
for i, file := range files { | ||
files[i] = ":(literal)" + file | ||
} | ||
cmd.AddDashesAndList(files...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, wouldn't it be better to add it to AddDashesAndList
directly?
That way, it will already be applied to every possible instance, I don't know of any usage that expects wildcards to work, and it is in the other cases not intended to be used as a wildcard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, wouldn't it be better to add it to
AddDashesAndList
directly?
This approach has been discussed before, it might break something.
Maybe next time we can introduce AddDashesAndLiterals
, but not change the behavior of AddDashesAndList
.
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #22935 +/- ##
==========================================
+ Coverage 47.14% 47.16% +0.02%
==========================================
Files 1149 1154 +5
Lines 151446 152196 +750
==========================================
+ Hits 71397 71790 +393
- Misses 71611 71938 +327
- Partials 8438 8468 +30
... and 16 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
* giteaofficial/main: Use `<nav>` instead of `<div>` in the global navbar (go-gitea#23125) Fix aria.js bugs: incorrect role element problem, mobile focus problem, tippy problem (go-gitea#23450) [skip ci] Updated translations via Crowdin Make time tooltips interactive (go-gitea#23526) Update mini-css-extract-plugin, remove postcss (go-gitea#23520) Fix review comment context menu clipped bug (go-gitea#23523) Add absent repounits to create/edit repo API (go-gitea#23500) Fix tags sort by creation time (descending) on branch/tag dropdowns (go-gitea#23491) Allow both fullname and username search when `DEFAULT_SHOW_FULL_NAME` is true (go-gitea#23463) Handle files starting with colons in WalkGitLog (go-gitea#22935) Change `Close` to either `Close issue` or `Close pull request` (go-gitea#23506) Update act (go-gitea#23512) Move pidfile creation from setting to web cmd package (go-gitea#23285)
Currently gitea shows no commit information for files starting with a colon.
I set up a minimal repro repository that reproduces this error once it's migrated on gitea
This is happening because the filenames piped to the
git log
command are written as is, and it doesn't work when you have a colon at the start of the filename, and you need to escape it.You can test it locally, if you do
git log returns nothing. However, if you do
git log -- "\:file"
, it will show the commit with the file change.This PR escapes the starting colons in paths in the
LogNameStatusRepo
function, making gitea return commit info about the file with the bad filename.This error shows up only with files starting with colon, anywhere else in filename is ok. Dashes at the beginning also seem to be working.
I don't know gitea internals well enough to know where else this error can pop up, so I'm keeping this PR small as suggested by your contributor guide