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

treewide: stop using types.string #247937

Merged
merged 1 commit into from
Aug 8, 2023
Merged

Conversation

jian-lin
Copy link
Contributor

@jian-lin jian-lin commented Aug 8, 2023

It is an error1 now.

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/)
  • 23.11 Release Notes (or backporting 23.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
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 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/` labels Aug 8, 2023
@jian-lin jian-lin mentioned this pull request Aug 8, 2023
12 tasks
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed none of these look like they should ever be merged with string concatenation.

@jian-lin
Copy link
Contributor Author

jian-lin commented Aug 8, 2023

Fixed the linked issue number in the commit message.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Aug 8, 2023
@AndersonTorres AndersonTorres mentioned this pull request Aug 8, 2023
12 tasks
@K900 K900 merged commit b0b00f0 into NixOS:master Aug 8, 2023
@infinisil
Copy link
Member

Oof I'm a bit surprised these made it in at all, the type should throw a deprecation warning when used. Thanks for the fix!

@jian-lin jian-lin deleted the fix-types.string branch August 8, 2023 16:29
@majiru
Copy link
Contributor

majiru commented Aug 8, 2023

I am sorry for writing one of PRs that used one of these. Do these warnings not appear when building with 'nix build'? I had one in my branch for a while but never noticed any diagnostic output regarding this.

@infinisil
Copy link
Member

@majiru They should definitely appear with both nix-build and nix build. What arguments did you use to test it?

@linsui
Copy link
Contributor

linsui commented Aug 9, 2023

I only tested my PR with nixos-rebuild. I didn't see any warning.

@emilazy
Copy link
Member

emilazy commented Aug 10, 2023

We had some of these lying around in nix-darwin and they did not throw up any warnings (otherwise I'd have fixed them earlier). I don't know why the warning wasn't working, but it seems like this went from being accepted without complaints directly to a hard error. Not a problem for us, but I just want to flag up that this could cause problems elsewhere in the ecosystem because of that.

@infinisil
Copy link
Member

Ahh darn, I figured out what went wrong. I thought I implemented nested type deprecation (#99132), but it turns out it had to be reverted (#121816), which I didn't remember.

@infinisil
Copy link
Member

infinisil commented Aug 10, 2023

I opened a PR with a revert and a lib.warn instead of a throw: #248278

@RyzeNGrind
Copy link

RyzeNGrind commented Aug 11, 2023

Hi any updates on removing the types.string deprecation error? I am just now getting into Nix and NIxOS ecosystem and I cannot rebuild my configuration due to multiple module/nixpkg references coming from this deprecated type error. Kindly advise.

[nixos@shogun:/mnt/c/Users/$USER/WSL]$ sudo nixos-rebuild switch --flake /etc/nixos#
building the system configuration...
error: The type `types.string` is deprecated. See https://github.com/NixOS/nixpkgs/pull/66346 for better alternative types.
(use '--show-trace' to show detailed location information)

I see that the master branch has a PR with a fix that's been merged. How do I go about replicating the changes made across the master branch into my Nix machines? Apologies if this is a noob question. Should I use the unstable or stable channels? I noticed the Nix manual indicates that the master branch and channels have a few days of lag...

@Atemu
Copy link
Member

Atemu commented Aug 11, 2023

@RyzeNGrind just wait a little and roll back your channel in the mean time. The fix will be in unstable in a few days at most.

@RyzeNGrind
Copy link

RyzeNGrind commented Aug 11, 2023

@Atemu Thank you for your response. Is there anyway I can hot patch this fix for now so I can keep working on my systems without the several day delay? Appreciate your help with this matter.

Edit: Any advice on how to roll back the channel would be appreciated as well as it doesn't seem straightforward

@roberth
Copy link
Member

roberth commented Aug 11, 2023

nix-channel is above my pay grade, but if you have a flake you could upgrade to master fairly easily for a couple of days.

Maybe use -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/master.tar.gz for something even less reproducible than nix-channel?

@emilazy
Copy link
Member

emilazy commented Aug 11, 2023

You can just use nix-channel --rollback, I believe. Repeat until the errors go away.

@Atemu
Copy link
Member

Atemu commented Aug 12, 2023

Alternatively, you can also briefly switch to the nixos-unstable-small channel which already has the fix. That's more trouble than it's worth though IMO.

This is a topic for a post on Discourse however, not this place.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-build-errors-out-nixos-unstable/31642/4

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-build-errors-out-nixos-unstable/31642/5

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: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants