BF: GitRepo.merge do not allow merging unrelated unconditionally

[yarikoptic]

As @bpoldrack identified in
#7291 (comment) the 629e575 which was part of the #4650 concerning introduction of minimal git version to be 2.19.1, and not removing that condition was was combined with argument to the merge function. That commit was released within good old 0.13.4.

This change merely reintroduces "if" statement BUT unfortunately that allow_unrelated nowhere exposed in the API really, and used only once with allow_unrelated=True in some single test. So I expect some tests to fail here and most likely us needing to introduce similar option in "update" to finalize this PR.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.98 🎉

Comparison is base (df23bd7) 88.71% compared to head (b6d7b6d) 90.69%.

Additional details and impacted files

@@            Coverage Diff             @@
##            maint    #7312      +/-   ##
==========================================
+ Coverage   88.71%   90.69%   +1.98%     
==========================================
  Files         327      327              
  Lines       44533    44545      +12     
  Branches     5910     5911       +1     
==========================================
+ Hits        39509    40402     +893     
+ Misses       5009     4128     -881     
  Partials       15       15              

Impacted Files	Coverage Δ
datalad/distribution/tests/test_update.py	99.82% <100.00%> (+<0.01%)	⬆️
datalad/support/gitrepo.py	92.50% <100.00%> (+<0.01%)	⬆️

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[adswa]

Appveyor and Travis are green, but the CrippledFS tests show

=================================== FAILURES ===================================
______________________ test_rerun_fastforwardable_mutator ______________________

path = '/crippledfs/datalad_temp_test_rerun_fastforwardable_mutator1umaj1wa'

    @slow
    @with_tempfile(mkdir=True)
    def test_rerun_fastforwardable_mutator(path=None):
        ds = Dataset(path).create()
        # keep direct repo accessor to speed things up
        ds_repo = ds.repo
        ds_repo.checkout(DEFAULT_BRANCH, options=["-b", "side"])
        ds.run("echo foo >>foo")
        ds_repo.checkout(DEFAULT_BRANCH)
        ds_repo.merge("side", options=["-m", "Merge side", "--no-ff"])
        #  o                 c_n
        #  |\
        #  | o               b_r
        #  |/
        #  o                 a_n
    
>       ds.rerun(since="", onto=DEFAULT_BRANCH + "^2")

/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/local/tests/test_rerun_merges.py:105: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/distribution/dataset.py:502: in apply_func
    return f(*args, **kwargs)
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/interface/base.py:773: in eval_func
    return return_func(*args, **kwargs)
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/interface/base.py:763: in return_func
    results = list(results)
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/interface/base.py:885: in _execute_command_
    allkwargs):
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/interface/utils.py:319: in _process_results
    for res in results:
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/local/rerun.py:278: in __call__
    for res in handler(ds, results):
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/local/rerun.py:453: in _rerun
    new_parents[1:])
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/dataset/gitrepo.py:461: in call_git
    keep_ends=True))
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/dataset/gitrepo.py:522: in call_git_items_
    sep=sep):
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/dataset/gitrepo.py:355: in _generator_call_git
    for file_no, content in generator:
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/_collections_abc.py:317: in __next__
    return self.send(None)
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/runner/nonasyncrunner.py:97: in send
    return self._locked_send(message)
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/runner/nonasyncrunner.py:122: in _locked_send
    self._check_result()
/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/runner/nonasyncrunner.py:93: in _check_result
    self.runner._check_result()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <datalad.runner.nonasyncrunner.ThreadedRunner object at 0x7f303e27f650>

    def _check_result(self):
        if self.exception_on_error is True:
            if self.return_code not in (0, None):
                protocol = self.protocol
                decoded_output = {
                    source: protocol.fd_infos[fileno][1].decode(protocol.encoding)
                    for source, fileno in (
                        ("stdout", protocol.stdout_fileno),
                        ("stderr", protocol.stderr_fileno))
                    if protocol.fd_infos[fileno][1] is not None
                }
                raise CommandError(
                    cmd=self.cmd,
                    code=self.return_code,
                    stdout=decoded_output.get("stdout", None),
>                   stderr=decoded_output.get("stderr", None)
                )
E               datalad.runner.exception.CommandError: CommandError: 'git -c diff.ignoreSubmodules=none merge -m 'Merge side
E               ' --no-ff --allow-unrelated-histories 508340379dd243ed50b8ca95b511a12999e4b5aa' failed with exitcode 128 [err: 'error: Your local changes to the following files would be overwritten by merge:
E               	foo
E               Please commit your changes or stash them before you merge.
E               Aborting']

/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/datalad/runner/nonasyncrunner.py:299: CommandError

[adswa]

If that test failure is unrelated, this PR did not result in test failures - shall we take it out of draft mode?

[yarikoptic]

If that test failure is unrelated, this PR did not result in test failures - shall we take it out of draft mode?

done -- ready for merge although someone really eager could have made a test so we carve in code that we do not merge unrelated histories by default ;)

[yarikoptic]

As it is user visible change, adding changelog

[yarikoptic]

added rudimentary unittest

[adswa]

I think this is a saner default that what is currently happening - so I'd be in favor of merging this.
Just to clarify, though: There is not outside switch to allow merging unrelated histories, right? Users would have to resort to doing it with Git?

[yarikoptic]

There is not outside switch to allow merging unrelated histories, right? Users would have to resort to doing it with Git?

AFAIK: yes, yes ;-) the situation should be sufficiently rare to not bother with an explicit switch IMHO. We might though improve error/hint messaging and/or provide some config variable to allow for temporary change to this.

[yarikoptic-gitmate]

PR released in 0.18.3