Skip to content

ci_find*: include sub and parent dirs - #1130

Merged
mvdbeek merged 4 commits into
galaxyproject:masterfrom
bernt-matthias:topic/sub-parent
Sep 21, 2026
Merged

mvdbeek merged 4 commits into
galaxyproject:masterfrom
bernt-matthias:topic/sub-parent

Conversation

@bernt-matthias

@bernt-matthias bernt-matthias commented Jan 17, 2021 •

Copy link
Copy Markdown
Collaborator

fixes #1129 and more:

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

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 .git directory?

@bernt-matthias bernt-matthias changed the title ci_find*: include some sub/parent dirs ci_find*: include sub and parent dirs Jan 17, 2021
@bernt-matthias
bernt-matthias force-pushed the topic/sub-parent branch 3 times, most recently from 44d0c6a to 2d7ab69 Compare January 17, 2021 13:55
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
@mvdbeek

mvdbeek commented Jan 18, 2021

Copy link
Copy Markdown
Member

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 .git directory?

To clarify, the problem here is if planemo's working dir is e.g tools-iuc/tools/minimap2, and not tools-iuc/ ?
I think that's fine, planemo ci_find* is really meant for running within a CI system, and I think it's reasonable to expect that the working directory is the repository root.

On the other hand I don't think the presence of .git is a good proxy to check for this.

@bernt-matthias

Copy link
Copy Markdown
Collaborator Author

the problem here is if planemo's working dir is e.g tools-iuc/tools/minimap2, and not tools-iuc/ ?

This might also be a problem. But I thought more of the case when the working dir is the parent of tools-iuc/.

I think that's fine, planemo ci_find* is really meant for running within a CI system, and I think it's reasonable to expect that the working directory is the repository root.

Then everything should be fine.

Comment thread planemo/ci.py
jmchilton and others added 3 commits September 20, 2026 08:59
Merge latest master to get fresh CI results on this long-open PR.

Conflict in planemo/ci.py: master reformatted the inline repo-diff loop
that this branch had extracted into changed_repos(); kept this branch's
call to the helper.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch predates planemo's adoption of black and isort, so the lines
it introduced fail the lint job. Reformatted with the versions pinned in
.pre-commit-config.yaml (black 26.1.0, isort 5.13.2); only lines added by
this branch are affected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Implements the change mvdbeek asked for and bernt-matthias agreed to in the
2021 review: the new changed-file -> tool/repository mapping becomes opt-in,
with its limitations listed in the flag's help text, instead of changing the
default for every ci_find* consumer. Without the flag filter_paths behaves
exactly as master does.

Also fixes three defects in the recursive path, each covered by a new test:

- changed_repos_extended returned slash-suffixed directories from glob, which
  can never match the relpath-normalized set filter_paths intersects against.
  Any shed repository without a subdirectory was silently dropped.
- changed_tools_extended passed directories straight to
  yield_tool_sources_on_paths, which raises when a path does not exist. A
  commit range that deleted a directory crashed the command.
- a tool file that failed to parse made its directory look empty, so the
  search escalated to the parent and selected every unrelated sibling. Tool
  files now stop the search whether or not they load, and load errors are
  reported rather than swallowed.

Ancestor directories are searched shallowly; only the directory a change
actually lives in is searched recursively. Directories are deduplicated and
scans memoized, matching what changed_repos already did.

Fixes galaxyproject#1129.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jmchilton

Copy link
Copy Markdown
Member

From Claude:

PR 1130 now does what the 2021 review actually converged on: new behavior opt-in behind --extended_git_diff, default path provably identical to master, three defects fixed with tests, and a tests/test_ci.py that didn't exist before.

@mvdbeek
mvdbeek merged commit b8547ff into galaxyproject:master Sep 21, 2026
15 checks passed
@mvdbeek

mvdbeek commented Sep 21, 2026

Copy link
Copy Markdown
Member

Nice, i think that'll catch a few drift issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci_find_tools misses changed tools if tool XML is unchanged

3 participants