Make asktosave_session work

[dimkr]

This script, introduced in 8985811 (Nov 25 2020), never did anything and can't do anything. Nothing runs it, and it doesn't even have execution permissions.

This can be a good workaround for #3986.

[radky2]

@dimkr

This PR is working very nicely for me in BW64 (also tested by forum member @wizard).

Do you anticipate closing this pull request soon?

Would like to include it in the next release of BW64.

Thanks

[dimkr]

@radky2 I think @rizalmart should explain the motivation behind 8985811 first: why was asktosave_session added if it's non-executable, and why is it unused? Did I miss anything?

(And I think it's good to do the yes/no thing in a graphical dialog, #3986 is only one reason.)

[radky2]

OK -- I will follow this thread anticipating reply by @rizalmart

[radky2]

Thank you @dimkr !

Concerning the labels of the YES/NO buttons, it seems the "--yes-label" and "--no-label" tags fail to apply the proper gettext when using Xdialog, but the button labels are correct when using dialog. That is, with Xdialog the buttons will always display "Yes" and "No" rather than "Save" and "No Save".

On the other hand, replacing the above button tags with the "--ok-label" and "--cancel-label" tags will provide the proper button labels with Xdialog (but this fails with dialog):

However, if I use all four of the above button tags, then the button text is correct with both Xdialog and dialog. For example:

$DLGEXE --title " $(gettext 'Save Session') " --timeout 60 --yes-label "$(gettext 'SAVE')" --ok-label "$(gettext 'SAVE')" --no-label "$(gettext 'NO SAVE')" --cancel-label "$(gettext 'NO SAVE')" --yesno "$(gettext 'Press ENTER key to save session... 
Or, press TAB then ENTER to not save session... 
Or, wait 60 seconds to shutdown without saving session...')" 0 0

Your thoughts are welcome.

[dimkr]

I don't mind changing this script. I don't think the text is the most user friendly piece of text I've ever seen, and it probably has zero translations, because it's unused. The strings can be changed freely without fear of breaking translations.

Also, I don't think that dialog support is important, because rc.shutdown has its own implementation of a yes/no dialog (using dialog) and doesn't use asktosave_session. With this PR, asktosave_session is only used from inside X, and rc.shutdown uses its internal implementation as fallback if asktosave_session didn't run (for example, if X crashed or if the user exited X).

(However, if @rizalmart uses this script in QuickPup and does the missing chmod +x during the build process, it's a different story.)

[dimkr]

@radky2 lmk what you think about b112e7e. Using the default labels solves the problem of missing translations, and "no" sounds more like an answer to the question "do you wish save now?" compared to "no save", which might sound more like "no, and I want to have no save file".

EDIT: also pushed 5a732a1, to make the console-based fallback at shutdown consistent with the graphical dialog. Same text, same buttons.

[radky2]

Yes, this works great and also provides consistency for the console-based fallback.

Thanks @dimkr !

[rizalmart]

@radky2 I think @rizalmart should explain the motivation behind 8985811 first: why was asktosave_session added if it's non-executable, and why is it unused? Did I miss anything?

(And I think it's good to do the yes/no thing in a graphical dialog, #3986 is only one reason.)

Here is the explanation. asktosave_session was intended to create an answer to save session upon shutdown process which can be hooked on shutdown event since /dev/console was unaccessible during shutdown if the user session starts especially with PAM.

[dimkr]

@rizalmart But why is it unused? What made you change your mind and leave it broken and unused?

[rizalmart]

@rizalmart But why is it unused? What made you change your mind and leave it broken and unused?

That was originally intended for boot process to ask the user to save current session if the PUPMODE was odd numbers. However due to long queued pull request I overlooked that script. If you are wondering why it was not marked executable because it was coded on GitHub file Editor, not uploaded file.

Right now I'm experimenting shutdown sequence on PAM session

[dimkr]

Right now I'm experimenting shutdown sequence on PAM session

Supporting PAM in Puppy is not easy, it's a very big change. A display manager is needed to get things right (start a session bus, create a session with elogind/logind, etc'), but no display manager understands Puppy's weird two user setup (root vs. spot), and PAM will definitely add some bloat. Some DMs are not yet Wayland-ready themselves and pull X.Org as a dependency, even if they support starting a Wayland compositor after login - that can be a problem, too.

Proper PAM support will need its own solutions instead of wmpoweroff, wmreboot, etc', so as far as I see, this ambition doesn't conflict with this PR.

[rizalmart]

Right now I'm experimenting shutdown sequence on PAM session

Supporting PAM in Puppy is not easy, it's a very big change. A display manager is needed to get things right (start a session bus, create a session with elogind/logind, etc'), but no display manager understands Puppy's weird two user setup (root vs. spot), and PAM will definitely add some bloat. Some DMs are not yet Wayland-ready themselves and pull X.Org as a dependency, even if they support starting a Wayland compositor after login - that can be a problem, too.

Proper PAM support will need its own solutions instead of wmpoweroff, wmreboot, etc', so as far as I see, this ambition doesn't conflict with this PR.

I successfully run login to root with PAM. However it doesn't create elogind session and unable to interact on /dev/console upon shutdown (it has the same problem on non-pam setup upon login). I'm struggling workaround to make elogind work because it threw No Session message.

My Ultimate goal was to integrate Puppy core on traditional linux setup which able to respect different init systems as possible. Once I add that to PR and it triggered panic on this GitHub repo

[dimkr]

I could be wrong, but AFAIK you need a session bus for the elogind+PAM+libpam-elogind stack to work. I tried to get this to work and failed because:

1. The session bus must be started before X, and Puppy doesn't have a "place" for this - this is messy to solve but definitely doable
2. If the session bus is started outside of X, something needs to kill it - normally, it's the DM's job to do this, but Puppy doesn't have a DM
3. Many applications get confused when they talk to elogind via libelogind (or libsystemd), because the session is root's session but these applications run as spot - the PAM stack relies on environment variables, uses XDG_RUNTIME_DIR and assumes that every process belongs to one session, but I haven't found a clean way to create a session that works for both root and spot

[dimkr]

Tests that need to be done when this is merged:

• PUPMODE 13 with periodic saving: it should save periodically without showing a dialog and save on shutdown without showing a dialog
• PUPMODE 13 without periodic saving: it should not save periodically and it should ask whether or not to save on shutdown, using a graphical dialog
• PUPMODE 13 booted with pfix=nox: it should ask whether or not to save after running reboot, and save if the user selects "yes"
• PUPMODE 13 booted with pfix=nox: it should ask whether or not to save after running reboot, and skip saving if the user selects "no
• PUPMODE 66: it should ask whether or not to save on shutdown, using a graphical dialog, and save if the user clicks "yes"
• PUPMODE 66: it should ask whether or not to save on shutdown, using a graphical dialog, and skip saving if the user clicks "no"
• PUPMODE 66 booted with pfix=nox: it should ask whether or not to save after running reboot