Merge ~lvoytek/ubuntu/+source/requests:remove-charset-normalizer into ubuntu/+source/requests:ubuntu/devel

[Lena Voytek]

Added charset-normalizer as an alternative dependency to chardet so it won't be included with dh_python3 / ${python3:Depends}. Since charset-normalizer is in universe it was blocking the new version of requests in main. Luckily it is just used as a backup for chardet which is in main, so making it an alternative doesn't break request's functionality.

PPA: ppa:lvoytek/requests-remove-charset-normalizer

Tested with:

# lxc launch images:ubuntu/kinetic test-requests
# lxc exec test-requests bash
# apt update && apt dist-upgrade -y
# apt install python3-requests
# apt install software-properties-common
# add-apt-repository ppa:lvoytek/requests-remove-charset-normalizer
# apt update && apt upgrade -y

[Christian Ehrhardt (paelzer)]

Hi,
an inline suggestion and an idea at an - hopefully easier to maintain, maybe even upstreamable - alternative:

AFAIU this is added by dh_python as it is referenced in the code.

Have you tried adding them explicitly with an or (without any code change).
The assumption (needs to be tested) would be that by being listed already they'd not be re-added.
And by being an alternate dependency starting with the one in universe it should be ok.

like d/control with the only change being:
Depends: python3-charset | python3-charset-normalizer

[Lena Voytek (lvoytek)]

The ppa finally finished building and I was able to confirm that your suggestion works properly. Thanks @paelzer!

[Bryce Harrington (bryce)]

I notice the changelog mentions "d/p/remove-charset-normalizer-dependency.patch" but there are no patches; should this rather be "d/control: " or was there supposed to be a patch?

Also, one thing I'm unclear on is the changelog description says it's removing python3-charset-normalizer, yet the diff actually is adding it as an alternative dependency. Not that it's wrong technically, just the description of the change and the actual change seem inconsistent?

review: Needs Fixing

[Lena Voytek (lvoytek)]

Ah thanks for catching that, I completely forgot to change the file name. I'll update the description too. Both were just leftovers from my previous iteration of the fix.

[Bryce Harrington (bryce)]

Ok, looks good. Do you want to wait for Christian to get back, to get his approval as well?
If not, and you need sponsorship I can handle that today, just let me know.

review: Approve

[Christian Ehrhardt (paelzer)]

Do you already have a PPA or a buildlog of a local build that shows the "no more auto-added" dependencies?

[Christian Ehrhardt (paelzer)]

This is not what I meant :-)
The current proposal is to add this to the build-depends which should have no direct influence onto the final dependencies of the binary package.

The package in main is "python3-requests"

And after build with this change I still see:
...
python3-requests_2.27.1+dfsg-1ubuntu2~kineticppa1_all.deb
...
Depends: python3-certifi, python3-charset-normalizer (>= 2.0.0), python3-idna, python3-urllib3 (>= 1.21.1), python3:any, ca-certificates, python3-chardet (>= 3.0.2)

There is no change, the package still depends on python3-charset-normalizer.
I tried a few more things in the place where I had hoped it would work.

We know that it comes from dh_python into ${python3:Depends} and you can list a dependency only once. So the idea was to move down the substvar and list it before as an alternative.

Something like:
@@ -28,12 +28,13 @@ Package: python3-requests
 Architecture: all
 Multi-Arch: foreign
 Depends:
- ${misc:Depends},
- ${python3:Depends},
  ca-certificates,
  python3-certifi,
  python3-chardet (>= 3.0.2),
  python3-urllib3 (>= 1.21.1),
+ python3-chardet (>= 3.0.2) | python3-charset-normalizer (>= 2.0.0),
+ ${python3:Depends},
+ ${misc:Depends},
 Suggests:
  python3-cryptography,
  python3-idna (>= 2.5),

But even that didn't work, so I guess dh_python does not want to be tricked here.
It really means you should go back to your initial approach of disabling it for real :-/

But out of this - the fact to check after build are:
- the final binary python3-requests shall no more depend on python3-charset-normalizer
- python3-requests shall still work fine and not suffer from only having one of those packages

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: bryce, lvoytek
Uploaders: bryce
MP auto-approved

review: Approve

[Lena Voytek (lvoytek)]

re-added the patch file, confirmed the dependency is gone using
ppa:lvoytek/requests-remove-charset-normalizer

# lxc launch images:ubuntu/kinetic test-requests
# lxc exec test-requests bash
# apt update && apt dist-upgrade -y
# apt install python3-requests
# apt install software-properties-common
# add-apt-repository ppa:lvoytek/requests-remove-charset-normalizer
# apt update && apt upgrade -y
# apt depends python3-requests
python3-requests
  Depends: python3-certifi
  Depends: python3-chardet (>= 3.0.2)
  Depends: python3-idna
  Depends: python3-urllib3 (>= 1.21.1)
  Depends: <python3:any>
    python3
  Depends: ca-certificates
  Breaks: awscli (<< 1.11.139)
  Suggests: python3-cryptography
  Suggests: python3-idna (>= 2.5)
  Suggests: python3-openssl
  Suggests: python3-socks
  Suggests: python-requests-doc

[Christian Ehrhardt (paelzer)]

The package isn't updated too often (for example I & J have the same version).
Nevertheless with this change this converts into regular work to merge.

Approving and uploading ... for now.
Since ubuntu1 is stuck in proposed I've uploaded with -v2.27.1+dfsg-1

Uploading requests_2.27.1+dfsg-1ubuntu2.dsc
Uploading requests_2.27.1+dfsg-1ubuntu2.debian.tar.xz
Uploading requests_2.27.1+dfsg-1ubuntu2_source.buildinfo
Uploading requests_2.27.1+dfsg-1ubuntu2_source.changes

I first wanted to ask you if you want to try submitting an "if ubuntu then ..." variant to Debian.
But then I realized the versions on the upstream code:

+- 'charset_normalizer~=2.0.0; python_version >= "3"',
+- 'chardet>=3.0.2,<5; python_version < "3"',
++ 'chardet>=3.0.2,<5',

That is usually a sign of a transition that will stay e.g. for py3 we want to use the newer more than the older. I checked upstream and it showed me:

https://github.com/psf/requests/commit/2ed84f55b22f19a1e1e8eea2e50963dce62052d3

Ok, this added charset_normalizer but with the fallback which we are now utilizing.
But then there is also:

https://github.com/psf/requests/commit/8bce583b9547c7b82d44c8e97f37cf9a16cbe758

Which makes it normalizer only for the coming version 2.28.
I think the proposed fix is fine as a stop-gap for now on 2.27.
But mid term we will need charset-normalizer in main, so we should start the MIR process right away to be ready.

Sadly there are other dependencies that will keep chardet in main at the same time.
So far it was python3-bs4 + python3-debian + python3-requests.
And AFAICS neither bs4 not pdebian seem to move, so despite the similarity we might end up needing both in main OR keep delta on either bs4+pdebian or requests.

Please track the migration of the upload, but also consider starting the MIR unless we find a better way.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 92eed73..8ccbead 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+requests (2.27.1+dfsg-1ubuntu2) kinetic; urgency=medium
+
+  * d/p/remove-charset-normalizer-dependency.patch: Remove charset-normalizer
+    as a dependency of requests (LP: #1975541)
+
+ -- Lena Voytek <[e-mail]>  Mon, 23 May 2022 15:06:07 -0700
+
 requests (2.27.1+dfsg-1ubuntu1) kinetic; urgency=medium
 
   * Fix autopkgtest when http_proxy, https_proxy or no_proxy variable is set
diff --git a/debian/patches/remove-charset-normalizer-dependency.patch b/debian/patches/remove-charset-normalizer-dependency.patch
new file mode 100644
index 0000000..5d4a598
--- /dev/null
+++ b/debian/patches/remove-charset-normalizer-dependency.patch
@@ -0,0 +1,91 @@
+Description: Remove charset-normalizer package as a backup to chardet
+ Since requests can use either chardet or charset-normalizer for character set
+ interpretations, and charset-normalizer is in universe, remove it as a
+ dependency to keep requests in main without issues.
+Forwarded: not-needed
+X-Not-Forwarded-Reason: charset-normalizer being in universe is Ubuntu-specific
+Author: Lena Voytek <[e-mail]>
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/requests/+bug/1975541
+Last-Update: 2022-05-23
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/requests/packages.py
++++ b/requests/packages.py
+@@ -1,12 +1,5 @@
+ import sys
+-
+-try:
+-    import chardet
+-except ImportError:
+-    import charset_normalizer as chardet
+-    import warnings
+-
+-    warnings.filterwarnings('ignore', 'Trying to detect', module='charset_normalizer')
++import chardet
+ 
+ # This code exists for backwards compatibility reasons.
+ # I don't like it either. Just look the other way. :)
+--- a/requests/compat.py
++++ b/requests/compat.py
+@@ -8,11 +8,7 @@
+ Python 3.
+ """
+ 
+-try:
+-    import chardet
+-except ImportError:
+-    import charset_normalizer as chardet
+-
++import chardet
+ import sys
+ 
+ # -------
+--- a/setup.py
++++ b/setup.py
+@@ -41,8 +41,7 @@
+ packages = ['requests']
+ 
+ requires = [
+-    'charset_normalizer~=2.0.0; python_version >= "3"',
+-    'chardet>=3.0.2,<5; python_version < "3"',
++    'chardet>=3.0.2,<5',
+     'idna>=2.5,<3; python_version < "3"',
+     'idna>=2.5,<4; python_version >= "3"',
+     'urllib3>=1.21.1,<1.27',
+--- a/requests/help.py
++++ b/requests/help.py
+@@ -11,10 +11,7 @@
+ 
+ from . import __version__ as requests_version
+ 
+-try:
+-    import charset_normalizer
+-except ImportError:
+-    charset_normalizer = None
++charset_normalizer = None
+ 
+ try:
+     import chardet
+@@ -113,7 +110,7 @@
+         'implementation': implementation_info,
+         'system_ssl': system_ssl_info,
+         'using_pyopenssl': pyopenssl is not None,
+-        'using_charset_normalizer': chardet is None,
++        'using_charset_normalizer': False,
+         'pyOpenSSL': pyopenssl_info,
+         'urllib3': urllib3_info,
+         'chardet': chardet_info,
+--- a/requests/__init__.py
++++ b/requests/__init__.py
+@@ -44,10 +44,7 @@
+ import warnings
+ from .exceptions import RequestsDependencyWarning
+ 
+-try:
+-    from charset_normalizer import __version__ as charset_normalizer_version
+-except ImportError:
+-    charset_normalizer_version = None
++charset_normalizer_version = None
+ 
+ try:
+     from chardet import __version__ as chardet_version
diff --git a/debian/patches/series b/debian/patches/series
index 5bde12d..81e1af9 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,3 @@
 0001-Remove-remote-images-traking-code-and-ads.patch
 0002-Fix-tests-with-HTTP-proxy.patch
+remove-charset-normalizer-dependency.patch