* [PATCH] bisect.c: remove unused includes @ 2022-03-31 19:44 Garrit Franke 2022-03-31 21:29 ` Junio C Hamano ` (5 more replies) 0 siblings, 6 replies; 12+ messages in thread From: Garrit Franke @ 2022-03-31 19:44 UTC (permalink / raw) To: git; +Cc: Garrit Franke Clean up includes no longer needed by bisect.c. Signed-off-by: Garrit Franke <garrit@slashdev.space> --- bisect.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/bisect.c b/bisect.c index 9e6a2b7f20..e07e2d215d 100644 --- a/bisect.c +++ b/bisect.c @@ -1,21 +1,12 @@ -#include "cache.h" #include "config.h" -#include "commit.h" -#include "diff.h" -#include "revision.h" #include "refs.h" #include "list-objects.h" #include "quote.h" -#include "hash-lookup.h" #include "run-command.h" #include "log-tree.h" #include "bisect.h" -#include "oid-array.h" -#include "strvec.h" -#include "commit-slab.h" #include "commit-reach.h" #include "object-store.h" -#include "dir.h" static struct oid_array good_revs; static struct oid_array skipped_revs; -- 2.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] bisect.c: remove unused includes 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke @ 2022-03-31 21:29 ` Junio C Hamano 2022-04-01 8:07 ` using iwyu (include-what-you-use) to analyze includes (was: [PATCH] bisect.c: remove unused includes) Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes Garrit Franke ` (4 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: Junio C Hamano @ 2022-03-31 21:29 UTC (permalink / raw) To: Garrit Franke; +Cc: git Garrit Franke <garrit@slashdev.space> writes: > Clean up includes no longer needed by bisect.c. > > Signed-off-by: Garrit Franke <garrit@slashdev.space> > --- > bisect.c | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/bisect.c b/bisect.c > index 9e6a2b7f20..e07e2d215d 100644 > --- a/bisect.c > +++ b/bisect.c > @@ -1,21 +1,12 @@ > -#include "cache.h" cf. Documentation/CodingGuidelines The first #include must be <git-compat-util.h>, or <cache.h> or <builtin.h>, which are well known to include <git-compat-util.h> first. Including <git-compat-util.h> indirectly by <config.h> -> <hashmap.h> -> <hash.h> -> <git-compat-util.h> does not count. > #include "config.h" > -#include "commit.h" Other headers may indirectly include <commit.h> as their implementation detail, but what matters is that *we* in this source file use what <commit.h> gives us ourselves, like the concrete shape of "struct commit_list". This change is not wanted. I'll stop here. There may be truly leftover "unused" includes among those removed by the remainder of this patch, but I suspect that some are like <commit.h> above, i.e. we directly use it, and because we do not want to be broken by some header file's implementation detail changing, we MUST include it ourselves. I think this should give us a useful guideline to sift through the rest, and an updated patch to remove truly unused ones are very much welcome. We may actually find some we are not directly including ourselves but we should (e.g. I do not see <string-list.h> included by us, but we clearly use structures and functions declared there, and probably is depending, wrongly, on some header file we include happens to indirectly include it). Thanks. > -#include "diff.h" > -#include "revision.h" > #include "refs.h" > #include "list-objects.h" > #include "quote.h" > -#include "hash-lookup.h" > #include "run-command.h" > #include "log-tree.h" > #include "bisect.h" > -#include "oid-array.h" > -#include "strvec.h" > -#include "commit-slab.h" > #include "commit-reach.h" > #include "object-store.h" > -#include "dir.h" > > static struct oid_array good_revs; > static struct oid_array skipped_revs; ^ permalink raw reply [flat|nested] 12+ messages in thread
* using iwyu (include-what-you-use) to analyze includes (was: [PATCH] bisect.c: remove unused includes) 2022-03-31 21:29 ` Junio C Hamano @ 2022-04-01 8:07 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 12+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-04-01 8:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: Garrit Franke, git On Thu, Mar 31 2022, Junio C Hamano wrote: [Changed $subject to make this easier to find] > Garrit Franke <garrit@slashdev.space> writes: > >> Clean up includes no longer needed by bisect.c. >> >> Signed-off-by: Garrit Franke <garrit@slashdev.space> >> --- >> bisect.c | 9 --------- >> 1 file changed, 9 deletions(-) >> >> diff --git a/bisect.c b/bisect.c >> index 9e6a2b7f20..e07e2d215d 100644 >> --- a/bisect.c >> +++ b/bisect.c >> @@ -1,21 +1,12 @@ >> -#include "cache.h" > > cf. Documentation/CodingGuidelines > > The first #include must be <git-compat-util.h>, or <cache.h> or > <builtin.h>, which are well known to include <git-compat-util.h> > first. > > Including <git-compat-util.h> indirectly by <config.h> -> > <hashmap.h> -> <hash.h> -> <git-compat-util.h> does not count. Also: Some built-ins don't include builtin.h as they should, a fix (or even basic CI check) for that would be most welcome. git grep -C2 -n -F -e builtin.h -e cache.h -e git-compat-util.h -- builtin I.e. we have this saying a lot of those are redundant: Documentation/CodingGuidelines- - The first #include in C files, except in platform specific compat/ Documentation/CodingGuidelines- implementations, must be either "git-compat-util.h", "cache.h" or Documentation/CodingGuidelines: "builtin.h". You do not have to include more than one of these. But maybe it's not worth it, anyway... >> #include "config.h" >> -#include "commit.h" > > Other headers may indirectly include <commit.h> as their > implementation detail, but what matters is that *we* in this source > file use what <commit.h> gives us ourselves, like the concrete shape > of "struct commit_list". This change is not wanted. > > I'll stop here. There may be truly leftover "unused" includes among > those removed by the remainder of this patch, but I suspect that > some are like <commit.h> above, i.e. we directly use it, and because > we do not want to be broken by some header file's implementation > detail changing, we MUST include it ourselves. > > I think this should give us a useful guideline to sift through the > rest, and an updated patch to remove truly unused ones are very much > welcome. We may actually find some we are not directly including > ourselves but we should (e.g. I do not see <string-list.h> included > by us, but we clearly use structures and functions declared there, > and probably is depending, wrongly, on some header file we include > happens to indirectly include it). ... For anyone interested in pursuing this, I think using the excellent include-what-you-use tool would be a nice start. We could even eventually add it to our CI if the false positive rate isn't bad (I haven't checked much): https://github.com/include-what-you-use/include-what-you-use E.g. in this case (I manually omitted the rest of the output, there's probably a iwyu option to omit it, but I didn't see how do that from skimming the docs): $ sudo apt install iwyu # YMMV $ make bisect.o CC=include-what-you-use CFLAGS="-Xiwyu --verbose=1" 2>&1 | grep -v -E -e '^#include <' -e '^#include "(cache|git-compat-util|gettext)\.h"' CC bisect.o (bisect.h has correct #includes/fwd-decls) bisect.c should add these lines: #include "hash.h" // for oideq, object_id, oidcmp, oidcpy, GIT_... #include "object.h" // for object, repo_clear_commit_marks #include "path.h" // for GIT_PATH_FUNC, git_pathdup #include "pretty.h" // for CMIT_FMT_UNSPECIFIED, format_commit_me... #include "repository.h" // for repository (ptr only), the_repository #include "strbuf.h" // for strbuf_release, strbuf, strbuf_getline_lf #include "string-list.h" // for string_list_append, string_list_clear bisect.c should remove these lines: - #include "hash-lookup.h" // lines 9-9 - struct commit_weight; // lines 76-76 Then if I patch it as: diff --git a/bisect.c b/bisect.c index 9e6a2b7f201..512430e3cc8 100644 --- a/bisect.c +++ b/bisect.c @@ -6,7 +6,6 @@ #include "refs.h" #include "list-objects.h" #include "quote.h" -#include "hash-lookup.h" #include "run-command.h" #include "log-tree.h" #include "bisect.h" @@ -16,6 +15,13 @@ #include "commit-reach.h" #include "object-store.h" #include "dir.h" +#include "hash.h" +#include "object.h" +#include "path.h" +#include "pretty.h" +#include "repository.h" +#include "strbuf.h" +#include "string-list.h" static struct oid_array good_revs; static struct oid_array skipped_revs; It's happier, but probably needs to be told to ignore define_commit_slab() somehow: $ make bisect.o CC=include-what-you-use CFLAGS="-Xiwyu --verbose=1" 2>&1 | grep -v -E -e '^#include <' -e '^#include "(cache|git-compat-util|gettext)\.h"' CC bisect.o (bisect.h has correct #includes/fwd-decls) bisect.c should add these lines: bisect.c should remove these lines: - struct commit_weight; // lines 82-82 That still needs to be massaged a bit, e.g. we should probably omit hash.h and anything else in cache.h and git-compat-util.h. Or maybe not & we should make those headers even lighter. It is rather annoying that changing some of those things leads to a complete re-build, but there's a trade-off there where we probably want things like gettext.h and other used-almost-everywhere headers in included by those. So take all the above with a huge grain of salt. I haven't used iwyu much, but it seems to be something that'll help us go in the direction Junio noted above. I think starting with: make -k git-objs <the CC etc. params above> And tackling the "should remove these lines" issues first would be a good start, e.g. for serve.c it says: serve.c should remove these lines: - #include "cache.h" // lines 1-1 - #include "strvec.h" // lines 6-6 We don't want that first one, but it's right about the second one. It's been orphaned since f0a35c9ce52 (serve: drop "keys" strvec, 2021-09-15), I skimmed some of the rist and they all seem like good suggestions. E.g. lockfile.h for builtin/apply.c, which isn't needed since 6d058c88264 (apply: move lockfile into `apply_state`, 2017-10-05). ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke 2022-03-31 21:29 ` Junio C Hamano @ 2022-04-05 11:45 ` Garrit Franke 2022-04-06 7:54 ` Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 1/4] contrib: add iwyu script Garrit Franke ` (3 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: Garrit Franke @ 2022-04-05 11:45 UTC (permalink / raw) To: avarab; +Cc: garrit, git, gitster [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=ANSI_X3.4-1968, Size: 1764 bytes --] I'm sorry about the half-baked patch of mine. I initially noticed that the "hash-lookup.h" wasn't needed (which was a correct assumption), and then went overboard when trying to clean up the other includes. This is truly a rookie mistake that only wastes the time of everyone involved. This won't happen again! On 01.04.22 10:07, Ævar Arnfjörð Bjarmason wrote: > ... For anyone interested in pursuing this, I think using the excellent > include-what-you-use tool would be a nice start. > > We could even eventually add it to our CI if the false positive rate > isn't bad (I haven't checked much): > https://github.com/include-what-you-use/include-what-you-use This seems to be a really nice tool indeed. I wouldn't be comfortable adding it to the CI just yet, but it did make it considerably easier to spot includes that could safely be removed. I think we could try battle-testing this tool in the codebase to get a sense of how it behaves. To start, I added your reference-command to a script under "contrib/iwyu" and ran it against the files you noted. Before breaking a bulk of the files, I wanted to make sure that this undertaking is headed in the right direction. Feedback is of course welcomed! Garrit Franke (4): contrib: add iwyu script bisect.c: remove unnecessary include serve.c: remove unnecessary include apply.c: remove unnecessary include bisect.c | 1 - builtin/apply.c | 1 - contrib/iwyu/README | 33 +++++++++++++++++++++++++++++++++ contrib/iwyu/iwyu.sh | 2 ++ serve.c | 1 - 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 contrib/iwyu/README create mode 100755 contrib/iwyu/iwyu.sh -- 2.35.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes 2022-04-05 11:45 ` [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes Garrit Franke @ 2022-04-06 7:54 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 12+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-04-06 7:54 UTC (permalink / raw) To: Garrit Franke; +Cc: git, gitster On Tue, Apr 05 2022, Garrit Franke wrote: > On 01.04.22 10:07, Ævar Arnfjörð Bjarmason wrote: Aside: I don't think I've ever seen encoded quoted-printable go quite so bad so fast. That went from =C3=86var to =C3=83=C6=92=C3=A2=E2=82=AC in one reply. Whatever your E-Mail is doing with encodings seems to be taking multiple passes through misencodings :) Don't worry about getting the name "right" or whatever, I'm amused by the encoding issue... >> ... For anyone interested in pursuing this, I think using the excellent >> include-what-you-use tool would be a nice start. >> >> We could even eventually add it to our CI if the false positive rate >> isn't bad (I haven't checked much): >> https://github.com/include-what-you-use/include-what-you-use > > This seems to be a really nice tool indeed. I wouldn't be comfortable > adding it to the CI just yet, but it did make it considerably easier to > spot includes that could safely be removed. Re the reply I had on 1/4 I think it's probably best to drop that in its current form, but the fixes themselves (perhaps with a re-roll for nits I posted in reply) seem good. I was really hoping though that if someone wanted to pursue this a bit more we'd get to the point of being able to run "make all test" on a source tree that iwyu would munge with all its suggestions, and then see if it outright failed to compile, or whether it would e.g. have faster compilation speed (or not..). > I think we could try battle-testing this tool in the codebase to get a > sense of how it behaves. To start, I added your reference-command to a > script under "contrib/iwyu" and ran it against the files you noted. > Before breaking a bulk of the files, I wanted to make sure that this > undertaking is headed in the right direction. Even if the patches aren't sent in making the actual changes a one-off script to e.g. wrap the fix_includes.py script I mentioned would be very interesting. We could then even run that in CI with relatively little setup, i.e. checkout <rev>, do munging, then compile. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/4] contrib: add iwyu script 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke 2022-03-31 21:29 ` Junio C Hamano 2022-04-05 11:45 ` [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes Garrit Franke @ 2022-04-05 11:45 ` Garrit Franke 2022-04-06 7:40 ` Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 2/4] bisect.c: remove unnecessary include Garrit Franke ` (2 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: Garrit Franke @ 2022-04-05 11:45 UTC (permalink / raw) To: avarab; +Cc: garrit, git, gitster add include-what-you-use helper. Signed-off-by: Garrit Franke <garrit@slashdev.space> --- contrib/iwyu/README | 33 +++++++++++++++++++++++++++++++++ contrib/iwyu/iwyu.sh | 2 ++ 2 files changed, 35 insertions(+) create mode 100644 contrib/iwyu/README create mode 100755 contrib/iwyu/iwyu.sh diff --git a/contrib/iwyu/README b/contrib/iwyu/README new file mode 100644 index 0000000000..5e2d218602 --- /dev/null +++ b/contrib/iwyu/README @@ -0,0 +1,33 @@ +Include What You Use +==================== + +Include what you use (iwyu) [1] is a tool that points out which headers a file +should include. Moreover, it can point out includes that are not used by a file, +which makes it especially handy for cleanup tasks. + +To run this script, you will need iwyu to be installed on your system. + +The "iwyu.sh" script runs iwyu on a given object and omits mandatory headers +defined in "Documentation/CodingGuidelines". + +Example usage: + + ./contrib/iwyu/iwyu.sh diff.o + +This yields: + + diff.c should remove these lines: + - #include "attr.h" // lines 13-13 + - #include "submodule-config.h" // lines 18-18 + +In its current form, this script should not be used to auto-generate patches, +since there are still some false-positives that only a human can resolve. It is +meant as a starting point for further cleanups. It could be nice to integrate +this as a step in our CI, but we're not quite there yet. + +The inspiration for this script came from this [2] email-thread. + +Garrit Franke <garrit@slashdev.space> + +[1]: https://github.com/include-what-you-use/include-what-you-use +[2]: https://lore.kernel.org/all/220401.8635ixp3f4.gmgdl@evledraar.gmail.com/#t \ No newline at end of file diff --git a/contrib/iwyu/iwyu.sh b/contrib/iwyu/iwyu.sh new file mode 100755 index 0000000000..3ef8639eae --- /dev/null +++ b/contrib/iwyu/iwyu.sh @@ -0,0 +1,2 @@ +make $1 CC=include-what-you-use CFLAGS="-Xiwyu --verbose=1" 2>&1 \ +| grep -v -E -e '^#include <' -e '^#include "(cache|git-compat-util|gettext)\.h"' \ No newline at end of file -- 2.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/4] contrib: add iwyu script 2022-04-05 11:45 ` [PATCH v2 1/4] contrib: add iwyu script Garrit Franke @ 2022-04-06 7:40 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 12+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-04-06 7:40 UTC (permalink / raw) To: Garrit Franke; +Cc: git, gitster On Tue, Apr 05 2022, Garrit Franke wrote: > add include-what-you-use helper. > > Signed-off-by: Garrit Franke <garrit@slashdev.space> > --- > contrib/iwyu/README | 33 +++++++++++++++++++++++++++++++++ > contrib/iwyu/iwyu.sh | 2 ++ > 2 files changed, 35 insertions(+) > create mode 100644 contrib/iwyu/README > create mode 100755 contrib/iwyu/iwyu.sh > > diff --git a/contrib/iwyu/README b/contrib/iwyu/README > new file mode 100644 > index 0000000000..5e2d218602 > --- /dev/null > +++ b/contrib/iwyu/README > @@ -0,0 +1,33 @@ > +Include What You Use > +==================== > + > +Include what you use (iwyu) [1] is a tool that points out which headers a file > +should include. Moreover, it can point out includes that are not used by a file, > +which makes it especially handy for cleanup tasks. > + > +To run this script, you will need iwyu to be installed on your system. > + > +The "iwyu.sh" script runs iwyu on a given object and omits mandatory headers > +defined in "Documentation/CodingGuidelines". > + > +Example usage: > + > + ./contrib/iwyu/iwyu.sh diff.o > + > +This yields: > + > + diff.c should remove these lines: > + - #include "attr.h" // lines 13-13 > + - #include "submodule-config.h" // lines 18-18 > + > +In its current form, this script should not be used to auto-generate patches, > +since there are still some false-positives that only a human can resolve. It is > +meant as a starting point for further cleanups. It could be nice to integrate > +this as a step in our CI, but we're not quite there yet. > + > +The inspiration for this script came from this [2] email-thread. > + > +Garrit Franke <garrit@slashdev.space> > + > +[1]: https://github.com/include-what-you-use/include-what-you-use > +[2]: https://lore.kernel.org/all/220401.8635ixp3f4.gmgdl@evledraar.gmail.com/#t > \ No newline at end of file Note the "No newline at end of file" warnings, new files should have \n at the end. > diff --git a/contrib/iwyu/iwyu.sh b/contrib/iwyu/iwyu.sh > new file mode 100755 > index 0000000000..3ef8639eae > --- /dev/null > +++ b/contrib/iwyu/iwyu.sh > @@ -0,0 +1,2 @@ > +make $1 CC=include-what-you-use CFLAGS="-Xiwyu --verbose=1" 2>&1 \ > +| grep -v -E -e '^#include <' -e '^#include "(cache|git-compat-util|gettext)\.h"' > \ No newline at end of file So that's just my one-line hack as-is (well, bisect.co replaced with $1) :) I think if we integrate some IWYU spport support that this is a rather dead end to pursue. A much better way would be to just do ("REAL_CC" is already understood by sparse): REAL_CC=gcc make CC=contrib/iwyu.sh I.e. we shouldn't run make and then parse its output, but to have make run a CC which wraps our REAL_CC. Then you could either have it "really" compile, as e.g. the "sparse" wrapper can do (note: note our sparse target but cgcc), or run whatever "make" target, and then only having to parse the output of iwyu, not iwyu+make. Also, it looks like iwyu is a nicely API'd python library, so trying to parse its output is probably a dead end too, can't we just ship a trivial API-using script for it that gets all this as nicely machine-readable data? Or, if you look at iwyu.git its own source tree has a fix_includes.py which seems to do that, *and* be able to "fix" the includes for you. All of that is just stuff I didn't have time to poke at when posting a quick reply in the original thread, but hopefully someone turning this into a series of patches will :) $ python3 fix_includes.py -h Usage: fix_includes.py [options] [filename] ... < <output from include-what-you-use script> OR fix_includes.py -s [other options] <filename> ... fix_includes.py reads the output from the include-what-you-use script on stdin -- run with --v=1 (default) verbose or above -- and, unless --sort_only or --dry_run is specified, modifies the files mentioned in the output, removing their old #include lines and replacing them with the lines given by the include_what_you_use script. It also sorts the #include and forward-declare lines. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] bisect.c: remove unnecessary include 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke ` (2 preceding siblings ...) 2022-04-05 11:45 ` [PATCH v2 1/4] contrib: add iwyu script Garrit Franke @ 2022-04-05 11:45 ` Garrit Franke 2022-04-06 7:50 ` Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 3/4] serve.c: " Garrit Franke 2022-04-05 11:45 ` [PATCH v2 4/4] apply.c: " Garrit Franke 5 siblings, 1 reply; 12+ messages in thread From: Garrit Franke @ 2022-04-05 11:45 UTC (permalink / raw) To: avarab; +Cc: garrit, git, gitster Remove include "hash-lookup.h". Signed-off-by: Garrit Franke <garrit@slashdev.space> --- bisect.c | 1 - 1 file changed, 1 deletion(-) diff --git a/bisect.c b/bisect.c index 9e6a2b7f20..1aced76257 100644 --- a/bisect.c +++ b/bisect.c @@ -6,7 +6,6 @@ #include "refs.h" #include "list-objects.h" #include "quote.h" -#include "hash-lookup.h" #include "run-command.h" #include "log-tree.h" #include "bisect.h" -- 2.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] bisect.c: remove unnecessary include 2022-04-05 11:45 ` [PATCH v2 2/4] bisect.c: remove unnecessary include Garrit Franke @ 2022-04-06 7:50 ` Ævar Arnfjörð Bjarmason 2022-04-06 16:41 ` Junio C Hamano 0 siblings, 1 reply; 12+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2022-04-06 7:50 UTC (permalink / raw) To: Garrit Franke; +Cc: git, gitster On Tue, Apr 05 2022, Garrit Franke wrote: > Remove include "hash-lookup.h". Like your 3/4 it would be nice to have a "orphaned since xyz", or was it never used etc? In these cases unlike most such C changes the compiler isn't of much use for validation (we might be including this implicitly), so an explanation saying why is helpful. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/4] bisect.c: remove unnecessary include 2022-04-06 7:50 ` Ævar Arnfjörð Bjarmason @ 2022-04-06 16:41 ` Junio C Hamano 0 siblings, 0 replies; 12+ messages in thread From: Junio C Hamano @ 2022-04-06 16:41 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Garrit Franke, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Tue, Apr 05 2022, Garrit Franke wrote: > >> Remove include "hash-lookup.h". > > Like your 3/4 it would be nice to have a "orphaned since xyz", or was it > never used etc? > > In these cases unlike most such C changes the compiler isn't of much use > for validation (we might be including this implicitly), so an > explanation saying why is helpful. Thanks for pointing it out. I found the lack of explanation in some but not all of the changes disturbing. ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/4] serve.c: remove unnecessary include 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke ` (3 preceding siblings ...) 2022-04-05 11:45 ` [PATCH v2 2/4] bisect.c: remove unnecessary include Garrit Franke @ 2022-04-05 11:45 ` Garrit Franke 2022-04-05 11:45 ` [PATCH v2 4/4] apply.c: " Garrit Franke 5 siblings, 0 replies; 12+ messages in thread From: Garrit Franke @ 2022-04-05 11:45 UTC (permalink / raw) To: avarab; +Cc: garrit, git, gitster Remove include "strvec.h" from serve.c, which is orphaned since f0a35c9ce52 (serve: drop "keys" strvec, 2021-09-15) Signed-off-by: Garrit Franke <garrit@slashdev.space> --- serve.c | 1 - 1 file changed, 1 deletion(-) diff --git a/serve.c b/serve.c index b3fe9b5126..733347f602 100644 --- a/serve.c +++ b/serve.c @@ -3,7 +3,6 @@ #include "config.h" #include "pkt-line.h" #include "version.h" -#include "strvec.h" #include "ls-refs.h" #include "protocol-caps.h" #include "serve.h" -- 2.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/4] apply.c: remove unnecessary include 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke ` (4 preceding siblings ...) 2022-04-05 11:45 ` [PATCH v2 3/4] serve.c: " Garrit Franke @ 2022-04-05 11:45 ` Garrit Franke 5 siblings, 0 replies; 12+ messages in thread From: Garrit Franke @ 2022-04-05 11:45 UTC (permalink / raw) To: avarab; +Cc: garrit, git, gitster Remove include "lockfile.h" from builtin/apply.c, which is orphaned since 6d058c88264 (apply: move lockfile into `apply_state`, 2017-10-05) Signed-off-by: Garrit Franke <garrit@slashdev.space> --- builtin/apply.c | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/apply.c b/builtin/apply.c index 3f099b9605..555219de40 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -1,7 +1,6 @@ #include "cache.h" #include "builtin.h" #include "parse-options.h" -#include "lockfile.h" #include "apply.h" static const char * const apply_usage[] = { -- 2.35.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-04-06 18:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-31 19:44 [PATCH] bisect.c: remove unused includes Garrit Franke 2022-03-31 21:29 ` Junio C Hamano 2022-04-01 8:07 ` using iwyu (include-what-you-use) to analyze includes (was: [PATCH] bisect.c: remove unused includes) Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 0/4] various: use iwyu (include-what-you-use) to analyze includes Garrit Franke 2022-04-06 7:54 ` Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 1/4] contrib: add iwyu script Garrit Franke 2022-04-06 7:40 ` Ævar Arnfjörð Bjarmason 2022-04-05 11:45 ` [PATCH v2 2/4] bisect.c: remove unnecessary include Garrit Franke 2022-04-06 7:50 ` Ævar Arnfjörð Bjarmason 2022-04-06 16:41 ` Junio C Hamano 2022-04-05 11:45 ` [PATCH v2 3/4] serve.c: " Garrit Franke 2022-04-05 11:45 ` [PATCH v2 4/4] apply.c: " Garrit Franke
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).