capistrano / capistrano/capistrano

Using set /w proc leads to confusing behavior with append, validation, mutation

Open
#1,700 2 comments 2 reactions 0 assignees View on GitHub
discuss!
Dominant language
Ruby
Stars
13k
Forks
1.7k
PR merge metrics
No merged PRs in 30d

Description

Background:

The `set :var, -> { ... }` style of passing a Proc to delay execution is a fairly common technique to set values that are lazily executed. This is similar to the `let` syntax in RSpec.

Typically (in my experience), we do this when one variable depends on another. For example, it is often the case that a value must include the stage name. At the time of declaring a variable in `deploy.rb`, the stage has not yet been loaded. So we do this:

``` ruby
# For example
set :log_file, -> { "#{fetch(:stage)}.log" }
```

Later, when we `fetch(:log_file)`, the Proc is called and we get e.g. `production.log`. So far, so good.

However, if we want to _mutate_ a lazily evaluated variable, then we get into trouble. Consider this:

``` ruby
set :log_files, -> { ["#{fetch(:stage)}.log"] }
append :log_files, "httpd.log"
```

The `append` call has a side-effect of immediately "dereferencing" `:log_files`. So when we later `fetch(:log_file)`, we git this: `[nil, "httpd.log"]`.

Another example. Imagine we have a Capistrano plugin that automatically manages environment variables used for our server process. It expects a Hash and initializes it to this default value:

``` ruby
# Default value
set_if_empty :server_env, -> { { "RAILS_ENV" => fetch(:stage) } }
```

What happens when we want to mutate the Hash?

``` ruby
# In deploy.rb
fetch(:server_env)["NEW_KEY"] = "value"
```

Oops, once again, when we later `fetch(:server_env)`, we get: `{ "RAILS_ENV" => nil, "NEW_KEY" => "value" }`.

This problem has plagued many plugin developers. People have gone great lengths to work around the problem, which ultimately makes it hard to understand what Capistrano is actually doing.

For example, in `capistrano-rails`, a hack is used to hook into the stage loading process:

``` ruby
# All this code just to set a single variable!
namespace :deploy do
task :set_rails_env do
set :rails_env, (fetch(:rails_env) || fetch(:stage))
end
end

Capistrano::DSL.stages.each do |stage|
after stage, 'deploy:set_rails_env'
end
```

Likewise, to append the assets directory to `:linked_dirs`, we can't simply set `:linked_dirs` to a Proc, since it very likely will get dereferenced by the user or by other plugins. So another hack is needed:

``` ruby
# we can't set linked_dirs in load:defaults,
# as assets_prefix will always have a default value
namespace :deploy do
task :set_linked_dirs do
set :linked_dirs, fetch(:linked_dirs, []).push("public/#{fetch(:assets_prefix)}")
end
end

after 'deploy:set_rails_env', 'deploy:set_linked_dirs'
```

---

I don't have a suggestion yet on how to improve this, but I would like to start the discussion.

Contributor guide

Open the contributing guide

Research direction

Start by tracing the set, fetch, and append entry points involved in Proc evaluation and mutation; the issue names no files or tests. Review the Capistrano::DSL behavior and the cited plugin workarounds, then establish the expected lazy-mutation semantics with maintainers. Done means the behavior is agreed and covered by regression tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
devops, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.