Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci_find*: include sub and parent dirs #1130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+41 −8
Open

ci_find*: include sub and parent dirs #1130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions planemo/ci.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import print_function

import copy
import glob
import math
import os

Expand All @@ -11,6 +12,10 @@
from planemo import git
from planemo import io
from planemo.shed import REPO_METADATA_FILES
from planemo.tools import (
is_tool_load_error,
yield_tool_sources_on_paths
)


def filter_paths(ctx, raw_paths, path_type="repo", **kwds):
Expand All @@ -26,14 +31,9 @@ def filter_paths(ctx, raw_paths, path_type="repo", **kwds):
if changed_in_commit_range is not None:
diff_files = git.diff(ctx, cwd, changed_in_commit_range)
if path_type == "repo":
diff_dirs = set(os.path.dirname(p) for p in diff_files)
diff_paths = set()
for diff_dir in diff_dirs:
diff_path = metadata_file_in_path(diff_dir)
if diff_path:
diff_paths.add(diff_path)
diff_paths = changed_repos(diff_files)
else:
diff_paths = diff_files
diff_paths = changed_tools(diff_files, ctx, cwd)

unique_paths = set(os.path.relpath(p, cwd) for p in raw_paths)
if diff_paths is not None:
Expand All @@ -55,6 +55,38 @@ def filter_paths(ctx, raw_paths, path_type="repo", **kwds):
return chunked_paths


def changed_repos(diff_files):
diff_dirs = set(os.path.dirname(p) for p in diff_files)
diff_paths = set()
for diff_dir in diff_dirs:
new_diff_paths = set()
while diff_dir != "" and len(new_diff_paths) == 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

for sub_dir in glob.glob(diff_dir + "/**/", recursive=True):
diff_path = metadata_file_in_path(sub_dir)
if diff_path:
new_diff_paths.add(diff_path)
diff_dir = os.path.split(diff_dir)[0]
diff_paths |= new_diff_paths
return diff_paths


def changed_tools(diff_files, ctx, cwd):
diff_paths = set()
for diff_file in diff_files:
diff_dir = os.path.dirname(diff_file)
# search for tool files in each non-root parent*
new_diff_paths = set()
while diff_dir != '' and len(new_diff_paths) == 0:
for (tool_path, tool_source) in yield_tool_sources_on_paths(ctx, [diff_dir], recursive=True):
if is_tool_load_error(tool_source):
continue
new_diff_paths.add(tool_path)
diff_dir = os.path.split(diff_dir)[0]
diff_paths |= new_diff_paths
diff_paths = set(os.path.relpath(p, cwd) for p in diff_paths)
return diff_paths


def metadata_file_in_path(diff_dir):
while diff_dir:
for metadata_file in REPO_METADATA_FILES:
Expand Down
3 changes: 2 additions & 1 deletion planemo/options.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,8 @@ def filter_exclude_from_option():
def filter_changed_in_commit_option():
return planemo_option(
"--changed_in_commit_range",
help="Exclude paths unchanged in git commit range.",
help="Include only tools (resp. repositories) contained in (non-root)"
"directories that include a file that changed in the given commit range.",
)


Expand Down