hyprwm / Hyprland

Hyprland is an independent, highly customizable, dynamic tiling Wayland compositor that doesn't sacrifice on its looks.
https://hyprland.org
BSD 3-Clause "New" or "Revised" License
18.57k stars 772 forks source link

Hardcoded /tmp/hypr problematic #155

Closed NickHastings closed 2 years ago

NickHastings commented 2 years ago

Hi,

this looks like an interesting project.

While looking at the code I noticed that the hardcoded paths /tmp/hypr/hyprland.log and /tmp/hypr/.socket.sock which will break if more than one instance of hyprland is running.

I would suggest making use of the XDG_RUNTIME_DIR and WAYLAND_DISPLAY environment variables for the socket file. Eg replace using /tmp/hyper/ with ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}-hypr/.

This location should be fine for the socket, but strictly speaking, log files shouldn't go under ${XDG_RUNTIME_DIR}. However, I'm guessing that the currently implemented logging will be changed as hyprland becomes more mature. Is there any reason not to use wlr_log() provided by wlroots?

Cheers,

Nick.

vaxerski commented 2 years ago

why is this a problem? You should not be running multiple sessions of hyprland at once anyways.

Is there any reason not to use wlr_log() provided by wlroots?

yes, it's in the name, it's for wlr, not me. I prefer having my own logs.

NickHastings commented 2 years ago

Hi,

why is this a problem? You should not be running multiple sessions of hyprland at once anyways.

You don't run nested compositors for testing purposes? Or perhaps hyprland can't run nested? I do this all the time with river and sway.

Also, consider what would happen if a user runs hyprland, then logs out and then another user then runs hyprland. The log file created by the first user would still exist and be owned by that user. In this case, the unlink("/tmp/hypr/hyprland.log"); in the CCompositor constructor would fail.

Is there any reason not to use wlr_log() provided by wlroots?

yes, it's in the name, it's for wlr, not me.

It can be used by compositors. Both sway and river use it.

I prefer having my own logs.

Fair enough, but again, hard coding that path seems like bad practice to me.

Cheers,

Nick.

vaxerski commented 2 years ago

You don't run nested compositors for testing purposes? Or perhaps hyprland can't run nested?

I run nested with debug and then it uses debug paths.

Also, consider what would happen if a user runs hyprland, then logs out and then another user then runs hyprland. The log file created by the first user would still exist and be owned by that user. In this case, the unlink("/tmp/hypr/hyprland.log"); in the CCompositor constructor would fail.

it wouldnt matter really

NickHastings commented 2 years ago

You don't run nested compositors for testing purposes? Or perhaps hyprland can't run nested?

I run nested with debug and then it uses debug paths.

Ok.

Also, consider what would happen if a user runs hyprland, then logs out and then another user then runs hyprland. The log file created by the first user would still exist and be owned by that user. In this case, the unlink("/tmp/hypr/hyprland.log"); in the CCompositor constructor would fail.

it wouldnt matter really

It's not just that the unlink() would fail, it would then mean that the log file could not be written to by the second user.

vaxerski commented 2 years ago

unlink would not fail, the file is not locked all the time by hyprland.

NickHastings commented 2 years ago

unlink would not fail, the file is not locked all the time by hyprland.

File ownership is the issue here. The file is owned by user1 and user2 is trying to unlink it.

vaxerski commented 2 years ago

why in the world would you want to do that though? I mean fair enough, that would be an issue, but why?

NickHastings commented 2 years ago

why in the world would you want to do that though? I mean fair enough, that would be an issue, but why?

This would happen if one user logged out and then another user logged in. Pretty much standard practice on a multiuser operating system.

vaxerski commented 2 years ago

done in 9486a230c74e8873621fdf64a33adaaa660762e3 and 6f3b004199a6b1bb0a8f5431d677ab926bdf19f3

jmc-figueira commented 1 year ago

why in the world would you want to do that though? I mean fair enough, that would be an issue, but why?

Sorry for necroing this issue, but I am facing an issue related to this.

I am using greetd with gtkgreet and it needs to load up Hyprland on the greeter user to "bootstrap" itself, as shown (taken from a Nix configuration, but this shouldn't matter for this issue):

greetd = {
  enable = true;
  restart = true;

  settings = {
    default_session = {
      command = "Hyprland --config /etc/greetd/hyprland-config";
      user = "greeter";
    };
  };
};

# (...)

"greetd/hyprland-config".text = ''
  monitor = ,preferred,auto,1.6

  input {
    kb_layout = pt
    kb_model = apple
    kb_variant = mac
  }

  exec-once = GTK_THEME=Adwaita:dark ${pkgs.greetd.gtkgreet}/bin/gtkgreet -l; hyprctl dispatch exit 0
'';

"greetd/environments".text = ''
  hyprland-wrapped
  zsh
'';

The issue I've encountered is that this creates the /tmp/hypr/{INSTANCE SIGNATURE}/... directory as belonging to the greeter user:

drwxr-xr-x   - greeter       3 set 05:54 ├── hypr
drwxr-x---   - greeter       3 set 05:54 │  └── _1662180875
srwxr-xr-x   0 greeter       3 set 05:54 │     ├── .socket.sock
srwxr-xr-x   0 greeter       3 set 05:54 │     ├── .socket2.sock
.rw-r--r-- 19k greeter       3 set 05:55 │     └── hyprland.log

This then prevents the instance of Hyprland launched by greetd with my user session from creating its own subdirectory according to its instance signature. While not being able to write logs might be a relatively minor issue, this has the secondary issue that the sockets will not be created, rendering any feature which requires them (such as hyprctl) unusable. Perhaps a flag similar to --config to specify a separate path for /tmp/hypr would fix the issue?