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

freeswitch: 1.10.3 -> 1.10.5 #96987

Merged
merged 3 commits into from Nov 17, 2020
Merged

freeswitch: 1.10.3 -> 1.10.5 #96987

merged 3 commits into from Nov 17, 2020

Conversation

ajs124
Copy link
Member

@ajs124 ajs124 commented Sep 3, 2020

Motivation for this change

Upstream release.

Things done
  • 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.

@ajs124 ajs124 marked this pull request as ready for review September 3, 2020 02:28
@ofborg ofborg bot requested review from 7c6f434c and misuzu September 3, 2020 02:43
misuzu
misuzu reviewed Sep 3, 2020
View reviewed changes
pkgs/development/libraries/spandsp/default.nix
stdenv.mkDerivation rec {
version = "0.0.6";
version = "3.0.0";
Copy link
Contributor

Choose a reason for hiding this comment

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

OpenWRT folks decided to package this as separate package, maybe we should too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. I don't remember what else uses this and generally we try to avoid multiple versions of libraries, but in this case it seems reasonable to duplicate.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked again. The only other things using spandsp are:

  • pkgs/development/libraries/gstreamer/bad/default.nix
  • pkgs/applications/networking/instant-messengers/baresip/default.nix

Which both seem to compile fine with this update. So I'd actually rather not duplicate this.

misuzu
misuzu reviewed Sep 3, 2020
View reviewed changes
pkgs/development/libraries/spandsp/default.nix
meta = {
description = "A portable and modular SIP User-Agent with audio and video support";
homepage = "http://www.creytiv.com/baresip.html";
homepage = "https://github.com/freeswitch/spandsp";
platforms = with stdenv.lib.platforms; linux;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
platforms = with stdenv.lib.platforms; linux;
platforms = with stdenv.lib.platforms; unix;

Copy link
Member Author

Choose a reason for hiding this comment

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

Have you tested this? I don't really have any way to test this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just tested macos 10.14, builds successfully.

misuzu
misuzu reviewed Sep 3, 2020
View reviewed changes
pkgs/development/libraries/sofia-sip/default.nix

meta = with stdenv.lib; {
description = "Open-source SIP User-Agent library, compliant with the IETF RFC3261 specification";
homepage = "http://sofia-sip.sourceforge.net/";
homepage = "https://github.com/freeswitch/sofia-sip";
platforms = platforms.linux;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
platforms = platforms.linux;
platforms = platforms.unix;

Copy link
Contributor

Choose a reason for hiding this comment

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

This does build on macos 10.14 if you provide it with the buildInput darwin.apple_sdk.frameworks.SystemConfiguration

Copy link
Member

Choose a reason for hiding this comment

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

We do have quite a bit of stuff with platform-conditional build inputs.

This was referenced Sep 5, 2020
Closed
Open
@risicle
Copy link
Contributor

risicle commented Sep 20, 2020 •
edited

Enabling both libs for darwin allows us to attempt building freeswitch which unfortunately fails (in linking some of its tests?), but we're at lease a step closer. 👍

make[4]: Entering directory '/private/tmp/nix-build-freeswitch-1.10.5.drv-0/source/src/mod/endpoints/mod_sofia'
  CC       test/test_sofia_funcs-test_sofia_funcs.o
  CC       libsofiamod_la-mod_sofia.lo
  CC       libsofiamod_la-sofia.lo
  CC       libsofiamod_la-sofia_json_api.lo
  CC       libsofiamod_la-sofia_glue.lo
  CC       libsofiamod_la-sofia_presence.lo
  CC       libsofiamod_la-sofia_reg.lo
  CC       libsofiamod_la-sofia_media.lo
  CC       libsofiamod_la-sip-dig.lo
  CC       libsofiamod_la-rtp.lo
  CCLD     libsofiamod.la
  CCLD     test/test_sofia_funcs
duplicate symbol _sofia_on_destroy in:
    test/test_sofia_funcs-test_sofia_funcs.o
    ./.libs/libsofiamod.a(libsofiamod_la-mod_sofia.o)

...

ld: 16 duplicate symbols for architecture x86_64

@ajs124
sofia-sip: 1.12.11 -> 1.13.2
9d39bd3 
this last release on sourceforge is from 2011-03-11
freeswitch seems to be using this and working on it, so we're switching
to their repo
@ajs124
spandsp: 0.0.6 -> 3.0.0
e184ced 
this looks like a new upstream and freeswitch uses this
@ajs124
freeswitch: 1.10.3 -> 1.10.5
8385c2f 
now depends on spandsp and sofia_sip instead of carrying them in-tree
@ofborg ofborg bot requested review from misuzu and 7c6f434c November 17, 2020 01:41
7c6f434c
7c6f434c reviewed Nov 17, 2020
View reviewed changes
pkgs/development/libraries/spandsp/default.nix
src = fetchFromGitHub {
owner = "freeswitch";
repo = pname;
rev = "6ec23e5a7e411a22d59e5678d12c4d2942c4a4b6"; # upstream does not seem to believe in tags
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add commit timestamp in the comments for easier later verification?

@7c6f434c
Copy link
Member

Does anyone has enough confidence about the NixOS test for freeswitch to confirm that the test failure is a problem with Python harness and everything inside the VM actually worked just fine? It looks like that…

@misuzu
Copy link
Contributor

misuzu commented Nov 17, 2020

Does anyone has enough confidence about the NixOS test for freeswitch to confirm that the test failure is a problem with Python harness and everything inside the VM actually worked just fine? It looks like that…

It works on top of nixos-unstable so should be fine.

@7c6f434c 7c6f434c merged commit c6b8a82 into NixOS:master Nov 17, 2020
@7c6f434c
Copy link
Member

@risicle feel free to mention me if you want to merge some macOS build fixes for that…

@ajs124 ajs124 deleted the upd/freeswitch branch November 17, 2020 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@risicle risicle risicle left review comments

@7c6f434c 7c6f434c 7c6f434c left review comments

@misuzu misuzu misuzu approved these changes

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@ajs124 @risicle @7c6f434c @misuzu