Skip to content
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

[r] Use artifacts if no args or system libs found #1454

Closed
wants to merge 4 commits into from

Conversation

eddelbuettel
Copy link
Contributor

@eddelbuettel eddelbuettel commented Jun 6, 2023

Note: This requires #1397 as a prerequisite.

Issue and/or context:

The R package build has, besides the usual R package dependencies, a dual dependency on TileDB Core and the TileDB-SOMA C++ library. This PR generalizes the build to access

  • either user-supplied local location of TileDB Core and/or TileDB-SOMA library installation (mixing is permitting)
  • optionally also automatic use of system-libraries if pkg-config is present and knows about the libraries
  • as a fallback (that can also be opted into) use of binary artifacts downloaded from GitHub

Changes:

This PR generalizes the build a to allow for the optional use, the use of partial optional use along with pkg-config, full use of pkg-config for both as before as well as an extended use of now two binary artifacts. The artifact case has been tested on Docker when no libraries were present.

Notes for Reviewer:

This PR depends on #1397 to produce the artifacts first. It also still requires a manual updating of which artifacts are downloaded, the pinning is currently hard.

@shortcut-integration
Copy link

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2023

Codecov Report

Patch coverage has no change and project coverage change: +27.70 🎉

Comparison is base (21520a6) 63.92% compared to head (de32b7b) 91.63%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1454       +/-   ##
===========================================
+ Coverage   63.92%   91.63%   +27.70%     
===========================================
  Files          99       30       -69     
  Lines        8220     2521     -5699     
===========================================
- Hits         5255     2310     -2945     
+ Misses       2965      211     -2754     
Flag Coverage Δ
python 91.63% <ø> (+0.51%) ⬆️
r ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 81 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@eddelbuettel eddelbuettel force-pushed the de/sc-29983/use_libtiledbsoma_artifacts branch from 70c9634 to d1d45c5 Compare June 14, 2023 19:28
@eddelbuettel
Copy link
Contributor Author

This PR is making good progress. We now have

  • use a GHA yaml file library-artifacts.yaml to produce library artifacts from libtiledbsoma
    • this can be used at release time and it will upload artifacts to the release
    • the same yaml file is also used as workflow in the normal r-ci workflow
  • refactor the normal GHA yaml file r-ci.yml to invoke library-artifacts.yaml
    • the library artifacts are also uploaded as artifacts by library-artifacts.yaml
    • so that r-ci.yml can use them and we have consistent provision and use of the library
    • in non-CI use we continue to either
      • use libtiledbsoma as system library
      • or as a user-provided library via flags to configure ("export mode')
      • or as artifacts downloaded from the release

What is still missing is a consistent propagation of version numbers from the top level to the different packages.

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