[PATCH] gitignore: Move ignores from global to subdirectories

Elliott Mitchell posted 1 patch 3 years, 8 months ago
Failed in applying to current master (apply log)
.gitignore                   | 29 -----------------------------
tools/misc/.gitignore        | 22 +++++++++++++++++++++-
xen/tools/kconfig/.gitignore |  6 ++++++
xen/xsm/flask/.gitignore     |  8 +++++++-
4 files changed, 34 insertions(+), 31 deletions(-)
[PATCH] gitignore: Move ignores from global to subdirectories
Posted by Elliott Mitchell 3 years, 8 months ago
Subdirectories which have .gitignore files should not be referenced in
the global .gitignore files.  Move several lines to appropriate subdirs.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>

---
Hopefully the commit message covers it.  When moved to the subdirectories
I'm using "./<file>" as otherwise any file sharing the name in a deeper
subdirectory would be subject to the match.

---
 .gitignore                   | 29 -----------------------------
 tools/misc/.gitignore        | 22 +++++++++++++++++++++-
 xen/tools/kconfig/.gitignore |  6 ++++++
 xen/xsm/flask/.gitignore     |  8 +++++++-
 4 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/.gitignore b/.gitignore
index bb0fb64d32..c8529bc858 100644
--- a/.gitignore
+++ b/.gitignore
@@ -201,21 +201,6 @@ tools/libxl/ssdt*
 tools/libxl/testenum
 tools/libxl/testenum.c
 tools/libxl/tmp.*
-tools/misc/cpuperf/cpuperf-perfcntr
-tools/misc/cpuperf/cpuperf-xen
-tools/misc/xc_shadow
-tools/misc/xen_cpuperf
-tools/misc/xen-cpuid
-tools/misc/xen-detect
-tools/misc/xen-diag
-tools/misc/xen-tmem-list-parse
-tools/misc/xen-livepatch
-tools/misc/xenperf
-tools/misc/xenpm
-tools/misc/xen-hvmctx
-tools/misc/xenlockprof
-tools/misc/lowmemd
-tools/misc/xencov
 tools/pkg-config/*
 tools/qemu-xen-build
 tools/xentrace/xenalyze
@@ -312,16 +297,7 @@ xen/test/livepatch/xen_bye_world.livepatch
 xen/test/livepatch/xen_hello_world.livepatch
 xen/test/livepatch/xen_nop.livepatch
 xen/test/livepatch/xen_replace_world.livepatch
-xen/tools/kconfig/.tmp_gtkcheck
-xen/tools/kconfig/.tmp_qtcheck
 xen/tools/symbols
-xen/xsm/flask/include/av_perm_to_string.h
-xen/xsm/flask/include/av_permissions.h
-xen/xsm/flask/include/class_to_string.h
-xen/xsm/flask/include/flask.h
-xen/xsm/flask/include/initial_sid_to_string.h
-xen/xsm/flask/policy.*
-xen/xsm/flask/xenpolicy-*
 tools/flask/policy/policy.conf
 tools/flask/policy/xenpolicy-*
 xen/xen
@@ -357,8 +333,6 @@ tools/include/xen-foreign/arm32.h
 tools/include/xen-foreign/arm64.h
 
 .git
-tools/misc/xen-hptool
-tools/misc/xen-mfndump
 tools/libs/toolcore/include/_*.h
 tools/libxc/_*.[ch]
 tools/libxl/_*.[ch]
@@ -370,9 +344,6 @@ tools/libxl/test_timedereg
 tools/libxl/test_fdderegrace
 tools/firmware/etherboot/eb-roms.h
 tools/firmware/etherboot/gpxe-git-snapshot.tar.gz
-tools/misc/xenwatchdogd
-tools/misc/xen-hvmcrash
-tools/misc/xen-lowmemd
 tools/libvchan/vchan-node[12]
 tools/ocaml/*/.ocamldep.make
 tools/ocaml/*/*.cm[ixao]
diff --git a/tools/misc/.gitignore b/tools/misc/.gitignore
index c5fe2cfccd..b561bb023e 100644
--- a/tools/misc/.gitignore
+++ b/tools/misc/.gitignore
@@ -1 +1,21 @@
-xen-ucode
+./cpuperf/cpuperf-perfcntr
+./cpuperf/cpuperf-xen
+./lowmemd
+./xc_shadow
+./xen-cpuid
+./xen-detect
+./xen-diag
+./xen-hptool
+./xen-hvmcrash
+./xen-hvmctx
+./xen-livepatch
+./xen-lowmemd
+./xen-mfndump
+./xen-tmem-list-parse
+./xen-ucode
+./xen_cpuperf
+./xencov
+./xenlockprof
+./xenperf
+./xenpm
+./xenwatchdogd
diff --git a/xen/tools/kconfig/.gitignore b/xen/tools/kconfig/.gitignore
index ca38e983d6..69780c04cd 100644
--- a/xen/tools/kconfig/.gitignore
+++ b/xen/tools/kconfig/.gitignore
@@ -16,3 +16,9 @@ mconf
 nconf
 qconf
 gconf
+
+#
+# temporary build tool checking files
+#
+./.tmp_gtkcheck
+./.tmp_qtcheck
diff --git a/xen/xsm/flask/.gitignore b/xen/xsm/flask/.gitignore
index 024edbe0ba..b10754e646 100644
--- a/xen/xsm/flask/.gitignore
+++ b/xen/xsm/flask/.gitignore
@@ -1 +1,7 @@
-/policy.c
+./include/av_perm_to_string.h
+./include/av_permissions.h
+./include/class_to_string.h
+./include/flask.h
+./include/initial_sid_to_string.h
+./policy.*
+./xenpolicy-*
-- 
2.20.1




-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445



Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Jan Beulich 3 years, 8 months ago
On 28.08.2020 04:57, Elliott Mitchell wrote:
> Subdirectories which have .gitignore files should not be referenced in
> the global .gitignore files.  Move several lines to appropriate subdirs.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> 
> ---
> Hopefully the commit message covers it.  When moved to the subdirectories
> I'm using "./<file>" as otherwise any file sharing the name in a deeper
> subdirectory would be subject to the match.

May I ask why this last sentence isn't part of the commit message?

Jan

Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Elliott Mitchell 3 years, 8 months ago
On Fri, Aug 28, 2020 at 09:24:41AM +0200, Jan Beulich wrote:
> On 28.08.2020 04:57, Elliott Mitchell wrote:
> > Subdirectories which have .gitignore files should not be referenced in
> > the global .gitignore files.  Move several lines to appropriate subdirs.
> > 
> > Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> > 
> > ---
> > Hopefully the commit message covers it.  When moved to the subdirectories
> > I'm using "./<file>" as otherwise any file sharing the name in a deeper
> > subdirectory would be subject to the match.
> 
> May I ask why this last sentence isn't part of the commit message?

My thinking is it was pretty straightforward to figure out when looking.
Not /quite/ obvious enough to avoid commenting in e-mail, but not quite
obscure enough to have in commit message.  This can go either way really.

The .gitignore files aren't very consistent.  I'm unsure whether it is
worth going after the inconsistencies, but it is certainly there.

Before this I noticed xen/xsm/flask/.gitignore had "/policy.c", which
overlapped with "xen/xsm/flask/policy.*" in the top-level .gitignore.
Checking the documentation on .gitignore files if it simply had
"policy.c", git would have ignored any file name "policy.c" in
subdirectories.

Is it better to prefix lines in the current directory with "./" versus
"/"?  (I kind of like "./" since it looks like a relative path, but it
*isn't* actually a relative path)

Should files in subdirectories also include "./"?  Preferences in
sorting?


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445



Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by George Dunlap 3 years, 8 months ago

> On Aug 31, 2020, at 7:37 AM, Elliott Mitchell <ehem+xen@m5p.com> wrote:
> 
> On Fri, Aug 28, 2020 at 09:24:41AM +0200, Jan Beulich wrote:
>> On 28.08.2020 04:57, Elliott Mitchell wrote:
>>> Subdirectories which have .gitignore files should not be referenced in
>>> the global .gitignore files.  Move several lines to appropriate subdirs.
>>> 
>>> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
>>> 
>>> ---
>>> Hopefully the commit message covers it.  When moved to the subdirectories
>>> I'm using "./<file>" as otherwise any file sharing the name in a deeper
>>> subdirectory would be subject to the match.
>> 
>> May I ask why this last sentence isn't part of the commit message?
> 
> My thinking is it was pretty straightforward to figure out when looking.
> Not /quite/ obvious enough to avoid commenting in e-mail, but not quite
> obscure enough to have in commit message.  This can go either way really.

Storing the extra paragraph in git is cheap; trying to reconstruct why someone made a change 10 years after the fact is often difficult.  Probably not worth a re-send — it can be moved into the commit message by the committer; but if you’re going to send v2 anyway, might as well move it in.

 -George

Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Jan Beulich 3 years, 8 months ago
On 31.08.2020 08:37, Elliott Mitchell wrote:
> On Fri, Aug 28, 2020 at 09:24:41AM +0200, Jan Beulich wrote:
>> On 28.08.2020 04:57, Elliott Mitchell wrote:
>>> Subdirectories which have .gitignore files should not be referenced in
>>> the global .gitignore files.  Move several lines to appropriate subdirs.
>>>
>>> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
>>>
>>> ---
>>> Hopefully the commit message covers it.  When moved to the subdirectories
>>> I'm using "./<file>" as otherwise any file sharing the name in a deeper
>>> subdirectory would be subject to the match.
>>
>> May I ask why this last sentence isn't part of the commit message?
> 
> My thinking is it was pretty straightforward to figure out when looking.
> Not /quite/ obvious enough to avoid commenting in e-mail, but not quite
> obscure enough to have in commit message.  This can go either way really.

Your statements below really look to me as if this wasn't this obvious
at all - ...

> The .gitignore files aren't very consistent.  I'm unsure whether it is
> worth going after the inconsistencies, but it is certainly there.
> 
> Before this I noticed xen/xsm/flask/.gitignore had "/policy.c", which
> overlapped with "xen/xsm/flask/policy.*" in the top-level .gitignore.
> Checking the documentation on .gitignore files if it simply had
> "policy.c", git would have ignored any file name "policy.c" in
> subdirectories.
> 
> Is it better to prefix lines in the current directory with "./" versus
> "/"?  (I kind of like "./" since it looks like a relative path, but it
> *isn't* actually a relative path)

... you even look to suggest here that there are two alternative
forms which both have the same meaning. Personally I agree that
./ may be more "natural" to use than /, but the question then is
what the conventions are. I can't answer this.

> Should files in subdirectories also include "./"?

If "no prefix at all" includes, as you say, also files in subdirs,
then the answer probably is "depends".

> Preferences in sorting?

Alphabetical sorting is what we generally aim for here.

Jan

Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Elliott Mitchell 3 years, 8 months ago
On Mon, Aug 31, 2020 at 10:04:54AM +0000, George Dunlap wrote:
> 
> Storing the extra paragraph in git is cheap; trying to reconstruct why someone made a change 10 years after the fact is often difficult.  Probably not worth a re-send ??? it can be moved into the commit message by the committer; but if you???re going to send v2 anyway, might as well move it in.
> 

I'm pretty sure there will be at this point.  Just an issue of how many
more adjustments there will be.


On Mon, Aug 31, 2020 at 08:52:45AM +0200, Jan Beulich wrote:
> On 31.08.2020 08:37, Elliott Mitchell wrote:
> > On Fri, Aug 28, 2020 at 09:24:41AM +0200, Jan Beulich wrote:
> >> On 28.08.2020 04:57, Elliott Mitchell wrote:
> >>> Subdirectories which have .gitignore files should not be referenced in
> >>> the global .gitignore files.  Move several lines to appropriate subdirs.
> >>>
> >>> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> >>>
> >>> ---
> >>> Hopefully the commit message covers it.  When moved to the subdirectories
> >>> I'm using "./<file>" as otherwise any file sharing the name in a deeper
> >>> subdirectory would be subject to the match.
> >>
> >> May I ask why this last sentence isn't part of the commit message?
> > 
> > My thinking is it was pretty straightforward to figure out when looking.
> > Not /quite/ obvious enough to avoid commenting in e-mail, but not quite
> > obscure enough to have in commit message.  This can go either way really.
> 
> Your statements below really look to me as if this wasn't this obvious
> at all - ...

Things were sufficiently obvious when it was important.  This though is
not an issue worthy of more discussion since I've got no real objection
to including the extra sentence.  I suspect there will be more changes
though...


> > The .gitignore files aren't very consistent.  I'm unsure whether it is
> > worth going after the inconsistencies, but it is certainly there.
> > 
> > Before this I noticed xen/xsm/flask/.gitignore had "/policy.c", which
> > overlapped with "xen/xsm/flask/policy.*" in the top-level .gitignore.
> > Checking the documentation on .gitignore files if it simply had
> > "policy.c", git would have ignored any file name "policy.c" in
> > subdirectories.
> > 
> > Is it better to prefix lines in the current directory with "./" versus
> > "/"?  (I kind of like "./" since it looks like a relative path, but it
> > *isn't* actually a relative path)
> 
> ... you even look to suggest here that there are two alternative
> forms which both have the same meaning. Personally I agree that
> ./ may be more "natural" to use than /, but the question then is
> what the conventions are. I can't answer this.
> 
> > Should files in subdirectories also include "./"?
> 
> If "no prefix at all" includes, as you say, also files in subdirs,
> then the answer probably is "depends".
> 
> > Preferences in sorting?
> 
> Alphabetical sorting is what we generally aim for here.

Going into specific example since those best demonstrate what I
observed.

Before this patch the top-level .gitignore included the lines:
@@
-tools/misc/cpuperf/cpuperf-perfcntr
-tools/misc/cpuperf/cpuperf-xen
-tools/misc/xc_shadow
-tools/misc/xen_cpuperf
-tools/misc/xen-cpuid
@@
-xen/xsm/flask/policy.*
-xen/xsm/flask/xenpolicy-*
 tools/flask/policy/policy.conf
 tools/flask/policy/xenpolicy-*
 xen/xen
@@

tools/misc/.gitignore had the single line:
xen-ucode

xen/xsm/flask/.gitignore had the single line:
/policy.c


You'll note from the second batch, .gitignore isn't consistently sorted.

xen/xsm/flask/.gitignore was actually good since it caused me to look at
the documentation for gitignore.  The effect is xen/xsm/flask/policy.c
is ignored, but xen/xsm/flask/policy/policy.c and
xen/xsm/flask/ss/policy.c will *not* be ignored.

tools/misc/.gitignore's format means if in the future subdirectories "a"
or "b" got created, tools/misc/a/xen-ucode and tools/misc/b/xen-ucode
*would* be ignored in addition to tools/misc/xen-ucode.  When looking at
the situation I decided this was /likely/ wrong, and so changed to
"./xen-ucode".

So there are a few variants of how tools/misc/.gitignore could look
after a patch.  Two sets of lines demonstrating the possibilities:

Example 0:
./cpuperf/cpuperf-perfcntr
./cpuperf/cpuperf-xen
./lowmemd
./xc_shadow

Example 1:
cpuperf/cpuperf-perfcntr
cpuperf/cpuperf-xen
/lowmemd
/xc_shadow

So which prefix is better for files in the current directory, "./" or
"/"?  "./" looks more like a reference to the current directory, "/" is
shorter and the single pre-existing example used the latter.

Should the paths "cpuperf/cpuperf-perfcntr" and "cpuperf/cpuperf-xen"
include that prefix?  I'm inclined towards having it everywhere for
greater consistency, but I'm not fully set in this strategy.


In other news, I think the tools/ and xen/ directories need their own
.gitignore files.  They are the largest portion of targeted filenames
and thus seem likely candidates for breaking off.

docs/ and stubdom/ are also candidates for similar action, though not as
big as the former.

This falls under the same heading of moving things from top-level
.gitignore to subdirectories, but since the above didn't already have
.gitignore files this could be worthy of a separate patch.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445



Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Jan Beulich 3 years, 8 months ago
On 01.09.2020 00:55, Elliott Mitchell wrote:
> On Mon, Aug 31, 2020 at 08:52:45AM +0200, Jan Beulich wrote:
>> On 31.08.2020 08:37, Elliott Mitchell wrote:
>>> Preferences in sorting?
>>
>> Alphabetical sorting is what we generally aim for here.
> 
> Going into specific example since those best demonstrate what I
> observed.
> 
> Before this patch the top-level .gitignore included the lines:
> @@
> -tools/misc/cpuperf/cpuperf-perfcntr
> -tools/misc/cpuperf/cpuperf-xen
> -tools/misc/xc_shadow
> -tools/misc/xen_cpuperf
> -tools/misc/xen-cpuid
> @@
> -xen/xsm/flask/policy.*
> -xen/xsm/flask/xenpolicy-*
>  tools/flask/policy/policy.conf
>  tools/flask/policy/xenpolicy-*
>  xen/xen
> @@
> 
> tools/misc/.gitignore had the single line:
> xen-ucode
> 
> xen/xsm/flask/.gitignore had the single line:
> /policy.c
> 
> 
> You'll note from the second batch, .gitignore isn't consistently sorted.

I'm aware, and hence I said "aim for". In cases like this what we
often do is adjust things incrementally, as lines get touched anyway.
Of course if you want to clean it up all in one go ...

Jan

Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Elliott Mitchell 3 years, 8 months ago
On Tue, Sep 01, 2020 at 08:01:30AM +0200, Jan Beulich wrote:
> I'm aware, and hence I said "aim for". In cases like this what we
> often do is adjust things incrementally, as lines get touched anyway.
> Of course if you want to clean it up all in one go ...

What I've got has turned into a patch series.  There are some general
.gitignore cleanup patches, followed by large mechanical fixes.

Who should be included as Cc for submitting these?  Usual pattern would
end up including all the general maintainers on all patches.  The reason
is several of these are taking pieces off of the top-level .gitignore and
moving those to subdirectory .gitignore files which would have shorter Cc
lists.  There wouldn't be actual effects at the top-level, merely those
subdirectories.  Should only the maintainers for the subdirectories be
Cc'd?

These also have limited testing.  The testing which is possible is to
simply run `git check-ignore -vn <filename>` and I'm lacking appropriate
testers.  I'm hopeful I'll get it right, but there is always that sweaty
palms moment worrying I severely goof'd...


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445



Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Jan Beulich 3 years, 8 months ago
On 05.09.2020 07:17, Elliott Mitchell wrote:
> On Tue, Sep 01, 2020 at 08:01:30AM +0200, Jan Beulich wrote:
>> I'm aware, and hence I said "aim for". In cases like this what we
>> often do is adjust things incrementally, as lines get touched anyway.
>> Of course if you want to clean it up all in one go ...
> 
> What I've got has turned into a patch series.  There are some general
> .gitignore cleanup patches, followed by large mechanical fixes.
> 
> Who should be included as Cc for submitting these?  Usual pattern would
> end up including all the general maintainers on all patches.  The reason
> is several of these are taking pieces off of the top-level .gitignore and
> moving those to subdirectory .gitignore files which would have shorter Cc
> lists.  There wouldn't be actual effects at the top-level, merely those
> subdirectories.  Should only the maintainers for the subdirectories be
> Cc'd?

I don't have a good suggestion or strong opinion either way here.
I can see reasons for going either route.

Jan

Re: [PATCH] gitignore: Move ignores from global to subdirectories
Posted by Elliott Mitchell 3 years, 8 months ago
On Tue, Sep 01, 2020 at 08:01:30AM +0200, Jan Beulich wrote:
> On 01.09.2020 00:55, Elliott Mitchell wrote:
> > On Mon, Aug 31, 2020 at 08:52:45AM +0200, Jan Beulich wrote:
> >> On 31.08.2020 08:37, Elliott Mitchell wrote:
> >>> Preferences in sorting?
> >>
> >> Alphabetical sorting is what we generally aim for here.
> > 
> > Going into specific example since those best demonstrate what I
> > observed.
> > 
> > Before this patch the top-level .gitignore included the lines:
> > @@
> > -tools/misc/cpuperf/cpuperf-perfcntr
> > -tools/misc/cpuperf/cpuperf-xen
> > -tools/misc/xc_shadow
> > -tools/misc/xen_cpuperf
> > -tools/misc/xen-cpuid
> > @@
> > -xen/xsm/flask/policy.*
> > -xen/xsm/flask/xenpolicy-*
> >  tools/flask/policy/policy.conf
> >  tools/flask/policy/xenpolicy-*
> >  xen/xen
> > @@
> > 
> > tools/misc/.gitignore had the single line:
> > xen-ucode
> > 
> > xen/xsm/flask/.gitignore had the single line:
> > /policy.c
> > 
> > 
> > You'll note from the second batch, .gitignore isn't consistently sorted.
> 
> I'm aware, and hence I said "aim for". In cases like this what we
> often do is adjust things incrementally, as lines get touched anyway.
> Of course if you want to clean it up all in one go ...

I may, though right now I'm having the experience of reading
documentation several times to double-check and discovering I
misinterpreted when testing.  There is also the difficulty of trying to
figure out why some things were done the way they were.

Really starts to look like everyone (including myself) tries to intuit
how .gitignore files work and doesn't /quite/ get it right 9 times out
of 10.

*Definitely* going to need that v2...


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445