Skip to content

Remove build-essential install #1383

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

Merged

Conversation

mathbunnyru
Copy link
Member

@mathbunnyru mathbunnyru commented Jun 23, 2021

PR description by @consideRatio

The build-essentials package was not a dependency to install other packages in the images of this repo, and it increased the minimal-notebook image size from 1.37GB to 1.55GB for example.

With the motivation that it is probably only relevant for users that depend on this image as a base image from other Dockerfiles where it is easy to install an additional apt package, this PR propose it is deleted. If this caused a breaking change for you, and you were using the image directly without using this image as a base image (via a FROM statement in a Dockerfile), please open an issue and describe that use case!

@consideRatio
Copy link
Member

What is the motivation for doing this?

@mathbunnyru
Copy link
Member Author

Reducing the sizes of images.
Why do we install something if it's not needed?

@consideRatio
Copy link
Member

What makes you know it isnt needed? I dont know its needed or not needed. Do you know why it was added in the first place?

@mathbunnyru
Copy link
Member Author

Before:

#7 naming to docker.io/jupyter/minimal-notebook:latest done
2614
#7 DONE 12.4s
2615
Built image size: 1.55GB

After:

#7 naming to docker.io/jupyter/minimal-notebook:latest done
2399
#7 DONE 10.6s
2400
Built image size: 1.37GB

@mathbunnyru
Copy link
Member Author

What makes you know it isnt needed? I dont know its needed or not needed. Do you know why it was added in the first place?

I didn't know that before trying this PR :)

The idea is that build-essential contains compiler / debugger and so on.
They are usually needed to build software.

But, usually python packages come as wheels, which are pre-built, so you don't have to build it on your machine.
It seems that this is true for the packages we're using. Because nothing broke.
So, for our images, it doesn't make sense to have build-essential preinstalled.

@mathbunnyru
Copy link
Member Author

Saving 180MB basically in every image makes me happy :)

Do you know why it was added in the first place?

No idea. It's in this repo from the beginning (2nd commit after init).
Probably some packages needed to be built from sources back then.
RIght now they seem to work without that.

@consideRatio
Copy link
Member

Hmmmmmmmm... Well this logic can make us delete a lot of packages as the image builda without them, but they can also have value to the end user.

I'm not confident about the values this package delivers, but if it is only needed for those using this image as a base image for their own dockerfile, then i think it is reasonable to delete it anyhow.

Do you know if this package can be reasonably used by people starting this container directly?

@mathbunnyru
Copy link
Member Author

Well this logic can make us delete a lot of packages as the image builda without them

I'm not sure about a lot, because we keep small things like vim-tiny to be able to easily use our images through bash.
And every case is kind of special.

Do you know if this package can be reasonably used by people starting this container directly?

I think the most commonly used packages come pre-built.
And I think, it's absolutely reasonable to ask our users to install build tools for the packages they need.
This way they can let ask the developer to build the package and upload it in a prebuilt way.

Also, we're only breaking people using latest version of our images, which is also OK, because latest is changing quite rapidly and updates are going to happen.

@consideRatio
Copy link
Member

I looked at the pangeo-docker-images if they have build-essentials, but they didn't. I'm leaning towards thinking this is the right call.

If someone would need it - I believe it would be needed from a Dockerfile using this image in a FROM statement - and if that is the case they can install it explicitly themselves very easily. It is a different matter if they would need it directly from starting the docker-stacks image directly. If this is wrong though, I think we should learn about the use case and re-consider at that point in time.

@mathbunnyru
Copy link
Member Author

Yes, I do think this will only break for users, which inherit from our images (using FROM)

@consideRatio
Copy link
Member

@mathbunnyru I added a PR description, this LGTM!

@mathbunnyru
Copy link
Member Author

Let's try this one out then. Hope it's gonna work fine :)

@mathbunnyru mathbunnyru merged commit 5211732 into jupyter:master Jun 23, 2021
Gehock added a commit to AaltoSciComp/jupyter-aalto-singleuser that referenced this pull request Sep 12, 2022
Installing R packages requires `make`. Having make and basic compilers
available is also useful for users. The package used to be installed
in the upstream minimal-notebook image, but it was removed in
jupyter/docker-stacks:4f7881bfb386f4a1522cb27668f5f68586f7110a.

See jupyter/docker-stacks#1383 for more info.
Gehock added a commit to AaltoSciComp/jupyter-aalto-singleuser that referenced this pull request Jul 13, 2023
Installing R packages requires `make`. Having make and basic compilers
available is also useful for users. The package used to be installed
in the upstream minimal-notebook image, but it was removed in
jupyter/docker-stacks:4f7881bfb386f4a1522cb27668f5f68586f7110a.

See jupyter/docker-stacks#1383 for more info.
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.

2 participants