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

pythonPackages.myfitnesspal: fix build #101760

Merged
merged 1 commit into from Oct 26, 2020

Conversation

@IvarWithoutBones
Copy link
Contributor

@IvarWithoutBones IvarWithoutBones commented Oct 26, 2020

Motivation for this change

Noticed this was broken on hydra, decided to fix.

ZHF: #97479

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Copy link
Member

@mweinelt mweinelt left a comment

This package has an entrypoint, which is basically an executable. It would therefore be appropriate to define it in all-packages.nix using toPythonApplication.

https://github.com/coddingtonbear/python-myfitnesspal/blob/782a0f7f6ffdb1ed2ae6e223774c18a5876f0035/setup.py#L27

https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/python.section.md#topythonapplication-function

pkgs/development/python-modules/myfitnesspal/default.nix Outdated Show resolved Hide resolved
@IvarWithoutBones IvarWithoutBones force-pushed the unbreak-myfitnesspal branch from 702ce30 to 58f14c5 Oct 26, 2020
@IvarWithoutBones
Copy link
Contributor Author

@IvarWithoutBones IvarWithoutBones commented Oct 26, 2020

This package has an entrypoint, which is basically an executable. It would therefore be appropriate to define it in all-packages.nix using toPythonApplication.

Done 👍 (I used python3Packages, hope that is fine?)

@mweinelt
Copy link
Member

@mweinelt mweinelt commented Oct 26, 2020

@mweinelt
Copy link
Member

@mweinelt mweinelt commented Oct 26, 2020

For some reason it can't import lxml.etree from the executable.

[nix-shell:~/.cache/nixpkgs-review/pr-101760]$ ./results/myfitnesspal/bin/myfitnesspal 
Traceback (most recent call last):
  File "/nix/store/1qqmr57abghbxqw95ig3x4q0q9fgsgm7-python3.8-myfitnesspal-1.16.1/bin/.myfitnesspal-wrapped", line 6, in <module>
    from myfitnesspal.cmdline import main
  File "/nix/store/kib0hpf9pdhdrc7h1pdjarm5gpk8j4q2-python3.7-myfitnesspal-1.16.1/lib/python3.7/site-packages/myfitnesspal/__init__.py", line 1, in <module>
    from myfitnesspal.client import Client  # noqa
  File "/nix/store/kib0hpf9pdhdrc7h1pdjarm5gpk8j4q2-python3.7-myfitnesspal-1.16.1/lib/python3.7/site-packages/myfitnesspal/client.py", line 9, in <module>
    import lxml.html
  File "/nix/store/pz16nz5jzjwmmn1hrngkc46lmwgm0pkp-python3.7-lxml-4.5.2/lib/python3.7/site-packages/lxml/html/__init__.py", line 53, in <module>
    from .. import etree
ImportError: cannot import name 'etree' from 'lxml' (/nix/store/pz16nz5jzjwmmn1hrngkc46lmwgm0pkp-python3.7-lxml-4.5.2/lib/python3.7/site-packages/lxml/__init__.py)

@IvarWithoutBones
Copy link
Contributor Author

@IvarWithoutBones IvarWithoutBones commented Oct 26, 2020

For some reason it can't import lxml.etree from the executable.

Getting the same error running from nixpkgs-review. Not sure how to fix this, seems to be a problem with toPythonApplication? The binary works fine from a local build.

~/nix/nixpkgs/pkgs/development/python-modules/myfitnesspal > nix-build -E '((import <nixpkgs> {}).python3Packages.callPackage (import ./default.nix) { })'
/nix/store/cn5c58igqiblh5dc22d4gl65xi35w9kq-python3.8-myfitnesspal-1.16.1
~/nix/nixpkgs/pkgs/development/python-modules/myfitnesspal > ./result/bin/myfitnesspal
usage: myfitnesspal [-h] [--loglevel LOGLEVEL] {store_password,store-password,delete_password,delete-password,day}
myfitnesspal: error: the following arguments are required: command

If anyone would have any hits that would be much appreciated. Not sure if we can create a wrapper for the python env.

@mweinelt
Copy link
Member

@mweinelt mweinelt commented Oct 26, 2020

I see the same, running nix-build -A myfitnesspal works just fine, nixpkgs-review fails. If push comes to shove, drop the all-packages reference, we're here to fix the build, improving it would be a bonus.

@IvarWithoutBones
Copy link
Contributor Author

@IvarWithoutBones IvarWithoutBones commented Oct 26, 2020

I see the same, running nix-build -A myfitnesspal works just fine, nixpkgs-review fails. If push comes to shove, drop the all-packages reference, we're here to fix the build, improving it would be a bonus.

Cool, I've added a TODO comment so the maintainer can fix this later. I'm personally not too invested in getting this to work, just wanted to get closer to ZHF :)

@IvarWithoutBones IvarWithoutBones force-pushed the unbreak-myfitnesspal branch from 58f14c5 to 7cea883 Oct 26, 2020
@mweinelt
Copy link
Member

@mweinelt mweinelt commented Oct 26, 2020

Result of nixpkgs-review pr 101760 1

2 packages built:
  • python37Packages.myfitnesspal
  • python38Packages.myfitnesspal

@mweinelt mweinelt merged commit 377d00d into NixOS:master Oct 26, 2020
13 of 16 checks passed
@IvarWithoutBones IvarWithoutBones deleted the unbreak-myfitnesspal branch Oct 26, 2020
@bhipple
Copy link
Contributor

@bhipple bhipple commented Oct 27, 2020

Thanks for the fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants