Skip to content

Introduce BuildFHSUserEnv with Bubblewrap as an alternative to chrootenv#94442

Merged
Mic92 merged 9 commits intoNixOS:masterfrom
Atemu:buildFHSUserEnvBw
Aug 19, 2020
Merged

Introduce BuildFHSUserEnv with Bubblewrap as an alternative to chrootenv#94442
Mic92 merged 9 commits intoNixOS:masterfrom
Atemu:buildFHSUserEnvBw

Conversation

@Atemu
Copy link
Copy Markdown
Member

@Atemu Atemu commented Aug 1, 2020 •
edited
Loading

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.

@ofborg ofborg bot added 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 This PR causes between 11 and 100 packages to rebuild on Linux. labels Aug 1, 2020
Mic92
Mic92 reviewed Aug 1, 2020
View reviewed changes
@Atemu Atemu force-pushed the buildFHSUserEnvBw branch from 6183999 to 4359af9 Compare August 4, 2020 15:17
@Atemu Atemu force-pushed the buildFHSUserEnvBw branch from 4359af9 to 30f618e Compare August 14, 2020 08:33
@Atemu
Copy link
Copy Markdown
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 Atemu force-pushed the buildFHSUserEnvBw branch from 30f618e to 9ea8a3d Compare August 14, 2020 08:45
@Atemu Atemu requested a review from Mic92 August 14, 2020 08:45
Mic92
Mic92 reviewed Aug 14, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 14, 2020
View reviewed changes
@Atemu Atemu force-pushed the buildFHSUserEnvBw branch from 9ea8a3d to 60aba8c Compare August 14, 2020 09:21
@Atemu Atemu mentioned this pull request Aug 14, 2020
Open
10 tasks
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
Mic92
Mic92 reviewed Aug 17, 2020
View reviewed changes
@Atemu Atemu force-pushed the buildFHSUserEnvBw branch from 60aba8c to 3f5157f Compare August 17, 2020 06:51
@Atemu
Copy link
Copy Markdown
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.

Mic92
Mic92 requested changes Aug 17, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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 Atemu force-pushed the buildFHSUserEnvBw branch from 17b30fb to 1bc200f Compare August 17, 2020 08:06
@Atemu
Copy link
Copy Markdown
Member Author

Atemu commented Aug 17, 2020

snake_case camelCase ;)

@Mic92
Copy link
Copy Markdown
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
Copy Markdown
Member

Mic92 commented Aug 17, 2020

Steam broke because of gstreamer, so it is unrelated.

@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Aug 17, 2020

@GrahamcOfBorg eval

@Atemu
Copy link
Copy Markdown
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
Copy Markdown
Member

Mic92 commented Aug 17, 2020

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

@Mic92 Mic92 force-pushed the buildFHSUserEnvBw branch from c6de416 to dca51dc Compare August 18, 2020 06:44
@Mic92
Copy link
Copy Markdown
Member

Mic92 commented Aug 18, 2020

Fixed via force push.

Mic92
Mic92 approved these changes Aug 18, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
@infinisil
update nixpkgs
d01572f 
to include NixOS/nixpkgs#94442 for a working
steam
@infinisil infinisil mentioned this pull request Aug 22, 2020
Merged
10 tasks
@sugar700 sugar700 mentioned this pull request Aug 25, 2020
Closed
10 tasks
This was referenced Sep 8, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mic92 Mic92 Mic92 approved these changes

Assignees

No one assigned

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 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Setting CAP_SYS_NICE=eip on vrcompositor-launcher crashes SteamVR

3 participants

@Atemu @Mic92 @illegalprime