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

[C++] Pin flatbuffers compiler version #35497

Closed
bkietz opened this issue May 8, 2023 · 2 comments · Fixed by #38192
Closed

[C++] Pin flatbuffers compiler version #35497

bkietz opened this issue May 8, 2023 · 2 comments · Fixed by #38192
Assignees
Milestone

Comments

@bkietz
Copy link
Member

bkietz commented May 8, 2023

Describe the bug, including details regarding any error messages, version, and platform.

I ran cpp/build-support/update-flatbuffers.sh and the regenerated headers included new failing static assertions like:

/home/ben/arrow/cpp/src/generated/Message_generated.h:11:41: error: static assertion failed: Non-compatible flatbuffers version included
   11 | static_assert(FLATBUFFERS_VERSION_MAJOR == 23 &&

The probable fix is to pin the version of the flatbuffers compiler used in c++. As of google/flatbuffers#7203 the compiler inserts these static assertions which assert the version of the compiler- but our vendored flatbuffers are at 1.12.0. conda-forge has that version and after installing it I don't see failing static assertions any more.

Component(s)

C++

@pitrou
Copy link
Member

pitrou commented May 24, 2023

If we do this, we should first bump our vendored version of flatbuffers headers to a recent well-known changeset.

@pitrou
Copy link
Member

pitrou commented Oct 9, 2023

cc @felipecrv

@pitrou pitrou added this to the 15.0.0 milestone Oct 9, 2023
@felipecrv felipecrv self-assigned this Oct 11, 2023
pitrou pushed a commit that referenced this issue Oct 12, 2023
### Rationale for this change

To use a more modern version of `flatc` in Arrow.

### What changes are included in this PR?

1) Re-generating the the C++ files with a `flatc` based on the latest tag on the `flatbuffer` repo.
2) Copy the minimal set of includes to our vendored folder
3) Manually re-apply the patches that have been applied to the previous headers to fix issues

### Are these changes tested?

Tested by building everything correctly.
* Closes: #35497

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
llama90 pushed a commit to llama90/arrow that referenced this issue Oct 12, 2023
…pache#38192)

### Rationale for this change

To use a more modern version of `flatc` in Arrow.

### What changes are included in this PR?

1) Re-generating the the C++ files with a `flatc` based on the latest tag on the `flatbuffer` repo.
2) Copy the minimal set of includes to our vendored folder
3) Manually re-apply the patches that have been applied to the previous headers to fix issues

### Are these changes tested?

Tested by building everything correctly.
* Closes: apache#35497

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
JerAguilon pushed a commit to JerAguilon/arrow that referenced this issue Oct 23, 2023
…pache#38192)

### Rationale for this change

To use a more modern version of `flatc` in Arrow.

### What changes are included in this PR?

1) Re-generating the the C++ files with a `flatc` based on the latest tag on the `flatbuffer` repo.
2) Copy the minimal set of includes to our vendored folder
3) Manually re-apply the patches that have been applied to the previous headers to fix issues

### Are these changes tested?

Tested by building everything correctly.
* Closes: apache#35497

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…pache#38192)

### Rationale for this change

To use a more modern version of `flatc` in Arrow.

### What changes are included in this PR?

1) Re-generating the the C++ files with a `flatc` based on the latest tag on the `flatbuffer` repo.
2) Copy the minimal set of includes to our vendored folder
3) Manually re-apply the patches that have been applied to the previous headers to fix issues

### Are these changes tested?

Tested by building everything correctly.
* Closes: apache#35497

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…pache#38192)

### Rationale for this change

To use a more modern version of `flatc` in Arrow.

### What changes are included in this PR?

1) Re-generating the the C++ files with a `flatc` based on the latest tag on the `flatbuffer` repo.
2) Copy the minimal set of includes to our vendored folder
3) Manually re-apply the patches that have been applied to the previous headers to fix issues

### Are these changes tested?

Tested by building everything correctly.
* Closes: apache#35497

Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants