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

Revert "rustc: build with jemalloc" #192328

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Revert "rustc: build with jemalloc" #192328

merged 1 commit into from
Sep 22, 2022

Conversation

zowoq
Copy link
Contributor

@zowoq zowoq commented Sep 21, 2022

This reverts commit 2a69902.

hopefully this fixes building x86_64-darwin under emulation on aarch64-darwin

Description of changes
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.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

This reverts commit 2a69902.

hopefully this fixes building x86_64-darwin under emulation on aarch64-darwin
@winterqt
Copy link
Member

What about the linked issue (rust-lang/rust#92173), though...? Can we risk that?

@ofborg ofborg bot requested review from cstrahan, madjar and globin September 21, 2022 23:39
@ofborg ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Sep 21, 2022
@zowoq
Copy link
Contributor Author

zowoq commented Sep 21, 2022

Can we risk that?

I'd rather not but hopefully this will unblock staging-next and buy us a bit of time.

@winterqt
Copy link
Member

I have a more proper solution for this, but don't have any more time today to run what I hope to be the final test build of rustc/cargo/fd (takes a while under Rosetta).

If I can get this working (tomorrow), I'll PR it. If not, we can merge this to unblock so we have more time to figure this out.

Unless you want to merge this now instead of waiting -- let me know.

@zowoq
Copy link
Contributor Author

zowoq commented Sep 22, 2022

I have a more proper solution for this

Can you push it to a branch? Maybe we can get someone else to test it?

@winterqt
Copy link
Member

winterqt commented Sep 22, 2022

After actually reading the comment I was basing this off of (rust-lang/rust#92173 (comment)), it looks like my idea (building rustc with the 11.0 SDK and system allocator) probably won't fix this.

We could always give it a try (winterqt@b34decf), but sadly it looks like we either have to fix compiling jemalloc under Rosetta (I may take a stab at this, though I have no idea what I'm doing), keep a native x86_64-darwin builder around (and use jemalloc), or use the system allocator (and maybe run into random crashes (see rust-lang/rust#92173)). I say these are the only options because it looks like the SDK bump wouldn't even help here, as it wouldn't get us to Xcode 13.3. (I have no clue what SDK version these issues started at though, so...)

@zowoq
Copy link
Contributor Author

zowoq commented Sep 22, 2022

winterqt@b34decf

I'd be a bit concerned about having rustc on 11.0 and cargo/buildRustpackage on 10.12, not sure if it might need other changes as well? #180931 is also working on moving rust to 11.0.

@winterqt
Copy link
Member

#180931 is also working on moving rust to 11.0.

Yup, I know -- this was just a stopgap (if it actually fixed the issue at hand), and might've caused some issues for a few packages.

(Though Go hasn't had any issues with this, AFAIK?)

@zowoq
Copy link
Contributor Author

zowoq commented Sep 22, 2022

Though Go hasn't had any issues with this, AFAIK?

Don't think there has been any issues but may not be a useful indicator, relatively few go packages use sdk features that aren't already propagated by go (and it only uses a couple).

At the moment I think merging this PR is probably the least bad option, for the next staging cycle we can try switching to the newer sdk or finding a jemalloc fix.

@winterqt
Copy link
Member

At the moment I think merging this PR is probably the least bad option, for the next staging cycle we can try switching to the newer sdk or finding a jemalloc fix.

Agreed -- let's buy us some time, even if this may be a ticking time bomb (maybe we should reopen #144704 so people are aware that this probably will be occurring on unstable?)

@winterqt winterqt merged commit 5397627 into NixOS:staging-next Sep 22, 2022
@zowoq zowoq deleted the rustc-jemalloc branch September 22, 2022 12:53
@zowoq
Copy link
Contributor Author

zowoq commented Sep 22, 2022

maybe we should reopen #144704 so people are aware that this probably will be occurring on unstable?

Good idea but I'll wait a bit until we're closer to it landing in master.

@zowoq
Copy link
Contributor Author

zowoq commented Sep 27, 2022

Reopened #144704.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants