Skip to content

clone: don't test remote url against filesystem #5983

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

Closed
wants to merge 2 commits into from

Conversation

boretrk
Copy link
Contributor

@boretrk boretrk commented Aug 12, 2021

resolves #5982

I removed the directory check in create_and_configure_origin
If the path given is to a bundle it will not be a directory but should probably have an absolute path anyway?

No test added, I'm not sure about the best approach to test this.
I suspect windows will act up on url's containing ':'

@boretrk
Copy link
Contributor Author

boretrk commented Aug 12, 2021

The test from #5950 could be expanded to also include stat, but git_path_exists and git_path_isdir doesn't specifically check errno and have no way of returning invalid path rather than non-existing path so it doesn't seem like a good option here.

@ethomson
Copy link
Member

Yeah, Windows is going to be the tricky bit here for sure. Let me do some testing over there.

@boretrk
Copy link
Contributor Author

boretrk commented Aug 25, 2021

Would it be possible to add
mkdir -p "<path-to-where-we-run-clone-test-from>/git:/github.com/libgit2/TestGitRepository"
to one of the docker builds?
That should make current tests fail if clone prioritizes git:/github.com/libgit2/TestGitRepository over git://github.com/libgit2/TestGitRepository without bothering anyone testing on Windows.

@boretrk
Copy link
Contributor Author

boretrk commented Aug 30, 2021

An alternative to checking the url for "://" could be to split up transport_find_fn() so that it can be asked if it is a known url, local path or file:// style url, but I don't know if we really gain anything from that.

@boretrk
Copy link
Contributor Author

boretrk commented Jan 1, 2022

I figured that the way to test this is to add a "http:/"-folder in the current directory for the online cloning tests.
I'm very uncertain on the proper way to do this in the clar environment so if @ethomson could take a look at it it would be great.

We can't really test this on Windows systems, but then again, it will never occur on those either.

@boretrk boretrk marked this pull request as draft January 23, 2022 19:26
@boretrk
Copy link
Contributor Author

boretrk commented Jan 23, 2022

Restructured and added comments to make the flow clearer.
I think it should be good to go, if it is OK to use git_futils_mkdir_r in the test initialization that is.

@boretrk boretrk marked this pull request as ready for review January 23, 2022 21:11
@boretrk boretrk mentioned this pull request Jan 29, 2022
@boretrk boretrk closed this Jul 19, 2022
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.

clone doesn't work if path exists locally
2 participants