Add ability to execute 'pull' command with additional args

[VadimKurnatovskiy]

Subject of the issue

After updating version from 1.11.0 to 1.19.1 command ::Git.open(directory, log: git_logger).pull('origin', [branch, flags]) it raises error:
gems/git-1.19.1/lib/git/lib.rb:1191:in 'command': cmd can not include a nested array (RuntimeError)

Temporarily fix is to call private method command directly:

git_instance.lib.send(:command, 'pull', 'origin', branch, flags)

Your environment

• git (1.11.0) -> git (1.19.1)
• Ruby 3.3.0

Steps to reproduce

g = ::Git.open(directory, log: git_logger)
branch = 'master'
flags = '--allow-unrelated-histories'

g.pull('origin', [branch, flags])

Expected behaviour

pull accept additional arguments

Actual behaviour

raises RuntimeError

[jcouball]

The usual way that additional flags are accepted by a Git::Lib method is to explicitly pass them in. If Git::Base#pull supported this option, it would look like:

working_git.pull('origin', 'branch', allow_unrelated_histories: true)

I would be happy to add that option for you.

For historical purposes, Git::Base#pull is documented as follows:

# pulls the given branch from the given remote into the current branch
#
#  @git.pull                          # pulls from origin/master
#  @git.pull('upstream')              # pulls from upstream/master
#  @git.pull('upstream', 'develope')  # pulls from upstream/develop
#
def pull(remote = nil, branch = nil)
  self.lib.pull(remote, branch)
end

and has been so for the last 11 years (according to git blame). I think it has been (up to now) a happy accident that your previous code worked.

Also, Git::Base#lib should be considered private (even though it actually isn't private). Good for testing and workarounds like you have done, but not for the long run.

[lHydra]

The usual way that additional flags are accepted by a Git::Lib method is to explicitly pass them in

@jcouball Can you please tell me why this approach is used?

Why don't we let the user pass the options he wants and not adjust them directly in the code?

[jcouball]

Thank you for the question, @lHydra, it is a good one.

I was not around when this decision was made and it wasn't documented as the 'general approach' but that is how the gem is constructed and how individual methods are documented.

I think this is a reasonable approach because many of the switches to the git command-line alter the behavior and output enough to make the git gem function incorrectly or not at all. To ensure that the gem works correctly, only options that are explicitly allowed are let through.

[Edited to add the following]

I don't know why a comprehensive sweep of available options "that work" were not added before this point in time, but here we are. There are probably many options that could be supported but are not. Because there is an overhead involved with each option added, we have adopted the approach of adding options as users see the need like in your case.

[jcouball]

What do you think about the following proposal @lHydra?

Barring a change to the status quo, I think there are a few action items that can be done as a response to this issue:

1. Fix @lHydra problem by adding the --allow-unrelated-histories flag to Git::Base#pull.
2. Update the documentation for this method to be proper Yard documentation. The current documentation is lacking in both form and content.
3. Document somewhere the overall design of how and why git command-line options are allowed or disallowed.

[lHydra]

What do you think about the following proposal @lHydra?

Barring a change to the status quo, I think there are a few action items that can be done as a response to this issue:

1. Fix @lHydra problem by adding the `--allow-unrelated-histories` flag to `Git::Base#pull`.

2. Update the documentation for this method to be proper Yard documentation. The current documentation is lacking in both form and content.

3. Document somewhere the overall design of how and why git command-line options are allowed or disallowed.

I think that would be a good solution @jcouball

Can I make a pull request to implement point 1 and 2?

[jcouball]

Can I make a pull request to implement point 1 and 2?

Sounds great. Thank you.

[light-flight]

jcouball this issue can be closed
PR: #697