Skip to content

feat: better tooling, linting, ci, readme#83

Merged
sebffischer merged 40 commits into
mainfrom
feat-tooling
Sep 4, 2025
Merged

feat: better tooling, linting, ci, readme#83
sebffischer merged 40 commits into
mainfrom
feat-tooling

Conversation

@sebffischer
Copy link
Copy Markdown
Collaborator

@sebffischer sebffischer commented Sep 3, 2025

Summary:

  • Improves the CI by:
    1. Adding spell checks
    2. Adding linter checks for C++ and R
  • Improving the README by:
    1. Simplifying the quickstart example
    2. Adding more acknowledgements
  • Adds a 'no-suggest' test which ensures that the test suite passes also in the absence of suggested packages
  • Addressed linter issues
  • Refactor the code for PJRTElementType class
  • Add Makefile to format code easily

@sebffischer
Copy link
Copy Markdown
Collaborator Author

sebffischer commented Sep 3, 2025

@dfalbel I am trying to setup a Cpp linter. I do this by running devtools::load_all() in the CI and then running clang-tidy and configure it to use compile_commands.json.

However, I still get these errors:

  • 'xla/autotuning.pb.h' file not found (many more like this)
  • Error: 'Rcpp.h' file not found

You can inspect the errors here

Do you understand why this happens? devtools::load_all() runs successfully

@sebffischer
Copy link
Copy Markdown
Collaborator Author

@dfalbel
Copy link
Copy Markdown
Collaborator

dfalbel commented Sep 3, 2025

You probably need to configure the include paths for the linter as those files are in non-conventional include paths and it probably doesn't know how to parse Makevars to extract the correct paths.

@sebffischer
Copy link
Copy Markdown
Collaborator Author

I thought I am building compile_commands.json by running pkgdown::load_all() in the CI, but somehow the file is not created. Do I need to configure anything else (except putting Config/build/compilation-database: true into the description) in order to ensure that compile_commands.json is built?
`

@sebffischer
Copy link
Copy Markdown
Collaborator Author

sebffischer commented Sep 3, 2025

So somehow the generation of the compilation database fails, weird:

Screenshot 2025-09-03 at 22 27 36

@sebffischer
Copy link
Copy Markdown
Collaborator Author

sebffischer commented Sep 3, 2025

This was a bug in pkgload, installing the dev version fixes it.

@sebffischer
Copy link
Copy Markdown
Collaborator Author

Now it's weird that g++ -std=gnu++17 is being used, but we require C++20

@sebffischer
Copy link
Copy Markdown
Collaborator Author

sebffischer commented Sep 4, 2025

For some reason, the compile_commands.json is still off. The files are listed as C files, see the "file: pjrt.c" below.

{
     "command": "g++ -std=gnu++20 -I\"/opt/R/4.5.1/lib/R/include\" -DNDEBUG -DUSENEWAPI -Iproto -I/include -I../inst/include -I\"/home/runner/work/_temp/Library/Rcpp/include\" -I/usr/local/include   -fvisibility=hidden -fpic  -g -O2   -c pjrt.cpp -o pjrt.o",
     "file": "pjrt.c",
     "directory": "/home/runner/work/pjrt/pjrt/src"
},

Compare this with my local compile_commands.json:

{
     "command": "clang++ -arch arm64 -std=gnu++20 -I\"/Library/Frameworks/R.framework/Versions/4.5-arm64/Resources/include\" -DNDEBUG -I/opt/homebrew/Cellar/protobuf@21/21.12_1/include -DUSENEWAPI -Iproto -I/include -I../inst/include -I'/Users/sebi/Library/R/arm64/4.5/library/Rcpp/include' -I/opt/homebrew/include   -fvisibility=hidden -fPIC  -falign-functions=64 -Wall -g -O2   -c pjrt.cpp -o pjrt.o",
     "file": "pjrt.cpp",
     "directory": "/Users/sebi/r-xla/pjrt/src"
},

I think this then causes the cpp headers to not be found, which results in these error messages:

2025-09-04T07:21:18.1829074Z ##[error]'numeric' file not found
2025-09-04T07:21:18.2900471Z ##[error]'limits' file not found
2025-09-04T07:21:18.1746012Z ##[error]'memory' file not found
2025-09-04T07:21:18.1747137Z ##[error]'string' file not found
2025-09-04T07:21:18.1756819Z ##[error]'optional' file not found

@sebffischer
Copy link
Copy Markdown
Collaborator Author

There was a bug in pkgload for which I submitted a fix: r-lib/pkgload#318

@sebffischer
Copy link
Copy Markdown
Collaborator Author

@dfalbel have you used clang-tidy? If so, which checks would you recommend?

@sebffischer sebffischer changed the title better tooling, linting, ci, readme feat: better tooling, linting, ci, readme Sep 4, 2025
@sebffischer sebffischer merged commit 5d9b4d1 into main Sep 4, 2025
11 of 13 checks passed
@sebffischer sebffischer deleted the feat-tooling branch September 4, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants