Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jovian-greeter: Some fixes, including brokenness fix #440

Closed
wants to merge 4 commits into from

Conversation

samueldr
Copy link
Member

@samueldr samueldr commented Nov 2, 2024

Look at the commits for the rationales.

Big TLDR: some session files could break the greeter.

Also, looking at the log is not great... I left some logging I ended-up needing to figure out an issue.

…tion

Environment variables are messy.

In a session file, `DesktopNames` is used to set up `XDG_CURRENT_DESKTOP`.

```
$ cat /nix/store/ybb3k3kb00xbm9fb1032nfv7ksmnqcgf-wayfire-0.9.0/share/wayland-sessions/wayfire.desktop
[Desktop Entry]
Name=Wayfire
Exec=wayfire
TryExec=wayfire
Icon=
Type=Application
DesktopNames=Wayfire;wlroots
```

Yeah... There's a semicolon. It's probably not correct. But still, it
happens.

Guess what happens when you have a haphazard semicolon into a shell
command invocation?

Hell opens up.
Without this, failure in the greeter would just crash. Not useful.
And promote session type message into `info`.

The net effect is we're now losing the annoying plymouth exception when
it's okay that it can't be ran. It's just noise.
@samueldr samueldr added 1. type: bug Something isn't working 2. priority: 2. normal 2. topic: system integration About our integration of Steam stuff within NixOS 2. topic: Bespoke stuff Things that are specific to Jovian Experiments labels Nov 2, 2024
@samueldr samueldr requested a review from K900 November 2, 2024 03:56
@@ -91,8 +92,11 @@ def start_session(self, command: List[str], environment: List[str]):
logging.debug("Failed to stop Plymouth", exc_info=ex)

# greetd before 0.9.0 doesn't support env
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not putting myself on the hook for learning python enough and greetd to fix this, if it is fixed. Though that would be an appropriate fix, too, maybe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should just yeet this tbh, even 24.05 (!) is on a new enough greetd to support doing things the normal way.

Copy link
Member Author

@samueldr samueldr Nov 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to handle it. (And yes.)

@@ -181,8 +185,17 @@ def _find_sessions(self, sessions: Iterable[str]) -> Optional[Session]:

return None

def handle_exception(exc_type, exc_value, exc_traceback):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't those get logged to greetd journal? I thought they did? Also, should probably exit(1) after.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They did not when I had the issues where I needed to escape that semicolon...

But I might have been holding it wrong? Or looking at it wrong?

(And yes, probably should exit...)

@K900
Copy link
Contributor

K900 commented Nov 3, 2024

Merged most of it manually.

@K900 K900 closed this Nov 3, 2024
@samueldr samueldr deleted the feature/greeter-fixes branch November 3, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. type: bug Something isn't working 2. priority: 2. normal 2. topic: Bespoke stuff Things that are specific to Jovian Experiments 2. topic: system integration About our integration of Steam stuff within NixOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants