Skip to content

Comments

Revert "nixos/stub-ld: init module"#287246

Closed
zimbatm wants to merge 1 commit intoNixOS:masterfrom
zimbatm:revert-269551
Closed

Revert "nixos/stub-ld: init module"#287246
zimbatm wants to merge 1 commit intoNixOS:masterfrom
zimbatm:revert-269551

Conversation

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Feb 8, 2024

This reverts commit 0863f6d introduced in #269551.

Fixes:

error: i686 Linux package set can only be used with the x86 family.

Description of changes

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.05 Release Notes (or backporting 23.05 and 23.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 8, 2024
@zimbatm zimbatm requested a review from tejing1 February 8, 2024 13:07
@tejing1
Copy link
Contributor

tejing1 commented Feb 8, 2024

Reverting seems extreme. Problems appear to be rather rare, and do have solutions for those having them, even if they aren't particularly ergonomic. If nothing else, this is always an option:

{ lib, modulesPath, ... }:
{
  disabledModules = [ (modulesPath + "/config/ldso.nix") ];
  options.environment.ldso = lib.mkOption { type = lib.types.unspecified; };
  options.environment.ldso32 = lib.mkOption { type = lib.types.unspecified; };
}

Also, keep in mind that reverting would break the config of anyone who explicitly disabled it when it came out, since the option would cease to exist.

Finally, this isn't even the right commit to revert. It's the ldso.nix module that has created issues.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 8, 2024
@github-actions github-actions bot removed 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Feb 12, 2024
@zimbatm
Copy link
Member Author

zimbatm commented Feb 12, 2024

This has to be fixed one way or another. The error is hard to debug; I don't expect most users to be able to bisect their nixpkgs to find where the problem was introduced. I wouldn't be so adamant if it were a leaf module, but this is core to the OS. The issue has been standing for two months.

I pushed a partial revert to be more graceful with the change. If you have a fix, then I won't push for that revert.

@tejing1
Copy link
Contributor

tejing1 commented Feb 12, 2024

The error is hard to debug; I don't expect most users to be able to bisect their nixpkgs to find where the problem was introduced.

This is rather difficult to fix. Adding an option to more easily disable the relevant functionality is easy enough, but it doesn't actually solve this part. I'm open to suggestions about how to do so. I currently still don't actually know much about the circumstances that cause issues here except that (probably) using nixos with a musl-based stdenv (on x86_64) causes an issue, though that doesn't give the same error message you quoted.

"I got an error message, so let's nuke the commit it bisects to" isn't exactly a constructive approach. Can we focus on what's actually going wrong, what circumstances it can happen in, and how best to fix it for everyone?

The issue has been standing for two months.

I'd like to point out here that no actual issue was ever filed (and still hasn't been), no reproducer, information about the circumstances of the error, or useful part of the trace was given, and you never followed up on the request for more information, or showed any further interest in solving the problem. Nor did anyone else report a similar issue afaik. It's not unreasonable that nothing happened until someone else came along with more useful information a week ago. And I still don't have enough information to feel certain if your issue is actually the same as theirs.

It's clear the misbehavior is confined to an extremely narrow portion of the userbase. Yoyo-ing functionality for everyone for that still seems unwarranted to me. Let's just get it diagnosed and fixed as quick as we can.

The basic question is: What should we change pkgs.stdenv.isx86_64 to in order to properly count out the cases where the i686 package set doesn't evaluate correctly? (Assuming that your problem is also a variant of that issue)

Remove the `environment.ldso32` option until it can be better thought
out.

The option creates a new instance of nixpkgs and doesn't work on all
architectures.

Fixes:

    error: i686 Linux package set can only be used with the x86 family.
@zimbatm
Copy link
Member Author

zimbatm commented Feb 13, 2024

Same, I'd rather put the energy towards a fix than argue.

Fundamentally, we have to avoid creating new instances of nixpkgs. I haven't benchmarked it, but they tend to be pretty expensive eval-wise. And both issues are related to this.

Anyways, I found a better compromise in #288509.

@zimbatm zimbatm closed this Feb 13, 2024
@zimbatm zimbatm deleted the revert-269551 branch February 13, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants