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

Jump to bottom

fix(tasks): ensure task precedence is preserved during loading #1600

Closed
wants to merge 1 commit into from
Closed

fix(tasks): ensure task precedence is preserved during loading #1600

wants to merge 1 commit into from

Conversation

Ajpantuso
Copy link
Contributor

Summary

Fixes #1593

This initial call to rev() ensures that lower precedence config files are processed first and consequently their tasks and aliases are inserted into the resulting Map first and overridden by high precedence tasks.

The additional call to rev() removed here inverted this behavior.

@jdx
Copy link
Owner

jdx commented Feb 3, 2024

given the test failure I think you removed the wrong one. That logic is so complicated I'm not sure though.

@Ajpantuso Ajpantuso force-pushed the apantuso/ensure_task_precedence branch from 4bf0dec to dbbab04 Compare February 3, 2024 02:41
@Ajpantuso
Copy link
Contributor Author

Lolol I see what the problem is:

Created an e2e script with just the following:

MISE_EXPERIMENTAL=1 mise config
MISE_EXPERIMENTAL=1 mise tasks

This is the output:

./e2e/run_test ./e2e/test_debug
::group::E2E ./e2e/test_debug
Path                                             Plugins          
~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml     tiny             
~/dev/github.com/jdx/mise/e2e/.e2e-tool-versions shellcheck, shfmt
~/dev/github.com/jdx/mise/e2e/.node-version      node             
~/dev/github.com/jdx/mise/.node-version          node             
Name        Description                 Source                                         
build                                   ~/dev/github.com/jdx/mise/e2e/.mise/tasks/build
configtask                              ~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml   
filetask    This is a test build script ~/dev/github.com/jdx/mise/.mise/tasks/filetask 
lint                                    ~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml   
test                                    ~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml

And after removing ~/dev/github.com/jdx/mise/.node-version:

./e2e/run_test ./e2e/test_bang
::group::E2E ./e2e/test_bang
Path                                             Plugins          
~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml     tiny             
~/dev/github.com/jdx/mise/e2e/.e2e-tool-versions shellcheck, shfmt
~/dev/github.com/jdx/mise/e2e/.node-version      node             
Name        Description                 Source                                            
build                                   ~/dev/github.com/jdx/mise/e2e/.mise/tasks/build   
configtask                              ~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml      
filetask    This is a test build script ~/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask
lint                                    ~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml      
test                                    ~/dev/github.com/jdx/mise/e2e/.e2e.mise.toml      
./e2e/test_bang: 0s

So I'll need to get e2e to ignore the legacy .node-version in the project root since that is generating the default task includes directories which rightfully should have higher precedence.

@Ajpantuso
Copy link
Contributor Author

Soft-blocking this on #1612 since difficult to identify if current e2e suite is correctly failing or not.

@Ajpantuso Ajpantuso mentioned this pull request Feb 3, 2024
Merged
@Ajpantuso Ajpantuso marked this pull request as draft February 5, 2024 13:43
@jdx
Copy link
Owner

jdx commented Feb 5, 2024

So I'll need to get e2e to ignore the legacy .node-version in the project root since that is generating the default task includes directories which rightfully should have higher precedence.

Related to this, I'm unsure if my strategy of using the fact there is a config file is even a good idea. I could see a user wanting to have tasks without a config file at all and the current solution doesn't allow for that.

@jdx
Copy link
Owner

jdx commented Feb 7, 2024

So I'll need to get e2e to ignore the legacy .node-version in the project root since that is generating the default task includes directories which rightfully should have higher precedence.

looking at this a bit deeper, I don't understand. If both ~/src/mise and ~/src/mise/e2e define a task, then the one in ~/src/mise/e2e should override what is in ~/src/mise.

@Ajpantuso
Copy link
Contributor Author

Yeah it doesn't really make sense so here is some debug

Debug:

Original

# Initial ordering
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e-tool-versions"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.node-version"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.config/mise/config.toml"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/.node-version"
# After first rev() call
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.config/mise/config.toml"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/.node-version"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/.mise/tasks/filetask"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.node-version"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e-tool-versions"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
# After second rev() call
[src/config/mod.rs:280] &n = "test"
[src/config/mod.rs:281] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:280] &n = "lint"
[src/config/mod.rs:281] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:280] &n = "configtask"
[src/config/mod.rs:281] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:280] &n = "filetask"
[src/config/mod.rs:281] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/.mise/tasks/filetask"
[src/config/mod.rs:280] &n = "build"
[src/config/mod.rs:281] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:280] &n = "filetask"
[src/config/mod.rs:281] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"

Removing last rev() call:

[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e-tool-versions"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.node-version"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/.node-version"
[src/config/mod.rs:217] cf.get_path() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.config/mise/config.toml"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.config/mise/config.toml"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/.node-version"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/.mise/tasks/filetask"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.node-version"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e-tool-versions"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:250] cf.get_path().to_path_buf() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:251] task.clone() = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:279] &n = "filetask"
[src/config/mod.rs:280] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/filetask"
[src/config/mod.rs:279] &n = "build"
[src/config/mod.rs:280] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.mise/tasks/build"
[src/config/mod.rs:279] &n = "filetask"
[src/config/mod.rs:280] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/.mise/tasks/filetask"
[src/config/mod.rs:279] &n = "configtask"
[src/config/mod.rs:280] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:279] &n = "lint"
[src/config/mod.rs:280] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"
[src/config/mod.rs:279] &n = "test"
[src/config/mod.rs:280] &t.config_source = "/home/apantuso/dev/github.com/jdx/mise/e2e/.e2e.mise.toml"

Seems like the issue based on what the OP described is more due to the root config being considered higher precedence than the project config.

@Ajpantuso
Copy link
Contributor Author

Closing in favor of #1625

@Ajpantuso Ajpantuso closed this Feb 7, 2024
@jdx
Copy link
Owner

jdx commented Feb 7, 2024

I did some work in #1625 to refactor and improve this. A test is failing I need to sort out though.

@Ajpantuso Ajpantuso deleted the apantuso/ensure_task_precedence branch February 7, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tasks are overwritten by inherited configurations
2 participants
@Ajpantuso @jdx