Skip to content

Commit

Permalink
update GLCI (warnings + docs) (#6586)
Browse files Browse the repository at this point in the history
* adjust NOTE count

* silent compiler warning

* update flags

* remove wvla flag

* update CI docs

* update markdown highlighting

* add todos

* remove integration deployment

* update filepaths

* fix grammar

* Update .ci/README.md

Co-authored-by: Michael Chirico <chiricom@google.com>

* mention when license needs to be renewed

---------

Co-authored-by: Michael Chirico <chiricom@google.com>
  • Loading branch information
ben-schwen and MichaelChirico authored Nov 2, 2024
1 parent 8fcb529 commit 6f85f86
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 13 deletions.
21 changes: 20 additions & 1 deletion .ci/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# data.table continuous integration and deployment

On each Pull Request opened in GitHub we run GitHub Actions test jobs to provide prompt feedback about the status of PR. Our main CI pipeline runs on GitLab CI nightly. GitLab repository automatically mirrors our GitHub repository and runs pipeline on `master` branch every night. It tests more environments and different configurations. It publish variety of artifacts.
On each Pull Request opened in GitHub we run GitHub Actions test jobs to provide prompt feedback about the status of PR. Our more thorough main CI pipeline runs nightly on GitLab CI. GitLab repository automatically mirrors our GitHub repository and runs pipeline on `master` branch every night. It tests more environments and different configurations. It publishes a variety of artifacts such as our [homepage](https://rdatatable.gitlab.io/data.table/) and [CRAN-like website for dev version](https://rdatatable.gitlab.io/data.table/web/packages/data.table/index.html), including windows binaries for the dev version.

## Environments

Expand Down Expand Up @@ -44,3 +44,22 @@ Base R implemented helper script, [originally proposed to base R](https://svn.r-
### [`publish.R`](./publish.R)

Base R implemented helper script to orchestrate generation of most artifacts and to arrange them nicely. It is being used only in [_integration_ stage in GitLab CI pipeline](./../.gitlab-ci.yml).

## GitLab Open Source Program

We are currently part of the [GitLab for Open Source Program](https://about.gitlab.com/solutions/open-source/). This gives us 50,000 compute minutes per month for our GitLab CI. Our license needs to be renewed yearly (around July) and is currently managed by @ben-schwen.

## Updating CI pipeline

Basic CI checks are also run on every push to the GitLab repository. This can **and should** be used for PRs changing the CI pipeline before merging them to master.

```shell
# fetch changes from remote (GitHub) and push them to GitLab
git fetch git@github.com:Rdatatable/data.table.git new_branch:new_branch
git push
# after updating on GitHub, pull changes from remote and push to GitLab
git pull git@github.com:Rdatatable/data.table.git new_branch
git push
```

Make sure to include a link to the pipeline results in your PR.
2 changes: 1 addition & 1 deletion .ci/publish.R
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ format.bins <- function(ver, bin_ver, cran.home, os.type, pkg, version, repodir)
plat.path = "windows"
} else if (os.type=="macosx") {
ext = "tgz"
plat.path = "macosx/el-capitan"
plat.path = "macosx/big-sur-arm64"
} else stop("format.bins only valid for 'windows' or 'macosx' os.type")
file = sprintf("bin/%s/contrib/%s/%s_%s.%s", plat.path, bin_ver, pkg, version, ext)
fe = file.exists(file.path(repodir, file))
Expand Down
13 changes: 4 additions & 9 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,15 +143,14 @@ test-lin-rel-cran:
<<: *test-lin
image: registry.gitlab.com/jangorecki/dockerfiles/r-base
variables:
_R_CHECK_COMPILATION_FLAGS_KNOWN_: "-Wvla"
_R_CHECK_CRAN_INCOMING_: "TRUE" ## stricter --as-cran checks should run in dev pipelines continuously (not sure what they are though)
_R_CHECK_CRAN_INCOMING_REMOTE_: "FALSE" ## Other than no URL checking (takes many minutes) or 'Days since last update 0' NOTEs needed, #3284
_R_CHECK_CRAN_INCOMING_TARBALL_THRESHOLD_: "7500000" ## bytes
_R_CHECK_PKG_SIZES_THRESHOLD_: "7" ## MB 'checking installed package size' NOTE
_R_CHECK_PKG_SIZES_THRESHOLD_: "10" ## MB 'checking installed package size' NOTE increased due to po
script:
- *install-deps
- echo 'CFLAGS=-g -O2 -fopenmp -Wall -Wvla -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' > ~/.R/Makevars
- echo 'CXXFLAGS=-g -O2 -fopenmp -Wall -Wvla -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' >> ~/.R/Makevars
- echo 'CFLAGS=-g -O2 -fopenmp -Wall -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' > ~/.R/Makevars
- echo 'CXXFLAGS=-g -O2 -fopenmp -Wall -pedantic -fstack-protector-strong -D_FORTIFY_SOURCE=2' >> ~/.R/Makevars
- R CMD check --as-cran $(ls -1t data.table_*.tar.gz | head -n 1)
- >-
Rscript -e 'l=tail(readLines("data.table.Rcheck/00check.log"), 1L); if (!identical(l, "Status: OK")) stop("Last line of ", shQuote("00check.log"), " is not ", shQuote("Status: OK"), " but ", shQuote(l)) else q("no")'
Expand Down Expand Up @@ -198,7 +197,7 @@ test-lin-dev-clang-cran:
- R CMD check --as-cran $(ls -1t data.table_*.tar.gz | head -n 1)
- (! grep "warning:" data.table.Rcheck/00install.out)
- >-
Rscript -e 'l=tail(readLines("data.table.Rcheck/00check.log"), 1L); notes<-"Status: 3 NOTEs"; if (!identical(l, notes)) stop("Last line of ", shQuote("00check.log"), " is not ", shQuote(notes), " (size of tarball, installed package size, non-API calls) but ", shQuote(l)) else q("no")'
Rscript -e 'l=tail(readLines("data.table.Rcheck/00check.log"), 1L); notes<-"Status: 2 NOTEs"; if (!identical(l, notes)) stop("Last line of ", shQuote("00check.log"), " is not ", shQuote(notes), " (size of tarball, non-API calls) but ", shQuote(l)) else q("no")'
# stated dependency on R
test-lin-ancient-cran:
Expand Down Expand Up @@ -311,7 +310,6 @@ integration:
- saas-linux-medium-amd64
only:
- master
- macos_binaries
needs: ["mirror-packages","build","test-lin-rel","test-lin-rel-cran","test-lin-dev-gcc-strict-cran","test-lin-dev-clang-cran","test-lin-rel-vanilla","test-lin-ancient-cran","test-win-rel","test-win-dev" ,"test-win-old","test-mac-rel","test-mac-old"]
script:
- R --version
Expand All @@ -333,9 +331,6 @@ integration:
- rm -f bus/mirror-packages/cran/bin/macosx/big-sur-arm64/contrib/$R_REL_VERSION/data.table_*.tgz
# - rm -f bus/mirror-packages/cran/bin/macosx/big-sur-arm64/contrib/$R_DEV_VERSION/data.table_*.tgz
- rm -f bus/mirror-packages/cran/bin/macosx/big-sur-arm64/contrib/$R_OLD_VERSION/data.table_*.tgz
#- rm -f bus/mirror-packages/cran/bin/macosx/el-capitan/contrib/$R_REL_VERSION/data.table_*.tgz
#- rm -f bus/mirror-packages/cran/bin/macosx/el-capitan/contrib/$R_DEV_VERSION/data.table_*.tgz
#- rm -f bus/mirror-packages/cran/bin/macosx/el-capitan/contrib/$R_OLD_VERSION/data.table_*.tgz
## merge mirror-packages and R devel packages
- mv bus/mirror-packages/cran bus/$CI_JOB_NAME/
## publish package sources
Expand Down
2 changes: 1 addition & 1 deletion src/freadR.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
// Where no halt is happening, we can just use raw Rprintf() or warning()
void __halt(bool warn, const char *format, ...); // see freadR.c
#define STOP(...) __halt(0, __VA_ARGS__)
static char internal_error_buff[1001]; // match internalErrSize
static char internal_error_buff[1001] __attribute__((unused)); // match internalErrSize // todo: fix imports such that compiler warns correctly #6468
#define INTERNAL_STOP(...) do {snprintf(internal_error_buff, 1000, __VA_ARGS__); __halt(0, "%s %s: %s. %s", _("Internal error in"), __func__, internal_error_buff, _("Please report to the data.table issues tracker"));} while (0)
#define DTPRINT Rprintf
#define DTWARN(...) warningsAreErrors ? __halt(1, __VA_ARGS__) : warning(__VA_ARGS__)
Expand Down
2 changes: 1 addition & 1 deletion src/fwrite.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "po.h"
#define STOP error
#define DTPRINT Rprintf
static char internal_error_buff[256];
static char internal_error_buff[256] __attribute__((unused)); // todo: fix imports such that compiler warns correctly #6468
#define INTERNAL_STOP(...) do {snprintf(internal_error_buff, 255, __VA_ARGS__); error("%s %s: %s. %s", _("Internal error in"), __func__, internal_error_buff, _("Please report to the data.table issues tracker"));} while (0)
#endif

Expand Down

0 comments on commit 6f85f86

Please sign in to comment.