dupeguru: init at 4.0.4 #75054

novoxd · 2019-12-05T17:12:02Z

Motivation for this change

Add dupeGuru programm https://dupeguru.voltaicideas.net .

Things done

novoxd · 2019-12-05T18:19:54Z

Just found that I need to rewrite source fetching, from fetchGit to fetchFromGitHub with submodules. (fixed)

novoxd · 2019-12-09T16:53:33Z

Please review, I don't know how long it takes to get a review in nixpkgs, but there are now auto-suggested reviewers. Maybe @doronbehar can help.

pkgs/applications/misc/dupeguru/default.nix

doronbehar · 2019-12-09T22:40:45Z

Yes.

novoxd · 2019-12-11T20:50:29Z

@doronbehar I don't know - but what next? Do I need to wait for this pr to be merged? Or I need to mention someone ho can add his review and merge? (I am quite new to nixpkgs)

doronbehar · 2019-12-11T21:37:42Z

It's OK, welcome aboard the Caos 😺. You can mention maybe @jonringer or @worldofpeace if days pass and nothing happens but you better get used to this - PRs can take infinitely long time to be merged..

pkgs/applications/misc/dupeguru/default.nix

novoxd · 2019-12-11T23:55:25Z

It's OK, welcome aboard the Caos . You can mention maybe @jonringer or @worldofpeace if days pass and nothing happens but you better get used to this - PRs can take infinitely long time to be merged..

@doronbehar thanks for your review and help.

jtojnar · 2019-12-13T13:07:37Z

pkgs/applications/misc/dupeguru/default.nix

+  #dontWrapQtApps = true;
+
+  #preFixup = ''
+  #  makeWrapperArgs+=("''${gappsWrapperArgs[@]}")


dontWrapQtApps = true; preFixup = '' makeWrapperArgs+=("''${qtWrapperArgs[@]}") '';

This should be sufficient to prevent double wrapping.

Okay, the issue is that the GUI executable is in in share, the thing in bin is just a symlink. And Python infrastructure only sets up things in bin:

nixpkgs/pkgs/development/interpreters/python/mk-python-derivation.nix

Line 149 in 8d5e90b

wrapPythonPrograms

nixpkgs/pkgs/development/interpreters/python/wrap.sh

Line 4 in 8d5e90b

wrapPythonProgramsIn "$out/bin" "$out $pythonPath"

Adding

postFixup = '' wrapPythonProgramsIn "$out/share/dupeguru/run.py" "$out $pythonPath" ''; should fix that.

Hmm, there is a bug in the Python wrapper, this line

nixpkgs/pkgs/development/interpreters/python/wrap.sh

Line 84 in 8d5e90b

local -a user_args="($makeWrapperArgs)"

needs to be

local -a user_args="${makeWrapperArgs[@]}"

to work around it, the preFixup section should go like

# TODO: A bug in python wrapper # see https://github.com/NixOS/nixpkgs/pull/75054#discussion_r357656916 makeWrapperArgs="''${qtWrapperArgs[@]}"

Sorry, should have been "$out/share/dupeguru" not "$out/share/dupeguru/run.py".

Fixing this

I take that back, testing this change on my laptop is impossible, @worldofpeace tag!

novoxd · 2019-12-13T13:09:38Z

@jtojnar I have some problems after trying to make things like in your review.

I get error:

✦ ➜ /nix/store/5l9ckal7javjd04qa4n2afsmlal9naj5-dupeguru-4.0.4/bin/dupeguru
Traceback (most recent call last):
  File "/nix/store/5l9ckal7javjd04qa4n2afsmlal9naj5-dupeguru-4.0.4/bin/dupeguru", line 12, in <module>
    from PyQt5.QtCore import QCoreApplication, QSettings
ModuleNotFoundError: No module named 'PyQt5'

It looks like required packages are not in the python interpreter what wraps out/bin/dupguru . I had the same error on start but here I don't know what to do or how to analyze the problem. Can you give any advice about figuring out why it fails?

But as I know I made default.nix correct, all by instructions. Can you please review changes, maybe I am mistaken somewhere? The problem can be in dupguru itself its Makefile is a little "broken" .
(the pr files are updated somehow it is not displaying in the tread)

doronbehar · 2019-12-13T13:29:19Z

@novoxudonoser Try to find some python apps in the collection which use qt and wrap it and you should be able to get an idea as for how to fix your issue.

pkgs/applications/misc/dupeguru/default.nix

jtojnar · 2019-12-13T13:28:07Z

pkgs/applications/misc/dupeguru/default.nix

+  #dontWrapQtApps = true;
+
+  #preFixup = ''
+  #  makeWrapperArgs+=("''${gappsWrapperArgs[@]}")


Okay, the issue is that the GUI executable is in in share, the thing in bin is just a symlink. And Python infrastructure only sets up things in bin:

nixpkgs/pkgs/development/interpreters/python/mk-python-derivation.nix

Line 149 in 8d5e90b

wrapPythonPrograms

nixpkgs/pkgs/development/interpreters/python/wrap.sh

