Subject: bug#37189: 25.4.1: vc-hg-ignore implementation is missing

From: Wolfgang Scherer <[email-masked]>
Date: Wed, 5 Feb 2020 06:18:08 +0100

>> Per-directory vs. per-tree
>> ==========================
>
> ...I don't feel I understand better your view of what you describe as
> fundamental design problems in this area.  I'm saying that after
> reading your description at least twice.  You make many assertions and
> tell what these features _should_ do, but there's little explanation
> of _why_ these assertions are true or why the commands should do this
> or that.

Usually a design does not explain why things should be the way they are specified. Also, having extracted the initial code for `vc` ignore files in the previous post, I did not think it was necessary to do it again. I also assume familiarity with the code in question and the mechanisms of ignore files for at least CVS, Git and Hg.

I think I have sufficiently shown that the original implemention of the ignore feature did not work correctly in a single case. If you disagree, just use emacs 26.3 and try to ignore files in `vc-dir-mode` or by calling `vc-ignore` interactively in some CVS, Git or Hg repositories. CVS will not work at all. As for Git, Hg, if you ignore anything below the root directory it will not work correctly. Even the entries on the root level are not properly anchored and/or escaped. I am just making this argument to show that an API change does not break anything, since it did not work to begin with.

I have further tried to fix the problem with the minimum amount of effort, which worked for some of the accepted corrections, but it did not work for `vc-hg-ignore`, which is unfortunately my main use case. However, in order to fix `vc-hg-ignore` without a hack (recognition of regular expressions as implemented in the patch of this bug report), it is necessary to expand the API of `vc-ignore` and the corresponding backend functions by at least adding a flag that indicates, whether the file parameter should be treated as a filepath or a pattern.

Other topics like per-subtree ignore files have not even been mentioned yet. But they do not need API changes and can therefore be implemented later as a change request for `vc-find-ignore-file`. Hint: the assumption that .gitignore or .hgignore are always relative to `vc-root-dir` is simply false.

Since I cannot comfortably explain the matters in the Email text format, I have prepared a HTML version at http://sw-amt.ws/README-emacs-vc-ignore-feature.html#vc-ignore-api-change-request. The description contains a discussion of wildcard pattern escaping/anchoring and two detailed (non-exhaustive) tables with use cases showing the implementation to be incorrect (even with the latest changes). The tables show, that the function `vc-ignore` should be able to produce different output for the same FILE / DIRECTORY input (e.g. "fil?.ext" nil), when

- submitted as filepath by `vc-dir-ignore`
- submitted as a wildcard specification by an interactive invocation of `vc-ignore`.

However, producing different output for the same input is inherently impossible for fully deterministic functions. Once that is understood, it becomes obvious, that the correct results can only be achieved, when the backend functions are supplied with additional information to determine whether a single file should be ignored (escaping the filepath properly) or whether a pattern should be added verbatim without escaping anything.

Since the ignore file sub-system did not even work at all before the last series of changes and the current request for `vc-hg-ignore` cannot be fixed cleanly without API changes and also because the ignore sub-system would still be broken, my proposition is to fix the design. Hence the draft design with some identified use cases for your convenience. Feel free to ignore it and try to fix the problem another way.

This is really all that is necessary to understand the problem. I have answered some further arguments, but they are really just repetitions.
>   Even the first step of your argument: that there's a
> fundamental difference between per-directory and per-tree ignore specs
> is left with almost no description of these fundamental differences.
> I don't think I have a clear idea of why you think so.

I am not sure, why such a description is needed. Although per-directory ignore files are simply a maximally reduced special case of per-tree ignore files, where all relative filepathes are identical to basenames, it is still not sufficient to implement only per-directory ignore files, since per-tree ignore files have additional requirements that are completely uncovered by the per-directory semantics of the original implementation. That is the same type of difference as for real numbers and complex numbers. Whether you want to call it "fundamental" or not, does not invalidate it. Real numbers are a limited subset of complex numbers and not the other way around.

The full specification how per-directory and per-tree ignore files work can be found in the man pages. If necessary, I can summarize those, but I would rather not.

Here is one fundamental difference between per-directory and per-tree ignore specs, which also shows that due to the conceptual lack of anchoring, per-directory algorithms cannot emulate per-tree algorithms. However, a per-tree algorithm with null anchors can perfectly well emulate per-directory algorithms.

- Per-directory ignore specifications do not need to be anchored.
- Per-tree ignore specifications must be properly anchored.

>> Use cases
>>
>> 1. Ignore a file in `vc-dir-mode`.
>>
>>    Write basename of absolute filepath into ignore file for directory.
>>
>> 2. Ignore marked files in `vc-dir-mode`.
>>
>>    Write basename of relative filepath into ignore file for directory.
>>
>> 3. Ignore absolute/relative filepath with `vc-ignore`.
>>
>>    Write basename of filepath into ignore file for directory.
>>
>> 4. Ignore pattern with `vc-ignore`.
>>
>>    Write pattern unaltered into ignore file of directory.
>
> This enumerates use cases and states their requirements, but doesn't
> explain those requirements.  I don't think I understand why sometimes
> the file specs need to be relative and sometimes absolute, because you
> just say so without any explanations.

Assuming familiarity with the code, it should be clear that `vc-dir-ignore` calls `vc-ignore` with the result from (vc-dir-current-file). In this case FILE is an absolute filepath.

For a set of marked files `vc-dir-ignore` calls `vc-ignore` repeatedly with the result from (vc-dir-fileinfo->name filearg). In this case FILE is a filepath relative to default-directory.

When calling `vc-ignore` interactively, the current directory is inserted at the prompt. In this case the FILE parameter is an absolute path unless the default directory is deleted first, then it becomes an undefined pattern.

When calling `vc-ignore` from lisp code any combination of FILE/DIRECTORY can be specified and should be processed sensibly.

>> The reference backend implementation is `vc-default-ignore`. Although
>> the documentation already specifies what the function should do in a
>> per-tree context, the original implementation strangely follows the
>> description of `vc-ignore` which is still per-directory.
>
> See, I don't even understand why you say vc-ignore is per-directory.

The **description** of `vc-ignore` is per-directory, while the **description** of `vc-default-ignore` is per-tree. That is obvious if you read both descriptions and do not confuse description and implementation.

The **implementation** of `vc-ignore` is inherently neither per-directory nor per-tree, because it only calls the appropriate backend function.

The original implementation of `vc-default-ignore` is per-directory only and therefore contradicts its description. The incorrect behavior is explained in bug #37217. While I partially corrected the implementation of `vc-default-ignore`, it is unsatisfactory in regard to a good abstraction of escaping and anchoring (incl. syntax detection), which would eliminate the need for specialized backend functions `vc-hg-ignore` and `vc-git-ignore`.

> That it accepts a directory as its argument doesn't yet mean it cannot
> support all of its subdirectories, recursively, which will make it
> per-tree.

The difference betwen per-directory and per-tree has nothing to do with a DIRECTORY parameter or recursion.
Per-tree is not a recursive per-directory algorithm and cannot be implemented as such.
However, per-directory is an automatic by-product of a per-tree algorithm (e.g. ignoring some file in the root directory).

A search of the `vc` source files for `per-\(dir\|tree\)` gives a hint that the concept of per-directory and per-tree is VCS specific:

vc-hooks.el\0110-(defcustom vc-handled-backends '(RCS CVS SVN SCCS SRC Bzr Git Hg Mtn)
vc-hooks.el\0111: ;; RCS, CVS, SVN, SCCS, and SRC come first because they are per-dir
vc-hooks.el\0112: ;; rather than per-tree. RCS comes first because of the multibackend

In regard to ignore file specifications, per-directory means that the file specification is a plain basename without directory parts, the scope of the specification is only the corresponding directory, no anchoring is necessary.
Per-tree means that the file specification may include directory pathes and unanchored file specifications are applied in all sub-directories, while anchored specs are only applied at the specified level.

Again, this is much better described in the respective man pages.

>   I'm probably missing something, but what?

The FILE parameter of `vc-ignore` is considered equivalent to a wildcard. This equivalence is false.

The design change is an additional parameter to `vc-ignore` and optionally new functions `vc-ignore-file`, `vc-ignore-pattern` to clarify the intended use.

Proposed shortcuts are:

`C-x v F` => `vc-ignore-file`
`C-x v G` => `vc-ignore-pattern`

in `vc-dir-mode`:

`F` => `vc-dir-ignore` => `vc-ignore-file`
`G` => `vc-ignore-pattern`.

> Once again, feel free to tell the discussion about the design is not
> useful and should be abandoned.

Are you saying that it is not useful to finally have a working ignore feature after more than 20 years?

The `vc` ignore feature implementation has not worked and still does not work correctly. But before adding some more adhoc patches without a final resolution, it is usually better to consider all problems and specify a working design.

Anyway, my contribution was not intended to be a final design but rather an incomplete collection of design criteria for the responsible maintainer. I also assumed, that the responsible maintainer has a general knowledge of the concepts for ignore files and specific knowledge of the `vc` ignore feature code.

>   I'm okay with discussing just the
> specific bug you have.

Unfortunately, the specific bug I encounter with `vc-hg-ignore` cannot be fixed without the API change.

>   But if we are to continue talking about design
> aspects, may I suggest that you explain your opinions more than you
> did until now?

The rationale for the design changes can be found in previous posts and other bug reports, where the problems with the implementation of `vc-hg-ignore` become apparent.

Among the differences between per-tree and per-directory semantics is the anchoring mechanism which is needed for per-tree and is unnecessary for per-directory ignore specifications.

Try to understand that although ignore specifications are always wildcard specifications, a generic frontend function like `vc-ignore` cannot ask other generic functions like `vc-dir-ignore` for properly escaped wildcard specifications, since the correct escape and anchoring mechanisms are unknown to frontend functions. Asking interactively for a raw pattern is feasible, since a user probably knows the appropriate syntax.

A frontend function should not be burdened with actually normalizing, escaping and anchoring filepath specifications. The high-level flag signaling either raw input or unescaped and unanchored filepath is sufficient and more readable.