Bug 42475 - [4.0] join kovalev@
Summary: [4.0] join kovalev@
Status: ASSIGNED
Alias: None
Product: Team Accounts
Classification: Development
Component: join (show other bugs)
Version: unspecified
Hardware: x86_64 Linux
: P5 normal
Assignee: Gleb F-Malinovskiy
QA Contact: Andrey Cherepanov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-18 10:31 MSK by Vasiliy Kovalev
Modified: 2024-03-13 11:59 MSK (History)
8 users (show)

See Also:


Attachments
ssh public key (102 bytes, application/vnd.ms-publisher)
2022-04-18 10:31 MSK, Vasiliy Kovalev
no flags Details
gpg public key (3.08 KB, application/vnd.ms-publisher)
2022-04-18 10:33 MSK, Vasiliy Kovalev
no flags Details
gpg public key (5.29 KB, application/vnd.ms-publisher)
2022-04-18 12:11 MSK, Vasiliy Kovalev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vasiliy Kovalev 2022-04-18 10:31:16 MSK
Created attachment 10590 [details]
ssh public key

Имя пользователя: kovalev
Почта для пересылки: kovalev.temp@yandex.ru
Намерения: хочу научиться собирать пакеты и принять участие в разработке
Имя ментора: nickel

Подписка: nickel@altlinux.org
Comment 1 Vasiliy Kovalev 2022-04-18 10:33:29 MSK
Created attachment 10591 [details]
gpg public key

gpg public key
Comment 2 Николай Костригин 2022-04-18 11:48:13 MSK
Менторство подтверждаю.

Почту для пересылки с временной рекомендую заменить на постоянную.
SSH ключ выглядит валидным.
GPG ключ договорились сделать со подключом для подписи с истекающим сроком действия.

Секретарю просьба дождаться обозначенных изменений.
Comment 3 Vasiliy Kovalev 2022-04-18 12:11:57 MSK
Created attachment 10593 [details]
gpg public key

заменил публичный gpg ключ
Comment 4 Vasiliy Kovalev 2022-04-18 12:12:22 MSK
почта для пересылки: kovalevvv@basealt.ru
Comment 5 Gleb F-Malinovskiy 2022-04-19 16:25:40 MSK
(In reply to Vasiliy Kovalev from comment #0)
> Created attachment 10590 [details]
> ssh public key
Ok.

(In reply to Vasiliy Kovalev from comment #3)
> Created attachment 10593 [details]
> gpg public key
Ok.
Comment 6 Николай Костригин 2022-05-11 19:36:14 MSK
Можно переходить к следующему шагу.
Comment 7 Gleb F-Malinovskiy 2022-05-17 20:30:37 MSK
(In reply to Vasiliy Kovalev from comment #4)
> почта для пересылки: kovalevvv@basealt.ru

Прошу прощения, я прозевал это сообщение.
Проверьте почту, пожалуйста.
Comment 8 Vasiliy Kovalev 2022-05-18 12:02:28 MSK
я проверил почту, рассылка работает по старому адресу kovalev.temp@yandex.ru

прошу изменить адрес почты для рассылки на kovalevvv@basealt.ru
Comment 9 Gleb F-Malinovskiy 2022-05-18 18:50:00 MSK
(In reply to Vasiliy Kovalev from comment #8)
> я проверил почту, рассылка работает по старому адресу kovalev.temp@yandex.ru

Рассылка чего работает?
 
> прошу изменить адрес почты для рассылки на kovalevvv@basealt.ru

Да, я увидел ваше пожелание.
Comment 10 Gleb F-Malinovskiy 2022-05-18 18:57:10 MSK
Почта подтверждена.
Comment 11 Vasiliy Kovalev 2022-05-18 19:02:50 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #9)
> Рассылка чего работает?
уведомления о действиях в bugzilla приходят на почту. думал это рассылка
Comment 12 Gleb F-Malinovskiy 2022-05-18 19:07:18 MSK
(In reply to Vasiliy Kovalev from comment #11)
> (Ответ для Gleb F-Malinovskiy на комментарий #9)
> > Рассылка чего работает?
> уведомления о действиях в bugzilla приходят на почту. думал это рассылка

Не, речь идёт про адрес, на который будет перенаправляться почта, которая будет приходить на ваш адрес в домене @altlinux.org .
Comment 13 Gleb F-Malinovskiy 2022-05-18 19:10:21 MSK
ssh ключ на gitery.alt зарегистрирован.
Адрес для пересылки создан.

T/J/S -> 2.3.
Comment 14 Николай Костригин 2022-06-24 15:37:38 MSK
Можно перейти к следующему этапу.
Comment 15 Gleb F-Malinovskiy 2022-06-28 12:42:47 MSK
ssh ключ на gyle.alt зарегистрирован.
Пакет alt-gpgkeys обновлён.

T/J/S -> 3.5.
Comment 17 Николай Костригин 2022-07-06 19:12:59 MSK
Полагаю, можно призвать второго ментора для оценки наработок кандидата.
Comment 18 Gleb F-Malinovskiy 2022-07-11 18:59:37 MSK
(In reply to Vasiliy Kovalev from comment #16)
> mdcat
> http://git.altlinux.org/people/kovalev/packages/mdcat.git
> (не собирается для архитектуры ppc64le)

Хорошая практика -- писать комментарий о том, почему та или иная архитектура исключается (и сами не забудете и другим будет понятно).
В данном случае дело в https://github.com/briansmith/ring/issues/389
Comment 19 Vasiliy Kovalev 2022-07-12 12:05:25 MSK
Благодарю за замечание, поправил спек.
Comment 20 Gleb F-Malinovskiy 2022-09-15 18:11:22 MSK
Призван ещё один человек (bircoph@) для независимой оценки готовности кандидата.

T/J/S -> 4.2.
Comment 21 Andrew Savchenko 2022-09-16 12:49:56 MSK
Добрый день!

Есть следующие вопросы:

(In reply to Vasiliy Kovalev from comment #16)
> http://git.altlinux.org/people/kovalev/packages/cmatrix.git

1.1) Зачем:
BuildRequires(pre): rpm-build-licenses
?

В spec макросы из этого пакета не используются.

1.2) configure вот так вот ругается:

*** neither the consolechars nor the setfont program was not found.  You                                                                                      
*** will not be able to see the characters in the matrix font in the                                                                                          
*** console without this program (it may still work in xterms). If you are                                                                                    
*** using Linux, the package containing this program is usually called                                                                                        
*** kbd, kbd-utils, or console-utils             

Почему kbd (предоставляющий setfont) нет в зависимостях пакета?

> http://git.altlinux.org/people/kovalev/packages/fwts.git

2.1) Аналогично 1.1)

2.2) Зачем этот костыль:
find %buildroot -type f -name "*.a" -delete
Если у configure есть опция --disable-static?

2.3)
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: AcpiEvaluateObject                                                                  
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: fwts_acpica_sem_count_clear                                                         
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: AcpiGetName                                                                         
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: fwts_acpica_get_object_names                                                        
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: fwts_acpica_init                                                                    
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: AcpiGetHandle                                                                       
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: fwts_acpica_set_fwts_framework                                                      
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: fwts_acpica_deinit                                                                  
verify-elf: WARNING: ./usr/lib64/fwts/libfwts.so.1.0.0: undefined symbol: fwts_acpica_sem_count_get                 

Все эти символы определены в libfwtsacpica.so, но линковки с ней нет, в libfwts.la тоже не указана.

Это серьёзная проблема, такой пакет не следует пропускать в репозиторий.

2.4)
lib.req: WARNING: /usr/src/tmp/fwts-buildroot/usr/lib64/fwts/libfwts.so.1.0.0: underlinked libraries: /lib64/libz.so.1
lib.req: WARNING: /usr/src/tmp/fwts-buildroot/usr/lib64/fwts/libfwts.so: underlinked libraries: /lib64/libz.so.1
lib.req: WARNING: /usr/src/tmp/fwts-buildroot/usr/lib64/fwts/libfwts.so.1: underlinked libraries: /lib64/libz.so.1

Это уже не критично, но лучше явно указать -lz.

> http://git.altlinux.org/people/kovalev/packages/VeraCrypt.git

3.1) Аналогично 1.1)

