-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(task): exclude inherited tasks from wildcard pattern matching #7850
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| #!/usr/bin/env bash | ||
| # Test that wildcard patterns (//...:task) do NOT match inherited tasks | ||
| # Only tasks explicitly defined in a subdirectory should match wildcards | ||
| # See: https://github.com/jdx/mise/discussions/6564#discussioncomment-15607613 | ||
|
|
||
| export MISE_EXPERIMENTAL=1 | ||
|
|
||
| # Create monorepo root with a "test" task | ||
| cat <<'TOML' >mise.toml | ||
| experimental_monorepo_root = true | ||
|
|
||
| [monorepo] | ||
| config_roots = ["packages/*"] | ||
|
|
||
| [tasks.test] | ||
| run = 'echo "root test"' | ||
|
|
||
| [tasks.root-only] | ||
| run = 'echo "root only task"' | ||
| TOML | ||
|
|
||
| # Create package-a WITH its own test task (should match //...:test) | ||
| mkdir -p packages/package-a | ||
| cat <<'TOML' >packages/package-a/mise.toml | ||
| [tasks.test] | ||
| run = 'echo "package-a test"' | ||
| TOML | ||
|
|
||
| # Create package-b WITHOUT its own test task (should NOT match //...:test due to inheritance) | ||
| mkdir -p packages/package-b | ||
| cat <<'TOML' >packages/package-b/mise.toml | ||
| [tasks.build] | ||
| run = 'echo "package-b build"' | ||
| TOML | ||
|
|
||
| # Create package-c WITH its own test task (should match //...:test) | ||
| mkdir -p packages/package-c | ||
| cat <<'TOML' >packages/package-c/mise.toml | ||
| [tasks.test] | ||
| run = 'echo "package-c test"' | ||
| TOML | ||
|
|
||
| mise trust --all | ||
|
|
||
| # --- Test 1: Wildcard should only match explicitly defined tasks --- | ||
| echo "=== Test 1: Wildcard should only match package-a and package-c (not package-b) ===" | ||
|
|
||
| # Should include package-a and package-c (they define test explicitly) | ||
| assert_contains "mise run '//packages/...:test'" "package-a test" | ||
| assert_contains "mise run '//packages/...:test'" "package-c test" | ||
|
|
||
| # Should NOT include package-b (its test is inherited from root) | ||
| assert_not_contains "mise run '//packages/...:test'" "root test" | ||
|
|
||
| # --- Test 2: Explicit paths still work for inherited tasks --- | ||
| echo "=== Test 2: Explicit path should work for inherited task ===" | ||
| assert_contains "mise run //packages/package-b:test" "root test" | ||
|
|
||
| # --- Test 3: Root wildcard with root task should still work --- | ||
| echo "=== Test 3: Root task still visible ===" | ||
| assert_contains "mise run //:test" "root test" | ||
|
|
||
| # --- Test 4: Task that only exists at root should NOT match //packages/...: wildcard --- | ||
| echo "=== Test 4: Root-only task should not match subdirectory wildcard ===" | ||
| # This should fail with "task not found" since no package explicitly defines root-only | ||
| assert_fail "mise run '//packages/...:root-only'" | ||
|
|
||
| # --- Test 5: Inherited tasks still visible in task list (with --all) --- | ||
| echo "=== Test 5: Inherited tasks visible in task list ===" | ||
| # Inherited tasks should still be listed | ||
| assert_contains "mise tasks --all" "//packages/package-b:test" | ||
| assert_contains "mise tasks --all" "//packages/package-b:root-only" | ||
|
|
||
| # --- Test 6: Circular dependency should NOT occur --- | ||
| # This was the original bug: root defines test = { depends = ['//...:test'] } | ||
| # which would create a cycle through inherited tasks | ||
| echo "=== Test 6: No circular dependency with wildcard depends ===" | ||
| cat <<'TOML' >mise.toml | ||
| experimental_monorepo_root = true | ||
|
|
||
| [monorepo] | ||
| config_roots = ["packages/*"] | ||
|
|
||
| [tasks.test] | ||
| depends = ['//packages/...:test'] | ||
| run = 'echo "root test complete"' | ||
| description = 'run all tests' | ||
| TOML | ||
|
|
||
| # This should work without circular dependency error | ||
| assert_contains "mise run //:test" "package-a test" | ||
| assert_contains "mise run //:test" "package-c test" | ||
| assert_contains "mise run //:test" "root test complete" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,6 +234,10 @@ pub struct Task { | |
| pub hide: bool, | ||
| #[serde(default)] | ||
| pub global: bool, | ||
| /// Whether this task was inherited from a parent config in a monorepo | ||
| /// (vs explicitly defined at this path). Used to filter wildcard matches. | ||
| #[serde(skip)] | ||
| pub inherited: bool, | ||
| #[serde(default)] | ||
| pub raw: bool, | ||
| #[serde(default)] | ||
|
|
@@ -1070,9 +1074,12 @@ fn match_tasks_with_context( | |
| parent_task: Option<&Task>, | ||
| ) -> Result<Vec<Task>> { | ||
| let resolved_pattern = resolve_task_pattern(&td.task, parent_task); | ||
| // When using wildcard patterns (containing "..."), filter out inherited tasks | ||
| let filter_inherited = resolved_pattern.contains("..."); | ||
|
||
| let matches = tasks | ||
| .get_matching(&resolved_pattern)? | ||
| .into_iter() | ||
| .filter(|t| !filter_inherited || !t.inherited) | ||
| .map(|t| { | ||
| let mut t = (*t).clone(); | ||
| t.args = td.args.clone(); | ||
|
|
@@ -1139,6 +1146,7 @@ impl Default for Task { | |
| dir: None, | ||
| hide: false, | ||
| global: false, | ||
| inherited: false, | ||
| raw: false, | ||
| sources: vec![], | ||
| outputs: Default::default(), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -288,6 +288,11 @@ pub async fn get_task_lists( | |
| let cur_tasks = tasks_with_aliases | ||
| .get_matching(&t)? | ||
| .into_iter() | ||
| // When using wildcard patterns (containing "..."), filter out inherited tasks | ||
| // to avoid matching tasks that exist only due to inheritance from parent configs. | ||
| // This prevents issues like circular dependencies when root defines | ||
| // `test = { depends = ['//...:test'] }` and subdirs inherit that task. | ||
| .filter(|task| !t.contains("...") || !task.inherited) | ||
|
||
| .cloned() | ||
| .collect_vec(); | ||
| if cur_tasks.is_empty() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tasks without a
config_root(whentask.config_rootisNone) will haveinheritedremain asfalseby default. If tasks can legitimately haveNoneas theirconfig_rootand should be considered inherited in certain scenarios, this logic may incorrectly classify them. Consider whether tasks withNoneconfig_root should have explicit handling.