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

gwyddion: 2.48 -> 2.55 (and add options) #83114

Merged
merged 2 commits into from Mar 27, 2020
Merged

gwyddion: 2.48 -> 2.55 (and add options) #83114

merged 2 commits into from Mar 27, 2020

Conversation

cgevans
Copy link
Contributor

@cgevans cgevans commented Mar 22, 2020

Motivation for this change

Gwyddion has a large number of optional dependencies. In this update, those dependencies have been added with options set mostly to true, and an option (default false) for working Python 2.7 support in the program and in Python (but without the gwyutils module) has been added. I have also added myself as a maintainer as no maintainer is currently set.

Upstream has made the gwyutils module intentionally hard to use by placing it outside the Python path, and I'm not sure the best way to fix this, or whether it should be fixed. I have also not added options for ruby and perl support, as I don't know how that support works (it isn't listed in the configuration summary).

Things done

(Tested both gwyddion (and confirmed no Python support) and python27Packages.gwyddion (and confirmed Python support))

  • 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.

@cgevans
Copy link
Contributor Author

cgevans commented Mar 22, 2020

@P-E-Meunier, it looks like by some coincidence you were actually the original author of this derivation?

@P-E-Meunier
Copy link
Contributor

Yes, when I noticed you hadn't done it yet ;-)

@P-E-Meunier
Copy link
Contributor

Adding an install phase takes care of your problem with the python path: ~

  installPhase = ''
    make install   
    mkdir -p $out/${pythonPackages.python.sitePackages}
    cp $out/share/gwyddion/pygwy/* $out/${pythonPackages.python.sitePackages}
  '';

But I can't get the python module to work, I keep getting an error: "could not import gtk". I don't understand where the python files are installed, do you?

@cgevans
Copy link
Contributor Author

cgevans commented Mar 22, 2020

But I can't get the python module to work, I keep getting an error: "could not import gtk". I don't understand where the python files are installed, do you?

That sounds like a dependency problem with its dependency on pygtk. It works for me, but I need to use something like the following: python27.withPackages (ps: [ps.gwyddion]). It may be that gwyddion's mechanism of interfacing with python is somehow escaping Nix's paths, and so works fine if Python 2.7 on the current system has pygtk/etc, but not if python27Packages.gwyddion is installed separately?

I'm primarily testing this in Python, not in the embedded console, as my primary use-case is scripting for consistent batch levelling and conversion, but it looks like the embedded console is also working for me.

@P-E-Meunier
Copy link
Contributor

works fine if Python 2.7 on the current system has pygtk/etc, but not if python27Packages.gwyddion is installed separately?

That is what propagatedBuildInputs are for. Btw, I can import pygtk fine, just not "gtk".

jonringer
jonringer reviewed Mar 24, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

please squash the fixup commits:

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

@jonringer
Copy link
Contributor

https://nixos.org/nixpkgs/manual/#submitting-changes-making-patches

@cgevans
gwyddion: 2.48 -> 2.55 (and add options)
1f86f1a 
Gwyddion has a large number of optional dependencies. In this update,
those dependencies have been added with options set mostly to true,
and an option (default false) for working Python 2.7 support in the
program and in Python has been added. I have also added myself as a
maintainer as no maintainer is currently set.
@cgevans
Copy link
Contributor Author

cgevans commented Mar 24, 2020

Done. Wasn't sure whether I should squash immediately, or leave the fixups there to show them for review before squashing.

@cgevans
Copy link
Contributor Author

cgevans commented Mar 25, 2020

@P-E-Meunier, does this work for you now, at least with python27.withPackages (ps: [ps.gwyddion])? I had another python 2.7 package that was confusing me about propagated dependencies; it should be fixed now.

In theory, this should build on macOS; maybe Damien could test it? The python API is enormously useful for consistent mass conversion of AFM images.

@P-E-Meunier
Copy link
Contributor

I does work, and TIL it's actually how Python packages are meant to be used:

https://nixos.org/nixpkgs/manual/#installing-python-and-packages

About OSX, I don't know how pygtk works there.

@jonringer
Copy link
Contributor

python27.withPackages (ps: [ps.gwyddion])

withPackages will install the packages in the base site-packages, having a list of python modules will just make them appear in PYTHONPATH

jonringer
jonringer approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

diff LGTM
commits LGTM

https://github.com/NixOS/nixpkgs/pull/83114
2 package built:
gwyddion python27Packages.gwyddion

congrats to @cgevans on his first PR :)

@jonringer jonringer merged commit 59fbc7a into NixOS:master Mar 27, 2020
@P-E-Meunier
Copy link
Contributor

I didn't know it was @cgevans' first PR. Congrats! Now I can't wait to meet the third user of Gwyddion in Nixpkgs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

@FRidh FRidh Awaiting requested review from FRidh

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cgevans @P-E-Meunier @jonringer @Janik-Haag