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

Pdfmod module #52484

Closed
wants to merge 1 commit into from
Closed

Pdfmod module #52484

wants to merge 1 commit into from

Conversation

ciil
Copy link
Member

@ciil ciil commented Dec 18, 2018

Motivation for this change

Change to new mono version due to mono/mono#6752 (mono 5.8 is the default, and it still fails with that, mono 5.10 should work, but as long as it builds with 5.14, I though why not go with that). Also pdfmod has always needed gconf to be installed as well, so I've included a stupid little module which just installs both packages together -- unsure how else to solve this otherwise.

/cc @obadz

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 nox --run "nox-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.

@jtojnar
Copy link
Contributor

jtojnar commented Dec 18, 2018

It is a responsibility of your desktop environment to enable GConf database, if you want to support legacy programs. Adding a module for each legacy program is too clutter-some, I would suggest introducing programs.legacy.gconf.enable instead.

Bumping the mono version is fine with me, though the requirement is weird – pdfmod is dead package after all.

@ciil
Copy link
Member Author

ciil commented Dec 18, 2018

@jtojnar I wouldn't quite put it like that. How should my desktop environment know that this one specific gtk program needs gconf? Especially for the newbie user, they'd just see that installing a specific package resulted in a runtime error every time they wanted open a file with it. And introducing a "hidden" feature like programs.legacy.gconf.enable wouldn't really help there either.

pdfmod being a dead package is certainly a valid point, however. In the interest of keeping nixpkgs relatively clean of such packages, maybe we should instead just completely remove it?

@obadz
Copy link
Contributor

obadz commented Dec 20, 2018

Can I suggest that we neither create a dummy NixOS module, nor drop the package from nixpkgs?

At the moment, the package produces a runtime error when GConf is missing. That's not ideal but probably better than the two choices above.

@obadz
Copy link
Contributor

obadz commented Dec 20, 2018

Re mono version, maybe let's just drop the override and go with the 'default' mono version since it's probably supported?

The fewer overrides, the fewer versions of mono users need to download (and we need to maintain).

@ciil
Copy link
Member Author

ciil commented Dec 20, 2018

mono 5.8 is the current default and that one still contains the magic number error on non-xterm terminals as mentioned above. Only mono versions >= 5.10, of which we package 5.14 as the latest stable release, contain that fix. I've changed the PR accordingly and excluded the dummy module again.

@obadz
Copy link
Contributor

obadz commented Dec 24, 2018

Could you please merge with #52712 ?

@obadz obadz closed this in 4ff22f2 Dec 25, 2018
@ciil ciil deleted the pdfmod-module branch December 25, 2018 18:34
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.

None yet

4 participants