Github actions for sf

[etiennebr]

@edzer, there's absolutely no rush, but maybe following #1234 we could move all sf tests to github actions? It should be much faster to run tests, and we can add support for Mac OS.

I think it will require distinct files since windows and mac don't support docker compose (to run the tests against PostgreSQL). I can figure this out if you are interested in the migration. Let me know if you'd rather stick to travis for now.

As @Robinlovelace mentionned, we could store the actions in https://github.com/edzer/sf_dep.

[edzer]

Yes, I am very much interested. Can it also take over code coverage somehow?

[etiennebr]

Yes, it still uses codecov

sf/.github/workflows/R-CMD-check.yaml

Lines 120 to 123 in 53f4dfb

	- name: Test coverage
	if: matrix.config.os == 'ubuntu-latest' && matrix.config.r == '3.6'
	run: covr::codecov(token = "${{secrets.CODECOV_TOKEN}}")
	shell: Rscript {0}

[etiennebr]

I have an issue with both Windows and Mac.

Windows

Everything looks fine, except a warning that configure.ac uses CRLF

checking line endings in shell scripts ... WARNING
Found the following shell script(s) with CR or CRLF line endings:
configure.ac
Non-Windows OSes require LF line endings.

Error in context

Surprisingly, the warning is on Windows, complaining for non-Windows OS, but I don't have this warning on other OS. Also appveyor doesn't complain. Maybe it has to do with the default encoding used in github actions.

Mac

This one seems to be more common, and I've seen issues revolving around these errors, but if anyone has a solution that could save me some time since I don't have a mac, so I need to debug it through github actions.

checking PROJ: checking whether PROJ and sqlite3 are available for linking:... no
configure: error: libproj not found in standard or given locations.
ERROR: configuration failed for package ‘sf’

• removing ‘/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/Rtmpvc3R1D/Rinst12a94b9a4b92/sf’
  -----------------------------------
  ERROR: package installation failed
  ##[error]Error in proc$get_built_file() : Build process failed
  Calls: ... build_package -> with_envvar -> force ->
  Execution halted
  ##[error]Process completed with exit code 1.

Error in context

I'll work on these, but if you have any idea feel free to jump in. Thanks.

[ateucher]

I can help with the Mac issue

[pat-s]

FYI since you are already using {tic} on Travis here: https://ropensci.org/technotes/2020/03/13/tic-ghactions/

[ateucher]

@etiennebr I think we may need to split up the workflows into Linux (with the postgres images) and mac/windows... I don't know a way to conditionally use services. Thoughts?

[etiennebr]

@ateucher, I came to the same conclusion about the services. Maybe it wasn't obvious from the logs, so here's the link to my fork PR etiennebr#69.

[edzer]

Great work! Looks like this can be closed, now?

[etiennebr]

Thanks, I'll probably need an extra week, but here's what I would still like to do:

• Add action for Windows (still has an issue with the configure.ac)
• Confirm that code coverage happens through actions (I'm somewhat not certain, although I see it in the reports)
• Switch to use actions/checkout@v2 on all checks (we did it only partially for mac and it seems to work so far)
• Should I completely deactivate travis and appveyor? I don't see any problem to keep them around for a bit, but they are redundant.

[ateucher]

Merging #1358 will simplify the macOS actions as well.

For codecov, there needs to be a CODECOV_TOKEN secret in the repo (store at https://github.com/r-spatial/sf/settings/secrets), generated from https://codecov.io/gh/r-spatial/sf/settings -> Repository Upload Token.

[pat-s]

Just browsed through the YAML file in #1358 and sharing my thoughts. I already saw that some other spatial pkgs adapt the sf GHA config so maybe it makes sense to polish it up/reduce clutter a bit to avoid populating all of this to other repos.

For codecov, there needs to be a CODECOV_TOKEN secret in the repo (store at r-spatial/sf/settings/secrets), generated from codecov.io/gh/r-spatial/sf/settings -> Repository Upload Token.

Should I completely deactivate travis and appveyor? I don't see any problem to keep them around for a bit, but they are redundant.

Travis is still doing the auto-deploy of the pkgdown site via {tic}. If you want to remove Travis and tic completely, keep in mind to do active pkgdown deployment in some other way.
Out of personal interest: Is there any troublesome reason not to continue with {tic} further? All you would have needed (and maybe 2-3 lines) would have been to run tic::use_ghactions_yml() to switch from Travis to GHA).

Comments

Not needed with covr v3.5.0.

https://github.com/ateucher/sf/blob/60f35347acdea80ef3fef88e9219b45144703d84/.github/workflows/R-CMD-check.yaml#L46

pkgconfig is now installed by default

https://github.com/ateucher/sf/blob/60f35347acdea80ef3fef88e9219b45144703d84/.github/workflows/R-CMD-check.yaml#L52-L53

This can be solved in a canonical way by setting SDKROOT = /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk

r-quantities/units#236 (comment)

https://github.com/ateucher/sf/blob/60f35347acdea80ef3fef88e9219b45144703d84/.github/workflows/R-CMD-check.yaml#L86

Unless you want the very latest libs, using a PPA is not necessary.

[etiennebr]

Thanks @pat-s this is very helpful. I am also surprised by the rapid adoption of these actions, so I agree they should be polished!

The only reason for not using tic to transition is out of pure ignorance and lack of time (which would probably have been better invested in reading tic documentation). I started fiddling with actions in geotidy, which didn't use tic and just copied it here and expanded it using the existing travis.yaml. When I read your earlier comment, I thought sf only used it for pkgdown auto-deploy. Now I understand it can do more.

[pat-s]

Thanks! No worries, I'm not affronted in any way - I assumed that this was the reason and it's very much understandable.
I don't wanna jump in and shout out loud to change everything - this is especially awkward if one is the maintainer of the suggested change that would be discussed 😆
And neither do I do so now because you have invested so much into the tidyverse approach already that you should probably just stick with it :)

{tic} can do a lot, yes - maybe even a bit too much. And it can be scary due to the large documentation and the complexity of CI in general (in its details). There are quite some opinionated differences to the tidyverse approach. But this is the wrong place to expand more on this.

LMK if I can help (happy to do so at any time).

[edzer]

          - {os: ubuntu-latest, r: 'release'}

gives consistent setup errors now for weeks, spamming me with error mails with every push. Can I switch this off, or does anyone know how to fix it?

[pat-s]

The issue comes from a faulty package cache because it was not invalidated for quite some time: https://github.com/r-spatial/sf/runs/702728725?check_suite_focus=true
{remotes} is still installed from R <= 4.0.

In GitHub in general, you can currently either

• Opt out from any GHA build notifications
• Get notifications only for failing builds (but then always)

This applies to all repos and cannot be specified in a per-repo base.

Fixing this particular issues requires to invalidate the current cache hash, for example by appending a 1 to these two lines: https://github.com/r-spatial/sf/actions/runs/113498945/workflow#L73-L74

(FWIW: To account for this issue and prevent broken package caches in general, {tic} builds a fresh cache daily in a CRON build)

[etiennebr]

Switching it off is fine for now. I think I want to use tic; it could help us in many places, but I don't have the bandwidth to do it short term. I'll close the issue since switching off seems to have the tests passing and I'll open a new one when I get to complete it. Or if @pat-s you want to open a PR using tic I would be happy to review it :)