Skip to content

Revert NetworkManager to 1.42.8#263985

Closed
SuperSandro2000 wants to merge 2 commits intoNixOS:staging-nextfrom
SuperSandro2000:NetworkManager-Plasma
Closed

Revert NetworkManager to 1.42.8#263985
SuperSandro2000 wants to merge 2 commits intoNixOS:staging-nextfrom
SuperSandro2000:NetworkManager-Plasma

Conversation

@SuperSandro2000
Copy link
Copy Markdown
Member

@SuperSandro2000 SuperSandro2000 commented Oct 28, 2023

Description of changes

NetworkManager 1.44.0 does not work with the current Plasma after a restart of the NetworkManager service. It can be sometimes worked around by unloading and loading the iwlwifi kernel module but things like wpa_supplicant continue to behave buggy.

Tested this on my plasma system and after this revert things worked again.
I am open for alternatives and testing things but I couldn't figure out a better way yesterday.

see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053707

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/)
  • 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.

@amaxine
Copy link
Copy Markdown
Contributor

amaxine commented Oct 28, 2023

Was this reported upstream at all? I don't see anything linked in the debian report, and I can't find anything in the nm gitlab, and I've not been able to reproduce this issue on GNOME 45.

Unfortunate timing, but i guess saves doing backports of 1.44 for the rest of 23.11 😅

@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 28, 2023
@ofborg ofborg bot requested review from amaxine, domenkozar, jtojnar and obadz October 28, 2023 17:24
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. labels Oct 28, 2023
@K900
Copy link
Copy Markdown
Contributor

K900 commented Oct 28, 2023

Restarting Plasma (e.g. with plasmashell --replace) fixes it. I'd rather look for a fix in plasma-nm here.

Copy link
Copy Markdown
Contributor

@K900 K900 left a comment

Choose a reason for hiding this comment

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

-1 on the revert, let me run it by the Plasma people first.

@SuperSandro2000
Copy link
Copy Markdown
Member Author

Was this reported upstream at all?

I couldn't find anything yet but I was also not sure what to search for exactly until now because I didn't have an error log.

Restarting Plasma (e.g. with plasmashell --replace) fixes it. I'd rather look for a fix in plasma-nm here.

That is a good observation! I'd also rather fix this but if we can't fix it until the end of November the current situation is not great.

@K900
Copy link
Copy Markdown
Contributor

K900 commented Oct 30, 2023

Filed upstream: https://bugs.kde.org/show_bug.cgi?id=476323

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Nov 1, 2023

Duplicate of a 4-month old bug? Sounds like they can't solve it fast enough for us.

@amaxine
Copy link
Copy Markdown
Contributor

amaxine commented Nov 1, 2023

I think the revert is the only acceptable choice for 23.11, but I don't love the idea of having to hold nm back after branch off in that situation.

There's a comment on the issue mentioning Plasma 6 has this resolved - I don't follow KDE at all, so I'd appreciate if someone could weigh in as to what the timeline is for it landing in nix? The comments indicate that this is a Qt 6 fix, rather than anything in plasma-nm, and I'm not sure how Qt and plasma versions are related, if at all.

@K900
Copy link
Copy Markdown
Contributor

K900 commented Nov 2, 2023

Plasma 6 is pre-alpha, and requires Qt 6. What do you think about just not restarting NM on activation? That would be an easy workaround we can then always remove.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Nov 2, 2023

At a superficial glance (these comments) I'd choose older NM for 23.11 and then perhaps immediately try newer one on unstable/master after fork-off.

@amaxine
Copy link
Copy Markdown
Contributor

amaxine commented Nov 2, 2023

I cannot say I love the idea of potentially breaking the module as a workaround for this, and would much rather we ship 1.42.x for 23.11 - I'm not sure what all the consequences would be, but this seems like a lot to find out so close to release. The revert is safer.

I assume going back to 1.44.x asap after branch off is going to be quite irritating for KDE users on Nix though, with there being 4 more months before Plasma 6 releases. I'm not sure what the right call is there, since it's not a bug in NM but in Plasma/Qt, but that's gonna matter more a month from now and not today.

@K900
Copy link
Copy Markdown
Contributor

K900 commented Nov 2, 2023

"Revert" or "leave broken" are not our only options. I am still looking into what's causing the bug on my end.

@amaxine
Copy link
Copy Markdown
Contributor

amaxine commented Nov 2, 2023

Right, I'm not hitting merge here - branch off is not for another 2.5 weeks, but if it comes to it I would rather revert than remove restarting NM on activation.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Nov 2, 2023

1.42 vs 1.44 is a difference better not left as late as branch-off date. There may be other packages where it matters, too.

@K900
Copy link
Copy Markdown
Contributor

K900 commented Nov 2, 2023

I think I have it.

@K900
Copy link
Copy Markdown
Contributor

K900 commented Nov 2, 2023

Please test: #264997

@K900
Copy link
Copy Markdown
Contributor

K900 commented Nov 2, 2023

Update: final fix is in #265055

@K900 K900 closed this Nov 2, 2023
@SuperSandro2000
Copy link
Copy Markdown
Member Author

Reopening so this is not being forgotten and because the linked PR doesn't fully fix the problem and we are really to late to have such breaking changes for the stable branch off.

@vcunat vcunat closed this in 66be221 Nov 3, 2023
natsukium pushed a commit to natsukium/nixpkgs that referenced this pull request Nov 4, 2023
@SuperSandro2000 SuperSandro2000 deleted the NetworkManager-Plasma branch November 5, 2023 00:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants