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.

Sign up for GitHub

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

Jump to bottom

Allow Tapioca commands to accept environment variables #1957

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Allow Tapioca commands to accept environment variables #1957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ Commands:
Options:
-c, [--config=<config file path>] # Path to the Tapioca configuration file
# Default: sorbet/tapioca/config.yml
[--env=key:value] # Environment variables to set before running Tapioca
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: it might be my lack of familiarity with Thor, but it wasn't immediately obvious to me how we'd be passing multiple environment variables to the same command (I understood after reading the tests).

Could we add an example somewhere in the README of how to pass these?

Copy link
Member Author

Choose a reason for hiding this comment

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

So, we already have a few hash type options and how they work is documented here: https://github.com/rails/thor/wiki/Method-Options#types-for-method_options

The main way is to use the --opt=key:value key2:value2 way, but if you mark the option as repeatable: true, then you can also do --opt=key:value --opt=key2:value2 too.

I guess I can add a section for how to pass env variables via command line or config file to make that more clear and link to the Thor docs for details.

-V, [--verbose], [--no-verbose], [--skip-verbose] # Verbose output for debugging purposes
# Default: false

Expand Down Expand Up @@ -122,6 +123,7 @@ Usage:
Options:
-c, [--config=<config file path>] # Path to the Tapioca configuration file
# Default: sorbet/tapioca/config.yml
[--env=key:value] # Environment variables to set before running Tapioca
-V, [--verbose], [--no-verbose], [--skip-verbose] # Verbose output for debugging purposes
# Default: false

Expand Down Expand Up @@ -202,6 +204,7 @@ Options:
# Default: true
-c, [--config=<config file path>] # Path to the Tapioca configuration file
# Default: sorbet/tapioca/config.yml
[--env=key:value] # Environment variables to set before running Tapioca
-V, [--verbose], [--no-verbose], [--skip-verbose] # Verbose output for debugging purposes
# Default: false

Expand Down Expand Up @@ -376,6 +379,7 @@ Options:
--typed, -t, [--typed-overrides=gem:level [gem:level ...]] # Override for typed sigils for pulled annotations
-c, [--config=<config file path>] # Path to the Tapioca configuration file
# Default: sorbet/tapioca/config.yml
[--env=key:value] # Environment variables to set before running Tapioca
-V, [--verbose], [--no-verbose], [--skip-verbose] # Verbose output for debugging purposes
# Default: false

Expand Down Expand Up @@ -505,6 +509,7 @@ Options:
[--compiler-options=key:value] # Options to pass to the DSL compilers
-c, [--config=<config file path>] # Path to the Tapioca configuration file
# Default: sorbet/tapioca/config.yml
[--env=key:value] # Environment variables to set before running Tapioca
-V, [--verbose], [--no-verbose], [--skip-verbose] # Verbose output for debugging purposes
# Default: false

Expand Down Expand Up @@ -900,6 +905,7 @@ Options:
-w, [--workers=N] # Number of parallel workers (default: auto)
-c, [--config=<config file path>] # Path to the Tapioca configuration file
# Default: sorbet/tapioca/config.yml
[--env=key:value] # Environment variables to set before running Tapioca
-V, [--verbose], [--no-verbose], [--skip-verbose] # Verbose output for debugging purposes
# Default: false

