Fix directory file dependency resolution#29
Conversation
gliargovas
added
work in progress
priority
|
Right now I am getting a strange fail in test 6 because of a file in the output dir that normally shouldn't exist. It is never referenced in any log trace, and when run independently the test passes, so I guess it has to do with the test script itself. |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
angelhof
left a comment
angelhof
left a comment
There was a problem hiding this comment.
Looks good! You can merge it if you add a brief comment regarding the dir/file resolution
| next_non_speculated.append(node_id) | ||
| return list(next_non_speculated) | ||
|
|
||
| def has_dir_file_dependency(self, first_cmd_set, second_cmd_set): |
There was a problem hiding this comment.
Could you add a comment explaining how is the check exactly supposed to work? If I understand correctly, if the first command creates has a directory in its write set (meaning that it creates it?) and the second reads from a file in this directory (but not its children directories), then there is a conflict? Shouldn't the second cmd write set also be checked? In any case, I think a comment above the function explaining what is it supposed to check and implement would be great :)
There was a problem hiding this comment.
You are right, this implementation was incorrect. Changed it to examine all relevant suffixes. Also, discovered some limitations with the parsing and directories. Maybe it's inevitable to switch to a more tight integration with the Riker trace, but at a later PR (I think this one has already too many changes).
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
|
OS:ubuntu-22.04 |
2 participants
No description provided.