Skip to content

Preserve the value of self.modules_repo across nested calls#3881

Merged
muffato merged 3 commits into
devfrom
modules_repo_nested_crossorg_subworkflows
Nov 14, 2025
Merged

Preserve the value of self.modules_repo across nested calls#3881
muffato merged 3 commits into
devfrom
modules_repo_nested_crossorg_subworkflows

Conversation

@muffato

@muffato muffato commented Nov 7, 2025

Copy link
Copy Markdown
Member

Fixes #3876

I think this is all that is needed to fix #3876. I ran the four commands listed in the issue and pipelines lint now works.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@muffato muffato force-pushed the modules_repo_nested_crossorg_subworkflows branch from 21598a1 to fb80c7d Compare November 7, 2025 08:42
@codecov

codecov Bot commented Nov 7, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.41%. Comparing base (c9d3b76) to head (eae5c97).
⚠️ Report is 14 commits behind head on dev.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@prototaxites

prototaxites commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

I have tested it, and it does seem to work correctly. The modules.json file is correct and subworkflow installation and linting work without failure.

@muffato muffato self-assigned this Nov 7, 2025
@muffato muffato requested a review from prototaxites November 7, 2025 09:10
mashehu
mashehu approved these changes Nov 7, 2025
View reviewed changes

@mashehu mashehu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice, could we have a test for this, so we don't run into the same thing in the future?

@muffato muffato changed the title Preserve the value of self.modules_repo across nested calls [do not merge] Preserve the value of self.modules_repo across nested calls Nov 7, 2025
mirpedrol
mirpedrol approved these changes Nov 7, 2025
View reviewed changes

@mirpedrol mirpedrol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!
I agree with @mashehu that we should add a test for this before merging 🙂

@muffato muffato force-pushed the modules_repo_nested_crossorg_subworkflows branch from 7a4e274 to eae5c97 Compare November 11, 2025 01:02
@muffato

muffato commented Nov 11, 2025

Copy link
Copy Markdown
Member Author

Test added and passing.

Requires nf-core-test/modules#4 to be merged and the last commit to be removed before merging

@muffato muffato changed the title [do not merge] Preserve the value of self.modules_repo across nested calls Preserve the value of self.modules_repo across nested calls Nov 11, 2025
@prototaxites

prototaxites commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Hi all, wondering if we would be able to squeeze this one into 3.5.0 as well? I am encountering this bug working on my pipeline and it would be good to be able to install the required subworkflows correctly!

@mashehu mashehu added this to the 3.5.0 milestone Nov 13, 2025
@muffato

muffato commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

Thanks. nf-core-test/modules#4 should probably be approved too (although technically main isn't protected there, and I could merge it without a review)

@muffato muffato force-pushed the modules_repo_nested_crossorg_subworkflows branch from eae5c97 to 6caa62e Compare November 14, 2025 11:50
@muffato muffato enabled auto-merge November 14, 2025 11:54
@muffato muffato linked an issue Nov 14, 2025 that may be closed by this pull request
Nested cross-organisational subworkflows cause tools failure and clashing namespaces #3876
Closed
@muffato muffato merged commit e3acb62 into dev Nov 14, 2025
112 of 113 checks passed
@muffato muffato deleted the modules_repo_nested_crossorg_subworkflows branch November 14, 2025 11:57
@muffato

muffato commented Nov 14, 2025

Copy link
Copy Markdown
Member Author

Thank you !

This was referenced Dec 10, 2025
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mashehu mashehu mashehu approved these changes

@mirpedrol mirpedrol mirpedrol approved these changes

@prototaxites prototaxites Awaiting requested review from prototaxites

Assignees

@muffato muffato

Labels

None yet

Projects

None yet

Milestone

3.5.0

Development

Successfully merging this pull request may close these issues.

Nested cross-organisational subworkflows cause tools failure and clashing namespaces

5 participants

@muffato @prototaxites @mashehu @mirpedrol @nf-core-bot