-
Notifications
You must be signed in to change notification settings - Fork 414
Feature/windows fixes #581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Feature/windows fixes #581
Conversation
…e exists first, and if not try a .exe
…ion to exit prematurely. Modified them as is appropriate for Windows and not lcm-logger is working as expected
… function from builds where it's not required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as all these changes are reasonable and fix at least some of the described issues; just let me know when you want to merge
if platform.system() == 'Windows': | ||
candidates = [ | ||
os.path.join(LCM_BIN_DIR, f"{name}.bat"), | ||
os.path.join(LCM_BIN_DIR, f"{name}.exe") | ||
] | ||
else: | ||
candidates = [os.path.join(LCM_BIN_DIR, name)] | ||
|
||
for executable in candidates: | ||
if os.path.exists(executable): | ||
return subprocess.call([executable, *args], close_fds=False) | ||
|
||
raise FileNotFoundError(f"No executable found for {name} in {LCM_BIN_DIR}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On windows the init.py function doesn't load all the binaries as expected. It attempts to launch .bat files but for some applications, such as lcm-logger.exe a .bat doesn't exist in the bin folder.
I can reproduce this issue. After running pip install . -v --config-settings=cmake.args=--preset=vcpkg-vs
with a venv activate, running lcm-logger
produces the error:
Traceback (most recent call last):
File "<frozen runpy>", line 198, in _run_module_as_main
File "<frozen runpy>", line 88, in _run_code
File "C:\Users\lcm\.venv\Scripts\lcm-logger.exe\__main__.py", line 7, in <module>
sys.exit(run_logger())
~~~~~~~~~~^^
File "C:\Users\lcm\.venv\Lib\site-packages\lcm\__init__.py", line 196, in run_logger
raise SystemExit(run_script('lcm-logger', sys.argv[1:]))
~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\lcm\.venv\Lib\site-packages\lcm\__init__.py", line 182, in run_script
return subprocess.call([os.path.join(LCM_BIN_DIR, name + file_extension), *args], close_fds=False)
~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\AppData\Local\Programs\Python\Python313-arm64\Lib\subprocess.py", line 397, in call
with Popen(*popenargs, **kwargs) as p:
~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\AppData\Local\Programs\Python\Python313-arm64\Lib\subprocess.py", line 1038, in __init__
self._execute_child(args, executable, preexec_fn, close_fds,
~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pass_fds, cwd, env,
^^^^^^^^^^^^^^^^^^^
...<5 lines>...
gid, gids, uid, umask,
^^^^^^^^^^^^^^^^^^^^^^
start_new_session, process_group)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\AppData\Local\Programs\Python\Python313-arm64\Lib\subprocess.py", line 1550, in _execute_child
hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
# no special security
^^^^^^^^^^^^^^^^^^^^^
...<4 lines>...
cwd,
^^^^
startupinfo)
^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified
These changes fix that issue for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as these changes all look reasonable and don't appear to break anything for me, but they don't appear to fix this issue:
The other issue I've found was lcm-logger.exe quits after creating the log file. This seems to be down to different ways of handling signals between windows and linux. I've made some minor windows-specific tweaks which allows lcm-logger.exe to run without issue on Windows.
Both lcm-logger
and lcm-logplayer
appear to be no-ops. No matter what commands I run, they immediately return with no output.
On windows the init.py function doesn't load all the binaries as expected. It attempts to launch
.bat
files but for some applications, such aslcm-logger.exe
a.bat
doesn't exist in thebin
folder.The other issue I've found was
lcm-logger.exe
quits after creating the log file. This seems to be down to different ways of handling signals between windows and linux. I've made some minor windows-specific tweaks which allowslcm-logger.exe
to run without issue on Windows.