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

Jump to bottom

pythonPackages.myfitnesspal: fix build #101760

Merged
merged 1 commit into from Oct 26, 2020
Merged

pythonPackages.myfitnesspal: fix build #101760

merged 1 commit into from Oct 26, 2020
+12 −5

Conversation

IvarWithoutBones
Copy link
Member

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.

@ofborg ofborg bot requested a review from bhipple October 26, 2020 13:22
mweinelt
mweinelt reviewed Oct 26, 2020
View reviewed changes
Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member Author

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 commented Oct 26, 2020 via email

@mweinelt
Copy link
Member

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
Member Author

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 commented Oct 26, 2020 •
edited

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
Member Author

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 :)

@mweinelt
Copy link
Member

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
@IvarWithoutBones IvarWithoutBones deleted the unbreak-myfitnesspal branch October 26, 2020 16:23
@IvarWithoutBones IvarWithoutBones mentioned this pull request Oct 26, 2020
Merged
10 tasks
@bhipple
Copy link
Contributor

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
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 requested changes

@mweinelt mweinelt mweinelt approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@bhipple bhipple Awaiting requested review from bhipple

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@IvarWithoutBones @mweinelt @bhipple @SuperSandro2000