Copilot commented on code in PR #6901: URL: https://github.com/apache/incubator-kie/pull/6901#discussion_r3802674680
########## script/dev/Dev.java: ########## @@ -0,0 +1,841 @@ +/* + * 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. + */ + +///usr/bin/env jbang "$0" "$@" ; exit $? +//JAVA 21 +//SOURCES ../ci/CiComputeBuildScopes.java + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; +import java.util.Properties; +import java.util.Set; + +/** + * Builds only the Maven modules your changes affect, using the same scope computation CI + * runs on pull requests ({@link CiComputeBuildScopes} and {@code DepGraph}, pulled in + * above via jbang {@code //SOURCES} rather than copied, so the two cannot disagree). + * + * <p>Driven by four settings in {@code .kie-dev/config}: <b>since</b> (the point in + * history to measure from), <b>uncommitted</b> (whether the working tree counts), + * <b>breadth</b> (how far to fan out) and <b>upstream</b> (whether to rebuild + * dependencies first). The first run writes that file with sensible defaults. + * + * <p>See {@code docs/DEV.md}. + */ +public class Dev { + + // ------------------------------------------------------------------ + // settings and state + // ------------------------------------------------------------------ + + static final Path STATE_DIR = Paths.get(".kie-dev"); + static final Path CONFIG_FILE = STATE_DIR.resolve("config.properties"); + private static final Path GRAPH_FILE = STATE_DIR.resolve("dep-graph.tsv"); + private static final Path CHANGED_FILES_FILE = STATE_DIR.resolve("changed-files.txt"); + private static final Path PL_UPSTREAM_FILE = STATE_DIR.resolve("pl-upstream.txt"); + private static final Path PL_AFFECTED_FILE = STATE_DIR.resolve("pl-affected.txt"); + private static final Path PL_CHANGED_FILE = STATE_DIR.resolve("pl-changed.txt"); + private static final Path MODULES_FILE = STATE_DIR.resolve("modules-to-build.txt"); + private static final Path UPSTREAM_FILE = STATE_DIR.resolve("upstream-modules.txt"); + private static final Path LAST_COMMAND_FILE = STATE_DIR.resolve("last-maven-command.txt"); + + static final String KEY_SINCE = "since"; + static final String KEY_UNCOMMITTED = "uncommitted"; + static final String KEY_BREADTH = "breadth"; + static final String KEY_UPSTREAM = "upstream"; + /** + * The point in history a build measures from: any git ref. Used as + * {@code merge-base(<ref>, HEAD)}, the same three-dot comparison CI makes, so that + * work that landed on the base branch after you started is not counted as yours. + * + * <p>The default, {@code HEAD}, falls out of that with no special case: its merge + * base with HEAD is HEAD, so nothing committed is included and you are left with the + * working tree alone. + */ + private static final String SINCE_DEFAULT = "HEAD"; + + /** How far a build fans out from what changed. */ + static final String BREADTH_CHANGED = "changed"; + private static final String BREADTH_AFFECTED = "affected"; + private static final List<String> BREADTH_VALUES = List.of(BREADTH_CHANGED, BREADTH_AFFECTED); + + /** Whether to rebuild dependencies before the main build. */ + private static final String UPSTREAM_AUTO = "auto"; + private static final String UPSTREAM_ALWAYS = "always"; + private static final String UPSTREAM_NEVER = "never"; + private static final List<String> UPSTREAM_VALUES = List.of(UPSTREAM_AUTO, UPSTREAM_ALWAYS, UPSTREAM_NEVER); + + private static final List<String> BOOLEAN_VALUES = List.of("true", "false"); + + /** + * The everyday case: what you have edited but not committed, and everything + * downstream of it, which is what CI would check. + * + * <p>A {@link LinkedHashMap} rather than {@code Map.of} so that the config file and + * the line printed on first run come out in a sensible order. + */ + private static final Map<String, String> DEFAULTS = new LinkedHashMap<>(Map.of()) {{ + put(KEY_SINCE, SINCE_DEFAULT); + put(KEY_UNCOMMITTED, "true"); + put(KEY_BREADTH, BREADTH_AFFECTED); + put(KEY_UPSTREAM, UPSTREAM_AUTO); + }}; + Review Comment: Double-brace initialization creates an anonymous subclass and can have surprising side effects (extra class, capturing, serialization quirks). Prefer a plain `LinkedHashMap` built in a static initializer (or a small helper method) to keep initialization explicit and avoid anonymous types. ########## script/ci/DepGraph.java: ########## @@ -0,0 +1,222 @@ +/* + * 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. + */ + +//JAVA 21 + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * The reactor dependency graph, as written by the {@code dep-graph-extractor} Maven + * extension and read by everything that needs to know how the modules relate. + * + * <p>Shared by {@code script/ci/CiComputeBuildScopes.java} and + * {@code script/dev/Dev.java} — pull it in with a jbang {@code //SOURCES} line rather + * than copying it, so that CI and local builds cannot disagree about the graph. + * + * <p>The file is a TSV of one-letter record types: + * <pre> + * P groupId:artifactId /abs/basedir + * D groupId:artifactId upstream-groupId:artifactId scope + * V groupId:artifactId version packaging + * L /abs/path/to/local/maven/repository + * B groupId:artifactId (an in-reactor BOM) + * </pre> + * Unknown record types are ignored, so the extractor can add more without breaking + * anything that reads it. + */ +public final class DepGraph { + + /** Module directory of each module. */ + public final Map<String, Path> gaToDir = new LinkedHashMap<>(); + /** Version of each module, when the extractor recorded it. */ + public final Map<String, String> gaToVersion = new LinkedHashMap<>(); + /** Maven packaging of each module, when the extractor recorded it. */ + public final Map<String, String> gaToPackaging = new LinkedHashMap<>(); + /** What each module depends on, directly. */ + public final Map<String, Set<String>> upstreamOf = new LinkedHashMap<>(); + /** What depends on each module, directly. */ + public final Map<String, Set<String>> downstreamOf = new LinkedHashMap<>(); + /** Every module directory, for telling a module's own files from a nested module's. */ + public final Set<Path> moduleDirs = new HashSet<>(); + /** The modules that are BOMs. */ + public final Set<String> boms = new LinkedHashSet<>(); + /** + * Why each edge exists, keyed by {@code "dependent|dependency"} — a Maven scope, or + * {@code parent} / {@code plugin} / {@code import}. Empty when the graph was written + * by an extractor that predates the 4th {@code D} field. + * + * <p>Build scoping deliberately ignores this: a test-scope edge still means the + * downstream module must be rebuilt. It is here for tooling that needs to tell what + * ships from what is only tested, such as separating test-only modules. + */ + public final Map<String, String> edgeScopes = new LinkedHashMap<>(); + /** The local Maven repository this graph was read with, or null if not recorded. */ + public Path localRepo; + + public static DepGraph parse(Path file) throws IOException { + DepGraph graph = new DepGraph(); + if (!Files.isRegularFile(file)) { + return graph; + } + for (String line : Files.readAllLines(file)) { + String[] parts = line.split("\t", -1); + if (parts.length < 2) continue; + switch (parts[0]) { + case "P" -> { + if (parts.length < 3) break; + Path dir = Paths.get(parts[2]).toAbsolutePath().normalize(); + graph.gaToDir.put(parts[1], dir); + graph.moduleDirs.add(dir); + graph.upstreamOf.computeIfAbsent(parts[1], k -> new LinkedHashSet<>()); + graph.downstreamOf.computeIfAbsent(parts[1], k -> new LinkedHashSet<>()); + } + case "D" -> { + if (parts.length < 3) break; + graph.addEdge(parts[1], parts[2]); + if (parts.length >= 4 && !parts[3].isEmpty()) { + graph.edgeScopes.put(parts[1] + "|" + parts[2], parts[3]); + } + } + case "V" -> { + if (parts.length < 4) break; + graph.gaToVersion.put(parts[1], parts[2]); + graph.gaToPackaging.put(parts[1], parts[3]); + } + case "L" -> graph.localRepo = Paths.get(parts[1]).toAbsolutePath().normalize(); + case "B" -> graph.boms.add(parts[1]); + default -> { /* ignore unknown record types */ } Review Comment: `Files.readAllLines(file)` loads the entire TSV into memory. Since this graph can be large (hundreds of modules + edges), consider streaming with `Files.newBufferedReader(...)` and reading line-by-line to reduce peak memory and improve scalability. ########## script/ci/CiComputeBuildScopes.java: ########## @@ -93,53 +95,32 @@ public static void main(String[] args) throws Exception { // 2. run dep-graph-extractor, writing dependency graph to file. // Persist the graph to DEP_GRAPH_EXTRACTOR__OUTPUT_FILE when set so downstream // tools (CiSummary) can reuse it without re-invoking Maven. - String graphFileEnv = System.getenv("DEP_GRAPH_EXTRACTOR__OUTPUT_FILE"); + String graphFileEnv = cfg("DEP_GRAPH_EXTRACTOR__OUTPUT_FILE"); Path graphFile = (graphFileEnv != null && !graphFileEnv.isBlank()) ? Paths.get(graphFileEnv).toAbsolutePath() : Files.createTempFile("dep-graph-", ".tsv"); - int rc = runMavenWithDepGraphExtractor(cwd, extractorJarPath, graphFile, extraMavenArgs); - if (!Files.isRegularFile(graphFile) || Files.size(graphFile) == 0) { - System.err.println("dep-graph-extractor failed (mvn rc=" + rc + ")"); - System.exit(1); - } - - // 3. parse graph - Map<String, Path> gaToDir = new HashMap<>(); - Map<String, Set<String>> upstreamOf = new HashMap<>(); // ga -> direct upstreams - Map<String, Set<String>> downstreamOf = new HashMap<>(); // ga -> direct downstreams - try (BufferedReader r = Files.newBufferedReader(graphFile)) { - String line; - while ((line = r.readLine()) != null) { - String[] parts = line.split("\t", -1); - if (parts.length < 3) continue; - switch (parts[0]) { - case "P" -> { - gaToDir.put(parts[1], Paths.get(parts[2]).toAbsolutePath().normalize()); - upstreamOf.computeIfAbsent(parts[1], k -> new HashSet<>()); - downstreamOf.computeIfAbsent(parts[1], k -> new HashSet<>()); - } - case "D" -> { - upstreamOf.computeIfAbsent(parts[1], k -> new HashSet<>()).add(parts[2]); - downstreamOf.computeIfAbsent(parts[2], k -> new HashSet<>()).add(parts[1]); - } - } + // Reading the reactor costs a full pass over every pom.xml, which is a steep + // price to pay on every invocation of a local dev loop. When + // DEP_GRAPH_EXTRACTOR__REUSE_IF_FRESH is set, keep a stamp of the reactor's + // poms next to the graph and re-extract only once one of them changes. + // Off by default, so CI always extracts from scratch. + Path stampFile = Paths.get(graphFile + ".stamp"); Review Comment: Building the stamp path via `Paths.get(graphFile + \".stamp\")` relies on `Path#toString()` string concatenation and reparsing, which is brittle across platforms and path edge cases. Prefer constructing it as a sibling path (e.g., `graphFile.resolveSibling(graphFile.getFileName() + \".stamp\")`) to keep it purely path-based. ########## docs/DEV.md: ########## @@ -0,0 +1,340 @@ +<!-- + 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. + --> + +# Building partially, with `make dev` + +This repository has more than 850 Maven modules, so building all of them to +check a change in one is rarely what you want. CI already avoids that on pull +requests — it works out which modules a change can possibly affect and builds +only those (see [PR_CHECKS_AND_CI.md](./PR_CHECKS_AND_CI.md)). `make dev` gives +you the same thing locally, driven by your working copy instead of a pull +request. + +It uses the very same scope computation CI runs, so the modules it builds are +the modules CI will build. By default it looks at what you have edited but not +committed, and skips tests — that is the inner loop, and CI is the safety net; +`make dev mvn -- install` runs the tests when you want them. + +## What it does + +- **Builds only what your change affects.** The same computation CI runs on pull + requests, applied to your working copy. +- **Optimises for the inner loop.** By default it looks at uncommitted work only, + skips tests and the reporting plugins, and builds in parallel. + `make dev mvn -- install` runs the tests when you want them. +- **Rebuilds stale dependencies for you.** Anything missing from `~/.m2`, or + older than the sources it was built from, is rebuilt first — so switching + branches or a fresh clone does not silently build against the wrong artifacts. +- **Remembers its settings.** Four lines in `.kie-dev/config.properties`, written on the + first run and editable by hand. +- **Lets you say what "your changes" means.** Since this branch started, or since + your last commit — with or without uncommitted work. +- **Shows you before it builds.** `make dev scope` prints the module list and + builds nothing. +- **Stays out of the way of Maven.** `make dev mvn -- …` passes your command + straight through, so any goal, profile or flag still works. +- **Caches the expensive part.** The reactor dependency graph is read once and + reused until a `pom.xml` changes, which is the difference between a two-second + and a forty-second start. +- **Works from a plain shell.** Your Maven if you have one, the devbox copy if + you do not. + +## Getting started + +```bash +make dev +``` + +That is the whole thing. The first run writes `.kie-dev/config.properties` with sensible +defaults and tells you so; edit that file, or override any setting for a single +run. + +The only prerequisites are [JBang](https://www.jbang.dev), Maven and git on your +`PATH` — this repository provides the first two through +[devbox](https://www.jetify.com/devbox), so `devbox shell` gets you all of them. +If one is missing you are told which, rather than left with a stack trace. You do not need to have built the repository first — dependencies that +are missing from `~/.m2`, or that are older than their sources, are rebuilt +automatically. See [Upstream modules](#upstream-modules) below. + +The Makefile is dedicated to these partial builds. Full builds are plain +Maven — see [BUILDING.md](./BUILDING.md). + +## Cheatsheet + +Every knob, in one place. + +### Commands + +| Command | Does | +| --- | --- | +| `make dev` | Build what your changes affect, using the saved settings | +| `make dev scope` | Print what would be built; build nothing | +| `make dev config` | Show the current settings | +| `make dev mvn -- <mvn args>` | Build them with your own Maven command | +| `make help` | List the targets and summarise the above | + +### Settings + +Four settings, in precedence order: **command line** → **`.kie-dev/config.properties`** → **default**. + +| Setting | Values | Default | +| --- | --- | --- | +| `since` | any git ref | `HEAD` | +| `uncommitted` | `true`, `false` | `true` | +| `breadth` | `changed`, `affected` | `affected` | +| `upstream` | `auto`, `always`, `never` | `auto` | + +```bash +make dev since=origin/main # this run only, not saved +make dev config # show what is saved +``` + +<details> +<summary>What each value means</summary> + +| `since` | Committed changes are measured from | +| --- | --- | +| `HEAD` | nothing committed — your working tree alone. The default | +| `origin/main` | where this branch was branched from main | +| `HEAD~1` | your last commit | +| any ref | `git merge-base <ref> HEAD` | + + +| `uncommitted` | The working tree | +| ------------- | ----------------------------------------------- | +| `true` | added on top: `git diff HEAD` + untracked files | +| `false` | ignored; only committed work counts | + +| `breadth` | Modules built | +| ---------- | --------------------------------------------- | +| `changed` | the modules whose files changed | +| `affected` | those plus everything transitively downstream — the default, as CI builds | + +| `upstream` | Dependencies rebuilt first, with tests skipped | +| ---------- | ----------------------------------------------------------------------------------------------------- | +| `auto` | those missing from the local repository or older than their sources, plus anything downstream of them | +| `always` | all of them | +| `never` | none; no check is run | + +</details> + +### Maven arguments + +`make dev` builds fast: parallel, fail-at-end, and skipping tests, enforcer, +checkstyle, formatter and ArchUnit. That is the inner loop — CI is what runs the +tests. + +When you want something else, `make dev mvn -- <mvn args>` takes the Maven command itself. +Everything after `--` replaces the default entirely: + +```bash +make dev mvn -- install # the same modules, with tests +make dev mvn -- clean install -Pfull +make dev mvn -- test -Dtest=MyTest +``` + +`--` is required — without it Make treats `-DskipTests` as one of its own +options. `make dev mvn` with nothing after the `--` is an error rather than a silent +second spelling of `make dev`. + +### Config file + +`.kie-dev/config.properties`, an ordinary Java properties file, safe to edit by hand: + +| Key | Value | +| --------------- | -------------------------------------------------------------- | +| `since` | as above | +| `uncommitted` | as above | +| `breadth` | as above | +| `upstream` | as above | + +### Environment variables + +| Variable | Effect | +| ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- | +| `KIE_DEV_SINCE`, `KIE_DEV_UNCOMMITTED`, `KIE_DEV_BREADTH`, `KIE_DEV_UPSTREAM` | What the `since=`/`breadth=`/`uncommitted=`/`upstream=` overrides set | +| `MVN` | Maven binary to run. Otherwise `mvn` from `PATH`, then the devbox copy under `.devbox/` | +| `MAVEN_OPTS` | Passed through to Maven as usual | +| `NO_COLOR` | Disables coloured output | +| `DEP_GRAPH_EXTRACTOR__JAR` | Use a prebuilt extractor jar instead of building one | +| `DEP_GRAPH_EXTRACTOR__EXTRA_MAVEN_ARGS` | Extra args for the graph extraction run, e.g. `-Psome-profile` | + +`DEP_GRAPH_EXTRACTOR__OUTPUT_FILE` and `DEP_GRAPH_EXTRACTOR__REUSE_IF_FRESH` +also exist, but `make dev` sets both itself, so setting them in the +environment has no effect here. They are how CI runs the same script without the +cache — see [PR_CHECKS_AND_CI.md](./PR_CHECKS_AND_CI.md). + +### State files + +All under `.kie-dev/`, all disposable — delete the directory to reset. + +| File | Contents | +| ------------------------------------------------------ | --------------------------------------------- | +| `config.properties` | your saved settings | +| `dep-graph.tsv` | the reactor dependency graph, cached between runs | +| `dep-graph.tsv.stamp` | fingerprint of every `pom.xml`, which invalidates that graph | +| `changed-files.txt` | changed files from the last run | +| `modules-to-build.txt` | the modules the last run built | +| `upstream-modules.txt` | the upstream modules it rebuilt first, if any | +| `last-maven-command.txt` | the Maven commands it issued, in full | +| `pl-changed.txt`, `pl-affected.txt`, `pl-upstream.txt` | the computed scopes from the last run | + +Module lists and Maven commands are printed as a count rather than in full — a +`-pl` argument with several hundred coordinates in it is not something anyone +reads. The files above are where the real thing lives, for when you need to +copy, paste or debug it. + +### Tests + +```bash +jbang script/dev/tests/DevTest.java +jbang script/dev/tests/MakefileTest.java +``` + +## Upstream modules + +A partial build can only resolve dependencies that are already installed in +`~/.m2`, and it only gives correct results if what is installed was built from +the sources you have now. Both go wrong routinely: the first time you use the +repository nothing is installed at all, and after switching branches or pulling, +what is installed no longer matches your working copy. + +So before the main build, the tool works out which of its dependencies cannot be +trusted, and rebuilds exactly those with tests skipped: + +- **not installed** — no artifact in the local Maven repository; +- **older than its sources** — the installed artifact predates a file in the + module it was built from; +- **downstream of either** — a module whose own dependency is being rebuilt. + +Typically nothing needs rebuilding and the check costs a second or so: + +``` +all 10 upstream modules are installed and up to date — nothing to rebuild +``` + +Otherwise it says what it is about to do, and why: + +``` +Upstream modules to rebuild first: 82 (.kie-dev/upstream-modules.txt) + 5 older than their sources, 77 downstream of those +``` + +This is controlled by `upstream`, which defaults to `auto` because that is Review Comment: Fix grammatical duplication: remove the extra \"that is\" so the sentence reads correctly. ########## docs/DEV.md: ########## @@ -0,0 +1,340 @@ +<!-- + 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. + --> + +# Building partially, with `make dev` + +This repository has more than 850 Maven modules, so building all of them to +check a change in one is rarely what you want. CI already avoids that on pull +requests — it works out which modules a change can possibly affect and builds +only those (see [PR_CHECKS_AND_CI.md](./PR_CHECKS_AND_CI.md)). `make dev` gives +you the same thing locally, driven by your working copy instead of a pull +request. + +It uses the very same scope computation CI runs, so the modules it builds are +the modules CI will build. By default it looks at what you have edited but not +committed, and skips tests — that is the inner loop, and CI is the safety net; +`make dev mvn -- install` runs the tests when you want them. + +## What it does + +- **Builds only what your change affects.** The same computation CI runs on pull + requests, applied to your working copy. +- **Optimises for the inner loop.** By default it looks at uncommitted work only, + skips tests and the reporting plugins, and builds in parallel. + `make dev mvn -- install` runs the tests when you want them. +- **Rebuilds stale dependencies for you.** Anything missing from `~/.m2`, or + older than the sources it was built from, is rebuilt first — so switching + branches or a fresh clone does not silently build against the wrong artifacts. +- **Remembers its settings.** Four lines in `.kie-dev/config.properties`, written on the + first run and editable by hand. +- **Lets you say what "your changes" means.** Since this branch started, or since + your last commit — with or without uncommitted work. +- **Shows you before it builds.** `make dev scope` prints the module list and + builds nothing. +- **Stays out of the way of Maven.** `make dev mvn -- …` passes your command + straight through, so any goal, profile or flag still works. +- **Caches the expensive part.** The reactor dependency graph is read once and + reused until a `pom.xml` changes, which is the difference between a two-second + and a forty-second start. +- **Works from a plain shell.** Your Maven if you have one, the devbox copy if + you do not. + +## Getting started + +```bash +make dev +``` + +That is the whole thing. The first run writes `.kie-dev/config.properties` with sensible +defaults and tells you so; edit that file, or override any setting for a single +run. + +The only prerequisites are [JBang](https://www.jbang.dev), Maven and git on your +`PATH` — this repository provides the first two through +[devbox](https://www.jetify.com/devbox), so `devbox shell` gets you all of them. +If one is missing you are told which, rather than left with a stack trace. You do not need to have built the repository first — dependencies that +are missing from `~/.m2`, or that are older than their sources, are rebuilt +automatically. See [Upstream modules](#upstream-modules) below. + +The Makefile is dedicated to these partial builds. Full builds are plain +Maven — see [BUILDING.md](./BUILDING.md). + +## Cheatsheet + +Every knob, in one place. + +### Commands + +| Command | Does | +| --- | --- | +| `make dev` | Build what your changes affect, using the saved settings | +| `make dev scope` | Print what would be built; build nothing | +| `make dev config` | Show the current settings | +| `make dev mvn -- <mvn args>` | Build them with your own Maven command | +| `make help` | List the targets and summarise the above | + +### Settings + +Four settings, in precedence order: **command line** → **`.kie-dev/config.properties`** → **default**. + +| Setting | Values | Default | +| --- | --- | --- | +| `since` | any git ref | `HEAD` | +| `uncommitted` | `true`, `false` | `true` | +| `breadth` | `changed`, `affected` | `affected` | +| `upstream` | `auto`, `always`, `never` | `auto` | + +```bash +make dev since=origin/main # this run only, not saved +make dev config # show what is saved +``` + +<details> +<summary>What each value means</summary> + +| `since` | Committed changes are measured from | +| --- | --- | +| `HEAD` | nothing committed — your working tree alone. The default | +| `origin/main` | where this branch was branched from main | +| `HEAD~1` | your last commit | +| any ref | `git merge-base <ref> HEAD` | + + +| `uncommitted` | The working tree | +| ------------- | ----------------------------------------------- | +| `true` | added on top: `git diff HEAD` + untracked files | +| `false` | ignored; only committed work counts | + +| `breadth` | Modules built | +| ---------- | --------------------------------------------- | +| `changed` | the modules whose files changed | +| `affected` | those plus everything transitively downstream — the default, as CI builds | + +| `upstream` | Dependencies rebuilt first, with tests skipped | +| ---------- | ----------------------------------------------------------------------------------------------------- | +| `auto` | those missing from the local repository or older than their sources, plus anything downstream of them | +| `always` | all of them | +| `never` | none; no check is run | + +</details> + +### Maven arguments + +`make dev` builds fast: parallel, fail-at-end, and skipping tests, enforcer, +checkstyle, formatter and ArchUnit. That is the inner loop — CI is what runs the +tests. + +When you want something else, `make dev mvn -- <mvn args>` takes the Maven command itself. +Everything after `--` replaces the default entirely: + +```bash +make dev mvn -- install # the same modules, with tests +make dev mvn -- clean install -Pfull +make dev mvn -- test -Dtest=MyTest +``` + +`--` is required — without it Make treats `-DskipTests` as one of its own +options. `make dev mvn` with nothing after the `--` is an error rather than a silent +second spelling of `make dev`. + +### Config file + +`.kie-dev/config.properties`, an ordinary Java properties file, safe to edit by hand: + +| Key | Value | +| --------------- | -------------------------------------------------------------- | +| `since` | as above | +| `uncommitted` | as above | +| `breadth` | as above | +| `upstream` | as above | + +### Environment variables + +| Variable | Effect | +| ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- | +| `KIE_DEV_SINCE`, `KIE_DEV_UNCOMMITTED`, `KIE_DEV_BREADTH`, `KIE_DEV_UPSTREAM` | What the `since=`/`breadth=`/`uncommitted=`/`upstream=` overrides set | +| `MVN` | Maven binary to run. Otherwise `mvn` from `PATH`, then the devbox copy under `.devbox/` | +| `MAVEN_OPTS` | Passed through to Maven as usual | +| `NO_COLOR` | Disables coloured output | +| `DEP_GRAPH_EXTRACTOR__JAR` | Use a prebuilt extractor jar instead of building one | +| `DEP_GRAPH_EXTRACTOR__EXTRA_MAVEN_ARGS` | Extra args for the graph extraction run, e.g. `-Psome-profile` | + +`DEP_GRAPH_EXTRACTOR__OUTPUT_FILE` and `DEP_GRAPH_EXTRACTOR__REUSE_IF_FRESH` +also exist, but `make dev` sets both itself, so setting them in the +environment has no effect here. They are how CI runs the same script without the +cache — see [PR_CHECKS_AND_CI.md](./PR_CHECKS_AND_CI.md). + +### State files + +All under `.kie-dev/`, all disposable — delete the directory to reset. + +| File | Contents | +| ------------------------------------------------------ | --------------------------------------------- | +| `config.properties` | your saved settings | +| `dep-graph.tsv` | the reactor dependency graph, cached between runs | +| `dep-graph.tsv.stamp` | fingerprint of every `pom.xml`, which invalidates that graph | +| `changed-files.txt` | changed files from the last run | +| `modules-to-build.txt` | the modules the last run built | +| `upstream-modules.txt` | the upstream modules it rebuilt first, if any | +| `last-maven-command.txt` | the Maven commands it issued, in full | +| `pl-changed.txt`, `pl-affected.txt`, `pl-upstream.txt` | the computed scopes from the last run | + +Module lists and Maven commands are printed as a count rather than in full — a +`-pl` argument with several hundred coordinates in it is not something anyone +reads. The files above are where the real thing lives, for when you need to +copy, paste or debug it. + +### Tests + +```bash +jbang script/dev/tests/DevTest.java +jbang script/dev/tests/MakefileTest.java +``` + +## Upstream modules + +A partial build can only resolve dependencies that are already installed in +`~/.m2`, and it only gives correct results if what is installed was built from +the sources you have now. Both go wrong routinely: the first time you use the +repository nothing is installed at all, and after switching branches or pulling, +what is installed no longer matches your working copy. + +So before the main build, the tool works out which of its dependencies cannot be +trusted, and rebuilds exactly those with tests skipped: + +- **not installed** — no artifact in the local Maven repository; +- **older than its sources** — the installed artifact predates a file in the + module it was built from; +- **downstream of either** — a module whose own dependency is being rebuilt. + +Typically nothing needs rebuilding and the check costs a second or so: + +``` +all 10 upstream modules are installed and up to date — nothing to rebuild +``` + +Otherwise it says what it is about to do, and why: + +``` +Upstream modules to rebuild first: 82 (.kie-dev/upstream-modules.txt) + 5 older than their sources, 77 downstream of those +``` + +This is controlled by `upstream`, which defaults to `auto` because that is +`auto` is almost always what you want: + +| `upstream` | Behaviour | +| ---------- | ---------------------------------------------------------------------- | +| `auto` | Rebuild only dependencies that are missing or out of date. The default | +| `always` | Rebuild every dependency, like CI does | +| `never` | Trust `~/.m2` as it is, and skip the check | + +```bash +make dev upstream=always # when in doubt +make dev upstream=never # when you know ~/.m2 is good and want the seconds back +``` + +Staleness is judged by file modification times, which is why `never` exists: if +something touches files without changing them, everything downstream will look +stale. Note also that this only reasons about modules *in this repository* — +dependencies from other repositories are ordinary Maven artifacts, and it is up +to you to have the right versions installed. + +## How it works + +1. `make dev` runs [`script/dev/Dev.java`](../script/dev/Dev.java) (with + `DevConfig`, `DevGit` and `DevIoUtil` alongside it). Review Comment: This references `DevConfig`, `DevGit`, and `DevIoUtil`, but the implementation shown in this PR is a single `Dev.java` file. Update this section to reflect the actual code layout (or add links/files if these helpers exist) to keep the docs accurate. ########## script/dev/tests/MakefileTest.java: ########## @@ -0,0 +1,168 @@ +/* + * 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. + */ + +///usr/bin/env jbang "$0" "$@" ; exit $? +//JAVA 21 +//DEPS org.junit.platform:junit-platform-console-standalone:1.11.4 +//DEPS org.assertj:assertj-core:3.26.3 + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; +import org.junit.platform.console.ConsoleLauncher; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; + +/** + * Tests for the {@code dev} target in the repository Makefile — {@code make dev mvn}, + * {@code make dev scope} and {@code make dev config}. + * + * These run {@code make --dry-run}, so they assert on the commands Make *would* run + * without running them — no Maven, no builds, no network. + * + * Most of what is tested here is Make's command-line parsing, which is subtle enough to + * be worth pinning down: the subcommand and every Maven argument reach Make as goals of + * its own, `--` is what stops it from swallowing `-DskipTests` as one of its options, and + * the catch-all rule that absorbs the leftovers must stay scoped to `dev`, so that a + * mistyped target still fails loudly. + * + * Run: + * jbang script/dev/tests/MakefileTest.java + */ +public class MakefileTest { + + static final Path REPO_ROOT = Paths.get("").toAbsolutePath(); + static final String SCRIPT = "script/dev/Dev.java"; + + public static void main(String[] args) { + ConsoleLauncher.main(new String[]{ + "execute", + "--select-class=" + MakefileTest.class.getName(), + "--exclude-engine=junit-vintage", + "--fail-if-no-tests" + }); + } + + @Test + void helpShowsEverySubcommand() throws Exception { + Result help = make("help"); + + assertThat(help.rc).isZero(); + assertThat(strippedOf(help.output)) + .contains("make dev ") + .contains("make dev scope") + .contains("make dev config") + .contains("make dev mvn") + .contains("docs/DEV.md"); + } + + @Test + void eachSubcommandReachesTheScriptAsItsFirstArgument() throws Exception { + assertThat(make("-n", "dev", "mvn").output).contains(SCRIPT + " mvn"); + assertThat(make("-n", "dev", "scope").output).contains(SCRIPT + " scope"); + assertThat(make("-n", "dev", "config").output).contains(SCRIPT + " config"); + } + + @Test + void mavenFlagsSurviveMakesOwnOptionParsing() throws Exception { + Result r = make("-n", "dev", "mvn", "--", "install", "-DskipTests", "-Pfull", "-T1C"); + + assertThat(r.rc).isZero(); + assertThat(r.output).contains("install -DskipTests -Pfull -T1C"); + } + + @Test + void overridesArePassedAsEnvironmentVariables() throws Exception { + Result r = make("-n", "dev", "since=HEAD", "breadth=changed"); + + assertThat(r.rc).isZero(); + assertThat(r.output).contains("KIE_DEV_SINCE=HEAD").contains("KIE_DEV_BREADTH=changed"); + } + + @Test + void leftoverMavenArgumentsDoNotRunAnythingThemselves() throws Exception { + Result r = make("-n", "dev", "mvn", "--", "clean", "install"); + + // Every printed command is either the script invocation or the catch-all no-op. + List<String> commands = r.output.lines() + .map(String::strip) + .filter(line -> !line.isEmpty()) + .filter(line -> !line.equals(":")) + .toList(); + assertThat(commands).hasSize(1); + assertThat(commands.get(0)).contains(SCRIPT); + } + + /** + * The catch-all that absorbs subcommands and Maven arguments is deliberately scoped + * to `dev`. Outside it, a mistyped target must still be an error. + */ + @Test + void aMistypedTargetStillFails() throws Exception { + Result r = make("-n", "dev-buld"); + + assertThat(r.rc).isNotZero(); + assertThat(r.output).contains("dev-buld"); + } + + @Test + void theMakefileHasNoTargetsThatCouldCollideWithMavenGoals() throws Exception { + // Subcommands and Maven goals both reach Make as goals, so a target named + // `clean`, `install` or `mvn` would silently run instead of being forwarded. + List<String> targets = new ArrayList<>(); + for (String line : Files.readAllLines(REPO_ROOT.resolve("Makefile"))) { + if (line.startsWith(".PHONY:")) { + targets.add(line.substring(".PHONY:".length()).strip()); + } + } + + assertThat(targets).isNotEmpty(); + assertThat(targets) + .doesNotContain("clean", "install", "test", "verify", "package", "deploy", "validate") + .doesNotContain("mvn", "scope", "config"); + } + + // ------------------------------------------------------------------ + // helpers + // ------------------------------------------------------------------ + + private record Result(int rc, String output) {} + + private static Result make(String... args) throws IOException, InterruptedException { + List<String> cmd = new ArrayList<>(); + cmd.add(System.getenv().getOrDefault("MAKE", "make")); + cmd.addAll(List.of(args)); + Process p = new ProcessBuilder(cmd) + .directory(REPO_ROOT.toFile()) + .redirectErrorStream(true) + .start(); + String output = new String(p.getInputStream().readAllBytes()); + return new Result(p.waitFor(), output); + } + + /** Drops the ANSI colour codes `make help` emits, so assertions can match plain text. */ + private static String strippedOf(String output) { + return output.replaceAll("\\[[0-9;]*m", ""); Review Comment: This regex embeds a literal ESC control character in the source, which is easy to corrupt via tooling/encoding and hard to read. Prefer using `\"\\u001B\\\\[[0-9;]*m\"` (or a named constant/Pattern) so the intent is clear and the source stays portable. -- 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]
