Summary: | Добавление статической библиотеки zstd | ||
---|---|---|---|
Product: | Sisyphus | Reporter: | keremet |
Component: | libzstd-devel | Assignee: | placeholder <placeholder> |
Status: | CLOSED FIXED | QA Contact: | qa-sisyphus |
Severity: | normal | ||
Priority: | P5 | CC: | glebfm, ldv, placeholder |
Version: | unstable | ||
Hardware: | x86_64 | ||
OS: | Linux |
Description
keremet
2023-08-07 10:07:25 MSK
(In reply to keremet from comment #0) > Хотелось бы, чтобы был пакет libzstd-devel-static c библиотекой для > статической линковки А что будет с ней линковаться? (Ответ для Gleb F-Malinovskiy на комментарий #1) > А что будет с ней линковаться? Библиотека нужна, чтобы была возможность скомпилировать форк Greenplum от Arenadata под Альт после вот этого коммита https://github.com/arenadata/gpdb/commit/7185bf85ea8f4abdfdeb08d5a53180ea2f389728 (In reply to keremet from comment #2) > (Ответ для Gleb F-Malinovskiy на комментарий #1) > > А что будет с ней линковаться? > > Библиотека нужна, чтобы была возможность скомпилировать форк Greenplum от > Arenadata под Альт после вот этого коммита > https://github.com/arenadata/gpdb/commit/ > 7185bf85ea8f4abdfdeb08d5a53180ea2f389728 Я думаю, что они неправы, когда называют эту фичу экспериментальной. Она не экспериментальная, но её наличие зависит от того, как собран zstd. В нашем случае libzstd собрана таким образом, что ZSTD_createDStream_advanced доступен в динамической библиотеке, её в этом случае и стоит использовать (т.е. просто сделать патч, который откатывает изменение про :libzstd.a). Т.о., я не вижу смысла собирать статическую библиотеку. Необходимость использовать статическую линковку описана в zstd.h: Advanced experimental APIs should never be used with a dynamically-linked library. They are not "stable"; their definitions or signatures may change in the future. Only static linking is allowed. https://git.altlinux.org/gears/z/zstd.git?p=zstd.git;a=blob;f=lib/zstd.h;h=e5c3f8b68b72e926b0720d7a2efec3f94792d92e;hb=69031eaa618e8271d78267f22bfa9c0d53a6d708#l101 и тут https://git.altlinux.org/gears/z/zstd.git?p=zstd.git;a=blob;f=lib/zstd.h;h=e5c3f8b68b72e926b0720d7a2efec3f94792d92e;hb=69031eaa618e8271d78267f22bfa9c0d53a6d708#l1161 Функция ZSTD_createDStream_advanced отмечена как ZSTDLIB_STATIC_API, поэтому необходимо собирать Greenplum со статической линковкой, иначе есть риск, что обновление пакета с libzstd.so поломает Greenplum. Во многих других популярных дистрибутивах libzstd.a можно установить из пакетов (https://pkgs.org/search/?q=libzstd.a), и было бы очень здорово, если бы такая же возможность была доступна и в Альте. Патч я бы мог подготовить сам. Прошу только его принять и не откатывать (In reply to keremet from comment #4) > Необходимость использовать статическую линковку описана в zstd.h: > Advanced experimental APIs should never be used with a dynamically-linked > library. They are not "stable"; their definitions or signatures may change > in the future. Only static linking is allowed. > https://git.altlinux.org/gears/z/zstd.git?p=zstd.git;a=blob;f=lib/zstd.h; > h=e5c3f8b68b72e926b0720d7a2efec3f94792d92e; > hb=69031eaa618e8271d78267f22bfa9c0d53a6d708#l101 Хм. Видимо, это значит, что динамическая библиотека вообще не должна эти символы экспортировать? (Ответ для Gleb F-Malinovskiy на комментарий #5) > Хм. Видимо, это значит, что динамическая библиотека вообще не должна эти > символы экспортировать? Разработчики пишут "should", значит рекомендуют не делать динамическую линковку, но не запрещают. Если кому-то очень хочется, то пусть линкуют динамически, главное, чтобы понимали, что после обновления библиотеки что-то может перестать работать. (In reply to keremet from comment #6) > Разработчики пишут "should", Там всё же "should never". (Ответ для Gleb F-Malinovskiy на комментарий #5) > Хм. Видимо, это значит, что динамическая библиотека вообще не должна эти > символы экспортировать? Защита построена на том, что компиляция не будет выполнена без объявления макроса ZSTD_STATIC_LINKING_ONLY. Так программист как бы подтверждает, что готов на риск. ``` #include <zstd.h> int main() { ZSTD_customMem c; ZSTD_createDStream_advanced(c); return 0; } ``` При компиляции такой программки будут ошибки ``` $ gcc 1.c -lzstd 1.c: In function ‘main’: 1.c:6:2: error: unknown type name ‘ZSTD_customMem’ 6 | ZSTD_customMem c; | ^~~~~~~~~~~~~~ 1.c:7:2: warning: implicit declaration of function ‘ZSTD_createDStream_advanced’; did you mean ‘ZSTD_createDStream’? [-Wimplicit-function-declaration] 7 | ZSTD_createDStream_advanced(c); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ | ZSTD_createDStream ``` А если компилировать так ``` gcc 1.c -lzstd -DZSTD_STATIC_LINKING_ONLY ```, то все соберется успешно. Да, это всё понятно. Но я считаю, что для дистрибутива будет лучше, если помимо сборки статической библиотеки мы уберём такие символы из динамической библиотеки совсем. (Ответ для Gleb F-Malinovskiy на комментарий #9) > Но я считаю, что для дистрибутива будет лучше, если помимо сборки > статической библиотеки мы уберём такие символы из динамической библиотеки > совсем. Если что-то убрать из динамической библиотеки, то есть риск, что тогда у кого-то что-то сломается раньше, чем апстрим zstd изменит API. Но так как наш Greenplum будет работать, сильно возражать не стану. Согласен на любой вариант (In reply to keremet from comment #10) > Если что-то убрать из динамической библиотеки, то есть риск, что тогда у > кого-то что-то сломается раньше, чем апстрим zstd изменит API. Если мы уберём символы, то мы сразу увидим, если ими кто-то пользовался по unmet-ам через set:versions. > Но так как наш Greenplum будет работать, сильно возражать не стану. Согласен на любой вариант Окей. Возьмётесь сделать эти два изменения? * добавление libzstd.a в виде пакета libzstd-devel-static (не забудьте добавить комментарий и написать в changelog, для какого пакета понадобился static-подпакет); * убирание ZSTD_STATIC_LINKING_ONLY из динамической библиотеки. (Ответ для Gleb F-Malinovskiy на комментарий #11) > Окей. Возьмётесь сделать эти два изменения? Да Кажется, апстрим никогда так не делал. dictBuilder/cover.c:1122:21: warning: redeclaration of 'ZDICT_optimizeTrainFromBuffer_cover' with different visibility (old visibility preserved) dictBuilder/cover.c:736:21: warning: redeclaration of 'ZDICT_trainFromBuffer_cover' with different visibility (old visibility preserved) dictBuilder/fastcover.c:549:1: warning: redeclaration of 'ZDICT_trainFromBuffer_fastCover' with different visibility (old visibility preserved) dictBuilder/fastcover.c:618:1: warning: redeclaration of 'ZDICT_optimizeTrainFromBuffer_fastCover' with different visibility (old visibility preserved) Ну и pzstd не собирается с ожидаемой диагностикой: ld: /usr/src/tmp/ccXXXXXX.ltrans1.ltrans.o:/usr/src/RPM/BUILD/zstd-1.5.5-alt1/contrib/pzstd/Options.h:64: undefined reference to `ZSTD_adjustCParams' ld: pzstd: hidden symbol `ZSTD_getParams' isn't defined ld: final link failed: bad value Похоже, что они написали эти комментарии не для себя. Создал задачу https://git.altlinux.org/tasks/327749/ В ней сборка завершилась с ошибкой на тестах. Ошибка воспроизводится стабильно. С точно такой же ошибкой сборка завершается и на текущем теге 1.5.5-alt1. Похоже, я что-то не так делаю, если пакет для 1.5.5-alt1 все-таки как-то собрался. Подскажите, пожалуйста, что можно сделать. (In reply to Gleb F-Malinovskiy from comment #9) > Да, это всё понятно. > Но я считаю, что для дистрибутива будет лучше, если помимо сборки > статической библиотеки мы уберём такие символы из динамической библиотеки > совсем. Из-за pzstd это превращается в сложную задачу, которую не факт, что кто-то захочет и сможет решить. (In reply to Dmitry V. Levin from comment #15) > Из-за pzstd это превращается в сложную задачу, которую не факт, что кто-то > захочет и сможет решить. Ну, попробовать стоило! :) zstd-1.5.5-alt2 -> sisyphus: Wed Aug 23 2023 Dmitry V. Levin <ldv@altlinux> 1.5.5-alt2 - Packaged static library (by Andrey Sokolov; closes: #47144). |