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

python3Minimal: Add top-level for a minimal Python 3 build #66762

Merged
merged 8 commits into from Aug 21, 2019

Conversation

@adisbladis
Copy link
Member

commented Aug 17, 2019

Motivation for this change

The regular python3 closure is pretty big, especially for non-interactive scripts.

Before:

$  du -sch (nix-store -qR (nix-build '<nixpkgs>' --no-out-link -A python3)) | sort -h
56K	/nix/store/mppjwxsqh2jfzsy06zqwpwra1dcn66f2-libffi-3.2.1
80K	/nix/store/g8al6vdf21mgfi5774qx6r6cil1j7zr8-bzip2-1.0.6.0.1
132K	/nix/store/w3wqang215is14xqajycbxmd52b44qkw-zlib-1.2.11
260K	/nix/store/lflh9l7j6v456xvk4sq8ax51pycz5l21-expat-2.2.7
344K	/nix/store/ji9kpgmbddkv2l2szzjfqinhja2322k0-xz-5.2.4
392K	/nix/store/f00sh4p25dcg3ywcr2hzj0wyb01hgyrn-readline-6.3p08
696K	/nix/store/1g2vh6nqpx630fz7dbgwds11b6q70wxk-gdbm-1.18.1
1.2M	/nix/store/ph2lq28vn677rs9ibxcffwgyd8rb0w04-sqlite-3.28.0
1.3M	/nix/store/xfghy8ixrhz3kyy6p724iv3cxji088dx-bash-4.4-p23
3.6M	/nix/store/rngzs8bxnajzpwnkk4896gy9d5s525ic-openssl-1.0.2s
7.8M	/nix/store/2fxvx0vh4h6j6z09ihd2ifcpqsy7lw68-ncurses-6.1-20190112
29M	/nix/store/iykxb0bmfjmi7s53kfg6pjbfpd8jmza6-glibc-2.27
57M	/nix/store/w7gsq8v86hni4ynaqgwwlnlny115ylng-python3-3.7.4
101M	total

After:

$ du -sch (nix-store -qR (nix-build '<nixpkgs>' --no-out-link -A python3Minimal)) | sort -h
56K	/nix/store/mppjwxsqh2jfzsy06zqwpwra1dcn66f2-libffi-3.2.1
80K	/nix/store/g8al6vdf21mgfi5774qx6r6cil1j7zr8-bzip2-1.0.6.0.1
132K	/nix/store/w3wqang215is14xqajycbxmd52b44qkw-zlib-1.2.11
260K	/nix/store/lflh9l7j6v456xvk4sq8ax51pycz5l21-expat-2.2.7
344K	/nix/store/ji9kpgmbddkv2l2szzjfqinhja2322k0-xz-5.2.4
1.3M	/nix/store/xfghy8ixrhz3kyy6p724iv3cxji088dx-bash-4.4-p23
29M	/nix/store/iykxb0bmfjmi7s53kfg6pjbfpd8jmza6-glibc-2.27
56M	/nix/store/f3gzz8iqb4g07qxbcrg4sw14xpxindb4-python3-3.7.4
87M	total
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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 nix-review --run "nix-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.
Notify maintainers

cc @

@adisbladis adisbladis requested review from Mic92, flokli and FRidh Aug 17, 2019
@adisbladis adisbladis changed the title Python minimal python3Minimal: Add top-level for a minimal Python 3 build Aug 17, 2019
@ofborg ofborg bot added the 6.topic: python label Aug 17, 2019
Copy link
Member

left a comment

Do you have a specific case in Nixpkgs in mind where you want to use this? In the past "crippled" versions were removed because of the confusion they could cause. Having tkinter in a separate package was a compromise for having a decent closure size. See
#19255

Consider also the name for when people do use nix-env -i.

