mirror/bitcoin
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#33702: contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO

Browse Source 
fad6118586 test: Fix "typo" in written invalid content (MarcoFalke)
fab085c15f contrib: Use text=True in subprocess over manual encoding handling (MarcoFalke)
fa71c15f86 scripted-diff: Bump copyright headers after encoding changes (MarcoFalke)
fae612424b contrib: Remove confusing and redundant encoding from IO (MarcoFalke)
fa7d72bd1b lint: Drop check to enforce encoding to be specified in Python scripts (MarcoFalke)
faf39d8539 test: Clarify that Python UTF-8 mode is the default today for most systems (MarcoFalke)
fa83e3a81d lint: Do not allow locale dependent shell scripts (MarcoFalke)

Pull request description:

  Historically, there was an attempt via `test/lint/lint-python-utf8-encoding.py` to enforce explicit UTF8 in every Python IO statement (`open`, `subprocess`, ...). However, the lint check has many problems:

  * The check is incomplete and many IO statements lack the explicit UTF8 specification.
  * It was added at a time when some systems were not UTF8 by default.
  * The check is brittle, as it depends on a fragile regex.

  In theory, now that the minimum Python version is 3.10 (since commit 2123c94448), the check could be replaced by `PYTHONWARNDEFAULTENCODING=1` from https://docs.python.org/3/whatsnew/3.10.html#optional-encodingwarning-and-encoding-locale-option. However, this comes with many other problems:

  * All our Python scripts already assume and require UTF8 to be set externally. On almost all modern systems, this is already the default. Some Windows versions do not have UTF8 by default and require `PYTHONUTF8=1` to be set for the tests to run already today (with or without the changes in this pull). Also, the CI and many other Bash scripts force UTF8 via `LC_ALL`. Finally, Python 3.15 will likely enable UTF8 on *all* systems by default, per https://peps.python.org/pep-0686/#abstract.
  * So adding UTF8 to every single IO call is redundant, verbose, and confusing, given that it is the expected default.

  So fix all issues, by:

  * Removing the `test/lint/lint-python-utf8-encoding.py` check.
  * Removing the encoding on the individual IO calls.
  * Clarifying the existing docs around the existing UTF8 requirement and assumption.

  Obviously, every IO call is still free to specify UTF8 or any other encoding explicitly, if there is a documented need for it in the future.

ACKs for top commit:
  theStack:
    re-ACK fad6118586
  laanwj:
    Re-ACK fad6118586

Tree-SHA512: 78025ea3508597d2299490347614f0ee3e4c66e3ba559ff50e498045a9c8bbd92f3a5ced18719d8fcebbd1e47bdbb56a0c85a5b73b425adb0ea4f02fe69c3149
This commit is contained in:
merge-script committed 2025-12-03 09:54:47 +00:00
2 parents 4c784b25c4 fad6118586
commit af0e6a65c9
63 files changed
+217 -299
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

