Re: [edk2-devel] [RFC PATCH 2/3] Maintainers.txt: add wildcard path association for Arm/AArch64

["Leif Lindholm" <leif.lindholm@linaro.org>]

On Thu, Jun 20, 2019 at 05:29:55PM +0200, Laszlo Ersek wrote:
> Hi Leif,
> 
> On 06/14/19 22:21, Leif Lindholm wrote:
> > Add Ard and Leif as responsible for any path matching
> > F: */Arm/
> > F: */AArch64/
> > 
> > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> > ---
> >  Maintainers.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Maintainers.txt b/Maintainers.txt
> > index cd32f9b00170..e415f51468d5 100644
> > --- a/Maintainers.txt
> > +++ b/Maintainers.txt
> > @@ -82,6 +82,14 @@ EDK II Releases:
> >  W: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning
> >  M: Liming Gao <liming.gao@intel.com>
> >  
> > +EDK II Architectures:
> > +---------------------
> > +ARM, AARCH64
> > +F: */AArch64/
> > +F: */Arm/
> > +M: Leif Lindholm <leif.lindholm@linaro.org>
> > +M: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > +
> >  EDK II Packages:
> >  ----------------
> >  ArmPkg
> > 
> 
> I'm a bit confused now.
> 
> * In the blurb, you write,
> 
> "Wildcard support is not fully filesystem compliant except in *first* or
> last position in the path"
> 
> (emphasis mine). That would invalidate the present patch (because I
> gather your intent is to match any pathname that has *any* component
> called Aarch64 or Arm).

What I actually meant by that was that it matches for files in the
top directory (but not directories), as well as at the end.
Will reword.

> * Upon checking the third patch in the series, the "first position"
> exception doesn't seem to be implemented actually. Which should make the
> present patch work, in practice. And, we could ignore the statement in
> the blurb (given that the blurb is never captured in the git history).
> 
> * However... the explanation from the first patch of the series, namely
> 
> +     F:   */net/*         all files in "any top level directory"/net
> 
> conflicts with the script then!

Well, that remains a copy-paste from the QEMU description.
I would be tempted to keep current behaviour and simply not support
that particular case for now - updating the documentation to describe
actual behaviour.

> So right now, we have 6 locations:
> (a) the blurb

Needs rewording (as per above).

> (b) the "All patches CC:d here" section from Maintainers.txt

*  - all files in top-level directory
*/ - all files in all subdirectories of top-level directory

So, yes, I definitely *cheat* with transforming the second one to
'.*/.*', but it works :)
(but more below on '*')

> (c) the "Tianocore Stewards" section from Maintainers.txt

The same.

> (d) the "F: */net/*" example pattern from Maintainers.txt

Cut & paste error in documentation.

> (e) the present patch
> (f) the script (patch #3)

(more)
You are correct however - the 'F: *' special case isn't actually
working as intended. Currently that returns the expected result
because it hits the '<default>' rule.
I'll rework that.

Regards,

Leif

> Locations (a) through (d) say that "*" in the first position matches top
> level entries only, while (d) and (e) state "*" in the first position
> matches any -- possibly multi-component -- pathname prefix.
> 
> It's one thing that we can make the script in patch #3 conform to what
> the blurb says. But, that still leaves us with the problem that patches
> #1 and #2, considered together, use "*" in the first position in
> opposite senses.
> 
> Thanks,
> Laszlo