Note there is also the boot.nix, though that is specifically for Darwin for bootstrapping.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@adisbladis adisbladis force-pushed the adisbladis:python-minimal branch 2 times, most recently from e985377 to 3b9c745 Aug 17, 2019
@ofborg ofborg bot requested a review from FRidh Aug 17, 2019
@adisbladis

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2019

Do you have a specific case in Nixpkgs in mind where you want to use this?

Particularly in https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/system/boot/loader/systemd-boot/systemd-boot-builder.py and in similar cases like #48378.

@FRidh

This comment has been minimized.

Copy link
Member

commented Aug 17, 2019

Is that really worth it? Many people will have another python3 on their system anyway.

@adisbladis adisbladis force-pushed the adisbladis:python-minimal branch from 3b9c745 to 09f0f1f Aug 17, 2019
@ofborg ofborg bot requested a review from lovek323 Aug 17, 2019
@adisbladis adisbladis force-pushed the adisbladis:python-minimal branch from 09f0f1f to a6c23e2 Aug 17, 2019
@adisbladis adisbladis removed the request for review from lovek323 Aug 17, 2019
@adisbladis

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2019

Is that really worth it? Many people will have another python3 on their system anyway.

Maybe not yet? I do however think it's worth it as a step in the right direction for smaller closures.

I also see the potential in other areas such as dockerTools which I personally use a lot in CI environments that are not always having a local cache, so any savings in dependency fetching is great.

In the past "crippled" versions were removed because of the confusion they could cause.

We explicitly call it python3Minimal so it should be obvious that it's not what you'd want to use by default.

As a side note: python3Minimal clocks in at 90483408 bytes according to nix path-info, perl clocks in at 84557752.
This makes Python a contender where it previously wasn't because of closure size concerns.

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 17, 2019

I really like having a minimal python for these and other noninteractive, script-style tasks.

Having it explicitly named python3Minimal avoids people accidentially using the "wrong python version", as it was the case before #19309.

We probably want to document the existence of it once some scripts start using it, but I'd be fine with merging this as it is.

@flokli
flokli approved these changes Aug 17, 2019
Copy link
Member

left a comment

the pname has to be different, especially when python3Minimal is in the top-level.

@flokli flokli force-pushed the adisbladis:python-minimal branch from 708bf29 to 7874afc Aug 18, 2019
@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2019

I did some more yakshaving, by adding knobs to strip the python-config things, IDLE, tkinter, tests, and all the compiled bytecode (which we can remove, but not the .py files).
All these changes didn't trigger a rebuild of the existing python3 package.

The python3Minimal derivation got down to 23MiB:

30M     /nix/store/iykxb0bmfjmi7s53kfg6pjbfpd8jmza6-glibc-2.27
280K    /nix/store/1nrimml11nlg7spkrq0ki1337cjc4222-expat-2.2.7
64K     /nix/store/8pivgdaciz2add2wv7ff13gbz0rijs3f-libffi-3.2.1
88K     /nix/store/g8al6vdf21mgfi5774qx6r6cil1j7zr8-bzip2-1.0.6.0.1
408K    /nix/store/ji9kpgmbddkv2l2szzjfqinhja2322k0-xz-5.2.4
152K    /nix/store/w3wqang215is14xqajycbxmd52b44qkw-zlib-1.2.11
1.4M    /nix/store/xfghy8ixrhz3kyy6p724iv3cxji088dx-bash-4.4-p23
23M     /nix/store/bcjr7rn1zqmji5igff3zsk8b0zxwh6vp-python3-minimal-3.7.4
1.1M    /nix/store/1jk0fd4jdg8k20dwwrqbrm7zckbrbzck-python3.7-setuptools-41.0.1
8.7M    /nix/store/cnfvks0ydnkxf5vmrq9ay1wd0mvp0ix8-python3.7-pip-19.1.1
964K    /nix/store/f7050r2xdwxq44asbcdip2s8pk8lb2ip-python3-minimal-3.7.4-env
65M     total
@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2019

Of course, removing .pyc files might have large performance impacts, but that seems to be how small we can get (by not changing compiler flags etc.)

@adisbladis

This comment has been minimized.

Copy link
Member Author

commented Aug 18, 2019

The python3Minimal derivation got down to 23MiB:

A fantastic size reduction! 🎉
We're now at a size reduction of ~41%.

Of course, removing .pyc files might have large performance impacts, but that seems to be how small we can get (by not changing compiler flags etc.)

This will only affect startup performance, runtime performance is the same.

@danbst

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2019

@flokli Actually, I went ahead and rebuilt it using musl, and it builds well:

du -scbh $(nix-store -qR result/)
3.7M    /nix/store/x79gpgq6n92salzdnvay4hn3015qq2f8-musl-1.1.22
152K    /nix/store/5ra7bfg9v4p8d2wwv7amlcxzfjgw4smd-zlib-1.2.11
1.3M    /nix/store/b26c3f1py2wy0qbfinfidivdd8kzzqkv-bash-4.4-p23
3.6M    /nix/store/bakcmwwia178r95h5wanz9ri3icjwkqs-openssl-1.0.2s
273K    /nix/store/cg7j2g06jfg1r7c5gvjgqbwjvrmzwx0v-expat-2.2.7
61K     /nix/store/hd8iw5kij9a4w6r84d7g8szfamf5210r-libffi-3.2.1
179K    /nix/store/l4p2hq7ny2qlxxypalxaqlkq6hyngm5v-xz-5.2.4
82K     /nix/store/pbfiliv1yb2gl1kaclva04ylwwrhmw64-bzip2-1.0.6.0.1
2.8M    /nix/store/sdscb1h5fz4kvvgrqrxvmkw082phxh9x-gcc-7.4.0-lib
27M     /nix/store/jh9km68sg37ghylhcmsk2nyl58ivgycf-python3-3.7.4
39M     total

Here's the script:

with import <nixpkgs> {
  overlays = [
   (self: super: {
     python3Slim =
       let smallPackage = self.pkgsMusl.python3.override {
             self = self.python3Slim;
             readline = null;
             ncurses = null;
             gdbm = null;
             sqlite = null;
           };
       in smallPackage.overrideAttrs (old: {
         postFixup = ''
           find $out -type d | grep -e 'test' -e 'tkinter' -e 'idle' | xargs rm -rf
           find $out -type d -iname '__pycache__' | xargs rm -rf
         '';
       });
   })
  ];
};
python3Slim

Time to play on Alpine field!

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 18, 2019

Building with musl might be another way to shave off some bits, but it's not really used anywhere yet.

Anyway, we both proved the size reductions we could archieve seem to be pretty large, which IMHO should justify its inclusion in nixpkgs, as a separate derivation.

@jonringer

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Another great use case for this would distributing minimal docker images as well. I would love to use nix to package docker images as it's much more composable than docker files.

ncurses = null;
gdbm = null;
sqlite = null;
stripConfig = true;

This comment has been minimized.

Copy link
@Mic92

Mic92 Aug 19, 2019

Contributor

Does this prevent building c extensions or embed this version of python in some projects depending on python-config?

This comment has been minimized.

Copy link
@flokli

flokli Aug 19, 2019

Contributor

yes. python-config from the passed python would be used to get include and linker flags, and as it doesn't exist, you won't be able to use it.

This comment has been minimized.

Copy link
@flokli

flokli Aug 21, 2019

Contributor

Sidenote: It seems you only need python-config if you want to reuse that version of python in another project, but building a python3Minimal.withPackages calling some C bindings works just fine.

@Mic92

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Another great use case for this would distributing minimal docker images as well. I would love to use nix to package docker images as it's much more composable than docker files.

Would you use a python without tls support in a docker container? I would quickly run into problems when communicating with external services.

@FRidh

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

I suggest disabling pkgs/withPackages/buildEnv.

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Another great use case for this would distributing minimal docker images as well. I would love to use nix to package docker images as it's much more composable than docker files.

Would you use a python without tls support in a docker container? I would quickly run into problems when communicating with external services.

It really depends on your usecase, whether that version of python is doing the actual ssl communication, or if it's just a helper script.

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

I suggest disabling pkgs/withPackages/buildEnv.

I don't see much benefit / closure size reduction from that. Let's say, you want to use all the "minimizing aspects" of the main python, but there's some language bindings only pulling in a library already part of your closure, and you want to make use of that.

@danbst

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

I agree with @Mic92 that removing OpenSSL is too restrictive.

@danbst

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Building with musl might be another way to shave off some bits, but it's not really used anywhere yet.

According to https://github.com/NixOS/rfcs/blob/0d1cc2015543039a6ffc73d821457dd9210f56d1/rfcs/0046-platform-support-tiers.md#tier-2-3, Musl-based package are on Tier 2 support, so they are infinitely-small close to be built by Hydra (Tier 2-ε is on Hydra).

cc @7c6f434c : can we suppose that Tier 2 support packages are allowed to be built by Hydra?

@7c6f434c

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

I don't have opinions on Hydra resource mangement, I am just trying to codify the current state (and prepare the language for evolving the description)!

What is the likely use case where Nixpkgs is used, Python3 is used but glibc is not used at all? Does Python3 path itself get smaller just because of switching to musl (not because of a finer tuning of configuration)?

@danbst

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

@7c6f434c well yeah, you are right - we can't strip glibc that easy. But current implementation will cause package rebuilds in python3Minimal.withPackages (p: with p; [ cryptography ]) - we can rebuild Musl packages as well here.

What is the likely use case where Nixpkgs is used, Python3 is used but glibc is not used at all?

Some docker/container stuff. Personally, I don't have a glibc-free usecase.

@flokli does it makes sense to patch .withPackages in such way, that ${python3} dep is replaced with ${python3Minimal} one? In that case we can use binary cache for py packages (instead of rebuilding for python3Minimal)

@FRidh

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

@danbst extension modules will then point to python3 instead of minimal, so you don't gain anything, in fact, it takes more size

@danbst

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

@FRidh I was talking about something like pkgs.replaceDependency or "Shipping Security Updates"-style rewrites. It is complicated, but doable.

@FRidh

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

I don't see much benefit / closure size reduction from that. Let's say, you want to use all the "minimizing aspects" of the main python, but there's some language bindings only pulling in a library already part of your closure, and you want to make use of that.

There is no size benefit. It is to prevent having to deal with tickets where users start building packages using that Python.

@jonringer

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Would you use a python without tls support in a docker container? I would quickly run into problems when communicating with external services.

No, I wouldn't. But these changes would make it a lot easier to override and add back tls/openssl, than it would to do all the closure minification myself.

@7c6f434c

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

@danbst

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

@7c6f434c in that sense, python package isn't much different in glibc and musl variants. It's size is same, and image size for python+bash would be reduced only if both are musl-based.

OTOH, glibc is 30Mb, and musl is 6Mb. So we tradeoff +6Mb excess size for composite images and -30Mb size reduction for standalone python images.

But I agree now this idea is bad. I've tried to use pkgsMusl.python3.withPackages and it turns out, that something somewhere still brings regular pkgs.python3 into closure, which is another case for investigation, not related to this PR.

@7c6f434c

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2019

So we do agree exposing the arguments in a composable way as in that PR is something useful to have in nixpkgs?

In that case, any feedback to the code itself?

@FRidh

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

This builds Python without optional dependencies.

We can't just use python3.override, as things like
python3Minimal.withPackages would pass the wrong python derivation into
these modules.
@flokli flokli force-pushed the adisbladis:python-minimal branch from 7874afc to 99e6ca2 Aug 21, 2019
@flokli

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2019

alright - this PR doesn't use python3Minimal so far, so let's merge it.

@flokli flokli merged commit 65b1948 into NixOS:master Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.