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

[pybindings] Fix linking on Mac #13766

Merged
merged 1 commit into from
Nov 6, 2020

Conversation

vicpopov
Copy link
Contributor

Mac has different linker defaults than linux, and by default it
does not allow undefined symbols to be left in resulting pybinding
library.
To allow it, we specifically pass linker argument when linking final
pybinding object.

This is done to avoid linking excessively to libpythonX.Y since at the
time pybinding is loaded into Python process, all libpython symbols
would be immediately available.

Problem was introduced in #13695, after which all Mac pybindings builds were broken.

References:
Linux: https://linux.die.net/man/1/ld, section

--allow-shlib-undefined
--no-allow-shlib-undefined
Allows or disallows undefined symbols in shared libraries. This switch is similar to --no-undefined except that it determines the behaviour when the undefined symbols are in a shared library rather than a regular object file. It does not affect how undefined symbols in regular object files are handled.
The default behaviour is to report errors for any undefined symbols referenced in shared libraries if the linker is being used to create an executable, but to allow them if the linker is being used to create a shared library.

The reasons for allowing undefined symbol references in shared libraries specified at link time are that:

- A shared library specified at link time may not be the same as the one that is available at load time, so the symbol might actually be resolvable at load time.
- There are some operating systems, eg BeOS and HPPA , where undefined symbols in shared libraries are normal.

The BeOS kernel for example patches shared libraries at load time to select whichever function is most appropriate for the current architecture. This is used, for example, to dynamically select an appropriate memset function.

MacOS: https://www.unix.com/man-page/osx/1/ld/, section

   Two-level namespace
     By default all references resolved to a dynamic library record the library to which they were resolved. At runtime, dyld uses that informa-
     tion to directly resolve symobls.	The alternative is to use the -flat_namespace option.  With flat namespace, the library is not recorded.
     At runtime, dyld will search each dynamic library in load order when resolving symbols. This is slower, but more like how other operating
     systems resolve symbols.
...
   Dynamic libraries undefines
     When linking for two-level namespace, ld does not verify that undefines in dylibs actually exist.	But when linking for flat namespace, ld
     does check that all undefines from all loaded dylibs have a matching definition.  This is sometimes used to force selected functions to be
     loaded from a static library.
...
     -undefined treatment
		 Specifies how undefined symbols are to be treated. Options are: error, warning, suppress, or dynamic_lookup.  The default is
		 error.

Somehow currently on Mac our linking is forced to be for flat namespace, and we can't avoid check for all symbols to be defined.
Option -undefined dynamic_lookup eases this check and allows linking to succeed.
Alternative would be to try to use option -bundle_loader, but that might be much more complicated.

@@ -32,7 +32,7 @@ omim_link_libraries(
link_qt5_core(${PROJECT_NAME})

if (PLATFORM_MAC)
omim_link_libraries(${PROJECT_NAME} ${LIBZ})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Линковка с libz - также избыточная (также странно, что нужна только на маке) - решается общим средством разрешения undefined символов.

Mac has different linker defaults than linux, and by default it
does not allow undefined symbols to be left in resulting pybinding
library.
To allow it, we specifically pass linker argument when linking final
pybinding object.

This is done to avoid linking excessively to libpythonX.Y since at the
time pybinding is loaded into Python process, all libpython symbols
would be immediately available.
@vicpopov vicpopov changed the title Fix linking pybindings on Mac [pybindings] Fix linking on Mac Sep 30, 2020
@milchakov
Copy link

да, кажется -undefined вполне удобно использовать, но какие есть альтернативы dynamic_lookup? Что-то с ходу не нашел документацию на возможные значения.

@milchakov
Copy link

@vicpopov написал в личку, что ответ на мой вопрос уже есть в описании реквеста.

@milchakov
Copy link

а как dynamic_lookup работает? ищет во всех путях, которые есть в переменных окружения?

@vicpopov
Copy link
Contributor Author

vicpopov commented Oct 1, 2020

а как dynamic_lookup работает? ищет во всех путях, которые есть в переменных окружения?

Насколько я понимаю, это инструкция линкеру забить на разрешение всех символов ввиду обещания "они будут разрешены динамически в рантайме". Чем это отличается от опции suppress - не очень понимаю.

@milchakov
Copy link

а как dynamic_lookup работает? ищет во всех путях, которые есть в переменных окружения?

Насколько я понимаю, это инструкция линкеру забить на разрешение всех символов ввиду обещания "они будут разрешены динамически в рантайме". Чем это отличается от опции suppress - не очень понимаю.

Понял. dynamic_lookup типо значит не сейчас искать а потом, когда будет библиотека подключаться в исполняемый файл... Как это обычно и работает везде... Спасибо.

@milchakov
Copy link

может быть добавлять эти параметры линковщика не для каждой библиотеки, а всегда, если активирован режим сборки pybindings?  В каком-то общем месте.

@vicpopov
Copy link
Contributor Author

vicpopov commented Oct 1, 2020

может быть добавлять эти параметры линковщика не для каждой библиотеки, а всегда, если активирован режим сборки pybindings?  В каком-то общем месте.

Вообще да, но я не знаю как это сделать. Это надо будет сильно порефакторить cmake-файлы, нужен спец по этому.

@milchakov
Copy link

кажется, есть три варианта:

  • добавить доп код в omim_link_libraries;
  • добавить доп метод типа omim_link_pybindings_libraries и вызывать его в библиотеках питонячих
  • добавить доп флаги в setup.py

@milchakov
Copy link

это из простого, что мне, как не особо разбирающемуся в этом человеку, пришло в голову.

@milchakov
Copy link

@mpimenov PTAL

Copy link
Contributor

@mpimenov mpimenov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mpimenov mpimenov merged commit 256c548 into mapsme:master Nov 6, 2020
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

Successfully merging this pull request may close these issues.

3 participants