- Notifications
You must be signed in to change notification settings - Fork2.5k
Moresafe.directory
improvements#6739
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
a7f3a5c
to028b6dd
CompareWe currently only canonicalize the temp sandbox directory on macOS,which is critical since `/tmp` is really `/private/tmp`. However, weshould do it everywhere, so that tests can actually expect a consistentoutcome by looking at `clar_sandbox_path()`.
We may need to strip multiple slashes at the end of a path; provide amethod to do so.
We may quickly want to determine if the given path is the root path('/') on POSIX, or the root of a drive letter (eg, 'A:/', 'C:\') onWindows.
Clar handles multiple levels of hierarchy in a test name _but_ it does soassuming that there are not tests at a parent folder level. In other words,if you have some tests at path/core.c and path/win32.c, then you cannot havetests in path.c. If you have tests in path.c, then the things in path/*.cwill be ignored.Move the tests in path.c into path/core.c.
028b6dd
to9e53232
CompareThe POSIX `realpath` function canonicalizes relative paths, symbolic links,and case (on case-insensitive filesystems). For example, on macOS, if youcreate some file `/private/tmp/FOO`, and you call `realpath("/tmp/foo")`,you get _the real path_ returned of `/private/tmp/FOO`.To emulate this behavior on win32, we call `GetFullPathName` to handle therelative to absolute path conversion, then call `GetLongPathName` to handlethe case canonicalization.
Ensure that we clean up cruft that we create for testing, so that futuretests don't have troubles.
Our error messages should provide the literal path that users should addto their safe directory allowlist, which means it should not have atrailing slash.
Ensure that we can support safe.directory entries that are prefixed with'%(prefix)/'.
'%(prefix)/' as a leading path name is safe to strip on all platforms.It doesn't make sense to use on non-Windows, but supporting it theredoes allow us to easily dev/test.
Provide a helper method in the tests to set up the safe.directoryconfiguration for a normal and bare repository to avoid some copy/pasta.(Some copypasta will remain, since there's customizations to the filethat are not trivially abstractable.)
On Windows, you may open a repo with UNC path format -- for example\\share\foo\bar, or as a git-style path, //share/foo/bar. In this world,Git for Windows expects you to translate this to$(prefix)//share/foo/bar.We can test for that by using the fact that local drives are exposed ashidden shares. For example, C:\Foo is //localhost/C$/Foo/.
When doing directory name munging to remove trailing slashes, ensurethat we do not remove the trailing slash from the path root, whetherthat's '/' or 'C:\'.
Recent versions of Git for Windows allow a sane UNC style path to beused directly in `safe.directory` (without needing the `%(prefix)`silliness). Match that behavior.
9e53232
to9c31bd0
CompareFixes#6737 |
As another improvement, the error message should provide the A fix would also be nice to have for 1.7.3 |
Right - sorry - I went back to your PR and realized that I never actually finished typing up the comments. We need to avoid multi-line error messages since some people use them in odd places. I don't have a great idea for how to plumb this through (the directory that's problematic vs the configuration command that you would run). Thisfeels like a good use for the "warnings API" that we keep talking about and never shipping, so let me give it some thought. |
I'm willing to back port to v1.7, but I'm also preparing to cut v1.8 if that makes more sense for you. |
I'm testing the main branch right now... |
Uh oh!
There was an error while loading.Please reload this page.
The gift that keeps on giving — it will rival
core.autocrlf
for user disappointment.A number of changes here:
%(prefix)
in front of paths.%(prefix)