New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor CI #308

Merged
merged 2 commits into from May 13, 2018

Conversation

Projects
None yet
2 participants
@tadeboro
Copy link
Contributor

tadeboro commented May 13, 2018

This pull request contains changes to Travis integration that will make future modifications a bit easier.

There are two things that this pull request achieves:

  1. clean-up of existing CI stuff (astyle related dependencies) and
  2. installation of fixed OpenSSL version that we would like to test building against.

This work will all us to rebase and apply changes in pull request #306 and resolve bug #304

@tadeboro tadeboro force-pushed the xlab-si:refactor-ci branch from d1e722a to 31b0003 May 13, 2018

@tadeboro tadeboro changed the title WIP: Refactor CI Refactor CI May 13, 2018

@DimitriPapadopoulos
Copy link
Collaborator

DimitriPapadopoulos left a comment

Just a few details. Overall this looks great.

.travis.yml Outdated
@@ -1,23 +1,19 @@
language: c

dist: trusty

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

I think there is no need to specify trusty here since we install a recent OpenSSL anyway.

This comment has been minimized.

@tadeboro

tadeboro May 13, 2018

Contributor

I added this because Travis will use Precise (12.04) by default, which reached its EOL about a year ago. But I can remove it without any problem.

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

Ah, OK then.

@@ -0,0 +1,18 @@
#!/bin/bash

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

Perhaps /bin/sh is sufficient.

This comment has been minimized.

@tadeboro

tadeboro May 13, 2018

Contributor

Will fix this.


VERSION=3.1
SRC=https://sourceforge.net/projects/astyle/files/astyle
SRC+=/astyle%20${VERSION}/astyle_${VERSION}_linux.tar.gz/download

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

Just a personal opinion, but I would prefer a single line even if it's too long. It's just a long URL. A single line is more readable.

This comment has been minimized.

@tadeboro

tadeboro May 13, 2018

Contributor

Sure.


PREFIX=$1

ln -s ${PREFIX}/bin/astyle $HOME/bin/astyle

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

Either ${PREFIX} / ${HOME} or $PREFIX / $HOME but not a mixture of both.

This comment has been minimized.

@tadeboro

tadeboro May 13, 2018

Contributor

I missed that $HOME. Will unifiy.

@@ -0,0 +1,17 @@
#!/bin/bash

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

Perhaps /bin/sh is sufficient.

This comment has been minimized.

@tadeboro

tadeboro May 13, 2018

Contributor

Will fix.


PREFIX=$1

[[ -x ${PREFIX}/bin/openssl ]] && exit 0

This comment has been minimized.

@DimitriPapadopoulos

DimitriPapadopoulos May 13, 2018

Collaborator

I'd use $PREFIX instead of ${PREFIX} for consistency with the rest of the script.

This comment has been minimized.

@tadeboro

tadeboro May 13, 2018

Contributor

Sure.

tadeboro added some commits May 13, 2018

Refactor Travis CI integration
Up until now, Travis integration has been specified inline in
configuration file, which is not optimal.

This commit refactors this integration by splitting out the
installation of astyle requirement into separate script that is cache
aware. Installation is now also done into separate cache folder that
can be also used by other dependencies.
Build openfortivpn against OpenSSL 1.0.2
Since older OpenSSL version are not supported anymore by the upstream,
it makes sense to test against something that is still actively
supported.

Additional benefit of this change is the fact that we will be testing
pkg-config integration a bit more, since previously, pkg-config
command only produced linker flags while now we will also get
preprocessor flags into compiler command.

@tadeboro tadeboro force-pushed the xlab-si:refactor-ci branch from c67056a to ca787ee May 13, 2018

@tadeboro

This comment has been minimized.

Copy link
Contributor

tadeboro commented May 13, 2018

I adjusted the sources as indicated in comments.

@DimitriPapadopoulos DimitriPapadopoulos merged commit 8a4da42 into adrienverge:master May 13, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tadeboro tadeboro deleted the xlab-si:refactor-ci branch May 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment