[PATCH] scripts: add developer script validate-commits

[Thomas De Schampheleire]

El lun., 31 dic. 2018 a las 1:53, Mads Kiilerich
(<mads at kiilerich.com>) escribió:
>
> On 12/30/18 8:52 PM, Thomas De Schampheleire wrote:
> > # HG changeset patch
> > # User Thomas De Schampheleire <thomas.de_schampheleire at nokia.com>
> > # Date 1546198086 -3600
> > #      Sun Dec 30 20:28:06 2018 +0100
> > # Node ID 08ccc957e6bc3b170ce2c5db4a63574b51528a79
> > # Parent  42d4c56ad79c4a492b8bc607aa070de32d9c6073
> > scripts: add developer script validate-commits
> >
> > validate-commits can be used to verify a series of commits before
> > submitting/pushing it. It will create a virtualenv and run run-all-cleanup
> > and pytest.
> >
> > Invocation:
> >   validate-commits <revset>
> >
> > Output:
> >   OK: 99c28d4459286261f7e798ab3fe60be3b0be2e7e (run-all-cleanup)
> >   OK: 99c28d4459286261f7e798ab3fe60be3b0be2e7e (pytest)
> >   OK: 8a79857fb430eea80556af8d7e7c88810eee7f25 (run-all-cleanup)
> > NOK: 8a79857fb430eea80556af8d7e7c88810eee7f25 (pytest)
> >   OK: 42d4c56ad79c4a492b8bc607aa070de32d9c6073 (run-all-cleanup)
> > NOK: 42d4c56ad79c4a492b8bc607aa070de32d9c6073 (pytest)
> >
> > diff --git a/scripts/validate-commits b/scripts/validate-commits
> > new file mode 100755
> > --- /dev/null
> > +++ b/scripts/validate-commits
> > @@ -0,0 +1,66 @@
> > +#!/usr/bin/env bash
> > +# Validate the specified commits against test suite and other checks.
> > +
> > +if [ -n "$VIRTUAL_ENV" ]; then
> > +    echo "Please run this script from outside a virtualenv."
> > +    exit 1
> > +fi
> > +
> > +if ! hg update --check . >/dev/null; then
> > +    echo "Working dir is not clean, please commit/revert changes first."
> > +    exit 1
> > +fi
> > +
> > +venv=$(mktemp -d kallithea-validatecommits-env-XXXXXX)
> > +resultfile=$(mktemp kallithea-validatecommits-result-XXXXXX)
> > +echo > "$resultfile"
> > +
> > +cleanup()
> > +{
> > +    rm -rf /tmp/kallithea-test*
> > +    rm -rf "$venv"
> > +}
> > +finish()
> > +{
> > +    cleanup
> > +    # print (possibly intermediate) results
> > +    cat "$resultfile"
> > +    rm "$resultfile"
> > +}
> > +trap finish EXIT
> > +
> > +for rev in $(hg log -r "$@" -T '{node}\n'); do
>
>
> Quoted $@ is special, and this will give confusing result if the script
> is invoked with less or more than 1 one parameter. I assume we should
> use $1 instead.
>
>
> > +    hg log -r "$rev"
> > +    hg update "$rev"
> > +
> > +    cleanup
> > +    virtualenv -p "$(command -v python2)" "$venv"
> > +    source "$venv/bin/activate"
> > +    pip install --upgrade pip setuptools
> > +    pip install -e .
> > +    pip install -r dev_requirements.txt
>
>
> I have found that in case of "different but compatible constraints", it
> is more stable to install both the app and dev requirements at once: pip
> install --upgrade -e . -r dev_requirements.txt
>
> We should perhaps change to recommend that everywhere?

'everywhere' would be in this script and in docs/contributing.rst, right?
In other places there is no installation of dev_requirements?

>
>
> > +    pip install python-ldap python-pam
>
>
> Should we just include these in dev_requirements.txt ?

I think both of these packages have dependencies expected on the host
system. python-ldap requires the ldap.h header file, normally provided
by the host system.
For 'core' developers like you and me, we could reasonably expect them
to 'sacrifice' their host system and install stuff they would not
otherwise need, but for casual developers it may be a bit too much.
(just my thoughts)

/Thomas