Project

General

Profile

Bug #12551

Set up a process to keep our fork of GNOME Shell's .desktop file and GDM's .session file up-to-date

Added by intrigeri over 2 years ago. Updated 26 days ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
Start date:
05/16/2017
Due date:
% Done:

100%

Feature Branch:
feature/16912-move-greeter-to-main-git-repo+force-all-tests
Type of work:
Code
Blueprint:
Starter:
Affected tool:
Greeter

Description

As noticed on #12364#note-33, our own config/gdm-shell-tails.desktop (in Tails Greeter's Git repo) tends to be behind /usr/share/applications/org.gnome.Shell.desktop as shipped in Debian. This can potentially cause serious issues, so IMO we need a process to keep it more up-to-date than what we've done in the past. I don't know what's the best way to do that, but at least a ticket per Tails release based on the next version of Debian would be a good start; completing said ticket would require creating the next one. But there are probably better ways to do so, e.g. generate our own .desktop file dynamically at package build time from the upstream one.

Same for gdm-tails.session, see f5e00d2d1051991e29720c422337e8855ca17cb2.


Related issues

Related to Tails - Bug #16288: No accessibility support in the Greeter on Buster Resolved 01/05/2019
Related to Tails - Bug #16951: Update gdm-tails.json for Bullseye Confirmed
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed
Blocked by Tails - Feature #16912: Move greeter source to main git repo Resolved

Associated revisions

Revision 378502fe (diff)
Added by intrigeri over 1 year ago

Update Greeter's gdm-tails.session for Buster (refs: #12551).

org.gnome.SettingsDaemon.A11yKeyboard does not exist anymore
which breaks the startup of gnome-session and thus the Greeter
was never displayed.

Revision b8a9438c (diff)
Added by intrigeri 9 months ago

Update forked gdm-tails.json for Buster (refs: #12551, #16288).

Revision 25673681 (diff)
Added by segfault 3 months ago

Create gdm-tails related files from the original GNOME files (refs: #12551)

We used to ship pathed files in the Greeter, which tend to be behind the
original files shipped in Debian. This can potentially cause serious
issues.

This copies the GNOME files as shipped in Debian and applies our patches
on top.

Revision a4195b31 (diff)
Added by segfault 3 months ago

31-gdm-tails fixup (refs: #12551)

Escape dots in regex and use extended regex.

Revision 46e0cc86 (diff)
Added by segfault about 1 month ago

Create gdm-tails related files from the original GNOME files (refs: #12551)

We used to ship patched files in the Greeter, which tend to be behind the
original files shipped in Debian. This can potentially cause serious
issues.

This copies the GNOME files as shipped in Debian and applies our patches
on top.

Revision 1d62bc2e (diff)
Added by segfault about 1 month ago

Create gdm-tails related files from the original GNOME files (refs: #12551)

We used to ship patched files in the Greeter, which tend to be behind the
original files shipped in Debian. This can potentially cause serious
issues.

This copies the GNOME files as shipped in Debian and applies our patches
on top.

Revision e1dae37f (diff)
Added by segfault about 1 month ago

Create gdm-tails related files from the original GNOME files (refs: #12551)

We used to ship patched files in the Greeter, which tend to be behind the
original files shipped in Debian. This can potentially cause serious
issues.

This copies the GNOME files as shipped in Debian and applies our patches
on top.

Revision b9d05a3c (diff)
Added by segfault about 1 month ago

Create gdm-tails related files from the original GNOME files (refs: #12551)

We used to ship patched files in the Greeter, which tend to be behind the
original files shipped in Debian. This can potentially cause serious
issues.

This copies the GNOME files as shipped in Debian and applies our patches
on top.

Revision 8caeebb4 (diff)
Added by intrigeri about 1 month ago

Avoid the need to deal with regexps when our needs are met by simpler solutions (refs: #12551)

Revision 26e16837 (diff)
Added by intrigeri about 1 month ago

Don't mask missing /usr/share/gdm/greeter/applications error case (refs: #12551).

Previously, "install -D" would create this directory if it does not exist.
Instead, let sed error out and abort the build if that's the case: this
directory is currently shipped by the gdm3 package; if that ever changes,
chances are that we need to move our gdm-shell-tails.desktop accordingly; better
notice that situation at build time, in a way that will point us towards the
root cause of the problem, instead of masking this change and then wondering why
our Greeter is not loaded correctly.

Revision 068b3720 (diff)
Added by intrigeri about 1 month ago

Make error messages point directly to the problematic file (refs: #12551).

This might save our future selves a little bit of time :)

Revision 9dd227dd (diff)
Added by intrigeri about 1 month ago

Make error message correctly reflect what we're checking for (refs: #12551)

Revision 0b2b5ef9 (diff)
Added by intrigeri about 1 month ago

Don't mask missing /usr/share/gnome-session/sessions error case (refs: #12551).

Same rationale as for /usr/share/gdm/greeter/applications.

Revision 85b6960d (diff)
Added by intrigeri about 1 month ago

Consistently use extended regular expressions (refs: #12551)

Mixing different regexp languages, particularly in a single script, is risky: it
increases the chances that we'll get confused, and for example copy'n'paste
a regexp from one command to the other, while they use different syntax.

Revision e0d2cefa (diff)
Added by intrigeri about 1 month ago

Make regular expressions stricter (refs: #12551)

Let's avoid weird behaviour in case there's ever something like:

RequiredComponents=org.gnome.ShellWhatever

or

RequiredComponents=org.gnome.Shell.Whatever

… in the original file.

Revision 98b01537
Added by intrigeri 26 days ago

Merge branch 'feature/16912-move-greeter-to-main-git-repo+force-all-tests' into devel (Fix-committed: #16912, #12551, #16757)

History

#1 Updated by intrigeri over 2 years ago

  • Target version set to Tails_3.5
  • Parent task changed from #8230 to #11643

I'd like this to be done before the time we might switch to tracking Buster, hence this target version early 2018. Sounds doable? Otherwise please reassign to me.

#2 Updated by alant about 2 years ago

Yes, it looks doable for me before the end of the year.

#3 Updated by intrigeri almost 2 years ago

  • Subject changed from Set up a process to keep our fork of GNOME Shell's .desktop file up-to-date to Set up a process to keep our fork of GNOME Shell's .desktop file and GDM's .session file up-to-date
  • Description updated (diff)

#4 Updated by u over 1 year ago

  • Target version changed from Tails_3.5 to Tails_3.6

Postponing.

#5 Updated by bertagaz over 1 year ago

  • Target version changed from Tails_3.6 to Tails_3.7

#6 Updated by intrigeri over 1 year ago

  • Status changed from Confirmed to In Progress

#7 Updated by bertagaz over 1 year ago

  • Target version changed from Tails_3.7 to Tails_3.8

#8 Updated by intrigeri about 1 year ago

  • Target version changed from Tails_3.8 to Tails_3.9

#9 Updated by intrigeri about 1 year ago

  • Target version changed from Tails_3.9 to Tails_3.10.1

#10 Updated by intrigeri 11 months ago

  • Target version changed from Tails_3.10.1 to Tails_3.11

#11 Updated by CyrilBrulebois 9 months ago

  • Target version changed from Tails_3.11 to Tails_3.12

#12 Updated by intrigeri 9 months ago

alant wrote:

Yes, it looks doable for me before the end of the year.

A while later: we've started working seriously on the port to Buster. Can you please give me a more realistic ETA? Thanks in advance!

#13 Updated by intrigeri 9 months ago

  • Related to Bug #16288: No accessibility support in the Greeter on Buster added

#14 Updated by sajolida 8 months ago

  • Parent task deleted (#11643)

#15 Updated by anonym 8 months ago

  • Target version changed from Tails_3.12 to Tails_3.13

#16 Updated by CyrilBrulebois 6 months ago

  • Target version changed from Tails_3.13 to Tails_3.14

#17 Updated by CyrilBrulebois 4 months ago

  • Target version changed from Tails_3.14 to Tails_3.15

#18 Updated by intrigeri 4 months ago

  • Assignee deleted (alant)
  • Target version changed from Tails_3.15 to Tails_4.0

This ticket was filed 2 years ago, Alan gave an ETA that's now 5 months in the past and did not reply to my request for an updated ETA ⇒ let's have FT (quite possibly me) do this as part of the 4.0 work. This being said, @alant, if you do join one of the Buster sprints, this could be a good task for you there :)

#19 Updated by intrigeri 4 months ago

#20 Updated by intrigeri 3 months ago

  • Status changed from In Progress to Confirmed

#21 Updated by segfault 3 months ago

  • Status changed from Confirmed to In Progress

#22 Updated by segfault 3 months ago

  • Status changed from In Progress to Needs Validation
  • % Done changed from 0 to 10

I wrote a build hook in the Tails repo to copy the original file as shipped in Debian and apply our changes on top. I think doing this during the Tails build is much easier than during Greeter build. If this makes sense, I can prepare another commit in the greeter repo to stop shipping the affected files there.

#23 Updated by intrigeri 3 months ago

  • Feature Branch set to bugfix/12551-keep-gdm-tails-files-up-to-date

#24 Updated by intrigeri 3 months ago

  • Status changed from Needs Validation to In Progress
  • Assignee set to segfault

I like it a lot!

Code review passes except:

  • You need to escape regexp special chars, e.g. the dots in "org.gnome.Shell"
  • Please consistently use extended regexps so we don't have to remember the subtle (or not so subtle) differences between N>1 regexp languages.

Also, what about /usr/share/gnome-shell/modes/gdm-tails.json? It's not explicitly listed on this ticket but it has exactly the same problem: see 8d4b0d95c2ccf06df9cd2fe8b2dee9d4427c2626 in greeter.git.

Just to avoid wasting your time: the corresponding changes in greeter.git shall be based on the feature/buster branch there.

#25 Updated by segfault 3 months ago

intrigeri wrote:

I like it a lot!

Code review passes except:

  • You need to escape regexp special chars, e.g. the dots in "org.gnome.Shell"
  • Please consistently use extended regexps so we don't have to remember the subtle (or not so subtle) differences between N>1 regexp languages.

I pushed a commit to fix that. I hope I didn't miss any other unescaped special chars.

Also, what about /usr/share/gnome-shell/modes/gdm-tails.json? It's not explicitly listed on this ticket but it has exactly the same problem: see 8d4b0d95c2ccf06df9cd2fe8b2dee9d4427c2626 in greeter.git.

It's not clear to me which of the differences between gdm-tails.json and classic.json are on purpose and which caused by updates of classic.json. Here is the complete diff of the classic.json from buster and gdm-tails.json from the feature/buster branch of greeter.git:

diff -Naur /usr/share/gnome-shell/modes/classic.json config/gdm-tails.json 
--- /usr/share/gnome-shell/modes/classic.json    2018-11-02 09:26:47.000000000 +0000
+++ config/gdm-tails.json    2019-06-23 17:52:05.676737605 +0000
@@ -1,9 +1,16 @@
 {
-    "parentMode": "user",
-    "stylesheetName": "gnome-classic.css",
-    "enabledExtensions": ["apps-menu@gnome-shell-extensions.gcampax.github.com", "places-menu@gnome-shell-extensions.gcampax.github.com", "launch-new-instance@gnome-shell-extensions.gcampax.github.com", "window-list@gnome-shell-extensions.gcampax.github.com"],
-    "panel": { "left": ["activities", "appMenu"],
-               "center": [],
-               "right": ["a11y", "keyboard", "dateMenu", "aggregateMenu"]
-             }
+    "hasNotifications": true,
+    "isGreeter": false,
+    "isPrimary": true,
+    "unlockDialog": null,
+    "isLocked": true,
+    "hasWindows": true,
+    "components": ["polkitAgent"],
+    "panel": {
+        "left": [],
+        "center": ["dateMenu"],
+        "right": ["a11y", "keyboard", "aggregateMenu"]
+    },
+    "panelStyle": "loginScreen",
+    "stylesheetName": "gnome-classic.css" 
 }

Just to avoid wasting your time: the corresponding changes in greeter.git shall be based on the feature/buster branch there.

Ack

#26 Updated by segfault 3 months ago

gdm-tails.json was created in 2016, for Tails based on Jessie I assume. Jessie had gnome-shell-extensions 3.14.2, which includes data/classic.json.in. That file wasn't updated since then, so I suspect all the differences are on purpose.

#27 Updated by intrigeri 3 months ago

FWIW it looks like our gdm-tails.json is a kind of hybrid between classic.json (meant for a regular GNOME Classic user session) and the gdm mode in js/ui/sessionMode.js (from GNOME Shell's source tree). And in passing, our panelStyle is loginScreen while GNOME's is login-screen — no idea if loginScreen actually does/means anything relevant.

gdm-tails.json was created in 2016, for Tails based on Jessie I assume. Jessie had gnome-shell-extensions 3.14.2, which includes data/classic.json.in. That file wasn't updated since then, so I suspect all the differences are on purpose.

Yep.

I'm not sure how we should handle this file. Forking the gdm mode at build time would make some sense: the only part we've ever had to update since 2016 is the "panel" bits, and we're using exactly the same config there as the gdm mode. But I don't know if/how we can access this config at Tails build time without downloading the GNOME Shell source package. I'm not sure it's worth the effort.

I propose we:

  • add a comment in this file to sum up what we've reverse-engineered here, so it's easier to find out where the 2 upstream components our file is derived from live
  • create a ticket with target version = Tails_5.0 that will be about:
    • sync'ing this file with Bullseye's
    • creating a similar ticket for 6.0
    • rationale: we need to bother only when we switch to a new Debian release

What do you think?

#28 Updated by segfault 2 months ago

intrigeri wrote:

FWIW it looks like our gdm-tails.json is a kind of hybrid between classic.json (meant for a regular GNOME Classic user session) and the gdm mode in js/ui/sessionMode.js (from GNOME Shell's source tree). And in passing, our panelStyle is loginScreen while GNOME's is login-screen — no idea if loginScreen actually does/means anything relevant.

Good catch. I don't think that setting the style to loginScreen has any effect. login-screen is defined in gnome-shell's CSS, loginScreen is not. We could either change that line to actually use the login-screen style or delete it to keep using the default panel style.

gdm-tails.json was created in 2016, for Tails based on Jessie I assume. Jessie had gnome-shell-extensions 3.14.2, which includes data/classic.json.in. That file wasn't updated since then, so I suspect all the differences are on purpose.

Yep.

I'm not sure how we should handle this file. Forking the gdm mode at build time would make some sense: the only part we've ever had to update since 2016 is the "panel" bits, and we're using exactly the same config there as the gdm mode. But I don't know if/how we can access this config at Tails build time without downloading the GNOME Shell source package. I'm not sure it's worth the effort.

I propose we:

  • add a comment in this file to sum up what we've reverse-engineered here, so it's easier to find out where the 2 upstream components our file is derived from live
  • create a ticket with target version = Tails_5.0 that will be about:
    • sync'ing this file with Bullseye's
    • creating a similar ticket for 6.0
    • rationale: we need to bother only when we switch to a new Debian release

Sounds good, but unfortunately JSON doesn't support comments. We could add the information to the Tails 5.0 ticket instead.

#29 Updated by intrigeri about 2 months ago

Just to be clear: are you waiting for further input from me?

#30 Updated by segfault about 2 months ago

intrigeri wrote:

Just to be clear: are you waiting for further input from me?

Yes, whether you're fine with adding the comment to the Tails 5.0 ticket instead of the JSON file.

#31 Updated by intrigeri about 2 months ago

Yes, whether you're fine with adding the comment to the Tails 5.0 ticket instead of the JSON file.

Sure, go for it :)

#32 Updated by segfault about 1 month ago

  • Related to Bug #16951: Update gdm-tails.json for Bullseye added

#33 Updated by segfault about 1 month ago

  • Feature Branch changed from bugfix/12551-keep-gdm-tails-files-up-to-date to bugfix/12551-keep-gdm-tails-files-up-to-date,greeter:bugfix/12551-keep-gdm-tails-files-up-to-date

intrigeri wrote:

  • create a ticket with target version = Tails_5.0 that will be about:
    • sync'ing this file with Bullseye's
    • creating a similar ticket for 6.0
    • rationale: we need to bother only when we switch to a new Debian release

Done, see #16951.

segfault wrote:

If this makes sense, I can prepare another commit in the greeter repo to stop shipping the affected files there.

I did that, but I didn't test it yet. intrigeri, on #11529 you said that I shouldn't upload releases with official looking versions for testing. But I think I have to upload a release here for testing. Can I use a version like 1.0.9+1.bugfix.12551.1 for that? Or is there another way?

#34 Updated by intrigeri about 1 month ago

you said that I shouldn't upload releases with official looking versions for testing. But I think I have to upload a release here for testing. Can I use a version like 1.0.9+1.bugfix.12551.1 for that? Or is there another way?

This would work. Note that 1.0.9 is not the latest one.

Alternatively, basing your branch on top of #16912 might be easier.

#35 Updated by segfault about 1 month ago

intrigeri wrote:

you said that I shouldn't upload releases with official looking versions for testing. But I think I have to upload a release here for testing. Can I use a version like 1.0.9+1.bugfix.12551.1 for that? Or is there another way?

This would work. Note that 1.0.9 is not the latest one.

Alternatively, basing your branch on top of #16912 might be easier.

Of course, that would be a lot easier. But I wasn't sure whether #16912 would make it into 4.0.

#36 Updated by intrigeri about 1 month ago

segfault wrote:

Of course, that would be a lot easier. But I wasn't sure whether #16912 would make it into 4.0.

I'm now very confident it'll make it, so feel free to rebase your work on top of #16912 :)

#37 Updated by segfault about 1 month ago

  • Status changed from In Progress to Needs Validation
  • Assignee deleted (segfault)

intrigeri wrote:

segfault wrote:

Of course, that would be a lot easier. But I wasn't sure whether #16912 would make it into 4.0.

I'm now very confident it'll make it, so feel free to rebase your work on top of #16912 :)

Done.

#38 Updated by intrigeri about 1 month ago

  • Assignee set to intrigeri

#39 Updated by intrigeri about 1 month ago

  • Blocked by Feature #16912: Move greeter source to main git repo added

#40 Updated by intrigeri about 1 month ago

  • Feature Branch changed from bugfix/12551-keep-gdm-tails-files-up-to-date,greeter:bugfix/12551-keep-gdm-tails-files-up-to-date to bugfix/12551-keep-gdm-tails-files-up-to-date

#41 Updated by intrigeri about 1 month ago

  • Status changed from Needs Validation to In Progress

#42 Updated by intrigeri about 1 month ago

  • Status changed from In Progress to Needs Validation
  • Assignee changed from intrigeri to segfault

I've pushed a bunch of small improvements (mostly maintainability & robustness, some that might be called nitpicking, but it's all in the general spirit of "let's avoid pointing too many loaded guns to our foots and hope nobody ever presses the trigger" :)

Please review these code changes and then reassign to me (I've not looked at test suite results yet).

#43 Updated by segfault about 1 month ago

  • Status changed from Needs Validation to In Progress
  • Assignee changed from segfault to intrigeri

intrigeri wrote:

I've pushed a bunch of small improvements (mostly maintainability & robustness, some that might be called nitpicking, but it's all in the general spirit of "let's avoid pointing too many loaded guns to our foots and hope nobody ever presses the trigger" :)

Please review these code changes and then reassign to me (I've not looked at test suite results yet).

LGTM. e0d2cefabbce2b3d4027d8ceefb27a59c2f0ef93 could cause the script to fail if org.gnome.Shell would ever be the last element in the list (without a trailing ;). But I see that that's better than an incorrect replacement.

#44 Updated by intrigeri about 1 month ago

  • Status changed from In Progress to Needs Validation

LGTM.

Thanks!

e0d2cefabbce2b3d4027d8ceefb27a59c2f0ef93 could cause the script to fail if org.gnome.Shell would ever be the last element in the list (without a trailing ;). But I see that that's better than an incorrect replacement.

Agreed on both counts.

#45 Updated by intrigeri about 1 month ago

Test suite works just as well as current devel and #16912 so I'm going to merge this into the branch for #16912 and delete this one \o/

#46 Updated by intrigeri about 1 month ago

  • Assignee changed from intrigeri to anonym
  • Feature Branch changed from bugfix/12551-keep-gdm-tails-files-up-to-date to feature/16912-move-greeter-to-main-git-repo+force-all-tests

@anonym, this passed the review so you can skip these bits when reviewing #16912.

#47 Updated by intrigeri 26 days ago

  • Status changed from Needs Validation to Fix committed
  • % Done changed from 10 to 100

#48 Updated by intrigeri 26 days ago

  • Status changed from Fix committed to Resolved

Also available in: Atom PDF