[PATCH 0/2] selinux-testsuite: fix perltidy in Travis CI

[PATCH 0/2] selinux-testsuite: fix perltidy in Travis CI

[Paul Moore]

A small patchset that fixes perltidy on Travis CI (Ubuntu 16.04 LTS)
by building a local version of perltidy using the current upstream
which is shipped with most modern distros.

---

Paul Moore (2):
      selinux-testsuite: use our own version of perltidy in the Travis CI tests
      selinux-testsuite: apply perltidy fixes


 .travis.yml                   |    9 ++++++++-
 tests/infiniband_endport/test |    2 +-
 tests/infiniband_pkey/test    |    2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

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

[Paul Moore]

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(-)

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

[PATCH 2/2] selinux-testsuite: apply perltidy fixes

[Paul Moore]

From: Paul Moore <paul@paul-moore.com>

Signed-off-by: Paul Moore <paul@paul-moore.com>
---
 tests/infiniband_endport/test |    2 +-
 tests/infiniband_pkey/test    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/infiniband_endport/test b/tests/infiniband_endport/test
index 0021e0d..f071fbc 100755
--- a/tests/infiniband_endport/test
+++ b/tests/infiniband_endport/test
@@ -40,7 +40,7 @@ ok( $result, 0 );
 
 foreach (@denied_device_port) {
     @dev_port_pair = split( / /, $_ );
-    $result = system
+    $result        = system
 "runcon -t test_ibendport_manage_subnet_t smpquery PKeyTable -C $dev_port_pair[0] -P $dev_port_pair[1] -D 1 2>/dev/null";
 
     if ( $result >> 8 eq 0 ) {
diff --git a/tests/infiniband_pkey/test b/tests/infiniband_pkey/test
index 3ee4d13..382c259 100755
--- a/tests/infiniband_pkey/test
+++ b/tests/infiniband_pkey/test
@@ -32,7 +32,7 @@ close($f);
 
 # The gid sysfs shows a fully expanded ipv6 address, just take the
 # top half.
-@tmp = unpack( '(a20)*', $gid );
+@tmp           = unpack( '(a20)*', $gid );
 $subnet_prefix = $tmp[0] . ":";
 
 @labeled_pkeys = split( /,/, $conf{SELINUX_INFINIBAND_TEST_LABELED_PKEYS} );

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

[Ondrej Mosnacek]

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.

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

[Paul Moore]

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!

-- 
paul moore
www.paul-moore.com