Open correctmost opened 1 week ago
That's because pty.fork()
secondary executor will be the thing that performs that call.
Maybe pylint can't handle forked code?
Hmm, mypy also flags that line as unreachable:
archinstall/lib/general.py:219: error: Statement is unreachable [unreachable]
I tried this sample code to try to understand the semantics better:
import os
import pty
def write():
return os.write(fd, b"foo")
raise AssertionError
os.fsync(fd)
pid, fd = pty.fork()
write()
The code doesn't seem to trigger the AssertionError
, which makes me think the fsync
call might not actually be performed.
I was thinking the archinstall code needed to be updated like the following, but I might be misunderstanding the existing command worker code:
if self.child_fd:
bytes_written = os.write(self.child_fd, data + (b'\n' if line_ending else b''))
os.fsync(self.child_fd)
return bytes_written
That example has no check if the code is executed in the parent or forked child.
Both threads will call write()
, whereas I check if we have a pid, and only the child process will have something there, the parent won't and thus skips that part of the code.
import pty
import random
pid, fd = pty.fork()
with open(f"/tmp/pid_{random.randint(0, 2000)}", "w") as fh:
fh.write(f"pid: {pid}, fd: {fd}")
Both threads will execute, but with different values :)
Thanks, the forking part makes sense :).
I think the main item of confusion for me is how the os.fsync
call on 219 can be reached after a return statement on line 218:
My understanding is that the return would stop further execution regardless of whether the code is executed by the child or parent process.
We can suppress the pylint and mypy warnings for line 219, but I would want to make sure it's a true false positive. Thanks!
Ah yeah that return never happens, sorry about that one hehe. We can either move that line up or remove it all together
Description
pylint --enable=unreachable .
picks up the following unreachablefsync
call:https://github.com/archlinux/archinstall/blob/7437668f1e51eb3c88f7a0ddc7ad7ef30b7f64a5/archinstall/lib/general.py#L217-L219
Warning