[Bug 6852] New: Unable to disentangle changing_parents and changing_files

[mercurial-bugs at mercurial-scm.org]

https://bz.mercurial-scm.org/show_bug.cgi?id=6852

            Bug ID: 6852
           Summary: Unable to disentangle changing_parents and
                    changing_files
           Product: Mercurial
           Version: 6.4
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: bug
          Priority: wish
         Component: Mercurial
          Assignee: bugzilla at mercurial-scm.org
          Reporter: jaraco at jaraco.com
                CC: mercurial-devel at mercurial-scm.org
    Python Version: ---

Mercurial 6.4 introduced `dirstate.changing_files` and made it mutually
exclusive with `dirstate.changing_parents`. Unfortunately, we've been unable
downstream to successfully honor this new constraint when accounting for
auto-widen operations.

As one particular example, consider an `hg copy` operation. The downstream
implementation adds these overrides for `cmdutil.copy` and
`scmutil.dirstatecopy`:

```
@eh.wrapfunction(cmdutil, 'copy')
def _overridecopy(orig, ui, repo, pats, opts, rename=False):
  if not pats or len(pats) == 1 or not citcutil.is_citc(repo):
    return orig(ui, repo, pats, opts, rename)
  srcs = pats[:-1]
  with repo.wlock(), repo.lock(), repo.transaction(b'citcrepo'):
    wctx = repo[None]
    sourcematcher = scmutil.match(wctx, srcs, opts, globbed=True)
    _expandnarrowspecfrommatcher(ui, repo, sourcematcher)
    return orig(ui, repo, pats, opts, rename)


@eh.wrapfunction(scmutil, 'dirstatecopy')
def _overridedirstatecopy(orig, ui, repo, wctx, src, dst, **kwargs):
  if not citcutil.is_citc(repo):
    return orig(ui, repo, wctx, src, dst, **kwargs)

  with repo.wlock(), repo.lock(), repo.transaction(b'citcrepo'):
    _expand_narrow_for_files(repo, [dst])
    return orig(ui, repo, wctx, src, dst, **kwargs)

```

Note that `_expand_narrow_for_files` and `_expandnarrowspecfrommatcher` both
expand the narrow spec:

```
def _expandnarrowspec(repo, addedincludes, expanddirstate=True):
  """Add the specified includes to narrowspec and update the dirstate."""
  if not addedincludes:
    return

  repo.ui.log(
      b'google_hgext',
      b'expanding narrowspec, adding %s\n',
      pycompat.bytestr(addedincludes),
  )

  # Only start a transaction after we know we're going to be modifying stuff.
  with repo.wlock(), repo.lock(), repo.transaction(b'expandnarrowspec'):
    # Read the current narrowspec that matches the dirstate.
    includes, excludes = repo.narrowpats

    # Let's keep any excludes the user may have manually added, even
    # though it's not something we recommend or support.
    newspec = (includes | addedincludes, excludes)
    # Set the new narrowspec (the one we got from CitC), so
    # manifest.walk() will be able to find the files to add. The
    # manifests are generated on the fly, so we don't need to download
    # them first, just add their paths to the narrowspec.
    repo.setnarrowpats(*newspec)

    if expanddirstate:
      updateworkingcopy(repo)
```

Which updates the working copy and thus requires a `changing_parents` context:

```
def updateworkingcopy(repo):
  with (
      repo.wlock(),
      repo.lock(),
      repo.transaction(b'auto-widen'),
      _changing_parents(repo),
  ):
    narrowspec.updateworkingcopy(repo)
    narrowspec.copytoworkingcopy(repo)
```

However, the copy command [unconditionally grabs
`changing_files`](https://foss.heptapod.net/mercurial/mercurial-devel/-/blob/1625fe807c04a490f9516bc8e14140e570c06146/mercurial/commands.py#L2502).
Even if one injects a layer between `commands.copy` and `cmdutil.copy` in order
for `_overridecopy` to inject its changing_parents behavior before
changing_files is entered, `cmdutil.copy` (with the changing_files context)
internally calls `scmutil.dirstatecopy`, which for autowiden requires the
changing_parents` context.

We've tried suppressing the `ProgrammingErrors` raised during
`_expandnarrowspec` (in addition to injecting `_overridecopy` early) in the
hopes that the earlier handling would be sufficient, but it's not the case. It
does seem to be the case that expanding the narrowspec in _overridedirstatecopy
is necessary, even for file copy operations.

Where we have seen some success is in [patching Mercurial to disable the
enforcement](https://gist.github.com/jaraco/a29a40517186e251eeab446ca2400aba)
in `changing_files` and `changing_parents` contexts (bypassing the
enforcement), since presumably these checks were unnecessary before and are
merely a nuisance (for now). Obviously, this condition is suboptimal and could
lead to further incompatibility down the road.

The question we'd like to answer:

How can one implicitly widen the narrow spec (which requires
`changing_parents`) during a `scmutil.dirstatecopy`, which is called by
`cmdutil.copy`, which holds a `changing_files` context?

-- 
You are receiving this mail because:
You are on the CC list for the bug.