CTU-IIG / demos-sched

Scheduler for simulation of avionics multi-core workloads on Linux
GNU General Public License v3.0
1 stars 2 forks source link

Make paths in YAML configuration files relative to the config, not cwd #32

Closed MatejKafka closed 3 years ago

MatejKafka commented 3 years ago

This would make it possible to run the config file without worrying about current working directory.

Implementation: Probably simplest would be to set cwd to config directory before invoking the scheduled program through sh. This could however probably break some scenarios where user wants to run the launched program itself in a given directory, although that seems unusual, given the need to run the program through config file.

wentasah commented 3 years ago

This would be also problematic, when using shell process redirection as a config path e.g. demos-sched -c <(generate-config). Process redirection is replaced with something like /dev/....

I think that it would be better to extend the configuration format to allow specifying the working directory. Something like:

cwd: ./
partitions: ...
windows: ...

When cwd is present, the directory would be changed as you propose. If the value is relative path, it would be relative to the configuration file.

MatejKafka commented 3 years ago

Is there any reason to use process substitution (<(...)) when demos also supports -C for inline string configuration? Also, even if so, I think we can safely conclude that if the config file is in /dev/fd directory, it is from process substitution, as I hope no-one is mad enough to store normal files under /dev/. :)

My issue with adding another parameter to the config format is that then the users have to know about it, and by default, they will encounter this pitfall, which doesn't seem like a good user experience to me. Even for more experienced users, it's one more thing to deal with whenever writing a new configuration file.

I think we should do the sane thing by default, and let the user opt-out if he needs to do something custom. The opt-out seems to make more sense inside the config file than as an extra command-line switch, as whether it's cwd-dependent is a property of the configuration, not the invocation.

Proposed implementation:

For inline configuration string (-C parameter): Keep current working directory and trust that the user knows what he's doing. For configuration file (-c parameter): If the configuration file contains no-cwd: yes (or something similar) parameter, keep working directory and continue as normal. Otherwise, check if it's /dev/fd/<n>:

wentasah commented 3 years ago
MatejKafka commented 3 years ago

Agree with first 2 points.

Ad point 3: I'd either add a switch to turn this check off, or use a warning instead of an error - user might want to intentionally use a pipe while setting cwd, however unlikely. The only scenario where I can imagine a warning being "unsafe" (causing an unrecoverable issue) is if the user tried to run something like rm -rf . inside demos with config file passed through a pipe, which I really hope no-one would try.

In the more common case where user tries to pipe the config file and the configuration fails, he will hopefully discover the warning fairly quickly in the output.

wentasah commented 3 years ago

OK, a warning would be sufficient.