Bug 47144 - Добавление статической библиотеки zstd
Summary: Добавление статической библиотеки zstd
Status: CLOSED FIXED
Alias: None
Product: Sisyphus
Classification: Development
Component: libzstd-devel (show other bugs)
Version: unstable
Hardware: x86_64 Linux
: P5 normal
Assignee: placeholder@altlinux.org
QA Contact: qa-sisyphus
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-08-07 10:07 MSK by keremet
Modified: 2023-08-25 02:40 MSK (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description keremet 2023-08-07 10:07:25 MSK
Хотелось бы, чтобы был пакет libzstd-devel-static c библиотекой для статической линковки
Comment 1 Gleb F-Malinovskiy 2023-08-07 10:11:13 MSK
(In reply to keremet from comment #0)
> Хотелось бы, чтобы был пакет libzstd-devel-static c библиотекой для
> статической линковки

А что будет с ней линковаться?
Comment 2 keremet 2023-08-07 10:23:28 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #1)
> А что будет с ней линковаться?

Библиотека нужна, чтобы была возможность скомпилировать форк Greenplum от Arenadata под Альт после вот этого коммита
https://github.com/arenadata/gpdb/commit/7185bf85ea8f4abdfdeb08d5a53180ea2f389728
Comment 3 Gleb F-Malinovskiy 2023-08-07 10:39:22 MSK
(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).
Т.о., я не вижу смысла собирать статическую библиотеку.
Comment 4 keremet 2023-08-08 11:38:04 MSK
Необходимость использовать статическую линковку описана в 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), и было бы очень здорово, если бы такая же возможность была доступна и в Альте. 

Патч я бы мог подготовить сам. Прошу только его принять и не откатывать
Comment 5 Gleb F-Malinovskiy 2023-08-08 12:06:31 MSK
(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
Хм.  Видимо, это значит, что динамическая библиотека вообще не должна эти символы экспортировать?
Comment 6 keremet 2023-08-08 22:01:31 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #5)
> Хм.  Видимо, это значит, что динамическая библиотека вообще не должна эти
> символы экспортировать?

Разработчики пишут "should", значит рекомендуют не делать динамическую линковку, но не запрещают. Если кому-то очень хочется, то пусть линкуют динамически, главное, чтобы понимали, что после обновления библиотеки что-то может перестать работать.
Comment 7 Gleb F-Malinovskiy 2023-08-08 22:07:14 MSK
(In reply to keremet from comment #6)
> Разработчики пишут "should",

Там всё же "should never".
Comment 8 keremet 2023-08-08 22:24:16 MSK
(Ответ для 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 ```, то все соберется успешно.
Comment 9 Gleb F-Malinovskiy 2023-08-08 22:37:47 MSK
Да, это всё понятно.
Но я считаю, что для дистрибутива будет лучше, если помимо сборки статической библиотеки мы уберём такие символы из динамической библиотеки совсем.
Comment 10 keremet 2023-08-09 11:48:33 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #9)
> Но я считаю, что для дистрибутива будет лучше, если помимо сборки
> статической библиотеки мы уберём такие символы из динамической библиотеки
> совсем.

Если что-то убрать из динамической библиотеки, то есть риск, что тогда у кого-то что-то сломается раньше, чем апстрим zstd изменит API. Но так как наш Greenplum будет работать, сильно возражать не стану. Согласен на любой вариант
Comment 11 Gleb F-Malinovskiy 2023-08-09 11:54:46 MSK
(In reply to keremet from comment #10)
> Если что-то убрать из динамической библиотеки, то есть риск, что тогда у
> кого-то что-то сломается раньше, чем апстрим zstd изменит API.
Если мы уберём символы, то мы сразу увидим, если ими кто-то пользовался по unmet-ам через set:versions.

> Но так как наш Greenplum будет работать, сильно возражать не стану. Согласен на любой вариант
Окей.  Возьмётесь сделать эти два изменения?
* добавление libzstd.a в виде пакета libzstd-devel-static (не забудьте добавить комментарий и написать в changelog, для какого пакета понадобился static-подпакет);
* убирание ZSTD_STATIC_LINKING_ONLY из динамической библиотеки.
Comment 12 keremet 2023-08-09 12:09:51 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #11)
> Окей.  Возьмётесь сделать эти два изменения?

Да
Comment 13 Dmitry V. Levin 2023-08-09 12:26:15 MSK
Кажется, апстрим никогда так не делал.

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

Похоже, что они написали эти комментарии не для себя.
Comment 14 keremet 2023-08-23 21:35:31 MSK
Создал задачу https://git.altlinux.org/tasks/327749/
В ней сборка завершилась с ошибкой на тестах. Ошибка воспроизводится стабильно. С точно такой же ошибкой сборка завершается и на текущем теге 1.5.5-alt1. Похоже, я что-то не так делаю, если пакет для 1.5.5-alt1 все-таки как-то собрался. Подскажите, пожалуйста, что можно сделать.
Comment 15 Dmitry V. Levin 2023-08-24 11:11:49 MSK
(In reply to Gleb F-Malinovskiy from comment #9)
> Да, это всё понятно.
> Но я считаю, что для дистрибутива будет лучше, если помимо сборки
> статической библиотеки мы уберём такие символы из динамической библиотеки
> совсем.

Из-за pzstd это превращается в сложную задачу, которую не факт, что кто-то захочет и сможет решить.
Comment 16 Gleb F-Malinovskiy 2023-08-24 11:27:48 MSK
(In reply to Dmitry V. Levin from comment #15)
> Из-за pzstd это превращается в сложную задачу, которую не факт, что кто-то
> захочет и сможет решить.

Ну, попробовать стоило! :)
Comment 17 Repository Robot 2023-08-25 02:40:04 MSK
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).