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

wip: openstackclient: 1.7.1 -> 3.11.0 #25898

Closed
wants to merge 1 commit into from

Conversation

nlewo
Copy link
Member

@nlewo nlewo commented May 18, 2017

Dependencies are private and have been generated by pypi2nix.

Motivation for this change

Upgrade

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

@mention-bot
Copy link

@nlewo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

# We don't have build requires
doCheck = false;

propagatedBuildInputs = [
Copy link
Member

Choose a reason for hiding this comment

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

Might as well use propagatedBuildInputs = lib.attrValues generated; considering they'are all propagated anyhow.

Copy link
Member Author

@nlewo nlewo May 19, 2017

Choose a reason for hiding this comment

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

yes, it would be better... but i get a strange error when I'm doing this:

nix-build ./ -A openstackclient  --show-trace
error: while evaluating the attribute ‘propagatedBuildInputs’ of the derivation ‘python-openstackclient-3.11.0’ at /.../nixpkgs/pkgs/development/interpreters/python/mk-python-derivation.nix:56:3:
cannot coerce a function to a string, at /.../nixpkgs/pkgs/development/interpreters/python/mk-python-derivation.nix:56:3

I don't know for now why these expressions are not equivalent.

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm, I have to use fix instead of fix' but I still don't understand (ok, I don't try too much to understand....).

homepage = "http://docs.openstack.org/developer/python-openstackclient";
license = "License :: OSI Approved :: Apache Software License";
description = "OpenStack Command-line Client";
};
Copy link
Member

Choose a reason for hiding this comment

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

maintainer?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@@ -4453,7 +4453,7 @@ in {

propagatedBuildInputs = with self; [
six keystoneclient prettytable oslo-utils oslo-i18n lxml httplib2 cliff
dogpile_cache appdirs anyjson pbr openstackclient
dogpile_cache appdirs anyjson pbr pkgs.openstackclient
Copy link
Member

Choose a reason for hiding this comment

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

This is going to cause trouble. What will happen is that all the propagatedBuildInputs of openstackclient will also be added to the PYTHONPATH of this package. Is your intention that it should have openstackclient on PATH?

Copy link
Member Author

Choose a reason for hiding this comment

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

openstackclient brings python modules that are required by ironicclient. So, it had to be in propagatedBuildInputs. But ironicclient is currently broken because cliff is. I will simply mark ironicclient as broken and explain why in a commen.

In fact, ironicclient should be removed and added to the openstaclient one... but I personally really don't need it.

@nlewo
Copy link
Member Author

nlewo commented May 24, 2017

@FRidh I think I addressed all of your requests. Could you have a look please?

@FRidh
Copy link
Member

FRidh commented May 30, 2017

The dependencies here are generated. What is missing is the script/documentation that was used to generate it.

@nlewo
Copy link
Member Author

nlewo commented May 30, 2017

Currently, the only missing thing is that pipy2nix doesn't generate dependencies in a seperated files. I prepared a PR for pypi2nix but I was waiting to get feedback on this current PR in order to see if this is pertinent or not from the nixpkgs point of view.

@FRidh
Copy link
Member

FRidh commented May 30, 2017

@nlewo I'm not really fond of having applications use their own dependency package sets because now you also have to make sure (from a security point of view) to keep those dependencies up to date.
Its to me as if you get a statically compiled binary that may use insecure versions of libraries.
As it becomes more common that (larger) applications fix their dependencies, there sometimes is just no other way.

I've just sent a mail to nix-dev about this general issue. I prefer not to make a decision on this by myself because we may be setting a precedent then.

@nlewo
Copy link
Member Author

nlewo commented May 30, 2017

@FRidh Ok, let's continue on nix-dev. FYI, I submitted the PR nix-community/pypi2nix#127

@FRidh
Copy link
Member

FRidh commented May 30, 2017

Currently, the only missing thing is that pipy2nix doesn't generate dependencies in a seperated files.

What do you mean here? Do you want a separate file per package? There's no need for that when its generated.

@nlewo
Copy link
Member Author

nlewo commented May 30, 2017

pypi2nix currently generates the python requirements nix expressions in a file that also contains a lot of pypi2nix specific expressions which are useless in the nixpkgs repo. So, I just proposed to generate all of these expressions in one dedicated file. This file is called requirements_generated.nix. It is imported by the main pypi2nix nix expression but could also be used alone in a nixpkgs application.

@FRidh
Copy link
Member

FRidh commented May 30, 2017

From the mailing list:

As an example, openstackclient needs a version of the requests
library which is older than our pythonPackages.requests. We then need
to add another version but we don't want to mix different versions in
the python packages set (to avoid conflicts). In this context, it seems
the only way to bring openstackclient was to provide its dependencies
as 'private' dependencies.

If you just need a single (or a few) different dependencies, then its quite straigthforward.
You can define this version in your openstackclient expression, and override the python.pkgs package set to use that version.

@nlewo
Copy link
Member Author

nlewo commented May 30, 2017

The main problem is that the request library is used by other openstack dependencies (directs and transitives) and I then need to also override them. I tryied to do this for keystone (which have a lot of common deps with openstackclient) but we concluded this is not the good way (#25752). This is why I started to use pypi2nix.

@FRidh
Copy link
Member

FRidh commented May 30, 2017

@nlewo perhaps I should have been more clear then. Its not ok to add additional versions of packages to python-packages.nix, however, in your expression, you can override python-packages.nix and provide custom versions. E.g.

let
  mypython = python.pkgs.override {
    packageOverrides = self: super: {
      requests = super.buildPythonPackage {
        ...
      }; 
     };
  };
in ...

Then, mypython.pkgs uses the new requests for both direct and transitive dependencies.

@nlewo
Copy link
Member Author

nlewo commented Jun 1, 2017

@FRidh As mentionned by chat, it seems more complicated to partially use generated expressions than using all of them.
Could you move forward on this PR?

@FRidh
Copy link
Member

FRidh commented Jun 1, 2017

you need to include documentation on how to update these packages

- dependencies are private and have been generated by pypi2nix
- ironicclient is marked as broken
@nlewo
Copy link
Member Author

nlewo commented Jun 1, 2017

@FRidh I reworked how requirments are generated and how they are used in order to minimize manual actions to facilitate the upgrade of this package. I use a patched version of pypi2nix to generate a clean file. I'm currently discussing with pypi2nix maintainers to see how this could be integrated...
The way the pipy2nix file is generated could be largely improved but I would prefer to let these works for future PR:)
So please consider this PR as WIP until we have some news from pypi2nix.

@nlewo nlewo changed the title openstackclient: 1.7.1 -> 3.11.0 wip: openstackclient: 1.7.1 -> 3.11.0 Jun 1, 2017
@nlewo
Copy link
Member Author

nlewo commented Jun 13, 2017

I finally put it into nixpkgs-python for now... Thanks @FRidh for all your comments.

@nlewo nlewo closed this Jun 13, 2017
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

4 participants