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

qnotero: fix #23438 #23440

Merged
merged 1 commit into from
Mar 7, 2017
Merged

qnotero: fix #23438 #23440

merged 1 commit into from
Mar 7, 2017

Conversation

nico202
Copy link
Contributor

@nico202 nico202 commented Mar 3, 2017

Motivation for this change

#23438

As commented in line 20-21, I could just use "../../../" to fix the path. However I'm not sure if things will keep the way they are, so I thought realpath is less sensible to changes. I have no problem in just replacing it

@mention-bot
Copy link

@nico202, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

@bennofs
Copy link
Contributor

bennofs commented Mar 6, 2017

@nico202 Thx for looking at this! If found another way to achieve the intended result, which also reduces the amount of patching necessary:

diff --git a/pkgs/applications/office/qnotero/default.nix b/pkgs/applications/office/qnotero/default.nix
index 5076a8f586..7b6bf14681 100644
--- a/pkgs/applications/office/qnotero/default.nix
+++ b/pkgs/applications/office/qnotero/default.nix
@@ -18,17 +18,12 @@ python3Packages.buildPythonPackage rec {
 
   patchPhase = ''
       substituteInPlace ./setup.py \
-        --replace "/usr/share" "$out/usr/share"
+        --replace "/usr/share" "usr/share"
 
       substituteInPlace ./libqnotero/_themes/default.py \
         --replace "/usr/share" "$out/usr/share"
   '';
 
-  postInstall = ''
-      mkdir -p "$out/usr/share/qnotero"
-      mv resources "$out/usr/share/qnotero"
-  '';
-
   meta = {
     description = "Quick access to Zotero references";
     homepage = http://www.cogsci.nl/software/qnotero;

Perhaps this would be a less-hacky solution to the problem?

@grahamc
Copy link
Member

grahamc commented Mar 6, 2017

Yes please on the patch.

@nico202
Copy link
Contributor Author

nico202 commented Mar 7, 2017

@bennofs @grahamc you mean like that? Seems to work :)

@bennofs bennofs merged commit 264b69c into NixOS:master Mar 7, 2017
@nico202 nico202 deleted the qnotero-fix branch March 7, 2017 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants