| Message ID | 1476957133-24382-1-git-send-email-anand.indukala@gmail.com |
|---|---|
| State | New |
| Headers | show |
[meta-comment] On 10/20/2016 04:52 AM, Anand J wrote: > Added script to check duplicate #include entries. This check will scan > and print the files in which duplicate #include entries are present. This appears to be a v2 patch. But it is missing a 0/2 cover letter, and mention of v2 in the subject line. 'git format-patch --cover-letter -v2' will do what you want (as well as git send-email, which understands all options accepted by format-patch); it's also possible to automate git to send a cover letter any time you send more than one patch by using 'git config format.coverLetter auto'. More patch submission tips at http://wiki.qemu.org/Contribute/SubmitAPatch > > Script might output false positive entries as well. Such entries should > not be removed. So if it finds any duplicate entries script will > terminate with an exit status 1. Then each and every file should be > checked manually and corrected if necessary. > > In order to enable the check use --check-duphead option with > script/clean-includes. > > Reviewed-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Anand J <anand.indukala@gmail.com> > --- > scripts/clean-includes | 50 +++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 37 insertions(+), 13 deletions(-) >
On 10/20/2016 04:52 AM, Anand J wrote: > Added script to check duplicate #include entries. This check will scan > and print the files in which duplicate #include entries are present. Subject line typo: s,script/,scripts/, > > Script might output false positive entries as well. Such entries should > not be removed. So if it finds any duplicate entries script will > terminate with an exit status 1. Then each and every file should be > checked manually and corrected if necessary. > > In order to enable the check use --check-duphead option with > script/clean-includes. Bike-shedding, but if you are going to put a dash between words, put it between ALL words, as in --check-dup-head. Or spell it out: --check-duplicate-headers. If that feels to long, and you want to abbreviate, then abbreviate further, as in --duphead. > > Reviewed-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Anand J <anand.indukala@gmail.com> > --- > # > +# If --check-duphead option is used, then check for duplicate > +# header files will be enabled. Grammar; I suggest: If --check-duphead is specified, additionally check for duplicate header includes. > +while true > +do > + case $1 in > + "--git") > + if [ $# -eq 1 ]; then > + echo "--git option requires an argument" > + exit 1 > + fi > + GITSUBJ="$2" > + GIT=yes > + shift > + shift > + ;; > + "--check-duphead") > + DUPHEAD=yes > + shift > + ;; > + *) Your option-parsing loop is missing a special case for --; for consistency with POSIX recommendations, you should have: --) shift break ;; *) as a way to end option parsing early (and force the next argument to be treated as a filename, even if it starts with -). > > +if [ "$DUPHEAD" = "yes" ]; then > + grep "^#include" $@ | sort | uniq -c | awk '{if ($1 > 1) print $0}' Missing quotes around $@. Looking forward to v3.
diff --git a/scripts/clean-includes b/scripts/clean-includes index 4412a55..76dd0e4 100755 --- a/scripts/clean-includes +++ b/scripts/clean-includes @@ -14,15 +14,18 @@ # the top-level directory. # Usage: -# clean-includes [--git subjectprefix] file ... +# clean-includes [--git subjectprefix] [--check-duphead] file ... # or -# clean-includes [--git subjectprefix] --all +# clean-includes [--git subjectprefix] [--check-duphead] --all # # If the --git subjectprefix option is given, then after making # the changes to the files this script will create a git commit # with the subject line "subjectprefix: Clean up includes" # and a boilerplate commit message. # +# If --check-duphead option is used, then check for duplicate +# header files will be enabled. +# # Using --all will cause clean-includes to run on the whole source # tree (excluding certain directories which are known not to need # handling). @@ -45,23 +48,36 @@ GIT=no +DUPHEAD=no # Extended regular expression defining files to ignore when using --all XDIRREGEX='^(tests/tcg|tests/multiboot|pc-bios|disas/libvixl)' -if [ $# -ne 0 ] && [ "$1" = "--git" ]; then - if [ $# -eq 1 ]; then - echo "--git option requires an argument" - exit 1 - fi - GITSUBJ="$2" - GIT=yes - shift - shift -fi +while true +do + case $1 in + "--git") + if [ $# -eq 1 ]; then + echo "--git option requires an argument" + exit 1 + fi + GITSUBJ="$2" + GIT=yes + shift + shift + ;; + "--check-duphead") + DUPHEAD=yes + shift + ;; + *) + break + ;; + esac +done if [ $# -eq 0 ]; then - echo "Usage: clean-includes [--git subjectprefix] [--all | foo.c ...]" + echo "Usage: clean-includes [--git subjectprefix] [--check-duphead] [--all | foo.c ...]" echo "(modifies the files in place)" exit 1 fi @@ -154,6 +170,14 @@ for f in "$@"; do done +if [ "$DUPHEAD" = "yes" ]; then + grep "^#include" $@ | sort | uniq -c | awk '{if ($1 > 1) print $0}' + if [ $? -eq 0 ]; then + echo "Found duplicate header file includes. Please check the above files manually." + exit 1 + fi +fi + if [ "$GIT" = "yes" ]; then git add -- "$@" git commit --signoff -F - <<EOF