-
Notifications
You must be signed in to change notification settings - Fork 90
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
Login shell #149
Login shell #149
Changes from 5 commits
db17269
5058023
b4213f2
388ebe3
0af1653
175c14c
57a47b5
5f5211d
b8859a1
8e9eb42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,8 @@ class LinuxCommand < CommandWrapperBase | |
Train::Options.attach(self) | ||
|
||
option :sudo, default: false | ||
option :shell, default: false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its a little bit sub-optimal to switch types of a variable, I would prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also lets add a comment to give users an idea of potential values |
||
option :login, default: false | ||
option :sudo_options, default: nil | ||
option :sudo_password, default: nil | ||
option :sudo_command, default: nil | ||
|
@@ -39,11 +41,12 @@ def initialize(backend, options) | |
validate_options(options) | ||
|
||
@sudo = options[:sudo] | ||
@shell = options[:shell] | ||
@login = options[:login] | ||
@sudo_options = options[:sudo_options] | ||
@sudo_password = options[:sudo_password] | ||
@sudo_command = options[:sudo_command] | ||
@user = options[:user] | ||
@prefix = build_prefix | ||
end | ||
|
||
# (see CommandWrapperBase::verify) | ||
|
@@ -71,18 +74,22 @@ def verify | |
|
||
# (see CommandWrapperBase::run) | ||
def run(command) | ||
@prefix + command | ||
shell_wrap(sudo_wrap(command)) | ||
end | ||
|
||
def self.active?(options) | ||
options.is_a?(Hash) && options[:sudo] | ||
options.is_a?(Hash) && ( | ||
options[:sudo] || | ||
options[:shell] || | ||
options[:login] | ||
) | ||
end | ||
|
||
private | ||
|
||
def build_prefix | ||
return '' unless @sudo | ||
return '' if @user == 'root' | ||
def sudo_wrap(cmd) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets add comments what these function do |
||
return cmd unless @sudo | ||
return cmd if @user == 'root' | ||
|
||
res = (@sudo_command || 'sudo') + ' ' | ||
|
||
|
@@ -93,7 +100,15 @@ def build_prefix | |
|
||
res << @sudo_options.to_s + ' ' unless @sudo_options.nil? | ||
|
||
res | ||
res + cmd | ||
end | ||
|
||
def shell_wrap(cmd) | ||
return cmd unless @shell || @login | ||
login_param = '--login ' if @login | ||
shell = @shell.instance_of?(String) ? @shell : '$SHELL' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer not to check for the type here |
||
|
||
"#{shell} #{login_param}<<< \"#{cmd}\"" | ||
end | ||
end | ||
|
||
|
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.
Why are we changing the default setting?
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 it's because we don't yet have frozen string literal comments in our files:
http://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/FrozenStringLiteralComment
I think it's ok to disable for now and let's tackle this in a rubocop cleanup session 😄