sel4-tutorials
sel4-tutorials copied to clipboard
tools: update to sh version 2
There was a change to the interface in sh version 2 where the output is a true string, instead of a special object with return_value field etc. There is an opt-in available to the old behaviour by passing the keyword argument _return_cmd=True--which is what I'm doing here.
That said, with this change (presumably) the older sh version 1 would no longer work due to the new keyword argument... on a note related to that, the FAQ mentions sh being designed for ease of embedding (essentially, paraphrasing), so it might be worth considering just vendoring one specific version.
I tested the ./init --tut hello-world with these changes applied, which works (I'm still working on the tutorials/learning the ropes wrt seL4).
I guess the CI failure here is from the CI environment using the older version of sh... since it's only used minimally and is a self-contained single-file no-deps thing, I honestly think it might be best to just pick one version and vendor it.
I guess the CI failure here is from the CI environment using the older version of
sh... since it's only used minimally and is a self-contained single-file no-deps thing, I honestly think it might be best to just pick one version and vendor it.
That's probably not a bad idea, yes.
It seems this PR is left unattended. The problem described here is an active problem. The ./init tool fails to exit successfully on all tutorials. The sel4-deps module doesn't specify which version of sh is required. So pip, by default, installs the latest version. All new installs of sel4-deps would have the issue described in this PR.
On the bright side, the tool works, but it crashes before it exists.
This might lead some people to wonder whether the ./init was successful or not.
Below you can see exact error.
-- Build files have been written to: /home/debian/Desktop/sel4-tutorials-manifest/interrupts_build
Traceback (most recent call last):
File "/home/debian/Desktop/sel4-tutorials-manifest/interrupts/../init", line 91, in <module>
sys.exit(main())
^^^^^^
File "/home/debian/Desktop/sel4-tutorials-manifest/interrupts/../init", line 81, in main
if result.exit_code != 0:
^^^^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'exit_code'
debian@debian:~/Desktop/sel4-tutorials-manifest/interrupts$
There are many ways to solve this, but currently the ideal way is to check which version of sh is installed and then use the right method. This would ensure both versions are supported, but as a negative the script would look weird.
For example:
if sh.__version__.startswith("1"):
sh.cmake(args + [tute_directory], _cwd=directory, _out=output, _err=output)
else:
sh.cmake(args + [tute_directory], _cwd=directory, _out=output, _err=output, _return_cmd=True)
Are you continuing your work on this PR @FireyFly ? If not, I would like to take over and fix the issue.
There are many ways to solve this, but currently the ideal way is to check which version of sh is installed and then use the right method. This would ensure both versions are supported, but as a negative the script would look weird.
Yes, that would be great (that it would work on both versions, not that it would look weird :-) -- but I don't think it's that weird).
Okay thanks. I'm on it!
Thanks for taking it over, @TunaCici! I think I'd still opt to vendoring one version (since that seems the intent of the library in the first place), but absent that I think yours is the better fix here. I'll close this as I'm not actively working on it/focus shifted away from seL4 for now.