Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
ethomson merged 13 commits intomainfromethomson/safedirectory
Feb 20, 2024
Merged

Moresafe.directory improvements#6739

ethomson merged 13 commits intomainfromethomson/safedirectory
Feb 20, 2024

Conversation

ethomson
Copy link
Member

@ethomsonethomson commentedFeb 18, 2024
edited
Loading

The gift that keeps on giving — it will rivalcore.autocrlf for user disappointment.

A number of changes here:

  1. We ensure that we cope correctly with%(prefix) in front of paths.
  2. We stop requiring that UNC paths start with%(prefix)
  3. Report the directory path without a trailing slash
  4. Ensure that root paths are correctly handled
  5. Ensure that the repository path is canonicalized on Windows — we were already doing this on POSIX, and this aligns with Git itself
  6. To cope with this, we need to start canonicalizing the clar sandbox paths so that the repository path is deterministic and we can continue joining paths simply

csware reacted with thumbs up emojiRandyMcMillan reacted with eyes emoji
@ethomsonethomsonforce-pushed theethomson/safedirectory branch 4 times, most recently froma7f3a5c to028b6ddCompareFebruary 19, 2024 15:41
We 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.
The 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.
@ethomson
Copy link
MemberAuthor

Fixes#6737

@csware
Copy link
Contributor

As another improvement, the error message should provide the%(prefix)/ as vanilla Git does.

A fix would also be nice to have for 1.7.3

@ethomson
Copy link
MemberAuthor

As another improvement, the error message should provide the %(prefix)/ as vanilla Git does.

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.

@ethomsonethomson merged commitcaf1747 intomainFeb 20, 2024
@ethomsonethomson deleted the ethomson/safedirectory branchFebruary 20, 2024 16:34
@ethomson
Copy link
MemberAuthor

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.

@csware
Copy link
Contributor

I'm testing the main branch right now...

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ethomson@csware

[8]ページ先頭

©2009-2025 Movatter.jp