From e240866d6570fedd5ed3de21af811f79ec4ece4f Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:32:28 +0200 Subject: [PATCH 01/24] fix: dedent groovy code block bodies by the fence indent --- gmd-core/release.md | 4 ++ .../alipsa/gmd/core/GmdTemplateEngine.groovy | 31 ++++++++- .../groovy/gmd/GmdTemplateEngineTest.groovy | 64 ++++++++++++++++++- 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/gmd-core/release.md b/gmd-core/release.md index f40d9c1..d8add86 100644 --- a/gmd-core/release.md +++ b/gmd-core/release.md @@ -1,6 +1,10 @@ # Gmd Release History ## v3.2.0, unreleased +- Indented ```{groovy} fences now execute with their body dedented by the fence's own + indent, and the echoed fence and body line up. Visible change: a multi-line string + literal whose continuation lines sit below the fence indent loses those spaces in the + evaluated source as well as the echo. - render `Chart`/`MatrixXChart` output (and `Html`) as a raw `` HTML tag instead of `![alt](data-uri){attrs}`, since CommonMark's image syntax doesn't support Pandoc-style attribute suffixes. This means `outputType=md` output can now contain raw HTML for charts instead of portable Markdown image syntax; the tag is followed by a blank line so it doesn't swallow subsequent Markdown as an HTML block. - restore the `HtmlDecorator.BOOTSTRAP_CSS` and `HIGHLIGHT_JS_CSS` compatibility constants - upgrade the JUnit BOM from 6.1.2 to 6.1.3 diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index 04a4abe..a822ec5 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -23,6 +23,15 @@ class GmdTemplateEngine { * ``` * Hello World * + * Indentation is lenient by design: a ```{groovy} fence is recognised at any + * indent, so a .gmd document written inside an indented Groovy string still + * works. The body is dedented by the fence's own indent so the two line up. + * The consequence is that an indented block cannot be used to *show* GMD + * syntax without running it - wrap such an example in a longer plain fence + * (````) instead. Only spaces are counted; a tab-indented fence is treated + * as column 0. The echoed fence is always emitted at column 0, so a Groovy + * block written inside a list item does not stay inside that item - the + * emitted block ends the list and the next item starts a new one. * @param text the gmd text to process * @return the gmd text with code blocks "expanded" */ @@ -45,6 +54,7 @@ class GmdTemplateEngine { boolean echo = true Character plainFenceChar = null int plainFenceLength = 0 + int codeBlockIndent = 0 String noSpaceLine StringBuilder codeBlockText = new StringBuilder() StringBuilder result = new StringBuilder() @@ -66,6 +76,7 @@ class GmdTemplateEngine { shouldBeProcessed = true codeBlockStart = true codeBlockEnd = false + codeBlockIndent = leadingSpaces(line) // echo is a property of one block, not of the entire document. echo = true if (noSpaceLine.toLowerCase().contains("echo=false")) { @@ -88,7 +99,7 @@ class GmdTemplateEngine { } if (codeBlockStart) { - codeBlockText.append(line).append('\n') + codeBlockText.append(dedent(line, codeBlockIndent)).append('\n') } if (codeBlockEnd) { @@ -174,6 +185,24 @@ class GmdTemplateEngine { return line.substring(indent) } + /** Number of leading space characters. Tabs are not counted; see processCodeBlocks. */ + private static int leadingSpaces(String line) { + int i = 0 + while (i < line.length() && line.charAt(i) == ' ') { + i++ + } + return i + } + + /** Removes up to {@code width} leading spaces. Shorter indents are left untouched. */ + private static String dedent(String line, int width) { + int i = 0 + while (i < width && i < line.length() && line.charAt(i) == ' ') { + i++ + } + return line.substring(i) + } + /** Length of the leading run of marker characters, i.e. the fence width. */ private static int fenceRunLength(String line, String marker) { int i = 0 diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index a3acb07..654eacf 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -69,15 +69,73 @@ Hello World assertEquals(""" Before ```groovy - // just some Groovy code - def x = 5 - out.println('Hello World') +// just some Groovy code +def x = 5 +out.println('Hello World') ``` Hello World After """, GmdTemplateEngine.processCodeBlocks(text)) } + @Test + void anIndentedGroovyFenceDedentsItsBody() { + String text = ''' +Example: + + ```{groovy} + out.println('EXECUTED') + ``` + +end +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains("```groovy\nout.println('EXECUTED')\n```"), + "The echoed body must line up with the fence that wraps it:\n$processed") + assertTrue(processed.contains('EXECUTED\n'), processed) + } + + @Test + void dedentOnlyRemovesTheFenceIndent() { + String text = ''' + ```{groovy} + out.println('deep') + ``` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains("```groovy\n out.println('deep')\n```"), + "Indentation beyond the fence indent must be preserved:\n$processed") + } + + @Test + void aColumnZeroFenceLeavesItsBodyAlone() { + String text = ''' +```{groovy} + out.println('indented body') +``` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains("```groovy\n out.println('indented body')\n```"), + "A fence at column 0 dedents by 0:\n$processed") + } + + @Test + void dedentAppliesToTheEvaluatedSource() { + String text = ''' + ```{groovy echo=false} + out.print("""a + b""") + ``` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('a\nb'), + "The dedent applies to the evaluated source too:\n$processed") + } + @Test void testInlineVars() { def text = """ From 05faf8ca2a3b647327798dbf78903f38bc5a6ee9 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:34:30 +0200 Subject: [PATCH 02/24] fix: do not expand inline variables inside indented code blocks --- gmd-core/release.md | 5 + .../alipsa/gmd/core/GmdTemplateEngine.groovy | 63 +++++++- .../groovy/gmd/GmdTemplateEngineTest.groovy | 143 +++++++++++++++++- 3 files changed, 206 insertions(+), 5 deletions(-) diff --git a/gmd-core/release.md b/gmd-core/release.md index d8add86..00dd517 100644 --- a/gmd-core/release.md +++ b/gmd-core/release.md @@ -1,6 +1,11 @@ # Gmd Release History ## v3.2.0, unreleased +- Inline `= expr ` is no longer expanded inside CommonMark indented code blocks (a line + indented four or more spaces beyond the document's own base indent). Indent is measured + relative to the indent of the document's first non-blank line, so a .gmd written inside an + indented Groovy string is unaffected. Lazy paragraph continuations and list-item + continuations still expand. - Indented ```{groovy} fences now execute with their body dedented by the fence's own indent, and the echoed fence and body line up. Visible change: a multi-line string literal whose continuation lines sit below the fence indent loses those spaces in the diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index a822ec5..13f1437 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -32,6 +32,11 @@ class GmdTemplateEngine { * as column 0. The echoed fence is always emitted at column 0, so a Groovy * block written inside a list item does not stay inside that item - the * emitted block ends the list and the next item starts a new one. + * For inline-variable expansion, the document base is the first non-blank + * line's indent. That is an invariant, not a global minimum: if the first + * line is shallower than a body indented four or more spaces beyond it, the + * body is treated as literal and does not expand. Put the first non-blank + * line at the document's own indent. * @param text the gmd text to process * @return the gmd text with code blocks "expanded" */ @@ -55,11 +60,15 @@ class GmdTemplateEngine { Character plainFenceChar = null int plainFenceLength = 0 int codeBlockIndent = 0 + boolean inIndentedCode = false + boolean previousLineWasParagraph = false + int listContentColumn = -1 String noSpaceLine StringBuilder codeBlockText = new StringBuilder() StringBuilder result = new StringBuilder() boolean endsWithNewline = text.endsWith('\n') List lines = text.readLines() + int documentIndent = baseIndent(lines) int count = 0 lines.each { line -> noSpaceLine = fenceCandidate(line) @@ -122,8 +131,38 @@ class GmdTemplateEngine { out.clear() codeBlockText.setLength(0) codeBlockEnd = false + inIndentedCode = false + previousLineWasParagraph = false + listContentColumn = -1 } else if (!codeBlockStart) { - if (plainFenceChar == null && line.contains('`=')) { + if (plainFenceChar != null || startsPlainFence) { + inIndentedCode = false + previousLineWasParagraph = false + } else if (line.isBlank()) { + previousLineWasParagraph = false + } else { + int relativeIndent = leadingSpaces(line) - documentIndent + def listMarker = line =~ /^\s*([-*+]|\d+[.)])\s+/ + if (relativeIndent < 4 && listMarker.find()) { + listContentColumn = listMarker.end() - documentIndent + inIndentedCode = false + } else { + if (listContentColumn >= 0 && relativeIndent < listContentColumn) { + listContentColumn = -1 + } + if (listContentColumn >= 0 && relativeIndent < listContentColumn + 4) { + inIndentedCode = false + } else { + if (relativeIndent < 4) { + inIndentedCode = false + } else if (!previousLineWasParagraph) { + inIndentedCode = true + } + } + } + previousLineWasParagraph = !inIndentedCode + } + if (plainFenceChar == null && !inIndentedCode && line.contains('`=')) { shouldBeProcessed = true result.append(expandInlineVars(line, engine)) } else { @@ -194,6 +233,28 @@ class GmdTemplateEngine { return i } + /** + * The indent the document sits at: the indent of its first non-blank line. + * Indented code blocks are recognised relative to this, so a .gmd written + * inside an indented Groovy string behaves the same as one written at + * column 0. + * + * Anchored on the first line deliberately: a global minimum could be + * collapsed by a column-0 line inside a code block or a stray column-0 + * prose line (HTML block, table row), reclassifying the whole document. + * The first line is therefore the invariant: when it is shallower than + * the body, body lines four or more spaces beyond it are literal. Authors + * must start a document at its own indent. + */ + private static int baseIndent(List lines) { + for (String line in lines) { + if (!line.isBlank()) { + return leadingSpaces(line) + } + } + return 0 + } + /** Removes up to {@code width} leading spaces. Shorter indents are left untouched. */ private static String dedent(String line, int width) { int i = 0 diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index 654eacf..d07984a 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -310,17 +310,152 @@ After } @Test - void indentedCodeBlocksAreAKnownGap() { + void indentedCodeBlocksAreLeftAlone() { String text = ''' ```{groovy echo=false} x = 5 ``` - literal `= x` is expanded here + literal `= x ` is not expanded here ''' String processed = GmdTemplateEngine.processCodeBlocks(text) - assertTrue(processed.contains('literal 5 is expanded here'), - "Known gap: indented code blocks are not protected:\n$processed") + assertTrue(processed.contains('literal `= x ` is not expanded here'), + "An indented code block is literal:\n$processed") + assertFalse(processed.contains('literal 5'), processed) + } + + @Test + void indentIsRelativeToTheDocumentBaseIndent() { + String text = """ + ```{groovy echo=false} + x = 5 + ``` + expanded `= x ` here + + literal `= x ` here + """ + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('expanded 5 here'), processed) + assertTrue(processed.contains('literal `= x ` here'), processed) + } + + @Test + void anIndentedLineContinuingAParagraphIsStillExpanded() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +Some prose + continues here with `= x ` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('continues here with 5'), processed) + } + + @Test + void aBlankLineDoesNotEndAnIndentedCodeBlock() { + String text = ''' +```{groovy echo=false} +x = 5 +``` + first `= x ` literal + + second `= x ` literal +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('first `= x ` literal'), processed) + assertTrue(processed.contains('second `= x ` literal'), processed) + } + + @Test + void aTabIndentedLineIsNotTreatedAsACodeBlock() { + String text = "```{groovy echo=false}\nx = 5\n```\n\tliteral `= x ` here\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal 5 here'), + "Known limitation: tabs are not counted as indentation:\n$processed") + } + + @Test + void crlfLinesAreClassifiedCorrectly() { + String text = "```{groovy echo=false}\r\nx = 5\r\n```\r\n literal `= x ` here\r\n \r\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), processed) + } + + @Test + void baseIndentIgnoresCodeBlockBodies() { + String text = """ + ```{groovy echo=false} + x = 5 + multi = '''line one +line two''' + ``` + Value `= x ` + """ + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('Value 5'), + "A column-0 line inside a code block must not change the document's base indent:\n$processed") + } + + @Test + void aStrayColumnZeroLineDoesNotCollapseTheBaseIndent() { + String text = """ + ```{groovy echo=false} + x = 5 + ``` +
html at column 0
+ expanded `= x ` here + """ + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('expanded 5 here'), + "A column-0 line must not collapse the base indent:\n$processed") + } + + @Test + void aListItemContinuationStillExpands() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +1. item + + continued `= x ` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('continued 5'), processed) + } + + @Test + void indentedCodeAfterAListIsLeftAlone() { + String text = ''' +```{groovy echo=false} +x = 5 +``` + +- item + +text + + literal `= x ` here +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), + "An indented code block after an ended list is literal:\n$processed") + assertFalse(processed.contains('literal 5'), processed) } @Test From 763ecb5ceb1c0c583e9fd948d8532902345f9d4c Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:35:01 +0200 Subject: [PATCH 03/24] fix: terminate unterminated code block output with a newline --- gmd-core/release.md | 4 ++++ .../alipsa/gmd/core/GmdTemplateEngine.groovy | 3 +++ .../groovy/gmd/GmdTemplateEngineTest.groovy | 21 +++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/gmd-core/release.md b/gmd-core/release.md index 00dd517..a981da3 100644 --- a/gmd-core/release.md +++ b/gmd-core/release.md @@ -1,6 +1,10 @@ # Gmd Release History ## v3.2.0, unreleased +- Output from a Groovy code block that does not end in a newline now gains one, so + `out.print('x')` no longer glues the next source line onto the output. Visible change: + `out.print('Hello ')` followed by prose now yields `Hello \nworld` instead of + `Hello world` in the Markdown (rendered identically by CommonMark). - Inline `= expr ` is no longer expanded inside CommonMark indented code blocks (a line indented four or more spaces beyond the document's own base indent). Indent is measured relative to the indent of the document's first non-blank line, so a .gmd written inside an diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index 13f1437..f8143bf 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -127,6 +127,9 @@ class GmdTemplateEngine { def output = out.toString() if (output.length() > 0) { result.append(output) + if (!output.endsWith('\n')) { + result.append('\n') + } } out.clear() codeBlockText.setLength(0) diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index d07984a..e2c7f8c 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -136,6 +136,27 @@ end "The dedent applies to the evaluated source too:\n$processed") } + @Test + void unterminatedCodeBlockOutputDoesNotSwallowTheNextLine() { + String text = "```{groovy echo=false}\nout.print('X')\n```\n# Heading\n" + + assertEquals("X\n# Heading\n", GmdTemplateEngine.processCodeBlocks(text)) + } + + @Test + void alreadyTerminatedOutputGainsNoExtraNewline() { + String text = "```{groovy echo=false}\nout.println('X')\n```\n# Heading\n" + + assertEquals("X\n# Heading\n", GmdTemplateEngine.processCodeBlocks(text)) + } + + @Test + void inlineOutPrintIsTerminatedBeforeTheNextLine() { + String text = "```{groovy echo=false}\nout.print('Hello ')\n```\nworld\n" + + assertEquals("Hello \nworld\n", GmdTemplateEngine.processCodeBlocks(text)) + } + @Test void testInlineVars() { def text = """ From 5c8d67773c44bc8175443cc51e66e6a30ed0e405 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:36:14 +0200 Subject: [PATCH 04/24] fix: GgChart printing always honours configured dimensions --- README.md | 5 ++-- gmd-core/release.md | 3 ++ .../groovy/se/alipsa/gmd/core/Printer.groovy | 28 +++++++++++-------- .../test/alipsa/groovy/gmd/GmdTest.groovy | 25 +++++++++++++++++ 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 742bddf..ea94278 100644 --- a/README.md +++ b/README.md @@ -91,8 +91,9 @@ GMD supports the [Matrix](https://github.com/Alipsa/matrix) library directly. `M `MatrixXChart` types can be used with the `out` PrintWriter object without needing to convert them into markdown first. Pict, Charm, and GG charts accept `width`, `height`, optional alt text, and optional HTML attributes (for example, `out.println(chart, 640, 480, 'Sales chart', [class: 'chart'])`). -They also accept alt text and attributes without explicit dimensions. Alt-only GG printing honors -the dimensions configured on the chart; explicit GG dimensions are restored after rendering. +They also accept alt text and attributes without explicit dimensions. Pict and Charm charts then +render at 800x600. GG charts always honor the `width` and `height` configured on the chart unless +you pass explicit dimensions, which are restored after rendering. MatrixXChart accepts optional alt text and HTML attributes. Here is an example: ```` diff --git a/gmd-core/release.md b/gmd-core/release.md index a981da3..244c62e 100644 --- a/gmd-core/release.md +++ b/gmd-core/release.md @@ -1,6 +1,9 @@ # Gmd Release History ## v3.2.0, unreleased +- **Breaking:** the `print(GgChart, double)` / `println(GgChart, double)` width-only + overload that Groovy default arguments generated is removed. Pass explicit width and + height, or omit both and let the chart's own dimensions apply. - Output from a Groovy code block that does not end in a newline now gains one, so `out.print('x')` no longer glues the next source line onto the output. Visible change: `out.print('Hello ')` followed by prose now yields `Hello \nworld` instead of diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy index def4b64..83b1452 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy @@ -161,20 +161,14 @@ class Printer extends PrintWriter { println(chartToMd(x, DEFAULT_CHART_WIDTH, DEFAULT_CHART_HEIGHT, alt, attributes)) } - /** - * Renders a GgChart at the supplied dimensions. The chart's original - * dimensions are restored before this method returns. - */ - void print(GgChart x, double width = DEFAULT_CHART_WIDTH, double height = DEFAULT_CHART_HEIGHT, String alt = '', Map attributes = [:]) { - printChart(chartToMd(x, width, height, alt, attributes)) + /** Renders a GgChart at the dimensions configured on the chart itself. */ + void print(GgChart x) { + printChart(svgToMd(x.render(), '', [:])) } - /** - * Renders a GgChart at the supplied dimensions. The chart's original - * dimensions are restored before this method returns. - */ - void println(GgChart x, double width = DEFAULT_CHART_WIDTH, double height = DEFAULT_CHART_HEIGHT, String alt = '', Map attributes = [:]) { - println(chartToMd(x, width, height, alt, attributes)) + /** @see #print(GgChart) */ + void println(GgChart x) { + println(svgToMd(x.render(), '', [:])) } void print(GgChart x, String alt, Map attributes = [:]) { @@ -185,6 +179,16 @@ class Printer extends PrintWriter { println(svgToMd(x.render(), alt, attributes)) } + /** Renders a GgChart at supplied dimensions, restoring its own afterwards. */ + void print(GgChart x, double width, double height, String alt = '', Map attributes = [:]) { + printChart(chartToMd(x, width, height, alt, attributes)) + } + + /** @see #print(GgChart, double, double, String, Map) */ + void println(GgChart x, double width, double height, String alt = '', Map attributes = [:]) { + println(chartToMd(x, width, height, alt, attributes)) + } + void print(MatrixXChart x, String alt = '', Map attributes = [:]) { printChart(chartToMd(x, alt, attributes)) } diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy index 78172b9..1a99a14 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy @@ -596,6 +596,8 @@ chart.width = 1200 chart.height = 900 out.println(chart, 640, 480, 'GG chart', [class: 'chart']) out.println(chart, 'GG default chart') +out.println(chart) +out.println(chart, 640.5, 480.5, 'GG BigDecimal dims') ``` ''' @@ -606,6 +608,29 @@ out.println(chart, 'GG default chart') List svgs = decodeSvgImages(md, 'GG chart should be SVG-backed') assertTrue(svgs[0].contains('width="640"') && svgs[0].contains('height="480"'), svgs[0].take(200)) assertTrue(svgs[1].contains('width="1200"') && svgs[1].contains('height="900"'), svgs[1].take(200)) + assertTrue(svgs[2].contains('width="1200"') && svgs[2].contains('height="900"'), + "print(GgChart) with no arguments must honour the chart's own dimensions: ${svgs[2].take(200)}") + assertTrue(svgs[3].contains('width="640"') && svgs[3].contains('height="480"'), + "double dimensions must accept Groovy's default decimal type (BigDecimal): ${svgs[3].take(200)}") + } + + @Test + void aGgChartWithNoConfiguredSizeRendersAtTheLibraryDefault() { + def text = ''' +```{groovy echo=false} +import static se.alipsa.matrix.gg.GgPlot.* +import se.alipsa.matrix.core.Matrix + +def data = Matrix.builder().data(x: [1, 2], y: [3, 4]).types(int, int).build() +out.println(ggplot(data, aes(x: 'x', y: 'y')) + geom_point()) +``` +''' + + List svgs = decodeSvgImages(new Gmd().gmdToMd(text), 'An unsized GG chart should still render') + + assertTrue(svgs[0].contains(' decodeSvgImages(String markdown, String message) { From cb85a45cec302969945107812951fa347d7a13ec Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:40:04 +0200 Subject: [PATCH 05/24] fix: only add Maven Central when the Gradle project has none --- gmd-gradle-plugin/readme.md | 9 ++ gmd-gradle-plugin/release.md | 3 + .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 22 +++-- .../gmd/gradle/GmdGradlePluginTest.groovy | 82 +++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) diff --git a/gmd-gradle-plugin/readme.md b/gmd-gradle-plugin/readme.md index da6f69b..e23d6b9 100644 --- a/gmd-gradle-plugin/readme.md +++ b/gmd-gradle-plugin/readme.md @@ -32,6 +32,15 @@ embedded styles, and syntax-highlighted code blocks. - `ivyVersion` - the version of ivy to use. Default is `2.6.0` - `runTaskBefore` - the task that the gmd plugin should run before. Default is 'test' +### Repositories + +The plugin resolves `gmd-core` and Groovy through a detached configuration, which uses the +project's repositories. If the project has not declared Maven Central, the plugin adds it. +If it has — under either `repo.maven.apache.org` or `repo1.maven.org` — nothing is added. +A project that resolves only through a private mirror should declare that mirror in +`dependencyResolutionManagement` in `settings.gradle` so the plugin has no reason to add +Central to the project itself. + The target task is called `processGmd` so it can be invoked from the command line as follows: ```bash diff --git a/gmd-gradle-plugin/release.md b/gmd-gradle-plugin/release.md index 39e8970..a169ca3 100644 --- a/gmd-gradle-plugin/release.md +++ b/gmd-gradle-plugin/release.md @@ -1,6 +1,9 @@ # GMD Gradle Plugin release history ## v3.2.0, unreleased +- **Breaking:** `GmdGradlePlugin.hasRepository(Project, MavenArtifactRepository)` is removed. + It was public static API on the plugin class; nothing in the repo called it. The plugin now + checks for an existing Maven Central declaration itself before adding one. - derive the default GMD core version from a generated plugin-version resource populated from the root Maven revision, failing clearly if metadata is unavailable - derive the development artifact version from the root Maven revision and require an explicit version when publishing - upgrade the Gradle Versions Plugin from 0.58.0 to 0.61.0 diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index 28c989b..483194e 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -4,6 +4,7 @@ import groovy.transform.CompileStatic import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.InvalidUserCodeException import org.gradle.api.artifacts.Configuration import org.gradle.api.artifacts.Dependency import org.gradle.api.artifacts.repositories.ArtifactRepository @@ -17,6 +18,8 @@ import java.util.Properties @CompileStatic class GmdGradlePlugin implements Plugin { + private static final List MAVEN_CENTRAL_HOSTS = ['repo.maven.apache.org', 'repo1.maven.org'] + @Override void apply(Project project) { GmdGradlePluginParams extension = project.extensions.create('gmdPlugin', GmdGradlePluginParams) @@ -107,9 +110,13 @@ class GmdGradlePlugin implements Plugin { static Configuration addDependencies(Project project, String groovyVersion, String log4jVersion, String gmdVersion, String ivyVersion) { - MavenArtifactRepository mavenCentral = project.repositories.mavenCentral() - if (!hasRepository(project, mavenCentral)) { - project.repositories.add(mavenCentral) + if (!hasMavenCentral(project)) { + try { + project.repositories.mavenCentral() + } catch (InvalidUserCodeException e) { + project.logger.info('Project repositories are managed in settings.gradle; ' + + 'resolving gmd-core through the settings repositories instead: ' + e.message) + } } List dependencies = [ @@ -124,9 +131,10 @@ class GmdGradlePlugin implements Plugin { return project.configurations.detachedConfiguration(dependencies.toArray(new Dependency[0])) } - static boolean hasRepository(Project project, MavenArtifactRepository repo) { - return project.repositories.find { - it instanceof MavenArtifactRepository && it.url == repo.url - } != null + static boolean hasMavenCentral(Project project) { + return project.repositories.any { ArtifactRepository repository -> + repository instanceof MavenArtifactRepository && + MAVEN_CENTRAL_HOSTS.contains(((MavenArtifactRepository) repository).url?.host) + } } } diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 0b29a16..5d389b6 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -81,6 +81,88 @@ class GmdGradlePluginTest { 'targetDirIsDefaultGmdOutput should default to false when set only by convention') } + @Test + void mavenCentralIsAddedOnlyWhenTheProjectHasNone() { + def project = ProjectBuilder.builder().build() + Assertions.assertEquals(0, project.repositories.size()) + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(1, project.repositories.size()) + } + + @Test + void anExistingMavenCentralIsNotDuplicated() { + def project = ProjectBuilder.builder().build() + project.repositories.mavenCentral() + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(1, project.repositories.size()) + } + + @Test + void mavenCentralIsRecognisedUnderItsOtherHost() { + def project = ProjectBuilder.builder().build() + project.repositories.maven { it.setUrl('https://repo1.maven.org/maven2') } + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(1, project.repositories.size()) + } + + @Test + void aPrivateMirrorStillGetsMavenCentralAdded() { + def project = ProjectBuilder.builder().build() + project.repositories.maven { it.setUrl('https://nexus.example.com/repository/maven-public') } + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(2, project.repositories.size()) + } + + @Test + void settingsManagedRepositoriesDoNotBreakTheBuild() { + File testProjectDir = new File('build/gmdSettingsReposTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'test.gmd').text = '# Greetings\n' + new File(testProjectDir, 'settings.gradle').text = ''' + pluginManagement { + repositories { mavenCentral() } + plugins { id 'se.alipsa.gmd.gmd-gradle-plugin' version '1.0.0' } + } + dependencyResolutionManagement { + repositoriesMode = RepositoriesMode.FAIL_ON_PROJECT_REPOS + repositories { mavenCentral() } + } + '''.stripIndent() + new File(testProjectDir, 'build.gradle').text = ''' + plugins { + id('base') + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + gmdPlugin { + sourceDir = 'src/test/gmd' + targetDir = 'build/target' + outputType = 'html' + gmdVersion = '3.1.0' + runTaskBefore = 'build' + } + '''.stripIndent() + + def result = GradleRunner.create().withProjectDir(testProjectDir) + .withArguments('processGmd').withPluginClasspath().forwardOutput().build() + + Assertions.assertEquals(SUCCESS, result.task(':processGmd').outcome, result.output) + Assertions.assertTrue(new File(testProjectDir, 'build/target/test.html').exists()) + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } + @Test void testPlugin() { File targetDir = null From a6c730dfa050d5ebb8dc937038fb1b907bc57ee6 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:40:28 +0200 Subject: [PATCH 06/24] fix: report nothing to do when Gradle source dir has no gmd files --- .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 25 +++++++----- .../gmd/gradle/GmdGradlePluginTest.groovy | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 91e172a..029aa5b 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -63,8 +63,8 @@ abstract class ProcessGmdTask extends DefaultTask { File source = getSourceDir().get().asFile File target = getTargetDir().get().asFile String output = getOutputType().get().trim().toLowerCase(Locale.ROOT) - File[] sources = source.listFiles({ File file -> file.isFile() && file.name.endsWith('.gmd') } as FileFilter) - Set generated = sources == null ? [] as Set : sources.collect { File file -> + File[] sources = gmdFilesIn(source) + Set generated = sources.collect { File file -> new File(target, file.name.substring(0, file.name.length() - 4) + ".${output}") } as Set return generated @@ -113,8 +113,8 @@ abstract class ProcessGmdTask extends DefaultTask { spec.args = [source.canonicalPath, target.canonicalPath, output] } result.assertNormalExitValue() - File[] sourceFiles = source.listFiles() - if (sourceFiles != null && sourceFiles.size() > 0) { + File[] sourceFiles = gmdFilesIn(source) + if (sourceFiles.length > 0) { if (target.exists()) { logger.quiet("Gmd files processed and written to ${target.canonicalPath}") } else { @@ -127,12 +127,9 @@ abstract class ProcessGmdTask extends DefaultTask { private void cleanStaleGeneratedFiles(File sourceDir, File targetDir, String outputType) { Set expected = [] as Set - File[] sources = sourceDir.listFiles({ File file -> file.isFile() && file.name.endsWith('.gmd') } as FileFilter) - if (sources != null) { - sources.each { file -> - String base = file.name.substring(0, file.name.length() - 4) - expected.add("${base}.${outputType}".toString()) - } + gmdFilesIn(sourceDir).each { file -> + String base = file.name.substring(0, file.name.length() - 4) + expected.add("${base}.${outputType}".toString()) } File[] generated = targetDir.listFiles({ File file -> file.isFile() && (file.name.endsWith('.md') || file.name.endsWith('.html') || file.name.endsWith('.pdf')) @@ -147,4 +144,12 @@ abstract class ProcessGmdTask extends DefaultTask { } } } + + /** The .gmd files in a directory, or an empty array when it cannot be read. */ + private static File[] gmdFilesIn(File directory) { + File[] files = directory.listFiles({ File file -> + file.isFile() && file.name.endsWith('.gmd') + } as FileFilter) + return files == null ? new File[0] : files + } } diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 5d389b6..6d31e8a 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -163,6 +163,45 @@ class GmdGradlePluginTest { } } + @Test + void aSourceDirWithoutGmdFilesReportsNothingToDo() { + File testProjectDir = new File('build/gmdNoGmdFilesTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'readme.txt').text = 'not a gmd file' + new File(testProjectDir, 'settings.gradle').text = ''' + pluginManagement { + repositories { mavenCentral() } + plugins { id 'se.alipsa.gmd.gmd-gradle-plugin' version '1.0.0' } + } + '''.stripIndent() + new File(testProjectDir, 'build.gradle').text = ''' + plugins { + id('base') + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + repositories { mavenCentral() } + gmdPlugin { + sourceDir = 'src/test/gmd' + targetDir = 'build/target' + outputType = 'html' + gmdVersion = '3.1.0' + runTaskBefore = 'build' + } + '''.stripIndent() + + def result = GradleRunner.create().withProjectDir(testProjectDir) + .withArguments('processGmd').withPluginClasspath().forwardOutput().build() + + Assertions.assertTrue(result.output.contains('No gmd files found in'), result.output) + Assertions.assertFalse(result.output.contains('Gmd files processed and written to'), result.output) + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } + @Test void testPlugin() { File targetDir = null From d0c0cbeda84a7b77fd2af368fd6053ecb0709c06 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:42:10 +0200 Subject: [PATCH 07/24] fix: restore interrupt flag and stop double wrapping Mojo failures --- .../se/alipsa/gmd/maven/GmdMavenPlugin.java | 23 ++++++++++++++++++- .../alipsa/gmd/maven/GmdMavenPluginTest.java | 5 +++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java index 79bee30..e478384 100644 --- a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java +++ b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java @@ -7,6 +7,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.concurrent.TimeUnit; import javax.inject.Inject; import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.AbstractMojo; @@ -182,6 +183,7 @@ public void execute() throws MojoExecutionException, MojoFailureException { // command-line length limit on Windows. File argFile = createArgFile(classpath.toString(), srcDir.getCanonicalPath(), outputDirectory.getCanonicalPath(), normalizedOutputType); + boolean interrupted = false; try { List command = new ArrayList<>(); command.add(getJavaExecutable()); @@ -190,7 +192,21 @@ public void execute() throws MojoExecutionException, MojoFailureException { ProcessBuilder processBuilder = new ProcessBuilder(command); processBuilder.inheritIO(); Process process = processBuilder.start(); - int exitCode = process.waitFor(); + int exitCode; + try { + exitCode = process.waitFor(); + } catch (InterruptedException e) { + process.destroy(); + try { + if (!process.waitFor(5, TimeUnit.SECONDS)) { + process.destroyForcibly(); + } + } catch (InterruptedException swallowed) { + process.destroyForcibly(); + } + interrupted = true; + throw new MojoExecutionException("Interrupted while waiting for the GMD processor", e); + } if (exitCode != 0) { throw new MojoFailureException("GmdProcessor exited with code " + exitCode); @@ -205,6 +221,9 @@ public void execute() throws MojoExecutionException, MojoFailureException { getLog().warn("Could not delete temporary classpath argfile " + argFile.getAbsolutePath() + ": " + e.getMessage()); } + if (interrupted) { + Thread.currentThread().interrupt(); + } } } else { // Fall back to using GmdProcessor directly with bundled dependencies @@ -219,6 +238,8 @@ public void execute() throws MojoExecutionException, MojoFailureException { } else { getLog().warn(td.getCanonicalPath() + " should exists but does not, something is probably wrong"); } + } catch (MojoExecutionException | MojoFailureException e) { + throw e; } catch (DependencyResolutionException e) { throw new MojoExecutionException("Failed to resolve dependencies", e); } catch (Exception e) { diff --git a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java index 799d5ad..0f14331 100644 --- a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java +++ b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java @@ -133,7 +133,10 @@ public void testGmdMavenPluginReportsForkFailure() throws Exception { setField(plugin, "outputType", "html"); MojoFailureException exception = assertThrows(MojoFailureException.class, plugin::execute); - assertTrue(exception.getMessage().contains("GmdProcessor exited with code")); + assertTrue(exception.getMessage().startsWith("GmdProcessor exited with code"), + "The plugin's own failure must not be rewrapped: " + exception.getMessage()); + assertFalse(exception.getMessage().contains("Failed to process gmd files in"), + "The plugin's own failure must not be rewrapped: " + exception.getMessage()); } private static void deleteDirectory(File directory) throws IOException { From 39cb09ca6899f0b00c1436eaa9c418124f139a87 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:44:31 +0200 Subject: [PATCH 08/24] fix: toHtml creates missing parent directories like toPdf --- .../se/alipsa/gmd/core/GmdCommandLine.groovy | 3 +-- .../test/alipsa/groovy/gmd/GmdTest.groovy | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy index 43a4be4..b35b0f0 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy @@ -26,9 +26,8 @@ class GmdCommandLine { static Closure toHtml(String from, String to) { return { Gmd gmd = new Gmd() - def html = gmdFileToHtml(from, gmd) File toFile = new File(to) - Files.writeString(toFile.toPath(), html, StandardCharsets.UTF_8) + gmd.gmdToHtml(readGmd(from), toFile) println "Wrote $toFile.absolutePath" } } diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy index 1a99a14..0ec8424 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy @@ -763,6 +763,29 @@ out.println(chart) assertTrue(pdfFile.length() > 100, "No pdf content") } + @Test + void commandLineToHtmlCreatesMissingParentDirectories() { + String file = getClass().getResource('/test.gmd').getFile() + File htmlFile = new File(testOutputDir, 'nested/output/dir/cli.html') + assertFalse(htmlFile.parentFile.exists(), 'The parent directory must not exist yet') + + Gmd.main('toHtml', file, htmlFile.absolutePath) + + assertTrue(htmlFile.exists(), 'toHtml must create the parent directories, as toPdf does') + assertTrue(htmlFile.length() > 100, 'No html content') + } + + @Test + void commandLineToPdfCreatesMissingParentDirectories() { + String file = getClass().getResource('/test.gmd').getFile() + File pdfFile = new File(testOutputDir, 'nested/pdf/dir/cli.pdf') + + Gmd.main('toPdf', file, pdfFile.absolutePath) + + assertTrue(pdfFile.exists()) + assertTrue(pdfFile.length() > 100, 'No pdf content') + } + @Test void testEnsureResultDoesNotPrint() { String text = """ From c382c5e4f02e585a0ccdae1bc5d098285b8ca733 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:48:44 +0200 Subject: [PATCH 09/24] test: keep newline coverage within the planned test count --- .../test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index e2c7f8c..563e83e 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -141,11 +141,8 @@ end String text = "```{groovy echo=false}\nout.print('X')\n```\n# Heading\n" assertEquals("X\n# Heading\n", GmdTemplateEngine.processCodeBlocks(text)) - } - @Test - void alreadyTerminatedOutputGainsNoExtraNewline() { - String text = "```{groovy echo=false}\nout.println('X')\n```\n# Heading\n" + text = "```{groovy echo=false}\nout.println('X')\n```\n# Heading\n" assertEquals("X\n# Heading\n", GmdTemplateEngine.processCodeBlocks(text)) } From eed98375988090c98c5d2aa57c5304e1d298b146 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 16:51:19 +0200 Subject: [PATCH 10/24] docs: add v3.2.0 review fixes plan --- req/v3.2.0-fixes.md | 1746 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 1746 insertions(+) create mode 100644 req/v3.2.0-fixes.md diff --git a/req/v3.2.0-fixes.md b/req/v3.2.0-fixes.md new file mode 100644 index 0000000..6c1bf6c --- /dev/null +++ b/req/v3.2.0-fixes.md @@ -0,0 +1,1746 @@ +# GMD 3.2.0 Review Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Clear the eight findings from the 3.2.0 code review — two template-engine indentation bugs, a code-block output bug, a GgChart dimension asymmetry, a Gradle repository side effect, a misleading Gradle log message, a swallowed Maven interrupt, and a CLI path bug. + +**Architecture:** Seven of the eight fixes are local and independent. The two template-engine fixes (Tasks 1 and 2) touch the same loop in `GmdTemplateEngine.processCodeBlocks` and must land in order. Both rest on the same new idea: **indentation is measured relative to the document's own base indent**, not absolutely. That is what lets GMD keep its existing leniency (a `.gmd` document written inside an indented Groovy triple-quoted string still works) while still honouring CommonMark's rule that a block indented four spaces *beyond the surrounding text* is literal. + +**Tech Stack:** Java 21, Groovy 5.1.3, Maven 3.9.9+, Gradle 9.7.1, JUnit Jupiter 6.1.3, Jsoup 1.23.2, Mockito (maven-plugin tests), Gradle TestKit + ProjectBuilder (gradle-plugin tests). + +**Spec:** This plan implements the findings recorded in the 3.2.0 code review; each task restates its finding in full, so no separate spec document is needed. + +## Global Constraints + +- Java 21 exactly (`maven.compiler.release=21`, enforced by maven-enforcer-plugin). +- The version lives only in `/pom.xml` `` (currently `3.2.0-SNAPSHOT`). Never hardcode a version anywhere else. +- **Baseline, measured on this checkout before any task:** `highlightjs-jvm` 15, `gmd-core` 67, `gmd-maven-plugin` 3 — 85 total via `mvn -o test`. The Gradle plugin builds separately (`cd gmd-gradle-plugin && ./gradlew test`). +- **Every task ends with the full suite green.** No task may leave a known-failing test for a later task to fix. +- Commit after every task. Conventional commit prefixes (`fix:`, `docs:`, `refactor:`). +- Do not reformat or restructure files beyond what a task asks for. These are surgical fixes to a released codebase. +- Three existing tests assert the *current* (buggy or asymmetric) behaviour and are deliberately rewritten by this plan: `GmdTemplateEngineTest.testNoEcho` (Task 1), `GmdTemplateEngineTest.indentedCodeBlocksAreAKnownGap` (Task 2), `GmdTest.testGgChart` (Task 4). No other existing test may be weakened or deleted. If a task makes an unrelated test fail, stop and report rather than editing that test. + +## Decisions already taken + +These were settled before planning; do not relitigate them. + +1. **Indented fences keep executing.** A ` ```{groovy} ` fence indented four or more spaces still runs. GMD's leniency here is load-bearing: `GmdTemplateEngineTest.testCodeOnly` and `testNoEcho` embed twelve-space-indented documents with no `stripIndent()`, and real users write `.gmd` inside indented Groovy strings. Task 1 therefore fixes only the *indent mismatch* the current code produces, not the execution itself. +2. **Known limitation, accepted:** because of (1), a `.gmd` document that tries to *show* GMD syntax by indenting it will still execute it. The documented workaround is a fenced block with a longer fence, which Task 1 leaves working. Task 1 records this in the method Javadoc. +3. **Gradle:** check for an existing Maven Central repository before calling `mavenCentral()`. The plugin still adds Central when the consumer has none. +4. **GgChart:** align on *always honouring the dimensions configured on the chart*, so only an explicit `width`/`height` argument overrides them. +5. **Echoed fences stay at column 0.** Task 1 makes the fence and its body agree by dedenting the body, not by re-emitting the fence at its original indent. Column 0 is always a valid fence position, so this can never turn the emitted ` ```groovy ` line into an indented code block. The cost, accepted here, is that a Groovy block written inside a list item is *not* kept inside that item: the emitted block breaks the list and the following item restarts as a new `
    `. That is the behaviour today as well, so Task 1 neither fixes nor worsens it. Preserving list nesting would need the fence re-emitted at `min(indent, 3)`, the body dedented by the remainder, and the captured output indented to match - a wider change, out of scope for 3.2.0. + +## Review Focus + +Input classes these fixes imply but that no task's happy path exercises. Each has a test assigned to the task that owns the code. + +1. **Tab-indented lines.** CommonMark counts a tab as four columns; the new indent helpers count spaces only, so a tab-indented literal block would still expand inline variables. → test in Task 2, pinning the documented space-only limitation. +2. **A uniformly indented document containing a genuinely indented literal block** (base indent + 4). Relative-indent classification must still protect it, or Task 2 only works for documents at column 0. → test in Task 2. +3. **CRLF input through the new indent helpers.** `readLines()` strips `\r`, but a whitespace-plus-`\r` line must still count as blank for paragraph tracking. → test in Task 2. +4. **A GgChart whose width/height were never set** (library defaults) printed with no arguments — must render at the library default, not 0×0 or NPE. → test in Task 4. +5. **A Gradle consumer that declared Central under its other host** (`repo1.maven.org` rather than `repo.maven.apache.org`), or with a trailing-slash difference. The URL check must recognise it, or Central is added twice. → test in Task 5. +6. **The first non-blank line defines the document base.** A stray column-0 line *after* an + otherwise uniformly indented first block (an HTML block, a table row, a hard-wrapped line) + must not collapse the base to 0 and silently stop later prose expanding inline variables. + Conversely, if that column-0 line is the first non-blank line and the body sits eight spaces + deeper, the body is classified as indented literal content and does not expand. That is the + accepted invariant: authors must put the first non-blank line at the document's own indent. + → the Task 2 test covers the later-stray-line case; the processCodeBlocks Javadoc records + both directions. + +## File Structure + +**Modified:** +- `gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy` — Tasks 1, 2, 3. The state machine in `processCodeBlocks` plus three new private static helpers. +- `gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy` — Task 4. Replaces the two `GgChart` default-argument methods with explicit overloads. +- `gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy` — Task 8. +- `gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy` — Task 5. +- `gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy` — Task 6. +- `gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java` — Task 7. +- `README.md` — Task 4 (GgChart dimension wording). +- `gmd-gradle-plugin/readme.md` — Task 5 (repository behaviour). +- `gmd-core/release.md` — Tasks 1, 2, 3, 4 (visible-behaviour entries in the live v3.2.0, unreleased section). +- `gmd-gradle-plugin/release.md` — Task 5 (the source-breaking `hasRepository` removal). + +**Test files modified:** +- `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy` — Tasks 1, 2, 3. +- `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy` — Tasks 4, 8. +- `gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy` — Tasks 5, 6. +- `gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java` — Task 7. + +--- + +Points worth your attention while reading: + +1. ~~Task 1, Step 8 rewrites testNoEcho's expected output.~~ **Settled — see decision (5).** Dedenting the body (rather than re-emitting the fence at its original indent) is the chosen approach, so a twelve-space-indented document emits its code body at column 0 while its prose keeps its indent. List nesting around Groovy blocks stays broken, as it is today; Step 6 records that in the Javadoc. +2. Task 2, Step 6 is the largest single edit — the classification block replacing lines 114-123. The relativeIndent >= 4 && previousLineWasParagraph case deliberately falls through leaving inIndentedCode unchanged; that's CommonMark's lazy-continuation rule and it's load-bearing for anIndentedLineContinuingAParagraphIsStillExpanded. +3. Task 4 drops the width-only `print(GgChart, double)` arity that Groovy's default arguments were generating. Nothing in the repo uses it. **Decision: remove it, do not shim it.** A shim could keep such a call compiling but not behaving: the generated arity meant "width w, height `DEFAULT_CHART_HEIGHT`", whereas after Task 4 the only sensible delegation is "width w, height from the chart" — a silent behaviour change for exactly the callers a shim claims to protect. Note the removal in the release notes instead. **The `double` parameter type is not part of this and must not be changed:** it is verified to accept every Groovy numeric type (`int`, `Long`, `Float`, and `BigDecimal` literals such as `640.5`), dynamically and under `@CompileStatic`. Narrowing it to `int` would reject `BigDecimal`; widening it to `Number` gains nothing and turns a call-site `MissingMethodException` into a late NPE inside `width as int`. +4. ~~Task 5 still adds Maven Central when the consumer has declared no repository at all.~~ **Settled - Task 5 now does both:** check first, and wrap the add in a `try/catch (InvalidUserCodeException)`. Measured on Gradle 9.7.1: under `FAIL_ON_PROJECT_REPOS` the settings repositories are not visible in `project.repositories` (size 0), so the check alone does not save us; and swallowing the rejection is safe because `detachedConfiguration` still resolves through the settings repositories. +5. ~~Unverified assumption in Task 5: `it.setUrl('...')` in the new ProjectBuilder tests.~~ **Verified:** Gradle 9.7.1 `MavenArtifactRepository` still declares both `void setUrl(Object)` and `URI getUrl()`, so the test form and the `url?.host` check in `hasMavenCentral` both compile. + +### Review round 2: findings against this plan, resolved + +`req/v3.2.0-review-findings.md` reviewed this plan; its open findings are folded in as follows. + +- **F1 (Task 2, Steps 1-4):** subsumed by review round 3 — `baseIndent` no longer scans + for a minimum at all. It anchors on the document's first non-blank line, so no line inside + a code block (or anywhere later in the document) can drag the base indent to 0 and + reclassify the whole document. The `baseIndentIgnoresCodeBlockBodies` guard is kept to pin + that a column-0 line inside a block must not affect classification. +- **F2 (Task 2, Steps 1, 4-6):** resolved with option (b) — minimal list tracking. A list-item + line records its content column; continuations indented less than contentColumn + 4 are + prose, not an indented code block. New test: + `aListItemContinuationStillExpands`. +- **F3 (Task 1, Steps 1, 5):** the dedent applies to the *evaluated source* as well as the echo. + Stated explicitly in Step 5 and pinned by `dedentAppliesToTheEvaluatedSource`. +- **F4 (Tasks 6, 8):** stale line references corrected against the current files. +- **F5 (Task 7, Step 3):** the fork is destroyed — forcibly after a 5-second grace period — and + the interrupt flag is restored *after* the argfile cleanup, the conventional order. +- **F6 (Task 7, Step 1):** the two no-double-wrap assertions are added to the existing + `testGmdMavenPluginReportsForkFailure` instead of duplicating its ~25-line mock setup. +- **F7 (Task 4):** decided explicitly — the width-only `print(GgChart, double)` / + `println(GgChart, double)` arity is **removed**, not shimmed, because a shim would preserve + compilation while silently changing the height that call resolves to. Recorded in the release + note. The `double` parameter type itself is unchanged and is not a defect; see attention + point 3. +- **F8 (Task 2, Step 6):** the list context is now closed when a non-blank, non-marker line sits + left of the item's content column (`relativeIndent < listContentColumn`), which subsumes the old + `relativeIndent < listItemIndent` reset — for a top-level list `listItemIndent == 0`, so the old + reset was unreachable and the list never closed. Indented code blocks after an ended list lost + their protection. New test: `indentedCodeAfterAListIsLeftAlone`. + +Maven test totals change accordingly: 103 (85 baseline, +19 new, -1 deleted `indentedCodeBlocksAreAKnownGap`). +The Gradle totals are unchanged: 4 baseline + 6 new = 10. + +### Review round 3: findings against this plan, resolved + +- **F9 (Task 2, Step 3, BLOCKER):** `baseIndent` counting only prose lines made + `indentedCodeBlocksAreLeftAlone`, `aBlankLineDoesNotEndAnIndentedCodeBlock` and + `crlfLinesAreClassifiedCorrectly` unsatisfiable — in each, the only prose lines are the + indented literal block itself, so the base became 4, relative indent 0, and the block was + expanded even after the fix. The helper now anchors on the first non-blank line of the + document. That subsumes both the fence-delimiter counting and F1's body-skip pre-pass: the + first line is never inside a block, and nothing later in the document is consulted. + Verified against every affected fixture: `indentedCodeBlocksAreLeftAlone` → 0, + `aBlankLineDoesNotEndAnIndentedCodeBlock` → 0, `crlfLinesAreClassifiedCorrectly` → 0, + `testInlineVars` → 8, `indentIsRelativeToTheDocumentBaseIndent` → 8, + `baseIndentIgnoresCodeBlockBodies` → 8, `testNoEcho`/`testCodeOnly` → 12, + `indentedCodeAfterAListIsLeftAlone` → 0. Accepted limitation: the first non-blank line + establishes the base in both directions. If it sits deeper than later prose, that prose + expands (today's behaviour); if it is shallower than a body indented four or more spaces + beyond it, that body is classified as literal and inline expansion is suppressed. Authors + must therefore put the first non-blank line at the document's own indent. The Review Focus 6 + test, `aStrayColumnZeroLineDoesNotCollapseTheBaseIndent`, covers only a stray shallower line + that appears later; the processCodeBlocks Javadoc records the complete invariant. +- **F10 (release notes, MEDIUM):** `gmd-core/release.md` (which has a live `## v3.2.0, + unreleased` section) and `gmd-gradle-plugin/release.md` now have explicit steps. Four + visible-behaviour entries are recorded — the width-only `print/println(GgChart, double)` + arity removal (source-breaking), Task 1's dedent of echoed and evaluated code-block + bodies, Task 2's indented-literal-block inline-expansion change, and Task 3's newline after + unterminated block output — plus the source-breaking removal of + `GmdGradlePlugin.hasRepository`. +- **F11 (Task 2, Step 6, LOW):** `listItemIndent` is dropped. After the F8 close rule, the + continuation check is only reached when `relativeIndent >= listContentColumn`, and + `listItemIndent < listContentColumn` always holds (the content column is past the marker), + so the `relativeIndent >= listItemIndent` clause could never be false. Retaining it read as + live protection that wasn't. + +### Task 1: Dedent Groovy code-block bodies by the fence indent + +**Finding #1.** `fenceCandidate()` correctly limits fence indentation to the three spaces CommonMark allows, but lines 56-58 bypass it with `line.trim()`, so a fence at any indent opens a block. The echoed result then replaces the fence line with `` ```groovy `` at column 0 while the body keeps its original indent: + +``` +Input: "Example:\n\n ```{groovy}\n out.println('EXECUTED')\n ```\n\nend\n" +Current: "Example:\n\n```groovy\n out.println('EXECUTED')\n```\nEXECUTED\n\nend\n" +Wanted: "Example:\n\n```groovy\nout.println('EXECUTED')\n```\nEXECUTED\n\nend\n" +``` + +Per decision (1) the block still executes; this task only realigns the body with the fence that wraps it. + +**Files:** +- Modify: `gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy:46-52` (state), `:65-73` (fence open), `:90-92` (body append), `:168-175` (helpers) +- Test: `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: two private static helpers that Task 2 also uses — + - `private static int leadingSpaces(String line)` → count of leading `' '` characters. + - `private static String dedent(String line, int width)` → the line with up to `width` leading `' '` characters removed. + +- [ ] **Step 1: Write the failing test** + +Add to `GmdTemplateEngineTest`: + +```groovy + @Test + void anIndentedGroovyFenceDedentsItsBody() { + String text = ''' +Example: + + ```{groovy} + out.println('EXECUTED') + ``` + +end +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains("```groovy\nout.println('EXECUTED')\n```"), + "The echoed body must line up with the fence that wraps it:\n$processed") + assertTrue(processed.contains('EXECUTED\n'), processed) + } + + @Test + void dedentOnlyRemovesTheFenceIndent() { + // The fence sits at 4; the body sits at 8. The extra 4 spaces are the + // author's own formatting and must survive. + String text = ''' + ```{groovy} + out.println('deep') + ``` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains("```groovy\n out.println('deep')\n```"), + "Indentation beyond the fence indent must be preserved:\n$processed") + } + + @Test + void aColumnZeroFenceLeavesItsBodyAlone() { + String text = ''' +```{groovy} + out.println('indented body') +``` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains("```groovy\n out.println('indented body')\n```"), + "A fence at column 0 dedents by 0:\n$processed") + } + + @Test + void dedentAppliesToTheEvaluatedSource() { + // The dedent is not cosmetic: codeBlockText feeds both the echo and the + // evaluated source. The continuation line sits below the fence indent + // (fence at 4, line at 2), so it loses both spaces and the multi-line + // string value changes. This pins that accepted behaviour (review F3). + String text = ''' + ```{groovy echo=false} + out.print("""a + b""") + ``` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('a\nb'), + "The dedent applies to the evaluated source too:\n$processed") + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTemplateEngineTest` +Expected: `anIndentedGroovyFenceDedentsItsBody`, `dedentOnlyRemovesTheFenceIndent` and `dedentAppliesToTheEvaluatedSource` FAIL (the body keeps its full indent / the string keeps its spaces). `aColumnZeroFenceLeavesItsBodyAlone` already passes — it is the regression guard. + +- [ ] **Step 3: Add the two helpers** + +Add next to the existing `fenceCandidate` / `fenceRunLength` helpers in `GmdTemplateEngine.groovy`: + +```groovy + /** Number of leading space characters. Tabs are not counted; see processCodeBlocks. */ + private static int leadingSpaces(String line) { + int i = 0 + while (i < line.length() && line.charAt(i) == ' ') { + i++ + } + return i + } + + /** Removes up to {@code width} leading spaces. Shorter indents are left untouched. */ + private static String dedent(String line, int width) { + int i = 0 + while (i < width && i < line.length() && line.charAt(i) == ' ') { + i++ + } + return line.substring(i) + } +``` + +- [ ] **Step 4: Record the fence indent when a Groovy block opens** + +Declare the new state variable alongside `plainFenceChar` (around line 46): + +```groovy + Character plainFenceChar = null + int plainFenceLength = 0 + int codeBlockIndent = 0 +``` + +Then, in the branch that opens a Groovy block (around line 65), capture the indent of the fence line: + +```groovy + if (plainFenceChar == null && noSpaceLine.startsWith('```{groovy')) { + shouldBeProcessed = true + codeBlockStart = true + codeBlockEnd = false + codeBlockIndent = leadingSpaces(line) + // echo is a property of one block, not of the entire document. + echo = true + if (noSpaceLine.toLowerCase().contains("echo=false")) { + echo = false; + } + } +``` + +- [ ] **Step 5: Dedent each body line as it is collected** + +Change the collection step (around line 90) from `codeBlockText.append(line)` to: + +```groovy + if (codeBlockStart) { + codeBlockText.append(dedent(line, codeBlockIndent)).append('\n') + } +``` + +The first line collected is the fence itself, which is replaced by `` ```groovy `` before echoing, so dedenting it is harmless. + +Note that the dedent applies to the **evaluated source** as well as the echo: `codeBlockText` feeds both the echo (`:97-98`) and `engine.eval` (`:101-106`). A continuation line indented less than the fence loses its spaces, so a multi-line string literal whose continuation sits below the fence indent changes value. That is accepted deliberately and pinned by `dedentAppliesToTheEvaluatedSource` (review F3). + +- [ ] **Step 6: Document the accepted limitation** + +Append to the `processCodeBlocks` Javadoc, above `@param text`: + +```groovy + * Indentation is lenient by design: a ```{groovy} fence is recognised at any + * indent, so a .gmd document written inside an indented Groovy string still + * works. The body is dedented by the fence's own indent so the two line up. + * The consequence is that an indented block cannot be used to *show* GMD + * syntax without running it - wrap such an example in a longer plain fence + * (````) instead. Only spaces are counted; a tab-indented fence is treated + * as column 0. The echoed fence is always emitted at column 0, so a Groovy + * block written inside a list item does not stay inside that item - the + * emitted block ends the list and the next item starts a new one. +``` + +- [ ] **Step 7: Run the new tests to verify they pass** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTemplateEngineTest` +Expected: the four new tests PASS. `testNoEcho` now FAILS — it asserts the old mismatch. That is expected and is fixed in the next step. + +- [ ] **Step 8: Update `testNoEcho` to the corrected output** + +`testNoEcho` embeds a twelve-space-indented document and asserts that the echoed body keeps its twelve spaces while the fence sits at column 0. Replace the expected value so the body is dedented by twelve: + +```groovy + @Test + void testNoEcho() { + String text = """ + Before + ```{groovy} + // just some Groovy code + def x = 5 + out.println('Hello World') + ``` + After + """ + + assertEquals(""" + Before +```groovy +// just some Groovy code +def x = 5 +out.println('Hello World') +``` +Hello World + After + """, GmdTemplateEngine.processCodeBlocks(text)) + } +``` + +Note the two trailing spaces after `out.println('Hello World')` are part of the original fixture; keep them. + +- [ ] **Step 9: Run the whole suite** + +Run: `mvn -o test` +Expected: BUILD SUCCESS, 89 tests (85 baseline + 4 new). + +- [ ] **Step 10: Record the release note** + +Add to the `## v3.2.0, unreleased` section of `gmd-core/release.md`: + +```markdown +- Indented ```{groovy} fences now execute with their body dedented by the fence's own + indent, and the echoed fence and body line up. Visible change: a multi-line string + literal whose continuation lines sit below the fence indent loses those spaces in the + evaluated source as well as the echo. +``` + +- [ ] **Step 11: Commit** + +```bash +git add gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy \ + gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy \ + gmd-core/release.md +git commit -m "fix: dedent groovy code block bodies by the fence indent + +An indented \`\`\`{groovy} fence emitted its body at the original indent while +the echoed fence sat at column 0. Dedent the body by the fence's own indent so +the two line up, and document that indented fences still execute by design. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 2: Stop expanding inline variables inside indented literal blocks + +**Finding #2.** The guard at line 115 only tracks *fenced* blocks (`plainFenceChar == null`), not indented ones, so CommonMark's indented code blocks are not protected: + +``` +Input: "text\n\n literal `= 1+1 ` here\n\nend\n" +Current: "text\n\n literal 2 here\n\nend\n" +Wanted: "text\n\n literal `= 1+1 ` here\n\nend\n" +``` + +**Indent is measured relative to the document's base indent** — the indent of the document's first non-blank line (review round 3, finding F9). This is what keeps GMD's existing leniency: `GmdTemplateEngineTest.testInlineVars` embeds an eight-space-indented document with no `stripIndent()` and expects expansion, and every `GmdTest` fixture relies on `stripIndent()` producing base indent 0. Only a block indented four spaces *beyond its neighbours* is literal. Anchoring on the first line — rather than scanning for the smallest prose indent — means a stray column-0 line later in the document (an HTML block, a table row) cannot collapse the base and silently reclassify every other line (Review Focus 6). It is deliberately not a universal safeguard: if the first non-blank line is shallower than the body, that body is measured relative to the shallower base and may be classified as literal. Authors must put the first non-blank line at the document's own indent. + +**Files:** +- Modify: `gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy:46-52` (state), `:114-123` (the non-code-block branch), `:94-113` (reset after a block), plus one new helper +- Test: `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy` + +**Interfaces:** +- Consumes: `leadingSpaces(String)` from Task 1. +- Produces: `private static int baseIndent(List lines)` → the `leadingSpaces` of the + first non-blank line, or 0 when the document is blank. Anchoring on the first line means no + later line — inside a code block or not — can drag the base to a smaller indent (review F1 + and review round 3 finding F9). + +- [ ] **Step 1: Write the failing tests** + +Add to `GmdTemplateEngineTest`. The first is the inversion of the existing `indentedCodeBlocksAreAKnownGap`; **delete that test** and add these: + +```groovy + @Test + void indentedCodeBlocksAreLeftAlone() { + String text = ''' +```{groovy echo=false} +x = 5 +``` + literal `= x ` is not expanded here +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` is not expanded here'), + "An indented code block is literal:\n$processed") + assertFalse(processed.contains('literal 5'), processed) + } + + @Test + void indentIsRelativeToTheDocumentBaseIndent() { + // Review Focus 2: every line is indented by 8; the literal block is at 12. + // The blank line matters - without it the indented line would be a lazy + // continuation of the paragraph above, which stays prose. + String text = """ + ```{groovy echo=false} + x = 5 + ``` + expanded `= x ` here + + literal `= x ` here + """ + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('expanded 5 here'), + "A line at the base indent is prose:\n$processed") + assertTrue(processed.contains('literal `= x ` here'), + "A line 4 beyond the base indent is literal:\n$processed") + } + + @Test + void anIndentedLineContinuingAParagraphIsStillExpanded() { + // CommonMark: an indented line directly after a paragraph line is a lazy + // continuation of that paragraph, not a code block. + String text = ''' +```{groovy echo=false} +x = 5 +``` +Some prose + continues here with `= x ` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('continues here with 5'), processed) + } + + @Test + void aBlankLineDoesNotEndAnIndentedCodeBlock() { + String text = ''' +```{groovy echo=false} +x = 5 +``` + first `= x ` literal + + second `= x ` literal +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('first `= x ` literal'), processed) + assertTrue(processed.contains('second `= x ` literal'), processed) + } + + @Test + void aTabIndentedLineIsNotTreatedAsACodeBlock() { + // Review Focus 1: only spaces are counted. This pins the documented + // limitation so a future change to tab handling is a deliberate one. + String text = "```{groovy echo=false}\nx = 5\n```\n\tliteral `= x ` here\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal 5 here'), + "Known limitation: tabs are not counted as indentation:\n$processed") + } + + @Test + void crlfLinesAreClassifiedCorrectly() { + // Review Focus 3: readLines() strips \\r, and a whitespace-only line must + // still count as blank when computing the base indent. + String text = "```{groovy echo=false}\r\nx = 5\r\n```\r\n literal `= x ` here\r\n \r\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), processed) + } + + @Test + void baseIndentIgnoresCodeBlockBodies() { + // Review F1 / round 3: `line two` sits at column 0 *inside* the Groovy + // block. The base indent is anchored on the document's first line, so + // a column-0 line inside a code block cannot drag the base to 0 and + // reclassify every prose line of this indented document as literal. + String text = """ + ```{groovy echo=false} + x = 5 + multi = '''line one +line two''' + ``` + Value `= x ` + """ + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('Value 5'), + "A column-0 line inside a code block must not change the document's base indent:\n$processed") + } + + @Test + void aStrayColumnZeroLineDoesNotCollapseTheBaseIndent() { + // Review Focus 6: an HTML block at column 0 in an otherwise + // 8-space-indented document must not drag the base indent to 0 - the + // lines at 8 would then sit at relative indent 8 and silently stop + // expanding. + String text = """ + ```{groovy echo=false} + x = 5 + ``` +
    html at column 0
    + expanded `= x ` here + """ + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('expanded 5 here'), + "A column-0 line must not collapse the base indent:\n$processed") + } + + @Test + void aListItemContinuationStillExpands() { + // Review F2: inside a list item, content is measured from the item's + // content column, so a 4-space continuation after a blank line is a + // paragraph of the item, not an indented code block. + String text = ''' +```{groovy echo=false} +x = 5 +``` +1. item + + continued `= x ` +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('continued 5'), processed) + } + + @Test + void indentedCodeAfterAListIsLeftAlone() { + // Review F8: a non-marker line at column 0 ends the list, so the + // indented block below is a code block again. Before the F8 fix the + // list context never closed — the reset required + // relativeIndent < listItemIndent, and for a top-level list + // listItemIndent == 0 — so the indented block was misclassified as a + // list continuation, expanded inline: 'literal 5 here'. x is bound in + // the block above so that expansion is observable. + String text = ''' +```{groovy echo=false} +x = 5 +``` + +- item + +text + + literal `= x ` here +''' + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), + "An indented code block after an ended list is literal:\n$processed") + assertFalse(processed.contains('literal 5'), processed) + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTemplateEngineTest` +Expected: `indentedCodeBlocksAreLeftAlone`, `indentIsRelativeToTheDocumentBaseIndent`, `aBlankLineDoesNotEndAnIndentedCodeBlock`, `crlfLinesAreClassifiedCorrectly` and `indentedCodeAfterAListIsLeftAlone` FAIL (the expressions are expanded). `anIndentedLineContinuingAParagraphIsStillExpanded`, `aTabIndentedLineIsNotTreatedAsACodeBlock`, `baseIndentIgnoresCodeBlockBodies`, `aStrayColumnZeroLineDoesNotCollapseTheBaseIndent` and `aListItemContinuationStillExpands` already pass — they are the regression guards (they pin the F1, F9 and F2 behaviours the implementation must preserve; `aStrayColumnZeroLineDoesNotCollapseTheBaseIndent` would fail under a global-minimum base indent, which is why the anchor-on-first-line approach is taken). + +- [ ] **Step 3: Add the base-indent helper** + +Add next to `leadingSpaces` in `GmdTemplateEngine.groovy`. The base indent is anchored on the +document's first non-blank line (review round 3, finding F9). Two benefits and one accepted +cost follow from that choice: + +- A column-0 line *inside* a ` ```{groovy} ` block or a plain fence can never drag the base + to 0 — the first line is never inside a block, and no later line is consulted. (A + global-minimum scan needed a fence-tracking pre-pass for this; review F1.) +- A stray column-0 line later in the document — an HTML block, a table row, a hard-wrapped + line — cannot collapse the base and silently reclassify every other line (Review Focus 6). +- The first non-blank line is the invariant, not a global minimum. If it is shallower than a + body indented four or more spaces beyond it, that body is classified as literal and inline + expansion is suppressed; authors must start the document at its own indent. + +```groovy + /** + * The indent the document sits at: the indent of its first non-blank + * line. Indented code blocks are recognised relative to this, so a .gmd + * written inside an indented Groovy string behaves the same as one + * written at column 0. + * + * Anchored on the first line deliberately: a global minimum could be + * collapsed by a column-0 line inside a code block or a stray column-0 + * prose line (HTML block, table row), reclassifying the whole document. + * The first line is therefore the invariant: when it is shallower than + * the body, body lines four or more spaces beyond it are literal. Authors + * must start a document at its own indent. + * Lines before the first non-blank one are blank and say nothing; when + * the document is entirely blank the base is 0. + */ + private static int baseIndent(List lines) { + for (String line in lines) { + if (!line.isBlank()) { + return leadingSpaces(line) + } + } + return 0 + } +``` + +- [ ] **Step 4: Add the tracking state** + +Next to `codeBlockIndent` from Task 1: + +```groovy + int codeBlockIndent = 0 + boolean inIndentedCode = false + boolean previousLineWasParagraph = false + // List context (review F2): while inside a list item, content is + // measured from the item's content column, not from column 0. + // listContentColumn is -1 when no list is open. + int listContentColumn = -1 +``` + +And after `List lines = text.readLines()`: + +```groovy + List lines = text.readLines() + // Anchored on the first non-blank line - see baseIndent. A column-0 + // line inside a code block or a stray column-0 prose line later in + // the document must not drag this to 0 (review F1, round 3 F9). + int documentIndent = baseIndent(lines) +``` + +- [ ] **Step 5: Reset the tracking when a Groovy block ends** + +In the `if (codeBlockEnd)` branch, after `codeBlockEnd = false` (around line 113), add: + +```groovy + codeBlockEnd = false + // A fenced block is not a paragraph, so an indented line + // straight after it starts an indented code block. The + // emitted block sits at column 0, so any surrounding list + // is over too. + inIndentedCode = false + previousLineWasParagraph = false + listContentColumn = -1 +``` + +- [ ] **Step 6: Classify each ordinary line, and gate the expansion on it** + +Replace the `else if (!codeBlockStart)` branch (lines 114-123) with: + +```groovy + } else if (!codeBlockStart) { + if (plainFenceChar != null || startsPlainFence) { + // Fenced content is handled by plainFenceChar; a fence + // line itself is never a paragraph. + inIndentedCode = false + previousLineWasParagraph = false + } else if (line.isBlank()) { + // A blank line neither starts nor ends an indented code + // block, but it does end a paragraph. A list item can + // contain blank lines, so listContentColumn survives. + previousLineWasParagraph = false + } else { + int relativeIndent = leadingSpaces(line) - documentIndent + def listMarker = line =~ /^\s*([-*+]|\d+[.)])\s+/ + if (relativeIndent < 4 && listMarker.find()) { + // A list item. Its content is measured from the + // item's content column, so a block indented four + // spaces inside the item is a paragraph of that + // item, not an indented code block (review F2). + listContentColumn = listMarker.end() - documentIndent + inIndentedCode = false + } else { + if (listContentColumn >= 0 && relativeIndent < listContentColumn) { + // Left of the item's content column and not a new + // marker: the list is over. This also subsumes the + // old relativeIndent < listItemIndent reset - for a + // top-level list that indent is 0, so the reset was + // unreachable and the list never closed (review F8): + // indented code blocks after an ended list were + // misclassified as continuations and expanded. + listContentColumn = -1 + } + // Only reached when relativeIndent >= listContentColumn, + // so a bare upper bound suffices (review round 3, F11). + if (listContentColumn >= 0 + && relativeIndent < listContentColumn + 4) { + // A continuation of the current list item: prose. + inIndentedCode = false + } else { + if (relativeIndent < 4) { + inIndentedCode = false + } else if (!previousLineWasParagraph) { + inIndentedCode = true + } + } + } + previousLineWasParagraph = !inIndentedCode + } + if (plainFenceChar == null && !inIndentedCode && line.contains('`=')) { + shouldBeProcessed = true + result.append(expandInlineVars(line, engine)) + } else { + result.append(line) + } + if (count < lines.size() - 1 || endsWithNewline) + result.append('\n') + } +``` + +Two rules are deliberately load-bearing: + +- The `relativeIndent >= 4 && previousLineWasParagraph` case (last `else` branch) falls through with `inIndentedCode` unchanged: that is a lazy paragraph continuation, which stays prose (`anIndentedLineContinuingAParagraphIsStillExpanded`). +- The list-marker regex is only honoured at `relativeIndent < 4`, so a `1. item` line *inside* an indented literal block does not open list context and steal the block's code status. + +Known limitations of the minimal list tracking, accepted for 3.2.0: nested lists overwrite the outer item's content column (a later line dedented below the nested item's indent resets list context entirely), and list context survives a plain fence indented to the item's content column. Neither affects inline-variable expansion for lines below relative indent 4. + +- [ ] **Step 7: Document the first-line base-indent invariant** + +Append to the `processCodeBlocks` Javadoc after Task 1's indentation paragraph: + +```groovy + * For inline-variable expansion, the document base is the first non-blank + * line's indent. That is an invariant, not a global minimum: if the first + * line is shallower than a body indented four or more spaces beyond it, the + * body is treated as literal and does not expand. Put the first non-blank + * line at the document's own indent. +``` + +- [ ] **Step 8: Run the new tests to verify they pass** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTemplateEngineTest` +Expected: all ten PASS, and `testInlineVars` (eight-space document, no `stripIndent()`) still PASSES — that is the proof that relative indent preserved the leniency. + +- [ ] **Step 9: Run the whole suite** + +Run: `mvn -o test` +Expected: BUILD SUCCESS, 98 tests (89 after Task 1, minus the deleted `indentedCodeBlocksAreAKnownGap`, plus 10 new). + +- [ ] **Step 10: Record the release note** + +Add to the `## v3.2.0, unreleased` section of `gmd-core/release.md`: + +```markdown +- Inline `= expr ` is no longer expanded inside CommonMark indented code blocks (a line + indented four or more spaces beyond the document's own base indent). Indent is measured + relative to the indent of the document's first non-blank line, so a .gmd written inside an + indented Groovy string is unaffected. Lazy paragraph continuations and list-item + continuations still expand. +``` + +- [ ] **Step 11: Commit** + +```bash +git add gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy \ + gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy \ + gmd-core/release.md +git commit -m "fix: do not expand inline variables inside indented code blocks + +Inline \`= expr \` was expanded inside CommonMark indented code blocks. Track +them, measuring indent relative to the document's own base indent so a .gmd +written inside an indented Groovy string keeps working. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 3: Terminate code-block output with a newline + +**Finding #4.** Captured output that does not end in a newline is concatenated with the next source line, silently destroying it: + +``` +Input: "```{groovy echo=false}\nout.print('X')\n```\n# Heading\n" +Current: "X# Heading\n" +Wanted: "X\n# Heading\n" +``` + +Every other emitter (`Printer.terminateHtmlBlock`, `Printer.printChart`) terminates its own block; the captured-output append does not. Verified safe against `GmdTest.printedChartKeepsCaptionInProse`, whose output already ends in `\n\n`, and against every existing `out.print` call site in `GmdTest`, which terminate their output via `terminateHtmlBlock`/`printChart`. + +Note this deliberately changes one observable behaviour: `out.print('Hello ')` followed by prose used to produce `Hello world` on one source line; it now produces `Hello \nworld`. CommonMark renders both as one paragraph (soft break), so the rendered output is unchanged — but the Markdown text differs, and the new test below pins that so a future revert is a deliberate decision, not silent drift. + +**Files:** +- Modify: `gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy:107-110` +- Test: `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: nothing later tasks depend on. + +- [ ] **Step 1: Write the failing test** + +```groovy + @Test + void unterminatedCodeBlockOutputDoesNotSwallowTheNextLine() { + String text = "```{groovy echo=false}\nout.print('X')\n```\n# Heading\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertEquals("X\n# Heading\n", processed) + } + + @Test + void alreadyTerminatedOutputGainsNoExtraNewline() { + String text = "```{groovy echo=false}\nout.println('X')\n```\n# Heading\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertEquals("X\n# Heading\n", processed) + } + + @Test + void inlineOutPrintIsTerminatedBeforeTheNextLine() { + // Pins the one observable behaviour change: out.print('Hello ') used to + // glue the next source line on ('Hello world'); it now terminates it + // ('Hello \nworld'). CommonMark renders both as one paragraph, but the + // Markdown text differs and must not drift back silently. + String text = "```{groovy echo=false}\nout.print('Hello ')\n```\nworld\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertEquals("Hello \nworld\n", processed) + } +``` + +- [ ] **Step 2: Run the tests to verify the first fails** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTemplateEngineTest` +Expected: `unterminatedCodeBlockOutputDoesNotSwallowTheNextLine` FAILS with actual `X# Heading\n`. `inlineOutPrintIsTerminatedBeforeTheNextLine` FAILS with actual `Hello world\n`. `alreadyTerminatedOutputGainsNoExtraNewline` PASSES — it guards against over-correcting. + +- [ ] **Step 3: Terminate the output** + +Replace lines 107-110 of `GmdTemplateEngine.groovy`: + +```groovy + def output = out.toString() + if (output.length() > 0) { + result.append(output) + // The document continues on its own line. Without this the + // next source line is glued onto unterminated output. + if (!output.endsWith('\n')) { + result.append('\n') + } + } +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTemplateEngineTest` +Expected: all three PASS. + +- [ ] **Step 5: Run the whole suite** + +Run: `mvn -o test` +Expected: BUILD SUCCESS, 100 tests. `GmdTest.printedChartKeepsCaptionInProse` must still pass — confirm it does before committing. + +- [ ] **Step 6: Record the release note** + +Add to the `## v3.2.0, unreleased` section of `gmd-core/release.md`: + +```markdown +- Output from a Groovy code block that does not end in a newline now gains one, so + `out.print('x')` no longer glues the next source line onto the output. Visible change: + `out.print('Hello ')` followed by prose now yields `Hello \nworld` instead of + `Hello world` in the Markdown (rendered identically by CommonMark). +``` + +- [ ] **Step 7: Commit** + +```bash +git add gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy \ + gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy \ + gmd-core/release.md +git commit -m "fix: terminate unterminated code block output with a newline + +out.print without a trailing newline glued the next source line onto the +captured output, silently destroying headings and list items. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 4: GgChart always honours its configured dimensions + +**Finding #5.** `print(GgChart, String alt, Map)` renders with `x.render()` (the chart's own width/height), while the default-argument overload renders at `DEFAULT_CHART_WIDTH`/`DEFAULT_CHART_HEIGHT`. So `out.println(chart)` renders 800×600 — discarding a configured `chart.width = 1200` — while `out.println(chart, 'alt')` renders 1200×900. + +Per decision (4), align on honouring the chart: only an explicit `width`/`height` argument overrides. Groovy default arguments cannot distinguish "not passed" from "passed 800", so the two default-argument methods are replaced by explicit overloads. + +**Files:** +- Modify: `gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy:164-186` +- Modify: `README.md` (the GGdimension sentence added in d7dfcd4) +- Test: `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy:587-606` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: the `Printer` GgChart surface becomes exactly — + - `void print(GgChart x)` / `void println(GgChart x)` + - `void print(GgChart x, String alt, Map attributes = [:])` / same for `println` + - `void print(GgChart x, double width, double height, String alt = '', Map attributes = [:])` / same for `println` + + This drops the width-only `print(GgChart, double)` arity the old default arguments generated; + see attention point 3 for why it is removed rather than shimmed. The `double` parameter type + is deliberate and stays as it is. `Chart` and `CharmChart` are **not** changed. + +- [ ] **Step 1: Write the failing test** + +Replace `GmdTest.testGgChart` with: + +```groovy + @Test + void testGgChart() { + def text = ''' +```{groovy echo=false} +import static se.alipsa.matrix.gg.GgPlot.* +import se.alipsa.matrix.core.Matrix + +def data = Matrix.builder().data(x: [1, 2], y: [3, 4]).types(int, int).build() +def chart = ggplot(data, aes(x: 'x', y: 'y')) + geom_point() +chart.width = 1200 +chart.height = 900 +out.println(chart, 640, 480, 'GG chart', [class: 'chart']) +out.println(chart, 'GG default chart') +out.println(chart) +out.println(chart, 640.5, 480.5, 'GG BigDecimal dims') +``` +''' + + String md = new Gmd().gmdToMd(text) + + assertTrue(md.contains('GG chart svgs = decodeSvgImages(md, 'GG chart should be SVG-backed') + assertTrue(svgs[0].contains('width="640"') && svgs[0].contains('height="480"'), svgs[0].take(200)) + assertTrue(svgs[1].contains('width="1200"') && svgs[1].contains('height="900"'), svgs[1].take(200)) + assertTrue(svgs[2].contains('width="1200"') && svgs[2].contains('height="900"'), + "print(GgChart) with no arguments must honour the chart's own dimensions: ${svgs[2].take(200)}") + // 640.5 is a BigDecimal literal in Groovy. The double parameters accept it by + // dispatch coercion, and chartToMd's `width as int` truncates to 640. This pins + // the parameter type: narrowing it to int would reject BigDecimal outright. + assertTrue(svgs[3].contains('width="640"') && svgs[3].contains('height="480"'), + "double dimensions must accept Groovy's default decimal type (BigDecimal): ${svgs[3].take(200)}") + } + + @Test + void aGgChartWithNoConfiguredSizeRendersAtTheLibraryDefault() { + // Review Focus 4: width/height are never set, so the library default applies. + def text = ''' +```{groovy echo=false} +import static se.alipsa.matrix.gg.GgPlot.* +import se.alipsa.matrix.core.Matrix + +def data = Matrix.builder().data(x: [1, 2], y: [3, 4]).types(int, int).build() +out.println(ggplot(data, aes(x: 'x', y: 'y')) + geom_point()) +``` +''' + + String md = new Gmd().gmdToMd(text) + List svgs = decodeSvgImages(md, 'An unsized GG chart should still render') + + assertTrue(svgs[0].contains(' attributes = [:]) { + printChart(svgToMd(x.render(), alt, attributes)) + } + + /** @see #print(GgChart, String, Map) */ + void println(GgChart x, String alt, Map attributes = [:]) { + println(svgToMd(x.render(), alt, attributes)) + } + + /** + * Renders a GgChart at the supplied dimensions, overriding the chart's own. + * The chart's original dimensions are restored before this method returns. + */ + void print(GgChart x, double width, double height, String alt = '', Map attributes = [:]) { + printChart(chartToMd(x, width, height, alt, attributes)) + } + + /** @see #print(GgChart, double, double, String, Map) */ + void println(GgChart x, double width, double height, String alt = '', Map attributes = [:]) { + println(chartToMd(x, width, height, alt, attributes)) + } +``` + +Leave the private `chartToMd(GgChart, double, double, String, Map)` helper — with its save/restore `finally` — exactly as it is, including its `double` parameters. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTest` +Expected: both PASS. + +- [ ] **Step 5: Correct the README** + +In `README.md`, replace: + +``` +They also accept alt text and attributes without explicit dimensions. Alt-only GG printing honors +the dimensions configured on the chart; explicit GG dimensions are restored after rendering. +``` + +with: + +``` +They also accept alt text and attributes without explicit dimensions. Pict and Charm charts then +render at 800x600. GG charts always honor the `width` and `height` configured on the chart unless +you pass explicit dimensions, which are restored after rendering. +``` + +- [ ] **Step 6: Record the release note** + +Add to the `## v3.2.0, unreleased` section of `gmd-core/release.md`: + +```markdown +- **Breaking:** the `print(GgChart, double)` / `println(GgChart, double)` width-only + overload that Groovy default arguments generated is removed. Pass explicit width and + height, or omit both and let the chart's own dimensions apply. +``` + +- [ ] **Step 7: Run the whole suite** + +Run: `mvn -o test` +Expected: BUILD SUCCESS, 101 tests. + +- [ ] **Step 8: Commit** + +```bash +git add gmd-core/src/main/groovy/se/alipsa/gmd/core/Printer.groovy \ + gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy \ + README.md gmd-core/release.md +git commit -m "fix: GgChart printing always honours its configured dimensions + +out.println(chart) rendered at 800x600 while out.println(chart, 'alt') honoured +chart.width/height. Replace the default-argument overloads with explicit ones so +only explicit width/height arguments override the chart. The width-only arity that +the default arguments generated is removed; nothing in the repo used it. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 5: Gradle plugin checks for Maven Central before adding it + +**Finding #3.** `project.repositories.mavenCentral()` **adds** the repository as a side effect, so the `hasRepository` guard beneath it is unreachable and every consuming project silently gains a project-level Maven Central. Verified on Gradle 9.7.1 with `ProjectBuilder`: + +``` +before: 0 +after mavenCentral(): 1 +contains: true +``` + +That changes resolution for the consumer's own configurations and, under `dependencyResolutionManagement { repositoriesMode = FAIL_ON_PROJECT_REPOS }`, the add happens inside `afterEvaluate` outside any try/catch. + +Per decision (3): look for an existing Maven Central declaration first and only add when there is none. + +**Files:** +- Modify: `gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy:107-131` +- Modify: `gmd-gradle-plugin/readme.md` +- Test: `gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: `static boolean hasRepository(Project, MavenArtifactRepository)` is **removed** and replaced by `static boolean hasMavenCentral(Project project)`. `addDependencies(Project, String, String, String, String)` keeps its exact signature. +- Needs one new import: `org.gradle.api.InvalidUserCodeException`. + +- [ ] **Step 1: Write the failing test** + +Add to `GmdGradlePluginTest`: + +```groovy + @Test + void mavenCentralIsAddedOnlyWhenTheProjectHasNone() { + def project = ProjectBuilder.builder().build() + Assertions.assertEquals(0, project.repositories.size()) + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(1, project.repositories.size(), + 'The plugin should add Maven Central when the project declares no repository') + } + + @Test + void anExistingMavenCentralIsNotDuplicated() { + def project = ProjectBuilder.builder().build() + project.repositories.mavenCentral() + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(1, project.repositories.size(), + 'The plugin must not add a second Maven Central') + } + + @Test + void mavenCentralIsRecognisedUnderItsOtherHost() { + // Review Focus 5: repo1.maven.org is the same repository, and a trailing + // slash must not defeat the check. + def project = ProjectBuilder.builder().build() + project.repositories.maven { it.setUrl('https://repo1.maven.org/maven2') } + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(1, project.repositories.size(), + 'repo1.maven.org is Maven Central and must not trigger a second add') + } + + @Test + void settingsManagedRepositoriesDoNotBreakTheBuild() { + // Measured on Gradle 9.7.1: under FAIL_ON_PROJECT_REPOS the settings + // repositories are NOT visible in project.repositories (size 0), so + // hasMavenCentral returns false and the add is attempted and rejected with + // InvalidUserCodeException. Swallowing it is safe - the detached + // configuration still resolves through the settings repositories. + // ProjectBuilder cannot model dependencyResolutionManagement, so this is a + // TestKit test. + File testProjectDir = new File('build/gmdSettingsReposTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'test.gmd').text = '# Greetings\n' + + new File(testProjectDir, 'settings.gradle').text = """ + pluginManagement { + repositories { mavenCentral() } + plugins { id 'se.alipsa.gmd.gmd-gradle-plugin' version "1.0.0" } + } + dependencyResolutionManagement { + repositoriesMode = RepositoriesMode.FAIL_ON_PROJECT_REPOS + repositories { mavenCentral() } + } + """.stripIndent() + + // Deliberately no project-level repositories block. + new File(testProjectDir, 'build.gradle').text = """ + plugins { + id('base') + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + gmdPlugin { + sourceDir = 'src/test/gmd' + targetDir = 'build/target' + outputType = 'html' + gmdVersion = '3.1.0' + runTaskBefore = 'build' + } + """.stripIndent() + + def result = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('processGmd') + .withPluginClasspath() + .forwardOutput() + .build() + + Assertions.assertEquals(SUCCESS, result.task(':processGmd').outcome, + "FAIL_ON_PROJECT_REPOS must not break the plugin:\n${result.output}") + Assertions.assertTrue(new File(testProjectDir, 'build/target/test.html').exists(), + 'The detached configuration must still resolve through the settings repositories') + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } + + @Test + void aPrivateMirrorStillGetsMavenCentralAdded() { + def project = ProjectBuilder.builder().build() + project.repositories.maven { it.setUrl('https://nexus.example.com/repository/maven-public') } + + GmdGradlePlugin.addDependencies(project, '5.1.3', '2.26.1', '3.1.0', '2.6.0') + + Assertions.assertEquals(2, project.repositories.size(), + 'A private mirror is not Maven Central, so the plugin still adds it') + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cd gmd-gradle-plugin && ./gradlew test --tests '*GmdGradlePluginTest*'` +Expected: `anExistingMavenCentralIsNotDuplicated` and `mavenCentralIsRecognisedUnderItsOtherHost` FAIL with 2 repositories. `settingsManagedRepositoriesDoNotBreakTheBuild` FAILS with `InvalidUserCodeException: Build was configured to prefer settings repositories over project repositories but repository 'MavenRepo' was added by build file 'build.gradle'`. + +- [ ] **Step 3: Replace the repository handling** + +In `GmdGradlePlugin.groovy`, replace `addDependencies`'s first three lines and the `hasRepository` method: + +```groovy + private static final List MAVEN_CENTRAL_HOSTS = ['repo.maven.apache.org', 'repo1.maven.org'] + + static Configuration addDependencies(Project project, + String groovyVersion, String log4jVersion, String gmdVersion, + String ivyVersion) { + // RepositoryHandler.mavenCentral() ADDS the repository as a side effect, so + // it must only be called when the project has not declared Central itself. + // Injecting it unconditionally changes resolution for the consumer's own + // configurations and fails under repositoriesMode = FAIL_ON_PROJECT_REPOS. + if (!hasMavenCentral(project)) { + try { + project.repositories.mavenCentral() + } catch (InvalidUserCodeException e) { + // repositoriesMode = PREFER_SETTINGS or FAIL_ON_PROJECT_REPOS. The settings + // repositories are NOT visible in project.repositories (measured: size 0), + // so hasMavenCentral cannot see them and the add above is rejected. The + // detached configuration below still resolves through them, so this is not + // an error - measured on Gradle 9.7.1, see Task 5 Step 1. + project.logger.info('Project repositories are managed in settings.gradle; ' + + 'resolving gmd-core through the settings repositories instead: ' + e.message) + } + } + + List dependencies = [ + project.dependencies.create("org.apache.groovy:groovy:${groovyVersion}"), + project.dependencies.create("org.apache.groovy:groovy-templates:${groovyVersion}"), + project.dependencies.create("org.apache.groovy:groovy-jsr223:${groovyVersion}"), + project.dependencies.create("org.apache.ivy:ivy:${ivyVersion}"), // needed for @Grab) + project.dependencies.create("org.apache.logging.log4j:log4j-core:${log4jVersion}"), + project.dependencies.create("se.alipsa.gmd:gmd-core:${gmdVersion}") + ] + + return project.configurations.detachedConfiguration(dependencies.toArray(new Dependency[0])) + } + + static boolean hasMavenCentral(Project project) { + return project.repositories.any { ArtifactRepository repository -> + repository instanceof MavenArtifactRepository && + MAVEN_CENTRAL_HOSTS.contains(((MavenArtifactRepository) repository).url?.host) + } + } +``` + +Delete the old `hasRepository` method entirely. It is `static` on a published plugin class, +so its removal is source-breaking for any build script that called it; that is recorded in +the release note in Step 5. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `cd gmd-gradle-plugin && ./gradlew test --tests '*GmdGradlePluginTest*'` +Expected: all five PASS, and the existing `testPlugin` TestKit test still PASSES. + +- [ ] **Step 5: Document the behaviour** + +Add to `gmd-gradle-plugin/readme.md`, under the configuration parameter list: + +```markdown +### Repositories + +The plugin resolves `gmd-core` and Groovy through a detached configuration, which uses the +project's repositories. If the project has not declared Maven Central, the plugin adds it. +If it has — under either `repo.maven.apache.org` or `repo1.maven.org` — nothing is added. +A project that resolves only through a private mirror should declare that mirror in +`dependencyResolutionManagement` in `settings.gradle` so the plugin has no reason to add +Central to the project itself. +``` + +Also add to the `## v3.2.0, unreleased` section of `gmd-gradle-plugin/release.md`: + +```markdown +- **Breaking:** `GmdGradlePlugin.hasRepository(Project, MavenArtifactRepository)` is removed. + It was public static API on the plugin class; nothing in the repo called it. The plugin now + checks for an existing Maven Central declaration itself before adding one. +``` + +- [ ] **Step 6: Commit** + +```bash +git add gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy \ + gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy \ + gmd-gradle-plugin/readme.md gmd-gradle-plugin/release.md +git commit -m "fix: only add Maven Central when the Gradle project has none + +RepositoryHandler.mavenCentral() adds the repository as a side effect, so the +guard beneath it was unreachable and every consuming project silently gained a +project-level Maven Central. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 6: Filter the Gradle source listing for .gmd files + +**Finding #6.** `ProcessGmdTask.process` line 116 uses an unfiltered `source.listFiles()` to decide which completion message to log, while the other three enumerations in the same codebase (`ProcessGmdTask:66`, `:129`, `GmdProcessor:49`) filter on `.gmd`. A source directory holding only non-`.gmd` files logs "Gmd files processed and written to …" although nothing was generated. + +**Files:** +- Modify: `gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy:58-71` (`getGeneratedFiles`), `:110-148` (`process` tail + `cleanStaleGeneratedFiles`) +- Test: `gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: `private static File[] gmdFilesIn(File directory)` → the `.gmd` files in the directory, or an empty array when the directory cannot be read. + +- [ ] **Step 1: Write the failing test** + +Add to `GmdGradlePluginTest`: + +```groovy + @Test + void aSourceDirWithoutGmdFilesReportsNothingToDo() { + File testProjectDir = new File('build/gmdNoGmdFilesTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'readme.txt').text = 'not a gmd file' + + new File(testProjectDir, 'settings.gradle').text = """ + pluginManagement { + repositories { mavenCentral() } + plugins { id 'se.alipsa.gmd.gmd-gradle-plugin' version "1.0.0" } + } + """.stripIndent() + + new File(testProjectDir, 'build.gradle').text = """ + plugins { + id('base') + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + repositories { mavenCentral() } + gmdPlugin { + sourceDir = 'src/test/gmd' + targetDir = 'build/target' + outputType = 'html' + gmdVersion = '3.1.0' + runTaskBefore = 'build' + } + """.stripIndent() + + def result = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('processGmd') + .withPluginClasspath() + .forwardOutput() + .build() + + Assertions.assertTrue(result.output.contains('No gmd files found in'), + "A source dir with no .gmd files must say so:\n${result.output}") + Assertions.assertFalse(result.output.contains('Gmd files processed and written to'), + "Nothing was generated, so nothing was written:\n${result.output}") + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `cd gmd-gradle-plugin && ./gradlew test --tests '*aSourceDirWithoutGmdFilesReportsNothingToDo*'` +Expected: FAIL — the output contains "Gmd files processed and written to". + +- [ ] **Step 3: Extract the shared filter** + +Add to `ProcessGmdTask.groovy`: + +```groovy + /** The .gmd files in a directory, or an empty array when it cannot be read. */ + private static File[] gmdFilesIn(File directory) { + File[] files = directory.listFiles({ File file -> + file.isFile() && file.name.endsWith('.gmd') + } as FileFilter) + return files == null ? new File[0] : files + } +``` + +- [ ] **Step 4: Use it in all three places** + +In `getGeneratedFiles()`, replace the `File[] sources = source.listFiles(...)` line and the null handling: + +```groovy + File[] sources = gmdFilesIn(source) + return sources.collect { File file -> + new File(target, file.name.substring(0, file.name.length() - 4) + ".${output}") + } as Set +``` + +In `process()`, replace the unfiltered listing: + +```groovy + File[] sourceFiles = gmdFilesIn(source) + if (sourceFiles.length > 0) { + if (target.exists()) { + logger.quiet("Gmd files processed and written to ${target.canonicalPath}") + } else { + logger.warn("${target.canonicalPath} should exists but does not, something is probably wrong") + } + } else { + logger.quiet("No gmd files found in ${source.canonicalPath}, nothing to do") + } +``` + +In `cleanStaleGeneratedFiles`, replace the listing and its null guard: + +```groovy + Set expected = [] as Set + gmdFilesIn(sourceDir).each { file -> + String base = file.name.substring(0, file.name.length() - 4) + expected.add("${base}.${outputType}".toString()) + } +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cd gmd-gradle-plugin && ./gradlew test` +Expected: the new test PASSES and every existing Gradle test still PASSES. + +- [ ] **Step 6: Commit** + +```bash +git add gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy \ + gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +git commit -m "fix: report nothing-to-do when the Gradle source dir has no gmd files + +The completion message used an unfiltered listFiles(), so a source directory +holding only non-.gmd files claimed output had been written. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 7: Maven plugin restores the interrupt flag and stops double-wrapping + +**Finding #7.** `process.waitFor()` at line 193 throws `InterruptedException`, which falls into the broad `catch (Exception e)` at line 224 and is rewrapped without `Thread.currentThread().interrupt()`; the forked JVM is left running. The same broad catch also rewraps the plugin's own `MojoFailureException`s, so "GmdProcessor exited with code 1" becomes "Failed to process gmd files in …: GmdProcessor exited with code 1". + +**Files:** +- Modify: `gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java:190-230` +- Test: `gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: nothing later tasks depend on. + +- [ ] **Step 1: Tighten the existing fork-failure test** + +Per review F6, do **not** add a new test — the planned `aMojoFailureIsNotDoubleWrapped` would have repeated the entire mock setup of the existing `testGmdMavenPluginReportsForkFailure` (`GmdMavenPluginTest.java:112-137`) and differed only in the assertions. Instead, replace the assertion block of that existing test: + +```java + MojoFailureException exception = assertThrows(MojoFailureException.class, plugin::execute); + // The plugin's own MojoFailureException must not be rewrapped by the broad catch. + assertTrue(exception.getMessage().startsWith("GmdProcessor exited with code"), + "The plugin's own failure must not be rewrapped: " + exception.getMessage()); + assertFalse(exception.getMessage().contains("Failed to process gmd files in"), + "The plugin's own failure must not be rewrapped: " + exception.getMessage()); +``` + +(`assertFalse` is already statically imported in that file.) + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `mvn -o -pl gmd-maven-plugin test -Dtest=GmdMavenPluginTest` +Expected: `testGmdMavenPluginReportsForkFailure` FAILS — the message starts with "Failed to process gmd files in". + +- [ ] **Step 3: Handle the interrupt explicitly** + +Add `import java.util.concurrent.TimeUnit;` to `GmdMavenPlugin.java`, then replace the `Process`/`waitFor` block and the argfile `finally` in `GmdMavenPlugin.execute()`: + +```java + File argFile = createArgFile(classpath.toString(), srcDir.getCanonicalPath(), + outputDirectory.getCanonicalPath(), normalizedOutputType); + // Set when the waitFor below is interrupted; the flag is restored after + // the argfile cleanup in the finally, not before (review F5). + boolean interrupted = false; + try { + List command = new ArrayList<>(); + command.add(getJavaExecutable()); + command.add("@" + argFile.getAbsolutePath()); + + ProcessBuilder processBuilder = new ProcessBuilder(command); + processBuilder.inheritIO(); + Process process = processBuilder.start(); + int exitCode; + try { + exitCode = process.waitFor(); + } catch (InterruptedException e) { + // Do not leave the fork running - that is the point of this catch. + // destroy() alone can leave it alive, so escalate to destroyForcibly() + // after a short grace period. + process.destroy(); + try { + if (!process.waitFor(5, TimeUnit.SECONDS)) { + process.destroyForcibly(); + } + } catch (InterruptedException swallowed) { + // The grace-period wait was interrupted too; kill the fork anyway. + process.destroyForcibly(); + } + // Do not swallow the interrupt - callers above us need the flag to + // see that the build was cancelled - but restore it after cleanup. + interrupted = true; + throw new MojoExecutionException("Interrupted while waiting for the GMD processor", e); + } + + if (exitCode != 0) { + throw new MojoFailureException("GmdProcessor exited with code " + exitCode); + } + } finally { + // Never let a cleanup failure here replace a pending exception from + // the try block (e.g. the non-zero exit code above) with a less + // informative one. + try { + Files.deleteIfExists(argFile.toPath()); + } catch (IOException e) { + getLog().warn("Could not delete temporary classpath argfile " + argFile.getAbsolutePath() + + ": " + e.getMessage()); + } + if (interrupted) { + // Conventional order: cleanup first, then restore the flag. + Thread.currentThread().interrupt(); + } + } +``` + +- [ ] **Step 4: Stop rewrapping the plugin's own exceptions** + +Add a rethrow clause before the broad `catch (Exception e)` at the end of `execute()`: + +```java + } catch (MojoExecutionException | MojoFailureException e) { + // Already a precise, actionable Mojo failure - rewrapping it only buries + // the message a level deeper. + throw e; + } catch (DependencyResolutionException e) { + throw new MojoExecutionException("Failed to resolve dependencies", e); + } catch (Exception e) { +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `mvn -o -pl gmd-maven-plugin test -Dtest=GmdMavenPluginTest` +Expected: the tightened `testGmdMavenPluginReportsForkFailure` PASSES. (The interrupt path itself has no automated test — exercising it needs a real forked JVM to interrupt — so it is verified by review, not by the suite.) + +- [ ] **Step 6: Run the whole suite** + +Run: `mvn -o test` +Expected: BUILD SUCCESS, 101 tests (unchanged from Task 4 — this task adds assertions to an existing test, not a new one). + +- [ ] **Step 7: Commit** + +```bash +git add gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java \ + gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java +git commit -m "fix: restore the interrupt flag and stop double-wrapping Mojo failures + +waitFor's InterruptedException fell into the broad catch, losing the interrupt +flag and leaving the fork running. The same catch also rewrapped the plugin's +own MojoFailureExceptions, burying their messages. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +### Task 8: CLI `toHtml` creates parent directories + +**Finding #8.** `GmdCommandLine.toHtml` writes with a bare `Files.writeString`, while `Gmd.writeUtf8` and `Gmd.htmlToPdf(String, File)` both call `ensureParentDirectory`. So `java -jar gmd.jar toHtml in.gmd out/dir/x.html` fails with a bare `NoSuchFileException` while `toPdf` to the same path succeeds. Verified: `Files.writeString` to a missing parent throws `NoSuchFileException`. + +The fix removes the duplication rather than repeating the mkdir: `Gmd.gmdToHtml(String, File)` already does exactly this work, through `writeUtf8`. + +**Files:** +- Modify: `gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy:26-34` (`toHtml`) +- Test: `gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: nothing later tasks depend on. `GmdCommandLine.toHtml(String, String)` keeps its signature; the private `gmdFileToHtml` stays, used by `toPdfStyled`. + +- [ ] **Step 1: Write the failing test** + +Add to `GmdTest`: + +```groovy + @Test + void commandLineToHtmlCreatesMissingParentDirectories() { + String file = getClass().getResource("/test.gmd").getFile() + File htmlFile = new File(testOutputDir, "nested/output/dir/cli.html") + assertFalse(htmlFile.parentFile.exists(), 'The parent directory must not exist yet') + + Gmd.main("toHtml", file, htmlFile.absolutePath) + + assertTrue(htmlFile.exists(), 'toHtml must create the parent directories, as toPdf does') + assertTrue(htmlFile.length() > 100, "No html content") + } + + @Test + void commandLineToPdfCreatesMissingParentDirectories() { + String file = getClass().getResource("/test.gmd").getFile() + File pdfFile = new File(testOutputDir, "nested/pdf/dir/cli.pdf") + + Gmd.main("toPdf", file, pdfFile.absolutePath) + + assertTrue(pdfFile.exists()) + assertTrue(pdfFile.length() > 100, "No pdf content") + } +``` + +- [ ] **Step 2: Run the tests to verify the first fails** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTest` +Expected: `commandLineToHtmlCreatesMissingParentDirectories` FAILS with `NoSuchFileException`. `commandLineToPdfCreatesMissingParentDirectories` PASSES — it is the proof that the two commands disagree today. + +- [ ] **Step 3: Route toHtml through Gmd.gmdToHtml(String, File)** + +In `GmdCommandLine.groovy`, replace `toHtml`: + +```groovy + static Closure toHtml(String from, String to) { + return { + Gmd gmd = new Gmd() + File toFile = new File(to) + // gmdToHtml(String, File) writes a decorated document through writeUtf8, + // which creates the parent directories the way toPdf already does. + gmd.gmdToHtml(readGmd(from), toFile) + println "Wrote $toFile.absolutePath" + } + } +``` + +Remove the now-unused `java.nio.charset.StandardCharsets` import only if `readGmd` no longer needs it — it does, so **keep both imports**. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `mvn -o -pl gmd-core test -Dtest=GmdTest` +Expected: both PASS, and the existing `gmdCommandLineTest` still PASSES. + +- [ ] **Step 5: Run the whole suite** + +Run: `mvn -o test && (cd gmd-gradle-plugin && ./gradlew test)` +Expected: BUILD SUCCESS both times, 103 Maven tests. + +- [ ] **Step 6: Commit** + +```bash +git add gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdCommandLine.groovy \ + gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTest.groovy +git commit -m "fix: toHtml creates missing parent directories like toPdf + +The CLI toHtml command wrote with a bare Files.writeString and failed with +NoSuchFileException where toPdf to the same path succeeded. Route it through +Gmd.gmdToHtml(String, File), which already ensures the parent directory. + +Co-Authored-By: Claude Opus 5 " +``` + +--- + +## Final verification + +- [ ] **Full build from clean** + +```bash +mvn -o clean install +cd gmd-gradle-plugin && ./gradlew clean build && cd .. +``` + +Expected: BUILD SUCCESS for both. Maven test count 103 (85 baseline, +19 new, -1 deleted `indentedCodeBlocksAreAKnownGap`); Gradle test count 4 baseline + 6 new = 10. + +- [ ] **Confirm the change surface matches the plan** + +```bash +git diff d7dfcd4 --stat +``` + +Expected: exactly the files listed in File Structure (Modified and Test files modified — +the eight modified sources/docs plus the two release notes, and the four test files) and +nothing else. The release notes recorded in Tasks 1, 2, 3, 4 and 5 are part of the planned +change surface, not extra drift. + +- [ ] **Re-run the two findings that had no automated repro before this plan** + +```bash +# Finding #1 - the body must now line up with the fence +printf 'Example:\n\n ```{groovy}\n out.println("X")\n ```\n\nend\n' > /tmp/f1.gmd +java -jar gmd-core/target/gmd-3.2.0-SNAPSHOT.jar toHtml /tmp/f1.gmd /tmp/f1.html && grep -c 'X' /tmp/f1.html + +# Finding #8 - a missing parent directory must be created +rm -rf /tmp/gmd-nested +java -jar gmd-core/target/gmd-3.2.0-SNAPSHOT.jar toHtml /tmp/f1.gmd /tmp/gmd-nested/a/b/f1.html +test -f /tmp/gmd-nested/a/b/f1.html && echo "finding 8 fixed" +``` From 99d69653314e829ebe12cd2947ce3c7ff00c29c3 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 17:03:23 +0200 Subject: [PATCH 11/24] fix versions --- README.md | 12 ++++++------ gmd-gradle-plugin/readme.md | 2 +- gmd-maven-plugin/readme.md | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index ea94278..6d4c41f 100644 --- a/README.md +++ b/README.md @@ -174,8 +174,8 @@ The library, which requires Java 21 or later, is available from maven central: Gradle: ```groovy -def groovyVersion = '5.1.1' // any 4.x version should work -implementation "se.alipsa.gmd:gmd-core:3.1.0" +def groovyVersion = '5.1.3' // any 5.x version should work +implementation "se.alipsa.gmd:gmd-core:3.2.0" implementation "org.apache.groovy:groovy:${groovyVersion}" implementation "org.apache.groovy:groovy-templates:${groovyVersion}" implementation "org.apache.groovy:groovy-jsr223:${groovyVersion}" @@ -189,22 +189,22 @@ Maven: se.alipsa.gmd gmd-core - 3.1.0 + 3.2.0 org.apache.groovy groovy - 5.1.1 + 5.1.3 org.apache.groovy groovy-templates - 5.1.1 + 5.1.3 org.apache.groovy groovy-jsr223 - 5.1.1 + 5.1.3 org.apache.ivy diff --git a/gmd-gradle-plugin/readme.md b/gmd-gradle-plugin/readme.md index e23d6b9..45a00cc 100644 --- a/gmd-gradle-plugin/readme.md +++ b/gmd-gradle-plugin/readme.md @@ -55,7 +55,7 @@ plugins { id('se.alipsa.gmd.gmd-gradle-plugin') } group = 'my.group' -version = '3.1.0' +version = '3.2.0' gmdPlugin { sourceDir = 'src/test/gmd' diff --git a/gmd-maven-plugin/readme.md b/gmd-maven-plugin/readme.md index dc83dfc..8ea1ecc 100644 --- a/gmd-maven-plugin/readme.md +++ b/gmd-maven-plugin/readme.md @@ -9,7 +9,7 @@ Usage is as follows: se.alipsa.gmd gmd-maven-plugin - 3.1.0 + 3.2.0 processGmd src/test/gmd @@ -44,7 +44,7 @@ If you don't want to run the plugin explicitly, you can add it to an existing li se.alipsa.gmd gmd-maven-plugin - 3.1.0 + 3.2.0 compile @@ -75,14 +75,14 @@ You can override the versions of dependencies used by the plugin by specifying t se.alipsa.gmd gmd-maven-plugin - 3.1.0 + 3.2.0 src/test/gmd target/gmd html - 5.0.3 + 5.1.3 2.26.1 - 3.0.1 + 3.1.0 From e605a2523869d28821f718d99412efd8658e613d Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 17:13:04 +0200 Subject: [PATCH 12/24] fix: cover Markdown leaf and block quote indentation --- .../alipsa/gmd/core/GmdTemplateEngine.groovy | 57 ++++++++++++++++--- .../groovy/gmd/GmdTemplateEngineTest.groovy | 42 ++++++++++++++ .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 20 ++++--- .../gmd/gradle/GmdGradlePluginTest.groovy | 5 +- 4 files changed, 106 insertions(+), 18 deletions(-) diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index f8143bf..f346282 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -62,6 +62,7 @@ class GmdTemplateEngine { int codeBlockIndent = 0 boolean inIndentedCode = false boolean previousLineWasParagraph = false + boolean previousLineWasBlockQuote = false int listContentColumn = -1 String noSpaceLine StringBuilder codeBlockText = new StringBuilder() @@ -125,29 +126,41 @@ class GmdTemplateEngine { //println("evaluating code block: $codeBlock") engine.eval(codeBlock + '\n""') def output = out.toString() + boolean emittedMarkdown = echo if (output.length() > 0) { result.append(output) if (!output.endsWith('\n')) { result.append('\n') } + emittedMarkdown = true } out.clear() codeBlockText.setLength(0) codeBlockEnd = false - inIndentedCode = false - previousLineWasParagraph = false - listContentColumn = -1 + if (emittedMarkdown) { + inIndentedCode = false + previousLineWasParagraph = false + previousLineWasBlockQuote = false + listContentColumn = -1 + } } else if (!codeBlockStart) { if (plainFenceChar != null || startsPlainFence) { inIndentedCode = false previousLineWasParagraph = false } else if (line.isBlank()) { previousLineWasParagraph = false + previousLineWasBlockQuote = false } else { - int relativeIndent = leadingSpaces(line) - documentIndent - def listMarker = line =~ /^\s*([-*+]|\d+[.)])\s+/ + String contentLine = withoutDocumentAndBlockQuotePrefixes(line, documentIndent) + boolean isBlockQuote = contentLine != withoutDocumentIndent(line, documentIndent) + if (isBlockQuote && !previousLineWasBlockQuote) { + previousLineWasParagraph = false + listContentColumn = -1 + } + int relativeIndent = leadingSpaces(contentLine) + def listMarker = contentLine =~ /^\s*([-*+]|\d+[.)])\s+/ if (relativeIndent < 4 && listMarker.find()) { - listContentColumn = listMarker.end() - documentIndent + listContentColumn = listMarker.end() inIndentedCode = false } else { if (listContentColumn >= 0 && relativeIndent < listContentColumn) { @@ -163,7 +176,8 @@ class GmdTemplateEngine { } } } - previousLineWasParagraph = !inIndentedCode + previousLineWasParagraph = !inIndentedCode && !isNonParagraphLeafBlock(contentLine) + previousLineWasBlockQuote = isBlockQuote } if (plainFenceChar == null && !inIndentedCode && line.contains('`=')) { shouldBeProcessed = true @@ -236,6 +250,35 @@ class GmdTemplateEngine { return i } + /** Remove the document's base indent without treating a shallower line as negative. */ + private static String withoutDocumentIndent(String line, int documentIndent) { + return line.substring(Math.min(documentIndent, leadingSpaces(line))) + } + + /** + * Remove block-quote container markers before measuring content indentation. + * A block quote may itself be indented by the document's base indent. + */ + private static String withoutDocumentAndBlockQuotePrefixes(String line, int documentIndent) { + String content = withoutDocumentIndent(line, documentIndent) + while (true) { + Matcher marker = content =~ /^ {0,3}>[ \t]?/ + if (!marker.find()) { + return content + } + content = content.substring(marker.end()) + } + } + + /** Headings and rules are leaf blocks, so following indented lines start code blocks. */ + private static boolean isNonParagraphLeafBlock(String line) { + if (line ==~ /^ {0,3}#{1,6}(?:[ \t]+.*)?$/ || line ==~ /^ {0,3}[=-]+[ \t]*$/) { + return true + } + String marker = line.trim().replaceAll(/[ \t]/, '') + return marker.length() >= 3 && (marker ==~ /\*+/ || marker ==~ /_+/ || marker ==~ /-+/) + } + /** * The indent the document sits at: the indent of its first non-blank line. * Indented code blocks are recognised relative to this, so a .gmd written diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index 563e83e..652890c 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -373,6 +373,48 @@ Some prose assertTrue(processed.contains('continues here with 5'), processed) } + @Test + void indentedCodeAfterLeafBlocksIsLeftAlone() { + ['# Heading', 'Title\n=====', '---'].each { leafBlock -> + String text = "```{groovy echo=false}\nx = 5\n```\n${leafBlock}\n literal `= x ` here\n" + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), processed) + assertFalse(processed.contains('literal 5'), processed) + } + } + + @Test + void indentedCodeInsideABlockQuoteIsLeftAlone() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +> literal `= x ` here +''' + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), processed) + assertFalse(processed.contains('literal 5'), processed) + } + + @Test + void anEmptyEchoFalseBlockPreservesThePreviousParagraph() { + String text = ''' +prose line +```{groovy echo=false} +x = 5 +``` + continued `= x ` here +''' + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('continued 5 here'), processed) + } + @Test void aBlankLineDoesNotEndAnIndentedCodeBlock() { String text = ''' diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 029aa5b..9ec4f87 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -73,7 +73,10 @@ abstract class ProcessGmdTask extends DefaultTask { @Input abstract org.gradle.api.provider.Property getOutputType() - @org.gradle.api.tasks.Classpath + // Resolving this detached configuration is deferred until process() knows + // there is a .gmd file to process. @Classpath would resolve it while Gradle + // snapshots task inputs, before that no-op check can run. + @org.gradle.api.tasks.Internal abstract ConfigurableFileCollection getClasspath() @Input @@ -92,6 +95,10 @@ abstract class ProcessGmdTask extends DefaultTask { logger.warn("Source directory ${source.canonicalPath} does not exist, nothing to do") return } + if (gmdFilesIn(source).length == 0) { + logger.quiet("No gmd files found in ${source.canonicalPath}, nothing to do") + return + } if (!target.exists()) { if (!target.mkdirs() && !target.isDirectory()) { throw new IllegalArgumentException("Could not create target directory ${target.canonicalPath}") @@ -113,15 +120,10 @@ abstract class ProcessGmdTask extends DefaultTask { spec.args = [source.canonicalPath, target.canonicalPath, output] } result.assertNormalExitValue() - File[] sourceFiles = gmdFilesIn(source) - if (sourceFiles.length > 0) { - if (target.exists()) { - logger.quiet("Gmd files processed and written to ${target.canonicalPath}") - } else { - logger.warn("${target.canonicalPath} should exists but does not, something is probably wrong") - } + if (target.exists()) { + logger.quiet("Gmd files processed and written to ${target.canonicalPath}") } else { - logger.quiet("No gmd files found in ${source.canonicalPath}, nothing to do") + logger.warn("${target.canonicalPath} should exists but does not, something is probably wrong") } } diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 6d31e8a..022a580 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -148,7 +148,7 @@ class GmdGradlePluginTest { sourceDir = 'src/test/gmd' targetDir = 'build/target' outputType = 'html' - gmdVersion = '3.1.0' + gmdVersion = 'not-a-real-gmd-version' runTaskBefore = 'build' } '''.stripIndent() @@ -187,7 +187,7 @@ class GmdGradlePluginTest { sourceDir = 'src/test/gmd' targetDir = 'build/target' outputType = 'html' - gmdVersion = '3.1.0' + gmdVersion = 'not-a-real-gmd-version' runTaskBefore = 'build' } '''.stripIndent() @@ -197,6 +197,7 @@ class GmdGradlePluginTest { Assertions.assertTrue(result.output.contains('No gmd files found in'), result.output) Assertions.assertFalse(result.output.contains('Gmd files processed and written to'), result.output) + Assertions.assertFalse(new File(testProjectDir, 'build/target').exists(), result.output) } finally { new AntBuilder().delete(dir: testProjectDir, failonerror: false) } From df844232b24c95d55f50512d37770f04c196280c Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 18:20:50 +0200 Subject: [PATCH 13/24] =?UTF-8?q?Finding=201=20=E2=80=94=20Gradle=20PREFER?= =?UTF-8?q?=5FSETTINGS=20repository=20mutation=20(confirmed).=20Verified?= =?UTF-8?q?=20empirically=20with=20a=20real=20Gradle=209.7.1=20build:=20un?= =?UTF-8?q?der=20PREFER=5FSETTINGS,=20project.repositories.mavenCentral()?= =?UTF-8?q?=20does=20not=20throw=20InvalidUserCodeException=20=E2=80=94=20?= =?UTF-8?q?it=20silently=20succeeds=20and=20triggers=20a=20warning=20that?= =?UTF-8?q?=20Gradle=20now=20flags=20as=20deprecated=20(incompatible=20wit?= =?UTF-8?q?h=20Gradle=2010).=20The=20old=20hasMavenCentral=20+=20catch-exc?= =?UTF-8?q?eption=20guard=20was=20blind=20to=20this.=20Fixed=20by=20detect?= =?UTF-8?q?ing=20the=20resolved=20RepositoriesMode=20up=20front=20via=20Gr?= =?UTF-8?q?adle's=20internal=20GradleInternal/SettingsInternal=20API=20(no?= =?UTF-8?q?=20public=20API=20exposes=20this=20to=20a=20project=20plugin;?= =?UTF-8?q?=20guarded=20with=20a=20try/catch(Throwable)=20fallback=20in=20?= =?UTF-8?q?case=20that=20internal=20API=20ever=20changes).=20Added=20a=20T?= =?UTF-8?q?estKit=20test=20(preferSettingsRepositoriesAreNotMutatedByThePr?= =?UTF-8?q?oject)=20proving=20project.repositories.size()=20stays=200=20an?= =?UTF-8?q?d=20the=20deprecation=20warning=20never=20fires.=20Also=20fixed?= =?UTF-8?q?=20the=20sibling=20settingsManagedRepositoriesDoNotBreakTheBuil?= =?UTF-8?q?d=20test,=20which=20was=20failing=20on=20this=20branch=20due=20?= =?UTF-8?q?to=20an=20unrelated=20typo=20(gmdVersion=20=3D=20'not-a-real-gm?= =?UTF-8?q?d-version').?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 2 — Maven plugin forced-destroy not awaited (confirmed). destroyForcibly() is asynchronous; the Mojo could return with the forked JVM still alive. Added destroyForciblyAndAwaitTermination, which blocks on waitFor() after the forced kill and retries if that wait is itself interrupted, restoring the interrupt flag once termination is confirmed. Added two unit tests covering both the "actually waits" and "second interrupt preserved" behavior. Also updated gmd-gradle-plugin/release.md and readme.md to reflect the corrected repository-detection behavior (the old readme's guidance was itself misleading — declaring a private mirror without setting repositoriesMode wouldn't have stopped the plugin from adding Central). --- gmd-gradle-plugin/readme.md | 8 ++- gmd-gradle-plugin/release.md | 4 +- .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 27 +++++++- .../gmd/gradle/GmdGradlePluginTest.groovy | 68 ++++++++++++++++++- .../se/alipsa/gmd/maven/GmdMavenPlugin.java | 27 +++++++- .../alipsa/gmd/maven/GmdMavenPluginTest.java | 40 +++++++++++ 6 files changed, 167 insertions(+), 7 deletions(-) diff --git a/gmd-gradle-plugin/readme.md b/gmd-gradle-plugin/readme.md index 45a00cc..7ba3462 100644 --- a/gmd-gradle-plugin/readme.md +++ b/gmd-gradle-plugin/readme.md @@ -38,8 +38,12 @@ The plugin resolves `gmd-core` and Groovy through a detached configuration, whic project's repositories. If the project has not declared Maven Central, the plugin adds it. If it has — under either `repo.maven.apache.org` or `repo1.maven.org` — nothing is added. A project that resolves only through a private mirror should declare that mirror in -`dependencyResolutionManagement` in `settings.gradle` so the plugin has no reason to add -Central to the project itself. +`dependencyResolutionManagement` in `settings.gradle` with `repositoriesMode` set to +`RepositoriesMode.PREFER_SETTINGS` or `FAIL_ON_PROJECT_REPOS`, so the plugin has no reason +to add Central to the project itself — the plugin detects either mode and skips the add. +Declaring the mirror alone, without one of those modes, is not enough: Gradle's default +`PREFER_PROJECT` mode still lets a project-level repository (including the one this plugin +would add) take precedence over the settings-declared mirror. The target task is called `processGmd` so it can be invoked from the command line as follows: diff --git a/gmd-gradle-plugin/release.md b/gmd-gradle-plugin/release.md index a169ca3..c8e54d9 100644 --- a/gmd-gradle-plugin/release.md +++ b/gmd-gradle-plugin/release.md @@ -3,7 +3,9 @@ ## v3.2.0, unreleased - **Breaking:** `GmdGradlePlugin.hasRepository(Project, MavenArtifactRepository)` is removed. It was public static API on the plugin class; nothing in the repo called it. The plugin now - checks for an existing Maven Central declaration itself before adding one. + checks for an existing Maven Central declaration itself before adding one, and also detects + when repositories are settings-managed (`PREFER_SETTINGS` or `FAIL_ON_PROJECT_REPOS`) so it + never adds a project-level repository Gradle would ignore or reject. - derive the default GMD core version from a generated plugin-version resource populated from the root Maven revision, failing clearly if metadata is unavailable - derive the development artifact version from the root Maven revision and require an explicit version when publishing - upgrade the Gradle Versions Plugin from 0.58.0 to 0.61.0 diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index 483194e..ba53b72 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -9,6 +9,8 @@ import org.gradle.api.artifacts.Configuration import org.gradle.api.artifacts.Dependency import org.gradle.api.artifacts.repositories.ArtifactRepository import org.gradle.api.artifacts.repositories.MavenArtifactRepository +import org.gradle.api.initialization.resolve.RepositoriesMode +import org.gradle.api.internal.GradleInternal import org.gradle.api.tasks.TaskProvider import java.io.IOException @@ -110,7 +112,7 @@ class GmdGradlePlugin implements Plugin { static Configuration addDependencies(Project project, String groovyVersion, String log4jVersion, String gmdVersion, String ivyVersion) { - if (!hasMavenCentral(project)) { + if (!isRepositoriesModeSettingsManaged(project) && !hasMavenCentral(project)) { try { project.repositories.mavenCentral() } catch (InvalidUserCodeException e) { @@ -137,4 +139,27 @@ class GmdGradlePlugin implements Plugin { MAVEN_CENTRAL_HOSTS.contains(((MavenArtifactRepository) repository).url?.host) } } + + /** + * Under {@code PREFER_SETTINGS}, Gradle does not reject a project-declared repository + * with {@link InvalidUserCodeException} the way it does under {@code FAIL_ON_PROJECT_REPOS}; + * it silently accepts (and deprecates) it instead. That leaves the {@code hasMavenCentral} + * check blind - project.repositories still reports zero entries beforehand - so without this + * check the plugin would add, and permanently mutate every consumer's project.repositories + * with, a repository that dependency resolution ignores anyway. There is no public API for a + * project plugin to read the resolved repositories mode, so this reaches into Gradle's + * internal API (bundled by gradleApi(), verified on Gradle 9.7.1). If that internal API is + * ever unavailable, this falls back to false and the InvalidUserCodeException guard in + * addDependencies still handles FAIL_ON_PROJECT_REPOS as before. + */ + private static boolean isRepositoriesModeSettingsManaged(Project project) { + try { + GradleInternal gradleInternal = (GradleInternal) project.gradle + RepositoriesMode mode = gradleInternal.settings.dependencyResolutionManagement.repositoriesMode + .getOrElse(RepositoriesMode.PREFER_PROJECT) + return mode != RepositoriesMode.PREFER_PROJECT + } catch (Throwable ignored) { + return false + } + } } diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 022a580..aa61d38 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -148,7 +148,7 @@ class GmdGradlePluginTest { sourceDir = 'src/test/gmd' targetDir = 'build/target' outputType = 'html' - gmdVersion = 'not-a-real-gmd-version' + gmdVersion = '3.1.0' // Keep the standalone TestKit test independent of unpublished snapshots. runTaskBefore = 'build' } '''.stripIndent() @@ -163,6 +163,72 @@ class GmdGradlePluginTest { } } + @Test + void preferSettingsRepositoriesAreNotMutatedByTheProject() { + // Copilot review on PR #11: under PREFER_SETTINGS, project.repositories.mavenCentral() + // is not rejected with InvalidUserCodeException the way it is under + // FAIL_ON_PROJECT_REPOS - Gradle silently accepts (and deprecates) it instead. Verify + // the plugin detects PREFER_SETTINGS up front and never adds to project.repositories, + // so the build carries no deprecation warning and the settings repositories are what + // actually resolve gmd-core. ProjectBuilder cannot model dependencyResolutionManagement, + // so this is a TestKit test. + File testProjectDir = new File('build/gmdPreferSettingsReposTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'test.gmd').text = '# Greetings\n' + + new File(testProjectDir, 'settings.gradle').text = ''' + pluginManagement { + repositories { mavenCentral() } + plugins { id 'se.alipsa.gmd.gmd-gradle-plugin' version '1.0.0' } + } + dependencyResolutionManagement { + repositoriesMode = RepositoriesMode.PREFER_SETTINGS + repositories { mavenCentral() } + } + '''.stripIndent() + + // Deliberately no project-level repositories block. + new File(testProjectDir, 'build.gradle').text = ''' + plugins { + id('base') + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + gmdPlugin { + sourceDir = 'src/test/gmd' + targetDir = 'build/target' + outputType = 'html' + gmdVersion = '3.1.0' + runTaskBefore = 'build' + } + tasks.register('printRepoCount') { + doLast { println "repoCount=" + project.repositories.size() } + } + '''.stripIndent() + + def result = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('processGmd', 'printRepoCount') + .withPluginClasspath() + .forwardOutput() + .build() + + Assertions.assertEquals(SUCCESS, result.task(':processGmd').outcome, + "PREFER_SETTINGS must not break the plugin:\n${result.output}") + Assertions.assertTrue(new File(testProjectDir, 'build/target/test.html').exists(), + 'The detached configuration must still resolve through the settings repositories') + Assertions.assertTrue(result.output.contains('repoCount=0'), + "The plugin must not add to project.repositories under PREFER_SETTINGS:\n${result.output}") + Assertions.assertFalse( + result.output.contains('prefer settings repositories over project repositories'), + "The plugin must not trigger Gradle's PREFER_SETTINGS deprecation warning:\n${result.output}") + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } + @Test void aSourceDirWithoutGmdFilesReportsNothingToDo() { File testProjectDir = new File('build/gmdNoGmdFilesTest') diff --git a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java index e478384..d6641f6 100644 --- a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java +++ b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java @@ -199,10 +199,10 @@ public void execute() throws MojoExecutionException, MojoFailureException { process.destroy(); try { if (!process.waitFor(5, TimeUnit.SECONDS)) { - process.destroyForcibly(); + destroyForciblyAndAwaitTermination(process); } } catch (InterruptedException swallowed) { - process.destroyForcibly(); + destroyForciblyAndAwaitTermination(process); } interrupted = true; throw new MojoExecutionException("Interrupted while waiting for the GMD processor", e); @@ -251,6 +251,29 @@ public void execute() throws MojoExecutionException, MojoFailureException { } } + /** + * {@link Process#destroyForcibly()} is asynchronous, so a caller that returns right + * after calling it can race the forked JVM's actual exit. This blocks until the + * process has really terminated, retrying {@link Process#waitFor()} if the wait + * itself is interrupted again rather than giving up and restoring that second + * interrupt on the calling thread once termination is confirmed. + */ + private static void destroyForciblyAndAwaitTermination(Process process) { + process.destroyForcibly(); + boolean interruptedAgain = false; + while (true) { + try { + process.waitFor(); + break; + } catch (InterruptedException e) { + interruptedAgain = true; + } + } + if (interruptedAgain) { + Thread.currentThread().interrupt(); + } + } + private List resolveDependencies() throws DependencyResolutionException { RepositorySystemSession repoSession = session.getRepositorySession(); diff --git a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java index 0f14331..58730df 100644 --- a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java +++ b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.nio.file.Files; import java.util.Arrays; import java.util.Comparator; @@ -139,6 +140,45 @@ public void testGmdMavenPluginReportsForkFailure() throws Exception { "The plugin's own failure must not be rewrapped: " + exception.getMessage()); } + @Test + public void destroyForciblyAndAwaitTerminationWaitsForRealTermination() throws Exception { + // Copilot review on PR #11: Process.destroyForcibly() is asynchronous, so returning + // right after calling it can race the forked JVM's actual exit. Verify the helper + // actually blocks on waitFor() after the forced kill instead of just firing it. + Process process = Mockito.mock(Process.class); + when(process.waitFor()).thenReturn(1); + + invokeDestroyForciblyAndAwaitTermination(process); + + Mockito.verify(process).destroyForcibly(); + Mockito.verify(process).waitFor(); + } + + @Test + public void destroyForciblyAndAwaitTerminationRetriesAndRestoresASecondInterrupt() throws Exception { + // A second interrupt arriving while waiting out the forced kill must not be lost: + // the wait is retried until termination is confirmed, then the interrupt is + // restored on the calling thread. + Process process = Mockito.mock(Process.class); + when(process.waitFor()) + .thenThrow(new InterruptedException("interrupted while awaiting forced termination")) + .thenReturn(1); + + Thread.interrupted(); // clear any stray flag left over from another test + invokeDestroyForciblyAndAwaitTermination(process); + + assertTrue(Thread.interrupted(), + "A second interrupt during the forced-kill wait must be restored on the caller's thread"); + Mockito.verify(process, Mockito.times(1)).destroyForcibly(); + Mockito.verify(process, Mockito.times(2)).waitFor(); + } + + private static void invokeDestroyForciblyAndAwaitTermination(Process process) throws Exception { + Method method = GmdMavenPlugin.class.getDeclaredMethod("destroyForciblyAndAwaitTermination", Process.class); + method.setAccessible(true); + method.invoke(null, process); + } + private static void deleteDirectory(File directory) throws IOException { if (!directory.exists()) { return; From 61dbd42aa75d1d09cf0a30ccd38d317459f354b5 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 18:39:17 +0200 Subject: [PATCH 14/24] fix: preserve Gradle cleanup and task inputs --- .../alipsa/gmd/core/GmdTemplateEngine.groovy | 10 ++++- .../groovy/gmd/GmdTemplateEngineTest.groovy | 2 +- .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 4 ++ .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 18 ++++++++- .../gmd/gradle/GmdGradlePluginTest.groovy | 24 ++++++++++++ .../se/alipsa/gmd/maven/GmdMavenPlugin.java | 31 +++++++-------- .../alipsa/gmd/maven/GmdMavenPluginTest.java | 38 ++++++++++++------- 7 files changed, 92 insertions(+), 35 deletions(-) diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index f346282..8c6ec56 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -159,7 +159,13 @@ class GmdTemplateEngine { } int relativeIndent = leadingSpaces(contentLine) def listMarker = contentLine =~ /^\s*([-*+]|\d+[.)])\s+/ - if (relativeIndent < 4 && listMarker.find()) { + boolean isLeafBlock = isNonParagraphLeafBlock(contentLine) + if (isLeafBlock) { + // CommonMark gives thematic breaks precedence over list markers + // such as the leading "* " in "* * *". + inIndentedCode = false + listContentColumn = -1 + } else if (relativeIndent < 4 && listMarker.find()) { listContentColumn = listMarker.end() inIndentedCode = false } else { @@ -176,7 +182,7 @@ class GmdTemplateEngine { } } } - previousLineWasParagraph = !inIndentedCode && !isNonParagraphLeafBlock(contentLine) + previousLineWasParagraph = !inIndentedCode && !isLeafBlock previousLineWasBlockQuote = isBlockQuote } if (plainFenceChar == null && !inIndentedCode && line.contains('`=')) { diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index 652890c..f16e5f8 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -375,7 +375,7 @@ Some prose @Test void indentedCodeAfterLeafBlocksIsLeftAlone() { - ['# Heading', 'Title\n=====', '---'].each { leafBlock -> + ['# Heading', 'Title\n=====', '---', '* * *', '- - -'].each { leafBlock -> String text = "```{groovy echo=false}\nx = 5\n```\n${leafBlock}\n literal `= x ` here\n" String processed = GmdTemplateEngine.processCodeBlocks(text) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index ba53b72..85cfc70 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -62,6 +62,10 @@ class GmdGradlePlugin implements Plugin { task.targetDir.set(resolvedTargetDir) task.outputType.set(outputType) task.classpath.from(configuration) + task.groovyVersion.set(extension.groovyVersion.get()) + task.log4jVersion.set(extension.log4jVersion.get()) + task.gmdVersion.set(extension.gmdVersion.get()) + task.ivyVersion.set(extension.ivyVersion.get()) task.targetDirIsDefaultGmdOutput.set(targetDirIsDefaultGmdOutput) if (targetDirIsDefaultGmdOutput) { task.dedicatedOutputDir.set(resolvedTargetDir) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 9ec4f87..88a5444 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -79,6 +79,18 @@ abstract class ProcessGmdTask extends DefaultTask { @org.gradle.api.tasks.Internal abstract ConfigurableFileCollection getClasspath() + @Input + abstract org.gradle.api.provider.Property getGroovyVersion() + + @Input + abstract org.gradle.api.provider.Property getLog4jVersion() + + @Input + abstract org.gradle.api.provider.Property getGmdVersion() + + @Input + abstract org.gradle.api.provider.Property getIvyVersion() + @Input abstract org.gradle.api.provider.Property getTargetDirIsDefaultGmdOutput() @@ -95,7 +107,11 @@ abstract class ProcessGmdTask extends DefaultTask { logger.warn("Source directory ${source.canonicalPath} does not exist, nothing to do") return } - if (gmdFilesIn(source).length == 0) { + File[] sourceFiles = gmdFilesIn(source) + if (sourceFiles.length == 0) { + if (getTargetDirIsDefaultGmdOutput().getOrElse(false) && target.isDirectory()) { + cleanStaleGeneratedFiles(source, target, output) + } logger.quiet("No gmd files found in ${source.canonicalPath}, nothing to do") return } diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index aa61d38..42b4c7c 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -81,6 +81,20 @@ class GmdGradlePluginTest { 'targetDirIsDefaultGmdOutput should default to false when set only by convention') } + @Test + void runtimeVersionsAreTrackedAsTaskInputs() { + def project = ProjectBuilder.builder().build() + ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() + task.groovyVersion.set('5.1.3') + task.log4jVersion.set('2.26.1') + task.gmdVersion.set('3.1.0') + task.ivyVersion.set('2.6.0') + + Assertions.assertTrue(task.inputs.properties.keySet().containsAll( + ['groovyVersion', 'log4jVersion', 'gmdVersion', 'ivyVersion'] as Set), + 'Every runtime dependency version must invalidate processGmd when changed') + } + @Test void mavenCentralIsAddedOnlyWhenTheProjectHasNone() { def project = ProjectBuilder.builder().build() @@ -355,6 +369,16 @@ class GmdGradlePluginTest { Assertions.assertEquals(org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE, cachedResult.task(":processGmd").outcome, 'A custom target must retain Gradle up-to-date checks') + buildFile.text = buildFile.text.replace("gmdVersion = '3.1.0'", "gmdVersion = '3.0.1'") + def changedVersionResult = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('processGmd', '--configuration-cache', '--parallel') + .withPluginClasspath() + .forwardOutput() + .build() + Assertions.assertEquals(SUCCESS, changedVersionResult.task(':processGmd').outcome, + 'Changing a runtime dependency version must rerun processGmd') + // the directory differs on a mac even though they point to the same place so cannot include def expected = "Gmd files processed and written to $targetDir.canonicalPath".toString() Assertions.assertTrue(result.output.contains(expected), "expected \n$expected, but output was ${result.output}") diff --git a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java index d6641f6..50d9071 100644 --- a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java +++ b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java @@ -199,10 +199,14 @@ public void execute() throws MojoExecutionException, MojoFailureException { process.destroy(); try { if (!process.waitFor(5, TimeUnit.SECONDS)) { - destroyForciblyAndAwaitTermination(process); + if (!destroyForciblyAndAwaitTermination(process)) { + getLog().warn("GMD processor did not terminate within 30 seconds after forced termination"); + } } } catch (InterruptedException swallowed) { - destroyForciblyAndAwaitTermination(process); + if (!destroyForciblyAndAwaitTermination(process)) { + getLog().warn("GMD processor did not terminate within 30 seconds after forced termination"); + } } interrupted = true; throw new MojoExecutionException("Interrupted while waiting for the GMD processor", e); @@ -253,24 +257,17 @@ public void execute() throws MojoExecutionException, MojoFailureException { /** * {@link Process#destroyForcibly()} is asynchronous, so a caller that returns right - * after calling it can race the forked JVM's actual exit. This blocks until the - * process has really terminated, retrying {@link Process#waitFor()} if the wait - * itself is interrupted again rather than giving up and restoring that second - * interrupt on the calling thread once termination is confirmed. + * after calling it can race the forked JVM's actual exit. Wait for a bounded + * period so cancellation remains able to return if the forked JVM survives the + * forced termination. A second interrupt is restored before returning. */ - private static void destroyForciblyAndAwaitTermination(Process process) { + private static boolean destroyForciblyAndAwaitTermination(Process process) { process.destroyForcibly(); - boolean interruptedAgain = false; - while (true) { - try { - process.waitFor(); - break; - } catch (InterruptedException e) { - interruptedAgain = true; - } - } - if (interruptedAgain) { + try { + return process.waitFor(30, TimeUnit.SECONDS); + } catch (InterruptedException e) { Thread.currentThread().interrupt(); + return false; } } diff --git a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java index 58730df..60ad672 100644 --- a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java +++ b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java @@ -17,6 +17,7 @@ import java.util.Arrays; import java.util.Comparator; import java.util.List; +import java.util.concurrent.TimeUnit; import org.eclipse.aether.RepositorySystem; import org.eclipse.aether.RepositorySystemSession; @@ -146,37 +147,46 @@ public void destroyForciblyAndAwaitTerminationWaitsForRealTermination() throws E // right after calling it can race the forked JVM's actual exit. Verify the helper // actually blocks on waitFor() after the forced kill instead of just firing it. Process process = Mockito.mock(Process.class); - when(process.waitFor()).thenReturn(1); + when(process.waitFor(30, TimeUnit.SECONDS)).thenReturn(true); - invokeDestroyForciblyAndAwaitTermination(process); + assertTrue(invokeDestroyForciblyAndAwaitTermination(process)); Mockito.verify(process).destroyForcibly(); - Mockito.verify(process).waitFor(); + Mockito.verify(process).waitFor(30, TimeUnit.SECONDS); } @Test - public void destroyForciblyAndAwaitTerminationRetriesAndRestoresASecondInterrupt() throws Exception { - // A second interrupt arriving while waiting out the forced kill must not be lost: - // the wait is retried until termination is confirmed, then the interrupt is - // restored on the calling thread. + public void destroyForciblyAndAwaitTerminationRestoresASecondInterruptWithoutWaitingForever() throws Exception { + // A second interrupt must remain a cancellation escape hatch rather than being + // swallowed by an unbounded retry loop. Process process = Mockito.mock(Process.class); - when(process.waitFor()) - .thenThrow(new InterruptedException("interrupted while awaiting forced termination")) - .thenReturn(1); + when(process.waitFor(30, TimeUnit.SECONDS)) + .thenThrow(new InterruptedException("interrupted while awaiting forced termination")); Thread.interrupted(); // clear any stray flag left over from another test - invokeDestroyForciblyAndAwaitTermination(process); + assertFalse(invokeDestroyForciblyAndAwaitTermination(process)); assertTrue(Thread.interrupted(), "A second interrupt during the forced-kill wait must be restored on the caller's thread"); Mockito.verify(process, Mockito.times(1)).destroyForcibly(); - Mockito.verify(process, Mockito.times(2)).waitFor(); + Mockito.verify(process, Mockito.times(1)).waitFor(30, TimeUnit.SECONDS); } - private static void invokeDestroyForciblyAndAwaitTermination(Process process) throws Exception { + @Test + public void destroyForciblyAndAwaitTerminationReturnsAfterTheBoundedWait() throws Exception { + Process process = Mockito.mock(Process.class); + when(process.waitFor(30, TimeUnit.SECONDS)).thenReturn(false); + + assertFalse(invokeDestroyForciblyAndAwaitTermination(process)); + + Mockito.verify(process).destroyForcibly(); + Mockito.verify(process).waitFor(30, TimeUnit.SECONDS); + } + + private static boolean invokeDestroyForciblyAndAwaitTermination(Process process) throws Exception { Method method = GmdMavenPlugin.class.getDeclaredMethod("destroyForciblyAndAwaitTermination", Process.class); method.setAccessible(true); - method.invoke(null, process); + return (boolean) method.invoke(null, process); } private static void deleteDirectory(File directory) throws IOException { From 52e1d7e4eb6147f54f273073cd9f194120f41732 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 19:05:16 +0200 Subject: [PATCH 15/24] =?UTF-8?q?=20=20=20=E2=80=A2=20testPlugin=20now=20p?= =?UTF-8?q?ins=20log4jVersion=20=3D=20'2.26.1'=20explicitly=20in=20the=20g?= =?UTF-8?q?enerated=20TestKit=20build=20script=20(GmdGradlePluginTest.groo?= =?UTF-8?q?vy:339),=20so=20the=20invalidation=20check=20doesn't=20rely=20?= =?UTF-8?q?=20=20=20=20=20on=20the=20convention=20default.=20=20=20=20?= =?UTF-8?q?=E2=80=A2=20The=20invalidation=20step=20swaps=20log4j=20instead?= =?UTF-8?q?=20of=20gmd-core=20(GmdGradlePluginTest.groovy:373):=20log4jVer?= =?UTF-8?q?sion=20=3D=20'2.26.1'=20=E2=86=92=20'2.25.1'.=20Both=20are=20@I?= =?UTF-8?q?nput=20properties=20on=20the=20task,=20so=20=20=20=20=20=20this?= =?UTF-8?q?=20exercises=20the=20identical=20invalidation=20path=20while=20?= =?UTF-8?q?leaving=20the=20runnable=20gmd-core:3.1.0=20in=20place=20?= =?UTF-8?q?=E2=80=94=20avoiding=20the=20JavaFX=20dependency=20graph=20that?= =?UTF-8?q?=20every=20pre-3.1.0=20gmd-core=20=20=20=20=20=20release=20carr?= =?UTF-8?q?ies.=20The=20assertion=20"Changing=20a=20runtime=20dependency?= =?UTF-8?q?=20version=20must=20rerun=20processGmd"=20is=20unchanged=20and?= =?UTF-8?q?=20now=20passes=20(processGmd=20reruns=20with=20SUCCESS=20after?= =?UTF-8?q?=20the=20=20=20=20=20=20swap).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 42b4c7c..31d8a58 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -336,7 +336,8 @@ class GmdGradlePluginTest { targetDir = 'build/target' outputType = 'html' gmdVersion = '3.1.0' // Keep the standalone TestKit test independent of unpublished snapshots. - runTaskBefore = 'build' // we dont have tests so specify the task to not get a warning + log4jVersion = '2.26.1' // set explicitly so the invalidation check below can swap it + runTaskBefore = 'build' // we dont have tests so specify the task to not get a warning } """.stripIndent() @@ -369,7 +370,7 @@ class GmdGradlePluginTest { Assertions.assertEquals(org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE, cachedResult.task(":processGmd").outcome, 'A custom target must retain Gradle up-to-date checks') - buildFile.text = buildFile.text.replace("gmdVersion = '3.1.0'", "gmdVersion = '3.0.1'") + buildFile.text = buildFile.text.replace("log4jVersion = '2.26.1'", "log4jVersion = '2.25.1'") def changedVersionResult = GradleRunner.create() .withProjectDir(testProjectDir) .withArguments('processGmd', '--configuration-cache', '--parallel') From f0287d5ca92c8ec395c4e6b44f4ad5e314396d25 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 19:27:58 +0200 Subject: [PATCH 16/24] =?UTF-8?q?Fix=201=20=E2=80=94=20direct=20task=20reg?= =?UTF-8?q?istration=20broken=20(MEDIUM)=20=E2=80=94=20ProcessGmdTask.groo?= =?UTF-8?q?vy:36-46=20now=20sets=20conventions=20for=20all=20four=20versio?= =?UTF-8?q?n=20@Inputs=20in=20the=20constructor,=20mirroring=20the=20=20?= =?UTF-8?q?=20=20plugin's=20defaults.=20I=20chose=20conventions=20over=20@?= =?UTF-8?q?Optional=20per=20the=20reviewer's=20tradeoff=20note:=20a=20dire?= =?UTF-8?q?ct=20registrant's=20classpath=20is=20then=20still=20tracked=20b?= =?UTF-8?q?y=20some=20version=20values=20=20=20=20rather=20than=20nothing.?= =?UTF-8?q?=20To=20prevent=20the=20two=20sites=20diverging,=20the=20litera?= =?UTF-8?q?ls=20now=20live=20as=20constants=20(DEFAULT=5FGROOVY=5FVERSION,?= =?UTF-8?q?=20DEFAULT=5FLOG4J=5FVERSION,=20DEFAULT=5FIVY=5FVERSION)=20shar?= =?UTF-8?q?ed=20=20=20=20with=20the=20extension=20conventions=20in=20GmdGr?= =?UTF-8?q?adlePlugin.groovy:28-32,=20and=20defaultGmdVersion()=20is=20now?= =?UTF-8?q?=20package-visible=20for=20the=20task's=20lazy=20convention.=20?= =?UTF-8?q?Regression=20test:=20=20=20=20directlyRegisteredTaskHasVersionI?= =?UTF-8?q?nputDefaults.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 2 — thematic break in a list item drops list context — the isLeafBlock branch in GmdTemplateEngine.groovy now applies the same dedent rule as the else branch (relativeIndent < listContentColumn), so --- aligned with the list content column keeps the list alive while * * * at column 0 still resets it. Tests: aThematicBreakInsideAListItemKeepsTheListContext, aThematicBreakDedentPastTheListContentColumnEndsTheList. Fix 3 — blank block-quote line classified as a paragraph — a quote line whose stripped content is blank (>, > ) is now classified as a blank line: it clears previousLineWasParagraph, sets previousLineWasBlockQuote, and leaves inIndentedCode/listContentColumn alone. Test: aBlankBlockQuoteLineDoesNotStartAParagraph. Fix 4 — misleading timeout diagnostic — destroyForciblyAndAwaitTermination now returns a ForcedTerminationAwait enum (TERMINATED/TIMED_OUT/INTERRUPTED), a single warnOnIncompleteForcedTermination helper replaces the two duplicated warn blocks, and the timeout is a FORCED_TERMINATION_TIMEOUT_SECONDS constant also used to build the message. The three reflection-based tests were updated to assert the enum names; the bounded-wait and interrupt-restoration behavior they pin is unchanged. Fix 6 — coupled-test comment — testPlugin now carries a comment stating that runtimeVersionsAreTrackedAsTaskInputs is load-bearing alongside the log4jVersion invalidation proof, plus a note that the pinned '2.26.1' duplicates GmdGradlePlugin.DEFAULT_LOG4J_VERSION. --- .../alipsa/gmd/core/GmdTemplateEngine.groovy | 56 ++++++++++++------- .../groovy/gmd/GmdTemplateEngineTest.groovy | 55 ++++++++++++++++++ .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 14 +++-- .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 8 +++ .../gmd/gradle/GmdGradlePluginTest.groovy | 22 +++++++- .../se/alipsa/gmd/maven/GmdMavenPlugin.java | 44 ++++++++++++--- .../alipsa/gmd/maven/GmdMavenPluginTest.java | 11 ++-- 7 files changed, 170 insertions(+), 40 deletions(-) diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index 8c6ec56..02cee9a 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -153,37 +153,51 @@ class GmdTemplateEngine { } else { String contentLine = withoutDocumentAndBlockQuotePrefixes(line, documentIndent) boolean isBlockQuote = contentLine != withoutDocumentIndent(line, documentIndent) - if (isBlockQuote && !previousLineWasBlockQuote) { + if (contentLine.isBlank()) { + // A block-quote line with no content (">" or "> ") is a blank + // line in CommonMark; classify it as one so a following + // indented line still counts as a code block, not a + // paragraph continuation. previousLineWasParagraph = false - listContentColumn = -1 - } - int relativeIndent = leadingSpaces(contentLine) - def listMarker = contentLine =~ /^\s*([-*+]|\d+[.)])\s+/ - boolean isLeafBlock = isNonParagraphLeafBlock(contentLine) - if (isLeafBlock) { - // CommonMark gives thematic breaks precedence over list markers - // such as the leading "* " in "* * *". - inIndentedCode = false - listContentColumn = -1 - } else if (relativeIndent < 4 && listMarker.find()) { - listContentColumn = listMarker.end() - inIndentedCode = false + previousLineWasBlockQuote = isBlockQuote } else { - if (listContentColumn >= 0 && relativeIndent < listContentColumn) { + if (isBlockQuote && !previousLineWasBlockQuote) { + previousLineWasParagraph = false listContentColumn = -1 } - if (listContentColumn >= 0 && relativeIndent < listContentColumn + 4) { + int relativeIndent = leadingSpaces(contentLine) + def listMarker = contentLine =~ /^\s*([-*+]|\d+[.)])\s+/ + boolean isLeafBlock = isNonParagraphLeafBlock(contentLine) + if (isLeafBlock) { + // CommonMark gives thematic breaks precedence over list markers + // such as the leading "* " in "* * *". + inIndentedCode = false + // A thematic break dedented past the list content column ends + // the list; one aligned with it (like " --- " in a list item) + // keeps the list context for the lines that follow. + if (listContentColumn >= 0 && relativeIndent < listContentColumn) { + listContentColumn = -1 + } + } else if (relativeIndent < 4 && listMarker.find()) { + listContentColumn = listMarker.end() inIndentedCode = false } else { - if (relativeIndent < 4) { + if (listContentColumn >= 0 && relativeIndent < listContentColumn) { + listContentColumn = -1 + } + if (listContentColumn >= 0 && relativeIndent < listContentColumn + 4) { inIndentedCode = false - } else if (!previousLineWasParagraph) { - inIndentedCode = true + } else { + if (relativeIndent < 4) { + inIndentedCode = false + } else if (!previousLineWasParagraph) { + inIndentedCode = true + } } } + previousLineWasParagraph = !inIndentedCode && !isLeafBlock + previousLineWasBlockQuote = isBlockQuote } - previousLineWasParagraph = !inIndentedCode && !isLeafBlock - previousLineWasBlockQuote = isBlockQuote } if (plainFenceChar == null && !inIndentedCode && line.contains('`=')) { shouldBeProcessed = true diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index f16e5f8..b8e69c1 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -498,6 +498,61 @@ x = 5 assertTrue(processed.contains('continued 5'), processed) } + @Test + void aThematicBreakInsideAListItemKeepsTheListContext() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +- item + + --- + + para `= x ` here +''' + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('para 5 here'), + "A thematic break aligned with the list content column must not end the list:\n$processed") + } + + @Test + void aThematicBreakDedentPastTheListContentColumnEndsTheList() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +- item + +--- + + literal `= x ` here +''' + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), + "A thematic break dedented past the list content column ends the list:\n$processed") + } + + @Test + void aBlankBlockQuoteLineDoesNotStartAParagraph() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +> para +> +> literal `= x ` here +''' + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), + "An indented line after a blank block-quote line is a code block:\n$processed") + } + @Test void indentedCodeAfterAListIsLeftAlone() { String text = ''' diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index 85cfc70..9088ae4 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -22,16 +22,22 @@ class GmdGradlePlugin implements Plugin { private static final List MAVEN_CENTRAL_HOSTS = ['repo.maven.apache.org', 'repo1.maven.org'] + // Shared with ProcessGmdTask's fail-safe conventions for direct task registration, + // so the two cannot silently diverge. + static final String DEFAULT_GROOVY_VERSION = '5.1.3' + static final String DEFAULT_LOG4J_VERSION = '2.26.1' + static final String DEFAULT_IVY_VERSION = '2.6.0' + @Override void apply(Project project) { GmdGradlePluginParams extension = project.extensions.create('gmdPlugin', GmdGradlePluginParams) extension.sourceDir.convention('src/main/gmd') extension.targetDir.convention('build/gmd') extension.outputType.convention('md') - extension.groovyVersion.convention('5.1.3') - extension.log4jVersion.convention('2.26.1') + extension.groovyVersion.convention(DEFAULT_GROOVY_VERSION) + extension.log4jVersion.convention(DEFAULT_LOG4J_VERSION) extension.gmdVersion.convention(project.providers.provider { defaultGmdVersion() }) - extension.ivyVersion.convention('2.6.0') + extension.ivyVersion.convention(DEFAULT_IVY_VERSION) extension.runTaskBefore.convention('test') TaskProvider processGmdTask = project.tasks.register('processGmd', ProcessGmdTask) @@ -83,7 +89,7 @@ class GmdGradlePlugin implements Plugin { } } - private static String defaultGmdVersion() { + static String defaultGmdVersion() { InputStream stream = GmdGradlePlugin.class.getResourceAsStream('/gmd-version.properties') if (stream == null) { throw new IllegalStateException( diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 88a5444..2208ba6 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -31,6 +31,14 @@ abstract class ProcessGmdTask extends DefaultTask { // going through GmdGradlePlugin's afterEvaluate wiring: skip cleanup // rather than fail validation or delete files in a directory it does not own. targetDirIsDefaultGmdOutput.convention(false) + // Fail-safe defaults mirroring GmdGradlePlugin's extension conventions, so + // direct registration also passes @Input validation. A direct registrant + // with a custom classpath should set these to its actual resolved versions + // to keep up-to-date checks accurate (classpath itself is @Internal). + groovyVersion.convention(GmdGradlePlugin.DEFAULT_GROOVY_VERSION) + log4jVersion.convention(GmdGradlePlugin.DEFAULT_LOG4J_VERSION) + gmdVersion.convention(project.provider { GmdGradlePlugin.defaultGmdVersion() }) + ivyVersion.convention(GmdGradlePlugin.DEFAULT_IVY_VERSION) } @InputDirectory diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 31d8a58..84725a9 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -95,6 +95,20 @@ class GmdGradlePluginTest { 'Every runtime dependency version must invalidate processGmd when changed') } + @Test + void directlyRegisteredTaskHasVersionInputDefaults() { + // Registering the task directly (bypassing GmdGradlePlugin's afterEvaluate wiring) + // must not leave the @Input version properties without values; Gradle would reject + // the task with "property 'groovyVersion' doesn't have a configured value". + def project = ProjectBuilder.builder().build() + ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() + + Assertions.assertEquals(GmdGradlePlugin.DEFAULT_GROOVY_VERSION, task.groovyVersion.get()) + Assertions.assertEquals(GmdGradlePlugin.DEFAULT_LOG4J_VERSION, task.log4jVersion.get()) + Assertions.assertEquals(GmdGradlePlugin.DEFAULT_IVY_VERSION, task.ivyVersion.get()) + Assertions.assertFalse(task.gmdVersion.get().isBlank()) + } + @Test void mavenCentralIsAddedOnlyWhenTheProjectHasNone() { def project = ProjectBuilder.builder().build() @@ -336,7 +350,8 @@ class GmdGradlePluginTest { targetDir = 'build/target' outputType = 'html' gmdVersion = '3.1.0' // Keep the standalone TestKit test independent of unpublished snapshots. - log4jVersion = '2.26.1' // set explicitly so the invalidation check below can swap it + log4jVersion = '2.26.1' // set explicitly so the invalidation check below can swap it; + // note this duplicates the plugin's default in GmdGradlePlugin.DEFAULT_LOG4J_VERSION runTaskBefore = 'build' // we dont have tests so specify the task to not get a warning } """.stripIndent() @@ -370,6 +385,11 @@ class GmdGradlePluginTest { Assertions.assertEquals(org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE, cachedResult.task(":processGmd").outcome, 'A custom target must retain Gradle up-to-date checks') + // This proves end-to-end that a version @Input invalidates the task, but only + // for log4jVersion; the runtimeVersionsAreTrackedAsTaskInputs unit test above + // asserts all four version properties (groovyVersion, log4jVersion, gmdVersion, + // ivyVersion) are registered as inputs. The two tests are coupled on purpose — + // do not delete the unit test as redundant; together they cover every version. buildFile.text = buildFile.text.replace("log4jVersion = '2.26.1'", "log4jVersion = '2.25.1'") def changedVersionResult = GradleRunner.create() .withProjectDir(testProjectDir) diff --git a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java index 50d9071..4d1719f 100644 --- a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java +++ b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java @@ -199,14 +199,10 @@ public void execute() throws MojoExecutionException, MojoFailureException { process.destroy(); try { if (!process.waitFor(5, TimeUnit.SECONDS)) { - if (!destroyForciblyAndAwaitTermination(process)) { - getLog().warn("GMD processor did not terminate within 30 seconds after forced termination"); - } + warnOnIncompleteForcedTermination(process); } } catch (InterruptedException swallowed) { - if (!destroyForciblyAndAwaitTermination(process)) { - getLog().warn("GMD processor did not terminate within 30 seconds after forced termination"); - } + warnOnIncompleteForcedTermination(process); } interrupted = true; throw new MojoExecutionException("Interrupted while waiting for the GMD processor", e); @@ -255,22 +251,52 @@ public void execute() throws MojoExecutionException, MojoFailureException { } } + private static final int FORCED_TERMINATION_TIMEOUT_SECONDS = 30; + /** * {@link Process#destroyForcibly()} is asynchronous, so a caller that returns right * after calling it can race the forked JVM's actual exit. Wait for a bounded * period so cancellation remains able to return if the forked JVM survives the * forced termination. A second interrupt is restored before returning. */ - private static boolean destroyForciblyAndAwaitTermination(Process process) { + private static ForcedTerminationAwait destroyForciblyAndAwaitTermination(Process process) { process.destroyForcibly(); try { - return process.waitFor(30, TimeUnit.SECONDS); + return process.waitFor(FORCED_TERMINATION_TIMEOUT_SECONDS, TimeUnit.SECONDS) + ? ForcedTerminationAwait.TERMINATED : ForcedTerminationAwait.TIMED_OUT; } catch (InterruptedException e) { Thread.currentThread().interrupt(); - return false; + return ForcedTerminationAwait.INTERRUPTED; + } + } + + /** + * Both callers of {@link #destroyForciblyAndAwaitTermination(Process)} report the + * outcome, but a timeout and a second interrupt need different diagnostics: the + * process may have exited microseconds after the interrupt, so claiming it survived + * the forced kill would be misleading. + */ + private void warnOnIncompleteForcedTermination(Process process) { + switch (destroyForciblyAndAwaitTermination(process)) { + case TIMED_OUT -> + getLog().warn("GMD processor did not terminate within " + + FORCED_TERMINATION_TIMEOUT_SECONDS + " seconds after forced termination"); + case INTERRUPTED -> + getLog().warn("Interrupted again while awaiting the GMD processor's termination " + + "after forced termination; its exit status is unknown"); + default -> { + // Terminated; nothing to report. + } } } + /** Outcome of awaiting a forcibly destroyed process, so callers can report it accurately. */ + private enum ForcedTerminationAwait { + TERMINATED, + TIMED_OUT, + INTERRUPTED + } + private List resolveDependencies() throws DependencyResolutionException { RepositorySystemSession repoSession = session.getRepositorySession(); diff --git a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java index 60ad672..889d065 100644 --- a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java +++ b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java @@ -149,7 +149,7 @@ public void destroyForciblyAndAwaitTerminationWaitsForRealTermination() throws E Process process = Mockito.mock(Process.class); when(process.waitFor(30, TimeUnit.SECONDS)).thenReturn(true); - assertTrue(invokeDestroyForciblyAndAwaitTermination(process)); + assertEquals("TERMINATED", invokeDestroyForciblyAndAwaitTermination(process).name()); Mockito.verify(process).destroyForcibly(); Mockito.verify(process).waitFor(30, TimeUnit.SECONDS); @@ -164,7 +164,7 @@ public void destroyForciblyAndAwaitTerminationRestoresASecondInterruptWithoutWai .thenThrow(new InterruptedException("interrupted while awaiting forced termination")); Thread.interrupted(); // clear any stray flag left over from another test - assertFalse(invokeDestroyForciblyAndAwaitTermination(process)); + assertEquals("INTERRUPTED", invokeDestroyForciblyAndAwaitTermination(process).name()); assertTrue(Thread.interrupted(), "A second interrupt during the forced-kill wait must be restored on the caller's thread"); @@ -177,16 +177,17 @@ public void destroyForciblyAndAwaitTerminationReturnsAfterTheBoundedWait() throw Process process = Mockito.mock(Process.class); when(process.waitFor(30, TimeUnit.SECONDS)).thenReturn(false); - assertFalse(invokeDestroyForciblyAndAwaitTermination(process)); + assertEquals("TIMED_OUT", invokeDestroyForciblyAndAwaitTermination(process).name()); Mockito.verify(process).destroyForcibly(); Mockito.verify(process).waitFor(30, TimeUnit.SECONDS); } - private static boolean invokeDestroyForciblyAndAwaitTermination(Process process) throws Exception { + @SuppressWarnings("rawtypes") + private static Enum invokeDestroyForciblyAndAwaitTermination(Process process) throws Exception { Method method = GmdMavenPlugin.class.getDeclaredMethod("destroyForciblyAndAwaitTermination", Process.class); method.setAccessible(true); - return (boolean) method.invoke(null, process); + return (Enum) method.invoke(null, process); } private static void deleteDirectory(File directory) throws IOException { From b73cb791e5024ddbc8e7c3667f471f2103f1c57d Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 19:57:17 +0200 Subject: [PATCH 17/24] fix: track runtime classpath via classpathIdentity and tidy timeout diagnostics - gradle plugin: add a required classpathIdentity @Input to ProcessGmdTask so a directly registered task's runtime classpath invalidates up-to-date checks; GmdGradlePlugin derives it from the four version inputs, and direct registrants must wire it from their own configuration or fail validation loudly instead of going silently stale (the version conventions alone never reflected a real classpath, and classpath itself is @Internal) - move the DEFAULT_* version constants and defaultGmdVersion() from GmdGradlePlugin onto ProcessGmdTask, package-scoped via @PackageScope, so no new public static API lands on the published plugin class and the dependency stays plugin -> task - maven plugin: switch explicitly on TERMINATED instead of default, move FORCED_TERMINATION_TIMEOUT_SECONDS into the field block, and read it reflectively in tests instead of hard-coding 30 seconds - add TestKit proofs: an unwired classpathIdentity fails loudly, and a wired identity invalidates a directly registered task when the classpath swaps --- gmd-gradle-plugin/release.md | 1 + .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 70 ++------ .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 75 +++++++- .../gmd/gradle/GmdGradlePluginTest.groovy | 166 +++++++++++++++++- .../se/alipsa/gmd/maven/GmdMavenPlugin.java | 12 +- .../alipsa/gmd/maven/GmdMavenPluginTest.java | 18 +- 6 files changed, 267 insertions(+), 75 deletions(-) diff --git a/gmd-gradle-plugin/release.md b/gmd-gradle-plugin/release.md index c8e54d9..b9a1854 100644 --- a/gmd-gradle-plugin/release.md +++ b/gmd-gradle-plugin/release.md @@ -13,6 +13,7 @@ - validate the Gradle Plugin Portal publication before uploading the plugin - upgrade groovy to 5.1.3 - clean stale generated files only from the dedicated default `build/gmd` output directory; custom target directories retain pre-existing and orphaned GMD output when sources are deleted, renamed, or change output type, while Gradle tracks their expected generated files individually for up-to-date checks +- add a required `classpathIdentity` `@Input` to `ProcessGmdTask` so a directly registered task's runtime classpath invalidates up-to-date checks; `GmdGradlePlugin` derives it from the four version inputs, and direct registrants must wire it from their own configuration or the build fails validation ## v3.1.1, in progress - replace the deprecated `Project.getProperties()` calls used by signing configuration with `findProperty`, keeping the plugin compatible with Gradle 10 diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index 9088ae4..acf4c66 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -13,31 +13,21 @@ import org.gradle.api.initialization.resolve.RepositoriesMode import org.gradle.api.internal.GradleInternal import org.gradle.api.tasks.TaskProvider -import java.io.IOException -import java.io.InputStream -import java.util.Properties - @CompileStatic class GmdGradlePlugin implements Plugin { private static final List MAVEN_CENTRAL_HOSTS = ['repo.maven.apache.org', 'repo1.maven.org'] - // Shared with ProcessGmdTask's fail-safe conventions for direct task registration, - // so the two cannot silently diverge. - static final String DEFAULT_GROOVY_VERSION = '5.1.3' - static final String DEFAULT_LOG4J_VERSION = '2.26.1' - static final String DEFAULT_IVY_VERSION = '2.6.0' - @Override void apply(Project project) { GmdGradlePluginParams extension = project.extensions.create('gmdPlugin', GmdGradlePluginParams) extension.sourceDir.convention('src/main/gmd') extension.targetDir.convention('build/gmd') extension.outputType.convention('md') - extension.groovyVersion.convention(DEFAULT_GROOVY_VERSION) - extension.log4jVersion.convention(DEFAULT_LOG4J_VERSION) - extension.gmdVersion.convention(project.providers.provider { defaultGmdVersion() }) - extension.ivyVersion.convention(DEFAULT_IVY_VERSION) + extension.groovyVersion.convention(ProcessGmdTask.DEFAULT_GROOVY_VERSION) + extension.log4jVersion.convention(ProcessGmdTask.DEFAULT_LOG4J_VERSION) + extension.gmdVersion.convention(project.providers.provider { ProcessGmdTask.defaultGmdVersion() }) + extension.ivyVersion.convention(ProcessGmdTask.DEFAULT_IVY_VERSION) extension.runTaskBefore.convention('test') TaskProvider processGmdTask = project.tasks.register('processGmd', ProcessGmdTask) @@ -46,11 +36,12 @@ class GmdGradlePlugin implements Plugin { String sourceDir = extension.sourceDir.get() String targetDir = extension.targetDir.get() String outputType = extension.outputType.get() + String groovyVersion = extension.groovyVersion.get() + String log4jVersion = extension.log4jVersion.get() + String gmdVersion = extension.gmdVersion.get() + String ivyVersion = extension.ivyVersion.get() Configuration configuration = addDependencies(project, - extension.groovyVersion.get(), - extension.log4jVersion.get(), - extension.gmdVersion.get(), - extension.ivyVersion.get() + groovyVersion, log4jVersion, gmdVersion, ivyVersion ) File resolvedTargetDir = project.file(targetDir) @@ -68,10 +59,15 @@ class GmdGradlePlugin implements Plugin { task.targetDir.set(resolvedTargetDir) task.outputType.set(outputType) task.classpath.from(configuration) - task.groovyVersion.set(extension.groovyVersion.get()) - task.log4jVersion.set(extension.log4jVersion.get()) - task.gmdVersion.set(extension.gmdVersion.get()) - task.ivyVersion.set(extension.ivyVersion.get()) + task.groovyVersion.set(groovyVersion) + task.log4jVersion.set(log4jVersion) + task.gmdVersion.set(gmdVersion) + task.ivyVersion.set(ivyVersion) + // The classpath itself is @Internal (resolving it during input + // snapshotting would defeat the deferred no-op check), so this + // identity string is what tracks the runtime for up-to-date checks. + task.classpathIdentity.set( + "groovy=${groovyVersion}, gmd=${gmdVersion}, ivy=${ivyVersion}, log4j=${log4jVersion}".toString()) task.targetDirIsDefaultGmdOutput.set(targetDirIsDefaultGmdOutput) if (targetDirIsDefaultGmdOutput) { task.dedicatedOutputDir.set(resolvedTargetDir) @@ -89,36 +85,6 @@ class GmdGradlePlugin implements Plugin { } } - static String defaultGmdVersion() { - InputStream stream = GmdGradlePlugin.class.getResourceAsStream('/gmd-version.properties') - if (stream == null) { - throw new IllegalStateException( - 'GMD core version metadata is missing from the Gradle plugin; set gmdPlugin.gmdVersion explicitly' - ) - } - try { - Properties properties = new Properties() - properties.load(stream) - String version = properties.getProperty('gmd.version') - String normalizedVersion = version == null ? null : version.trim() - if (normalizedVersion == null || normalizedVersion.isEmpty() - || normalizedVersion.contains('$') || normalizedVersion.contains('{')) { - throw new IllegalStateException( - 'GMD core version metadata is invalid; set gmdPlugin.gmdVersion explicitly' - ) - } - return normalizedVersion - } catch (IOException e) { - throw new IllegalStateException('Could not read GMD core version metadata', e) - } finally { - try { - stream.close() - } catch (IOException ignored) { - // Ignore cleanup failures while resolving the version resource. - } - } - } - static Configuration addDependencies(Project project, String groovyVersion, String log4jVersion, String gmdVersion, String ivyVersion) { diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 2208ba6..1aa1530 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -1,6 +1,7 @@ package se.alipsa.gmd.gradle import groovy.transform.CompileStatic +import groovy.transform.PackageScope import org.gradle.api.DefaultTask import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.DirectoryProperty @@ -24,6 +25,16 @@ abstract class ProcessGmdTask extends DefaultTask { private final ExecOperations execOperations + // Defaults for the version @Input conventions below. Package-scoped so they + // do not become public static API on this published class; GmdGradlePlugin + // (same package) reuses them for its extension conventions. + @PackageScope + static final String DEFAULT_GROOVY_VERSION = '5.1.3' + @PackageScope + static final String DEFAULT_LOG4J_VERSION = '2.26.1' + @PackageScope + static final String DEFAULT_IVY_VERSION = '2.6.0' + @Inject ProcessGmdTask(ExecOperations execOperations) { this.execOperations = execOperations @@ -32,13 +43,50 @@ abstract class ProcessGmdTask extends DefaultTask { // rather than fail validation or delete files in a directory it does not own. targetDirIsDefaultGmdOutput.convention(false) // Fail-safe defaults mirroring GmdGradlePlugin's extension conventions, so - // direct registration also passes @Input validation. A direct registrant - // with a custom classpath should set these to its actual resolved versions - // to keep up-to-date checks accurate (classpath itself is @Internal). - groovyVersion.convention(GmdGradlePlugin.DEFAULT_GROOVY_VERSION) - log4jVersion.convention(GmdGradlePlugin.DEFAULT_LOG4J_VERSION) - gmdVersion.convention(project.provider { GmdGradlePlugin.defaultGmdVersion() }) - ivyVersion.convention(GmdGradlePlugin.DEFAULT_IVY_VERSION) + // direct registration passes @Input validation and the task stays runnable. + // These constants never reflect a real classpath: they exist for runnability, + // not correctness. Up-to-date checks are silently wrong when a direct + // registrant leaves them (and the unwired classpathIdentity) in place — see + // classpathIdentity for the property that restores correct invalidation. + groovyVersion.convention(DEFAULT_GROOVY_VERSION) + log4jVersion.convention(DEFAULT_LOG4J_VERSION) + gmdVersion.convention(project.provider { defaultGmdVersion() }) + ivyVersion.convention(DEFAULT_IVY_VERSION) + } + + /** + * The gmd-core version matching this plugin, from the generated + * gmd-version.properties resource. Fails clearly when the resource is + * missing or unexpanded. + */ + static String defaultGmdVersion() { + InputStream stream = ProcessGmdTask.class.getResourceAsStream('/gmd-version.properties') + if (stream == null) { + throw new IllegalStateException( + 'GMD core version metadata is missing from the Gradle plugin; set gmdPlugin.gmdVersion explicitly' + ) + } + try { + Properties properties = new Properties() + properties.load(stream) + String version = properties.getProperty('gmd.version') + String normalizedVersion = version == null ? null : version.trim() + if (normalizedVersion == null || normalizedVersion.isEmpty() + || normalizedVersion.contains('$') || normalizedVersion.contains('{')) { + throw new IllegalStateException( + 'GMD core version metadata is invalid; set gmdPlugin.gmdVersion explicitly' + ) + } + return normalizedVersion + } catch (IOException e) { + throw new IllegalStateException('Could not read GMD core version metadata', e) + } finally { + try { + stream.close() + } catch (IOException ignored) { + // Ignore cleanup failures while resolving the version resource. + } + } } @InputDirectory @@ -87,6 +135,19 @@ abstract class ProcessGmdTask extends DefaultTask { @org.gradle.api.tasks.Internal abstract ConfigurableFileCollection getClasspath() + /** + * Fingerprint of the runtime classpath used to fork GmdProcessor. The + * classpath itself is {@code @Internal} (see above), so this property is + * what invalidates up-to-date checks when the runtime changes. + * GmdGradlePlugin derives it from its four version inputs; anyone + * registering this task directly must wire it from their own configuration + * (e.g. resolved file names or coordinates). It deliberately has no + * convention: an unwired value fails validation loudly instead of letting a + * classpath swap pass up-to-date checks silently. + */ + @Input + abstract org.gradle.api.provider.Property getClasspathIdentity() + @Input abstract org.gradle.api.provider.Property getGroovyVersion() diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 84725a9..3016e20 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -48,7 +48,7 @@ class GmdGradlePluginTest { @Test void defaultGmdVersionComesFromGeneratedResource() { - URL resource = GmdGradlePlugin.class.getResource('/gmd-version.properties') + URL resource = ProcessGmdTask.class.getResource('/gmd-version.properties') Assertions.assertNotNull(resource, 'The plugin version resource must be generated during processResources') Properties properties = new Properties() @@ -64,7 +64,7 @@ class GmdGradlePluginTest { Assertions.assertEquals(expectedVersion, resourceVersion) Assertions.assertFalse(resourceVersion.contains('$'), "The generated resource must be expanded: $resourceVersion") - def method = GmdGradlePlugin.class.getDeclaredMethod('defaultGmdVersion') + def method = ProcessGmdTask.class.getDeclaredMethod('defaultGmdVersion') method.setAccessible(true) Assertions.assertEquals(resourceVersion, method.invoke(null)) } @@ -100,15 +100,28 @@ class GmdGradlePluginTest { // Registering the task directly (bypassing GmdGradlePlugin's afterEvaluate wiring) // must not leave the @Input version properties without values; Gradle would reject // the task with "property 'groovyVersion' doesn't have a configured value". + // The literals duplicate ProcessGmdTask's package-scoped DEFAULT_* constants. def project = ProjectBuilder.builder().build() ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() - Assertions.assertEquals(GmdGradlePlugin.DEFAULT_GROOVY_VERSION, task.groovyVersion.get()) - Assertions.assertEquals(GmdGradlePlugin.DEFAULT_LOG4J_VERSION, task.log4jVersion.get()) - Assertions.assertEquals(GmdGradlePlugin.DEFAULT_IVY_VERSION, task.ivyVersion.get()) + Assertions.assertEquals('5.1.3', task.groovyVersion.get()) + Assertions.assertEquals('2.26.1', task.log4jVersion.get()) + Assertions.assertEquals('2.6.0', task.ivyVersion.get()) Assertions.assertFalse(task.gmdVersion.get().isBlank()) } + @Test + void classpathIdentityHasNoConventionForDirectRegistration() { + // classpathIdentity is deliberately required with no convention: a direct + // registrant who does not wire it must fail validation loudly rather than + // have a classpath swap pass up-to-date checks silently. + def project = ProjectBuilder.builder().build() + ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() + + Assertions.assertFalse(task.classpathIdentity.isPresent(), + 'classpathIdentity must stay unwired so Gradle reports a missing value') + } + @Test void mavenCentralIsAddedOnlyWhenTheProjectHasNone() { def project = ProjectBuilder.builder().build() @@ -297,6 +310,147 @@ class GmdGradlePluginTest { } } + @Test + void directlyRegisteredTaskWithoutClasspathIdentityFailsLoudly() { + // PR #11 review: the version @Input conventions make a directly registered + // task validate, but with classpath @Internal nothing tracks a classpath + // swap. classpathIdentity is the required fingerprint; leaving it unwired + // must fail validation instead of going silently stale. + File testProjectDir = new File('build/gmdDirectTaskMissingIdentityTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'test.gmd').text = '# Greetings\n' + // Own settings file: the test project lives under this build's directory, + // so without one Gradle would adopt the plugin build's settings.gradle. + new File(testProjectDir, 'settings.gradle').text = "rootProject.name = 'gmd-direct-task-missing-identity'\n" + new File(testProjectDir, 'build.gradle').text = ''' + import se.alipsa.gmd.gradle.ProcessGmdTask + + plugins { + id('base') + // Applied for its classes only; the task below is registered directly, + // bypassing GmdGradlePlugin's processGmd wiring. + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + + repositories { mavenCentral() } + + def gmdRuntime = configurations.detachedConfiguration( + dependencies.create('se.alipsa.gmd:gmd-core:3.1.0') + ) + + tasks.register('directGmd', ProcessGmdTask) { + sourceDir = file('src/test/gmd') + targetDir = file('build/target') + outputType = 'html' + classpath.from(gmdRuntime) + } + '''.stripIndent() + + def result = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('directGmd') + .withPluginClasspath() + .forwardOutput() + .buildAndFail() + + Assertions.assertTrue(result.output.contains( + "property 'classpathIdentity' doesn't have a configured value"), + "An unwired classpathIdentity must fail loudly:\n${result.output}") + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } + + @Test + void directlyRegisteredTaskInvalidatesWhenItsWiredClasspathIdentityChanges() { + // End-to-end proof of the fix for silent stale up-to-date checks on a + // directly registered task: wiring classpathIdentity from the resolved + // runtime must invalidate the task when that runtime changes. The swap + // changes only log4j-core so the runnable gmd-core:3.1.0 stays in place + // (pre-3.1.0 gmd-core releases carry a JavaFX dependency graph). + File testProjectDir = new File('build/gmdDirectTaskIdentityTest') + try { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + File srcDir = new File(testProjectDir, 'src/test/gmd') + srcDir.mkdirs() + new File(srcDir, 'test.gmd').text = '# Greetings\n' + // Own settings file: the test project lives under this build's directory, + // so without one Gradle would adopt the plugin build's settings.gradle. + new File(testProjectDir, 'settings.gradle').text = "rootProject.name = 'gmd-direct-task-identity'\n" + File buildFile = new File(testProjectDir, 'build.gradle') + buildFile.text = ''' + import se.alipsa.gmd.gradle.ProcessGmdTask + + plugins { + id('base') + // Applied for its classes only; the task below is registered directly, + // bypassing GmdGradlePlugin's processGmd wiring. + id 'se.alipsa.gmd.gmd-gradle-plugin' + } + + repositories { mavenCentral() } + + def log4jVersion = '2.26.1' + // Mirror the runtime GmdGradlePlugin assembles: gmd-core's published POM + // does not bring Groovy transitively. + def gmdRuntime = configurations.detachedConfiguration( + dependencies.create('se.alipsa.gmd:gmd-core:3.1.0'), + dependencies.create('org.apache.groovy:groovy:5.1.3'), + dependencies.create('org.apache.groovy:groovy-templates:5.1.3'), + dependencies.create('org.apache.groovy:groovy-jsr223:5.1.3'), + dependencies.create('org.apache.ivy:ivy:2.6.0'), + dependencies.create("org.apache.logging.log4j:log4j-core:${log4jVersion}") + ) + + tasks.register('directGmd', ProcessGmdTask) { + sourceDir = file('src/test/gmd') + targetDir = file('build/target') + outputType = 'html' + classpath.from(gmdRuntime) + // The supported direct-registration contract: wire the identity from + // the actual resolved runtime so a classpath swap invalidates the task. + classpathIdentity = gmdRuntime.resolve().collect { "${it.name}-${it.length()}" }.join(',') + } + '''.stripIndent() + + def result = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('directGmd') + .withPluginClasspath() + .forwardOutput() + .build() + Assertions.assertEquals(SUCCESS, result.task(':directGmd').outcome, result.output) + Assertions.assertTrue(new File(testProjectDir, 'build/target/test.html').exists()) + + def cachedResult = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('directGmd') + .withPluginClasspath() + .forwardOutput() + .build() + Assertions.assertEquals(org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE, + cachedResult.task(':directGmd').outcome, + 'An unchanged runtime must keep the task up-to-date:\n' + cachedResult.output) + + buildFile.text = buildFile.text.replace( + "def log4jVersion = '2.26.1'", "def log4jVersion = '2.25.1'") + def changedRuntimeResult = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('directGmd') + .withPluginClasspath() + .forwardOutput() + .build() + Assertions.assertEquals(SUCCESS, changedRuntimeResult.task(':directGmd').outcome, + 'Swapping the runtime classpath must rerun a directly registered task:\n' + + changedRuntimeResult.output) + } finally { + new AntBuilder().delete(dir: testProjectDir, failonerror: false) + } + } + @Test void testPlugin() { File targetDir = null @@ -351,7 +505,7 @@ class GmdGradlePluginTest { outputType = 'html' gmdVersion = '3.1.0' // Keep the standalone TestKit test independent of unpublished snapshots. log4jVersion = '2.26.1' // set explicitly so the invalidation check below can swap it; - // note this duplicates the plugin's default in GmdGradlePlugin.DEFAULT_LOG4J_VERSION + // note this duplicates ProcessGmdTask's package-scoped default log4j version runTaskBefore = 'build' // we dont have tests so specify the task to not get a warning } """.stripIndent() diff --git a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java index 4d1719f..936f5e0 100644 --- a/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java +++ b/gmd-maven-plugin/src/main/java/se/alipsa/gmd/maven/GmdMavenPlugin.java @@ -63,6 +63,12 @@ public class GmdMavenPlugin extends AbstractMojo { @Inject private RepositorySystem repositorySystem; + /** + * Bounded wait after {@link Process#destroyForcibly()} so cancelling the build + * cannot hang forever on a forked JVM that survives the forced kill. + */ + private static final int FORCED_TERMINATION_TIMEOUT_SECONDS = 30; + /** * Default constructor. */ @@ -251,8 +257,6 @@ public void execute() throws MojoExecutionException, MojoFailureException { } } - private static final int FORCED_TERMINATION_TIMEOUT_SECONDS = 30; - /** * {@link Process#destroyForcibly()} is asynchronous, so a caller that returns right * after calling it can race the forked JVM's actual exit. Wait for a bounded @@ -284,8 +288,8 @@ private void warnOnIncompleteForcedTermination(Process process) { case INTERRUPTED -> getLog().warn("Interrupted again while awaiting the GMD processor's termination " + "after forced termination; its exit status is unknown"); - default -> { - // Terminated; nothing to report. + case TERMINATED -> { + // Nothing to report. } } } diff --git a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java index 889d065..2055efd 100644 --- a/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java +++ b/gmd-maven-plugin/src/test/java/test/alipsa/gmd/maven/GmdMavenPluginTest.java @@ -147,12 +147,12 @@ public void destroyForciblyAndAwaitTerminationWaitsForRealTermination() throws E // right after calling it can race the forked JVM's actual exit. Verify the helper // actually blocks on waitFor() after the forced kill instead of just firing it. Process process = Mockito.mock(Process.class); - when(process.waitFor(30, TimeUnit.SECONDS)).thenReturn(true); + when(process.waitFor(forcedTerminationTimeoutSeconds(), TimeUnit.SECONDS)).thenReturn(true); assertEquals("TERMINATED", invokeDestroyForciblyAndAwaitTermination(process).name()); Mockito.verify(process).destroyForcibly(); - Mockito.verify(process).waitFor(30, TimeUnit.SECONDS); + Mockito.verify(process).waitFor(forcedTerminationTimeoutSeconds(), TimeUnit.SECONDS); } @Test @@ -160,7 +160,7 @@ public void destroyForciblyAndAwaitTerminationRestoresASecondInterruptWithoutWai // A second interrupt must remain a cancellation escape hatch rather than being // swallowed by an unbounded retry loop. Process process = Mockito.mock(Process.class); - when(process.waitFor(30, TimeUnit.SECONDS)) + when(process.waitFor(forcedTerminationTimeoutSeconds(), TimeUnit.SECONDS)) .thenThrow(new InterruptedException("interrupted while awaiting forced termination")); Thread.interrupted(); // clear any stray flag left over from another test @@ -169,18 +169,24 @@ public void destroyForciblyAndAwaitTerminationRestoresASecondInterruptWithoutWai assertTrue(Thread.interrupted(), "A second interrupt during the forced-kill wait must be restored on the caller's thread"); Mockito.verify(process, Mockito.times(1)).destroyForcibly(); - Mockito.verify(process, Mockito.times(1)).waitFor(30, TimeUnit.SECONDS); + Mockito.verify(process, Mockito.times(1)).waitFor(forcedTerminationTimeoutSeconds(), TimeUnit.SECONDS); } @Test public void destroyForciblyAndAwaitTerminationReturnsAfterTheBoundedWait() throws Exception { Process process = Mockito.mock(Process.class); - when(process.waitFor(30, TimeUnit.SECONDS)).thenReturn(false); + when(process.waitFor(forcedTerminationTimeoutSeconds(), TimeUnit.SECONDS)).thenReturn(false); assertEquals("TIMED_OUT", invokeDestroyForciblyAndAwaitTermination(process).name()); Mockito.verify(process).destroyForcibly(); - Mockito.verify(process).waitFor(30, TimeUnit.SECONDS); + Mockito.verify(process).waitFor(forcedTerminationTimeoutSeconds(), TimeUnit.SECONDS); + } + + private static long forcedTerminationTimeoutSeconds() throws Exception { + Field timeout = GmdMavenPlugin.class.getDeclaredField("FORCED_TERMINATION_TIMEOUT_SECONDS"); + timeout.setAccessible(true); + return timeout.getLong(null); } @SuppressWarnings("rawtypes") From 19baec0dddb45d5fe5cde0872cac1814576a1204 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 20:47:24 +0200 Subject: [PATCH 18/24] fix: clarify Gradle classpath identity contract --- gmd-gradle-plugin/release.md | 2 +- .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 13 ++-- .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 56 ++++---------- .../gmd/gradle/GmdGradlePluginTest.groovy | 75 +++++++++---------- 4 files changed, 56 insertions(+), 90 deletions(-) diff --git a/gmd-gradle-plugin/release.md b/gmd-gradle-plugin/release.md index b9a1854..e7cb39f 100644 --- a/gmd-gradle-plugin/release.md +++ b/gmd-gradle-plugin/release.md @@ -13,7 +13,7 @@ - validate the Gradle Plugin Portal publication before uploading the plugin - upgrade groovy to 5.1.3 - clean stale generated files only from the dedicated default `build/gmd` output directory; custom target directories retain pre-existing and orphaned GMD output when sources are deleted, renamed, or change output type, while Gradle tracks their expected generated files individually for up-to-date checks -- add a required `classpathIdentity` `@Input` to `ProcessGmdTask` so a directly registered task's runtime classpath invalidates up-to-date checks; `GmdGradlePlugin` derives it from the four version inputs, and direct registrants must wire it from their own configuration or the build fails validation +- add a required, non-blank `classpathIdentity` `@Input` to `ProcessGmdTask` so a directly registered task's declared runtime selection invalidates up-to-date checks; `GmdGradlePlugin` derives it from its dependency versions, and direct registrants must wire it from their own configuration or the build fails validation ## v3.1.1, in progress - replace the deprecated `Project.getProperties()` calls used by signing configuration with `findProperty`, keeping the plugin compatible with Gradle 10 diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index acf4c66..efadf15 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -24,10 +24,10 @@ class GmdGradlePlugin implements Plugin { extension.sourceDir.convention('src/main/gmd') extension.targetDir.convention('build/gmd') extension.outputType.convention('md') - extension.groovyVersion.convention(ProcessGmdTask.DEFAULT_GROOVY_VERSION) - extension.log4jVersion.convention(ProcessGmdTask.DEFAULT_LOG4J_VERSION) + extension.groovyVersion.convention('5.1.3') + extension.log4jVersion.convention('2.26.1') extension.gmdVersion.convention(project.providers.provider { ProcessGmdTask.defaultGmdVersion() }) - extension.ivyVersion.convention(ProcessGmdTask.DEFAULT_IVY_VERSION) + extension.ivyVersion.convention('2.6.0') extension.runTaskBefore.convention('test') TaskProvider processGmdTask = project.tasks.register('processGmd', ProcessGmdTask) @@ -59,13 +59,10 @@ class GmdGradlePlugin implements Plugin { task.targetDir.set(resolvedTargetDir) task.outputType.set(outputType) task.classpath.from(configuration) - task.groovyVersion.set(groovyVersion) - task.log4jVersion.set(log4jVersion) - task.gmdVersion.set(gmdVersion) - task.ivyVersion.set(ivyVersion) // The classpath itself is @Internal (resolving it during input // snapshotting would defeat the deferred no-op check), so this - // identity string is what tracks the runtime for up-to-date checks. + // declared dependency identity is what tracks the runtime selection + // for up-to-date checks. task.classpathIdentity.set( "groovy=${groovyVersion}, gmd=${gmdVersion}, ivy=${ivyVersion}, log4j=${log4jVersion}".toString()) task.targetDirIsDefaultGmdOutput.set(targetDirIsDefaultGmdOutput) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 1aa1530..63e0b47 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -25,16 +25,6 @@ abstract class ProcessGmdTask extends DefaultTask { private final ExecOperations execOperations - // Defaults for the version @Input conventions below. Package-scoped so they - // do not become public static API on this published class; GmdGradlePlugin - // (same package) reuses them for its extension conventions. - @PackageScope - static final String DEFAULT_GROOVY_VERSION = '5.1.3' - @PackageScope - static final String DEFAULT_LOG4J_VERSION = '2.26.1' - @PackageScope - static final String DEFAULT_IVY_VERSION = '2.6.0' - @Inject ProcessGmdTask(ExecOperations execOperations) { this.execOperations = execOperations @@ -42,16 +32,6 @@ abstract class ProcessGmdTask extends DefaultTask { // going through GmdGradlePlugin's afterEvaluate wiring: skip cleanup // rather than fail validation or delete files in a directory it does not own. targetDirIsDefaultGmdOutput.convention(false) - // Fail-safe defaults mirroring GmdGradlePlugin's extension conventions, so - // direct registration passes @Input validation and the task stays runnable. - // These constants never reflect a real classpath: they exist for runnability, - // not correctness. Up-to-date checks are silently wrong when a direct - // registrant leaves them (and the unwired classpathIdentity) in place — see - // classpathIdentity for the property that restores correct invalidation. - groovyVersion.convention(DEFAULT_GROOVY_VERSION) - log4jVersion.convention(DEFAULT_LOG4J_VERSION) - gmdVersion.convention(project.provider { defaultGmdVersion() }) - ivyVersion.convention(DEFAULT_IVY_VERSION) } /** @@ -59,6 +39,7 @@ abstract class ProcessGmdTask extends DefaultTask { * gmd-version.properties resource. Fails clearly when the resource is * missing or unexpanded. */ + @PackageScope static String defaultGmdVersion() { InputStream stream = ProcessGmdTask.class.getResourceAsStream('/gmd-version.properties') if (stream == null) { @@ -136,30 +117,22 @@ abstract class ProcessGmdTask extends DefaultTask { abstract ConfigurableFileCollection getClasspath() /** - * Fingerprint of the runtime classpath used to fork GmdProcessor. The - * classpath itself is {@code @Internal} (see above), so this property is - * what invalidates up-to-date checks when the runtime changes. - * GmdGradlePlugin derives it from its four version inputs; anyone - * registering this task directly must wire it from their own configuration - * (e.g. resolved file names or coordinates). It deliberately has no - * convention: an unwired value fails validation loudly instead of letting a - * classpath swap pass up-to-date checks silently. + * Stable identity for the runtime classpath selection used to fork + * GmdProcessor. The classpath itself is {@code @Internal} (see above), so + * this property invalidates up-to-date checks when that selection changes. + * GmdGradlePlugin derives it from its declared dependency versions. Anyone + * registering this task directly must wire it from their configuration + * without resolving it during configuration, for example + * {@code cfg.allDependencies.collect { "$it.group:$it.name:$it.version" }.sort().join(',')}. + * It deliberately has no convention: an unwired value fails validation + * loudly instead of letting a classpath swap pass up-to-date checks silently. + * This is an identity of declared runtime inputs, not a fingerprint of the + * fully resolved classpath; direct registrants must update it whenever their + * runtime selection changes. */ @Input abstract org.gradle.api.provider.Property getClasspathIdentity() - @Input - abstract org.gradle.api.provider.Property getGroovyVersion() - - @Input - abstract org.gradle.api.provider.Property getLog4jVersion() - - @Input - abstract org.gradle.api.provider.Property getGmdVersion() - - @Input - abstract org.gradle.api.provider.Property getIvyVersion() - @Input abstract org.gradle.api.provider.Property getTargetDirIsDefaultGmdOutput() @@ -184,6 +157,9 @@ abstract class ProcessGmdTask extends DefaultTask { logger.quiet("No gmd files found in ${source.canonicalPath}, nothing to do") return } + if (getClasspathIdentity().get().trim().isEmpty()) { + throw new IllegalArgumentException('classpathIdentity must not be blank') + } if (!target.exists()) { if (!target.mkdirs() && !target.isDirectory()) { throw new IllegalArgumentException("Could not create target directory ${target.canonicalPath}") diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 3016e20..125ab1f 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -82,32 +82,16 @@ class GmdGradlePluginTest { } @Test - void runtimeVersionsAreTrackedAsTaskInputs() { + void classpathIdentityIsTheOnlyRuntimeSelectionInput() { def project = ProjectBuilder.builder().build() ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() - task.groovyVersion.set('5.1.3') - task.log4jVersion.set('2.26.1') - task.gmdVersion.set('3.1.0') - task.ivyVersion.set('2.6.0') - - Assertions.assertTrue(task.inputs.properties.keySet().containsAll( - ['groovyVersion', 'log4jVersion', 'gmdVersion', 'ivyVersion'] as Set), - 'Every runtime dependency version must invalidate processGmd when changed') - } + task.classpathIdentity.set('declared-runtime-dependencies') - @Test - void directlyRegisteredTaskHasVersionInputDefaults() { - // Registering the task directly (bypassing GmdGradlePlugin's afterEvaluate wiring) - // must not leave the @Input version properties without values; Gradle would reject - // the task with "property 'groovyVersion' doesn't have a configured value". - // The literals duplicate ProcessGmdTask's package-scoped DEFAULT_* constants. - def project = ProjectBuilder.builder().build() - ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() - - Assertions.assertEquals('5.1.3', task.groovyVersion.get()) - Assertions.assertEquals('2.26.1', task.log4jVersion.get()) - Assertions.assertEquals('2.6.0', task.ivyVersion.get()) - Assertions.assertFalse(task.gmdVersion.get().isBlank()) + Set inputs = task.inputs.properties.keySet() + Assertions.assertTrue(inputs.contains('classpathIdentity')) + Assertions.assertTrue(inputs.intersect( + ['groovyVersion', 'log4jVersion', 'gmdVersion', 'ivyVersion'] as Set).isEmpty(), + 'classpathIdentity must be the sole runtime-selection task input') } @Test @@ -311,11 +295,9 @@ class GmdGradlePluginTest { } @Test - void directlyRegisteredTaskWithoutClasspathIdentityFailsLoudly() { - // PR #11 review: the version @Input conventions make a directly registered - // task validate, but with classpath @Internal nothing tracks a classpath - // swap. classpathIdentity is the required fingerprint; leaving it unwired - // must fail validation instead of going silently stale. + void directlyRegisteredTaskWithoutUsableClasspathIdentityFailsLoudly() { + // classpathIdentity is the required task input; leaving it unwired must + // fail validation, while setting it blank must fail in the task action. File testProjectDir = new File('build/gmdDirectTaskMissingIdentityTest') try { new AntBuilder().delete(dir: testProjectDir, failonerror: false) @@ -325,7 +307,8 @@ class GmdGradlePluginTest { // Own settings file: the test project lives under this build's directory, // so without one Gradle would adopt the plugin build's settings.gradle. new File(testProjectDir, 'settings.gradle').text = "rootProject.name = 'gmd-direct-task-missing-identity'\n" - new File(testProjectDir, 'build.gradle').text = ''' + File buildFile = new File(testProjectDir, 'build.gradle') + String buildScript = ''' import se.alipsa.gmd.gradle.ProcessGmdTask plugins { @@ -348,6 +331,7 @@ class GmdGradlePluginTest { classpath.from(gmdRuntime) } '''.stripIndent() + buildFile.text = buildScript def result = GradleRunner.create() .withProjectDir(testProjectDir) @@ -359,6 +343,17 @@ class GmdGradlePluginTest { Assertions.assertTrue(result.output.contains( "property 'classpathIdentity' doesn't have a configured value"), "An unwired classpathIdentity must fail loudly:\n${result.output}") + + buildFile.text = buildScript.replace( + 'classpath.from(gmdRuntime)', "classpath.from(gmdRuntime)\n classpathIdentity = ''") + def blankResult = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments('directGmd') + .withPluginClasspath() + .forwardOutput() + .buildAndFail() + Assertions.assertTrue(blankResult.output.contains('classpathIdentity must not be blank'), + "A blank classpathIdentity must fail loudly:\n${blankResult.output}") } finally { new AntBuilder().delete(dir: testProjectDir, failonerror: false) } @@ -367,8 +362,8 @@ class GmdGradlePluginTest { @Test void directlyRegisteredTaskInvalidatesWhenItsWiredClasspathIdentityChanges() { // End-to-end proof of the fix for silent stale up-to-date checks on a - // directly registered task: wiring classpathIdentity from the resolved - // runtime must invalidate the task when that runtime changes. The swap + // directly registered task: wiring classpathIdentity from declared runtime + // dependencies must invalidate the task when that selection changes. The swap // changes only log4j-core so the runnable gmd-core:3.1.0 stays in place // (pre-3.1.0 gmd-core releases carry a JavaFX dependency graph). File testProjectDir = new File('build/gmdDirectTaskIdentityTest') @@ -410,9 +405,11 @@ class GmdGradlePluginTest { targetDir = file('build/target') outputType = 'html' classpath.from(gmdRuntime) - // The supported direct-registration contract: wire the identity from - // the actual resolved runtime so a classpath swap invalidates the task. - classpathIdentity = gmdRuntime.resolve().collect { "${it.name}-${it.length()}" }.join(',') + // The supported direct-registration contract: derive the identity from + // declared dependencies, without resolving the configuration eagerly. + classpathIdentity = gmdRuntime.allDependencies.collect { + "${it.group}:${it.name}:${it.version}" + }.sort().join(',') } '''.stripIndent() @@ -504,8 +501,7 @@ class GmdGradlePluginTest { targetDir = 'build/target' outputType = 'html' gmdVersion = '3.1.0' // Keep the standalone TestKit test independent of unpublished snapshots. - log4jVersion = '2.26.1' // set explicitly so the invalidation check below can swap it; - // note this duplicates ProcessGmdTask's package-scoped default log4j version + log4jVersion = '2.26.1' // set explicitly so the invalidation check below can swap it runTaskBefore = 'build' // we dont have tests so specify the task to not get a warning } """.stripIndent() @@ -539,11 +535,8 @@ class GmdGradlePluginTest { Assertions.assertEquals(org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE, cachedResult.task(":processGmd").outcome, 'A custom target must retain Gradle up-to-date checks') - // This proves end-to-end that a version @Input invalidates the task, but only - // for log4jVersion; the runtimeVersionsAreTrackedAsTaskInputs unit test above - // asserts all four version properties (groovyVersion, log4jVersion, gmdVersion, - // ivyVersion) are registered as inputs. The two tests are coupled on purpose — - // do not delete the unit test as redundant; together they cover every version. + // This proves end-to-end that classpathIdentity invalidates the task when + // the plugin changes a declared runtime dependency version. buildFile.text = buildFile.text.replace("log4jVersion = '2.26.1'", "log4jVersion = '2.25.1'") def changedVersionResult = GradleRunner.create() .withProjectDir(testProjectDir) From 9179bfee7d0ddb565eb1b0b493fa2760ed07a3a8 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 20:59:10 +0200 Subject: [PATCH 19/24] docs: clarify Gradle classpath identity limits --- gmd-gradle-plugin/release.md | 2 +- .../main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/gmd-gradle-plugin/release.md b/gmd-gradle-plugin/release.md index e7cb39f..431970c 100644 --- a/gmd-gradle-plugin/release.md +++ b/gmd-gradle-plugin/release.md @@ -13,7 +13,7 @@ - validate the Gradle Plugin Portal publication before uploading the plugin - upgrade groovy to 5.1.3 - clean stale generated files only from the dedicated default `build/gmd` output directory; custom target directories retain pre-existing and orphaned GMD output when sources are deleted, renamed, or change output type, while Gradle tracks their expected generated files individually for up-to-date checks -- add a required, non-blank `classpathIdentity` `@Input` to `ProcessGmdTask` so a directly registered task's declared runtime selection invalidates up-to-date checks; `GmdGradlePlugin` derives it from its dependency versions, and direct registrants must wire it from their own configuration or the build fails validation +- add a required `classpathIdentity` `@Input` to `ProcessGmdTask` so a directly registered task's declared runtime selection invalidates up-to-date checks; `GmdGradlePlugin` derives it from its dependency versions, direct registrants must wire it from their own configuration or the build fails validation, and blank values are rejected when the task processes GMD files ## v3.1.1, in progress - replace the deprecated `Project.getProperties()` calls used by signing configuration with `findProperty`, keeping the plugin compatible with Gradle 10 diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index 63e0b47..ce78101 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -127,8 +127,11 @@ abstract class ProcessGmdTask extends DefaultTask { * It deliberately has no convention: an unwired value fails validation * loudly instead of letting a classpath swap pass up-to-date checks silently. * This is an identity of declared runtime inputs, not a fingerprint of the - * fully resolved classpath; direct registrants must update it whenever their - * runtime selection changes. + * fully resolved classpath. Therefore, a changed transitive resolution can + * leave the task up-to-date when its declared dependencies are unchanged. + * The example updates automatically as declared dependencies change; + * registrants using a hard-coded identity must keep it in sync with their + * runtime selection. */ @Input abstract org.gradle.api.provider.Property getClasspathIdentity() From f1d0e21134942741068af27409aa31cbcaee1f06 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 21:30:45 +0200 Subject: [PATCH 20/24] fix: track runtime classpath and fence context --- .../alipsa/gmd/core/GmdTemplateEngine.groovy | 2 + .../groovy/gmd/GmdTemplateEngineTest.groovy | 20 +++ gmd-gradle-plugin/release.md | 2 +- .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 6 - .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 37 +++--- .../gmd/gradle/GmdGradlePluginTest.groovy | 114 +++--------------- 6 files changed, 56 insertions(+), 125 deletions(-) diff --git a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy index 02cee9a..f458f3d 100644 --- a/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy +++ b/gmd-core/src/main/groovy/se/alipsa/gmd/core/GmdTemplateEngine.groovy @@ -147,6 +147,8 @@ class GmdTemplateEngine { if (plainFenceChar != null || startsPlainFence) { inIndentedCode = false previousLineWasParagraph = false + previousLineWasBlockQuote = false + listContentColumn = -1 } else if (line.isBlank()) { previousLineWasParagraph = false previousLineWasBlockQuote = false diff --git a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy index b8e69c1..5b8489e 100644 --- a/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy +++ b/gmd-core/src/test/groovy/test/alipsa/groovy/gmd/GmdTemplateEngineTest.groovy @@ -400,6 +400,26 @@ x = 5 assertFalse(processed.contains('literal 5'), processed) } + @Test + void aPlainFenceResetsBlockQuoteAndListContext() { + String text = ''' +```{groovy echo=false} +x = 5 +``` +> - item +```text +plain +``` +> +> literal `= x ` here +''' + + String processed = GmdTemplateEngine.processCodeBlocks(text) + + assertTrue(processed.contains('literal `= x ` here'), processed) + assertFalse(processed.contains('literal 5 here'), processed) + } + @Test void anEmptyEchoFalseBlockPreservesThePreviousParagraph() { String text = ''' diff --git a/gmd-gradle-plugin/release.md b/gmd-gradle-plugin/release.md index 431970c..b34fe57 100644 --- a/gmd-gradle-plugin/release.md +++ b/gmd-gradle-plugin/release.md @@ -13,7 +13,7 @@ - validate the Gradle Plugin Portal publication before uploading the plugin - upgrade groovy to 5.1.3 - clean stale generated files only from the dedicated default `build/gmd` output directory; custom target directories retain pre-existing and orphaned GMD output when sources are deleted, renamed, or change output type, while Gradle tracks their expected generated files individually for up-to-date checks -- add a required `classpathIdentity` `@Input` to `ProcessGmdTask` so a directly registered task's declared runtime selection invalidates up-to-date checks; `GmdGradlePlugin` derives it from its dependency versions, direct registrants must wire it from their own configuration or the build fails validation, and blank values are rejected when the task processes GMD files +- track the actual GMD processor runtime classpath for up-to-date checks, including changed dynamic or transitive dependencies; avoid resolving it when no `.gmd` source exists so the no-op and stale-output cleanup paths remain dependency-free ## v3.1.1, in progress - replace the deprecated `Project.getProperties()` calls used by signing configuration with `findProperty`, keeping the plugin compatible with Gradle 10 diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index efadf15..322fb6c 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -59,12 +59,6 @@ class GmdGradlePlugin implements Plugin { task.targetDir.set(resolvedTargetDir) task.outputType.set(outputType) task.classpath.from(configuration) - // The classpath itself is @Internal (resolving it during input - // snapshotting would defeat the deferred no-op check), so this - // declared dependency identity is what tracks the runtime selection - // for up-to-date checks. - task.classpathIdentity.set( - "groovy=${groovyVersion}, gmd=${gmdVersion}, ivy=${ivyVersion}, log4j=${log4jVersion}".toString()) task.targetDirIsDefaultGmdOutput.set(targetDirIsDefaultGmdOutput) if (targetDirIsDefaultGmdOutput) { task.dedicatedOutputDir.set(resolvedTargetDir) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index ce78101..ba062c5 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -5,6 +5,8 @@ import groovy.transform.PackageScope import org.gradle.api.DefaultTask import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.DirectoryProperty +import org.gradle.api.file.FileCollection +import org.gradle.api.tasks.Classpath import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputDirectory import org.gradle.api.tasks.Optional @@ -110,31 +112,23 @@ abstract class ProcessGmdTask extends DefaultTask { @Input abstract org.gradle.api.provider.Property getOutputType() - // Resolving this detached configuration is deferred until process() knows - // there is a .gmd file to process. @Classpath would resolve it while Gradle - // snapshots task inputs, before that no-op check can run. + // Used only by the task action. getTrackedClasspath() exposes this runtime + // to Gradle's input snapshotter when there is a .gmd file to process. @org.gradle.api.tasks.Internal abstract ConfigurableFileCollection getClasspath() /** - * Stable identity for the runtime classpath selection used to fork - * GmdProcessor. The classpath itself is {@code @Internal} (see above), so - * this property invalidates up-to-date checks when that selection changes. - * GmdGradlePlugin derives it from its declared dependency versions. Anyone - * registering this task directly must wire it from their configuration - * without resolving it during configuration, for example - * {@code cfg.allDependencies.collect { "$it.group:$it.name:$it.version" }.sort().join(',')}. - * It deliberately has no convention: an unwired value fails validation - * loudly instead of letting a classpath swap pass up-to-date checks silently. - * This is an identity of declared runtime inputs, not a fingerprint of the - * fully resolved classpath. Therefore, a changed transitive resolution can - * leave the task up-to-date when its declared dependencies are unchanged. - * The example updates automatically as declared dependencies change; - * registrants using a hard-coded identity must keep it in sync with their - * runtime selection. + * The actual runtime classpath used to fork GmdProcessor. When there is no + * .gmd source, returns an empty collection so Gradle can run the no-op and + * stale-output cleanup paths without resolving detached dependencies. */ - @Input - abstract org.gradle.api.provider.Property getClasspathIdentity() + @Classpath + FileCollection getTrackedClasspath() { + if (!getSourceDir().isPresent() || gmdFilesIn(getSourceDir().get().asFile).length == 0) { + return project.files() + } + return getClasspath() + } @Input abstract org.gradle.api.provider.Property getTargetDirIsDefaultGmdOutput() @@ -160,9 +154,6 @@ abstract class ProcessGmdTask extends DefaultTask { logger.quiet("No gmd files found in ${source.canonicalPath}, nothing to do") return } - if (getClasspathIdentity().get().trim().isEmpty()) { - throw new IllegalArgumentException('classpathIdentity must not be blank') - } if (!target.exists()) { if (!target.mkdirs() && !target.isDirectory()) { throw new IllegalArgumentException("Could not create target directory ${target.canonicalPath}") diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 125ab1f..e04ec01 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -82,28 +82,22 @@ class GmdGradlePluginTest { } @Test - void classpathIdentityIsTheOnlyRuntimeSelectionInput() { + void runtimeClasspathIsTrackedAsAClasspathInput() { def project = ProjectBuilder.builder().build() ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() - task.classpathIdentity.set('declared-runtime-dependencies') - - Set inputs = task.inputs.properties.keySet() - Assertions.assertTrue(inputs.contains('classpathIdentity')) - Assertions.assertTrue(inputs.intersect( - ['groovyVersion', 'log4jVersion', 'gmdVersion', 'ivyVersion'] as Set).isEmpty(), - 'classpathIdentity must be the sole runtime-selection task input') - } - - @Test - void classpathIdentityHasNoConventionForDirectRegistration() { - // classpathIdentity is deliberately required with no convention: a direct - // registrant who does not wire it must fail validation loudly rather than - // have a classpath swap pass up-to-date checks silently. - def project = ProjectBuilder.builder().build() - ProcessGmdTask task = project.tasks.register('siteGmd', ProcessGmdTask).get() - - Assertions.assertFalse(task.classpathIdentity.isPresent(), - 'classpathIdentity must stay unwired so Gradle reports a missing value') + File sourceDir = new File(project.projectDir, 'src/gmd') + sourceDir.mkdirs() + new File(sourceDir, 'test.gmd').text = '# Greetings\n' + File runtimeJar = new File(project.buildDir, 'runtime.jar') + runtimeJar.parentFile.mkdirs() + runtimeJar.createNewFile() + task.sourceDir.set(sourceDir) + task.classpath.from(runtimeJar) + + Assertions.assertTrue(task.inputs.files.files.contains(runtimeJar), + 'The resolved processor runtime must participate in up-to-date checks') + Assertions.assertFalse(task.inputs.properties.containsKey('classpathIdentity'), + 'A declared-version identity must not replace the actual runtime classpath') } @Test @@ -295,75 +289,10 @@ class GmdGradlePluginTest { } @Test - void directlyRegisteredTaskWithoutUsableClasspathIdentityFailsLoudly() { - // classpathIdentity is the required task input; leaving it unwired must - // fail validation, while setting it blank must fail in the task action. - File testProjectDir = new File('build/gmdDirectTaskMissingIdentityTest') - try { - new AntBuilder().delete(dir: testProjectDir, failonerror: false) - File srcDir = new File(testProjectDir, 'src/test/gmd') - srcDir.mkdirs() - new File(srcDir, 'test.gmd').text = '# Greetings\n' - // Own settings file: the test project lives under this build's directory, - // so without one Gradle would adopt the plugin build's settings.gradle. - new File(testProjectDir, 'settings.gradle').text = "rootProject.name = 'gmd-direct-task-missing-identity'\n" - File buildFile = new File(testProjectDir, 'build.gradle') - String buildScript = ''' - import se.alipsa.gmd.gradle.ProcessGmdTask - - plugins { - id('base') - // Applied for its classes only; the task below is registered directly, - // bypassing GmdGradlePlugin's processGmd wiring. - id 'se.alipsa.gmd.gmd-gradle-plugin' - } - - repositories { mavenCentral() } - - def gmdRuntime = configurations.detachedConfiguration( - dependencies.create('se.alipsa.gmd:gmd-core:3.1.0') - ) - - tasks.register('directGmd', ProcessGmdTask) { - sourceDir = file('src/test/gmd') - targetDir = file('build/target') - outputType = 'html' - classpath.from(gmdRuntime) - } - '''.stripIndent() - buildFile.text = buildScript - - def result = GradleRunner.create() - .withProjectDir(testProjectDir) - .withArguments('directGmd') - .withPluginClasspath() - .forwardOutput() - .buildAndFail() - - Assertions.assertTrue(result.output.contains( - "property 'classpathIdentity' doesn't have a configured value"), - "An unwired classpathIdentity must fail loudly:\n${result.output}") - - buildFile.text = buildScript.replace( - 'classpath.from(gmdRuntime)', "classpath.from(gmdRuntime)\n classpathIdentity = ''") - def blankResult = GradleRunner.create() - .withProjectDir(testProjectDir) - .withArguments('directGmd') - .withPluginClasspath() - .forwardOutput() - .buildAndFail() - Assertions.assertTrue(blankResult.output.contains('classpathIdentity must not be blank'), - "A blank classpathIdentity must fail loudly:\n${blankResult.output}") - } finally { - new AntBuilder().delete(dir: testProjectDir, failonerror: false) - } - } - - @Test - void directlyRegisteredTaskInvalidatesWhenItsWiredClasspathIdentityChanges() { + void directlyRegisteredTaskInvalidatesWhenItsRuntimeClasspathChanges() { // End-to-end proof of the fix for silent stale up-to-date checks on a - // directly registered task: wiring classpathIdentity from declared runtime - // dependencies must invalidate the task when that selection changes. The swap + // directly registered task: its actual runtime classpath must invalidate + // the task when a dependency changes. The swap // changes only log4j-core so the runnable gmd-core:3.1.0 stays in place // (pre-3.1.0 gmd-core releases carry a JavaFX dependency graph). File testProjectDir = new File('build/gmdDirectTaskIdentityTest') @@ -405,11 +334,6 @@ class GmdGradlePluginTest { targetDir = file('build/target') outputType = 'html' classpath.from(gmdRuntime) - // The supported direct-registration contract: derive the identity from - // declared dependencies, without resolving the configuration eagerly. - classpathIdentity = gmdRuntime.allDependencies.collect { - "${it.group}:${it.name}:${it.version}" - }.sort().join(',') } '''.stripIndent() @@ -535,8 +459,8 @@ class GmdGradlePluginTest { Assertions.assertEquals(org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE, cachedResult.task(":processGmd").outcome, 'A custom target must retain Gradle up-to-date checks') - // This proves end-to-end that classpathIdentity invalidates the task when - // the plugin changes a declared runtime dependency version. + // This proves end-to-end that the resolved classpath invalidates the task + // when the plugin changes a runtime dependency version. buildFile.text = buildFile.text.replace("log4jVersion = '2.26.1'", "log4jVersion = '2.25.1'") def changedVersionResult = GradleRunner.create() .withProjectDir(testProjectDir) From cfc650e2b8277674d2456fdf34555acbb4839bf1 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 22:00:19 +0200 Subject: [PATCH 21/24] fix: defer Gradle runtime resolution in config cache --- .../alipsa/gmd/gradle/GmdGradlePlugin.groovy | 20 +++++++++++++-- .../alipsa/gmd/gradle/ProcessGmdTask.groovy | 21 ++++------------ .../gmd/gradle/GmdGradlePluginTest.groovy | 25 +++++++++++++------ 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy index 322fb6c..594c5de 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/GmdGradlePlugin.groovy @@ -17,6 +17,7 @@ import org.gradle.api.tasks.TaskProvider class GmdGradlePlugin implements Plugin { private static final List MAVEN_CENTRAL_HOSTS = ['repo.maven.apache.org', 'repo1.maven.org'] + private static final String GMD_PROCESSOR_RUNTIME_CONFIGURATION = 'gmdProcessorRuntime' @Override void apply(Project project) { @@ -58,7 +59,10 @@ class GmdGradlePlugin implements Plugin { task.sourceDir.set(project.file(sourceDir)) task.targetDir.set(resolvedTargetDir) task.outputType.set(outputType) - task.classpath.from(configuration) + File configuredSourceDir = project.file(sourceDir) + task.runtimeClasspath.from(project.providers.provider { + hasGmdFiles(configuredSourceDir) ? configuration : project.files() + }) task.targetDirIsDefaultGmdOutput.set(targetDirIsDefaultGmdOutput) if (targetDirIsDefaultGmdOutput) { task.dedicatedOutputDir.set(resolvedTargetDir) @@ -97,7 +101,12 @@ class GmdGradlePlugin implements Plugin { project.dependencies.create("se.alipsa.gmd:gmd-core:${gmdVersion}") ] - return project.configurations.detachedConfiguration(dependencies.toArray(new Dependency[0])) + Configuration configuration = project.configurations.maybeCreate(GMD_PROCESSOR_RUNTIME_CONFIGURATION) + configuration.canBeConsumed = false + configuration.canBeResolved = true + configuration.visible = false + configuration.dependencies.addAll(dependencies) + return configuration } static boolean hasMavenCentral(Project project) { @@ -107,6 +116,13 @@ class GmdGradlePlugin implements Plugin { } } + private static boolean hasGmdFiles(File directory) { + File[] files = directory.listFiles({ File file -> + file.isFile() && file.name.endsWith('.gmd') + } as FileFilter) + return files != null && files.length > 0 + } + /** * Under {@code PREFER_SETTINGS}, Gradle does not reject a project-declared repository * with {@link InvalidUserCodeException} the way it does under {@code FAIL_ON_PROJECT_REPOS}; diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index ba062c5..c16edb8 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -3,9 +3,8 @@ package se.alipsa.gmd.gradle import groovy.transform.CompileStatic import groovy.transform.PackageScope import org.gradle.api.DefaultTask -import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.DirectoryProperty -import org.gradle.api.file.FileCollection +import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.tasks.Classpath import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputDirectory @@ -112,23 +111,13 @@ abstract class ProcessGmdTask extends DefaultTask { @Input abstract org.gradle.api.provider.Property getOutputType() - // Used only by the task action. getTrackedClasspath() exposes this runtime - // to Gradle's input snapshotter when there is a .gmd file to process. - @org.gradle.api.tasks.Internal - abstract ConfigurableFileCollection getClasspath() - /** * The actual runtime classpath used to fork GmdProcessor. When there is no - * .gmd source, returns an empty collection so Gradle can run the no-op and - * stale-output cleanup paths without resolving detached dependencies. + * .gmd source, its provider supplies an empty collection so Gradle can run the + * no-op and stale-output cleanup paths without resolving processor dependencies. */ @Classpath - FileCollection getTrackedClasspath() { - if (!getSourceDir().isPresent() || gmdFilesIn(getSourceDir().get().asFile).length == 0) { - return project.files() - } - return getClasspath() - } + abstract ConfigurableFileCollection getRuntimeClasspath() @Input abstract org.gradle.api.provider.Property getTargetDirIsDefaultGmdOutput() @@ -170,7 +159,7 @@ abstract class ProcessGmdTask extends DefaultTask { } def result = execOperations.javaexec { JavaExecSpec spec -> - spec.classpath = getClasspath() + spec.classpath = getRuntimeClasspath() spec.mainClass.set('se.alipsa.gmd.core.GmdProcessor') spec.args = [source.canonicalPath, target.canonicalPath, output] } diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index e04ec01..6d72ee5 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -92,7 +92,7 @@ class GmdGradlePluginTest { runtimeJar.parentFile.mkdirs() runtimeJar.createNewFile() task.sourceDir.set(sourceDir) - task.classpath.from(runtimeJar) + task.runtimeClasspath.from(runtimeJar) Assertions.assertTrue(task.inputs.files.files.contains(runtimeJar), 'The resolved processor runtime must participate in up-to-date checks') @@ -237,7 +237,7 @@ class GmdGradlePluginTest { Assertions.assertEquals(SUCCESS, result.task(':processGmd').outcome, "PREFER_SETTINGS must not break the plugin:\n${result.output}") Assertions.assertTrue(new File(testProjectDir, 'build/target/test.html').exists(), - 'The detached configuration must still resolve through the settings repositories') + 'The processor runtime must still resolve through the settings repositories') Assertions.assertTrue(result.output.contains('repoCount=0'), "The plugin must not add to project.repositories under PREFER_SETTINGS:\n${result.output}") Assertions.assertFalse( @@ -249,7 +249,7 @@ class GmdGradlePluginTest { } @Test - void aSourceDirWithoutGmdFilesReportsNothingToDo() { + void aSourceDirWithoutGmdFilesDoesNotResolveTheRuntime() { File testProjectDir = new File('build/gmdNoGmdFilesTest') try { new AntBuilder().delete(dir: testProjectDir, failonerror: false) @@ -277,8 +277,16 @@ class GmdGradlePluginTest { } '''.stripIndent() + def helpResult = GradleRunner.create().withProjectDir(testProjectDir) + .withArguments('help', '--configuration-cache', '--offline') + .withPluginClasspath().forwardOutput().build() + + Assertions.assertEquals(SUCCESS, helpResult.task(':help').outcome, + "Configuration-cache storage must not resolve the unavailable runtime:\n${helpResult.output}") + def result = GradleRunner.create().withProjectDir(testProjectDir) - .withArguments('processGmd').withPluginClasspath().forwardOutput().build() + .withArguments('processGmd', '--configuration-cache', '--offline') + .withPluginClasspath().forwardOutput().build() Assertions.assertTrue(result.output.contains('No gmd files found in'), result.output) Assertions.assertFalse(result.output.contains('Gmd files processed and written to'), result.output) @@ -320,20 +328,23 @@ class GmdGradlePluginTest { def log4jVersion = '2.26.1' // Mirror the runtime GmdGradlePlugin assembles: gmd-core's published POM // does not bring Groovy transitively. - def gmdRuntime = configurations.detachedConfiguration( + def gmdRuntime = configurations.create('directGmdRuntime') + gmdRuntime.canBeConsumed = false + gmdRuntime.canBeResolved = true + gmdRuntime.dependencies.addAll([ dependencies.create('se.alipsa.gmd:gmd-core:3.1.0'), dependencies.create('org.apache.groovy:groovy:5.1.3'), dependencies.create('org.apache.groovy:groovy-templates:5.1.3'), dependencies.create('org.apache.groovy:groovy-jsr223:5.1.3'), dependencies.create('org.apache.ivy:ivy:2.6.0'), dependencies.create("org.apache.logging.log4j:log4j-core:${log4jVersion}") - ) + ]) tasks.register('directGmd', ProcessGmdTask) { sourceDir = file('src/test/gmd') targetDir = file('build/target') outputType = 'html' - classpath.from(gmdRuntime) + runtimeClasspath.from(gmdRuntime) } '''.stripIndent() From 70dffdbf7bf3b550f6eaf2301339807239d778b0 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 22:12:50 +0200 Subject: [PATCH 22/24] fix: retain legacy Gradle classpath property --- .../groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy | 10 ++++++++++ .../test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy | 6 +++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy index c16edb8..ccfa510 100644 --- a/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy +++ b/gmd-gradle-plugin/src/main/groovy/se/alipsa/gmd/gradle/ProcessGmdTask.groovy @@ -119,6 +119,16 @@ abstract class ProcessGmdTask extends DefaultTask { @Classpath abstract ConfigurableFileCollection getRuntimeClasspath() + /** + * @deprecated Use {@link #getRuntimeClasspath()}. This alias returns the + * same collection for compatibility with directly registered tasks. + */ + @Deprecated + @org.gradle.api.tasks.Internal + ConfigurableFileCollection getClasspath() { + return getRuntimeClasspath() + } + @Input abstract org.gradle.api.provider.Property getTargetDirIsDefaultGmdOutput() diff --git a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy index 6d72ee5..b8b8af9 100644 --- a/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy +++ b/gmd-gradle-plugin/src/test/groovy/test/alipsa/gmd/gradle/GmdGradlePluginTest.groovy @@ -94,6 +94,8 @@ class GmdGradlePluginTest { task.sourceDir.set(sourceDir) task.runtimeClasspath.from(runtimeJar) + Assertions.assertSame(task.runtimeClasspath, task.classpath, + 'The deprecated classpath alias must delegate to runtimeClasspath') Assertions.assertTrue(task.inputs.files.files.contains(runtimeJar), 'The resolved processor runtime must participate in up-to-date checks') Assertions.assertFalse(task.inputs.properties.containsKey('classpathIdentity'), @@ -344,7 +346,9 @@ class GmdGradlePluginTest { sourceDir = file('src/test/gmd') targetDir = file('build/target') outputType = 'html' - runtimeClasspath.from(gmdRuntime) + // Legacy direct registrations used this property before + // runtimeClasspath was introduced. + classpath.from(gmdRuntime) } '''.stripIndent() From a2b9b831995ba3de05762335a05ee811e08b1f98 Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 23:02:15 +0200 Subject: [PATCH 23/24] docs: describe Gradle runtime configuration --- gmd-gradle-plugin/readme.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/gmd-gradle-plugin/readme.md b/gmd-gradle-plugin/readme.md index 7ba3462..9215169 100644 --- a/gmd-gradle-plugin/readme.md +++ b/gmd-gradle-plugin/readme.md @@ -34,8 +34,9 @@ embedded styles, and syntax-highlighted code blocks. ### Repositories -The plugin resolves `gmd-core` and Groovy through a detached configuration, which uses the -project's repositories. If the project has not declared Maven Central, the plugin adds it. +The plugin resolves `gmd-core` and Groovy through its named `gmdProcessorRuntime` project +configuration, which uses the project's repositories. If the project has not declared Maven +Central, the plugin adds it. If it has — under either `repo.maven.apache.org` or `repo1.maven.org` — nothing is added. A project that resolves only through a private mirror should declare that mirror in `dependencyResolutionManagement` in `settings.gradle` with `repositoriesMode` set to From 688b4bde00dace12e681bd38578dca0fecaf34cf Mon Sep 17 00:00:00 2001 From: per Date: Thu, 24 Sep 2026 23:06:15 +0200 Subject: [PATCH 24/24] docs: align Maven GMD version example --- gmd-maven-plugin/readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gmd-maven-plugin/readme.md b/gmd-maven-plugin/readme.md index 8ea1ecc..af739e8 100644 --- a/gmd-maven-plugin/readme.md +++ b/gmd-maven-plugin/readme.md @@ -82,7 +82,7 @@ You can override the versions of dependencies used by the plugin by specifying t html 5.1.3 2.26.1 - 3.1.0 + 3.2.0