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

libnghttp2 nghttp2 1.62.1 #171634

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

libnghttp2 nghttp2 1.62.1 #171634

wants to merge 2 commits into from

Conversation

bevanjkay
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@bevanjkay bevanjkay changed the title libnghttp2 1.62.0 libnghttp2 nghttp2 1.62.0 May 14, 2024
@github-actions github-actions bot added CI-linux-self-hosted Build on Linux self-hosted runner long build Needs CI-long-timeout labels May 14, 2024
@bevanjkay bevanjkay force-pushed the libnghttp2-1.14.0 branch 2 times, most recently from cff6848 to 7fbc4a3 Compare May 14, 2024 03:32
@bevanjkay bevanjkay added CI-linux-self-hosted-deps Test dependents on Linux self-hosted runner and removed CI-linux-self-hosted Build on Linux self-hosted runner labels May 28, 2024
@bevanjkay bevanjkay changed the title libnghttp2 nghttp2 1.62.0 libnghttp2 nghttp2 1.62.1 May 28, 2024
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@bevanjkay bevanjkay removed the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@bevanjkay bevanjkay removed the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@bevanjkay bevanjkay added CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. and removed CI-linux-self-hosted Build on Linux self-hosted runner labels May 28, 2024
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@bevanjkay bevanjkay removed the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@bevanjkay bevanjkay removed the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@bevanjkay bevanjkay removed the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@carlocab carlocab added long build Needs CI-long-timeout CI-no-bottle-cache Disable bottle cache long dependent tests Set a long timeout for dependent testing and removed long build Needs CI-long-timeout labels May 28, 2024
@bevanjkay
Copy link
Member Author

bevanjkay commented May 28, 2024

Upstream issue for GCC11 build failure on Linux - nghttp2/nghttp2#2194

I'm not sure that I have the correct combination of fails_with.

Missing runtime dependency in latest linux CI run

 /home/linuxbrew/.linuxbrew/Cellar/nghttp2/1.62.1/bin/nghttp: /lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.32' not found (required by /home/linuxbrew/.linuxbrew/Cellar/nghttp2/1.62.1/bin/nghttp)


on_linux do
fails_with gcc: "5"
depends_on "gcc@13"
Copy link
Member

Choose a reason for hiding this comment

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

I think the test failure is because we're mixing different compiler versions between libnghttp2 and nghttp2.

But either of them failing with the host compiler is a huge problem. CC @Homebrew/core for thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

Adding a dependency with libnghttp2 would have massive effects on the dependency tree of homebrew-core as a whole so wouldn't really be acceptable without discussion.

There may also concern even for older Clang. If libnghttp2 is broken on older Clang, we must drop support for macOS 10.11 in Homebrew/brew.

However, it seems like libnghttp2 might be ok though which is interesting. Are we only hitting the issue with the nghttp2 executables? That's less of an issue and generally should work OK even though libnghttp2 use an older GCC.

Copy link
Member

Choose a reason for hiding this comment

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

That's less of an issue and generally should work OK even though libnghttp2 use an older GCC.

I'd have thought we'd be ok using a newer GCC only for nghttp2 and not libnghttp2, but the test block disagrees, unfortunately.

Copy link
Member

@cho-m cho-m May 28, 2024

Choose a reason for hiding this comment

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

That's less of an issue and generally should work OK even though libnghttp2 use an older GCC.

I'd have thought we'd be ok using a newer GCC only for nghttp2 and not libnghttp2, but the test block disagrees, unfortunately.

Can try adding Formula["gcc@13"].opt_lib/"gcc/13" to RPATH. Not sure if it will precede #{HOMEBREW_PREFIX}/lib/gcc/current (otherwise may have some extra missing linkage reports whenever brew gcc is installed on CI).


EDIT: Though we may want to wait for upstream response if they will support GCC 11 given it is commonly used in LTS distros.

Copy link
Member

Choose a reason for hiding this comment

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

Adding a dependency with libnghttp2 would have massive effects on the dependency tree of homebrew-core as a whole so wouldn't really be acceptable without discussion.

Is this just on Linux?

If libnghttp2 is broken on older Clang, we must drop support for macOS 10.11 in Homebrew/brew.

Why is that, can you elaborate?

Copy link
Member

@Bo98 Bo98 May 28, 2024

Choose a reason for hiding this comment

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

I'd have thought we'd be ok using a newer GCC only for nghttp2 and not libnghttp2, but the test block disagrees, unfortunately.

libnghttp2 shouldn't be using C++:

Compiling libnghttp2 C source code requires a C99 compiler. gcc 4.8 is known to be adequate. In order to compile the C++ source code, C++20 compliant compiler is required. At least g++ >= 1.12 and clang++ >= 1.15 are known to work.

Is this just on Linux?
Why is that, can you elaborate?

Because it's a dependency for curl, and we require brewed curl on older macOS.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, compiling nghttp2 (and not libnghttp2) with Homebrew gcc should just work. It'll fail the Linux-only GCC dependency audit, though.

Copy link
Member

Choose a reason for hiding this comment

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

Because it's a dependency for curl, and we require brewed curl on older macOS.

Gotcha, thanks. Yeh, I think it'd either be: drop it as a dependency, drop it as a dependency on old macOS versions (dangerous with no CI coverage) or drop those macOS versions.

It'll fail the Linux-only GCC dependency audit, though.

Seems like a good case to opt-out of.

Copy link
Member

Choose a reason for hiding this comment

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

Let's see if using gcc does indeed work before we add an audit opt-out... dbded26

Copy link
Member

Choose a reason for hiding this comment

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

Ok, that works. Homebrew/brew#17384

sha256 "c4bcf5cf73d5305fc479206676027533bb06d4ff2840eb672f6265ba3239031e"
end
on_macos do
depends_on "llvm" => :build if DevelopmentTools.clang_build_version <= 1400
Copy link
Member

Choose a reason for hiding this comment

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

What macOs version does this correspond to?


on_linux do
fails_with gcc: "5"
depends_on "gcc@13"
Copy link
Member

Choose a reason for hiding this comment

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

Adding a dependency with libnghttp2 would have massive effects on the dependency tree of homebrew-core as a whole so wouldn't really be acceptable without discussion.

Is this just on Linux?

If libnghttp2 is broken on older Clang, we must drop support for macOS 10.11 in Homebrew/brew.

Why is that, can you elaborate?

nghttp2: use C++20

nghttp2: remove linux patch
@github-actions github-actions bot added the CI-linux-self-hosted Build on Linux self-hosted runner label May 28, 2024
@carlocab carlocab removed CI-linux-self-hosted Build on Linux self-hosted runner long build Needs CI-long-timeout labels May 28, 2024
carlocab added a commit to Homebrew/brew that referenced this pull request May 28, 2024
Needed for Homebrew/homebrew-core#171634 due to nghttp2/nghttp2#2194.

I've avoided adding an allowlist for now because I really don't want to
see this list grow.
@carlocab carlocab removed the CI-no-bottle-cache Disable bottle cache label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-linux-self-hosted-deps Test dependents on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long dependent tests Set a long timeout for dependent testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants