search

riscv-software-src / riscof

BSD 3-Clause "New" or "Revised" License
61 stars 39 forks source link

absolute paths in generated `config.ini` #51

Open jeras opened 2 years ago

jeras commented 2 years ago

The generated config.ini contains absolute paths, there are two issues with this:

  1. The file is not portable to a different folder without changing the paths from absolute to relative,
  2. If the file is committed to a public repo, some private information about the developer could be exposed (username).
jeras commented 2 years ago

After further reading I noticed: https://riscof.readthedocs.io/en/stable/inputs.html#file-path-specification The option 2 can cause confusion, for example, when both paths from 2 and 3 exist. I would argue option 3 should have a higher priority.

jeras commented 2 years ago

I now noticed: Hint: All paths provided by riscof are absolute and it is advised to always use absolute paths while executing/generating commands to avoid errors.

Please provide an explanation for the use of absolute paths, this is contrary to expectations of experienced developers and therefore requires proper arguments.

pawks commented 2 years ago

The intermediate files(such as database and test lists) are architected in a way that they can be manipulated/consumed by other programs as needed. A riscof run consists of 4 different phases - config check, database generation, testlist generation and test run on models. Each of the individual phases can be run independently and the generated outputs can be used as inputs to the next phase(after manipulation if necessary). Any change in the work directory or environment parameters might result in an incorrect run. To prevent this, relative paths are almost never used throughout riscof. All paths are turned into absolute at the start and maintained throughout the program.

jeras commented 2 years ago

So as I understand you, the hint is not referring to config.ini, but to how riscof interfaces with the user provided DUT code (simulator executable or makefile, ...).

I would still argue the generated config.ini should have relative paths.

There is a related question I did not ask yet, why are the same paths specified in the [RISCOF] section (keys ReferencePluginPath and DUTPluginPath) and inside plugin sections as pluginpath key. The duplication opens questions on which of the two paths would be used if they were different. If the two paths have different uses, this was not clear from the documentation.

Would it be possible to list multiple Reference/DUT plugins but only selecting a pair in the [RISCOF] section (all could be listed, but the unused ones would be commented out)?

pawks commented 2 years ago

So as I understand you, the hint is not referring to config.ini, but to how riscof interfaces with the user provided DUT code (simulator executable or makefile, ...).

Yes. It also includes the paths in the database files and testlists generated. I would still argue the generated config.ini should have relative paths.

I can see why this would add value. This can be added as an optional feature enabled by a boolean switch in the cli. There is a related question I did not ask yet, why are the same paths specified in the [RISCOF] section (keys ReferencePluginPath and DUTPluginPath) and inside plugin sections as pluginpath key. The duplication opens questions on which of the two paths would be used if they were different. If the two paths have different uses, this was not clear from the documentation.

The paths under the RISCOF node are for use inside riscof and are not available to the plugins themselves by default. Any values which need to be passed to the plugins should be passed via these nodes. Would it be possible to list multiple Reference/DUT plugins but only selecting a pair in the [RISCOF] section (all could be listed, but the unused ones would be commented out)?

Yes. It is possible to do so. Only the plugins mentioned as DUTPlugin and ReferencePlugin are loaded & used during a run but the number of nodes in the ini file is not limited by this.