bleachbit: 2.0 -> 2.2 #58152
bleachbit: 2.0 -> 2.2 #58152
Conversation
| chmod +x $out/bin/bleachbit | ||
|
|
||
| substituteInPlace $out/bin/bleachbit --replace "#!/usr/bin/env python" "#!${pythonPackages.python.interpreter}" | ||
| make install SHELL=${stdenv.shell} prefix=${placeholder "out"} |
There was a problem hiding this comment.
Can't you drop this entirely and just use makeFlags?
|
On Fri, 22 Mar 2019 19:08:20 -0700, worldofpeace ***@***.***> wrote:
worldofpeace commented on this pull request.
> };
+ nativeBuildInputs = [ gettext ];
buildInputs = [ pythonPackages.wrapPython ];
```suggestion
```
Unneeded as it's done in `mk-python-derivation.nix`.
Hmm, curious. If I remove it the build fails trying to run `msgfmt`.
Or does it work for you w/o the `gettext` input?
…
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#58152 (review) part: text/html
|
|
On Fri, 22 Mar 2019 19:09:29 -0700, worldofpeace ***@***.***> wrote:
worldofpeace commented on this pull request.
> doCheck = false;
postInstall = ''
- mkdir -p $out/bin
- cp bleachbit.py $out/bin/bleachbit
- chmod +x $out/bin/bleachbit
-
- substituteInPlace $out/bin/bleachbit --replace "#!/usr/bin/env python" "#!${pythonPackages.python.interpreter}"
+ make install SHELL=${stdenv.shell} prefix=${placeholder "out"}
Can't you drop this entirely and just use `makeFlags`?
Not AFAICT? Normally yes (and wouldn't need to set SHELL ourselves) but
I don't see a way to do that here.
If that seems wrong or it works for you LMK haha :).
…
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#58152 (review) part: text/html
|
|
@dtzWill I think my comment on in email appears to be different than what I communicated. For example I suggested to remove |
Per reviewer feedback, thanks!
Ahh realized that we should set diff --git a/pkgs/applications/misc/bleachbit/default.nix b/pkgs/applications/misc/bleachbit/default.nix
index b2348bbf752..7b4b9cd0885 100644
--- a/pkgs/applications/misc/bleachbit/default.nix
+++ b/pkgs/applications/misc/bleachbit/default.nix
@@ -3,6 +3,8 @@ pythonPackages.buildPythonApplication rec {
pname = "bleachbit";
version = "2.2";
+ format = "other";
+
src = fetchurl {
url = "mirror://sourceforge/${pname}/${pname}-${version}.tar.bz2";
sha256 = "1yj9bc3k6s1aib7znb79h5rybfv691zz4szxkwf9fm9nr0dws603";
@@ -17,11 +19,13 @@ pythonPackages.buildPythonApplication rec {
find -type f -exec sed -i -e 's@/usr/bin@${placeholder "out"}/bin@g' {} \;
'';
- doCheck = false;
+ dontBuild = true;
- postInstall = ''
- make install SHELL=${stdenv.shell} prefix=${placeholder "out"}
- '';
+ installFlags = [ "prefix=${placeholder "out"}" ];
+
+ doCheck = true;
+
+ checkTarget = "tests";
propagatedBuildInputs = with pythonPackages; [ pygtk ];
|
|
On Fri, 22 Mar 2019 21:42:55 -0700, worldofpeace ***@***.***> wrote:
@dtzWill I think my comment on the mailing list appears to be different than what I communicated.
For example I suggested to remove `wrapPython`not `gettext`. and said
"Unneeded as it's done in mk-python-derivation.nix.".
...
Yes you are correct, I completely misread/misunderstood.
Sorry :3. Pushed commit making the change, good call thanks! :)
…
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#58152 (comment) part: text/html
|
Patch by @worldbypeace, during PR review. Thanks!
|
Don't see the tests running in the output, not sure why. Guess we should just leave that out so it doesn't look confusing. |
|
ping @dtzWill |
|
Pushed commit disabling tests, sorry for delay.
Was hoping to fix them (moving to `installCheckPhase = ` gets them
running) but too many small details to fixup so punting on that for now.
…On Sun, 31 Mar 2019 21:56:31 -0700, worldofpeace ***@***.***> wrote:
ping @dtzWill
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#58152 (comment) part: text/html
|
Looks good. It just needs a squash. I'd try to maybe keep some of the comments on what was done. Other than that this works 👍 . |
Motivation for this change
https://www.bleachbit.org/news/bleachbit-22
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)