Avoid opening Specfile with write-access#532
Conversation
Per discussion in Copr issue tracker: fedora-copr/copr#4266
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies specfile/specfile.py to open files in read-only mode ('r') instead of read-write mode ('r+') during initialization, path updates, and file reopening. A critical issue was identified where these changes will cause the save() method to fail with an io.UnsupportedOperation error, as it relies on _reopen_named_file() providing a writable file handle. It is recommended to update _reopen_named_file() to accept a mode parameter to support both read and write operations.
| return | ||
| self._file.close() | ||
| self._file = self.path.open("r+", **self.ENCODING_ARGS) | ||
| self._file = self.path.open("r", **self.ENCODING_ARGS) |
There was a problem hiding this comment.
Changing the file opening mode to "r" in _reopen_named_file() will cause the save() method to fail. The save() method (lines 294-306) calls _reopen_named_file() and then attempts to truncate and write to the file handle. Since the file is now opened in read-only mode, these operations will raise an io.UnsupportedOperation error.
To resolve this, _reopen_named_file() should be updated to accept a mode parameter (defaulting to "r"), and save() should be updated to call it with mode="r+". Since save() is not currently part of this diff, you should include it in the pull request to apply the necessary changes.
|
✔️ pre-commit SUCCESS in 2m 10s |
|
Seems this change is indeed wrong. Do whatever you prefer with this PR (guide me to provide a better patch, but even closing is fine). It was easier for me to open this than filling a proper issue... |
|
specfile is primarily read-write by design. I think the proper fix here would be to try to open the path with |
|
Ok, lemme close this one and have an issue instead: #534 |
3 participants
Per discussion in Copr issue tracker:
fedora-copr/copr#4266