test/functional/feature_config_args.py
+21 -21
View File
@@ -55,7 +55,7 @@ class ConfArgsTest(BitcoinTestFramework):
os.rename(conf_path.with_suffix('.confbkp'), conf_path)
self.log.debug('Verifying includeconf directive pointing to directory is caught')
with open(conf_path, 'a', encoding='utf-8') as conf:
with open(conf_path, 'a') as conf:
conf.write(f'includeconf={self.nodes[0].datadir_path}\n')
self.nodes[0].assert_start_raises_init_error(
extra_args=['-regtest'],
@@ -68,12 +68,12 @@ class ConfArgsTest(BitcoinTestFramework):
self.log.info('Disabling configuration via -noconf')
conf_path = self.nodes[0].datadir_path / 'bitcoin.conf'
with open(conf_path, encoding='utf-8') as conf:
with open(conf_path) as conf:
settings = [f'-{line.rstrip()}' for line in conf if len(line) > 1 and line[0] != '[']
os.rename(conf_path, conf_path.with_suffix('.confbkp'))
self.log.debug('Verifying garbage in config can be detected')
with open(conf_path, 'a', encoding='utf-8') as conf:
with open(conf_path, 'a') as conf:
conf.write('garbage\n')
self.nodes[0].assert_start_raises_init_error(
extra_args=['-regtest'],
@@ -107,9 +107,9 @@ class ConfArgsTest(BitcoinTestFramework):
expected_msg=conf_in_config_file_err,
)
inc_conf_file_path = self.nodes[0].datadir_path / 'include.conf'
with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a', encoding='utf-8') as conf:
with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a') as conf:
conf.write(f'includeconf={inc_conf_file_path}\n')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('conf=some.conf\n')
self.nodes[0].assert_start_raises_init_error(
expected_msg=conf_in_config_file_err,
@@ -119,65 +119,65 @@ class ConfArgsTest(BitcoinTestFramework):
expected_msg='Error: Error parsing command line arguments: Invalid parameter -dash_cli=1',
extra_args=['-dash_cli=1'],
)
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('dash_conf=1\n')
with self.nodes[0].assert_debug_log(expected_msgs=['Ignoring unknown configuration value dash_conf']):
self.start_node(0)
self.stop_node(0)
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('reindex=1\n')
with self.nodes[0].assert_debug_log(expected_msgs=["[warning] reindex=1 is set in the configuration file, which will significantly slow down startup. Consider removing or commenting out this option for better performance, unless there is currently a condition which makes rebuilding the indexes necessary"]):
self.start_node(0)
self.stop_node(0)
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('-dash=1\n')
self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 1: -dash=1, options in configuration file must be specified without leading -')
if self.is_wallet_compiled():
with open(inc_conf_file_path, 'w', encoding='utf8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write("wallet=foo\n")
self.nodes[0].assert_start_raises_init_error(expected_msg=f'Error: Config setting for -wallet only applied on {self.chain} network when in [{self.chain}] section.')
main_conf_file_path = self.nodes[0].datadir_path / "bitcoin_main.conf"
util.write_config(main_conf_file_path, n=0, chain='', extra_config=f'includeconf={inc_conf_file_path}\n')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('acceptnonstdtxn=1\n')
self.nodes[0].assert_start_raises_init_error(extra_args=[f"-conf={main_conf_file_path}", "-allowignoredconf"], expected_msg='Error: acceptnonstdtxn is not currently supported for main chain')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('nono\n')
self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('server=1\nrpcuser=someuser\nrpcpassword=some#pass')
self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 3, using # in rpcpassword can be ambiguous and should be avoided')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('server=1\nrpcuser=someuser\nmain.rpcpassword=some#pass')
self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 3, using # in rpcpassword can be ambiguous and should be avoided')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('server=1\nrpcuser=someuser\n[main]\nrpcpassword=some#pass')
self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 4, using # in rpcpassword can be ambiguous and should be avoided')
inc_conf_file2_path = self.nodes[0].datadir_path / 'include2.conf'
with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a', encoding='utf-8') as conf:
with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a') as conf:
conf.write(f'includeconf={inc_conf_file2_path}\n')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('testnot.datadir=1\n')
with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file2_path, 'w') as conf:
conf.write('[testnet]\n')
self.restart_node(0)
self.nodes[0].stop_node(expected_stderr=f'Warning: {inc_conf_file_path}:1 Section [testnot] is not recognized.{os.linesep}{inc_conf_file2_path}:1 Section [testnet] is not recognized.')
with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file_path, 'w') as conf:
conf.write('') # clear
with open(inc_conf_file2_path, 'w', encoding='utf-8') as conf:
with open(inc_conf_file2_path, 'w') as conf:
conf.write('') # clear
def test_config_file_log(self):
@@ -522,9 +522,9 @@ class ConfArgsTest(BitcoinTestFramework):
conf_file = default_data_dir / "bitcoin.conf"
# datadir needs to be set before [chain] section
with open(conf_file, encoding='utf8') as f:
with open(conf_file) as f:
conf_file_contents = f.read()
with open(conf_file, 'w', encoding='utf8') as f:
with open(conf_file, 'w') as f:
f.write(f"datadir={new_data_dir}\n")
f.write(conf_file_contents)