tiagobento commented on code in PR #6860:
URL: https://github.com/apache/incubator-kie/pull/6860#discussion_r3806347501
##########
script/ci/CiComputeBuildScopes.java:
##########
@@ -202,6 +220,102 @@ private static void writeLines(Path out,
Collection<String> lines) throws IOExce
Files.write(out, sorted);
}
+ private static final List<String> EXPECTED_CATEGORIES =
List.of("optaplanner", "kogito-runtimes", "kogito-apps");
+
+ static Map<String, Set<Path>> parseModuleCategories(Path rootPom, Path
cwd) throws IOException {
+ Map<String, Set<Path>> categories = new LinkedHashMap<>();
+ categories.put("drools", new LinkedHashSet<>());
+ for (String cat : EXPECTED_CATEGORIES) {
+ categories.put(cat, new LinkedHashSet<>());
+ }
+
+ String currentCategory = "drools";
+ boolean inModules = false;
+ Set<String> seenBegins = new HashSet<>();
+ Set<String> seenEnds = new HashSet<>();
+
+ Pattern beginPattern =
Pattern.compile("<!--\\s*BEGIN\\s+(\\S+)\\s+modules\\s+\\(auto\\)\\s*-->");
+ Pattern endPattern =
Pattern.compile("<!--\\s*END\\s+(\\S+)\\s+modules\\s+\\(auto\\)\\s*-->");
+ Pattern modulePattern = Pattern.compile("<module>(.+)</module>");
Review Comment:
I'm a concerned with this parsing strategy. As it relies on comments being
maintained in the top-level pom.xml file. This simply mimics the old structure
we had with the repos, and doesn't to separate things logically per domain. I
don't think we should carry forward the old, artificial grouping we had.
Can we try to make this partitioned by domain? We can have even more
partitions, like Kogito Data-Index, Kogito Jobs Service, for example.
In `kie-tools`, when we needed to do a similar partitioning effort in order
to keep PR checks build times in check, what I did was I created partitioning
files which listed the leaf modules which represented good vertical partitions
of the dependency DAG.
We don't have this anymore since the 10.3.x+ stream effort began, but you
can see how it used to look like in 10.2.
https://github.com/apache/incubator-kie-tools/tree/10.2.0/.github/supporting-files/ci/partitions
Note that there are 2 partition files only, with an implicit 3rd partition
which contained all the modules that were not listed explicitly in neither of
those 2 partition files.
##########
.github/actions/ci-setup/action.yaml:
##########
@@ -0,0 +1,128 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+name: "CI Setup"
+description: "Shared CI setup: checkout, Java, JBang, Maven settings, build
scope computation"
+
+inputs:
+ ref:
+ description: "Base ref for PR checkout"
+ required: true
+ java-version:
+ description: "Java version to set up"
+ required: true
+
+runs:
+ using: "composite"
+ steps:
+ - name: 'SETUP :: Free disk space (Ubuntu)'
+ if: runner.os == 'Linux'
+ shell: bash
+ run: |
+ sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc
+ df -h
+
+ - name: 'SETUP :: Enable long file paths (Windows)'
+ if: runner.os == 'Windows'
+ shell: pwsh
+ run: |
+ git config --system core.longpaths true
+ reg add "HKLM\SYSTEM\CurrentControlSet\Control\FileSystem" /v
LongPathsEnabled /t REG_DWORD /d 1 /f
+
+ - name: "SETUP :: Checkout @ Simulated squashed-merge if PR, else checkout
current commit"
+ uses: ./.github/actions/checkout-pr
+ with:
+ ref: ${{ inputs.ref }}
+
+ - uses: actions/setup-java@v5
+ name: 'SETUP :: Java ${{ inputs.java-version }}'
+ with:
+ java-version: ${{ inputs.java-version }}
+ distribution: temurin
+ cache: maven
+
+ # Temporary workaround while jbangdev/setup-jbang action doesn't include
checksum verification.
+ # See https://github.com/apache/infrastructure-actions/pull/806
+ - name: 'SETUP :: Install JBang'
+ shell: bash
+ run: |
+ curl -sL
https://github.com/jbangdev/jbang/releases/download/v0.138.0/jbang-0.138.0.zip
-o jbang.zip
+ echo "3c9fb9ac5823ae5ab9d136dec08b896e2a0f6f6b313689b21f43c511b7c34b85
jbang.zip" | sha256sum -c -
+ unzip -q jbang.zip
+ echo "${{ github.workspace }}/jbang-0.138.0/bin" >> $GITHUB_PATH
+
+ - name: 'SETUP :: Block apache.snapshots Maven repository'
+ shell: bash
+ run: |
+ mkdir -p "$HOME/.m2"
+ cat > "$HOME/.m2/settings.xml" <<'EOF'
+ <settings xmlns="http://maven.apache.org/SETTINGS/1.2.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/SETTINGS/1.2.0
https://maven.apache.org/xsd/settings-1.2.0.xsd";>
+ <mirrors>
+ <mirror>
+ <id>block-apache-snapshots</id>
+ <mirrorOf>apache.snapshots,apache-snapshot-repository</mirrorOf>
+ <url>https://repository.apache.org/snapshots/</url>
+ <blocked>true</blocked>
+ </mirror>
+ </mirrors>
+ </settings>
+ EOF
+
+ - name: 'SETUP :: Set paths for temporary files'
+ shell: bash
+ run: |
+ echo "CHANGED_FILES_FILE=${{ runner.temp }}/changed-files.txt" >>
"$GITHUB_ENV"
+ echo "MAVEN_PL_AFFECTED_FILE=${{ runner.temp }}/maven-pl-affected.txt"
>> "$GITHUB_ENV"
+ echo "MAVEN_PL_UPSTREAM_FILE=${{ runner.temp }}/maven-pl-upstream.txt"
>> "$GITHUB_ENV"
+ echo "MAVEN_PL_CHANGED_FILE=${{ runner.temp }}/maven-pl-changed.txt"
>> "$GITHUB_ENV"
+ echo "DEP_GRAPH_EXTRACTOR__OUTPUT_FILE=${{ runner.temp
}}/dep-graph.tsv" >> "$GITHUB_ENV"
+ echo "MAVEN_PL_AFFECTED_DROOLS_FILE=${{ runner.temp
}}/maven-pl-affected-drools.txt" >> "$GITHUB_ENV"
+ echo "MAVEN_PL_AFFECTED_OPTAPLANNER_FILE=${{ runner.temp
}}/maven-pl-affected-optaplanner.txt" >> "$GITHUB_ENV"
+ echo "MAVEN_PL_AFFECTED_KOGITO_RUNTIMES_FILE=${{ runner.temp
}}/maven-pl-affected-kogito-runtimes.txt" >> "$GITHUB_ENV"
+ echo "MAVEN_PL_AFFECTED_KOGITO_APPS_FILE=${{ runner.temp
}}/maven-pl-affected-kogito-apps.txt" >> "$GITHUB_ENV"
+ touch "${{ runner.temp }}/changed-files.txt"
+ touch "${{ runner.temp }}/maven-pl-affected.txt"
+ touch "${{ runner.temp }}/maven-pl-upstream.txt"
+ touch "${{ runner.temp }}/maven-pl-changed.txt"
+ touch "${{ runner.temp }}/maven-pl-affected-drools.txt"
+ touch "${{ runner.temp }}/maven-pl-affected-optaplanner.txt"
+ touch "${{ runner.temp }}/maven-pl-affected-kogito-runtimes.txt"
+ touch "${{ runner.temp }}/maven-pl-affected-kogito-apps.txt"
Review Comment:
Declaring 4 fixes partitions I believe will limit the flexibility of the
whole mechanism, and further constrain us into fitting new modules into these 4
boxes. Can we try to make partitions more fluid? Adding/removing a new
partition should be easy enough. We can use file names to name them, and list
the leaf modules which belong in each partition inside the files. E.g.,
```
drools.txt
optaplanner.txt
kogito-runtimes.txt
kogito-data-index.txt
kogito-jobs-service.txt
```
and so on.
##########
pom.xml:
##########
@@ -152,6 +152,9 @@
</build>
<modules>
+ <!-- DO NOT remove the BEGIN/END comments. They are used by
CiComputeBuildScopes for parallel CI job split
+ When you add a new module, put it in one of the 4 categories (drools,
optaplanner, kogito-runtimes, kogito-apps) -->
+ <!-- BEGIN drools modules (auto) -->
Review Comment:
I see you're putting some guardrails in place, but I think we can do better.
The "partitions" files listing leaf modules which bring a larger dependency
hierarchy with them will give us a lot more flexibility, and will allow us to
not couple the pom.xml with a partitioning solution for PR checks and CI.
##########
.github/workflows/ci.yaml:
##########
@@ -216,14 +248,145 @@ jobs:
uses: actions/upload-artifact@v7
if: ${{ always() }}
with:
- name: build-logs_${{ matrix.os }}_java-${{ matrix.java }}
+ name: build-logs_optaplanner_${{ matrix.os }}_java-${{ matrix.java }}
path: '**/build.log'
- name: Upload Build Compare
uses: actions/upload-artifact@v7
if: ${{ always() }}
with:
- name: build-compare_${{ matrix.os }}_java-${{ matrix.java }}
+ name: build-compare_optaplanner_${{ matrix.os }}_java-${{
matrix.java }}
+ path: |
+ **/*.buildcompare
+ **/*.buildinfo
+
+ test-kogito-runtimes:
+ if: github.event.action != 'closed' && github.event_name == 'pull_request'
+ name: 'kogito-runtimes :: ${{ matrix.os }}, Java ${{ matrix.java }}'
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false
+ matrix:
+ os: [ubuntu-latest]
+ java: [17, 21]
+ steps:
+ - name: "SETUP :: Checkout to load local actions"
+ uses: actions/checkout@v7
+
+ - uses: ./.github/actions/ci-setup
+ with:
+ ref: ${{ github.base_ref }}
+ java-version: ${{ matrix.java }}
+
+ - name: "PR CHECK :: BUILD :: Upstream + drools + optaplanner modules
(tests skipped)"
+ shell: bash
+ run: |
+ [ ! -s "$MAVEN_PL_AFFECTED_KOGITO_RUNTIMES_FILE" ] && echo "No
affected kogito-runtimes modules. Skipping upstream build." && exit 0
+ upstream=$(paste -sd, "$MAVEN_PL_UPSTREAM_FILE")
+ drools=$(paste -sd, "$MAVEN_PL_AFFECTED_DROOLS_FILE")
+ optaplanner=$(paste -sd, "$MAVEN_PL_AFFECTED_OPTAPLANNER_FILE")
+ pl=$(echo "$upstream,$drools,$optaplanner" | sed
's/^,//;s/,,*/,/g;s/,$//')
+ [ -z "$pl" ] && echo "No modules to build. Skipping." && exit 0
+ mvn --batch-mode --no-transfer-progress -T 1C -fae -DskipTests
-DskipITs -Dquarkus.build.skip=true -Denforcer.skip=true -Dcheckstyle.skip=true
-Dformatter.skip=true -Darchunit.skip=true
-Dsurefire.redirectTestOutputToFile=true -pl "$pl" install
Review Comment:
Commenting on this one, but it applies to each of the fixed 4 partitions.
There's a lot of duplication here. If we make partitions dynamic (and thus more
flexible), we remove the duplication and keep things simpler to change as we
evolve our codebase. We really need a CI system that's not rigid for us to have
room to grow / adapt.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
