Improve CI path-changes-based-triggers to work better (#79127)
authorAnkit Jain <radical@gmail.com>
Tue, 7 Feb 2023 20:49:22 +0000 (15:49 -0500)
committerGitHub <noreply@github.com>
Tue, 7 Feb 2023 20:49:22 +0000 (15:49 -0500)
 ## Background:

`eng/pipelines/common/evaluate-default-paths.yml` is used to specify named subsets of paths, and `evaluate-changed-paths.sh` decides which subsets have "changed files". And these subsets are used in conditions for the various jobs to determine when they should be run.

---
`evaluate-changed-paths.sh: Add support for include+exclude combined`

 ## Problem

This script currently supports `--include`, and `--exclude`
parameters to determine if we have any changed paths.

```
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.
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 included paths
if any of these two finds changes, then a variable will be set to true.
```

As described above, these two are evaluated exclusively.
For example:

```
include: [ '/a/b/*' ], exclude: [ '/a/b/d.txt' ]
```

- This would return true if there are changes:
  - `[ '/a/b/c.txt' ]` - caught by `include`
  - or `[ '/r/s.txt' ]` - caught by `exclude`

- but it would also return true for `[ '/a/b/d.txt' ]` because `include`
does not consider the `exclude` list.

 ## Solution:

- This commit adds a new `--combined` parameter which essentially supports
`$include - $exclude`. Thus allowing exact more constrained path
specification.

- It is passed in addition to `include`, and `exclude`.

Given:
```
include: [ '/a/b/*' ], exclude: [ '/a/b/d.txt' ]
```

- For the following path changes:
  - `[ '/a/b/c.txt' ]` - `true`
  - `[ '/r/s.txt' ]` - `false` because this does not fall in
    `$include - $exclude`
  - `[ '/a/b/d.txt' ]` - `false` - excluded in `$include - $exclude`

The implementation is trivially implemented by passing both the lists to
`git diff` which supports this already.

---

- Track subset name changes in the ymls
- Update `wasm` jobs to have tighter trigger conditions

- This results in wasm specific changes only triggering relevant wasm jobs. For example, if there are wasm debugger changes then only the wasm debugger jobs will be triggered. Or if there are only workload manifest[1] changes then Wasm.Build.Tests will be triggered only.

eng/pipelines/common/evaluate-default-paths.yml
eng/pipelines/common/evaluate-paths-job.yml
eng/pipelines/common/templates/wasm-build-only.yml
eng/pipelines/common/templates/wasm-build-tests.yml
eng/pipelines/common/templates/wasm-debugger-tests.yml
eng/pipelines/common/templates/wasm-library-tests.yml
eng/pipelines/common/templates/wasm-runtime-tests.yml
eng/pipelines/evaluate-changed-paths.sh
eng/pipelines/global-build.yml
eng/pipelines/runtime-linker-tests.yml

index dc01caf..fd0697b 100644 (file)
@@ -10,8 +10,11 @@ parameters:
         eng/testing/scenarios/BuildWasmAppsJobsList.txt
         eng/testing/tests.wasm.targets
         src/libraries/sendtohelix-wasm.targets
+        src/mono/mono/**/*wasm*
         src/mono/nuget/Microsoft.NET.Runtime.WebAssembly.Sdk/*
         src/mono/nuget/Microsoft.NET.Runtime.wasm.Sample.Mono/*
+        src/mono/nuget/Microsoft.NETCore.BrowserDebugHost.Transport/*
+        src/mono/nuget/Microsoft.NET.Workload*
         src/mono/sample/wasm/*
         src/mono/wasi/*
         src/mono/wasm/*
@@ -23,10 +26,25 @@ parameters:
     _wasm_pipelines: [
         eng/pipelines/**/*wasm*
     ]
+    _wasm_src_native: [
+        src/native/minipal/*
+        src/native/libs/CMakeLists.txt
+        src/native/libs/configure.cmake
+        src/native/libs/build*
+        src/native/libs/Common/*
+        src/native/libs/System.Globalization.Native/*
+        src/native/libs/System.IO.Compression.Native/*
+        src/native/libs/System.Native/*
+    ]
+
     # src/workloads is only used in runtime-official builds
     # where evaluate-paths is not used
     _always_exclude: [
         eng/pipelines/common/evaluate-default-paths.yml
+        '*.md'
+        LICENSE.TXT
+        PATENTS.TXT
+        THIRD-PARTY-NOTICES.TXT
         src/workloads/*
     ]
 
@@ -42,10 +60,6 @@ jobs:
       - src/native/libs/System.IO.Compression.Native/*
       exclude:
       - eng/Version.Details.xml
-      - '*.md'
-      - LICENSE.TXT
-      - PATENTS.TXT
-      - THIRD-PARTY-NOTICES.TXT
       - docs/*
       - src/installer/*
       - src/mono/*
@@ -71,10 +85,6 @@ jobs:
       - ${{ parameters._const_paths._always_exclude }}
 
       - eng/Version.Details.xml
-      - '*.md'
-      - LICENSE.TXT
-      - PATENTS.TXT
-      - THIRD-PARTY-NOTICES.TXT
       - docs/*
       - src/installer/*
       - src/coreclr/*
@@ -89,10 +99,6 @@ jobs:
     - subset: libraries
       exclude:
       - eng/Version.Details.xml
-      - '*.md'
-      - LICENSE.TXT
-      - PATENTS.TXT
-      - THIRD-PARTY-NOTICES.TXT
       - docs/*
       - src/installer/*
       - src/mono/*
@@ -114,21 +120,11 @@ jobs:
       include:
       - src/tools/illink/*
 
-    - subset: non_runtimetests
-      exclude:
-      - src/tests/*
-      - ${{ parameters._const_paths._wasm_pipelines }}
-      - ${{ parameters._const_paths._always_exclude }}
-
     - subset: installer
       include:
       - docs/manpages/*
       exclude:
       - eng/Version.Details.xml
-      - '*.md'
-      - LICENSE.TXT
-      - PATENTS.TXT
-      - THIRD-PARTY-NOTICES.TXT
       - docs/*
       - src/coreclr/*
       - src/mono/*
@@ -139,6 +135,7 @@ jobs:
       - eng/pipelines/coreclr/*
       - eng/pipelines/mono/*
       - eng/pipelines/libraries/*
+      - ${{ parameters._const_paths._wasm_specific_only }}
       - ${{ parameters._const_paths._wasm_pipelines }}
       - ${{ parameters._const_paths._always_exclude }}
 
@@ -161,10 +158,11 @@ jobs:
 
     #
     # ** WASM **
-    # Changes in *only* Wasm.Build.Tests, or debugger, are very self-contained,
-    # so we try to avoid triggering only those relevants tests
+    # Changes in *only* Wasm.Build.Tests, debugger, or runtime-tests are very
+    # self-contained, so we try to trigger only those relevants tests
     #
     - subset: wasmbuildtests
+      combined: true
       include:
       - eng/Version.Details.xml
       - eng/Versions.props
@@ -181,9 +179,7 @@ jobs:
       - src/mono/nuget/Microsoft.NET.Runtime.MonoTargets.Sdk/*
       - src/mono/nuget/Microsoft.NET.Runtime.WebAssembly.Sdk/*
       - src/mono/nuget/Microsoft.NET.Runtime.wasm.Sample.Mono/*
-      - src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Current.Manifest/*
-      - src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.net6.Manifest/*
-      - src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.net7.Manifest/*
+      - src/mono/nuget/Microsoft.NET.Workload*
       - src/mono/nuget/Microsoft.NETCore.BrowserDebugHost.Transport/*
       - src/mono/wasm/build/*
       - src/mono/wasm/emscripten-version.txt
@@ -191,10 +187,14 @@ jobs:
       - src/mono/wasm/runtime/*
       - src/mono/wasm/templates/*
       - src/mono/wasm/Wasm.Build.Tests/*
+      - ${{ parameters._const_paths._wasm_src_native }}
       - src/tasks/*
       - ${{ parameters._const_paths._wasm_pipelines }}
+      exclude:
+      - ${{ parameters._const_paths._always_exclude }}
 
     - subset: wasmdebuggertests
+      combined: true
       include:
       - eng/testing/ProvisioningVersions.props
       - src/libraries/System.Runtime.InteropServices/*
@@ -202,53 +202,47 @@ jobs:
       - src/mono/mono/*
       - src/mono/wasm/debugger/*
       - src/mono/wasm/runtime/*
+      - ${{ parameters._const_paths._wasm_src_native }}
       - ${{ parameters._const_paths._wasm_pipelines }}
-
-    - subset: any_other_than_wasm_wbt_dbg
       exclude:
-      - src/mono/wasm/Wasm.Build.Tests/*
-      - src/mono/wasm/debugger/*
-      - ${{ parameters._const_paths._wasm_pipelines }}
+      - src/mono/nuget/*
       - ${{ parameters._const_paths._always_exclude }}
 
-    - subset: wasm_wbt_or_dbg
+    # wasm/runtimetests need to be run
+    - subset: wasm_runtimetests
+      combined: true
       include:
-      - src/mono/wasm/Wasm.Build.Tests/*
+      - src/tests/*
+      - src/mono/*
+      - ${{ parameters._const_paths._wasm_src_native }}
+      exclude:
+      - src/mono/nuget/*
+      - src/mono/sample/*
+      - src/mono/tests/*
+      - src/mono/tools/*
+      - src/mono/wasi/*
       - src/mono/wasm/debugger/*
+      - src/mono/wasm/Wasm.Build.Tests/*
+      - ${{ parameters._const_paths._wasm_pipelines }}
+      - ${{ parameters._const_paths._always_exclude }}
 
-    - subset: wasm
+    # Wasm except Wasm.build.Tests, and debugger
+    - subset: wasm_specific_except_wbt_dbg
+      combined: true
       include:
       - ${{ parameters._const_paths._wasm_specific_only }}
       - ${{ parameters._const_paths._wasm_pipelines }}
       # other paths that should also trigger wasm jobs
       - src/mono/*
-
-    # libraries with some wasm specific code
-    - subset: wasm_libraries
-      include:
-      - src/libraries/Common/*
-      - src/libraries/System.Console/*
-      - src/libraries/System.Diagnostics.FileVersionInfo/tests/*
-      - src/libraries/System.IO.Compression/*
-      - src/libraries/System.IO.MemoryMappedFiles/*
-      - src/libraries/System.Net.Http/*
-      - src/libraries/System.Net.Mail/*
-      - src/libraries/System.Net.Primitives/*
-      - src/libraries/System.Net.WebClient/*
-      - src/libraries/System.Net.WebProxy/*
-      - src/libraries/System.Net.WebSockets.Client/*
-      - src/libraries/System.Net.WebSockets/*
-      - src/libraries/System.Runtime.InteropServices.JavaScript/*
-      - src/libraries/System.Runtime.InteropServices/*
-      - src/libraries/System.Runtime.Serialization.Formatters/*
-      - src/libraries/System.Security.Cryptography/*
-      - src/libraries/System.Text.Encodings.Web/*
-
-    # anything other than wasm-specific paths
-    - subset: non_wasm
       exclude:
-      - ${{ parameters._const_paths._wasm_specific_only }}
-      - ${{ parameters._const_paths._wasm_pipelines }}
+      - eng/testing/scenarios/BuildWasmAppsJobsList.txt
+      - eng/testing/workloads-testing.targets
+      - src/mono/mono/component/mini-wasm-debugger.c
+      - src/mono/wasm/debugger/*
+      - src/mono/wasm/Wasm.Build.Tests/*
+      - src/mono/nuget/Microsoft.NET.Runtime*
+      - src/mono/nuget/Microsoft.NET.Workload*
+      - src/mono/nuget/Microsoft.NETCore.BrowserDebugHost.Transport/*
       - ${{ parameters._const_paths._always_exclude }}
 
     # anything other than mono, or wasm specific paths
@@ -258,7 +252,6 @@ jobs:
       - ${{ parameters._const_paths._wasm_specific_only }}
       - ${{ parameters._const_paths._wasm_pipelines }}
       - ${{ parameters._const_paths._always_exclude }}
-
       - eng/testing/tests.mobile.targets
       - src/mono/*
       - src/tasks/AndroidAppBuilder/*
index 670fba6..64c3a12 100644 (file)
@@ -41,6 +41,8 @@ jobs:
           parameters:
             subsetName: ${{ path.subset }}
             arguments:
+            - ${{ if eq(path.combined, true) }}:
+              - --combined
             # The commit that we're building is always a merge commit that is merging into the target branch.
             # So the first parent of the commit is on the target branch and the second parent is on the source branch.
             - --difftarget HEAD^1
index b25cf85..f97e0ef 100644 (file)
@@ -26,11 +26,7 @@ jobs:
           or(
             eq(variables['wasmDarcDependenciesChanged'], true),
             eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
-            and(
-              eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm.containsChange'], true),
-              eq(dependencies.evaluate_paths.outputs['SetPathVars_non_runtimetests.containsChange'], true)),
-            eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_libraries.containsChange'], true),
-            eq(dependencies.evaluate_paths.outputs['SetPathVars_mono_excluding_wasm.containsChange'], true))
+            eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_specific_except_wbt_dbg.containsChange'], true))
           ]
     jobParameters:
       isExtraPlatforms: ${{ parameters.isExtraPlatformsBuild }}
index 3ade2d7..ff35e11 100644 (file)
@@ -34,11 +34,6 @@ jobs:
       nameSuffix: WasmBuildTests
       buildArgs: -s mono+libs+host+packs+libs.tests -c $(_BuildConfig) /p:ArchiveTests=true /p:TestWasmBuildTests=true /p:TestAssemblies=false /p:BrowserHost=$(_hostedOs)
       timeoutInMinutes: 180
-      # if !alwaysRun, then:
-      #   if this is runtime-wasm (isWasmOnlyBuild):
-      #     - then run only if it would not have run on default pipelines (based
-      #       on path changes)
-      #     - else run based on path changes
       condition: >-
         or(
           eq(variables['alwaysRunVar'], true),
index ff3b33f..994b169 100644 (file)
@@ -27,6 +27,7 @@ jobs:
         value: $[
           or(
             eq(variables['wasmDarcDependenciesChanged'], true),
+            eq(dependencies.evaluate_paths_outputs['DarcDependenciesChanged.Microsoft_DotNet_HotReload_Utils_Generator_BuildTool'], true),
             eq(dependencies.evaluate_paths.outputs['SetPathVars_wasmdebuggertests.containsChange'], true))
           ]
     jobParameters:
index 380a217..adfc43e 100644 (file)
@@ -27,25 +27,16 @@ jobs:
       # map dependencies variables to local variables
       - name: alwaysRunVar
         value: ${{ parameters.alwaysRun }}
+      # - wasm darc deps changed
+      # - any libs that can have wasm specific changes
+      # - any other wasm specific changes that are not wbt, or dbg
       - name: shouldRunOnDefaultPipelines
         value: $[
           or(
             eq(variables['wasmDarcDependenciesChanged'], true),
-            and(
-              ne(variables['onlyWBTOrDbgTestHaveChanges'], true),
-              or(
-                eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
-                eq(dependencies.evaluate_paths.outputs['SetPathVars_mono_excluding_wasm.containsChange'], true),
-                eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_libraries.containsChange'], true),
-                and(
-                  eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm.containsChange'], true),
-                  eq(dependencies.evaluate_paths.outputs['SetPathVars_non_runtimetests.containsChange'], true)))))
-          ]
-      - name: onlyWBTOrDbgTestHaveChanges
-        value:
-          and(
-            eq(dependencies.evaluate_paths.outputs.SetPathVars_wasm_wbt_or_dbg.containsChange, true),
-            ne(dependencies.evaluate_paths.outputs.SetPathVars_any_other_than_wasm_wbt_dbg.containsChange, true))
+            eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
+            eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_specific_except_wbt_dbg.containsChange'], true))
+         ]
       - name: _wasmRunSmokeTestsOnlyArg
         value: /p:RunSmokeTestsOnly=${{ eq(parameters.shouldRunSmokeOnly, true) }}
       - name: chromeInstallArg
index 7a9aa31..2808afd 100644 (file)
@@ -27,8 +27,7 @@ jobs:
         value: $[
           or(
             eq(variables['wasmDarcDependenciesChanged'], true),
-            eq(dependencies.evaluate_paths.outputs['SetPathVars_runtimetests.containsChange'], true),
-            eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm.containsChange'], true))
+            eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_runtimetests.containsChange'], true))
           ]
     jobParameters:
       testGroup: innerloop
@@ -37,12 +36,6 @@ jobs:
       runtimeVariant: monointerpreter
       buildArgs: -s mono+libs -c $(_BuildConfig)
       timeoutInMinutes: 180
-      # FIXME: will get triggered by only wbt/dbg changes
-      # if !alwaysRun, then:
-      #   if this is runtime-wasm (isWasmOnlyBuild):
-      #     - then run only if it would not have run on default pipelines (based
-      #       on path changes)
-      #     - else run based on path changes
       condition: >-
         or(
           eq(variables['alwaysRunVar'], true),
index b145f39..37b9e5a 100755 (executable)
@@ -6,6 +6,10 @@ Scenarios:
   2. include paths are specified
       Will only include paths specified in the list.
   3. exclude + include:
+     a. `--combined` is passed:
+      Will use the include paths, then use exclude paths on that set, and if that has any changes
+      then a variable will be set to true.
+     b. `--combined` is not passed (default):
       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 included paths
       if any of these two finds changes, then a variable will be set to true.
@@ -33,6 +37,7 @@ usage()
   echo "  --difftarget <value>       SHA or branch to diff against. (i.e: HEAD^1, origin/main, 0f4hd36, etc.)"
   echo "  --excludepaths <value>     Escaped list of paths to exclude from diff separated by '+'. (i.e: 'src/libraries/*+'src/installer/*')"
   echo "  --includepaths <value>     Escaped list of paths to include on diff separated by '+'. (i.e: 'src/libraries/System.Private.CoreLib/*')"
+  echo "  --combined                 Uses both exclude, and include paths"
   echo "  --subset                   Subset name for which we're evaluating in order to include it in logs"
   echo "  --azurevariable            Name of azure devops variable to create if change meets filter criteria"
   echo ""
@@ -59,6 +64,7 @@ include_paths=()
 subset_name=''
 azure_variable=''
 diff_target=''
+combined=false
 
 while [[ $# > 0 ]]; do
   opt="$(echo "${1/#--/-}" | tr "[:upper:]" "[:lower:]")"
@@ -81,6 +87,9 @@ while [[ $# > 0 ]]; do
       include_paths+=(${tmp[@]})
       shift
       ;;
+    -combined)
+      combined=true
+      ;;
     -subset)
       subset_name=$2
       shift
@@ -138,7 +147,7 @@ probePaths() {
 
   if [[ ${#exclude_paths[@]} -gt 0 ]]; then
     echo ""
-    echo "******* Probing $_subset exclude paths *******";
+    echo "******* Collecting $_subset exclude paths *******";
     for _path in "${exclude_paths[@]}"; do
       echo "$_path"
       if [[ -z "$exclude_path_string" ]]; then
@@ -147,16 +156,11 @@ probePaths() {
         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
+  if [[ ${#include_paths[@]} -gt 0 ]]; then
     echo ""
-    echo "******* Probing $_subset include paths *******";
+    echo "******* Collecting $_subset include paths *******";
     for _path in "${include_paths[@]}"; do
       echo "$_path"
       if [[ -z "$include_path_string" ]]; then
@@ -165,10 +169,34 @@ probePaths() {
         include_path_string="$include_path_string :$_path"
       fi
     done
+  fi
 
-    if ! probePathsWithExitCode $include_path_string; then
+  if [[ "$combined" == "true" ]]; then
+    # Try both include, and exclude
+    # finds all the changes in include files, then excludes the exclude files
+    local combined_path_string="$include_path_string $exclude_path_string"
+    echo "******* Probing $_subset combined paths *******";
+    if ! probePathsWithExitCode $combined_path_string; then
       found_applying_changes=true
-      printMatchedPaths $include_path_string
+      printMatchedPaths $combined_path_string
+    fi
+  else
+    # First try exclude
+    if [[ ${#exclude_paths[@]} -gt 0 ]]; then
+      echo "******* Probing $_subset exclude paths *******";
+      if ! probePathsWithExitCode $exclude_path_string; then
+        found_applying_changes=true
+        printMatchedPaths $exclude_path_string
+      fi
+    fi
+
+    # if no changes found, then try include
+    if [[ $found_applying_changes != true && ${#include_paths[@]} -gt 0 ]]; then
+      echo "******* Probing $_subset include paths *******";
+      if ! probePathsWithExitCode $include_path_string; then
+        found_applying_changes=true
+        printMatchedPaths $include_path_string
+      fi
     fi
   fi
 
index 1e47945..a13286c 100644 (file)
@@ -176,3 +176,5 @@ extends:
             extraStepsParameters:
               name: SourceBuildPackages
             timeoutInMinutes: 95
+            condition:
+                eq(dependencies.evaluate_paths.outputs['SetPathVars_non_mono_and_wasm.containsChange'], true)
index 698d0f8..3b965ac 100644 (file)
@@ -121,8 +121,8 @@ extends:
             condition:
               or(
                 eq(variables['isRollingBuild'], true),
-                eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_libraries.containsChange'], true),
-                eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm.containsChange'], true),
+                eq(dependencies.evaluate_paths.outputs['SetPathVars_libraries.containsChange'], true),
+                eq(dependencies.evaluate_paths.outputs['SetPathVars_wasm_specific_except_wbt_dbg.containsChange'], true),
                 eq(dependencies.evaluate_paths.outputs['DarcDependenciesChanged.Microsoft_NET_ILLink_Tasks'], true))
             extraStepsTemplate: /eng/pipelines/libraries/execute-trimming-tests-steps.yml
             extraStepsParameters: