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

optimization debian package manager tweaks #158

Closed
wants to merge 1 commit into from

Conversation

Rajpratik71
Copy link

By default, Ubuntu or Debian based "apt" or "apt-get" system installs recommended but not suggested packages .

By passing "--no-install-recommends" option, the user lets apt-get know not to consider recommended packages as a dependency to install.

This results in smaller downloads and installation of packages .

Refer to blog at Ubuntu Blog .

Signed-off-by: Pratik Raj [email protected]

@Rajpratik71
Copy link
Author

@fud @akyrtzi @lgaches requesting review

@akyrtzi
Copy link

akyrtzi commented Mar 9, 2020

\cc @shahmishal

@shahmishal
Copy link
Member

@swift-ci test

1 similar comment
@shahmishal
Copy link
Member

@swift-ci test

Copy link
Contributor

@ianpartridge ianpartridge left a comment

Choose a reason for hiding this comment

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

In principle, great idea!

How many unnecessary packages does this remove, and what effect does it have on final image size?

@@ -3,7 +3,7 @@ LABEL maintainer="Swift Infrastructure <[email protected]>"
LABEL Description="Docker Container for the Swift programming language"

RUN export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true && apt-get -q update && \
apt-get -q install -y \
apt-get --no-install-recommends install -y apt-utils ca-certificates \
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put the apt-utils and ca-certificates on their own line, to match the rest of the packages.

@tomerd
Copy link
Contributor

tomerd commented May 12, 2020

hi @Rajpratik71, would you like to work with us to bring this over the finish line?

@Rajpratik71 Rajpratik71 force-pushed the enhancement/apt branch 2 times, most recently from 9b9dd10 to 8504a69 Compare June 13, 2020 03:23
By default, Ubuntu or Debian based "apt" or "apt-get" system installs recommended but not suggested packages .

By passing "--no-install-recommends" option, the user lets apt-get know not to consider recommended packages as a dependency to install.

This results in smaller downloads and installation of packages .

Refer to blog at [Ubuntu Blog](https://ubuntu.com/blog/we-reduced-our-docker-images-by-60-with-no-install-recommends) .

Also , added packages apt-utils ca-certificates

Because build is

1.  Slow because "apt-utils" not installed

2. to avoid build to exits with error without having certificate in wget , curl or even git clone

Signed-off-by: Pratik Raj <[email protected]>
@Rajpratik71
Copy link
Author

hi @Rajpratik71, would you like to work with us to bring this over the finish line?

Hi , rebased to upstream and also done changes as requested by @ianpartridge

@Rajpratik71 Rajpratik71 requested a review from ianpartridge June 13, 2020 03:28
@shahmishal
Copy link
Member

@swift-ci test

@tomerd
Copy link
Contributor

tomerd commented Jun 16, 2020

thanks so much @Rajpratik71, can you remind me why we needed to add the apt-utils and ca-certificates packages as part of this PR?

additionally, we recently added support for Ubuntu 20.04 (5.2 and nightlies) which I think can use the same optimization?

@Rajpratik71
Copy link
Author

thanks so much @Rajpratik71, can you remind me why we needed to add the apt-utils and ca-certificates packages as part of this PR?

additionally, we recently added support for Ubuntu 20.04 (5.2 and nightlies) which I think can use the same optimization?

added packages apt-utils ca-certificates

Because build is

  1. Slow because "apt-utils" not installed

  2. to avoid build to exits with error without having certificate in wget , curl or even git clone

@tomerd
Copy link
Contributor

tomerd commented Jun 17, 2020

hi @Rajpratik71

  1. for my education, how did it work before this change? were they part of recommend packages?

  2. could you please apply the same logic/patch to the recently added 20.04 images

thanks a ton for your contribution!

@Rajpratik71
Copy link
Author

Rajpratik71 commented Jun 17, 2020

hi @Rajpratik71

  1. for my education, how did it work before this change? were they part of recommend packages?
  2. could you please apply the same logic/patch to the recently added 20.04 images

thanks a ton for your contribution!

Hi @tomerd ,

These packages are not there before . e.g If any debian based system doesn't have "apt-utils" installed , then in that case any package installation takes time while configuring and gives suggestion to install "apt-utils" . that is why i put that here .

And I put "ca-certificates" to avoid build to exits with error without having certificate in wget , curl or even git clone.

And yes , for 20.04 images will do that

@tomerd
Copy link
Contributor

tomerd commented Jun 29, 2020

@swift-ci Please test

@shahmishal shahmishal closed this Oct 6, 2020
@shahmishal
Copy link
Member

The Swift project moved the default branch to main and deleted master branch, so GitHub automatically closed the PR. Please re-create the pull request with main branch.

More detail about the branch update - https://forums.swift.org/t/updating-branch-names/40412

@Rajpratik71
Copy link
Author

The Swift project moved the default branch to main and deleted master branch, so GitHub automatically closed the PR. Please re-create the pull request with main branch.

More detail about the branch update - https://forums.swift.org/t/updating-branch-names/40412

created fresh pull to main @ #203

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.

5 participants