| Message ID | 20200828025747.GA25246@mattapan.m5p.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gitignore: Move ignores from global to subdirectories | expand |
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
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?
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
> 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
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.
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
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...
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...
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
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-*
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(-)