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

evince: enable all backends #58277

Merged
merged 2 commits into from Apr 5, 2019
Merged

evince: enable all backends #58277

merged 2 commits into from Apr 5, 2019

Conversation

gebner
Copy link
Member

@gebner gebner commented Mar 25, 2019

Motivation for this change

Evince can no longer read postscript. See #58259.

I also tried to add meson support, but there is no meson.build file in the release tarball--only in the git repository.

cc @jtojnar @worldofpeace

Things done
  • 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 nix-review --run "nix-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.

@GrahamcOfBorg GrahamcOfBorg added the 6.topic: GNOME GNOME desktop environment and its underlying platform label Mar 25, 2019
@worldofpeace
Copy link
Contributor

I also tried to add meson support, but there is no meson.build file in the release tarball--only in the git repository.

I checked and that is true, but I don't think it makes sense to note it in the News and yet it isn't in the archive.
So maybe that's a mistake?

It's acceptable to just use fetchFromGitLab.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 25, 2019 •
edited

When working on the update, I also noticed this and discounted it as the meson build system not being stable.

@jtojnar jtojnar closed this Mar 25, 2019
@jtojnar jtojnar reopened this Mar 25, 2019
@worldofpeace
Copy link
Contributor

discounted it as the meson build system not being stable.

I've looked and it seems it wasn't tested on windows, which wouldn't be important for us.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 25, 2019 via email

@worldofpeace
Copy link
Contributor

Can we ask for a point release with meson files in EXTRA_DIST then?

I'd be happy to. Just not sure if https://gitlab.gnome.org/GNOME/evince/issues/ is the right place for that kind of issue?

@gebner
Copy link
Member Author

gebner commented Mar 25, 2019

Ok, ported to meson using gitlab as src.

@jtojnar
Copy link
Contributor

jtojnar commented Mar 25, 2019 via email

@gebner gebner requested a review from infinisil as a code owner March 26, 2019 11:09
worldofpeace
worldofpeace approved these changes Mar 26, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace 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, very cooperative change ❇️

Just awaiting @jtojnar comments

jtojnar
jtojnar reviewed Mar 26, 2019
View reviewed changes
pkgs/desktops/gnome-3/core/evince/default.nix Outdated
"-Dt1lib=enabled"
"-Dxps=${if supportXPS then "enabled" else "disabled"}"
"-Dmultimedia=${if supportMultimedia then "enabled" else "disabled"}"
"-Dgtk_unix_print=enabled"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use -Dauto_features=enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this looks much nicer. Why are auto features even disabled by default?

Copy link
Contributor

Choose a reason for hiding this comment

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

I did not really think it through when I added it 😜

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps due to less predictability in case some packages propagate others?

@hedning hedning force-pushed the gnome-3.32 branch 3 times, most recently from a47dd4c to a02139a Compare April 4, 2019 11:17
@gebner gebner changed the base branch from gnome-3.32 to staging April 5, 2019 11:37
@gebner
Copy link
Member Author

gebner commented Apr 5, 2019

@worldofpeace @jtojnar Anything missing from your side? I've rebased this PR onto staging.

@jtojnar jtojnar added this to In Progress in GNOME Apr 5, 2019
worldofpeace
worldofpeace approved these changes Apr 5, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace 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, builds and executes locally.

@worldofpeace worldofpeace merged commit 92866a3 into NixOS:staging Apr 5, 2019
GNOME automation moved this from In Progress to Done Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace approved these changes

@hedning hedning Awaiting requested review from hedning

@vcunat vcunat Awaiting requested review from vcunat

@infinisil infinisil Awaiting requested review from infinisil

@jtojnar jtojnar Awaiting requested review from jtojnar

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
GNOME
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gebner @worldofpeace @jtojnar @vcunat @GrahamcOfBorg