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

Update LINUX_CI_OS_VERSION to Ubuntu 22.04 #13733

Merged
merged 4 commits into from
Sep 6, 2022

Conversation

sjackman
Copy link
Member

@sjackman sjackman commented Aug 22, 2022

  • Change LINUX_CI_OS_VERSION from Ubuntu 16.04 to Ubuntu 22.04
  • Change LINUX_GLIBC_CI_VERSION from 2.23 to 2.35
  • Change LINUX_GCC_CI_VERSION from 5.0 to 11.0
  • Change LINUX_PREFERRED_GCC_FORMULA from gcc@5 to gcc@11
  • Build the Docker image ghcr.io/homebrew/ubuntu22.04:master

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

@sjackman sjackman self-assigned this Aug 22, 2022
@BrewTestBot
Copy link
Member

Review period will end on 2022-08-23 at 06:15:58 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Aug 22, 2022
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looks good so far! Any docs that need updated?

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Aug 23, 2022
@BrewTestBot
Copy link
Member

Review period ended.

@MikeMcQuaid MikeMcQuaid marked this pull request as ready for review August 23, 2022 12:00
@MikeMcQuaid
Copy link
Member

Wanting this to land for 3.6.0.

@sjackman sjackman changed the title Update CI_OS_VERSION CI_GLIBC_VERSION preferred_gcc Update LINUX_CI_OS_VERSION and related constants Aug 25, 2022
@sjackman
Copy link
Member Author

sjackman commented Aug 25, 2022

https://github.com/Homebrew/brew/runs/8023876296?check_suite_focus=true#step:5:8
After merging PR #13577 this PR now fails with Error: maximum tree depth (10) exceeded calculating gcc@11 dependency tree!.

@sjackman
Copy link
Member Author

sjackman commented Aug 25, 2022

Bumping LINUX_GLIBC_CI_VERSION causes the problem. Bumping LINUX_GLIBC_CI_VERSION is fine.

Bump LINUX_GLIBC_CI_VERSION

-  LINUX_GLIBC_CI_VERSION = "2.23"
+  LINUX_GLIBC_CI_VERSION = "2.35"
$ brew install hello
Error: maximum tree depth (10) exceeded calculating gcc@5 dependency tree!

Bump LINUX_GCC_CI_VERSION

-  LINUX_GCC_CI_VERSION = "5.0"
+  LINUX_GCC_CI_VERSION = "11.0"
$ brew install hello

==> Installing dependencies for hello: gmp, isl@0.18, mpfr, libmpc, zlib, binutils, linux-headers@5.15, glibc and gcc@5

@MikeMcQuaid
Copy link
Member

After merging PR #13577 this PR now fails with Error: maximum tree depth (10) exceeded calculating gcc@11 dependency tree!.

@sjackman Going to need you to dig in a bit harder on these sorts of issues, please. I don't even really have a Linux machine to test these on. If the value for that variable needs changed: consider changing it as part of this PR.

@sjackman
Copy link
Member Author

Going to need you to dig in a bit harder on these sorts of issues, please. I don't even really have a Linux machine to test these on. If the value for that variable needs changed: consider changing it as part of this PR.

https://docs.docker.com/desktop/install/mac-install/
I do all of my testing on Linux using Docker Desktop for macOS and docker run -it --rm homebrew/ubuntu16.04.

The circular dependencies in Homebrew/core were resolved by (I believe)…

Thank you @Bo98 and @danielnachun!

@MikeMcQuaid
Copy link
Member

Going to need you to dig in a bit harder on these sorts of issues, please. I don't even really have a Linux machine to test these on. If the value for that variable needs changed: consider changing it as part of this PR.

https://docs.docker.com/desktop/install/mac-install/ I do all of my testing on Linux using Docker Desktop for macOS and docker run -it --rm homebrew/ubuntu16.04.

@sjackman This was not a request for you to tell me how to obtain a Linux machine (which runs incredibly slowly/buggily on all my ARM macOS machines) but instead a request for you as a maintainer to be able to attempt to resolve issues like that you reported yourself if they are blocking your PRs in future, thanks ❤️

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Blocked on:

  • figuring out what to do with homebrew/ubuntu16.04:master vs. homebrew/ubuntu16.04:latest
  • legitimate brew tests failures

Library/Homebrew/formula_auditor.rb Outdated Show resolved Hide resolved
Library/Homebrew/os.rb Outdated Show resolved Hide resolved
@sjackman
Copy link
Member Author

sjackman commented Aug 31, 2022

Commit 60c3b18: Use ubuntu-22.04 for the CI tests rather than ubuntu-latest, which is currently ubuntu-20.04.
See https://github.com/actions/runner-images#available-images

@Bo98
Copy link
Member

Bo98 commented Sep 3, 2022

https://ubuntu.com/about/release-cycle Ubuntu 16.04 is not EOL. It's Extended Security Maintenance (ESM) until 2026. Still a valid question though.

Does ESM apply to the public Docker images?

https://hub.docker.com/_/ubuntu The web page says that everything from Ubuntu 14.04 LTS and up is currently supported.

The datestamps in the tags there are 2019 and 2021 and ESM usually requires login and attaching a device with a token (ua attach ...) so was wondering how that exactly worked, particularly with the device limit.

Seems quite vague. Their page on Docker images suggests 10 year support requires contacting them: https://ubuntu.com/security/docker-images

@sjackman
Copy link
Member Author

sjackman commented Sep 4, 2022

I believe this PR is ready to be merged and Homebrew 3.6.0 tagged!

@MikeMcQuaid
Copy link
Member

I can push that error message image shown above to Docker Hub and GHCR once this PR has been merged and master CI has built and pushed the new homebrew/ubuntu22.04:master image.

@sjackman Could this be made part of this PR instead? If not, presumably this needs to happen before 3.6.0 is merged rather than after? Ideally I think it'd be done in a PR merged before this one.

@sjackman
Copy link
Member Author

sjackman commented Sep 5, 2022

I can push that error message image shown above to Docker Hub and GHCR once this PR has been merged and master CI has built and pushed the new homebrew/ubuntu22.04:master image.

@sjackman Could this be made part of this PR instead? If not, presumably this needs to happen before 3.6.0 is merged rather than after? Ideally I think it'd be done in a PR merged before this one.

#13733 (comment)
I was planning on pushing this retired-message Docker image from the interactive command line using docker login && docker push homebrew/ubuntu16.04:master after we'd successfully built the Docker image homebrew/ubuntu22.04:master by merging this PR, and merged PR Homebrew/homebrew-core#108590, and successfully used it to build at least one bottle.

Tasks

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

I was planning on pushing this retired-message Docker image from the interactive command line using docker login && docker push homebrew/ubuntu16.04:master after we'd successfully built the Docker image homebrew/ubuntu22.04:master by merging this PR, and merged PR Homebrew/homebrew-core#108590, and successfully used it to build at least one bottle.

@sjackman Could this be done by a/the GitHub Action in a new PR instead of by you manually through the CLI? I think that'd be preferable.

This does not need to block this PR but would be good to be open before the tag.


FYI that we've landed some fairly major other PRs today that I want to be merged for ~24h before we tag 3.6.0 to surface any issues.

@MikeMcQuaid
Copy link
Member

This branch has conflicts that must be resolved

Note that these need to be addressed, sorry!

FYI that we've landed some fairly major other PRs today that I want to be merged for ~24h before we tag 3.6.0 to surface any issues.

Sorry, the specific question here: do we need to tag ASAP after merging this and/or Homebrew/homebrew-core#108590?

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks @sjackman!

@sjackman
Copy link
Member Author

sjackman commented Sep 5, 2022

Sorry, the specific question here: do we need to tag ASAP after merging this and/or Homebrew/homebrew-core#108590?

After merging PR Homebrew/homebrew-core#108590, we need to tag Homebrew 3.6.0 ASAP before merging any bottle PRs in Homebrew/core.

@sjackman sjackman force-pushed the sj/ubuntu22.04 branch 2 times, most recently from 31e8945 to 301ae8f Compare September 5, 2022 15:45
@MikeMcQuaid
Copy link
Member

MikeMcQuaid commented Sep 5, 2022

Sorry, the specific question here: do we need to tag ASAP after merging this and/or Homebrew/homebrew-core#108590?

After merging PR Homebrew/homebrew-core#108590, we need to tag Homebrew 3.6.0 ASAP before merging any bottle PRs in Homebrew/core.

FYI that we've landed some fairly major other PRs today that I want to be merged for ~24h before we tag 3.6.0 to surface any issues.

Thanks. What about merging this PR? Could it be done 1/12/24 hours before Homebrew/homebrew-core#108590?

My most recent commit 301ae8f CI tests.yml: Deploy the retired Docker image will retire the image homebrew/ubuntu16.04:master, which is currently used by Homebrew/core to build bottles. Merging this PR as is will make it impossible to build bottles until PR Homebrew/homebrew-core#108590 is merged.

Other than that retired image commit, any time frame is fine for merging this PR. Merging this PR and giving it ~24h to burn in before tagging 3.6.0 is a good idea.

I'll separate that commit from this PR and push it to its own PR.

@carlocab
Copy link
Member

carlocab commented Sep 5, 2022

Sorry, the specific question here: do we need to tag ASAP after merging this and/or Homebrew/homebrew-core#108590?

After merging PR Homebrew/homebrew-core#108590, we need to tag Homebrew 3.6.0 ASAP before merging any bottle PRs in Homebrew/core.

It seems like we should tag 3.6.0 before merging Homebrew/homebrew-core#108590, but the merge should be done quickly after the tag in order to avoid having to install glibc and gcc@11 in CI.

Edit: But, having seen the checklist at #13733 (comment), the reverse order makes sense too.

@sjackman
Copy link
Member Author

sjackman commented Sep 5, 2022

I'll separate that commit from this PR and push it to its own PR.

Comment on lines 9 to 16
it "is correctly detected" do
expect(UnpackStrategy.detect(path)).to(be_a(described_class).or(be_a(UnpackStrategy::Tar)))
end
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't UnpackStrategy::detect work here?

Copy link
Member Author

@sjackman sjackman Sep 5, 2022

Choose a reason for hiding this comment

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

def self.can_extract?(path)
return true if path.magic_number.match?(/\A.{257}ustar/n)
return false unless [Bzip2, Gzip, Lzip, Xz, Zstd].any? { |s| s.can_extract?(path) }
# Check if `tar` can list the contents, then it can also extract it.
stdout, _, status = system_command("tar", args: ["--list", "--file", path], print_stderr: false)
status.success? && !stdout.empty?
end

UnpackStrategy.detect(path) for a .tar.XXX file returns either UnpackStrategy::Tar if the host's tar is able to extract that compressed file or UnpackStrategy::XXX otherwise (such as UnpackStrategy::Zstd). On macOS UnpackStrategy.detect("container.tar.zst") returns UnpackStrategy::Zstd, and on ubuntu-22.04 it returns UnpackStrategy::Tar, because the host's version of tar is recent enough and zstd is installed.

Copy link
Member

Choose a reason for hiding this comment

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

This seems worth documenting in a comment, if not adjusted in shared_examples.rb.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@sjackman
Copy link
Member Author

sjackman commented Sep 6, 2022

$ brew tests --only=cask/audit

Failed examples:
[160](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:161)

[161](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:162)
rspec ./test/cask/audit_spec.rb[1:3:1:2:1] # Cask::Audit#run! required stanzas when missing sha256 is expected to fail with /sha256 stanza is required/
[162](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:163)
rspec ./test/cask/audit_spec.rb[1:3:1:4:1] # Cask::Audit#run! required stanzas when missing name is expected to fail with /name stanza is required/
[163](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:164)
rspec ./test/cask/audit_spec.rb[1:3:1:5:1] # Cask::Audit#run! required stanzas when missing homepage is expected to fail with /homepage stanza is required/
[164](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:165)
rspec ./test/cask/audit_spec.rb[1:3:1:3:1] # Cask::Audit#run! required stanzas when missing url is expected to fail with /url stanza is required/
[165](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:166)
rspec ./test/cask/audit_spec.rb:1122 # Cask::Audit#run! checking verified when there is no homepage is expected to fail with /a homepage stanza is required/
[166](https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:167)

https://github.com/Homebrew/brew/runs/8196408355?check_suite_focus=true#step:14:160

I'm going to need some help troubleshooting these tests that are failing now.

@carlocab
Copy link
Member

carlocab commented Sep 6, 2022

Failures likely fixed by #13813.

- Change `LINUX_CI_OS_VERSION` from `Ubuntu 16.04` to `Ubuntu 22.04`
- Change `LINUX_GLIBC_CI_VERSION` from `2.23` to `2.35`
- Change `LINUX_GCC_CI_VERSION` from `5.0` to `11.0`
- Change `LINUX_PREFERRED_GCC_FORMULA` from `gcc@5` to `gcc@11`
- Build the Docker image `ghcr.io/homebrew/ubuntu22.04:master`
Use ubuntu-22.04 for the CI tests rather than ubuntu-latest,
which is currently ubuntu-20.04.
@MikeMcQuaid MikeMcQuaid merged commit 0334658 into Homebrew:master Sep 6, 2022
@sjackman sjackman deleted the sj/ubuntu22.04 branch September 6, 2022 16:11
@github-actions github-actions bot added the outdated PR was locked due to age label Oct 7, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants