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

dino: 2019-10-28 -> 0.1.0 #78789

Merged
merged 4 commits into from Jan 30, 2020
Merged

dino: 2019-10-28 -> 0.1.0 #78789

merged 4 commits into from Jan 30, 2020

Conversation

alyssais
Copy link
Member

Motivation for this change
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.

@alyssais alyssais closed this Jan 29, 2020
@ofborg ofborg bot requested a review from Mic92 January 30, 2020 00:04
@alyssais
dino: 2019-10-28 -> 0.1.0
d506e9d 
The system libsignal-protocol-c is preferred now, so fetching
submodules is no longer required.

The other libraries added are ones that the build complained about.  I
suspect they are optional dependencies.  Adding them makes the build
quieter, and has basically no impact on runtime closure size, so no
harm in making them available to keep the build system happy.
@alyssais
dino: drop unnecessary enableParallelBuilding
131ec9d 
This is the default.
@alyssais
dino: add myself as a maintainer
adb89be
@alyssais alyssais reopened this Jan 30, 2020
andir
andir approved these changes Jan 30, 2020
View reviewed changes
Copy link
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

Works like a charm for me. I really like the new read & delivery markers :)

@alyssais
Copy link
Member Author

@GrahamcOfBorg eval

@alyssais
dino: fix rendering of logo in about window
091b710 
Previously:

** (dino:7442): CRITICAL **: 00:35:22.373: file /build/source/main/src/ui/unified_window.vala: line 86: uncaught error: Unrecognized image file format (gdk-pixbuf-error-quark, 3)

(dino:7442): Gtk-WARNING **: 00:35:22.601: Found an icon but could not load it. Most likely gdk-pixbuf does not provide SVG support.
@andir
Copy link
Member

andir commented Jan 30, 2020

Interestingly I didn't run into that about dialog issue. It just shows here. What platform are you running this on?

orivej
orivej reviewed Jan 30, 2020
View reviewed changes
pkgs/applications/networking/instant-messengers/dino/default.nix
libsepol
libpsl
brotli
libsignal-protocol-c
Copy link
Contributor

Choose a reason for hiding this comment

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

These (except libsignal-protocol-c) are private dependencies of the actual dependencies that should neither be propagated nor added manually to dino. For example, libpsl and brotli come from libsoup-2.4.pc:

Requires: glib-2.0 >=  2.38, gobject-2.0 >=  2.38, gio-2.0 >=  2.38
Requires.private: libxml-2.0, sqlite3, libpsl >=  0.20, libbrotlidec, zlib

(To be precise, glib uses utillinuxMinimal rather than utillinux.)

The warnings we see, such as Package 'mount', required by 'gio-2.0', not found, come from CMake running both pkg-config --libs and pkg-config --static --libs to populate both <XXX>_LIBRARIES and <XXX>_STATIC_LIBRARIES (docs), but dino has no use for the latter. Currently these warnings can not be disabled: https://gitlab.kitware.com/cmake/cmake/issues/18158

(They could be prevented by pruning Requires.private from shared-only libraries akin to #51767, although it can not be detected if a library is shared-only from the .pc file alone, and this is just a warning.)

@orivej
Copy link
Contributor

orivej commented Jan 30, 2020

Interestingly I didn't run into that about dialog issue. It just shows here.

Me too. It should not be necessary to list librsvg explicitly because even without it the resulting dino binary wrapper contains GDK_PIXBUF_MODULE_FILE=…librsvg….

@Mic92 Mic92 merged commit acf3b25 into NixOS:master Jan 30, 2020
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Jan 30, 2020
@Mic92 @dtzWill
dino: 2019-10-28 -> 0.1.0 (NixOS#78789)
1c242b8 
dino: 2019-10-28 -> 0.1.0
(cherry picked from commit acf3b25)
@alyssais
Copy link
Member Author

alyssais commented Jan 30, 2020 via email

@alyssais
Copy link
Member Author

alyssais commented Jan 30, 2020 via email

anna328p pushed a commit to anna328p/nixpkgs that referenced this pull request Feb 2, 2020
@Mic92 @anna328p
dino: 2019-10-28 -> 0.1.0 (NixOS#78789)
7f34358 
dino: 2019-10-28 -> 0.1.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orivej orivej orivej left review comments

@andir andir andir approved these changes

@Mic92 Mic92 Awaiting requested review from Mic92

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

Successfully merging this pull request may close these issues.

None yet
4 participants
@alyssais @andir @orivej @Mic92