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

build: standardize apt-get usage, remove blackfire from apt sources, for #6235, for #6078 #6236

Merged
merged 4 commits into from
Jun 2, 2024

Conversation

stasadev
Copy link
Member

@stasadev stasadev commented May 23, 2024

The Issue

apt-get update can fail in the user's custom Dockerfiles.

I thought we could search for apt-get update using a regex and automatically replace it in the resulting Dockerfile, but that seems like overkill because people can write it with different variations, like:

  • apt-get update
  • apt-get update -y
  • apt-get update -qq
  • apt-get update -qq >/dev/null

And the same with apt, so it's probably impossible to write a good regex to cover all cases and not break anything.

How This PR Solves The Issue

Replaces apt-get update with (apt-get update || true) in the docs.

Uses apt-get instead of apt everywhere.

Removes /etc/apt/sources.list.d/blackfire.list as we pin the blackfire package to a specific version and don't need it in apt-get update:

Manual Testing Instructions

  1. ddev start
  2. ddev ssh
  3. sudo apt-get update | grep blackfire should give an empty result

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@stasadev stasadev requested a review from a team as a code owner May 23, 2024 09:29
@rfay
Copy link
Member

rfay commented May 23, 2024

IMO we should remove the /etc/apt/sources.list.d/blackfire.list after the installation in the ddev-webserver Dockerfile, since Blackfire is such low priority and since it so often causes trouble. That can be done here or in a later PR.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Thanks for this! Let's just use apt-get update and apt-get install and apt-get upgrade` in all the places, just for consistency. It also prevents the "no consistent api" message or whatever it is.

@stasadev stasadev requested a review from a team as a code owner May 23, 2024 20:45
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label May 23, 2024
Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Thanks, looks perfect.

@stasadev stasadev changed the title docs: update custom Dockerfile build instruction, for #6235 docs: update custom Dockerfile build instruction, for #6235, for #6078 May 23, 2024
@stasadev stasadev force-pushed the 20240523_stasadev_apt_get_update_or_true branch from 8327dd8 to 16ee902 Compare May 23, 2024 20:59
@stasadev stasadev changed the title docs: update custom Dockerfile build instruction, for #6235, for #6078 build: standardize apt-get usage, remove blackfire from apt sources, for #6235, for #6078 May 23, 2024
Copy link

github-actions bot commented May 23, 2024

@stasadev stasadev force-pushed the 20240523_stasadev_apt_get_update_or_true branch from 16ee902 to 00f4fd5 Compare May 24, 2024 17:54
@rfay rfay merged commit e083d34 into ddev:master Jun 2, 2024
22 checks passed
@stasadev stasadev deleted the 20240523_stasadev_apt_get_update_or_true branch June 2, 2024 18:16
jonesrussell pushed a commit to jonesrussell/ddev that referenced this pull request Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants