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 access to SCM::Plugin instance from deploy scripts #2086

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

Conversation

mvastola
Copy link

Accessible via scm_plugin method.

Summary

Provides a way to access the deployment's Capistrano::SCM::Plugin instance from within a deploy script. (via an scm_plugin method)

My use case for this was that I wanted to invoke git:check on my local machine, from which I trigger the deployment. As my local machine does not have a release role, it isn't possible to simply invoke the built-in rake task.

Short checklist

  • Did you run bundle exec rubocop -a to fix linter issues?
  • If relevant, did you create a test?
  • Did you confirm that the RSpec tests pass?

Accessible via `scm_plugin` method.
end

attr_reader :scm_plugin
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I wonder if this should be :scm_plugins (plural) as an array? I believe the installer allows someone to install multiple SCM plugins. Or in any case, it doesn't prevent this from happening. If there were to be multiple SCM plugins installed, then a singular scm_plugin is misleading and establishes the wrong API contract.

In which case I think @scm_plugins should be initialized to an empty array, and then your code could be changed slightly:

@scm_plugins << plugin if provides_scm?(plugin)

What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I didn't realize that was possible.

Hrm. I did consider making this accessible for all plugins though. The reason why I didn't just do that was twofold:

  • using an array can easily become confusing (even if we limit it to scm plugins) because you basically have to run #detect on it every time you want to access something
  • the obvious solution to that would be something like a hash, there is no straight-forward mechanism (that I know of) to get a plugin's name.

Maybe the better solution in this case would be to add an optional as argument so you can do, for instance install_plugin Capistrano::SCM::Git, as: :git, and that will define a git method that returns the plugin?

Let me know what you think.

This does make me realize a way around all this though (though it's less elegant). Something like:

set :git, Capistrano::SCM::Git.new
install_plugin fetch(:git)

Copy link
Author

@mvastola mvastola Jun 20, 2021

Choose a reason for hiding this comment

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

Hm. Looking at this some more I'm inclined to ask how common and supported a use case it is to use multiple repository types.

While this may in fact be possible, it seems like there hasn't been much effort elsewhere in the codebase to support it. For example, all repository types share the settings :repo_url and :branch. Also the configure_scm method and all of SCMResolver seems to only support a single SCM.

It seems that if using multiple SCMs is possible, it more than likely requires a fair bit monkey patching to work. This is particularly because the settings :repo_url and :branch are agnostic of the SCM used. While it's possible to run different scm services at the same URL (for instance, relying on the services having different default ports), it seems like it would be a pretty unusual setup.

And if support is shaky at best, I think it's reasonable to consider adding that support out of scope for this PR.

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.

None yet

2 participants