facebook / mysql-5.6

Facebook's branch of the Oracle MySQL database. This includes MyRocks.
http://myrocks.io
Other
2.46k stars 711 forks source link

Fix vectordb build, including macOS portability fixes #1447

Closed laurynas-biveinis closed 2 months ago

laurynas-biveinis commented 2 months ago

Fix some vectordb build issues, both general and macOS-specific:

laurynas-biveinis commented 2 months ago

This PR allows me to build on macOS but not Linux, where I haven't figured out which flavor of openmp to install and how to point to it yet, but this unblocks my current work alread. The "-lgfortran"-removing bit might be build-breaking, let me know, and I will revert it.

facebook-github-bot commented 2 months ago

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

luqun commented 2 months ago

This PR allows me to build on macOS but not Linux, where I haven't figured out which flavor of openmp to install and how to point to it yet, but this unblocks my current work alread. The "-lgfortran"-removing bit might be build-breaking, let me know, and I will revert it.

yep. The "-lgfortran"-removing cause build-breaking, could you revert it?

laurynas-biveinis commented 2 months ago

This PR allows me to build on macOS but not Linux, where I haven't figured out which flavor of openmp to install and how to point to it yet, but this unblocks my current work alread. The "-lgfortran"-removing bit might be build-breaking, let me know, and I will revert it.

yep. The "-lgfortran"-removing cause build-breaking, could you revert it?

I will. Could you show the full linker error? Maybe I can replace it with something better at the same time

luqun commented 2 months ago

This PR allows me to build on macOS but not Linux, where I haven't figured out which flavor of openmp to install and how to point to it yet, but this unblocks my current work alread. The "-lgfortran"-removing bit might be build-breaking, let me know, and I will revert it.

yep. The "-lgfortran"-removing cause build-breaking, could you revert it?

I will. Could you show the full linker error? Maybe I can replace it with something better at the same time

ld.lld: error: undefined symbol: _gfortran_concat_string
>>> referenced by dgesvd.f
>>>               dgesvd.o:(dgesvd_) in archive /OpenBLAS/0.3.20/lib/libopenblas_pic.a
>>> referenced by dgesvd.f
>>>               dgesvd.o:(dgesvd_) in archive /OpenBLAS/0.3.20/lib/libopenblas_pic.a
>>> referenced by dormbr.f
>>>               dormbr.o:(dormbr_) in archive /OpenBLAS/0.3.20/lib/libopenblas_pic.a
facebook-github-bot commented 2 months ago

@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.

laurynas-biveinis commented 2 months ago

@luqun, replaced -lgfortran with a CMake variable initialized from gfortran --print-file-name libgfortran.a, add a missing standard C++ include as an LLVM 14+ fix, and rebased

facebook-github-bot commented 2 months ago

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

luqun commented 2 months ago

Another linker error:

ld.lld: error: undefined symbol: quadmath_snprintf
>>> referenced by write_float.def:922 (../.././libgfortran/io/write_float.def:922)
>>>               write.o:(determine_en_precision) in archive /libgcc/11.x/lib/libgfortran.a
>>> referenced by write_float.def:1125 (../.././libgfortran/io/write_float.def:1125)
>>>               write.o:(get_float_string) in archive /libgcc/11.x/lib/libgfortran.a
>>> referenced by write_float.def:1125 (../.././libgfortran/io/write_float.def:1125)
>>>               write.o:(get_float_string) in archive /ibgcc/11.x/lib/libgfortran.a
>>> referenced 1 more times
laurynas-biveinis commented 2 months ago

@luqun I can push another fix that looks up libquadmath.a the same way, but this error is suspicious if your original plain -lgfortran was not failing. It suggests that the default -lgfortran and gfortran executable are using different toolchains, and I cannot confirm that remotely unless I push some CMake message calls and ask you to run them. So we have three options: 1) push gfortran looking up libquadmath.a; 2) push diagnostics to confirm whether -lgfortran and gfortran point to identical or different toolchains; 3) revert to your original -lgfortran, and me workarounding the issue. What do you think?

luqun commented 2 months ago

Maybe Let's try 2) first, if not, then 1).. if still doesn't work, use 3)

facebook-github-bot commented 2 months ago

@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.

laurynas-biveinis commented 2 months ago

My thinking was incorrect. I am using standard system toolchain, that has no notion of Fortran libraries, with the GCC installed as a third-party toolchain. I have fixed it here with a build workaround, and the existing -lgtfortran is correct, remove the patch, ready for review

facebook-github-bot commented 2 months ago

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 2 months ago

This pull request has been merged in facebook/mysql-5.6@f5ddb37ed297c1055a0f8398bd018d1f567a2772.