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.

Sign up for GitHub

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.glances: fix darwin build #78404

Merged
merged 1 commit into from Jan 31, 2020
Merged

pythonPackages.glances: fix darwin build #78404

merged 1 commit into from Jan 31, 2020
+29 −9

Conversation

cust0dian-old
Copy link
Contributor

Motivation for this change

Currently glances builds only on Linux due to unconditional dependency on hddtemp (which supports only Linux).

These changes also fix IP plugin init failure (which was reported and fixed upstream) and nix-specific segfault due to CoreFoundation mismatch.

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.

cust0dian-old
cust0dian-old commented Jan 24, 2020
View reviewed changes
pkgs/development/python-modules/glances/default.nix
# Relevant: https://github.com/NixOS/nixpkgs/issues/24693
makeWrapperArgs = lib.optional stdenv.isDarwin [
"--set" "DYLD_FRAMEWORK_PATH" "/System/Library/Frameworks"
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quoting @LnL7 on IRC:

<LnL> yeah, we solved this for binaries but for languages with an interpreter it's still a problem and requires DYLD_FRAMEWORK_PATH in the wrapper

@cust0dian-old cust0dian-old changed the title glances: fix darwin build pythonPackages.glances: fix darwin build Jan 24, 2020
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jan 24, 2020
@cust0dian-old
Copy link
Contributor Author

Note that build fails on Python 3.7 and 3.8 due to missing psutil(?):

builder for '/nix/store/s5fm7dj32ja43gvizw2qrkv5mxilsk2p-python3.8-glances-3.1.3.drv' failed with exit code 1; last 10 log lines:
  running install tests
  no Makefile or custom buildPhase, doing nothing
  pythonCatchConflictsPhase
  pythonRemoveBinBytecodePhase
  pythonImportsCheckPhase
  Executing pythonImportsCheckPhase
  setuptoolsCheckPhase
  Executing setuptoolsCheckPhase
  running test
  psutil library not found. Glances cannot start.
builder for '/nix/store/k7hi80d9c5gg2vlixiv1qgl5g285fgh9-python3.7-glances-3.1.3.drv' failed with exit code 1; last 10 log lines:
  running install tests
  no Makefile or custom buildPhase, doing nothing
  pythonCatchConflictsPhase
  pythonRemoveBinBytecodePhase
  pythonImportsCheckPhase
  Executing pythonImportsCheckPhase
  setuptoolsCheckPhase
  Executing setuptoolsCheckPhase
  running test
  psutil library not found. Glances cannot start.

but I'm not sure if it's related to my changes? I can debug this further if it's an issue.

jonringer
jonringer reviewed Jan 24, 2020
View reviewed changes
pkgs/development/python-modules/glances/default.nix
# Relevant: https://github.com/NixOS/nixpkgs/issues/24693
makeWrapperArgs = lib.optional stdenv.isDarwin [
"--set" "DYLD_FRAMEWORK_PATH" "/System/Library/Frameworks"
];

doCheck = true;
checkInputs = [ unittest2 ];
Copy link
Contributor

@jonringer jonringer Jan 24, 2020 •
edited

Choose a reason for hiding this comment

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

Suggested change
checkInputs = [ unittest2 ];
checkInputs = [ unittest2 ] ++ lib.optionals stdenv.isDarwin [ psutil ];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, I didn't realize nixpkgs-review runs build in a sandbox. 2.7 build succeeded there only because it was already built without sandbox during my testing.

On Darwin with sandbox all builds (2.7, 3.7, 3.8) fail in the same way, even if I add psutil to checkInputs. Do you happen to know if sandbox on Darwin is reliable and endorsed? Should I dig deeper to make these builds work there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OfBorg built packages on Darwin just fine, so I'm guessing these failures have something to do with my env.

@jonringer
Copy link
Contributor

@GrahamcOfBorg build python27Packages.glances python37Packages.glances python38Packages.glances

@ofborg ofborg bot requested a review from primeos January 24, 2020 10:36
@jonringer
Copy link
Contributor

please squash commits

git reset HEAD^
git add pkgs/
git commit --amend --no-edit
git push .. ... --force

primeos
primeos approved these changes Jan 24, 2020
View reviewed changes
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

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

I cannot test Darwin changes (that's why I didn't reply yet, but as maintainer I probably should do it anyway) but the diff LGTM.

So basically ACK (once squashed) and my thanks to the reviewers :)

@ofborg ofborg bot requested a review from primeos January 25, 2020 06:56
@cust0dian-old
Copy link
Contributor Author

cust0dian-old commented Jan 31, 2020 •
edited

@jonringer sorry for the ping, I know this is far from the only PR where a review is requested from you — I just wanted to get an idea of when you think you'll have time to take a look at it again?

These are the same changes you've reviewed before, just all squashed into one commit.

I'm hoping that with yours and primeos' approval it will be clear that it's ready to be merged for when whoever has time to do that comes along.

@LnL7 LnL7 merged commit 62bbc2a into NixOS:master Jan 31, 2020
@jonringer
Copy link
Contributor

sorry, been busy with real life obligations

@cust0dian-old
Copy link
Contributor Author

No worries, hope everything is alright!

@cust0dian-old cust0dian-old deleted the glances-darwin-build branch January 31, 2020 08:08
anna328p pushed a commit to anna328p/nixpkgs that referenced this pull request Feb 2, 2020
@LnL7 @anna328p
Merge pull request NixOS#78404 from cust0dian/glances-darwin-build
b0bf390 
pythonPackages.glances: fix darwin build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LnL7 LnL7 LnL7 approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@primeos primeos Awaiting requested review from primeos

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: python 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
5 participants
@cust0dian-old @jonringer @LnL7 @primeos @cust0dian