Skip to content
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

SSH: Support URLs with ports #3146

Merged
merged 3 commits into from Feb 7, 2019
Merged

SSH: Support URLs with ports #3146

merged 3 commits into from Feb 7, 2019

Conversation

@kyleam
Copy link
Contributor

@kyleam kyleam commented Feb 7, 2019

This is mainly for the benefit of the ReproMan project. There we are managing SSH resources that the user may not have configured in .ssh/config, so we need to call create-sibling with an SSH URL that includes port. Without these changes, doing that results in an error like

stderr: 'fatal: ssh variant 'simple' does not support setting port' [cmd.py:wait:415] (GitCommandError)

Just for reference, here is the ssh.variant part of man git-config:

ssh.variant
    By default, Git determines the command line arguments to use based on the basename of the configured SSH
    command (configured using the environment variable GIT_SSH or GIT_SSH_COMMAND or the config setting
    core.sshCommand). If the basename is unrecognized, Git will attempt to detect support of OpenSSH options
    by first invoking the configured SSH command with the -G (print configuration) option and will
    subsequently use OpenSSH options (if that is successful) or no options besides the host and remote
    command (if it fails).

    The config variable ssh.variant can be set to override this detection. Valid values are ssh (to use
    OpenSSH options), plink, putty, tortoiseplink, simple (no options except the host and remote command).
    The default auto-detection can be explicitly requested using the value auto. Any other value is treated
    as ssh. This setting can also be overridden via the environment variable GIT_SSH_VARIANT.

    The current command-line parameters used for each variant are as follows:

    ·   ssh - [-p port] [-4] [-6] [-o option] [username@]host command

    ·   simple - [username@]host command

    ·   plink or putty - [-P port] [-4] [-6] [username@]host command

    ·   tortoiseplink - [-P port] [-4] [-6] -batch [username@]host command

    Except for the simple variant, command-line parameters are likely to change as git gains new features.

  • Add test for create-sibling that uses port in URL.
kyleam added 2 commits Feb 6, 2019
SSHConnection accepts ssh URLs that contain a port.  This will be
translated as an argument to the "-p" option for SSH, but scp spells
it as "-P" (capital).
@kyleam kyleam added the WIP label Feb 7, 2019
Otherwise, Git thinks sshrun can only handle the arguments

    [username@]host command

and will refuse to work with URLs that contain ports, leaving 'datalad
sshrun's --port flag unused.  That's not usually a big deal, because
most users will configure their hosts in .ssh/config, but it does
prevent programs like ReproMan---that want to manager resources
_without_ the user needing to change their .ssh/config---from using
DataLad interfaces like create-sibling.

However, one worry is that using the "ssh" value is claiming sshrun
can handle

    [-p port] [-4] [-6] [-o option] [username@]host command

but we don't support -4, -6, or -o.  I haven't been able to trigger an
example where Git tries to pass these, but I'm also not quite sure how
I would.  To make it possible to use URLs with ports, let's risk it,
and see if we run into any issues.
@kyleam kyleam removed the WIP label Feb 7, 2019
@yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Feb 7, 2019

Great! Didn't know, makes sense for poor Git to try to be that smart. We use regular (open)ssh so indeed makes sense to hardcode it. Thus will merge now
But just for my own clarity -- I do not get why Git then fails to autodetect ssh, and resorts to simple? for me -G seems to work

$> ssh -G ssh://localhost:22 
user yoh
hostname localhost
port 22
...

any additional hints/explanation would be most welcome!

@yarikoptic yarikoptic merged commit 697d8b0 into datalad:master Feb 7, 2019
1 of 3 checks passed
@kyleam
Copy link
Contributor Author

@kyleam kyleam commented Feb 7, 2019

We use regular (open)ssh so indeed makes sense to hardcode it. Thus will merge now
But just for my own clarity -- I do not get why Git then fails to autodetect ssh, and resorts to simple? for me -G seems to work

My understanding (given in 3ba7648) is that we use datalad sshrun as GIT_SSH_COMMAND. Hence Git not knowing what to do with it.

@kyleam kyleam deleted the ssh-variant branch Feb 7, 2019
@yarikoptic yarikoptic added this to the Release 0.11.2 milestone Feb 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants