Update the toolinfo of WitnessMap for version 0.0.2-dev #1124
Conversation
|
Do you need to be able to execute old WitnessMap versions with BenchExec? If not, you can remove the outdated code. |
There was a problem hiding this comment.
Bug-fix releases should increase the 3rd number, feature additions (like this) should increase the 2nd number of the version number. (-dev releases should not deserve an own Tool-Info Module.)
Backwards compatibility is required, but not for versions from a day ago that are not yet widely in use.
I would say you should rewrite this once 1.0 is released, and then only support that version and remove all other code.
Sounds good, I will do the required changes once the release of version 1.0 is done |
|
@PhilippWendler Thanks for your comment also, which I did not see because I took so long to write mine. |
|
@dbeyer This should not have been merged. For example, besides introducing unnecessary complexity, there is also the problem that it does not cache the version. |
|
The rule for backwards compatibility of tool-info modules is that if a tool-info module was part of a BenchExec release, then future versions of BenchExec should aim at supporting all the tool versions that the older BenchExec supported as well. So right now we have no expectation of backwards compatibility, but as soon as this code gets into a BenchExec release, it should be kept there. The version number of the tool does not really has an influence on this. So if you want to get rid of code for supporting old versions, then do so before the next BenchExec release, which is at the latest tomorrow morning. |
We needed it merged for a documented pre-run (i.e., with a main-git-hash of BenchExec), All this code will be gone in a few hours. |
Yes, that part of my review was placed here although mentioned for WitnessMap. |
Sorry about that I though that your comment #1093 (comment) referred to the kind of backwards compatibility I implemented here. |
You are correct about this. But this is only relevant for released versions of BenchExec. |
Sorry about that, should be fixed by #1125 |
Thanks for the explanation! |
Currently WitnessMap only copied the witness from a validation benchmark file into the target directory. In general, this is insufficient, since the contents of the witness need to be adjusted to match those of the input task.
Version 0.0.2-dev of WitnessMap adds this support and the toolinfo needs to also be able to interact with it.