-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Only run JIT formatting job if there are JIT changes #61632
Only run JIT formatting job if there are JIT changes #61632
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
@dotnet/jit-contrib |
@@ -99,6 +99,9 @@ jobs: | |||
- src/coreclr/vm/* | |||
exclude: | |||
- '*' | |||
- subset: coreclr_jit | |||
include: | |||
- src/coreclr/jit/* |
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.
I presume this means all files in the jit
directory and all subdirectories of the jit
directory?
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.
looks like it.
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.
Yes. If you want to know more about how this works, here is the summary of how it works:
runtime/eng/pipelines/evaluate-changed-paths.sh
Lines 2 to 17 in d471a03
: ' | |
Scenarios: | |
1. exclude paths are specified | |
Will include all paths except the ones in the exclude list. | |
2. include paths are specified | |
Will only include paths specified in the list. | |
3. exclude + include: | |
1st we evaluate changes for all paths except ones in excluded list. If we can not find | |
any applicable changes like that, then we evaluate changes for incldued paths | |
if any of these two finds changes, then a variable will be set to true. | |
In order to consume this variable in a yaml pipeline, reference it via: $[ dependencies.<JobName>.outputs["<StepName>_<subset>.containschange"] ] | |
Example: | |
-difftarget ''HEAD^1'' -subset coreclr -includepaths ''src/libraries/System.Private.CoreLib/*'' -excludepaths ''src/libraries/*+src/installer/*'' | |
This example will include ALL path changes except the ones under src/libraries/*!System.Private.CoreLib/* |
And this is the logic that takes care of that:
runtime/eng/pipelines/evaluate-changed-paths.sh
Lines 132 to 183 in d471a03
probePaths() { | |
local _subset=$subset_name | |
local _azure_devops_var_name=$azure_variable | |
local exclude_path_string="" | |
local include_path_string="" | |
local found_applying_changes=false | |
if [[ ${#exclude_paths[@]} -gt 0 ]]; then | |
echo "" | |
echo "******* Probing $_subset exclude paths *******"; | |
for _path in "${exclude_paths[@]}"; do | |
echo "$_path" | |
if [[ "$exclude_path_string" == "" ]]; then | |
exclude_path_string=":!$_path" | |
else | |
exclude_path_string="$exclude_path_string :!$_path" | |
fi | |
done | |
if ! probePathsWithExitCode $exclude_path_string; then | |
found_applying_changes=true | |
printMatchedPaths $exclude_path_string | |
fi | |
fi | |
if [[ $found_applying_changes != true && ${#include_paths[@]} -gt 0 ]]; then | |
echo "" | |
echo "******* Probing $_subset include paths *******"; | |
for _path in "${include_paths[@]}"; do | |
echo "$_path" | |
if [[ "$include_path_string" == "" ]]; then | |
include_path_string=":$_path" | |
else | |
include_path_string="$include_path_string :$_path" | |
fi | |
done | |
if ! probePathsWithExitCode $include_path_string; then | |
found_applying_changes=true | |
printMatchedPaths $include_path_string | |
fi | |
fi | |
if [[ $found_applying_changes == true ]]; then | |
echo "" | |
echo "Setting pipeline variable $_azure_devops_var_name=true" | |
Write-PipelineSetVariable -name $_azure_devops_var_name -value true | |
else | |
echo "" | |
echo "No changed files for $_subset" | |
fi | |
} |
This looks right to me but someone like @safern should OK it. |
No description provided.