Skip to content

Commit

Permalink
aws_shell: move to pythonPackages
Browse files Browse the repository at this point in the history 
This allows to have awscli running both under python 2 and 3 as
necessary.
  • Loading branch information
@zimbatm
zimbatm committed Jan 10, 2018
1 parent 29b8e5f commit ffd35a7
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkgs/top-level/all-packages.nix
View file
Expand Up @@ -539,7 +539,7 @@ with pkgs;

awslogs = callPackage ../tools/admin/awslogs { };

aws_shell = pythonPackages.callPackage ../tools/admin/aws_shell { };
aws_shell = pythonPackages.aws_shell;

azure-cli = nodePackages.azure-cli;

Expand Down
2 changes: 2 additions & 0 deletions pkgs/top-level/python-packages.nix
View file
Expand Up @@ -177,6 +177,8 @@ in {

awscli = callPackage ../tools/admin/awscli { };

aws_shell = callPackage ../tools/admin/aws_shell { };

# packages defined elsewhere

backports_csv = callPackage ../development/python-modules/backports_csv {};
Expand Down

6 comments on commit ffd35a7

@FRidh
Copy link
Member

@FRidh FRidh commented on ffd35a7 Jan 10, 2018

Choose a reason for hiding this comment

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

Can you revert this. python-packages.nix should not import from anywhere but python-modules.

This allows to have awscli running both under python 2 and 3 as
necessary.

Is it necessary?

@zimbatm
Copy link
Member Author

Choose a reason for hiding this comment

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

@FRidh how about I move the package under python-modules then?

The issue is that when using nix-shell, because of the python env vars get set, it doesn't work very well to have both 3 and 2 packages at the same time. Plus in the future we probably want to default on python 3.

@FRidh
Copy link
Member

@FRidh FRidh commented on ffd35a7 Jan 11, 2018

Choose a reason for hiding this comment

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

How do you use it with nix-shell? If you use e.g.

nix-shell -p aws_shell -p 'python3.withPackages(ps: with ps; [ your packages ])'

then there should not be any issues. If you instead use

nix-shell -p aws_shell -p python3Packages.your -p python3Packages.packages

then there will be issues because this method should not be used.

The best available solution is with 1.12pre

nix run -f . aws_shell  'python3.withPackages(ps: with ps; [ your packages ])'

because nix run does not run any setup hooks.

@zimbatm
Copy link
Member Author

Choose a reason for hiding this comment

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

I use shell.nix to declare development dependencies in various projects so it's always going to load these environment variables no?

Here is the current example:

let
  pkgsSrc = builtins.fetchTarball "https://github.com/NixOS/nixpkgs/archive/ffd35a7849f6477eeba9deeea33c8dcc2d582383.tar.gz";
  pkgs = import pkgsSrc { config = {}; overlays = []; };
in
pkgs.mkShell {
  buildInputs = (with pkgs; [
    bashInteractive
    terraform-full
  ]) ++ (with pkgs.python3Packages; [
    awscli
    aws_shell
    notebook
  ]);
}

@FRidh
Copy link
Member

@FRidh FRidh commented on ffd35a7 Jan 11, 2018 •
edited

Choose a reason for hiding this comment

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

Correct. The issue is that we propagate Python as build input and thus introduce the setup hook. The hook makes sense when developing a package with the -A option but otherwise it does not. python.withPackages or python.buildEnv won't propagate this hook, so wrapping each Python application in one of these will solve the issue even better (#16672)

@zimbatm
Copy link
Member Author

@zimbatm zimbatm commented on ffd35a7 Jan 15, 2018 •
edited

Choose a reason for hiding this comment

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

Reverted in fad6250 and 1775b03. I think that apps should have a mkPythonApp that doesn't forward these environment variables

Please sign in to comment.