3.2) Я не понимаю, зачем комбинируется использование тарбола по тегу и наложение патчей отдельными файлами через %patch. Безусловно, так делать можно, но мне представляется избыточным: если используются файлы с патчами, то сборка тарбола по тегу и вообще .gear/tags не нужны; если есть желание работать с тегами, то изменения следует делать прямо коммитами в репозитории, обычно для этого отдельную ветку используют.

3.3)
056-debuginfo.brp: WARNING: You have 1 stripped ELF objects. Please compile with debugging information!
056-debuginfo.brp: WARNING: An excerpt from the list of affected files follows:
  ./usr/bin/veracrypt

Нужно исправить. Debuginfo важен, особенно для такого проекта.

Сходу по логу не видно, где это происходит. Рекомендую добавить VERBOSE=1 к опциям make: там сразу становится видно, где удалить strip, а если почитать Makefile, то очевидно как это проще всего сделать.

3.4) Это больше вопрос к апстриму, но всё же интересно мнение мейнтенера:
In function 'twofish_set_key':
cc1: warning: iteration 4 invokes undefined behavior [-Waggressive-loop-optimizations]
../Crypto/Twofish.c:626:23: note: within this loop
  626 |         for (i = 0; i != 40; i += 2)
      |                     ~~^~~~~

Сходу я в 4-й итерации не увидел проблем, но там достаточно странный код, в т.ч. не понятно, почему критерий останова i != 40 вместо i > 40. Поскольку код по криптографии в достаточно важном месте, то такое предупреждение не следует игнорировать и с ним нужно разобраться. Может быть и ложное предупреждение, но нужно смотреть сгенерированный код, чтоб точно понять, что там творится.

3.5) Неверно указана лицензия для пакета: там не только Apache-2.0, но и TrueCrypt-3.0. Притом обе сразу, почитайте хедеры исходников и файл License.txt.

Последнюю нужно будет добавить в пакет common-licenses.

> http://git.altlinux.org/people/kovalev/packages/mdcat.git

4.1)
   Compiling fehler-macros v1.0.0
warning: Failed to build manpage: No such file or directory (os error 2)

Если посмотреть содержимое man mdcat, то там ерунда, а не man: просто переименованный mdcat.1.adoc вместо корректно отфоматированного man.

Мейнтенер почему-то вместо решения проблемы сборки man просто переименовал исходник в mdcat.1

4.2) У пакета есть тесты, но spec их не использует. Следует реализовать секцию %check.

> airsane
> http://git.altlinux.org/people/kovalev/packages/airsane.git
> https://git.altlinux.org/gears/a/airsane.git?p=airsane.git;a=commit;
> h=23e117705f3854c1352ca7b0c7af15fe2a0e3105
> плюс принято изменение в upstream
> https://github.com/SimulPiscator/AirSane/pull/89
Comment 22 Andrew Savchenko 2022-09-16 12:53:50 MSK
Пардон, случайно отправил, не дописав обзор (tab+space). Не люблю багзиллу за это.

Итак, продолжим:

4.1) Из build.rs видно, что нужен asciidoctor. Его нет в сборочных зависимостях пакета, поэтому и возникает ошибка сборки, которую мейнтенер обошёл грубым образом, подложил не man отформатированную страницу (а исходник) как man.
Comment 23 Andrew Savchenko 2022-09-16 13:01:21 MSK
> http://git.altlinux.org/people/kovalev/packages/airsane.git

5.1) аналогично 3.2)

5.2) Мелкое замечание: в командах sed используется '|' в качестве разделителя, поэтому экранирование перед '/' не нужно, поскольку он становится обычным символом — это сделает команды более читаемыми.
Comment 24 Andrew Savchenko 2022-09-16 13:08:44 MSK
Итоговое заключение:

На мой взгляд кандидат не вычитывает логи сборки на предмет warning/error, что приводит к допущению ряда ошибок, в т.ч. серьёзных проблем как 2.3. Кроме того, ряд повторяющихся однотипных проблем (1.1, 3.2) указывает, что автор кода не до конца разобрался с порядком работы и особенностями сборки пакетов. Есть подозрение, что копировался шаблонный spec без разбора всех его деталей.

В то же время следует отметить, что автор проявляет аккуратность в написании spec-файлов, следит за отсутствием неупакованных файлов, старается корректно отслеживать лицензии (но не без ошибок, см. 3.5), старается использовать тесты (но не всегда, см. 4.2).

Поэтому я считаю, что кандидат пока что не готов к самостоятельно работе в Сизифе. Предлагаю провести работу над указанными ошибками и по возможности собрать ещё один пакет без подсказок.
Comment 25 Николай Костригин 2022-09-16 14:04:40 MSK
(Ответ для Andrew Savchenko на комментарий #23)
> > http://git.altlinux.org/people/kovalev/packages/airsane.git
> 
> 5.1) аналогично 3.2)
> 
> 5.2) Мелкое замечание: в командах sed используется '|' в качестве
> разделителя, поэтому экранирование перед '/' не нужно, поскольку он
> становится обычным символом — это сделает команды более читаемыми.

Андрей, спасибо за ревью.
По поводу экранирования в sed, - это скорее замечание для меня.
Не обращал на это внимание раньше, проверил - работает:

$ echo "/usr/local/bin/airsaned" | sed 's|\/usr\/local\/bin\/airsaned|/usr/bin/airsaned|'
/usr/bin/airsaned
$ echo "/usr/local/bin/airsaned" | sed 's|/usr/local/bin/airsaned|/usr/bin/airsaned|'
/usr/bin/airsaned

По поводу остальных замечаний, я думаю, мы пройдем по ним с кандидатом, а для закрепления выберем еще один пакет для самостоятельной сборки.
Comment 26 Andrew Savchenko 2022-09-16 18:30:36 MSK
(In reply to Andrew Savchenko from comment #21)
> Сходу я в 4-й итерации не увидел проблем, но там достаточно странный код, в
> т.ч. не понятно, почему критерий останова i != 40 вместо i > 40.

i < 40, конечно же, опечатка
Comment 27 Николай Костригин 2022-12-27 18:10:20 MSK
К кандидату просьба: не затягивать с устранением замечаний.
Самостоятельно упакетить https://pypi.org/project/lief/
Comment 29 Vitaly Lipatov 2023-04-09 01:35:24 MSK
Скажите, а зачем на кандидата так много пакетов на сборку навешали, что он год их собирает? Разве не достаточно было одного пакета?

Уже хотелось бы всё собранное увидеть в Сизифе.
Comment 30 Vasiliy Kovalev 2023-08-05 03:56:16 MSK
Все пакеты собираются по тегу - такую стратегию предпочитаю удобной для дальнейших слияний.

#303164 EPERM #12 sisyphus cmatrix.git=2.0-alt2
#303165 TESTED #9 [test-only] p10 cmatrix.git=2.0-alt2

#303168 EPERM #10 sisyphus fwts.git=23.07.00-alt1
#324403 TESTED #2 [test-only] p10 fwts.git=23.07.00-alt1

#303166 EPERM #6 sisyphus VeraCrypt.git=1.25.9-alt2
#326404 TESTED #1 [test-only] p10 VeraCrypt.git=1.25.9-alt2
https://github.com/veracrypt/VeraCrypt/commit/5a6b445f0ed51b0f06c4f0212f060ab45113b670

#303173 EPERM #12 sisyphus mdcat.git=2.0.3-alt1
#324432 TESTED #2 [test-only] p10 mdcat.git=1.0.0-alt1  // версия понижена для поддержки компилятором rust

#314385 EPERM #7 sisyphus liblief.git=0.12.3-alt1
#318502 TESTED #3 [test-only] p10 liblief.git=0.12.3-alt1

#312900 EPERM #6 sisyphus airsane.git=0.3.5-alt1
#326407 FAILED #3 [test-only] p10 airsane.git=0.3.5-alt1

Дополнительно:
Сопровождал пакет alsa-ucm-conf
https://packages.altlinux.org/ru/sisyphus/srpms/alsa-ucm-conf/
Comment 31 Vasiliy Kovalev 2023-08-07 11:19:14 MSK
(Ответ для Vasiliy Kovalev на комментарий #30)
> 
> #312900 EPERM #6 sisyphus airsane.git=0.3.5-alt1
> #326407 FAILED #3 [test-only] p10 airsane.git=0.3.5-alt1

https://github.com/SimulPiscator/AirSane/commit/e8baea9ecd5e612d6519576487d4e3e8bd3f65ee
Comment 32 Николай Костригин 2023-08-07 14:10:00 MSK
(Ответ для Vasiliy Kovalev на комментарий #30)
> Все пакеты собираются по тегу - такую стратегию предпочитаю удобной для
> дальнейших слияний.
[...]
> 
> Дополнительно:
> Сопровождал пакет alsa-ucm-conf
> https://packages.altlinux.org/ru/sisyphus/srpms/alsa-ucm-conf/

bircoph@, остались ли еще замечания к работе кандидата?
Можем ли двигаться дальше?
Comment 33 Николай Костригин 2023-08-21 19:30:33 MSK
(Ответ для Николай Костригин на комментарий #32)
[...]
> bircoph@, остались ли еще замечания к работе кандидата?
> Можем ли двигаться дальше?

Видимо, у Андрея нет времени вернуться к ревью этого вступления. Может быть, чтобы следовать букве закона [1], призвать еще одного ментора для оценки выполненных кандидатом работ?

[1] https://www.altlinux.org/Team/Join/Secretary
Comment 34 Andrew Savchenko 2023-08-22 09:17:50 MSK
(Ответ для Николай Костригин на комментарий #33)
> (Ответ для Николай Костригин на комментарий #32)
> [...]
> > bircoph@, остались ли еще замечания к работе кандидата?
> > Можем ли двигаться дальше?
> 
> Видимо, у Андрея нет времени вернуться к ревью этого вступления. Может быть,
> чтобы следовать букве закона [1], призвать еще одного ментора для оценки
> выполненных кандидатом работ?
> 
> [1] https://www.altlinux.org/Team/Join/Secretary

Я посмотрю на этой неделе. Объём проделанной работы на самом деле очень большой.
Comment 35 Anton Farygin 2023-09-04 19:02:01 MSK
Видимо у Андрея неделя оказалась очень большой.
Мне понадобился новый airscane и я его отревьювил и заапрувил.
Comment 36 Anton Farygin 2023-09-04 19:05:23 MSK
По Veracrypt, если есть возможность включить тесты - то надо включить.

Всё остальное ок.
Какие-то патчи было бы неплохо отдать в апстрим.
Comment 37 Anton Farygin 2023-09-04 19:08:41 MSK
liblief.git=0.12.3-alt1 посмотрел, вопросов нет, только большая просьба отдавать патчи в апстрим. Заапрувил и запустил.
Comment 38 Andrew Savchenko 2023-09-05 06:49:58 MSK
(Ответ для Anton Farygin на комментарий #35)
> Видимо у Андрея неделя оказалась очень большой.

Проблема в том, что кандидат слишком много пакетов собрал в ходе join, вместо того, чтоб просто довести до ума вопросы, по которым были замечания. Поэтому попытка их все сразу рассмотреть проваливается раз за разом. Могу по частям попробовать.

> Мне понадобился новый airscane и я его отревьювил и заапрувил.

А где результат этого review? Или всё идеально и замечаний нет?

Вообще, у нас было джентельменское соглашение, когда руководители сотрудников (прямые или косвенные) не делают им review.
Comment 39 Anton Farygin 2023-09-05 08:35:14 MSK
Андрей, смотреть 10 пакетов в течении года - это форменное безобразие.
Не знаю ничего ни про какие договорённости - мне важно что бы Василий закончил JOIN и получил все необходимые для этого знания. 

По пакету никаких замечаний с моей стороны нет, иначе бы я его не пропустил.
Comment 40 Andrew Savchenko 2023-09-05 17:48:00 MSK
(Ответ для Anton Farygin на комментарий #39)
> Андрей, смотреть 10 пакетов в течении года - это форменное безобразие.

Антон, какие 10 пакетов в течении года? Посмотри сообщения выше. Мой анализ был завершён 2022-09-16, первый ответ на него от кандидата я получил 2022-02-28. Более 5 месяцев никакой реакции не было, при том, что я сделал рецензирование на следующий же день после просьбы посмотреть.

В целом, это право кандидата сколь угодно долго отвечать на вопросы — понятно, что все мы можем быть заняты другими делами. Но тогда выдвигать подобные претензии рецензенту не разумно.

> Не знаю ничего ни про какие договорённости - мне важно что бы Василий
> закончил JOIN и получил все необходимые для этого знания. 

Это выглядит как попытка пропихнуть своего кандидата в обход процедуры Join.
Comment 41 Anton Farygin 2023-09-05 18:15:33 MSK
Мне важно что бы JOIN не буксовал. Сейчас буксует.
Вообще нет вопросов к кандидату, когда он затягивает с исправлениями (особенно если он не является сотрудником заинтересованной компании). Всегда есть вопросы к процедуре и к тем, кто по ней ведёт.

А про попытку "пропихнуть" на фоне того, сколько это тянется - выглядит смешно.

Я конечно могу пройтись по всему списку JOIN и везде попинать, но в данном случае мне нужна была работа Василия в Team.

Есть по делу какие-то замечания ?
Comment 42 Andrew Savchenko 2023-09-05 20:11:35 MSK
Доброго времени суток, Василий!

(Ответ для Vasiliy Kovalev на комментарий #28)
1) cmatrix
> http://git.altlinux.org/people/kovalev/packages/cmatrix.git

Проблема с лишним BR как была, как и осталась.
=========
1.1) Зачем:
BuildRequires(pre): rpm-build-licenses
?

В spec макросы из этого пакета не используются.
=========

2) fwts
> http://git.altlinux.org/people/kovalev/packages/fwts.git

ок

3) VeraCrypt
> http://git.altlinux.org/people/kovalev/packages/VeraCrypt.git

ок (тест уже включен в цели make по линковке)

4) mdcat
> http://git.altlinux.org/people/kovalev/packages/mdcat.git

Почему выключен LTO?
=========
[profile.release]
debug = true
lto = false
=========
LTO следует включать, за исключением ситуаций, когда нет никакой возможности исправить. Типовые проблемы LTO рассмотрены на вики: https://www.altlinux.org/LTO

Остальные пакеты посмотрю позже, давайте для определённости до 17.09. Пока что прошу исправить указанные замечания.
Comment 43 Anton Farygin 2023-09-06 08:33:44 MSK
Андрей, большая просьба - там где возражений нет, аппрувить и запускать сборочные задания.
https://packages.altlinux.org/ru/tasks/303168/
Comment 44 Vasiliy Kovalev 2023-09-06 12:46:00 MSK
Добрый день.

(Ответ для Andrew Savchenko на комментарий #42)
> 1) cmatrix
> > http://git.altlinux.org/people/kovalev/packages/cmatrix.git
> 
> Проблема с лишним BR как была, как и осталась.
Исправил.

> 4) mdcat
> > http://git.altlinux.org/people/kovalev/packages/mdcat.git
> 
> Почему выключен LTO?
> =========
> [profile.release]
> debug = true
> lto = false
> =========
Возникли проблемы сборки для архитектур i586 и armh (https://git.altlinux.org/tasks/303173/logs/events.9.1.log), я выбрал жесткий метод и отключил LTO для всех, полагая что ошибка памяти плавающая и вылезет на других архитектурах.
Теперь вынес отключение LTO для двух архитектур в отдельные коммиты, потому как не нашел другого способа исправить.
Comment 45 Gleb F-Malinovskiy 2023-09-06 17:27:08 MSK
(In reply to Anton Farygin from comment #43)
> https://packages.altlinux.org/ru/tasks/303168/
А зачем нужен патч 0001-fix-build-on-i586-arch.patch?
Comment 46 Vasiliy Kovalev 2023-09-06 17:36:24 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #45)
> (In reply to Anton Farygin from comment #43)
> > https://packages.altlinux.org/ru/tasks/303168/
> А зачем нужен патч 0001-fix-build-on-i586-arch.patch?

Фикс сборки для p10 (https://git.altlinux.org/tasks/324403/logs/events.1.1.log)
Comment 47 Gleb F-Malinovskiy 2023-09-06 18:11:50 MSK
(In reply to Vasiliy Kovalev from comment #46)
> Фикс сборки для p10
> (https://git.altlinux.org/tasks/324403/logs/events.1.1.log)

Но ведь это же нигде не написано?  Надо записать, что gcc10 10.3.1-alt2 в p10 неправильно определяет длину ->d_name и именно поэтому нужно выключить warning.  Ещё сам патч не очень аккуратный -- вы выключаете этот warning таким образом, что это влияет на весь файл, гораздо правильнее было бы выключить warning перед проблемной строкой, а после неё включить обратно через "#pragma GCC diagnostic pop", это должно быть легко исправить.

Это сложнее, так что просто на заметку:
По поводу 7 sed-выражений в секции %prep хочется сказать, что это самый неудачный вид патчей на код из всех возможных, с одной стороны их невозможно воспринимать другим людям, а с другой они отвалятся при первой возможности и вы этого не заметите потому что ошибки это не вызовет.
* Вообще идеально было бы исправить эту проблему в апстриме, тот факт, что эти библиотеки взаимозависимы значит, что две библиотеки вообще не нужны.
* В качестве альтернативы sed-патчам я бы предложил сделать обычный патч, который бы добавил в соответствующий Makefile.am дополнительную библиотеку, которая бы не устанавливались, но содержала бы нужный SONAME.
Comment 48 Vasiliy Kovalev 2023-09-06 21:05:21 MSK
(Ответ для Gleb F-Malinovskiy на комментарий #47)
> (In reply to Vasiliy Kovalev from comment #46)
> > Фикс сборки для p10
> > (https://git.altlinux.org/tasks/324403/logs/events.1.1.log)
> 
> Но ведь это же нигде не написано?  Надо записать, что gcc10 10.3.1-alt2 в
> p10 неправильно определяет длину ->d_name и именно поэтому нужно выключить
> warning.  Ещё сам патч не очень аккуратный -- вы выключаете этот warning
> таким образом, что это влияет на весь файл, гораздо правильнее было бы
> выключить warning перед проблемной строкой, а после неё включить обратно
> через "#pragma GCC diagnostic pop", это должно быть легко исправить.

Исправил.

> Это сложнее, так что просто на заметку:
> По поводу 7 sed-выражений в секции %prep хочется сказать, что это самый
> неудачный вид патчей на код из всех возможных, с одной стороны их невозможно
> воспринимать другим людям, а с другой они отвалятся при первой возможности и
> вы этого не заметите потому что ошибки это не вызовет.
> * Вообще идеально было бы исправить эту проблему в апстриме, тот факт, что
> эти библиотеки взаимозависимы значит, что две библиотеки вообще не нужны.
> * В качестве альтернативы sed-патчам я бы предложил сделать обычный патч,
> который бы добавил в соответствующий Makefile.am дополнительную библиотеку,
> которая бы не устанавливались, но содержала бы нужный SONAME.

Ок, постараюсь учесть в следующих обновлениях.
Comment 49 Николай Костригин 2023-10-03 20:25:15 MSK
(Ответ для Николай Костригин на комментарий #33)
[...]
> 
> Видимо, у Андрея нет времени вернуться к ревью этого вступления. Может быть,
> чтобы следовать букве закона [1], призвать еще одного ментора для оценки
> выполненных кандидатом работ?
> 
> [1] https://www.altlinux.org/Team/Join/Secretary

Поскольку Андрей вернулся к ревью заявки, логично вернуть ее на нужную стадию.
Comment 50 Andrew Savchenko 2023-10-07 01:13:21 MSK
(Ответ для Vasiliy Kovalev на комментарий #44)
> (Ответ для Andrew Savchenko на комментарий #42)
> > 1) cmatrix
> > > http://git.altlinux.org/people/kovalev/packages/cmatrix.git
> > 
> > Проблема с лишним BR как была, как и осталась.
> Исправил.

Хорошо, замечаний больше нет, пропустил.
Comment 51 Andrew Savchenko 2023-10-07 01:50:19 MSK
(Ответ для Vasiliy Kovalev на комментарий #48)
> (Ответ для Gleb F-Malinovskiy на комментарий #47)
> > (In reply to Vasiliy Kovalev from comment #46)
> > > Фикс сборки для p10
> > > (https://git.altlinux.org/tasks/324403/logs/events.1.1.log)
> > 
> > Но ведь это же нигде не написано?  Надо записать, что gcc10 10.3.1-alt2 в
> > p10 неправильно определяет длину ->d_name и именно поэтому нужно выключить
> > warning.  Ещё сам патч не очень аккуратный -- вы выключаете этот warning
> > таким образом, что это влияет на весь файл, гораздо правильнее было бы
> > выключить warning перед проблемной строкой, а после неё включить обратно
> > через "#pragma GCC diagnostic pop", это должно быть легко исправить.
> 
> Исправил.

По fwts тоже вопросов нет, заапрувил.

(Ответ для Anton Farygin на комментарий #43)
> Андрей, большая просьба - там где возражений нет, аппрувить и запускать
> сборочные задания.
> https://packages.altlinux.org/ru/tasks/303168/

По апруву вопросов нет, а вот запускать задания следует самому мейнтенеру. Так правильнее на мой взгляд — вдруг он test-only захочет или ещё что.
Comment 52 Andrew Savchenko 2023-10-07 02:05:09 MSK
(Ответ для Andrew Savchenko на комментарий #42)
> 3) VeraCrypt
> > http://git.altlinux.org/people/kovalev/packages/VeraCrypt.git
> 
> ок (тест уже включен в цели make по линковке)

Посмотрел патчи. Есть вопрос по 0003-Core-Unix-fix-warning-ignoring-return-value.patch: в текущем виде ошибка chown будет просто проигнорирована, что плохо. Следует throw использовать, throw_sys_if, например.
Comment 53 Andrew Savchenko 2023-10-07 02:16:49 MSK
(Ответ для Vasiliy Kovalev на комментарий #44)
> > 4) mdcat
> > > http://git.altlinux.org/people/kovalev/packages/mdcat.git
> > 
> > Почему выключен LTO?
> > =========
> > [profile.release]
> > debug = true
> > lto = false
> > =========
> Возникли проблемы сборки для архитектур i586 и armh
> (https://git.altlinux.org/tasks/303173/logs/events.9.1.log), я выбрал
> жесткий метод и отключил LTO для всех, полагая что ошибка памяти плавающая и
> вылезет на других архитектурах.
> Теперь вынес отключение LTO для двух архитектур в отдельные коммиты, потому
> как не нашел другого способа исправить.

Вместо двух одинаковых блоков для этих архитектур, следует использовать один: %ifarch поддерживает списки:

%ifarch i586 armh

В этом же спеке:

sed -i "s|asciidoctor|$(rpm -qa | grep asciidoctor | xargs rpm -ql | grep "bin\/asciidoctor" | head -1)|" build.rs

Зачем такие сложности? Там достаточно вызова asciidoctor из пакета asciidoctor.

Зачем в BR gem-asciidoctor, если он есть в зависимостях asciidoctor?
Comment 54 Andrew Savchenko 2023-10-07 02:30:05 MSK
(Ответ для Vasiliy Kovalev на комментарий #30)
> #312900 EPERM #6 sisyphus airsane.git=0.3.5-alt1

warning: File listed twice: /etc/airsane/ignore.conf
warning: File listed twice: /etc/airsane/options.conf
Comment 55 Andrew Savchenko 2023-10-07 02:40:24 MSK
(Ответ для Vasiliy Kovalev на комментарий #30)
> #314385 EPERM #7 sisyphus liblief.git=0.12.3-alt1

ок

> Дополнительно:
> Сопровождал пакет alsa-ucm-conf
> https://packages.altlinux.org/ru/sisyphus/srpms/alsa-ucm-conf/

Спасибо, вроде ок.

Осталось исправить проблемы с veracrypt, mdcat и airsane.
Comment 56 Vasiliy Kovalev 2023-10-09 14:35:46 MSK
VeraCrypt:

(Ответ для Andrew Savchenko на комментарий #52)
> Посмотрел патчи. Есть вопрос по
> 0003-Core-Unix-fix-warning-ignoring-return-value.patch: в текущем виде
> ошибка chown будет просто проигнорирована, что плохо. Следует throw
> использовать, throw_sys_if, например.

Обновил пакет VeraCrypt и заодно исправил:
https://github.com/veracrypt/VeraCrypt/pull/1229
https://git.altlinux.org/tasks/303166/
------------------------------------------------------------
mdcat:

(Ответ для Andrew Savchenko на комментарий #53)
> Вместо двух одинаковых блоков для этих архитектур, следует использовать
> один: %ifarch поддерживает списки:
> 
> %ifarch i586 armh

Здесь я специально разбил на два коммита, потому что логи ошибок отличаются для этих двух архитектур, и если исправления покроют не все ошибки - будет проще ревертить.
https://git.altlinux.org/people/kovalev/packages/?p=mdcat.git;a=shortlog;h=0d206b1bf03a30e4bbfc19d4c32a6f061acf8401

> В этом же спеке:
> 
> sed -i "s|asciidoctor|$(rpm -qa | grep asciidoctor | xargs rpm -ql | grep
> "bin\/asciidoctor" | head -1)|" build.rs
> 
> Зачем такие сложности? Там достаточно вызова asciidoctor из пакета
> asciidoctor.
> 
> Зачем в BR gem-asciidoctor, если он есть в зависимостях asciidoctor?

Действительно, исправил. Почему такое получилось уже не помню, вроде пытался сделать сборку правильной и на p10 и на сизифе.
Обновил версию дополнительно:
https://git.altlinux.org/tasks/303173
------------------------------------------------------------
airsane:

(Ответ для Andrew Savchenko на комментарий #54)
> (Ответ для Vasiliy Kovalev на комментарий #30)
> > #312900 EPERM #6 sisyphus airsane.git=0.3.5-alt1
> 
> warning: File listed twice: /etc/airsane/ignore.conf
> warning: File listed twice: /etc/airsane/options.conf

Исправил:
https://git.altlinux.org/people/kovalev/packages/?p=airsane.git;a=commitdiff;h=92ca1cbb29b599b239eea899f139a5daa4d44e6f

Применю уже в следующем обновлении, если не критично.
Comment 57 Николай Костригин 2023-11-23 13:55:58 MSK
Андрей, осталось ли еще что-то, что мешает перейти к 5.0 ?
Comment 58 Gleb F-Malinovskiy 2023-12-05 19:23:10 MSK
Адрес подписан на devel@, теперь это делается раньше -- в пункте 3.6.
Comment 59 Anton Farygin 2024-03-11 14:57:26 MSK
К сожалению рецензент не справляется со своими задачами. Просьба Секретарю поискать другого, более активного рецензента для Василия.
Comment 60 Andrew Savchenko 2024-03-13 03:16:03 MSK
К сожалению, Антон забыл, что у процедуры Join существует чёткий регламент[1] и менять статус Join бага могут только секретарь, ментор и рецензент в описанных в [1] ситуациях. Поскольку Антон не относится ни к одной из вышеперечисленных категорий, вмешательство им в T/J/S данного бага неправомерно и статус возвращён на прежний.

По существу есть следующий вопрос. Задача Join убедится в том, что кандидат обладает нужными навыками для деятельности в Team, что касается не только сборки пакетов, но и взаимоедействия с Team. Обычно проверка ограничивается областью деятельности кандидата, но у Василия она очень широка, что, безусловно, вызывает уважение, но также ставит дополнительные вопросы.

Мне недавно поступили нарекания по поводу патчей ядра от Василия, что они не отправляются в апстрим и порой берутся из произвольных источников, нарушая принцип Upstream first. Безусловно, из-за своенравных особенностей мейнтенеров стабильных веток там могут быть свои проблемы, но хотя бы обсуждать на lore нужно.

Я вижу, что существенная часть патчей уже отправлена[2] — это замечательно. Но не понимаю, почему [3] сделан в обход апстрима. Хотелось бы узнать мнение Василия по этому поводу.

[1] https://www.altlinux.org/Team/Join/Secretary
[2] https://lore.kernel.org/all/?q=f%3Akovalev%40altlinux.org
[3] https://lists.altlinux.org/pipermail/devel-kernel/2024-March/008039.html
Comment 61 Anton Farygin 2024-03-13 09:10:27 MSK
(Ответ для Andrew Savchenko на комментарий #60)
> К сожалению, Антон забыл, что у процедуры Join существует чёткий
> регламент[1] и менять статус Join бага могут только секретарь, ментор и
> рецензент в описанных в [1] ситуациях. Поскольку Антон не относится ни к
> одной из вышеперечисленных категорий, вмешательство им в T/J/S данного бага
> неправомерно и статус возвращён на прежний.

Да не проблема, думаю что ментор и сам в состоянии понять, что ревьювер недееспособен,уж коль он целых 5 месяцев не отвечал на вопрос.
Comment 62 Vasiliy Kovalev 2024-03-13 10:20:47 MSK
(Ответ для Andrew Savchenko на комментарий #60)
> Мне недавно поступили нарекания по поводу патчей ядра от Василия, что они не
> отправляются в апстрим и порой берутся из произвольных источников, нарушая
> принцип Upstream first. Безусловно, из-за своенравных особенностей
> мейнтенеров стабильных веток там могут быть свои проблемы, но хотя бы
> обсуждать на lore нужно.
>
> Я вижу, что существенная часть патчей уже отправлена[2] — это замечательно.
> Но не понимаю, почему [3] сделан в обход апстрима. Хотелось бы узнать мнение
> Василия по этому поводу.

Я там же [4] и ответил, что этот патч [3] только для альтового ядра (в апстриме не существует кода, который бы исправлял этот патч, поэтому в обход апстрима) и в описании я указал хэш коммита (f63b2a193575f82), который он исправляет - это вся необходимая информация без воды, достаточная для принятия патча. Раз уж отличный от апстрима код имеется в нашем ядре, то и проблемы к которым он приводит должны быть исправлены.

А то, что ранее первоначально acp драйвер для кодека es8336 и связанные с ним изменения для драйвера es816 были добавлены прямиком в наше ядро  в обход апстрима - это другая история и я понял, что так делать неправильно, но другого варианта получить ~год назад работающий звук из коробки на некоторых моделях Huawei с ЦПУ AMD не было, потому что драйвер был еще сырой. Однако, до сих пор нет решения на стабильных апстрим ветках.

> [2] https://lore.kernel.org/all/?q=f%3Akovalev%40altlinux.org
> [3] https://lists.altlinux.org/pipermail/devel-kernel/2024-March/008039.html

[4] https://lists.altlinux.org/pipermail/devel-kernel/2024-March/008041.html
Comment 63 Николай Костригин 2024-03-13 11:59:33 MSK
(Ответ для Andrew Savchenko на комментарий #60)
> К сожалению, Антон забыл, что у процедуры Join существует чёткий
> регламент[1] и менять статус Join бага могут только секретарь, ментор 
> 
Я могу повторить откат, как ментор, т.к., к сожалению,  только такие возвратно-поступательные движения дают импульс этой процедуре вступления. 

> По существу есть следующий вопрос. Задача Join убедится в том, что кандидат
> обладает нужными навыками для деятельности в Team, что касается не только
> сборки пакетов, но и взаимоедействия с Team. Обычно проверка ограничивается
> областью деятельности кандидата, но у Василия она очень широка, что,
> безусловно, вызывает уважение, но также ставит дополнительные вопросы.

Получается, чтобы пройти join быстро нужно, по-возможности, не проявлять лишнего энтузиазма? Я согласен, что большой объем рецензирования затрудняет его выполнение в один присест...

> Мне недавно поступили нарекания по поводу патчей ядра от Василия,

но патчи Василия к ядру не были предметом его заявки на вступление в team...
Зачем усложнять и без того непростую задачу?

> что они не
> отправляются в апстрим и порой берутся из произвольных источников, нарушая
> принцип Upstream first. 

Можно ссылку на утвержденную политику "Upstream first"?
Я не нашёл. Хотя бы страничку на вики, где это рекомендуется соблюдать вступающим в команду.

> 
> Я вижу, что существенная часть патчей уже отправлена[2] — это замечательно.
> Но не понимаю, почему [3] сделан в обход апстрима. Хотелось бы узнать мнение
> Василия по этому поводу.

В дополнение к тому, что Василий ответил, скажу, что обкатка этих патчей проводилась на многих моделях в течение некоторого времени. Оказывалось, что не смотря на то, что звуковая подсистема построена на одинаковых чипах, разные производители для разных моделей довольно творчески подходили к разводке сигналов определения наличия гарнитуры. Когда патчи стабилизировались, стали отправлять. 

> 
> [1] https://www.altlinux.org/Team/Join/Secretary
> [2] https://lore.kernel.org/all/?q=f%3Akovalev%40altlinux.org
> [3] https://lists.altlinux.org/pipermail/devel-kernel/2024-March/008039.html