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

calls: 0.2.0 -> 0.3.1, callaudiod: 0.0.4 -> 0.1.0 #114879

Merged
merged 3 commits into from
Mar 17, 2021

Conversation

Pacman99
Copy link
Contributor

@Pacman99 Pacman99 commented Mar 2, 2021

Motivation for this change

Update callaudiod and calls. There was an api change in callaudiod, so both need to be updated at the same time, or calls won't build.

closes #111865 , both updates need to be in one PR

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.

@r-rmcgibbo
Copy link

r-rmcgibbo commented Mar 2, 2021

Result of nixpkgs-review pr 114879 at b3e02cb9 run on aarch64-linux 1

2 packages built:
1 suggestion:
  • warning: meson-cmake

    Meson uses CMake as a fallback dependency resolution method and it likely is not necessary here. The message about cmake not being found is purely informational.

    Near pkgs/applications/networking/calls/default.nix:40:3:

       |
    40 |   nativeBuildInputs = [
       |   ^
    

Result of nixpkgs-review pr 114879 at b3e02cb9 run on x86_64-linux 1

2 packages built:
1 suggestion:
  • warning: meson-cmake

    Meson uses CMake as a fallback dependency resolution method and it likely is not necessary here. The message about cmake not being found is purely informational.

    Near pkgs/applications/networking/calls/default.nix:40:3:

       |
    40 |   nativeBuildInputs = [
       |   ^
    

@Pacman99 Pacman99 force-pushed the calls-update branch 2 times, most recently from 91f7ae6 to fb33b0b Compare March 2, 2021 20:41
@Pacman99
Copy link
Contributor Author

Pacman99 commented Mar 2, 2021

Also I got around to running this app on my pinephone and realized evolution-data-server needs to be built with phone number support for calls to match contacts.

So should I add phone number support to evolution-data-server by default or make it optional and override evolution-data-server just for calls.

@Pacman99 Pacman99 changed the title Update calls and callaudiod calls: 0.3.0 -> 0.4.0, callaudiod: 0.0.4 -> 0.1.0 Mar 3, 2021
@lheckemann
Copy link
Member

👍 on enabling by default, doesn't sound like a big deal in terms of closure size or anything.

@Pacman99
Copy link
Contributor Author

Pacman99 commented Mar 4, 2021

Made a separate PR for evolution phone support to target staging: #115123

These two tasks can happen separately since calls still builds ok without the change.

@Pacman99 Pacman99 force-pushed the calls-update branch 3 times, most recently from ef76708 to 9d7d9f5 Compare March 14, 2021 02:45
@Pacman99 Pacman99 changed the title calls: 0.3.0 -> 0.4.0, callaudiod: 0.0.4 -> 0.1.0 calls: 0.2.0 -> 0.3.1, callaudiod: 0.0.4 -> 0.1.0 Mar 14, 2021
pkgs/applications/networking/calls/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/calls/default.nix Outdated Show resolved Hide resolved
pkgs/applications/networking/calls/default.nix Outdated Show resolved Hide resolved
@Pacman99
Copy link
Contributor Author

Pacman99 commented Mar 14, 2021

I'll fix the extra dependencies soon. I added a few extra after misinterpreting the checkPhase error. I don't think either cmake or vala glib is needed, but I have to confirm.

mesonFlags = [
# docs fail to build
# https://source.puri.sm/Librem5/calls/issues/99
"-Dgtk_doc=false"
Copy link
Member

Choose a reason for hiding this comment

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

Actually, I left the false here even thought it is the default value so that we remember to enable it once the issue is fixed. Which seems to be the case now.

Would need to add "devdoc" to outputs attribute. And gtk-doc and docbook-xsl-nons and one of docbook_xml_dtd_XY (where XY is e.g. 43, based on the docbook warnings in the build log) to nativeBuildInputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed this and enabled the docs. I checked the warnings, it was 43. It also needed glib(gio) to build.

@jtojnar
Copy link
Member

jtojnar commented Mar 14, 2021

CMake probably is not but Vala is likely needed for bindings.

@jtojnar jtojnar merged commit 1b70a02 into NixOS:master Mar 17, 2021
@jtojnar
Copy link
Member

jtojnar commented Mar 17, 2021

Thanks.

@Pacman99 Pacman99 deleted the calls-update branch March 17, 2021 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants