-
Notifications
You must be signed in to change notification settings - Fork 0
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
Remove the support for SCP syntax for ssh locations. #54
Conversation
The SCP syntax, of the form `username@hostname:path`, makes inferring the type of location from a URL more difficult. Additionally it introduces two ways of doing the same thing for little benefit. The only advantage this syntax had (other than conciseness) is that its path is relative by default, whereas the full "ssh://" URL syntax has an absolute path. We can provide an alternative by interpreting paths that start with `/~/` as relative. This works and makes sense because, barring any unusual SSH server setups, the initial working directory of an SFTP server is the user's home directory. This is similar as what Git supports, though Git does a proper expansion of the tilde, and also supports other users' home directory, eg. `/~alice/`. This only works thanks to a server side git binary, which we don't have. Interestingly, Mercurial actually solves that problem another way, by not including the separating slash in the path: `ssh://hostname/path` is relative, `ssh://hostname//path` is absolute.
Part of the motivation here is making the implementation of the |
src/outpack/location_ssh.py
Outdated
raise Exception(msg) | ||
# This condition is equivalent to path.is_relative_to("/~"), except that | ||
# function isn't available on Python 3.8. | ||
home = PurePosixPath("/~") |
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.
I think I prefer the mercurial approach here tbh. How strongly do you feel about this?
My reasoning is that relative paths will probably be more commonly used (most people will be used to scp ...
popping files in their home directory unless they take additional action. I know that this is slightly inconsistent with http urls though.
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.
Yeah I think I actually prefer the mercurial approach too. My main reason for doing it this way is that more people will be familiar with Git than Mercurial.
That being said, probably very few people actually use Git in this way and instead use whatever magic GitHub does, so maybe it doesn’t matter. I’ll change it to the mercurial semantics.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
==========================================
- Coverage 99.28% 97.76% -1.52%
==========================================
Files 45 50 +5
Lines 3351 4027 +676
Branches 527 660 +133
==========================================
+ Hits 3327 3937 +610
- Misses 21 65 +44
- Partials 3 25 +22 ☔ View full report in Codecov by Sentry. |
a3cf620
to
7873bd6
Compare
The SCP syntax, of the form
username@hostname:path
, makes inferring the type of location from a URL more difficult. Additionally it introduces two ways of doing the same thing for little benefit.The only advantage this syntax had (other than conciseness) is that its path was relative by default, whereas the full "ssh://hostname/path" URL syntax always interpreted the path as absolute (following Git's semantics). We now follow the Mercurial semantics, where
ssh://hostname/path
is relative to the user's home directory andssh://hostname//path
is absolute.