Expand Down Expand Up @@ -938,11 +944,17 @@ The full configuration file, with each option and its default value, would look
```yaml
---
require:
env: {}
verbose: false
postrequire: sorbet/tapioca/require.rb
todo:
env: {}
verbose: false
todo_file: sorbet/rbi/todo.rbi
file_header: true
dsl:
env: {}
verbose: false
outdir: sorbet/rbi/dsl
file_header: true
only: []
Expand All @@ -958,6 +970,8 @@ dsl:
skip_constant: []
compiler_options: {}
gem:
env: {}
verbose: false
outdir: sorbet/rbi/gems
file_header: true
all: false
Expand All @@ -978,6 +992,8 @@ gem:
environment: development
halt_upon_load_error: true
check_shims:
env: {}
verbose: false
gem_rbi_dir: sorbet/rbi/gems
dsl_rbi_dir: sorbet/rbi/dsl
shim_rbi_dir: sorbet/rbi/shims
Expand All @@ -986,6 +1002,8 @@ check_shims:
payload: true
workers: 1
annotations:
env: {}
verbose: false
sources:
- https://raw.githubusercontent.com/Shopify/rbi-central/main
netrc: true
Expand Down
4 changes: 4 additions & 0 deletions lib/tapioca/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ class Cli < Thor
type: :string,
desc: "Path to the Tapioca configuration file",
default: TAPIOCA_CONFIG_FILE
class_option :env,
type: :hash,
desc: "Environment variables to set before running Tapioca",
repeatable: true
class_option :verbose,
aliases: ["-V"],
type: :boolean,
Expand Down
1 change: 1 addition & 0 deletions lib/tapioca/helpers/config_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def validate_config!(config_file, config)
).returns(T::Array[ConfigError])
end
def validate_config_options(command_options, config_key, config_options)
command_options = T.unsafe(self.class).class_options.merge(command_options)
config_options.filter_map do |config_option_key, config_option_value|
command_option = command_options[config_option_key.to_sym]
error_msg = "unknown option `#{config_option_key}` for key `#{config_key}`"
Expand Down
12 changes: 11 additions & 1 deletion lib/tapioca/helpers/env_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,20 @@ module EnvHelper

requires_ancestor { Thor }

private

sig { params(options: T::Hash[Symbol, T.untyped]).void }
def set_environment(options) # rubocop:disable Naming/AccessorMethodName
ENV["RAILS_ENV"] = ENV["RACK_ENV"] = options[:environment]
set_all_environment_variables(options[:env])
ENV["RAILS_ENV"] = ENV["RACK_ENV"] = options[:environment] if options[:environment]
ENV["RUBY_DEBUG_LAZY"] = "1"
end

sig { params(options: T.nilable(T::Hash[String, String])).void }
def set_all_environment_variables(options) # rubocop:disable Naming/AccessorMethodName
(options || {}).each do |key, value|
ENV[key] = value
Copy link
Collaborator

Choose a reason for hiding this comment

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

So the --env values are overriding the ENV, is this really what we want?

If so, I think we should document this in the option description or the README.

Copy link
Member

Choose a reason for hiding this comment

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

Also, if this is indeed what we want, is there a reason not to use ENV.merge!(options)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I didn't think of ENV.merge!, that sounds better!

Copy link
Member Author

Choose a reason for hiding this comment

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

So the --env values are overriding the ENV, is this really what we want?

What else could they be doing? Tapioca sets those env variables as the user has passed them and we don't care if they override some values or not. bin/tapioca dsl --env FOO:42 is the same thing that happens when someone runs FOO=42 bin/tapioca dsl, in either case we override whatever the value of the FOO env variable might have been to make it 42 for the duration of that run.

end
end
end
end
11 changes: 11 additions & 0 deletions spec/tapioca/cli/dsl_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2677,6 +2677,17 @@ def title=(title); end
RACK ENVIRONMENT: staging
OUT
end

it "must accept and set custom environment variables" do
@project.write!("lib/post.rb", <<~RB)
$stderr.puts "ENVIRONMENT: \#{ENV.to_h.inspect}"
RB

result = @project.tapioca("dsl --env Foo:Bar --env Baz:1")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a test showing the behaviour of overriding system env?


assert_stderr_includes(result, '"Foo"=>"Bar"')
assert_stderr_includes(result, '"Baz"=>"1"')
end
end

describe "list compilers" do
Expand Down
5 changes: 3 additions & 2 deletions tasks/readme.rake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ task :readme do
end

def skip_option?(option)
option.name == "auth"
ignored_options = ["auth", "config"]
ignored_options.include?(option.name)
end

def option_value(option)
Expand All @@ -54,7 +55,7 @@ task :readme do
end

def command_options(command)
command.options.filter_map do |name, opt|
Tapioca::Cli.class_options.merge(command.options).filter_map do |name, opt|
next if skip_option?(opt)

[name.to_s, option_value(opt)]
Expand Down
Loading