Line 4 in 8d5e90b

wrapPythonProgramsIn "$out/bin" "$out $pythonPath"

Adding

postFixup = '' wrapPythonProgramsIn "$out/share/dupeguru/run.py" "$out $pythonPath" ''; should fix that.

pkgs/applications/misc/dupeguru/default.nix

novoxd · 2019-12-13T14:28:17Z

@jtojnar made all as you said. The problem is still there. Seems that we are missing something. The most interesting thin that in d409fe3 all worked, I guess that the problem is - that using buildPythonApplication for some bugs or wrong configuration same actions like in d409fe3 are not reproduced.

jtojnar · 2019-12-13T14:30:52Z

Sorry, should have been "$out/share/dupeguru" not "$out/share/dupeguru/run.py".

novoxd · 2019-12-13T14:32:01Z

@novoxudonoser Try to find some python apps in the collection which use qt and wrap it and you should be able to get an idea as for how to fix your issue.

While the building of the first version of derivation I looked to many examples. And I picked stdenv.mkDerivation cause the make file in dupguru is a little "broken" (seems like symlink to python file breaks nixpkgs standart buildPythonApplication process ). Rith now I don't have enough knowledge to understand what breaks here. I think more deep research in nix build process and python section can help, and I plan to do it as part of my education process.

novoxd · 2019-12-13T14:42:43Z

@jtojnar finally it worked! Thanks a lot for your help, how I said upper - I don't have enough knowledge to make this derivation correct on my own.

pkgs/applications/misc/dupeguru/default.nix

jtojnar · 2019-12-13T14:49:43Z

@jtojnar finally it worked! Thanks a lot for your help, how I said upper - I don't have enough knowledge to make this derivation correct on my own.

Unfortunately, bash is such a terrible language that lot of these things are more complicated than they should be.

pkgs/applications/misc/dupeguru/default.nix

jtojnar · 2019-12-13T14:52:27Z

pkgs/applications/misc/dupeguru/default.nix

