bioperl / bioperl-live

Core BioPerl 1.x code
http://bioperl.org
296 stars 182 forks source link

Add initial GitHub Actions workflow #374

Closed zmughal closed 1 year ago

zmughal commented 1 year ago

Since Travis CI is changed its terms, many FOSS projects on GitHub have started using GitHub Actions instead. This was previously discussed at https://github.com/bioperl/bioperl-live/issues/224#issuecomment-1093038760.


This PR is an initial working CI workflow to get started with GitHub Actions. I believe that further work can be done to enable Coveralls code coverage.

I can either add that in this PR or in subsequent PRs.

I have added Coveralls support to this PR

zmughal commented 1 year ago

To make the most out of the Coveralls service for code coverage, enabling the Status API would be helpful as discussed at https://github.com/bioperl/bioperl-live/issues/224#issuecomment-322060871.

zmughal commented 1 year ago

As an aside, if a workflow must be created for the rest of the repos, it might be worth creating a shared set of GitHub Actions as done at https://github.com/PDLPorters/devops/tree/master/github-actions and https://github.com/Perl-GPU/devops/tree/main/github-actions.

zmughal commented 1 year ago

@cjfields, @carandraug, reaching out for review of this.

Also, thoughts on standardising the CI across the organisation?

cjfields commented 1 year ago

@cjfields, @carandraug, reaching out for review of this.

Also, thoughts on standardising the CI across the organisation?

Awesome work, thanks! If we can come up with a standard, yes it makes sense to standardize for all repos under the bioperl org.

Let's give it a week, if @carandraug or others don't have any objections or comments I'll merge.

hlapp commented 1 year ago

Super cool, thanks so much @zmughal 🎉

zmughal commented 1 year ago

@cjfields, @carandraug, reaching out for review of this. Also, thoughts on standardising the CI across the organisation?

Awesome work, thanks! If we can come up with a standard, yes it makes sense to standardize for all repos under the bioperl org.

If we create a devops repository, I can start gathering data on dependencies there and start putting together a shared GitHub Action for doing the same as what this PR does within that. I can't since I'm not in the org.

carandraug commented 1 year ago

I'm not familiar with github actions. However, why are we using cpanm to install the dependencies? I've packaged all the bioperl development dependencies to Debian years ago so they should have already reached ubuntu, so an apt-get install should suffice and be faster (and avoid the problem of dependencies changing in the mean time).

cjfields commented 1 year ago

Awesome work, thanks! If we can come up with a standard, yes it makes sense to standardize for all repos under the bioperl org.

If we create a devops repository, I can start gathering data on dependencies there and start putting together a shared GitHub Action for doing the same as what this PR does within that. I can't since I'm not in the org.

That can be fixed pretty easily :)

cjfields commented 1 year ago

I'm not familiar with github actions. However, why are we using cpanm to install the dependencies? I've packaged all the bioperl development dependencies to Debian years ago so they should have already reached ubuntu, so an apt-get install should suffice and be faster (and avoid the problem of dependencies changing in the mean time).

I agree, though maybe we need to test both paths somewhat? I'd think cpanm would be the most common (but possibly brittle) installation path, apt the most stable.

zmughal commented 1 year ago

I'm not familiar with github actions. However, why are we using cpanm to install the dependencies?

This GitHub Actions workflow is similar to the previous Travis CI set up and that also used cpanm.

I've packaged all the bioperl development dependencies to Debian years ago so they should have already reached ubuntu, so an apt-get install should suffice and be faster (and avoid the problem of dependencies changing in the mean time).

Installing via a CPAN client is the typical scenario for libraries that will go on CPAN as there isn't a way to pin to a specific version for library code (using a cpanfile or Carton would help if pinning is needed, but version pinning that way is more meant to be used for application code).

It wouldn't be too much work to test that as well to establish a baseline of dependency versions, but it would be a separate workflow (and it would have to use the system Perl so could only test against that Perl version). This would either explicitly list the packages or use apt-file to get them:

$ cpanm -q --showdeps ./BioPerl-1.7.8.tar.gz  | perl -lpe 's,::,/,g; $_ = qq{/usr/share/perl5/$_.pm}' | apt-file search -lFf -
libdata-stag-perl
liberror-perl
libgraph-perl
[...]
libwww-perl
[...]
libyaml-perl

I suppose adding this would help Debian users and possibly catch things for the Debian Med and Debian Perl packaging team. I'm thinking it should be done in a container (stable: debian:bullseye, oldstable: debian:buster). I see that the current Dockerfile uses ubuntu:14.04.

cjfields commented 1 year ago

Personally I'd say go with the path that is easiest to maintain and build upon. If it works for bioperl-live (which is by far the most complicated), it should work for any other repos under the bioperl org.

zmughal commented 1 year ago

@carandraug, I have a branch that tests using Debian containers as well. It's quite small and should be reusable across the organisation: https://github.com/zmughal-contrib/bioperl-live/commit/6751bb8c2d818af41a8cf56b12ba47e2dd6cb4b2.

commit 6751bb8c2d818af41a8cf56b12ba47e2dd6cb4b2

    Add test using Debian container

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 05b8185da..762b2f04f 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -9,6 +9,8 @@ on:
 env:
   PKG_DEPS_UBUNTU: >-
     libdb-dev
+  PKG_DEPS_DEBIAN: >-
+    libdb-dev

 jobs:
   dist:
@@ -122,3 +124,29 @@ jobs:
         run: |
           cpanm --verbose --test-only .
           cover -report Coveralls
+
+  containers:
+    needs: dist
+    runs-on: ubuntu-latest
+    container: ${{ matrix.container }}
+    strategy:
+      fail-fast: false
+      matrix:
+        container: ['debian:bullseye', 'debian:bookworm']
+    steps:
+      - name: Get dist artifact
+        uses: actions/download-artifact@v3
+        with:
+          name: dist
+      - name: Setup system deps (apt)
+        if: ${{ startsWith(matrix.container, 'debian:') }}
+        run: |
+          apt-get -y update && apt-get install -y --no-install-recommends perl cpanminus make apt-file
+          apt-file update
+          apt-get install -y --no-install-recommends \
+            ${{ env.PKG_DEPS_DEBIAN }} \
+            $( cpanm -q --showdeps .  | perl -MConfig -MCwd=abs_path '-M5;@prefixes = map abs_path($_), @Config{qw(vendorlibexp vendorarchexp)}' -lpe 's,~.*$,,; s,::,/,g; $mod = $_; $_ = join qq{\n}, map { qq{$_/${mod}.pm} } @prefixes' | apt-file search -lFf - )
+
+      - name: Run tests
+        run: |
+          cpanm --verbose --test-only .

Also, this revealed a failing test! I'll open an issue for that.

zmughal commented 1 year ago

Any other specific changes I should make here? This is just the first step and when consolidated into a shared reusable workflow, it can be modified in a single place for the whole organisation.

cjfields commented 1 year ago

I don't think there are any objections to this apart from maybe simple tweaks which can be accomplished later. Thanks for taking this on @zmughal !

zmughal commented 1 year ago

I don't think there are any objections to this apart from maybe simple tweaks which can be accomplished later. Thanks for taking this on @zmughal !

Great! Another thing that would be useful to have is to go to https://coveralls.io/github/bioperl/bioperl-live/settings and enable the Status API so that the coverage can be displayed as part of the pull request. A screenshot of an example setting: image

cjfields commented 1 year ago

I don't think there are any objections to this apart from maybe simple tweaks which can be accomplished later. Thanks for taking this on @zmughal !

Great! Another thing that would be useful to have is to go to https://coveralls.io/github/bioperl/bioperl-live/settings and enable the Status API so that the coverage can be displayed as part of the pull request. A screenshot of an example setting: image

@zmughal done!