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

connman: 1.37 -> 1.38 #83473

Merged
merged 21 commits into from Mar 28, 2020
Merged

connman: 1.37 -> 1.38 #83473

merged 21 commits into from Mar 28, 2020

Conversation

doronbehar
Copy link
Contributor

Motivation for this change
  • Update connman 1.38, see change log.
  • Cleanup build for easy enable / disable of features.
Things done
  • Rewrite all features' enable / disable flags.
  • Cleaned up dependencies, including unneeded ones.
  • Added 2 new build flavors - connmanFull and connmanThin with several more / less features enabled / disabled.
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

cc @matejc

@doronbehar
connman: format argument lists and inputs
59613f5
@doronbehar
connman: 1.37 -> 1.38
7cdd874
@doronbehar
connman: remove unneeded deps
bc4d2ab
@doronbehar
connman: remove unneeded hooks
184d741
@doronbehar
connman: quote out in configure flag
e9569a0
@doronbehar
connman: quote homepage
e55ad48
@doronbehar
connman: upgrade to readline latest
a0e966d
@doronbehar
connman: doCheck and split outputs
50ce1e6
@doronbehar
connman: cleanup build from the groundup
53d8831
@doronbehar
connman: Add 2 more build flavors
ce502a4
@doronbehar
connman: Fix comments after @erikarvstedt's comments
cdc20ac
@doronbehar
connman: Use global reference to lib.optionals
e59deaf
@doronbehar
connman: Use patchPhase instead of preConfigure
be59cb3
@doronbehar
connman: declare build flavors idiomaticly
3b89674 
Use tesseract as a source for inspiration - rename default.nix ->
connman.nix and declare build flavors in default.nix .
@doronbehar
connman: Switch to postPatch from patchPhase
fa5f7b8
@doronbehar
connmanMinimal: remove use of lowPrio
a3d2e48
@doronbehar
connman: remove import of networkmanager
22b27dd
@doronbehar
connman: reverse order of build flavors declaration
aa44c23
@doronbehar
connman: remove unneeded deps from nativeBuildInputs
b03bf77
@doronbehar doronbehar mentioned this pull request Mar 27, 2020
Closed
13 tasks
@ofborg ofborg bot requested a review from matejc March 27, 2020 07:59
@worldofpeace
Copy link
Contributor

Perhaps @jtojnar or @romildo (I think he was interested in connman) could look at this.

@romildo romildo self-requested a review March 28, 2020 01:07
romildo
romildo approved these changes Mar 28, 2020
View reviewed changes
Copy link
Contributor

@romildo romildo left a comment •
edited

Choose a reason for hiding this comment

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

This PR looks good to me. The many options are well organized.

I have tested on my notebook with wifi and ethernet connections.

Maybe an option could be added to the connman service to select the connman package, as there will be three to choose from: connman, connmanMinimal and connmanFull.

@doronbehar doronbehar changed the title Update connman connman: 1.37 -> 1.38 Mar 28, 2020
@doronbehar
Copy link
Contributor Author

That's definitely a good idea @romildo . I created an option for the package and I think it works - tested with my own nixos-config. I've added some TODOs regarding network-manager+connman, see last commit. BTW This my first contribution to nixos' modules :).

worldofpeace
worldofpeace reviewed Mar 28, 2020
View reviewed changes
nixos/modules/services/networking/connman.nix
Comment on lines +99 to 103
# TODO: connman seemingly can be used along network manager and
# connmanFull supports this - so this should be worked out somehow
assertion = !config.networking.networkmanager.enable;
message = "You can not use services.connman with networking.networkmanager";
}];
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also open an issue? We have a tracker for these things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@worldofpeace it's just that I personally never have used or tried to use network manager at all. It'll take me quiet an effort to write a good bug report so I think it'd be proper to wait for someone else to want that feature. Hopefully he'll encounter this comment indicating at least someone was aware of this..

worldofpeace
worldofpeace approved these changes Mar 28, 2020
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

All looks good for to me. An issue would be nice for that todo.

@worldofpeace worldofpeace merged commit d5cfaf5 into NixOS:master Mar 28, 2020
@doronbehar doronbehar deleted the update-connman branch March 2, 2023 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romildo romildo romildo approved these changes

@worldofpeace worldofpeace worldofpeace approved these changes

@matejc matejc Awaiting requested review from matejc

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@doronbehar @worldofpeace @romildo