+
+python3Packages.buildPythonApplication rec {
+  pname = "dupeguru";
+  format = "other";


Sorry if I were unclear, I meant below version and above src.

pname = "dupeguru"; version = "4.0.4"; format = "other"; src = fetchFromGitHub {

pkgs/applications/misc/dupeguru/default.nix

jtojnar · 2019-12-13T15:01:42Z

Looking at the makeFile:

It appears that the buildFlags are already part of the default make target (make all):
https://github.com/arsenetar/dupeguru/blob/2ea02bd7b50ce12ba489bc90717c4b578641deaf/Makefile#L47

It also suggests that we pass NO_VENV=1 to make:

https://github.com/arsenetar/dupeguru/blob/2ea02bd7b50ce12ba489bc90717c4b578641deaf/Makefile#L22-L24

I recommend dropping buildFlags and installFlags and just using:

  makeFlags = [
    "PREFIX=${placeholder ''out''}"
    "NO_VENV=1"
  ];

pkgs/applications/misc/dupeguru/default.nix

novoxd · 2019-12-13T15:07:27Z

I recommend dropping buildFlags and installFlags and just using:
  makeFlags = [
    "PREFIX=${placeholder ''out''}"
    "NO_VENV=1"
  ];

Nice! I did not even think about this.

jtojnar · 2019-12-13T15:10:54Z

pkgs/applications/misc/dupeguru/default.nix

+    "--assume-old=pyc"
+  ];
+
+  doCheck = false;


We could probably do

checkInputs = with python3Packages; [ pytest pytest-monkeyplus ]; pytestFlagsArray = [ "core" "hscommon" ];

but that would require packaging pytest-monkeyplus.

If you do not feel like packaging it, please add a comment explaining that we are missing some test dependencies.

I think that it will be a nice task for another pr. I don't feel myself ready to do it right now.

Could you add it as a comment then?

Suggested change

doCheck = false;

# TODO: package pytest-monkeyplus for running tests

# https://github.com/NixOS/nixpkgs/pull/75054/files#r357690123

doCheck = false;

@jtojnar fixed.

jtojnar · 2019-12-13T15:12:33Z

Also please split adding a maintainer into a separate commit a squash all dupeguru fixes.

jtojnar

Looks good to me, great job.

jtojnar

Looks good to me, great job.

novoxd · 2019-12-13T15:29:41Z

@jtojnar - what next? Is there any need to mention somebody who can make an additional review and merge pr?

jtojnar · 2019-12-13T15:30:25Z

I set it to merge once CI succeeds.

novoxd · 2019-12-13T15:31:25Z

@jtojnar thanks you a lot again, you made my day, +1 maintainer to nixpkgs.

novoxd force-pushed the dupeguru branch 2 times, most recently from 559e18c to 4a3cd3f Compare December 5, 2019 17:58

ofborg bot added 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Dec 5, 2019

doronbehar requested changes Dec 9, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

doronbehar reviewed Dec 9, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

novoxd requested a review from doronbehar December 10, 2019 11:47

doronbehar approved these changes Dec 11, 2019

View reviewed changes

jtojnar requested changes Dec 11, 2019

View reviewed changes

jtojnar reviewed Dec 13, 2019

View reviewed changes

novoxd requested a review from jtojnar December 13, 2019 13:10

ofborg bot added 10.rebuild-darwin: 1-10 and removed 10.rebuild-darwin: 0 labels Dec 13, 2019

jtojnar requested changes Dec 13, 2019

View reviewed changes

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

novoxd requested a review from jtojnar December 13, 2019 14:41

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Show resolved Hide resolved

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

ofborg bot added 11.by: package-maintainer 10.rebuild-darwin: 0 and removed 10.rebuild-darwin: 1-10 labels Dec 13, 2019

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Outdated Show resolved Hide resolved

novoxd requested a review from jtojnar December 13, 2019 14:56

jtojnar reviewed Dec 13, 2019

View reviewed changes

pkgs/applications/misc/dupeguru/default.nix Show resolved Hide resolved

novoxd requested a review from jtojnar December 13, 2019 15:07

jtojnar reviewed Dec 13, 2019

View reviewed changes

add novox maintainer

e9767b9

novoxd force-pushed the dupeguru branch from 652dcd7 to a4b5196 Compare December 13, 2019 15:20

novoxd requested a review from jtojnar December 13, 2019 15:20

dupeguru: init at 4.0.4

c2cf629

novoxd force-pushed the dupeguru branch from a4b5196 to c2cf629 Compare December 13, 2019 15:26

jtojnar requested changes Dec 13, 2019

View reviewed changes

jtojnar approved these changes Dec 13, 2019

View reviewed changes

jtojnar merged commit 968afa4 into NixOS:master Dec 13, 2019

novoxd deleted the dupeguru branch December 13, 2019 15:38

lovesegfault mentioned this pull request Dec 17, 2019

lollypop broken with Namespace Gtk not available #75844

Closed

worldofpeace mentioned this pull request Dec 22, 2019

wrapPython: makeWrapperArgs should be an array #76158

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dupeguru: init at 4.0.4 #75054

dupeguru: init at 4.0.4 #75054

novoxd commented Dec 5, 2019

novoxd commented Dec 5, 2019 •

edited

novoxd commented Dec 9, 2019

doronbehar commented Dec 9, 2019 via email

novoxd commented Dec 11, 2019

doronbehar commented Dec 11, 2019

novoxd commented Dec 11, 2019

jtojnar Dec 13, 2019

jtojnar Dec 13, 2019

jtojnar Dec 13, 2019 •

edited

jtojnar Dec 13, 2019

lovesegfault Dec 18, 2019

lovesegfault Dec 18, 2019

novoxd commented Dec 13, 2019 •

edited

doronbehar commented Dec 13, 2019

jtojnar Dec 13, 2019

novoxd commented Dec 13, 2019

jtojnar commented Dec 13, 2019 •

edited

novoxd commented Dec 13, 2019

novoxd commented Dec 13, 2019 •

edited

jtojnar commented Dec 13, 2019

jtojnar Dec 13, 2019

jtojnar commented Dec 13, 2019

novoxd commented Dec 13, 2019

jtojnar Dec 13, 2019

novoxd Dec 13, 2019

jtojnar Dec 13, 2019

novoxd Dec 13, 2019

jtojnar commented Dec 13, 2019

jtojnar left a comment

jtojnar left a comment

novoxd commented Dec 13, 2019 •

edited

jtojnar commented Dec 13, 2019

novoxd commented Dec 13, 2019

dupeguru: init at 4.0.4 #75054

dupeguru: init at 4.0.4 #75054

Conversation

novoxd commented Dec 5, 2019

Motivation for this change

Things done

novoxd commented Dec 5, 2019 • edited

novoxd commented Dec 9, 2019

doronbehar commented Dec 9, 2019 via email

novoxd commented Dec 11, 2019

doronbehar commented Dec 11, 2019

novoxd commented Dec 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtojnar Dec 13, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

novoxd commented Dec 13, 2019 • edited

doronbehar commented Dec 13, 2019

Choose a reason for hiding this comment

novoxd commented Dec 13, 2019

jtojnar commented Dec 13, 2019 • edited

novoxd commented Dec 13, 2019

novoxd commented Dec 13, 2019 • edited

jtojnar commented Dec 13, 2019

Choose a reason for hiding this comment

jtojnar commented Dec 13, 2019

novoxd commented Dec 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtojnar commented Dec 13, 2019

jtojnar left a comment

Choose a reason for hiding this comment

jtojnar left a comment

Choose a reason for hiding this comment

novoxd commented Dec 13, 2019 • edited

jtojnar commented Dec 13, 2019

novoxd commented Dec 13, 2019

novoxd commented Dec 5, 2019 •

edited

jtojnar Dec 13, 2019 •

edited

novoxd commented Dec 13, 2019 •

edited

jtojnar commented Dec 13, 2019 •

edited

novoxd commented Dec 13, 2019 •

edited

novoxd commented Dec 13, 2019 •

edited