ci_find*: include sub and parent dirs #1130
Conversation
bernt-matthias
changed the title
bernt-matthias
force-pushed
the
topic/sub-parent
branch
3 times, most recently
from
44d0c6a
to
2d7ab69
Compare
include also tools/repos with: - change of test data - change of supplementary scripts (this already was the case for ci_find_repos) - change of macros where the tools are in sub-directories (this failed for ci_find_repos if each tool has an individual shed file) - change of a single tool in a collection where the shed file is in the parent
bernt-matthias
force-pushed
the
topic/sub-parent
branch
from
2d7ab69
to
7fb3f5a
Compare
To clarify, the problem here is if planemo's working dir is e.g On the other hand I don't think the presence of |
This might also be a problem. But I thought more of the case when the working dir is the parent of
Then everything should be fine. |
| diff_paths = set() | ||
| for diff_dir in diff_dirs: | ||
| new_diff_paths = set() | ||
| while diff_dir != "" and len(new_diff_paths) == 0: |
There was a problem hiding this comment.
Maybe a bit more explanation on the potential problem:
With diff_dir != "" I ensure that glob.glob(diff_dir + "/**/" is not called for the working dir. This is needed because otherwise a change of a file in the repository root would lead to the inclusion of all tools / repos.
Hence calling from outside of the repo this would not work .. BUT this is irrelevant, because git diff will fail before that.
Calling from subdirs would lead to ignoring changes in this dir.
Maybe we can assert the assumption by comparing the output of git rev-parse --show-toplevel with the working dir.
There was a problem hiding this comment.
I think we could avoid going into these details maybe if you added a new flag for the recursive behavior (maybe --recursive) where in the help text you can list the limitations and how this is supposed to be used ?
There was a problem hiding this comment.
That said if this lets you make progress on the planemo action I'm happy to merge and iterate on it if there are issues.
There was a problem hiding this comment.
That's fine. I can easily use test cases that include changes in the tool xml.
Just need to think of something different than --recursive, because this does not capture the essence. Maybe --extended_git_diff.
fixes #1129 and more:
include also tools/repos with:
currently this will "fail" if the working dir of the planemo run is not the root of the repository: then changing for instance the README of the repository would lead to the inclusion of the all tools/repos in the repository. We could check if its the root by checking if there is a
.gitdirectory?