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: drop gnome2.GConf dependency #340633

Merged
merged 1 commit into from
Sep 9, 2024
Merged

Conversation

fabianhjr
Copy link
Member

@fabianhjr fabianhjr commented Sep 9, 2024

Description of changes

GConf depends on ORBit2 which no longer compiles on #340612

ORBit2 / GConf are EOL/Archived

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@fabianhjr fabianhjr requested review from Atemu and abysssol September 9, 2024 00:11
@github-actions github-actions bot added the 6.topic: steam Steam game store/launcher (store.steampowered.com) label Sep 9, 2024
@ofborg ofborg bot requested a review from jagajaga September 9, 2024 01:15
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Sep 9, 2024
@abysssol
Copy link
Contributor

abysssol commented Sep 9, 2024

Do you know why gnome2.GConf was included in the first place, what in particular required it (it was added in #8299, though I didn't see any mention/discussion of GConf)? Are you confident that it's no longer needed by any games or by steam? Have you tested this change to ensure that nothing is broken by its removal?

If GConf is still required by something (and I haven't yet seen evidence to the contrary; it's common for software to be relied on long after it's end of life), then it seems to me that the best course of action would be to manually override the compiler version for ORBit to gcc_13 so that it still compiles and is usable, thus allowing GConf to remain.

However, I should probably mention that I'm not familiar with steam, nor the general c/c++ ecosystem, so it's entirely possible that I've missed something important.

@fabianhjr
Copy link
Member Author

Are you confident that it's no longer needed by any games or by steam?

Not really, it could cause breakage.

About to test after booting into the gcc-14 branch. There is still some things I have to fix or build before that.

manually override the compiler version for ORBit to gcc_13

afaik that can also cause breakage and why it wasn't a course of action I considered. UnU

@Atemu
Copy link
Member

Atemu commented Sep 9, 2024

Is it possible to build gconf without orbit?

Otherwise, I don't see why we shouldn't just pin gcc for orbit instead. Sure, that kicks down the bucket for a few years but I'm not aware of any compat issues here. This is common practice. If there were compat issues between binaries built by different compilers, that'd be a bug in the compiler(s).

@fabianhjr
Copy link
Member Author

Have been able to build and run Steam without GConf on gcc14.

Additionally lightlly played some games like Disco Elysium / Dota 2

@fabianhjr
Copy link
Member Author

I feel like if GConf isn't needed in general it would be better to remove it from the standard fhsenv.

@K900
Copy link
Contributor

K900 commented Sep 9, 2024

gconf >3 might be needed, 2 definitely is not in most cases.

@fabianhjr
Copy link
Member Author

This drops GConf 3.2.6 specifically.

  pname = "gconf";
  version = "3.2.6";

@K900
Copy link
Contributor

K900 commented Sep 9, 2024

Ugh I mean dconf (from GNOME 3+) over gconf (from GNOME 2)

@fabianhjr fabianhjr mentioned this pull request Sep 9, 2024
13 tasks
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Sep 9, 2024

First of all: If any one or two games require this, then they should use the override options. We need to stop that the steam fhs is a trash pile for all libraries anyone can ever need.

Then #39976 and I think the required daemon for gconf is gone for years, so it doesn't even work. (Can't find the comment though)

GConf depends on ORBit2 which no longer compiles on NixOS#340612

ORBit2 / GConf are EOL/Archived:

- https://gitlab.gnome.org/Archive/orbit2 (last commit on main branch 8 years ago)
- https://gitlab.gnome.org/Archive/gconf (last commit on main branch 8 years ago)
@fabianhjr
Copy link
Member Author

Had to rebase due to conflict after merge of #340710

@SuperSandro2000
Copy link
Member

Those libraries have been deprecated for 10+ years and require daemons that no-one has running these days.

#322599

@K900 K900 merged commit 013c5df into NixOS:master Sep 9, 2024
8 of 9 checks passed
@K900
Copy link
Contributor

K900 commented Sep 9, 2024

Let's just send it and see what happens.

@fabianhjr fabianhjr deleted the steam-drop-gconf branch September 9, 2024 14:44
@abysssol
Copy link
Contributor

abysssol commented Sep 9, 2024

First of all: If any one or two games require this, then they should use the override options. We need to stop that the steam fhs is a trash pile for all libraries anyone can ever need.

How is a normal user supposed to figure out what dependency is missing from a game and add that to extraPackages? Is this a well documented process?

I would consider myself a relatively advanced nix user, but I've never found any relevant documentation on this. Whenever I have found a Steam game that has a Linux version that doesn't work correctly, the only options I had were either to force enable Steam's Linux runtime in the individual game's settings, or if that failed then force the use of Proton with the Windows version of the game.

This seemingly also requires each user to independently troubleshoot these missing dependencies, wasting time solving a problem that others have likely already solved.

One potential solution (in addition to better documentation) would be to create a nixos option where a list of game names may be specified from a set where each entry maps to that game's required dependencies.

Perhaps something like the following:

programs.steam = {
  enable = true;
  gameCompat = [
    "Quaver"
    "Mark of the Ninja: Remastered"
    "Dead Cells"
    "Valheim"
  ];
};

And it might be implemented something like this:

let
  cfg = config.programs.steam;

  # could be defined in another file and `import`ed
  compatLibs = {
    "Quaver" = [
      # required dependencies
    ];
    "Mark of the Ninja: Remastered" = [
      # required dependencies
    ];
    # etc
  };

  gameCompatPackages = builtins.concatMap (game: builtins.getAttr game compatLibs) cfg.gameCompat;
in
{
  options.programs.steam = {
    gameCompat = lib.mkOption {
      type = lib.types.listOf (lib.types.enum (builtins.attrNames compatLibs));
      default = [ ];
      example = [ "Stardew Valley" ];
      description = ''
        Adds dependencies required by the specified games.

        Only relevant for native Linux games, not Windows games run under Proton.
      '';
    };

    # `gameCompatPackages` could then be combined with `extraPackages`
    package = lib.mkOption {
      # code omitted
      apply = steam: steam.override (prev: {
          # code omitted
          extraPkgs = p: (
            cfg.extraPackages
              ++ gameCompatPackages
              ++ lib.optionals (prev ? extraPkgs) (prev.extraPkgs p)
          );
          # code omitted
      });
    };
  };
}

Then #39976 and I think the required daemon for gconf is gone for years, so it doesn't even work. (Can't find the comment though)

Those libraries have been deprecated for 10+ years and require daemons that no-one has running these days.

#322599

Deprecated libraries may still be required by something; "deprecated" is in no way equivalent to "unused".

If the daemon is gone, that could mean that anything that would break from GConf being removed is already broken, but not necessarily. It may be possible that a program may dynamically link against the GConf library, crashing due to the linker being unable to find the library, but if the library is present and the function call returns an error due to the daemon not being available, perhaps another fallback code path is taken that doesn't require GConf (this is really more so a fault with dynamic linking).

This is why it would be best to know why exactly GConf was added in the first place; we would be able to test whether what required it still does at all (whether some game or steam itself), and if it is still required, whether the dependant is important enough. Consider the principle of Chesterton's fence; it's always best to understand why something is the way it is before changing or removing it.

@SuperSandro2000
Copy link
Member

the only options I had were either to force enable steam's Linux runtime in the individual game's settings

That's the plan. Otherwise we fight an downhill battle against slow updating linux distros.
Then the problem is hopefully in many cases with the steam linux runtime and not the packages inside nixpkgs.

One potential solution (in addition to better documentation) would be to create a nixos option where a list of game names may be specified from a set where each entry maps to that game's required dependencies.

That's a good fit for an out of tree module. We don't want to collect potential endless lists of libraries for proprietary games you cannot easily verify yourself.
Also it would be kinda annoying to test any new games as you would need to rebuild your system with only that package to make sure nothing common is missing.

Deprecated libraries may still be required by something; "deprecated" is in no way equivalent to "unused".

The equivalent is that you're game only runs on Windows Vista. Way to old for the tastes of NixOS and we want to drop gnome2 packages since years.

This is why it would be best to know why exactly GConf was added in the first place; we would be able to test whether what required it still does at all (whether some game or steam itself), and if it is still required, whether the dependant is important enough.

Before I take a look at the git blame, I bet 5 I-was-right-coins that we cannot easily figure that out.

Funny: why do we use curl with gnutls 7dff2c5 🤷🏼

So it was added to steam because steam itself required it which is no longer the case a3ef4b9

So we cannot know which games might need it. The truth is probably more than 0 but in reality we cannot know.

@abysssol
Copy link
Contributor

the only options I had were either to force enable Steam's Linux runtime in the individual game's settings

That's the plan.

Okay, that would make sense, except wasn't the point of #8299 to disable Steam's runtime and use native dependencies? Am I misunderstanding? Was that never the intention? Or was that how Steam was handled in the past, but isn't any more? That pr is almost a decade old; I wouldn't be surprised.

The equivalent is that your game only runs on Windows Vista. Way to old for the tastes of NixOS and we want to drop gnome2 packages since years.

So it was added to Steam because Steam itself required it which is no longer the case a3ef4b9

So we cannot know which games might need it. The truth is probably more than 0 but in reality we cannot know.

If the idea is to ensure that Steam works correctly, and that games are only expected to work with Steam's runtime, then removing GConf does make sense, since Steam apparently no longer needs it.

And the same probably applies to curl with gnutls (#340710), though performance may have been a reason for switching (unfortunately the reason wasn't left in the code or commit message; I'll take this as a lesson to be much more generous with comments/explanations of reasoning for code changes).

@K900
Copy link
Contributor

K900 commented Sep 10, 2024

Steam has been using its own packaged runtime libraries for a very long time now, with no supported option to switch this off.

@SuperSandro2000
Copy link
Member

Okay, that would make sense, except wasn't the point of #8299 to disable Steam's runtime and use native dependencies? Am I misunderstanding? Was that never the intention? Or was that how Steam was handled in the past, but isn't any more? That pr is almost a decade old; I wouldn't be surprised.

NixOS cannot possible provide the binaries in old versions of the runtime. Also we then don't need to debug ABI incompatiblies or need to worry about the exact versions of the libraries. The runtime is downloaded anyway and saves us a bunch of maintenance and debugging and more things hopefully just work and the steam package becomes smaller and easier to maintain.

And the same probably applies to curl with gnutls (#340710), though performance may have been a reason for switching (unfortunately the reason wasn't left in the code or commit message; I'll take this as a lesson to be much more generous with comments/explanations of reasoning for code changes).

That probably just matched what steam was using at the time. IIRC curl is used to upload crash reports. Does performance matter there? Probably not.

Comments need to be on point and as long as necessary but as short as privilege. No one likes to read really long comments that have no information.

@Atemu
Copy link
Member

Atemu commented Sep 11, 2024

Steam has been using its own packaged runtime libraries for a very long time now, with no supported option to switch this off.

Right but only for steam itself. Native titles run through steam will attempt to use the fhsenv's binaries unless they opt-into using the Steam linux runtime or you force its usage.

This is why you need to be a bit more careful about dropping old versions of software in Steam's fhsenv; their intended use is by games. Most games are legacy abandonware after a year or so and are still enjoyed many decades after being abandoned.
We can't just drop things out of the steam fhsenv just because they're old/legacy. Fixing build failures in legacy software is likely to be a disproportionate effort, so in this case it probably couldn't really be avoided but we shouldn't strive to remove legacy packages from the Steam fhsenv.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Sep 11, 2024

The steam package right now is multiple gigabytes in size and almost as big as the rest of my desktop.

I guess we are then moving all libraries not required for steam out of the small package, so that people that know what they are doing, don't need to download gigabytes of libraries they don't really want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: steam Steam game store/launcher (store.steampowered.com) 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants