git-afsantos / haros

H(igh) A(ssurance) ROS - Static analysis of ROS application code.
MIT License
191 stars 37 forks source link

Solution for PyFlwor installation #47

Closed git-afsantos closed 5 years ago

git-afsantos commented 5 years ago

Currently, pyflwor requires an editable VCS install, and this is neither standard nor convenient (see #38).

43 made pyflwor an optional dependency, but a better solution for its installation should be worked on.

Ideally, haros would not depend on pyflwor, or a better alternative would be used instead.

Here are the currently considered options to overcome this issue.

1. Try to fix the original

Find the issue in pyflwor that makes it write additional files through PLY, and submit a PR if it can be avoided. Possible candidates include this line and this line.

The major drawback with this option is that the PR might not be accepted, or it might not be released afterwards.

2. Ship a modified version in HAROS

Fix the original issue, so that no files are written (if possible), and ship the altered version with HAROS itself. If this cannot be done, at least explore the parameters that allow a custom directory for the PLY table files, and redirect the outputs (~/.haros would be an option here).

The major drawback of this approach is that pyflwor is licensed under BSD, while HAROS is MIT.

3. Force the use of a virtualenv

When using virtualenv it seems that the original issue is not present. Using virtualenv is a recommended practice in general, in any case, and so HAROS could adopt this option as its standard mode of operation.

The major drawback of this option is forcing a certain type of installation/usage.

4. Monkey patch pyflwor

This is possibly a more laborious alternative, but one that could work. Instead of using the high-level API of pyflwor, change the problematic areas (the two lines linked above) via monkey patching, redirect the generated files to ~/.haros and use the patched version.

While this option can make some things easier (e.g. a simple pip install pyflwor-ext would work just fine), it is not as maintainable.

nlimpert commented 5 years ago

I think option 1 would be ideal but as you said might not make it upstream.

As we discovered that haros works fine in haros_catkin using catkin_virtualenv in https://github.com/git-afsantos/haros/issues/38 I'd be in favor of option 3. However I wonder how that would work if haros would be installable via rosdep - as you mentioned in option 3 this would enforce an installation method and thus is probably not feasible for installations on a system level.

Do you think option 2 would require a modification of pyflwor if we manage to just remap the directories that pyflwor wants to write in to either ~/.haros or /tmp? I had a look at this during the last week trying to remap the execution path by setting the execution path with os.chdir but that did not help (this was a rough guess towards a fix to be honest).

git-afsantos commented 5 years ago

@nlimpert:

Do you think option 2 would require a modification of pyflwor if we manage to just remap the directories that pyflwor wants to write in to either ~/.haros or /tmp? I had a look at this during the last week trying to remap the execution path by setting the execution path with os.chdir but that did not help (this was a rough guess towards a fix to be honest).

If I understood correctly, you are basically suggesting option 4 (monkey patching on the HAROS side). I am leaning more towards this option too. While it may not be as maintainable in the long term (supposing that pyflwor might change in the future and HAROS keeps using it), it might be the most direct option to put into practice. I will be giving this a try.