Bug 42475
| Summary: | [4.0] join kovalev@ | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Team Accounts | Reporter: | Vasiliy Kovalev <kovalev.temp> | ||||||||
| Component: | join | Assignee: | Gleb F-Malinovskiy <glebfm> | ||||||||
| Status: | ASSIGNED --- | QA Contact: | Andrey Cherepanov <cas> | ||||||||
| Severity: | normal | ||||||||||
| Priority: | P5 | CC: | bircoph, glebfm, kovalev.temp, kovalevvv, lav, ldv, nickel, rider | ||||||||
| Version: | unspecified | ||||||||||
| Hardware: | x86_64 | ||||||||||
| OS: | Linux | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 10591 [details]
gpg public key
gpg public key
Менторство подтверждаю. Почту для пересылки с временной рекомендую заменить на постоянную. SSH ключ выглядит валидным. GPG ключ договорились сделать со подключом для подписи с истекающим сроком действия. Секретарю просьба дождаться обозначенных изменений. Created attachment 10593 [details]
gpg public key
заменил публичный gpg ключ
почта для пересылки: kovalevvv@basealt.ru (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. Можно переходить к следующему шагу. (In reply to Vasiliy Kovalev from comment #4) > почта для пересылки: kovalevvv@basealt.ru Прошу прощения, я прозевал это сообщение. Проверьте почту, пожалуйста. я проверил почту, рассылка работает по старому адресу kovalev.temp@yandex.ru прошу изменить адрес почты для рассылки на kovalevvv@basealt.ru (In reply to Vasiliy Kovalev from comment #8) > я проверил почту, рассылка работает по старому адресу kovalev.temp@yandex.ru Рассылка чего работает? > прошу изменить адрес почты для рассылки на kovalevvv@basealt.ru Да, я увидел ваше пожелание. Почта подтверждена. (Ответ для Gleb F-Malinovskiy на комментарий #9) > Рассылка чего работает? уведомления о действиях в bugzilla приходят на почту. думал это рассылка (In reply to Vasiliy Kovalev from comment #11) > (Ответ для Gleb F-Malinovskiy на комментарий #9) > > Рассылка чего работает? > уведомления о действиях в bugzilla приходят на почту. думал это рассылка Не, речь идёт про адрес, на который будет перенаправляться почта, которая будет приходить на ваш адрес в домене @altlinux.org . ssh ключ на gitery.alt зарегистрирован. Адрес для пересылки создан. T/J/S -> 2.3. Можно перейти к следующему этапу. ssh ключ на gyle.alt зарегистрирован. Пакет alt-gpgkeys обновлён. T/J/S -> 3.5. Собрал следующие пакеты: cmatrix http://git.altlinux.org/people/kovalev/packages/cmatrix.git https://git.altlinux.org/tasks/303164/ https://git.altlinux.org/tasks/303165/ fwts http://git.altlinux.org/people/kovalev/packages/fwts.git https://git.altlinux.org/tasks/303168/ VeraCrypt http://git.altlinux.org/people/kovalev/packages/VeraCrypt.git https://git.altlinux.org/tasks/303166/ mdcat http://git.altlinux.org/people/kovalev/packages/mdcat.git https://git.altlinux.org/tasks/303173/ (не собирается для архитектуры ppc64le) 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 Полагаю, можно призвать второго ментора для оценки наработок кандидата. (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 Благодарю за замечание, поправил спек. Призван ещё один человек (bircoph@) для независимой оценки готовности кандидата. T/J/S -> 4.2. Добрый день! Есть следующие вопросы: (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 Пардон, случайно отправил, не дописав обзор (tab+space). Не люблю багзиллу за это. Итак, продолжим: 4.1) Из build.rs видно, что нужен asciidoctor. Его нет в сборочных зависимостях пакета, поэтому и возникает ошибка сборки, которую мейнтенер обошёл грубым образом, подложил не man отформатированную страницу (а исходник) как man. > http://git.altlinux.org/people/kovalev/packages/airsane.git
5.1) аналогично 3.2)
5.2) Мелкое замечание: в командах sed используется '|' в качестве разделителя, поэтому экранирование перед '/' не нужно, поскольку он становится обычным символом — это сделает команды более читаемыми.
Итоговое заключение: На мой взгляд кандидат не вычитывает логи сборки на предмет warning/error, что приводит к допущению ряда ошибок, в т.ч. серьёзных проблем как 2.3. Кроме того, ряд повторяющихся однотипных проблем (1.1, 3.2) указывает, что автор кода не до конца разобрался с порядком работы и особенностями сборки пакетов. Есть подозрение, что копировался шаблонный spec без разбора всех его деталей. В то же время следует отметить, что автор проявляет аккуратность в написании spec-файлов, следит за отсутствием неупакованных файлов, старается корректно отслеживать лицензии (но не без ошибок, см. 3.5), старается использовать тесты (но не всегда, см. 4.2). Поэтому я считаю, что кандидат пока что не готов к самостоятельно работе в Сизифе. Предлагаю провести работу над указанными ошибками и по возможности собрать ещё один пакет без подсказок. (Ответ для 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 По поводу остальных замечаний, я думаю, мы пройдем по ним с кандидатом, а для закрепления выберем еще один пакет для самостоятельной сборки. (In reply to Andrew Savchenko from comment #21) > Сходу я в 4-й итерации не увидел проблем, но там достаточно странный код, в > т.ч. не понятно, почему критерий останова i != 40 вместо i > 40. i < 40, конечно же, опечатка К кандидату просьба: не затягивать с устранением замечаний. Самостоятельно упакетить https://pypi.org/project/lief/ Доброго времени суток! Andrew Savchenko, благодарю за замечания. Проделал работу над ошибками и недочетами предыдущих пакетов, а также собрал новый (liblief): cmatrix http://git.altlinux.org/people/kovalev/packages/cmatrix.git https://git.altlinux.org/tasks/303164/ https://git.altlinux.org/tasks/303165/ fwts http://git.altlinux.org/people/kovalev/packages/fwts.git https://git.altlinux.org/tasks/303168/ один коммит принят в апстрим https://git.launchpad.net/fwts/commit/?id=a2aa84f648c3c5b442d9c6582897bbc17b0aa78e VeraCrypt http://git.altlinux.org/people/kovalev/packages/VeraCrypt.git https://git.altlinux.org/tasks/303166/ mdcat http://git.altlinux.org/people/kovalev/packages/mdcat.git https://git.altlinux.org/tasks/303173/ airsane http://git.altlinux.org/people/kovalev/packages/airsane.git https://git.altlinux.org/tasks/312900/ liblief http://git.altlinux.org/people/kovalev/packages/liblief.git https://git.altlinux.org/tasks/314385/ Скажите, а зачем на кандидата так много пакетов на сборку навешали, что он год их собирает? Разве не достаточно было одного пакета? Уже хотелось бы всё собранное увидеть в Сизифе. Все пакеты собираются по тегу - такую стратегию предпочитаю удобной для дальнейших слияний. #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/ (Ответ для 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 (Ответ для Vasiliy Kovalev на комментарий #30) > Все пакеты собираются по тегу - такую стратегию предпочитаю удобной для > дальнейших слияний. [...] > > Дополнительно: > Сопровождал пакет alsa-ucm-conf > https://packages.altlinux.org/ru/sisyphus/srpms/alsa-ucm-conf/ bircoph@, остались ли еще замечания к работе кандидата? Можем ли двигаться дальше? (Ответ для Николай Костригин на комментарий #32) [...] > bircoph@, остались ли еще замечания к работе кандидата? > Можем ли двигаться дальше? Видимо, у Андрея нет времени вернуться к ревью этого вступления. Может быть, чтобы следовать букве закона [1], призвать еще одного ментора для оценки выполненных кандидатом работ? [1] https://www.altlinux.org/Team/Join/Secretary (Ответ для Николай Костригин на комментарий #33) > (Ответ для Николай Костригин на комментарий #32) > [...] > > bircoph@, остались ли еще замечания к работе кандидата? > > Можем ли двигаться дальше? > > Видимо, у Андрея нет времени вернуться к ревью этого вступления. Может быть, > чтобы следовать букве закона [1], призвать еще одного ментора для оценки > выполненных кандидатом работ? > > [1] https://www.altlinux.org/Team/Join/Secretary Я посмотрю на этой неделе. Объём проделанной работы на самом деле очень большой. Видимо у Андрея неделя оказалась очень большой. Мне понадобился новый airscane и я его отревьювил и заапрувил. По Veracrypt, если есть возможность включить тесты - то надо включить. Всё остальное ок. Какие-то патчи было бы неплохо отдать в апстрим. liblief.git=0.12.3-alt1 посмотрел, вопросов нет, только большая просьба отдавать патчи в апстрим. Заапрувил и запустил. (Ответ для Anton Farygin на комментарий #35) > Видимо у Андрея неделя оказалась очень большой. Проблема в том, что кандидат слишком много пакетов собрал в ходе join, вместо того, чтоб просто довести до ума вопросы, по которым были замечания. Поэтому попытка их все сразу рассмотреть проваливается раз за разом. Могу по частям попробовать. > Мне понадобился новый airscane и я его отревьювил и заапрувил. А где результат этого review? Или всё идеально и замечаний нет? Вообще, у нас было джентельменское соглашение, когда руководители сотрудников (прямые или косвенные) не делают им review. Андрей, смотреть 10 пакетов в течении года - это форменное безобразие. Не знаю ничего ни про какие договорённости - мне важно что бы Василий закончил JOIN и получил все необходимые для этого знания. По пакету никаких замечаний с моей стороны нет, иначе бы я его не пропустил. (Ответ для Anton Farygin на комментарий #39) > Андрей, смотреть 10 пакетов в течении года - это форменное безобразие. Антон, какие 10 пакетов в течении года? Посмотри сообщения выше. Мой анализ был завершён 2022-09-16, первый ответ на него от кандидата я получил 2022-02-28. Более 5 месяцев никакой реакции не было, при том, что я сделал рецензирование на следующий же день после просьбы посмотреть. В целом, это право кандидата сколь угодно долго отвечать на вопросы — понятно, что все мы можем быть заняты другими делами. Но тогда выдвигать подобные претензии рецензенту не разумно. > Не знаю ничего ни про какие договорённости - мне важно что бы Василий > закончил JOIN и получил все необходимые для этого знания. Это выглядит как попытка пропихнуть своего кандидата в обход процедуры Join. Мне важно что бы JOIN не буксовал. Сейчас буксует. Вообще нет вопросов к кандидату, когда он затягивает с исправлениями (особенно если он не является сотрудником заинтересованной компании). Всегда есть вопросы к процедуре и к тем, кто по ней ведёт. А про попытку "пропихнуть" на фоне того, сколько это тянется - выглядит смешно. Я конечно могу пройтись по всему списку JOIN и везде попинать, но в данном случае мне нужна была работа Василия в Team. Есть по делу какие-то замечания ? Доброго времени суток, Василий! (Ответ для 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. Пока что прошу исправить указанные замечания. Андрей, большая просьба - там где возражений нет, аппрувить и запускать сборочные задания. https://packages.altlinux.org/ru/tasks/303168/ Добрый день. (Ответ для 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 для двух архитектур в отдельные коммиты, потому как не нашел другого способа исправить. (In reply to Anton Farygin from comment #43) > https://packages.altlinux.org/ru/tasks/303168/ А зачем нужен патч 0001-fix-build-on-i586-arch.patch? (Ответ для 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) (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. (Ответ для 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. Ок, постараюсь учесть в следующих обновлениях. (Ответ для Николай Костригин на комментарий #33) [...] > > Видимо, у Андрея нет времени вернуться к ревью этого вступления. Может быть, > чтобы следовать букве закона [1], призвать еще одного ментора для оценки > выполненных кандидатом работ? > > [1] https://www.altlinux.org/Team/Join/Secretary Поскольку Андрей вернулся к ревью заявки, логично вернуть ее на нужную стадию. (Ответ для Vasiliy Kovalev на комментарий #44) > (Ответ для Andrew Savchenko на комментарий #42) > > 1) cmatrix > > > http://git.altlinux.org/people/kovalev/packages/cmatrix.git > > > > Проблема с лишним BR как была, как и осталась. > Исправил. Хорошо, замечаний больше нет, пропустил. (Ответ для 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 захочет или ещё что. (Ответ для 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, например. (Ответ для 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? (Ответ для 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 (Ответ для 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. 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 Применю уже в следующем обновлении, если не критично. Андрей, осталось ли еще что-то, что мешает перейти к 5.0 ? Адрес подписан на devel@, теперь это делается раньше -- в пункте 3.6. К сожалению рецензент не справляется со своими задачами. Просьба Секретарю поискать другого, более активного рецензента для Василия. К сожалению, Антон забыл, что у процедуры 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 (Ответ для Andrew Savchenko на комментарий #60) > К сожалению, Антон забыл, что у процедуры Join существует чёткий > регламент[1] и менять статус Join бага могут только секретарь, ментор и > рецензент в описанных в [1] ситуациях. Поскольку Антон не относится ни к > одной из вышеперечисленных категорий, вмешательство им в T/J/S данного бага > неправомерно и статус возвращён на прежний. Да не проблема, думаю что ментор и сам в состоянии понять, что ревьювер недееспособен,уж коль он целых 5 месяцев не отвечал на вопрос. (Ответ для 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 (Ответ для 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 Добрый день, заявка по-прежнему актуальна, обновил статус по пакетам: 1) VeraCrypt https://git.altlinux.org/tasks/303166/ Пакетом не занимаюсь, так как в репозитории уже сопровождается https://packages.altlinux.org/ru/sisyphus/srpms/veracrypt/ 2) liblief https://git.altlinux.org/tasks/383801/ Обновил до предрелизной версии 0.17.0, которая в отличии от последней доступной 0.16.5 включает исправления предупреждений сборки: https://github.com/lief-project/LIEF/pull/1195 + патч из https://github.com/lief-project/LIEF/pull/1198 , который еще на этапе рассмотрения. 3) mdcat https://git.altlinux.org/tasks/303173/ Обновил до последней доступной версии, но так как проект больше не сопровождается https://github.com/swsnr/mdcat/commit/735e6a2fe4a8c850a74fdd10c549809fa20e9ada собрал альтернативу - mcat https://git.altlinux.org/tasks/384362/ Эта программа помимо md поддерживает вывод в терминале содержимого файлов других форматов, в том числе pdf, jpeg и mp4, но для последнего требуется зависимость ffmpeg . Я не стал ее явно указывать в спеке, оставив установку пакета ffmpeg на усмотрение пользователя, тем более при работе с mp4 файлами программа явно сообщает об отсутствующем пакете ffmpeg . По поводу патчей в обход апстрима: > [3] https://lists.altlinux.org/pipermail/devel-kernel/2024-March/008039.html принят [1] в alt ветки вместе с патчами [2] и [3] . [1] https://git.altlinux.org/people/kernelbot/packages/?p=kernel-image.git&a=commit&h=300009d9ba50fb4cc97712df2326ff221395eb7b [2] https://git.altlinux.org/people/kernelbot/packages/?p=kernel-image.git&a=commit&h=9cedc68253ad99482dd1e44dfd47d94a959da2d7 [3] https://git.altlinux.org/people/kernelbot/packages/?p=kernel-image.git&a=commit&h=b47743bb0720af6ad6d9bf18d7923b54f5c66b07 (In reply to Vasiliy Kovalev from comment #64) > Добрый день, > заявка по-прежнему актуальна, обновил статус по пакетам: [...] Просьба секретарю призвать рецензента. |
Description
Vasiliy Kovalev 2022-04-18 10:31:16 MSK