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

libreoffice-qt: init at 6.4.3.2 #89395

Merged
merged 1 commit into from Jun 4, 2020
Merged

libreoffice-qt: init at 6.4.3.2 #89395

merged 1 commit into from Jun 4, 2020

Conversation

peterhoeg
Copy link
Member

@peterhoeg peterhoeg commented Jun 3, 2020 •
edited

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.

@peterhoeg peterhoeg changed the title <!-- To help with the large amounts of pull requests, we would appreciate your reviews of other pull requests, especially simple package updates. Just leave a comment describing what you have tested in the relevant package/service. Reviewing helps to reduce the average time-to-merge for everyone. Thanks a lot if you do! List of open PRs: https://github.com/NixOS/nixpkgs/pulls Reviewing guidelines: https://hydra.nixos.org/job/nixpkgs/trunk/manual/latest/download/1/nixpkgs/manual.html#chap-reviewing-contributions --> libreoffice-qt: init at 6.4.3.2 Jun 3, 2020
@ofborg ofborg bot requested a review from 7c6f434c June 3, 2020 09:52
@peterhoeg
Copy link
Member Author

So the PR description was hosed - sorry about the noise.

libreoffice-qt looks much nicer than it's GTK3 cousin when run under KDE.

Fixes #57846

Cc: @7c6f434c @ajs124 @mmilata

@peterhoeg
Copy link
Member Author

Relates to #73387 and #68414

@peterhoeg
Copy link
Member Author

Only building the fresh flavour but there is nothing preventing us from doing still as well.

ajs124
ajs124 approved these changes Jun 3, 2020
View reviewed changes
Copy link
Member

@ajs124 ajs124 left a comment

Choose a reason for hiding this comment

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

We might want to do fresh and still, but I'm fine with only fresh.

Compiled and ran. This does indeed look nicer on KDE.

@peterhoeg peterhoeg merged commit 9a847fa into NixOS:master Jun 4, 2020
@lukateras lukateras mentioned this pull request Jun 4, 2020
Closed
@B4dM4n B4dM4n mentioned this pull request Jun 5, 2020
Closed
jtojnar
jtojnar reviewed Jun 6, 2020
View reviewed changes
pkgs/applications/office/libreoffice/default.nix
@@ -57,8 +60,9 @@ in (stdenv.mkDerivation rec {

# For some reason librdf_redland sometimes refers to rasqal.h instead
# of rasqal/rasqal.h
NIX_CFLAGS_COMPILE = "-I${librdf_rasqal}/include/rasqal"
+ stdenv.lib.optionalString stdenv.isx86_64 " -mno-fma";
NIX_CFLAGS_COMPILE = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Environment variables need to be strings, this will be an error with structured attrs.

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 had a read through https://nixos.mayflower.consulting/blog/2020/01/20/structured-attrs/ which was really quite interesting. Don't we consider a list a primitive type on nix?

Copy link
Contributor

Choose a reason for hiding this comment

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

Reasoning here: #72074 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the pointers, I'll open a separate issue to address this.

@peterhoeg peterhoeg deleted the f/lo_kde branch June 8, 2020 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

@ajs124 ajs124 ajs124 approved these changes

@7c6f434c 7c6f434c Awaiting requested review from 7c6f434c

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 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
@peterhoeg @jtojnar @ajs124