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

netatalk: 3.1.7 -> 3.1.11 #31377

Merged
merged 3 commits into from Nov 9, 2017
Merged

netatalk: 3.1.7 -> 3.1.11 #31377

merged 3 commits into from Nov 9, 2017

Conversation

kyren
Copy link
Contributor

@kyren kyren commented Nov 7, 2017

Motivation for this change

Netatalk 3.1.11 is the latest stable release. There is also a nasty bug
with netatalk < 3.1.11 (https://sourceforge.net/p/netatalk/bugs/636/)
that prevents using netatalk shares for time machine backups.

I've tested all the executable files in result/bin, and there are script files there that depend on python and perl using /usr/bin/env. I'm not sure if that's a problem or not, but it is the same situation as before this patch.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@orivej
Copy link
Contributor

orivej commented Nov 8, 2017

I've tested all the executable files in result/bin, and there are script files there that depend on python and perl using /usr/bin/env. I'm not sure if that's a problem or not, but it is the same situation as before this patch.

This is fine for optional and almost useless programs, otherwise just add python or perl to the nativeBuildInputs and the default fixup phase will substitute them into scripts. If you don't need to run those scripts, we could merge this as is.

You can also add yourself to maintainers to review changes to this package in the future (when we resume notifying maintainers).

Netatalk 3.1.11 is the latest stable release.  There is also a nasty bug
with netatalk < 3.1.11 (https://sourceforge.net/p/netatalk/bugs/636/)
that prevents using netatalk shares for time machine backups.
Adds `perl` and `python2` to nativeBuildInputs, to make the included
perl and python utility script `#!` lines point at the right place.
@kyren
Copy link
Contributor Author

kyren commented Nov 8, 2017

Okay, so I've added perl and python2 to nativeBuildInputs, and that actually fixes all of the included utility scripts except for afpstats.py, which does not work because it requires the python library for dbus. There was also a travis build failure before my latest commit that I'm not entirely sure about yet.

In case it wasn't obvious though this is my first nixpkgs PR, and I feel like I'm a bit out of my depth. I'm not sure exactly how to fix afpstats.py, I know that I need to add a runtime dependency on python2Packages.dbus... somehow, but I wasn't able to figure it out. I'm also not sure whether I added the correct version of python to nativeBuildInputs, I mean the scripts work, but should it have been python27 instead of python2?

@orivej
Copy link
Contributor

orivej commented Nov 9, 2017

I'm not sure exactly how to fix afpstats.py, I know that I need to add a runtime dependency on python2Packages.dbus... somehow, but I wasn't able to figure it out.

Packages built with buildPythonPackage automatically add all their python build inputs into .py programs (by patching them). This is not such a package, and corresponding action is generally achieved by:

{
  # Import wrapPythonPrograms function.
  nativeBuildInputs = [ pythonPackages.wrapPython ];
  # Export pythonPath.
  pythonPath = with pythonPackages; [ mpd pygtk dbus-python notify mutagen ];
  # Wrap all python programs in $out/bin.
  postInstall = "wrapPythonPrograms";
}

Here I've chosen to be more specific, and instead of wrapping all python programs I only add the path to dbus into afpstats:

{
  postInstall = ''
    buildPythonPath ${python.pkgs.dbus-python}
    patchPythonScript $out/bin/afpstats
  '';
}

I'm also not sure whether I added the correct version of python to nativeBuildInputs, I mean the scripts work, but should it have been python27 instead of python2?

Usually we say just python which defaults to python27, and if we need another version it is specified in the callPackage argument in all-packages.nix.

@kyren
Copy link
Contributor Author

kyren commented Nov 9, 2017

Thank you a lot for all the help and explanation, it all seems pretty reasonable and clear.

I feel like instead of helping all I did was trigger the people who actually know what they're doing to come fix things for me, but I appreciate it all the same.

I'm pretty sure I'm actually going to stick with nixos for real this time, so hopefully I'll get better at this as time goes on :)

@orivej orivej merged commit 8bc47ab into NixOS:master Nov 9, 2017
@orivej
Copy link
Contributor

orivej commented Nov 9, 2017

You are welcome!

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

Successfully merging this pull request may close these issues.

None yet

5 participants