zestsoftware / zest.releaser

Python software releasing made easy and repeatable
https://zestreleaser.readthedocs.io
GNU General Public License v2.0
198 stars 62 forks source link

ValueError: binary mode doesn't take an encoding argument #391

Closed LvffY closed 1 year ago

LvffY commented 1 year ago

I wanted to try out the modifications made by #389.

Unfortuntately, I've fall into an issue that seems releated to the major version upgrade you've made.

I have a simple python (with src/tests directory layout) and trya fullrealease --no-input command (I've also tried fullrealease manually and fall into the same error)

To reproduce

## Plugin used for package release.
## More informations : https://zestreleaser.readthedocs.io/en/latest/
[zest.releaser]
create-wheel = yes
encoding = utf-8
tag-format = {version}
prefix-message = [TOOLS] 🤖 [ci skip]
date-format = %%Y-%%m-%%d %%H:%%M:%%S
[testenv:release]
description = "Environment for release actions"
skip_install = true
passenv =
## This is needed to be able to do custom pypi repository
    PIP_CONFIG_FILE
## This is needed to define our upload repository
    TWINE_REPOSITORY
## This is needed to run git command (inside zest.releaser)
    HOME
deps =
    zest.releaser[recommended]
commands =
    fullrelease --no-input
tox -e release

The error

release run-test-pre: PYTHONHASHSEED='3440558210'
release run-test: commands[0] | fullrelease --no-input
INFO: Starting prerelease.
INFO: Run pyroma on the package before tagging?
INFO: Auto-responding 'yes'.
INFO: ------------------------------
INFO: Checking /home/vsts/work/1/s
INFO: Getting metadata for wheel...
running dist_info
creating /tmp/tmpd6263zdx/cdc_test_core.egg-info
writing /tmp/tmpd6263zdx/cdc_test_core.egg-info/PKG-INFO
writing dependency_links to /tmp/tmpd6263zdx/cdc_test_core.egg-info/dependency_links.txt
writing entry points to /tmp/tmpd6263zdx/cdc_test_core.egg-info/entry_points.txt
writing requirements to /tmp/tmpd6263zdx/cdc_test_core.egg-info/requires.txt
writing top-level names to /tmp/tmpd6263zdx/cdc_test_core.egg-info/top_level.txt
writing manifest file '/tmp/tmpd6263zdx/cdc_test_core.egg-info/SOURCES.txt'
reading manifest file '/tmp/tmpd6263zdx/cdc_test_core.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching '*.md' under directory 'src'
warning: no previously-included files found matching '*.properties'
warning: no previously-included files found matching 'constraints*'
warning: no previously-included files found matching 'CONTRIBUTING.md'
warning: no previously-included files found matching 'pylintrc'
warning: no previously-included files found matching 'pytest.ini'
warning: no previously-included files found matching 'pylint-report.txt'
warning: no previously-included files found matching 'conftest.py'
warning: no previously-included files found matching 'bumpversion.cfg'
warning: no previously-included files found matching 'tox.ini'
warning: no previously-included files matching '*.py' found under directory 'tests'
warning: no previously-included files matching '*' found under directory 'docs'
warning: no previously-included files matching '*' found under directory 'pipelines'
warning: no previously-included files found matching 'azure-pipelines.yml'
writing manifest file '/tmp/tmpd6263zdx/cdc_test_core.egg-info/SOURCES.txt'
creating '/tmp/tmpd6263zdx/cdc_test_core-0.0.0.0.dev0.dist-info'
INFO: Found cdc-test-core
INFO: ------------------------------
INFO: Your package does neither have a license field nor any license classifiers.
INFO: ------------------------------
INFO: Final rating: 9/10
INFO: Cottage Cheese
INFO: ------------------------------
INFO: Do you want to run check-manifest?
INFO: Auto-responding 'yes'.
lists of files in version control and sdist match
Traceback (most recent call last):
  File "/home/vsts/work/1/s/.tox/release/bin/fullrelease", line 8, in <module>
    sys.exit(main())
  File "/home/vsts/work/1/s/.tox/release/lib/python3.8/site-packages/zest/releaser/fullrelease.py", line 23, in main
    prereleaser.run()
  File "/home/vsts/work/1/s/.tox/release/lib/python3.8/site-packages/zest/releaser/baserelease.py", line 413, in run
    self.prepare()
  File "/home/vsts/work/1/s/.tox/release/lib/python3.8/site-packages/zest/releaser/prerelease.py", line 70, in prepare
    self._grab_history()
  File "/home/vsts/work/1/s/.tox/release/lib/python3.8/site-packages/zest/releaser/baserelease.py", line 121, in _grab_history
    history_lines, history_encoding = read_text_file(
  File "/home/vsts/work/1/s/.tox/release/lib/python3.8/site-packages/zest/releaser/utils.py", line 85, in read_text_file
    with open(filename, "rb", encoding=fallback_encoding) as filehandler:
ValueError: binary mode doesn't take an encoding argument
ERROR: InvocationError for command /home/vsts/work/1/s/.tox/release/bin/fullrelease --no-input (exited with code 1)
___________________________________ summary ____________________________________
ERROR:   release: commands failed
LvffY commented 1 year ago

@reinout @mauritsvanrees I think this is quite urgent because I think every one using the latest version of this lib will be impacted by this error

reinout commented 1 year ago

I've taken your setup.cfg, did a quick git init in an empty dir, added a 0.1.dev0 version.txt and ran fullrelease: no error. I'll have to look a bit further.

reinout commented 1 year ago

Ah, I had to add a changelog file, too. Now I'm getting the error.

mauritsvanrees commented 1 year ago

This is only when you explicitly set an encoding in setup.cfg. I don't think I have that in any of the many packages that I release. So not everybody is impacted. But should be fixed, yes.

reinout commented 1 year ago

@LvffY : found it, partially. Everything should work for you if you remove the "encoding=" line from your setup.cfg. Without it, fullrelease runs fine.

Some left-over code from the weird magic we had to do for python2.

reinout commented 1 year ago

@mauritsvanrees : great minds think alike and comment at the same time? :-)

I wonder: is that "encoding" setting even needed anymore now that we're on python 3?

LvffY commented 1 year ago

@reinout @mauritsvanrees I'll try to remove the encoding and tell you :)

May be a warning on the change log could be useful ?

Anyway, thanks for your very quick answer :)

mauritsvanrees commented 1 year ago

We have this:

with open(filename, "rb", encoding=encoding) as filehandler:
    data = filehandler.read()

That probably should either be this:

with open(filename, "r", encoding=encoding) as filehandler:
    data = filehandler.read()

or this:

with open(filename, "rb") as filehandler:
    data = filehandler.read().decode(encoding)

Or maybe try both.

It would be good to add a test, because apparently this is not caught by the current ones.

LvffY commented 1 year ago

@reinout @mauritsvanrees you were right : removing the encoding property fixed my issue :)

I think that we can't still close the issue because this is just a workaround and bad properties shouldn't break the program, right ?