Bug 34721

Fixed in Sisyphus by task #203066

(In reply to comment #1)
> Fixed in Sisyphus by task #203066

Спасибо!

gear-store-tags неудачно там сохранил значения: patches/advanced не включает последние изменения из alt18, в результате сгененрированный diff их отменяет.

Это можно видеть в патче на session-child.c в https://packages.altlinux.org/en/Sisyphus/srpms/lightdm/patches/lightdm-1.16.7-alt19-advanced.patch , строка 479:

@@ -811,7 +848,8 @@ session_child_run (int argc, char **argv)
         if (error)
             g_printerr ("Error removing X authority: %s\n", error->message);
         g_clear_error (&error);
-        /* Ignore this error, don't exit, continue closing the session. */
+        if (!result)
+            _exit (EXIT_FAILURE);
     }
 
     /* Close the Console Kit session */

Тут нужно придерживаться правило, чтобы patches/advanced всегда был наследником patches/alt-fixes, а также прошлого значения patches/advanced. К сожаленью, gear это не проверяет (аналогично сборочному коммиту), хотя смысл похожий.

Нужно переделать...

Ещё лучше не совмещать несвязанные whitespace changes и исправления в одном коммите.

Советую также предложить upstream-у на https://github.com/CanonicalLtd/lightdm/pulls?q=is%3Apr+is%3Aclosed ; мою поправку смёрджили.

(In reply to comment #3)

> Советую также предложить upstream-у на
> https://github.com/CanonicalLtd/lightdm/pulls?q=is%3Apr+is%3Aclosed ; мою
> поправку смёрджили.

Я, кстати, ради простоты создания pull request-а сделал свой патч не очередным коммитом в линейно тянувшейся ветке patches/alt-fixes, а взял за основу upstream-ный коммит соответствующего релиза, а потом свой коммит смёрджил ("параллельно") в patches/alt-fixes.

gear-схеме с diff-ами не противоречит.

(In reply to comment #3)
> Ещё лучше не совмещать несвязанные whitespace changes и исправления в одном
> коммите.
> 
> Советую также предложить upstream-у на
> https://github.com/CanonicalLtd/lightdm/pulls?q=is%3Apr+is%3Aclosed ; мою
> поправку смёрджили.

Взглянул на глазок. Где-то значительно после 1.16 код, связанный с vnc-сеансами, был существенно переработан; у них это, наверное, уже исправлено.
Однако я 1.26 не собирал и ручаться не буду. :)

(In reply to comment #2)
> (In reply to comment #1)
> > Fixed in Sisyphus by task #203066
> 
> Спасибо!
> 
> gear-store-tags неудачно там сохранил значения: patches/advanced не включает
> последние изменения из alt18, в результате сгененрированный diff их отменяет.
> 
> Это можно видеть в патче на session-child.c в
> https://packages.altlinux.org/en/Sisyphus/srpms/lightdm/patches/lightdm-1.16.7-alt19-advanced.patch
> , строка 479:
> 
> @@ -811,7 +848,8 @@ session_child_run (int argc, char **argv)
>          if (error)
>              g_printerr ("Error removing X authority: %s\n", error->message);
>          g_clear_error (&error);
> -        /* Ignore this error, don't exit, continue closing the session. */
> +        if (!result)
> +            _exit (EXIT_FAILURE);
>      }
> 
>      /* Close the Console Kit session */
> 
> Тут нужно придерживаться правило, чтобы patches/advanced всегда был наследником
> patches/alt-fixes, а также прошлого значения patches/advanced. К сожаленью,
> gear это не проверяет (аналогично сборочному коммиту), хотя смысл похожий.
> 
> Нужно переделать...

Торопыжка был голодный, проглотил утюг холодный...

Переделал в alt20.
Глазами проверил генерируемый %nvr-advanced.patch, вроде всё хорошо.
Кстати, так и не понятно до сих пор, зачем аж две ветки с патчами?

(In reply to comment #5)
> Взглянул на глазок. Где-то значительно после 1.16 код, связанный с
> vnc-сеансами, был существенно переработан; у них это, наверное, уже исправлено.

https://github.com/CanonicalLtd/lightdm/blob/master/src/seat-xvnc.c#L112

> Однако я 1.26 не собирал и ручаться не буду. :)

(In reply to comment #6)

> Переделал в alt20.
> Глазами проверил генерируемый %nvr-advanced.patch, вроде всё хорошо.

Спасибо!

> Кстати, так и не понятно до сих пор, зачем аж две ветки с патчами?

Не знаю, по-моему, если и разделять, то лучше на отдельные, логически независимые ветки-патчи, а одну ветку в другую мёрджить, есди они логически зависимы.

Но по отдельной ветке на патч -- больше мороки. Я так практиковался делать в https://www.altlinux.org/%D0%97%D0%BD%D0%B0%D0%BA%D0%BE%D0%BC%D1%81%D1%82%D0%B2%D0%BE_%D1%81%D0%BE_%D1%81%D1%85%D0%B5%D0%BC%D0%BE%D0%B9_%D1%83%D0%BF%D0%B0%D0%BA%D0%BE%D0%B2%D0%BA%D0%B8_pam-pkcs11 .

Либо уж просто последовательность коммитов-патчей в одной ветке. 

Если это для удобства мейнтейнера (две ветки), то всё равно непонятно, зачем генерировать два diff-а, а не один. (Это бы от таких ошибок избавило.)

(In reply to comment #8)

> Если это для удобства мейнтейнера (две ветки), то всё равно непонятно, зачем
> генерировать два diff-а, а не один. (Это бы от таких ошибок избавило.)

Ну не избавило бы, но сделало бы их менее вероятными.