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

Update vdirsyncer #33052

Closed
wants to merge 6 commits into from
Closed

Conversation

matthiasbeyer
Copy link
Contributor

Motivation for this change

Targets #33050

Things done

vdirsyncer requires a new dependency which is written in Rust. Unfortunately, these patches make me build rustc from source (so I aborted this) so I cannot test whether packaging works.

I'm not sure how to proceed here.

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

@matthiasbeyer matthiasbeyer requested a review from FRidh as a code owner December 25, 2017 14:22
@@ -0,0 +1,22 @@
{ lib, python3Packages, buildPythonPackage, fetchurl }:
Copy link
Member

Choose a reason for hiding this comment

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

no pythonPackages, individual packages as parameters

version = "0.1.1";
name = "${pname}-${version}";

src = fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

fetchPypi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does not work. I tested it, but it can't find the package. That's why I used fetchurl.

Copy link
Member

Choose a reason for hiding this comment

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

because you request a zip. It needs `extension = "zip";'

buildPythonPackage rec {
pname = "milksnake";
version = "0.1.1";
name = "${pname}-${version}";
Copy link
Member

Choose a reason for hiding this comment

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

drop name

sha256 = "12bdyc2kjqpiq8wrbsk7ymcq2xdakri3bsvaqgsyzj33wyzbcwzy";
};

buildInputs = with python3Packages; [
Copy link
Member

Choose a reason for hiding this comment

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

cffi only at build time?


src = fetchurl {
url = "https://pypi.python.org/packages/04/12/358c2c7a27f06a71d69e44a4b7f62411524c25e6c7284b5a0f757a9ba068/milksnake-0.1.1.zip";
sha256 = "12bdyc2kjqpiq8wrbsk7ymcq2xdakri3bsvaqgsyzj33wyzbcwzy";
};

buildInputs = with python3Packages; [
propagatedBuildInputs = [
Copy link
Member

Choose a reason for hiding this comment

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

Still, how is this going to work? If it uses cffi it tries to load a library, and for that it first needs to find it. How will it find that library outside build-time? And which library does it need?

milksnake

pkgs.cargo
pkgs.rustc
Copy link
Member

Choose a reason for hiding this comment

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

Please import cargo and rustc without relying on pkgs. This is needed for vdirsyncer.override {} to work.

pname = "milksnake";
version = "0.1.1";

src = fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

fetchPypi with extension = "zip";

};

propagatedBuildInputs = [
cffi
Copy link
Member

Choose a reason for hiding this comment

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

no patching is needed?

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 don't know how to do this, TBH.

meta = with lib; {
description = "A python library that extends setuptools for binary extensions.";
homepage = https://pypi.python.org/pypi/milksnake/;
license = licenses.apl;
Copy link
Member

Choose a reason for hiding this comment

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

maintainer?

@@ -1,17 +1,17 @@
{ stdenv, fetchurl, python3Packages, glibcLocales }:
{ stdenv, fetchurl, python3Packages, glibcLocales, pkgs, rust, cargo }:
Copy link
Member

Choose a reason for hiding this comment

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

no pkgs


# Packaging documentation at:
# https://github.com/untitaker/vdirsyncer/blob/master/docs/packaging.rst
let
pythonPackages = python3Packages;
in
pythonPackages.buildPythonApplication rec {
version = "0.16.3";
version = "0.17.0a1";
name = "vdirsyncer-${version}";

src = fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

fetchPypi


# Packaging documentation at:
# https://github.com/untitaker/vdirsyncer/blob/master/docs/packaging.rst
let
pythonPackages = python3Packages;
in
pythonPackages.buildPythonApplication rec {
version = "0.16.3";
version = "0.17.0a1";
Copy link
Member

Choose a reason for hiding this comment

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

we typically do not have alpha's in Nixpkgs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to leave this unmerged (even after the remaining issues with this PR are solved) until I can bump to a new stable release (if there is any).

@xeji
Copy link
Contributor

xeji commented Aug 5, 2018

closing as outdated, master has vdirsyncer 0.17.0a3 now.

@xeji xeji closed this Aug 5, 2018
@matthiasbeyer matthiasbeyer deleted the update-vdirsyncer branch May 18, 2019 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants