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

allow to use ssh agent globally #19

Closed
wants to merge 1 commit into from

Conversation

SailorMax
Copy link

Git can be used in separated consoles or in different programs (VSCode, GitExtensions,..) and all of them has to have access to started ssh-agent. But this require global SSH-variables.
Here is my solution for that.

Git can be used in separated consoles or in different programs (VSCode, GitExtensions,..) and all of them has to have access to started ssh-agent. But this require global SSH-variables.
Here is my solution for that.
@dscho
Copy link
Member

dscho commented Mar 7, 2020

Maybe the script can also verify before starting an agent that no other agent is running already?

We will also have to think about the scenario where a native OpenSSH (C:\Windows\system32\OpenSSH if I remember correctly) is in use: that OpenSSH's agent, and Git for Windows', are most likely incompatible with one another.

@dscho
Copy link
Member

dscho commented Mar 7, 2020

About that native OpenSSH conflict: see git-for-windows/git#1683 for more details.

And once we sorted that out, we should probably move this PR (and the 32-bit one) to git-for-windows/MINGW-packages, as the original file (that is installed into the SDK via updates of the mingw-w64-git package) comes from here: https://github.com/git-for-windows/MINGW-packages/blob/master/mingw-w64-git/start-ssh-agent.cmd

@SailorMax
Copy link
Author

Maybe the script can also verify before starting an agent that no other agent is running already?

We will also have to think about the scenario where a native OpenSSH (C:\Windows\system32\OpenSSH if I remember correctly) is in use: that OpenSSH's agent, and Git for Windows', are most likely incompatible with one another.

  1. Native OpenSSH has some problems long time: ssh-agent: agent returned different signature type PowerShell/Win32-OpenSSH#1263
  2. Native OpenSSH by default has disabled ssh-agent
  3. I did not add new script to the project. My change just make it work globally.

@dscho
Copy link
Member

dscho commented Mar 9, 2020

3\. I did not add new script to the project. My change just make it work globally.

Yes, but the problem there is that it does interfere with other SSH agents, e.g. if anybody runs a particular instance of Cygwin as part of their day-job project.

There are many scenarios where users may run an SSH agent without knowing it. At the moment, Git for Windows' start-ssh-agent.cmd won't interfere with those. With your changes, it will start to interfere with those.

And since the change does not come with any toggle, such users will not only be surprised, but they will be actively thrown into problems they cannot easily solve.

I could imagine, however, that we could guard this new SETX behavior behind a new config option, ssh.agent.global for example. Given the ramifications, I would want the default to be false.

And once we have that in place, we can add it as a new option in the installer.

@SailorMax
Copy link
Author

May be then simpler make separate script start-ssh-agent-global.cmd?
Solution with config is too hard to use :) Where is config? How to check it from .cmd? I, as git user, never find this setting.

@SailorMax
Copy link
Author

@dscho I currently trying to make separate script and have question about this line:
@ECHO %cmdcmdline% | @FINDSTR /l "\"\"" >NUL

What did you find by it? I not sure understand the idea.
If call the script from explorer, we have command line like :
cmd /c ""start-ssh-agent.cmd" "

Your finder see "" and do not exit the script. Is it was the idea?

@dscho
Copy link
Member

dscho commented Mar 9, 2020

May be then simpler make separate script start-ssh-agent-global.cmd?

Yes, but it would also proliferate the number of files in <Git>\cmd for what is essentially just a flag.

Solution with config is too hard to use :) Where is config?

That is easy to answer: you would warn the user if the config setting is unset, and tell them where/how they can configure whether to configure the SSH agent globally.

How to check it from .cmd?

@REM Enable extensions, the `verify` call is a trick from the setlocal help
@VERIFY other 2>nul
@SETLOCAL EnableDelayedExpansion

@SET ssh_agent_global=unset
@FOR /F "usebackq tokens=*" %%i IN (`git config --bool ssh.agent.global`) DO @SET ssh_agent_global=%%i
@IF "!ssh_agent_global!" == "unset" (
	    @ECHO Talk about the config setting, and what we're defaulting to
) ELSE IF "!ssh_agent_global!" == "true" (
	    @ECHO Replace this with the @SETX calls
)

I, as git user, never find this setting.

Except, of course, if the command tells you what you need to know.

@dscho I currently trying to make separate script and have question about this line:
@ECHO %cmdcmdline% | @FINDSTR /l "\"\"" >NUL

What did you find by it? I not sure understand the idea.

I am not sure, either. It was added in msysgit/msysgit@b0657b1

I guess that this was to discern whether the script was double-clicked, or run from an interactive CMD.

@SailorMax
Copy link
Author

@FOR /F "usebackq tokens=*" %%i IN (`git config --bool ssh.agent.global`) DO @SET ssh_agent_global=%%i

Technically ssh and ssh-agent is not part of Git => It's hard to understand why I have to setup this setting for Git.

Currently I am more inclined to an argument or a separate script.

@SailorMax
Copy link
Author

If make additional argument, user on first run will see some kind of hint about our argument to use it globally. And if he require use it globally, he have to:

  1. setup shortcut to this script in startup/ folder with --global argument
  2. or make separate shortcut with --global argument to run it manually

If make separate script, user can:

  1. or just make shortcut in startup/ folder
  2. or just run it when require, without any modifications.

Not sure which method is better. But like methods like "just push the button" :)

@dscho
Copy link
Member

dscho commented Mar 11, 2020

And if he require use it globally, he have to

Do you mean to disallow women from using this script? I don't.

@dscho
Copy link
Member

dscho commented Mar 11, 2020

Currently I am more inclined to an argument or a separate script.

As I pointed out, I don't think that a separate script is a good idea.

The additional argument would require prior knowledge, and it also won't work if a user double-clicks the script to open a new Console.

It would also make it a lot harder to make this visible via the installer. Only a config option could do that.

@SailorMax
Copy link
Author

Lets sync pros/cons of all solutions:

config option:
+ ??
+ no additional files
- git option for non git component. Where user have to find documentation about it?
- we can't use this script in different cases. Or only globally (+local), or only locally

argument:
+ we can use the script in different cases
+ no additional files
- we have to know about it by, at least, first run the script without arguments to see some hint about it
- we have to setup shortcut with argument to use it in autostart

separate script:
+ we can use one script for case with console and second for init global service
+ we don't need setup anything (learn configs or arguments). For autostart just make default shortcut in autostart/
- we have additional script

Can you fill your +/-?

@SailorMax
Copy link
Author

And about compatibility with Windows native ssh-agent:

Your script search in memory ssh-agent.exe. If we use Windows native ssh-agent, your script start to work with native service. If we will setup global variables as is in pull request, we just setup "link" to native ssh-agent via variables. Where is incompatibility? Can you describe the case?

@dscho
Copy link
Member

dscho commented Mar 12, 2020

config option:
+ ??

I already mentioned a rather large pro: it can be configured in the Git for Windows installer, which is the most visible documentation about this feature that most Git for Windows users will ever get.

- git option for non git component. Where user have to find documentation about it?

There is ample precedent for this. It is a Git-related setting, so it belongs into that config.

- we can't use this script in different cases. Or only globally (+local), or only locally

argument:
+ we can use the script in different cases

That is a good point.

And I think it is in line with many, many other config settings that it should be possible to configure the default in the Git config, and have a command-line option to override it.

@dscho
Copy link
Member

dscho commented Mar 12, 2020

Let's continue the discussion over at git-for-windows/MINGW-packages#39.

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.

2 participants