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

building 3.0.12 using -std=c++17 #8395

Open
real-dam opened this issue Jan 16, 2025 · 9 comments
Open

building 3.0.12 using -std=c++17 #8395

real-dam opened this issue Jan 16, 2025 · 9 comments

Comments

@real-dam
Copy link
Contributor

Hi,

I am having trouble building the Debian package of Firebird 3.0.12.33787 with -std=c++17. The reason I need to use C++17 is that ICU v76.1 requires it and is expected to be part of the next stable release (no timeline yet).

First I tried patching various prefixes to pass -std=c++17 (patch). Then I had to patch out throw(...) declarations since these are not allowed in C++17 (patch).

Now the build proceeds until the just-built isql and gfix are invoked and they fail with what looks like memory corruption/thread synchronization errors.

Full build log with --enable-debug and stack traces of the failures is available here.

Thanks for any ideas.

@aafemt
Copy link
Contributor

aafemt commented Jan 16, 2025

ICU is a dynamic library and has only C interface. Even from the sources included into the Firebird tree it is built completely separately. Why did you decide to modify old Firebird build to comply with new ICU rules?

@real-dam
Copy link
Contributor Author

According to its authors, ICU requires C++17 since version 75.

The Debian package firebird3.0 uses the system-wide ICU library. When building the package from source and at run-time. When the system-wide ICU library is updated, the Firebird packages still need to be buildable so that bugs can be fixed and new builds released.

Debian is in the process of upgrading ICU from 72 to 76, and firebird3.0 packages stand in the way. I am looking for a way to make this possible without removing firebird3.0 from the next stable release.

@aafemt
Copy link
Contributor

aafemt commented Jan 16, 2025

AFAIU, it is ICU code that requires C++17. Firebird is using ICU binaries. Binaries has no such requirement so Firebird also doesn't require C++17 to use ICU.

@asfernandes
Copy link
Member

Will you will convert any C program using ICU to C++17? No.
Firebird uses ICU from its C interface only, so this should not be a requirement.

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 16, 2025 via email

@real-dam
Copy link
Contributor Author

Thanks, @AlexPeshkoff, I will test with 37c1bca applied and report back.

@real-dam
Copy link
Contributor Author

That still failed, but adding -DU_SHOW_CPLUSPLUS_HEADER_API=0 to it makes the build pass. May be something specific to ICU 76.1 (i.e. not needed for 75.1)

@AlexPeshkoff
Copy link
Member

AlexPeshkoff commented Jan 16, 2025

Somewhy I'm not too much surprised that they've changed controlling define name. Can you prepare PR?

@real-dam
Copy link
Contributor Author

Sure. For which branch(es)?

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

No branches or pull requests

4 participants