Updated Makefile and docs for fedsd utility#262
Conversation
|
BTW, |
|
It works this way, but seems more like a hack... |
edwardalee
left a comment
There was a problem hiding this comment.
One problem with the fix to the Makefile is that now a file in /usr/local/bin is a symbolic link into a user's clone of the repo. This seems problematic to me. Would it work instead to copy launch-fedsd.sh into /usr/local/bin (or wherever is specified with INSTALL_PREFIX)?
Also, for the other changes that try to prepend a path to trace_to_csv using an INSTALL_PREFIX argument to fedsd, this seems awkward. If trace_to_csv is in the user's PATH, it seems like it would not be necessary. Can we just check to see whether trace_to_csv is in the user's path and issue a friendly error message if not?
|
I am sorry about this. I didnt test the fedsd utility. I think I agree with Edward, it is not unreasonable to demand that for |
|
What is, btw, the reason for having this launch shell |
|
The Zephyr test failure is fixed by updating LF ref here: #263 |
No need for the launch script, IMO. Let make install copy the Python script and be done with it? Make sure it allows for specifying an installation prefix... |
Can a python program be made executable? I was unaware of that... How does the OS know which python to invoke? |
|
|
Just pushed a fix, where A detail worth mentioning in this update is that one does not need saying Regarding |
What configuration effort? On Windows, a bash script must be executed in WSL, and in WSL running a script with a |
Yes yes, you're right! Same for Cygwin. |
Since the introduction of WSL, there is a whole lot less to worry about. We can just tell Windows users to use WSL... |
|
To build confidence in multi-platform support, just add some tests in CI and let them run them on all platforms. I don't believe there are currently any tests, so please add those. |
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
Co-authored-by: Marten Lohstroh <marten@berkeley.edu>
lhstrh
left a comment
There was a problem hiding this comment.
Modulo suggestions, this looks good, but there are still no tests, so we don't have any assurance that this works and will keep working...
|
Since it appears that what's in |
When installing the tracing executables to
/usr/local/bin,fedsdwill not work, because it is operating on a dangling symlink.This PR provides a fix.