Skip to content

gnome: switch to makeScopeWithSplicing#154751

Closed
NickCao wants to merge 1 commit intoNixOS:masterfrom
NickCao:gnome
Closed

gnome: switch to makeScopeWithSplicing#154751
NickCao wants to merge 1 commit intoNixOS:masterfrom
NickCao:gnome

Conversation

@NickCao
Copy link
Member

@NickCao NickCao commented Jan 12, 2022

Motivation for this change

makeScope breaks splicing thus makes cross compilation hard in package sets like gnome, by switching to makeScopeWithSplicing, the situation can be dramatically improved.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Jan 12, 2022
@NickCao NickCao added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jan 12, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 12, 2022
@NickCao
Copy link
Member Author

NickCao commented Jan 12, 2022

Use gnome-power-manager as an example, it won't cross build without this pr applied:

gnome-power-manager> /nix/store/5h3ziqff9fkn0jvqmvl3bssfd5l0dg5n-glib-riscv64-unknown-linux-gnu-2.70.2-dev/bin/glib-compile-resources: line 1: syntax error: unexpected "("

And it is obvious that glib from hostPlatform instead of buildPlatform was used, despite that it is in nativeBuildInputs.
With this pr and #148618, #151184 and NickCao@01192ff applied, it can be successfully crossed from x86_64-linux to riscv64-linux, the same applies to other packages in the gnome package set.

@NickCao NickCao marked this pull request as ready for review January 12, 2022 12:36
@NickCao
Copy link
Member Author

NickCao commented Jan 12, 2022

cc @Mindavi as I think you might also be interested.

@Ericson2314
Copy link
Member

Thanks for doing this! Always make me very happy to see others fixing cross compilation issues.

@NickCao
Copy link
Member Author

NickCao commented Jan 16, 2022

After the removal of aliases, cross evaluation is failing on throw. While they are outside the scope, they seem to be brought back in by the spliced package sets.

@NickCao NickCao marked this pull request as draft January 16, 2022 02:14
@jtojnar
Copy link
Member

jtojnar commented Jan 16, 2022

Can we detect that the attrset is being spliced and hide the aliases in that case as well?

@jtojnar
Copy link
Member

jtojnar commented Jan 16, 2022

This ugly patch seems to work:

--- a/pkgs/desktops/gnome/default.nix
+++ b/pkgs/desktops/gnome/default.nix
@@ -1,12 +1,12 @@
-{ config, pkgs, lib, splicePackages, newScope
+{ config, pkgs, lib, splicePackages, newScope, isSpliced ? false
 , pkgsBuildBuild, pkgsBuildHost, pkgsBuildTarget, pkgsHostHost, pkgsTargetTarget }:
 
 lib.makeScopeWithSplicing splicePackages newScope {
-  selfBuildBuild = pkgsBuildBuild.gnome;
-  selfBuildHost = pkgsBuildHost.gnome;
-  selfBuildTarget = pkgsBuildTarget.gnome;
-  selfHostHost = pkgsHostHost.gnome;
-  selfTargetTarget = pkgsTargetTarget.gnome or {};
+  selfBuildBuild = pkgsBuildBuild.gnome.override { isSpliced = true; };
+  selfBuildHost = pkgsBuildHost.gnome.override { isSpliced = true; };
+  selfBuildTarget = pkgsBuildTarget.gnome.override { isSpliced = true; };
+  selfHostHost = pkgsHostHost.gnome.override { isSpliced = true; };
+  selfTargetTarget = if pkgsTargetTarget ? gnome then pkgsTargetTarget.gnome.override { isSpliced = true; } else { };
 } (self: { }) (spliced0: { })
 (self: let inherit (self) callPackage; in {
   updateScript = callPackage ./update.nix { };
@@ -288,7 +288,7 @@ lib.makeScopeWithSplicing splicePackages newScope {
   gnome-autoar = callPackage ./misc/gnome-autoar { };
 
   gnome-packagekit = callPackage ./misc/gnome-packagekit { };
-}) // lib.optionalAttrs (config.allowAliases or true) {
+}) // lib.optionalAttrs (config.allowAliases or true && !isSpliced) {
 #### Legacy aliases. They need to be outside the scope or they will shadow the attributes from parent scope.
 
   bijiben = throw "The ‘gnome.bijiben’ alias was removed on 2022-01-13. Please use ‘gnome.gnome-notes’ directly."; # added 2018-09-26

@NickCao
Copy link
Member Author

NickCao commented Jan 16, 2022

This seems to be a viable fix, really ugly though. Let's keep this pr as a draft until we really figure out the proper way to deal with the conflict between makeScope and splicing.

@jtojnar
Copy link
Member

jtojnar commented Jan 16, 2022

Well, applying the aliases on top of the scope is really ugly too. And so is the splicing code.
Maybe the best option is really just biting the bullet and moving everything under gnome into the top-level.

@Ericson2314
Copy link
Member

Hmm as jank as splicing is, I am surprised the throwing attributes would effect the other attributes.

@jtojnar
Copy link
Member

jtojnar commented Jan 16, 2022

The issue is that the throwing attributes shadow attributes from pkgs, which are used by other packages in the scope.

We are intentionally moving them outside the scope to avoid the shadowing while leaving them accessible as attributes under gnome. But that will fail when the attributes are actually pulled through pkgs*.gnome since those include the throwers.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 31, 2022
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 11, 2022
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2024
@NickCao
Copy link
Member Author

NickCao commented Sep 9, 2024

The gnome scope no longer exists.

@NickCao NickCao closed this Sep 9, 2024
@NickCao NickCao deleted the gnome branch September 10, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: GNOME GNOME desktop environment and its underlying platform 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants