[PATCH] eol: support alternate location for .hgeol file (issue3975)

[Martin Geisler]

Jorge Acereda <jacereda at brainstorm.es> writes:

> On Jul 11, 2013, at 4:01 PM, Martin Geisler wrote:
>
>> Jorge Acereda <jacereda at brainstorm.es> writes:
>> 
>>> # HG changeset patch
>>> # User Jorge Acereda <jacereda at brainstorm.es>
>>> # Date 1373469140 -7200
>>> # Branch stable
>>> # Node ID 3487e904f78164e70cf91552419e1ecf235d8daf
>>> # Parent  fbdac607bff3dfc0056268ca77e524a5ddd4f665
>>> eol: support alternate location for .hgeol file (issue3975)
>> 
>> I'm still not completely convinced that this is a good idea, but here
>> are some review comments on your patch.
>
> Nor am I, if you have a good solution for this scenario I'd be glad to
> hear it.
>
> Basically I have 80 subrepos and I don't want to maintain 80 .hgeol
> files.

Here is a possible alternative: Writing a update-subrepo-hgeol.sh script
in the outer repository should be simple. It would do

  #!/bin/sh

  hg debugsub | grep '^path' | cut -c 6- | while read -r sub; do
      cp dot.hgeol "$sub/.hgeol"
  done

That is, it copies dot.hgeol from the outer repository into the
subrepositories. Maybe you've already rejected this for some reason?

>>> +An alternate location for the ``.hgeol`` configuration file can be
>>> +specified in a ``config`` setting in the ``[eol]`` section of your
>>> +``hgrc``. This can be useful when dealing with lots of subrepos.
>>> When +the ``.hgeol`` file is present in a repository, it will take
>>> +precedence over the ``config`` setting.
>> 
>> I think 'config' is too generic -- what about using 'eolpath' instead?
>
> eol.eolpath seems a bit redundant. 'configpath'?

Yeah, that sounds better to my ears.

>> Falling back to the configured path feels backwards: when configure
>> something I expect it to override the normal behavior.
>
> You might want to keep 78 subrepos sharing the same hgeol and 2
> subrepos might have special needs that can be defined via their own
> .hgeol.
>
>
>> Maybe one could instead read both files (if they exist)? So by
>> default Mercurial reads no file, when the eol extension is enabled it
>> reads <repo>/.hgeol by default and with your setting you can extend
>> the list of files read. That somehow sounds better to my ears.
>
> Would be cool to specify something like:
>
> [eol]
> configpaths = ~/sharedcfg/hgeol .hgeol
>
> to specify that both files should be read, but I'm not familiar with
> the .ini syntax. How do you specify a list in a .ini?

We have a ui.configlist function to read lists from .ini files and 'hg
help config' says that:

    List values are separated by whitespace or comma, except when values
    are placed in double quotation marks:

      allow_read = "John Doe, PhD", brian, betty

    Quotation marks can be escaped by prefixing them with a backslash.
    Only quotation marks at the beginning of a word is counted as a
    quotation (e.g., "foo"bar baz" is the list of "foo"bar" and "baz").

So the syntax you use above would work.

>>> +Example ``.hgrc`` file specifying the location of the ``.hgeol`` file::
>>> +
>>> +  [eol]
>>> +  config = ~/shared-cfg/hgeol
>>> +
>>> +The ``.hgeol`` file use the same syntax as all other Mercurial
>>> configuration files. It uses two sections, ``[patterns]`` and
>>> ``[repository]``.
>>> 
>>> @@ -207,7 +219,12 @@
>>>                 if node is None:
>>>                     # Cannot use workingctx.data() since it would load
>>>                     # and cache the filters before we configure them.
>>> -                    data = repo.wfile('.hgeol').read()
>>> +                    try:
>>> +                        f = repo.wfile('.hgeol')
>>> +                    except IOError:
>>> +                        f = open(util.expandpath(ui.config('eol', 'config')))
>> 
>> I guess this will read a relative path relative to the current working
>> directory? It should probably be changed to be relative to the
>> repository root.
>
>
> I was thinking only absolute paths would be useful. Is there any
> utility to open a path relative to the root?

Yes, I believe what would be the wfile function above. But I think
Laurens is right that the paths should be relative to the path of the
current config file. I don't remember what utility functions we have for
that.

-- 
Martin Geisler