test_git_push: make tests work on newer libgit2-s #4184
Conversation
This should make it unnecessary to disable a test in martinvonz#4080.
There was a problem hiding this comment.
I agree that we'll need a TODO comment. And if we leave a TODO, I slightly prefer more scoped workaround (such as just disable the single failing test on Windows.)
If we want to add more practical workaround, I'll probably insert a workaround to commands::git::clone::absolute_git_source().
| @@ -38,7 +38,8 @@ fn set_up() -> (TestEnvironment, PathBuf) { | |||
| "git", | |||
| "clone", | |||
| "--config-toml=git.auto-local-branch=true", | |||
| origin_git_repo_path.to_str().unwrap(), | |||
| // Git and libgit2 > 1.8 do not allow backslashes in remote names | |||
| &origin_git_repo_path.to_str().unwrap().replace("\\", "/"), | |||
There was a problem hiding this comment.
nit: might be safer to apply dunce::simplified() first. I didn't know the canonicalized Windows path can have forward slashes.
There was a problem hiding this comment.
didn't know the canonicalized Windows path can have forward slashes.
It's madness. Officially, they can't. For libgit2 and Git, who knows. See also rhysd/path-slash#6.
You can see my draft of a more principled solution in #4188, but I do not like where it is going so far.
|
This PR might still be useful for reference, but I no longer plan on merging it. |
This should make it unnecessary to disable a test
in #4080.
See also CI run in #4183
This is a messy fix, I should probably add a TODO for doing something more reasonable when
jj git clonegets passed a path with backslahshes.Update: The CI seems slow, but I'll mark this as ready for review. Hopefully, the CI will recover on its own.
Update 2: I don't think this is urgent, so I'll see about a more complete fix with a better error message when
jj git cloneorgit remote addget an invalid remote name.