Skip to content

Commit

Permalink
Fixes #63. Doesn't check the do-it-once option @vpartington suggested
Browse files Browse the repository at this point in the history
  • Loading branch information
hierynomus committed Dec 7, 2012
1 parent 9a72370 commit fcdb1dc
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 9 deletions.
19 changes: 10 additions & 9 deletions src/main/java/com/xebialabs/overthere/ssh/SshConnection.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,7 @@
import static com.xebialabs.overthere.ConnectionOptions.PASSWORD;
import static com.xebialabs.overthere.ConnectionOptions.PORT;
import static com.xebialabs.overthere.ConnectionOptions.USERNAME;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.ALLOCATE_DEFAULT_PTY;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.ALLOCATE_DEFAULT_PTY_DEFAULT;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.ALLOCATE_PTY;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.CONNECTION_TYPE;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.INTERACTIVE_KEYBOARD_AUTH_PROMPT_REGEX;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.INTERACTIVE_KEYBOARD_AUTH_PROMPT_REGEX_DEFAULT;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.PASSPHRASE;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.PRIVATE_KEY_FILE;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.SSH_PORT_DEFAULT;
import static com.xebialabs.overthere.ssh.SshConnectionBuilder.*;

This comment has been minimized.

Copy link
@vpartington

vpartington Dec 8, 2012

Contributor

Grmbl. Didn't we synchronize our formatting and organize imports conventions?

import static java.net.InetSocketAddress.createUnresolved;

/**
Expand Down Expand Up @@ -100,6 +92,8 @@ abstract class SshConnection extends BaseOverthereConnection {

protected final boolean allocateDefaultPty;

protected final boolean openShellBeforeExecute;

protected String allocatePty;

protected SSHClient sshClient;
Expand Down Expand Up @@ -129,6 +123,7 @@ public SshConnection(final String protocol, final ConnectionOptions options, fin
logger.warn("The " + ALLOCATE_DEFAULT_PTY + " connection option has been deprecated in favour of the " + ALLOCATE_PTY + " option. See https://github.com/xebialabs/overthere#ssh_allocatePty");
}
this.allocatePty = options.getOptional(ALLOCATE_PTY);
this.openShellBeforeExecute = options.getBoolean(OPEN_SHELL_BEFORE_EXECUTE, OPEN_SHELL_BEFORE_EXECUTE_DEFAULT);
}

protected void connect() {
Expand Down Expand Up @@ -238,6 +233,12 @@ public OverthereProcess startProcess(final CmdLine commandLine) {

CmdLine cmd = processCommandLine(commandLine);
try {
if (openShellBeforeExecute) {
logger.debug("Creating a temporary shell to allow for deferred home dir creation.");
Session.Shell shell = getSshClient().startSession().startShell();

This comment has been minimized.

Copy link
@vpartington

vpartington Dec 8, 2012

Contributor

Shouldn't that session be closed too? Or just move it down and use the session that is created on line 242.

This comment has been minimized.

Copy link
@hierynomus

hierynomus Dec 10, 2012

Author Contributor

Can't reuse the session, Ssh/J explicitly disallows that.

See Session javadoc.

This comment has been minimized.

Copy link
@vpartington

vpartington Dec 12, 2012

Contributor

OK, but shouldn't this code close that session? I'm talking about the session returned by startSession() in line #237.

This comment has been minimized.

Copy link
@hierynomus

hierynomus Dec 12, 2012

Author Contributor

I already pushed that 2 days ago ;) e19c136

shell.close();
}

Session session = getSshClient().startSession();
if (allocatePty != null && !allocatePty.isEmpty()) {
if (allocateDefaultPty) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,19 @@ public class SshConnectionBuilder implements OverthereConnectionBuilder {
*/
public static final boolean SUDO_QUOTE_COMMAND_DEFAULT = false;

/**
* Name of the {@link ConnectionOptions connection option} used to specify whether or not to open a shell directly
* before executing a remote command. This may be necessary in case a user does not yet have a home directory on a
* machine, and this is created for him when he opens a shell.
*/
public static final String OPEN_SHELL_BEFORE_EXECUTE = "openShellBeforeExecute";

/**
* Default value (<code>false</code>) of the {@link ConnectionOptions connection option} used to specify whether or
* not to open a shell directory before executing a command.
*/
public static final boolean OPEN_SHELL_BEFORE_EXECUTE_DEFAULT = false;

protected SshConnection connection;

public SshConnectionBuilder(String type, ConnectionOptions options, AddressPortMapper mapper) {
Expand Down

1 comment on commit fcdb1dc

@vpartington
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice implementation. Let's see how it works without the only-do-it-once check first.

Please sign in to comment.