diff mbox series

[1/2] selinux-testsuite: use our own version of perltidy in the Travis CI tests

Message ID 156927696325.621177.10551869484430505777.stgit@chester (mailing list archive)
State Accepted
Headers show
Series selinux-testsuite: fix perltidy in Travis CI | expand

Commit Message

Paul Moore Sept. 23, 2019, 10:16 p.m. UTC
From: Paul Moore <paul@paul-moore.com>

Unfortunately the perltidy results differ between moden distros and the
current Travis CI environment.  This patch attempts to address this by
using the current upstream perltidy in the Travis CI tests.

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 .travis.yml |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Ondrej Mosnacek Sept. 24, 2019, 7:26 a.m. UTC | #1
On Tue, Sep 24, 2019 at 12:16 AM Paul Moore <paul@paul-moore.com> wrote:
> From: Paul Moore <paul@paul-moore.com>
>
> Unfortunately the perltidy results differ between moden distros and the
> current Travis CI environment.  This patch attempts to address this by
> using the current upstream perltidy in the Travis CI tests.

Generally thumbs up from me, although I have a few comments below.

>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
>  .travis.yml |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 61bb1f2..256e92c 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -6,10 +6,17 @@ addons:
>    apt:
>      packages:
>        - astyle
> -      - perltidy
>        - libselinux1-dev
>        - libsctp-dev
>
> +before_install:
> +  - git clone https://github.com/perltidy/perltidy.git perltidy

I think it would be safer to add here something like:

- git checkout 8551fc60fc515cd290ba38ee8c758c1f4df52b56

That way the Travis checks won't suddenly break when something changes
in the master branch (where I'd expect things to change/break once in
a while). IMHO having to bump the commit manually from time to time is
less bothersome than having to deal with random changes in the
upstream branch.

> +  - |
> +    (cd perltidy;
> +     perl Makefile.PL;
> +     make;
> +     sudo make install)

This is not a big deal, but you might want to join these with '&&'
instead of ';' since if an earlier command fails, it doesn't make much
sense to try to run the rest (the pipeline would then almost certainly
fail later anyway).

> +
>  script:
>    - tools/check-syntax -f && git diff --exit-code
>    - make
>

--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.
Paul Moore Sept. 24, 2019, 5:19 p.m. UTC | #2
On Tue, Sep 24, 2019 at 3:26 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Tue, Sep 24, 2019 at 12:16 AM Paul Moore <paul@paul-moore.com> wrote:
> > From: Paul Moore <paul@paul-moore.com>
> >
> > Unfortunately the perltidy results differ between moden distros and the
> > current Travis CI environment.  This patch attempts to address this by
> > using the current upstream perltidy in the Travis CI tests.
>
> Generally thumbs up from me, although I have a few comments below.

...

> > +before_install:
> > +  - git clone https://github.com/perltidy/perltidy.git perltidy
>
> I think it would be safer to add here something like:
>
> - git checkout 8551fc60fc515cd290ba38ee8c758c1f4df52b56
>
> That way the Travis checks won't suddenly break when something changes
> in the master branch (where I'd expect things to change/break once in
> a while). IMHO having to bump the commit manually from time to time is
> less bothersome than having to deal with random changes in the
> upstream branch.

I added a comment indicating that 8551fc60fc51 is a "known good" HEAD,
but I'd prefer to just stick with whatever HEAD might be unless we
find it to be problematic.  If it turns out that things are breaking
often we can always checkout a specific point in time.

> > +  - |
> > +    (cd perltidy;
> > +     perl Makefile.PL;
> > +     make;
> > +     sudo make install)
>
> This is not a big deal, but you might want to join these with '&&'
> instead of ';' since if an earlier command fails, it doesn't make much
> sense to try to run the rest (the pipeline would then almost certainly
> fail later anyway).

Good point.  I'll make that change and push the commits.

Thanks for the review!
diff mbox series

Patch

diff --git a/.travis.yml b/.travis.yml
index 61bb1f2..256e92c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -6,10 +6,17 @@  addons:
   apt:
     packages:
       - astyle
-      - perltidy
       - libselinux1-dev
       - libsctp-dev
 
+before_install:
+  - git clone https://github.com/perltidy/perltidy.git perltidy
+  - |
+    (cd perltidy;
+     perl Makefile.PL;
+     make;
+     sudo make install)
+
 script:
   - tools/check-syntax -f && git diff --exit-code
   - make