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.

Sign up for GitHub

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

Jump to bottom

NetworkManager, ModemManager: move all symlinks to alias, add alias for modem-manager.service #70325

Closed
wants to merge 7 commits into from
Closed

NetworkManager, ModemManager: move all symlinks to alias, add alias for modem-manager.service #70325

wants to merge 7 commits into from
+97 −51

Conversation

worldofpeace
Copy link
Contributor

Motivation for this change

Thinking this fixes #70301

I haven't tested this, but we need to make sure there's no issues with NetworkManager and users
doing nixos-rebuild switch --upgrade.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

ttuegel and others added 5 commits September 28, 2019 17:33
@ttuegel
zbar: Use multiple outputs

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
3837059
@ttuegel
mariadb.connector-c: Use multiple outputs to reduce closure size
ce2bb4d
@ttuegel
Revert "zbar: Use multiple outputs"
686237e 
This reverts commit 3837059.
@jtojnar @ttuegel
zbar: clean up (NixOS#68389)
92a51dd 
zbar: clean up
(cherry picked from commit 8752ff2)
@ttuegel
Merge pull request NixOS#70025 from ttuegel/closure-size--staging-19.09
9e8e87f 
More closure size improvements for NixOS 19.09
@worldofpeace worldofpeace added the 9.needs: port to stable A PR needs a backport to the stable release. label Oct 3, 2019
@worldofpeace
Copy link
Contributor Author

I'd say this is pretty important for a release critical component.

@worldofpeace worldofpeace added the 1.severity: blocker This is preventing another PR or issue from being completed label Oct 3, 2019
@ofborg ofborg 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 Oct 3, 2019
@ofborg ofborg bot requested review from Phreedom, obadz and domenkozar October 3, 2019 14:27
hedning
hedning reviewed Oct 4, 2019
View reviewed changes
nixos/modules/services/networking/networkmanager.nix Outdated
"dbus-org.freedesktop.NetworkManager.service"
# Add the legacy service name from before #51382 to prevent NetworkManager
# from not starting back up:
# TODO: remove this once 19.09 is released
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to remove this in master as we've forked off 19.09?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I think we might actually have to remove it during 20.03 branching off.
Still a possibility we could cause an issue for a unstable user?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, right, so that would be for people upgrading from 18.09 -> 20.03 with switch?

Still a possibility we could cause an issue for a unstable user?

Not entirely sure. But it's there to handle rebuild switch from legacy-name to new config with upstream name. So unstable users should have the upstream name already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not entirely sure. But it's there to handle rebuild switch from legacy-name to new config with upstream name. So unstable users should have the upstream name already?

Now that I think about it, making guarantees like this for unstable doesn't seem right.
We just want to make sure that rebuild switching from 19.03 -> 19.09 works fine.
I'm not sure if 18.09 -> 19.03 would be a seamless switch.

@worldofpeace
networkmanager: no network-manager.service symlink
75b0905 
We removed the symlinks for the alias in
* f99bdb2

but we also need to move this definition into the module.
Else we get issues like NixOS#70301.

Fixes NixOS#70301.
@worldofpeace
nixos/networkmanager: add compat alias for modem-manager.service
33b705b 
We probably need this as well.
@worldofpeace worldofpeace force-pushed the networkmanager-no-symlinks-ever branch from bb04483 to 33b705b Compare October 4, 2019 00:41
@ofborg ofborg bot added 8.has: changelog 8.has: documentation This PR adds or changes documentation 2.status: merge conflict This PR has merge conflicts with the target branch and removed 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: emacs Text editor 6.topic: erlang 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: golang 6.topic: haskell 6.topic: python 6.topic: qt/kde 6.topic: ruby 6.topic: rust 6.topic: stdenv Standard environment 6.topic: vim 6.topic: xfce The Xfce Desktop Environment 8.has: changelog 8.has: documentation This PR adds or changes documentation 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 4, 2019
@worldofpeace
Copy link
Contributor Author

Realized that I already dropped the legacy service in staging

@flokli
Copy link
Contributor

flokli commented Oct 4, 2019

@worldofpeace this means, this PR isn't needed for master/unstable?

@worldofpeace
Copy link
Contributor Author

@worldofpeace this means, this PR isn't needed for master/unstable?

Yeah, I've re-targeted this to release-19.09. It seems I did all the correct work 1ff3a0c I'm just starting to forgot because its been in staging so long 😅

Not sure what's up with the commit graph here...

@worldofpeace
Copy link
Contributor Author

merged in cc40ae7 d82663c

@worldofpeace worldofpeace deleted the networkmanager-no-symlinks-ever branch October 9, 2019 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hedning hedning

@Mic92 Mic92

@flokli flokli

@dtzWill dtzWill

@vcunat vcunat

Assignees
No one assigned
Labels
1.severity: blocker This is preventing another PR or issue from being completed 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/` 9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 501-1000 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@worldofpeace @flokli @hedning @ttuegel @jtojnar