[dpdk-dev] [PATCH v2] scripts: add git hook scripts for checkpatch and auto doc generation
On Thu, Dec 03, 2015 at 11:09:30AM -0800, Thomas Monjalon wrote: > Ferruh, > I have a lot of questions :) > > 2015-11-27 14:34, Ferruh Yigit: > > --- a/scripts/checkpatches.sh > > +++ b/scripts/checkpatches.sh > > @@ -43,6 +43,7 @@ length=${DPDK_CHECKPATCH_LINE_LENGTH:-80} > > > > # override default Linux options > > options="--no-tree" > > +options="$options ${GIT_HOOK_OPTIONS}" > > What is the purpose of this variable? > Why adding some options would be specific to git hooks? > When you use "commit -s", sign-off msg added after this script run, also when you do "commit --ammend" that piece of patch does not have sign-off part, so pre-commit hook can have problems with sign-off check and git hook specific option is to pass --no-signoff option. > > +++ b/scripts/git-hooks/deploy.sh > > @@ -0,0 +1,20 @@ > > + > > +SELF=$(basename $0) > > + > > +if [ ! -f ${SELF} ]; then > > + echo "Please run script from folder where script is" > > + exit 1 > > +fi > > You do not need to exit. Just cd $(dirname $0). > Thanks, I will do that. > > +for f in ${FILES}; do > > + ln -sf ${SCRIPT_FOLDER}/${f} ${TARGET_FOLDER}/${f} > > +done; > > You do not need the ; > OK > > --- /dev/null > > +++ b/scripts/git-hooks/post-commit > > +if [ -n "$RTE_DOC_OUT" ]; then > > + OUT_CMD="O=${RTE_DOC_OUT}" > > +fi > > How to pass RTE_DOC_OUT to the hook? > Why not use load-devel-config.sh? > It is possbile pass by exporting it as environment variable, but since load-devel-config is already there, I will use it. > > +make ${OUT_CMD} doc-guides-html 2>&1 > /dev/null > > +make ${OUT_CMD} doc-api-html 2>&1 > /dev/null > > Why hiding the errors? > To prevent noise, but I double check and enabling error messages not noisy, so I will enable it. > > --- /dev/null > > +++ b/scripts/git-hooks/post-merge > > Does it work for "git pull --rebase"? No, it is not working, for that pre-rebase is required, easy to create a link for that but I am not sure about increasing number of hooks, with pre-rebase hook, an internal rebase too will trigger doc generation, which is not good I believe. > > > --- /dev/null > > +++ b/scripts/git-hooks/pre-commit > > +# If "git commit" called with "--no-verify" option, pre-commit hooks > > +# bypassed and this script not called, checkpatch bypassed > > Possible reword: It is skipped with the option "--no-verify" of "git commit". > OK, thanks. > > +RTE_CHECKPATCH=$PWD/scripts/checkpatches.sh > > What is PWD when calling the hook. May it be a subdir? > Even you call git commands from a subdir, git hooks scripts run from root dir, so this will be always correct. > > +PATCH=/tmp/dpdk-git-auto-checkpatch-$$.patch > > You need to remove this temporary file when exiting. > But it would be better to use stdin. > It may need a patch on checkpatches.sh wrapper. > In chekpatch.pl: "When FILE is - read standard input". > I also prefer using stdin, and previous patch was like that, but checkpatches.sh requires a file, let me check possible update in checkpatches.sh > > +export GIT_HOOK_OPTIONS=--no-signoff > > This variable is wrongly named. > Will rename. > > +git diff --cached > ${PATCH} > > +exec ${RTE_CHECKPATCH} ${PATCH} > > When the new "make install" will be applied, I think we should skip > these files. Please consider patching mk/rte.sdkinstall.mk. ok, I will update skdinstall.mk Thanks for the review. ferruh
[dpdk-dev] [PATCH v2] scripts: add git hook scripts for checkpatch and auto doc generation
Ferruh, I have a lot of questions :) 2015-11-27 14:34, Ferruh Yigit: > --- a/scripts/checkpatches.sh > +++ b/scripts/checkpatches.sh > @@ -43,6 +43,7 @@ length=${DPDK_CHECKPATCH_LINE_LENGTH:-80} > > # override default Linux options > options="--no-tree" > +options="$options ${GIT_HOOK_OPTIONS}" What is the purpose of this variable? Why adding some options would be specific to git hooks? > +++ b/scripts/git-hooks/deploy.sh > @@ -0,0 +1,20 @@ > + > +SELF=$(basename $0) > + > +if [ ! -f ${SELF} ]; then > + echo "Please run script from folder where script is" > + exit 1 > +fi You do not need to exit. Just cd $(dirname $0). > +for f in ${FILES}; do > + ln -sf ${SCRIPT_FOLDER}/${f} ${TARGET_FOLDER}/${f} > +done; You do not need the ; > --- /dev/null > +++ b/scripts/git-hooks/post-commit > +if [ -n "$RTE_DOC_OUT" ]; then > + OUT_CMD="O=${RTE_DOC_OUT}" > +fi How to pass RTE_DOC_OUT to the hook? Why not use load-devel-config.sh? > +make ${OUT_CMD} doc-guides-html 2>&1 > /dev/null > +make ${OUT_CMD} doc-api-html 2>&1 > /dev/null Why hiding the errors? > --- /dev/null > +++ b/scripts/git-hooks/post-merge Does it work for "git pull --rebase"? > --- /dev/null > +++ b/scripts/git-hooks/pre-commit > +# If "git commit" called with "--no-verify" option, pre-commit hooks > +# bypassed and this script not called, checkpatch bypassed Possible reword: It is skipped with the option "--no-verify" of "git commit". > +RTE_CHECKPATCH=$PWD/scripts/checkpatches.sh What is PWD when calling the hook. May it be a subdir? > +PATCH=/tmp/dpdk-git-auto-checkpatch-$$.patch You need to remove this temporary file when exiting. But it would be better to use stdin. It may need a patch on checkpatches.sh wrapper. In chekpatch.pl: "When FILE is - read standard input". > +export GIT_HOOK_OPTIONS=--no-signoff This variable is wrongly named. > +git diff --cached > ${PATCH} > +exec ${RTE_CHECKPATCH} ${PATCH} When the new "make install" will be applied, I think we should skip these files. Please consider patching mk/rte.sdkinstall.mk. Thanks
[dpdk-dev] [PATCH v2] scripts: add git hook scripts for checkpatch and auto doc generation
These scripts are to automate some common tasks, scripts needs to be deployed to specific folder to become active. Scripts: post-commit: Triggers after commit complete, re-generates api and guides html documents. "RTE_DOC_OUT" environment variable configures document output folder. Same script can be used on server side with name "post-update", so documentation can auto updated after each push to server. post-merge: Same script as "post-commit", but triggered after git pull pre-commit: Does a checkpatch check before commit started. This script relies on scripts/checkpatches.sh script. checkpathes.sh should be running well to use this git hook script. This script can bypassed by commit "--no-verify" argument. Deployment: To make scripts active they need to be in /.git/hooks folder. Alternatively "deploy.sh" script can be used, it simply links all scripts into proper folder. Script names are significant and shouldn't changed. Signed-off-by: Ferruh Yigit --- scripts/checkpatches.sh | 1 + scripts/git-hooks/deploy.sh | 20 scripts/git-hooks/post-commit | 10 ++ scripts/git-hooks/post-merge | 1 + scripts/git-hooks/pre-commit | 17 + 5 files changed, 49 insertions(+) create mode 100755 scripts/git-hooks/deploy.sh create mode 100755 scripts/git-hooks/post-commit create mode 12 scripts/git-hooks/post-merge create mode 100755 scripts/git-hooks/pre-commit diff --git a/scripts/checkpatches.sh b/scripts/checkpatches.sh index afc611b..8192514 100755 --- a/scripts/checkpatches.sh +++ b/scripts/checkpatches.sh @@ -43,6 +43,7 @@ length=${DPDK_CHECKPATCH_LINE_LENGTH:-80} # override default Linux options options="--no-tree" +options="$options ${GIT_HOOK_OPTIONS}" options="$options --max-line-length=$length" options="$options --show-types" options="$options --ignore=LINUX_VERSION_CODE,FILE_PATH_CHANGES,\ diff --git a/scripts/git-hooks/deploy.sh b/scripts/git-hooks/deploy.sh new file mode 100755 index 000..070fb6e --- /dev/null +++ b/scripts/git-hooks/deploy.sh @@ -0,0 +1,20 @@ + +SELF=$(basename $0) + +if [ ! -f ${SELF} ]; then + echo "Please run script from folder where script is" + exit 1 +fi + +FILES=$(ls | grep -v ${SELF}) + +TARGET_FOLDER="../../.git/hooks" +SCRIPT_FOLDER="../../scripts/git-hooks" + +if [ ! -d ${TARGET_FOLDER} ]; then + exit 2 +fi + +for f in ${FILES}; do + ln -sf ${SCRIPT_FOLDER}/${f} ${TARGET_FOLDER}/${f} +done; diff --git a/scripts/git-hooks/post-commit b/scripts/git-hooks/post-commit new file mode 100755 index 000..2a76f96 --- /dev/null +++ b/scripts/git-hooks/post-commit @@ -0,0 +1,10 @@ +# +# Create docs after each commit +# + +if [ -n "$RTE_DOC_OUT" ]; then + OUT_CMD="O=${RTE_DOC_OUT}" +fi + +make ${OUT_CMD} doc-guides-html 2>&1 > /dev/null +make ${OUT_CMD} doc-api-html 2>&1 > /dev/null diff --git a/scripts/git-hooks/post-merge b/scripts/git-hooks/post-merge new file mode 12 index 000..ace4560 --- /dev/null +++ b/scripts/git-hooks/post-merge @@ -0,0 +1 @@ +post-commit \ No newline at end of file diff --git a/scripts/git-hooks/pre-commit b/scripts/git-hooks/pre-commit new file mode 100755 index 000..c46b27d --- /dev/null +++ b/scripts/git-hooks/pre-commit @@ -0,0 +1,17 @@ +# +# Check patch with checkpatch script before commit +# +# If checkpatch fails, commit fails +# +# Relies on scripts/checkpathes.sh script as checkpatch.pl wrapper +# +# If "git commit" called with "--no-verify" option, pre-commit hooks +# bypassed and this script not called, checkpatch bypassed +# + +RTE_CHECKPATCH=$PWD/scripts/checkpatches.sh +PATCH=/tmp/dpdk-git-auto-checkpatch-$$.patch +export GIT_HOOK_OPTIONS=--no-signoff + +git diff --cached > ${PATCH} +exec ${RTE_CHECKPATCH} ${PATCH} -- 2.5.0