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

Introduce BuildFHSUserEnv with Bubblewrap as an alternative to chrootenv #94442

Merged
merged 9 commits into from Aug 19, 2020

Conversation

Atemu
Copy link
Member

@Atemu Atemu commented Aug 1, 2020

Motivation for this change

#55973 (comment)

Fixes #92798

/cc @Mic92 @illegalprime

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Atemu
Copy link
Member Author

Atemu commented Aug 14, 2020

I've come up with a cleaner solution to selectively use bubblewrap, will rebase to fix merge conflicts now

@Atemu
Copy link
Member Author

Atemu commented Aug 17, 2020

Sorry but this is just bikeshedding at this point. The FHSEnv infrastructure is in need of a refactoring but that's WAY out of scope for this PR.

Feel free to commit that stuff yourself (the branch is open) but honestly, it'd be better put in a separate PR. After we have deprecated chrootenv ideally.

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Sorry but consistent variable naming is not just bikeshedding. This is a builder and changing those variables later on can potentially break someones code.

@Atemu
Copy link
Member Author

Atemu commented Aug 17, 2020

The variable names are consistent with the original build-fhs-chrootenv. Many of your suggestions are actually on variable names from the original. Take a look at the blame.

No, those names aren't what they should be but this PR is not a general refactoring of buildFHSUserEnv, variable names and code style. Intentionally so. Else it'd be going nowhere and end up with the same fate as the original PR.
Perhaps it's an unavoidable fate though :/

@Atemu
Copy link
Member Author

Atemu commented Aug 17, 2020

snake_case camelCase ;)

@Mic92
Copy link
Member

Mic92 commented Aug 17, 2020

Result of nixpkgs-review pr 94442 1

1 package failed to build:
- steam-run-native
10 packages built:
- android-studio
- androidStudioPackages.beta
- androidStudioPackages.canary
- androidStudioPackages.dev
- kodiPlugins.steam-launcher
- lutris
- lutris-free
- steam
- steam-run
- steamcmd

@Mic92
Copy link
Member

Mic92 commented Aug 17, 2020

Steam broke because of gstreamer, so it is unrelated.

@Mic92
Copy link
Member

Mic92 commented Aug 17, 2020

@GrahamcOfBorg eval

@Atemu
Copy link
Member Author

Atemu commented Aug 17, 2020

It's actually just the -native version, the regular (steamrt) steam is fine.

Was the nix-update hash change intended? That seems out of place.

@Mic92
Copy link
Member

Mic92 commented Aug 17, 2020

No sorry. I was testing nix-update with nix-update and it staged this file.

@Mic92
Copy link
Member

Mic92 commented Aug 18, 2020

Fixed via force push.

Copy link
Member

@Mic92 Mic92 left a comment

Choose a reason for hiding this comment

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

Good to merge after next eval.

@Mic92 Mic92 merged commit bd0e645 into NixOS:master Aug 19, 2020
@Atemu
Copy link
Member Author

Atemu commented Aug 20, 2020

Thank you!

@Atemu Atemu deleted the buildFHSUserEnvBw branch August 20, 2020 03:32
infinisil added a commit to infinisil/system that referenced this pull request Aug 21, 2020
to include NixOS/nixpkgs#94442 for a working
steam
@infinisil infinisil mentioned this pull request Aug 22, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting CAP_SYS_NICE=eip on vrcompositor-launcher crashes SteamVR
3 participants