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

steam: Do $PATH lookup in steam.desktop instead of hardcoding derivation #101859

Merged
merged 1 commit into from Oct 27, 2020

Conversation

andir
Copy link
Member

@andir andir commented Oct 27, 2020

Note: I received this patch via E-Mail. As I'm not using any of the
"fancy" DEs I can't really judge if this is how and what we want to do
here.

steam: Do $PATH lookup in steam.desktop instead of hardcoding derivation

The desktop application and the absoloute path work fine.
But consider desktop environments such as KDE where, in the application
menu, one can right click entries and pin them to widgets/panels, add
them to the desktop, etc.

Doing so effectively means copying
/run/current-system/sw/share/applications/steam.desktop to
~/.local/share/plasma_icons/ or ~/Desktop/, i.e. managed stated gets
duplicated outside the nix scope.

The problem here is that steam.desktop hardcodes

Exec=/nix/store/-steam/bin/steam %U

this means such copies will point at wrong/outdated derivations once
the steam package changes, i.e. widgets/panels/desktop icons will no
longer work and must be recreated.

Therefore replace the absoloute path with a $PATH lookup to allow "safe"
copying; this isn't optimal but other applications such Firefox and
Thunderbrid currently behave the same way ($PATH lookup in their
.desktop file).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS - nix-build -A steam

The desktop application and the absoloute path work fine.
But consider desktop environments such as KDE where, in the application
menu, one can right click entries and pin them to widgets/panels, add
them to the desktop, etc.

Doing so effectively means copying
/run/current-system/sw/share/applications/steam.desktop to
~/.local/share/plasma_icons/ or ~/Desktop/, i.e. managed stated gets
duplicated outside the nix scope.

The problem here is that steam.desktop hardcodes

	Exec=/nix/store/<derivation hash>-steam/bin/steam %U

this means such copies will point at wrong/outdated derivations once
the steam package changes, i.e. widgets/panels/desktop icons will no
longer work and must be recreated.

Therefore replace the absoloute path with a $PATH lookup to allow "safe"
copying;  this isn't optimal but other applications such Firefox and
Thunderbrid currently behave the same way ($PATH lookup in their
.desktop file).
@andir andir changed the title ###### Motivation for this change steam: Do $PATH lookup in steam.desktop instead of hardcoding derivation Oct 27, 2020
@andir andir requested a review from jagajaga October 27, 2020 15:40
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

This is in fact the correct way to go. Most application launches, with a couple exceptions, will make "launcher item" (I think in xfce), and it will have the hardcoded nix store path forever and you'd have to remove them

@worldofpeace worldofpeace merged commit c110da1 into NixOS:master Oct 27, 2020
@worldofpeace
Copy link
Contributor

port to stable needed

@worldofpeace worldofpeace added the 9.needs: port to stable A PR needs a backport to the stable release. label Oct 27, 2020
@andir
Copy link
Member Author

andir commented Oct 28, 2020 via email

@andir andir deleted the steam-desktop branch October 28, 2020 01:10
@erictapen erictapen added 8.has: port to stable A PR already has a backport to the stable release. and removed 9.needs: port to stable A PR needs a backport to the stable release. labels Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: port to stable A PR already has a backport to the stable release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants