Skip to content

Disable HTTP/2 support again - #1010

Open
TheOneRing wants to merge 1 commit into
mainfrom
work/http2
Open

Disable HTTP/2 support again#1010
TheOneRing wants to merge 1 commit into
mainfrom
work/http2

Conversation

@TheOneRing

Copy link
Copy Markdown
Contributor

Release 3.0 showed that Qt is still not ready

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR re-disables HTTP/2 by default due to observed Qt issues in the 3.0 release, while keeping a controlled way to opt back into HTTP/2 for testing via build-time and runtime switches.

Changes:

  • Adds a top-level CMake option WITH_HTTP2 (default OFF) and feature info entry to represent HTTP/2 support status.
  • Passes the build-time HTTP/2 default into accessmanager.cpp via a per-source compile definition.
  • Updates AccessManager::createRequest to set QNetworkRequest::Http2AllowedAttribute based on build-time default and an environment variable override (OPENCLOUD_USE_HTTP2).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
CMakeLists.txt Introduces WITH_HTTP2 option (default off) and documents known Qt bugs / override env var.
src/libsync/CMakeLists.txt Defines WITH_HTTP2 compile-time macro for accessmanager.cpp based on the CMake option.
src/libsync/accessmanager.cpp Centralizes per-request HTTP/2 enablement based on build default and OPENCLOUD_USE_HTTP2.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/libsync/accessmanager.cpp Outdated
Release 3.0 showed that Qt is still not ready

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libsync/accessmanager.cpp:96

  • The HTTP/2 gating condition is inverted for the common case where the attribute is unset. attribute(...).toBool() returns false for an invalid/unset QVariant, so this block won’t run and Qt’s default (HTTP/2 allowed) remains in effect even when useHttp2 is false. If the intention is “preserve an explicit false set by callers, otherwise apply the global build/env default”, check validity first and only skip when explicitly false.
    // Qt's default value is true, if we explicitly set it to false, we don't want to use HTTP/2
    if (newRequest.attribute(QNetworkRequest::Http2AllowedAttribute).toBool()) {
        newRequest.setAttribute(QNetworkRequest::Http2AllowedAttribute, useHttp2);
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants