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

Jump to bottom

Hostapd bump #56087

Merged
merged 1 commit into from Feb 24, 2019
Merged

Hostapd bump #56087

merged 1 commit into from Feb 24, 2019

Conversation

clefru
Copy link
Contributor

@clefru clefru commented Feb 20, 2019 •
edited

Note: Unfortunately, I strongly disagree with the software development practices of hostapd and wpa_supplicant, which is: too long of a release cycle and no stable branch with backporting of fixes. Also I am not able to review or track upstream commits to see what's good for backporting. All I can do, is send a PR and hope that upstream testing was sufficient. Given that 2.7 ships 2 years worth of changes, I do expect a fallout however! :(

Motivation for this change

Upstream release

Things done

Pushed to my laptop to test wpa_supplicant.
Pushed to my nixos access point to test hostapd.

  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@vcunat
Copy link
Member

vcunat commented Feb 20, 2019

Some small fallout risk seems normal for master/unstable. The wording made me think you filed this against the stable 18.09 branch :-)

dasJ
dasJ approved these changes Feb 20, 2019
View reviewed changes
Copy link
Member

@dasJ dasJ left a comment

Choose a reason for hiding this comment

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

Looks good, you could add a comment to the patch mentioning that it's not a security patch but a build fix

@dasJ
Copy link
Member

dasJ commented Feb 20, 2019

Sorry, it was on the other side of the huge removal block in the diff. Never mind about my previous statement

@delroth
Copy link
Contributor

delroth commented Feb 23, 2019

I had #55926 pending to bump wpa_supplicant but no reviews in a week. Feel free to close that PR if this one gets merged.

@ryantm
Copy link
Member

ryantm commented Feb 23, 2019

@GrahamcOfBorg build hostapd wpa_supplicant

@ryantm
Copy link
Member

ryantm commented Feb 23, 2019

I'm kind of surprised so little depends on wpa_supplicant.
@vcunat Are you okay with merging this? It looks okay to me, but I didn't test it.

@clefru
Copy link
Contributor Author

clefru commented Feb 23, 2019

I had #55926 pending to bump wpa_supplicant but no reviews in a week. Feel free to close that PR if this one gets merged.

Sorry for missing your PR. I just used hostapd as a search term.

@ryantm
Copy link
Member

ryantm commented Feb 24, 2019

@xeji merged the other one, so this needs to be thinned out to be just hostapd, @clefru

@vcunat
Copy link
Member

vcunat commented Feb 24, 2019

I haven't done any kind of testing either, not even looking at NEWS. I can't see anything blocking this either, but it would be better if someone actually used it before merging :-)

@clefru
Copy link
Contributor Author

clefru commented Feb 24, 2019

I removed the wpa_supplicant part from this PR as it was pushed in the other.

@clefru
Copy link
Contributor Author

clefru commented Feb 24, 2019

@vcunat hostapd 2.7 is running on my AP since two or three days. Haven't noticed anything bad.

@ryantm
Copy link
Member

ryantm commented Feb 24, 2019

@GrahamcOfBorg build hostapd

@ryantm ryantm merged commit 92e399c into NixOS:master Feb 24, 2019
@ryantm
Copy link
Member

ryantm commented Feb 24, 2019

@clefru thanks; please consider adding yourself as a maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dasJ dasJ dasJ approved these changes

@Phreedom Phreedom Awaiting requested review from Phreedom

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@clefru @vcunat @dasJ @delroth @ryantm @GrahamcOfBorg