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/ci] Unbreak wheel builds #2398

Merged
merged 1 commit into from
Apr 5, 2024
Merged

[r/ci] Unbreak wheel builds #2398

merged 1 commit into from
Apr 5, 2024

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Apr 5, 2024

Issue and/or context:

#2393

Changes:

nanoarrow.c mods were passing all regular per-commit CI but were failing wheel builds which are done nightly, and at release time.

Compile error appearing in wheel builds:

    /project/tiledbsoma-1.5.0rc0.post284.dev1878736499/dist_links/libtiledbsoma/src/utils/nanoarrow.c:1775:28: error: null destination pointer [-Werror=format-truncation=]
     1775 |     n_chars_last = snprintf(out, n, ">");
          |                    ~~~~~~~~^~~~~~~~~~~~~

Notes for Reviewer:

This unblocks my attempt to tag 1.9.4.

Validation: I manually ran https://github.com/single-cell-data/TileDB-SOMA/actions/runs/8572181618/job/23494196523 with PR #2390 which is essentially this PR.

@eddelbuettel
Copy link
Contributor

eddelbuettel commented Apr 5, 2024

Nice catch of an uncovereed corner:

int64_t ArrowSchemaToString(const struct ArrowSchema* schema, char* out, int64_t n,
char recursive) {
if (schema == NULL) {
return snprintf(out, n, "[invalid: pointer is null]");
}
if (schema->release == NULL) {
return snprintf(out, n, "[invalid: schema is released]");
}
if (out == NULL) {
return 0;
}

I found upstream quite receptive for the few issues I filed so you could consider mentioning it at their tracker. From following their repo, I am fairly certain they build under Python, do not yet (AFAIK) build at PyPi (but "should be soon") and may not have hit this wheels issue.

Copy link

codecov bot commented Apr 5, 2024

Codecov Report

Merging #2398 (5483ea2) into main (f1fb30b) will increase coverage by 13.95%.
The diff coverage is n/a.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2398       +/-   ##
===========================================
+ Coverage   66.07%   80.03%   +13.95%     
===========================================
  Files         144       91       -53     
  Lines       13002     8593     -4409     
  Branches      510        0      -510     
===========================================
- Hits         8591     6877     -1714     
+ Misses       4311     1716     -2595     
+ Partials      100        0      -100     
Flag Coverage Δ
libtiledbsoma ?
python 90.61% <ø> (ø)
r 71.23% <ø> (ø)

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

Components Coverage Δ
python_api 90.61% <ø> (ø)
libtiledbsoma ∅ <ø> (∅)

@johnkerl
Copy link
Member Author

johnkerl commented Apr 5, 2024

@eddelbuettel thank you. I'll accept that request as a follow-up task.

We need to tag 1.9.4 today and this is blocking the build. I won't be waiting on upstream today.

Happy to accommodate as follow-up.

@eddelbuettel
Copy link
Contributor

(The subject line tagging may be off as the wheel that breaks is a Python wheel.)

@eddelbuettel
Copy link
Contributor

@johnkerl I did not suggest to wait for upstream to fix it, you have a fix. But as it is a vendored file, and to save us from future surprises if we ever update the vendoring to a newer version, it may be beneficial to communicate the fix upstream.

@johnkerl
Copy link
Member Author

johnkerl commented Apr 5, 2024

[sc-44675]

Copy link

This pull request has been linked to Shortcut Story #44675: tiledbsoma 1.9.4.

@aaronwolen aaronwolen self-requested a review April 5, 2024 15:55
@johnkerl
Copy link
Member Author

johnkerl commented Apr 5, 2024

@eddelbuettel we are in agreement

@johnkerl johnkerl removed the request for review from ihnorton April 5, 2024 15:59
@johnkerl johnkerl merged commit 5b02a00 into main Apr 5, 2024
13 of 16 checks passed
@johnkerl johnkerl deleted the kerl/unbreak-wheel-builds branch April 5, 2024 16:14
github-actions bot pushed a commit that referenced this pull request Apr 5, 2024
johnkerl added a commit that referenced this pull request Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants