Skip to content

[3.10] bpo-45461: Fix IncrementalDecoder and StreamReader in the "unicode-escape" codec (GH-28939)#28943

Merged
serhiy-storchaka merged 2 commits into
python:3.10from
miss-islington:backport-c96d154-3.10
Oct 14, 2021
Merged

[3.10] bpo-45461: Fix IncrementalDecoder and StreamReader in the "unicode-escape" codec (GH-28939)#28943
serhiy-storchaka merged 2 commits into
python:3.10from
miss-islington:backport-c96d154-3.10

Conversation

@miss-islington

@miss-islington miss-islington commented Oct 14, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

They support now splitting escape sequences between input chunks.

Add the third parameter "final" in codecs.unicode_escape_decode().
It is True by default to match the former behavior.
(cherry picked from commit c96d154)

Co-authored-by: Serhiy Storchaka storchaka@gmail.com

https://bugs.python.org/issue45461

@serhiy-storchaka @miss-islington
bpo-45461: Fix IncrementalDecoder and StreamReader in the "unicode-es…
03bda99 
…cape" codec (pythonGH-28939)

They support now splitting escape sequences between input chunks.

Add the third parameter "final" in codecs.unicode_escape_decode().
It is True by default to match the former behavior.
(cherry picked from commit c96d154)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@miss-islington

miss-islington commented Oct 14, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka: Status check is done, and it's a success ✅ .

1 similar comment
@miss-islington

miss-islington commented Oct 14, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka: Status check is done, and it's a success ✅ .

@bedevere-bot bedevere-bot mentioned this pull request Oct 14, 2021
Merged
@miss-islington

miss-islington commented Oct 14, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka: Status check is done, and it's a success ✅ .

@bedevere-bot bedevere-bot added the type-bug An unexpected behavior, bug, or error label Oct 14, 2021
@miss-islington

miss-islington commented Oct 14, 2021

Copy link
Copy Markdown
Contributor Author

@serhiy-storchaka: Status check is done, and it's a success ❌ .

@serhiy-storchaka

serhiy-storchaka commented Oct 14, 2021

Copy link
Copy Markdown
Member

@pablogsal, how to unblock merge of this PR?

@pablogsal

pablogsal commented Oct 14, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

@pablogsal, how to unblock merge of this PR?

You need to regenerate the ABI file by running make regen-abidump. Just make sure you run this on the same system that the CI runs on, using Docker for example:

cpython/.github/workflows/build.yml

Lines 56 to 76 in f40b230

check_abi:
name: 'Check if the ABI has changed'
runs-on: ubuntu-20.04
needs: check_source
if: needs.check_source.outputs.run_tests == 'true'
steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
- name: Install Dependencies
run: |
sudo ./.github/workflows/posix-deps-apt.sh
sudo apt-get install -yq abigail-tools
- name: Build CPython
env:
CFLAGS: -g3 -O0
run: |
# Build Python with the libpython dynamic library
./configure --enable-shared
make -j4
- name: Check for changes in the ABI
run: make check-abidump

I can do it for you if you prefer.

For example:

# On the CPython repo with the 3.10 branch checked out
docker run -it --rm  -v `pwd`:/src ubuntu:20.04
cd /src
./.github/workflows/posix-deps-apt.sh 
apt-get install -yq abigail-tools 
export CFLAGS='-O0 -g3'
./configure --enable-shared 
make -j
make regen-abidump

@serhiy-storchaka

serhiy-storchaka commented Oct 14, 2021

Copy link
Copy Markdown
Member

Can this be automated? I tried to regenerate it cutting corners, but different steps take a long time, and I made some errors. The resulting diff is too large.

@serhiy-storchaka

serhiy-storchaka commented Oct 14, 2021

Copy link
Copy Markdown
Member

Also an installed python3.10 was required by default to regenerate sources for python3.10.

PYTHON_FOR_REGEN?=python3.10

@pablogsal

pablogsal commented Oct 14, 2021

Copy link
Copy Markdown
Member

Can this be automated?

No, If the test fails is because the ABI is breaking so it needs manual validation by the release manager. I don't want to provide shortcuts for this

Also an installed python3.10 was required by default to regenerate sources for python3.10.

Eh? Not sure I understand, you can use any python for this (like the installed python3 in ubuntu)

@pablogsal

pablogsal commented Oct 14, 2021

Copy link
Copy Markdown
Member

The resulting diff is too large.

You are producing sort of a binary file, so the size of the diff is not relevant. The relevant part is that the check suceeds.

@serhiy-storchaka
serhiy-storchaka merged commit 0bff4cc into python:3.10 Oct 14, 2021
@miss-islington
miss-islington deleted the backport-c96d154-3.10 branch October 14, 2021 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@lysnikolaou lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner

Assignees

No one assigned

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@miss-islington @serhiy-storchaka @pablogsal @the-knights-who-say-ni @bedevere-bot