ci: add RelWithDebInfo build and fix free-nonheap-object error#812
ci: add RelWithDebInfo build and fix free-nonheap-object error#812zhjwpku wants to merge 3 commits into
Conversation
| FromJsonMap<std::shared_ptr<SnapshotRef>>(json, kRefs, SnapshotRefFromJson)); | ||
| ICEBERG_ASSIGN_OR_RAISE(auto refs, FromJsonMap<std::shared_ptr<SnapshotRef>>( | ||
| json, kRefs, SnapshotRefFromJson)); | ||
| table_metadata->refs = std::move(refs); |
There was a problem hiding this comment.
seems like a more readable version of previous version, however it seems disconnected from the change discussed in this PR?
There was a problem hiding this comment.
Yeah, this is related to the new RelWithDebInfo coverage: that build exposed the -Werror=free-nonheap-object failure, so this change avoids assigning directly through ICEBERG_ASSIGN_OR_RAISE and fixes the issue surfaced by the new CI job. I'll update the PR title to make that relationship clearer.
| name: AMD64 Ubuntu 26.04 (${{ matrix.cmake_build_type }}) | ||
| runs-on: ubuntu-26.04 | ||
| timeout-minutes: 30 | ||
| timeout-minutes: 60 |
There was a problem hiding this comment.
Is my understanding correct that the timeout was doubled becuase now for ubuntu 2 build configurations are going to be executed?
There was a problem hiding this comment.
Not really. Each build runs as its own job. I doubled that because this is a cold build, no cache can be utilized. Also, RelWithDebInfo may take longer since it's an -O2 build. It shouldn't take this long once it's merged into main and sccache starts taking effect.
zhjwpku
changed the title
2 participants
Adding a RelWithDebInfo build revealed a build error. This PR fixes the issue.