Open JWCS opened 1 year ago
This is a vexing behavior and I certainly sympathize with your frustration, but I'm not totally convinced that having RMF internally interpret a leading ~
as the home directory is the right move. I say that because the use of a leading ~
to mean a home directory is a platform-specific behavior. Once the path information is moving around inside of RMF, it's simply a string, and it's not obvious to me that RMF should take on the role of interpreting that string in special ways. It would technically be valid (albeit pathological) for a user to create a directory with the name ~
and use it in a relative path name.
In general if I need to refer to the home directory in a path name that I'm passing into an application, I'll use the $HOME
environment variable. Inside of a launch file you can use environment variables in launch arguments like this:
<arg name="my_file_path" value="$(env HOME)/my/directory" />
So my current stance on this is "won't fix" but I'll leave this issue open for now in case people think my stance is unreasonable and want to insist that the behavior is changed.
Oh, I agree, I don't think you should add any hacky parsing to rmf for paths. I think it'd be great if one of the three happened:
/**
* Loads the input file as a single YAML document.
*
* @throws {@link ParserException} if it is malformed.
* @throws {@link BadFile} if the file cannot be loaded.
*/
YAML_CPP_API Node LoadFile(const std::string& filename);
I'd be willing to make a PR, if there's a way you prefer.
I definitely like option (2). It's always good to be loud and clear about failure modes. Option (1) would be good to do in addition to (2).
If you're willing to open a PR for that I'll be more than happy to give it a quick review and merge.
https://github.com/open-rmf/rmf_ros2/blob/3c17991d6cd003a75986676cc7c0134b8212138b/rmf_fleet_adapter/src/rmf_fleet_adapter/agv/parse_graph.cpp#L30
Passing in a valid path, of the form
~/path/to/nav_graph/0.yaml
to parse_graph resulted in a raised exception from yaml-cpp that no file was found. This was tested on self-compiled main, ubuntu 20, ros foxy, using the full_control fleet adapter launch file. The solution/realization for me was that yaml-cpp was not performing tilde expansion, so a form of/home/myself/path/to/nav_graph/0.yaml
was sufficient.The main problem is the cryptic-ness of not accepting a shell-valid filepath, when it may get passed in via the shell cmdline args. I've only tested/suffered the tilde expansion, but symbolic links and relative paths are also common cmdline path inputs.
Maybe this could be short-term fixed with either a comment by the line, though a cleaner long-term interface would resolve the filepath before passing to yaml-cpp, &/or catching the raised no-file-found to give a more specific error msg.
On a deeper note, this seems symptomatic of relying on yaml-cpp to do input (file-format) sanitization, but not doing input (path) sanitization to yaml-cpp. If there's any other deep-ish files that take raw user input, identifying them and listing them somewhere so that they can be checked for appropriate input sanitization would be nice project enhancement.