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