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

rustc: 1.65.0 -> 1.66.0 #206270

Merged
merged 3 commits into from
Dec 23, 2022
Merged

rustc: 1.65.0 -> 1.66.0 #206270

merged 3 commits into from
Dec 23, 2022

Conversation

figsoda
Copy link
Member

@figsoda figsoda commented Dec 15, 2022

Description of changes

https://blog.rust-lang.org/2022/12/15/Rust-1.66.0.html

built and tested fd on master (x86_64-linux)

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/)
  • 23.05 Release Notes (or backporting 22.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.

@figsoda
Copy link
Member Author

figsoda commented Dec 15, 2022

@ofborg build fd

@tjni
Copy link
Contributor

tjni commented Dec 15, 2022

Thank you for putting this out so quickly.

/cc @winterqt per #190093

@figsoda
Copy link
Member Author

figsoda commented Dec 15, 2022

I don't have a darwin machine so I can't test the stuff mentioned in #190093, feel free to directly push to this pr if you have any changes you want to make

@winterqt
Copy link
Member

winterqt commented Dec 15, 2022

I will do so by the end of the day (EST), thank you both.

@figsoda (and @tjni, if you want more Darwin power) https://github.com/winterqt/darwin-build-box exists as of recently, feel free to request access. :)

@winterqt

This comment was marked as outdated.

@winterqt
Copy link
Member

Turns out building 11k packages wasn't needed to figure out the issue.

I've described the strategy for dealing with this in 70d39d1, as well as added a note that I believe is correct in ebce964. Let me know if anything is unclear.

cc @zowoq

@ofborg ofborg bot requested review from retrry, cstrahan and globin December 16, 2022 00:55
@tjni
Copy link
Contributor

tjni commented Dec 17, 2022

I opened vectordotdev/vector#15627 to track fixing vector when compiled using Rust 1.66.

@figsoda
Copy link
Member Author

figsoda commented Dec 17, 2022

Thanks @tjni! we should be able to just remove the #![deny(warnings)] if upstream doesn't fix the issue before this gets into master

@figsoda
Copy link
Member Author

figsoda commented Dec 23, 2022

zowoq hasn't had any github acitivity for 10 days, perhaps we can just merge this and fix the issues that might pop up later

@tjni
Copy link
Contributor

tjni commented Dec 23, 2022

I'm in favor of merging it.

@winterqt
Copy link
Member

Just to confirm: does my approach to the libiconv situation make sense to you both, are there any issues? This should cause no breakages right now, but it just makes things easier for us to eventually start only adding it when needed by not propagating it alongside rustc (only adding it to the inputs of any Rust package using buildRustPackage, as we did previously).

@tjni
Copy link
Contributor

tjni commented Dec 23, 2022

It makes sense to me at a high level (I'm not as in tune with the details). To move is in that direction, what needs to happen so that:

  1. Rust 1.66 is used to bootstrap.
  2. Packages bump their libc versions.

@winterqt
Copy link
Member

I think there may be a typo in your message, or is that a question?

Either way, assuming I understand your question(?) correctly:

  1. libiconv being a dependency to build rustc doesn't matter too much here, and will go away when 1.66 is used to bootstrap; it doesn't propagate anymore.
  2. Once enough packages in the repo bump their libc version, we can go through and find which ones fail without it, and manually add those. I'd say we can maybe start doing this around March? (Just pulled a date out of thin air.)

Does that clarify things?

@figsoda
Copy link
Member Author

figsoda commented Dec 23, 2022

Makes sense to me as well, we just need to keep an eye on the packages that don't use buildRustPackage as they might still depend on an older version of libc

IIUC removing libiconv from buildRustPackage should also work, would just require a lot more work and we might as well wait a little longer for packages to update their dependencies

@winterqt
Copy link
Member

IIUC removing libiconv from buildRustPackage should also work, would just require a lot more work and we might as well wait a little longer for packages to update their dependencies

Exactly, that'll be the next step, per my last comment.

Give me one sec to adjust the commit message, please.

This reverts commit b6fc00b.

Rust 1.66.0 contains a fix for libiconv being linked unconditionally on macOS, but this only applies to packages that don't depend on older versions of `libc`.

For now, let's go back to including libiconv in `buildInputs` by default for packages that use `buildRustPackage`. As packages bump their `libc` versions, we can eventually stop including it by default, and manually add it where needed.
@winterqt
Copy link
Member

Let me know how those messages sound, adjusted some wording and formatting. Then we should be good to go!

@figsoda
Copy link
Member Author

figsoda commented Dec 23, 2022

messages & changes both lgtm, thanks!

@tjni tjni mentioned this pull request Dec 23, 2022
13 tasks
@tjni
Copy link
Contributor

tjni commented Dec 23, 2022

Thanks for the context. This LGTM (though might need to be rebased against latest staging, which should include the cargo auditable changes). I created #207451 to fix vector pending upstream fix.

@figsoda figsoda merged commit ca370da into NixOS:staging Dec 23, 2022
@figsoda figsoda deleted the rustc branch December 23, 2022 23:09
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.

None yet

3 participants