test, contrib, refactor: use with when opening a file

[brunoerg]

When manipulating a file in Python without using with(), you have to close the file manually, so this PR does it in get_block_hashes (contrib/linearize/linearize-data.py).

Edit: this PR does it for all occurances that previously weren't using with.

[laanwj]

When manipulating a file in Python without using with(),

You're right, though prefer fixing this by using with …. Or is there a specific reason to avoid it here?

[brunoerg]

You're right, though prefer fixing this by using with …. Or is there a specific reason to avoid it here?

@laanwj, I didn't do it before because since in other places it closes the file after manipulating it (without using with), it would be simple just add a close()here, but I 100% prefer with, can I modify all the occurances to use with?

[theStack]

You're right, though prefer fixing this by using with …. Or is there a specific reason to avoid it here?

@laanwj, I didn't do it before because since in other places it closes the file after manipulating it (without using with), it would be simple just add a close()here, but I 100% prefer with, can I modify all the occurances to use with?

Concept ACK on identifying further such cases and fixing them. Especially for small scripts not closing is not a big deal in practice though; here is a nice list of arguments why it it strongly recommended to do it anyway: https://stackoverflow.com/a/25070939

Assuming that the following used regex is reliable, we have 17 occurences of open() calls without the with syntax (didn't check them in detail, but I guess many of them never call close):

$ git grep "=[ ]*open(" ./contrib/ ./test | wc -l
      17
$ git grep "=[ ]*open(" ./contrib/ ./test
contrib/devtools/copyright_header.py:    f = open(filename, 'r', encoding="utf8")
contrib/devtools/copyright_header.py:    f = open(filename, 'w', encoding="utf8")
contrib/linearize/linearize-data.py:    f = open(settings['hashlist'], "r", encoding="utf8")
contrib/linearize/linearize-data.py:            self.outF = open(self.outFname, "wb")
contrib/linearize/linearize-data.py:                    self.inF = open(fname, "rb")
contrib/linearize/linearize-data.py:    f = open(sys.argv[1], encoding="utf8")
contrib/linearize/linearize-hashes.py:    f = open(sys.argv[1], encoding="utf8")
contrib/verify-commits/verify-commits.py:    verified_root = open(dirname + "/trusted-git-root", "r", encoding="utf8").read().splitlines()[0]
contrib/verify-commits/verify-commits.py:    verified_sha512_root = open(dirname + "/trusted-sha512-root-commit", "r", encoding="utf8").read().splitlines()[0]
contrib/verify-commits/verify-commits.py:    revsig_allowed = open(dirname + "/allow-revsig-commits", "r", encoding="utf-8").read().splitlines()
contrib/verify-commits/verify-commits.py:    unclean_merge_allowed = open(dirname + "/allow-unclean-merge-commits", "r", encoding="utf-8").read().splitlines()
contrib/verify-commits/verify-commits.py:    incorrect_sha512_allowed = open(dirname + "/allow-incorrect-sha512-commits", "r", encoding="utf-8").read().splitlines()
test/functional/feature_config_args.py:        conf_file_contents = open(conf_file, encoding='utf8').read()
test/functional/feature_versionbits_warning.py:        alert_text = open(self.alert_filename, 'r', encoding='utf8').read()
test/util/test_runner.py:    raw_data = open(input_filename, encoding="utf8").read()
test/util/test_runner.py:        inputData = open(filename, encoding="utf8").read()
test/util/test_runner.py:            outputData = open(os.path.join(testDir, outputFn), encoding="utf8").read()

[laanwj]

can I modify all the occurances to use with?

Sounds good to me. It removes the burden of having the explicitly think about closing (removes the entire bug class of "forget to close") so is very much preferred when it's possible.

[brunoerg]

Thanks, @laanwj and @theStack. I gonna fix all of them.

[brunoerg]

Force-pushed addressing @laanwj and @theStack comments. Just changed the title to address it.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #24985 (doc, test: Compilation for 64-bit Windows with msys2 by rnapoles)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[brunoerg]

I think we could put in the docs a recommendation to use when when opening files in Python. What do you think?

[laanwj]

Code review ACK 027aab6

I think we could put in the docs a recommendation to use when when opening files in Python. What do you think?

Not sure. Isn't that like a general Python thing, not specific to our project? I don't think it's worthwhile to maintain a document of basic Python programming language best practices in our project.