Skip to content

Commit 9125116

Browse files
drernieclaude
andauthored
Migrate to new Nextflow plugin build system (#323) (#324)
## Summary Migrates the build from the legacy `buildSrc` / `launch.sh` / `plugins/build.gradle` layout to the new `io.nextflow.nextflow-plugin` Gradle plugin, per the [Seqera migration guide](https://docs.seqera.io/nextflow/guides/migrate-plugin). This combines PR steps 1+2 from the issue breakdown — they're interdependent (deleting the old build files without a replacement breaks CI), so they're shipped together. Refs #323. ## What changed - **Deleted** `buildSrc/`, `launch.sh`, `plugins/build.gradle`, `plugins/nf-quilt/build.gradle`, `groovysh-task.gradle`, `gradle-groovysh-init.gradle`, and the now-redundant `plugins/nf-quilt/src/resources/META-INF/{MANIFEST.MF,extensions.idx}` (the Gradle plugin generates these). - **Moved** sources to the standard layout the new plugin expects: - `plugins/nf-quilt/src/main` → `src/main/groovy` - `plugins/nf-quilt/src/test` → `src/test/groovy` - `plugins/nf-quilt/src/resources` → `src/main/resources` - **`settings.gradle`** reduced to `rootProject.name = 'nf-quilt'`. - **New root `build.gradle`** applies `io.nextflow.nextflow-plugin` 1.0.0-beta.6, declares `quiltcore`, `jackson-databind`, `jackson-datatype-jsr310`, `commons-io` as runtime deps, and configures `nextflowPlugin {}` with `className = 'nextflow.quilt.QuiltPlugin'`, provider `Quilt Data`, and three extension points (`QuiltObserverFactory`, `QuiltPathFactory`, `QuiltPathSerializer`). Plugin version is inlined as `0.9.2` (unchanged). - **`Makefile`** rewritten around `assemble` / `installPlugin` / `releasePlugin`. The legacy `pkg-test`, `dyn-test`, `s3-*` integration targets are preserved but now use `nextflow` directly (the `launch.sh` shim is gone) and depend on `installPlugin` to seed `~/.nextflow/plugins/`. - **`.github/workflows/test.yml`** artifact paths updated from `plugins/nf-quilt/build/reports/` to `build/reports/`. - **Tests:** `QuiltSpecification` and `QuiltPkgTest` now read the generated `MANIFEST.MF` from `build/tmp/jar/` (where the new plugin emits it), with the legacy paths kept as fallbacks. The `test` task gets the standard `--add-opens` JVM args and `dependsOn 'jar'` so the manifest exists before tests run. - **`gradle.properties`** cleared (legacy Groovy/JDK toolchain keys are no longer needed — the Gradle plugin handles all of that). ## Deviations from the migration guide - **MANIFEST.MF retained as a build artifact only.** The guide assumes you'd just instantiate factories directly in tests. Our `QuiltSpecification` does a full `Plugins.init` round-trip, which needs `MANIFEST.MF` on disk. Rather than rewrite all 200+ tests, I pointed the `TestPluginDescriptorFinder` at the generated manifest and added `tasks.named('test') { dependsOn 'jar' }`. - **Plugin version inlined** in `build.gradle` rather than read from a now-deleted `MANIFEST.MF` source file. This matches the migration guide's example. - **JaCoCo coverage** is no longer wired up — the legacy `verifyCoverage` task is dropped from `make test`. The new Gradle plugin doesn't apply JaCoCo, and reapplying it cleanly is out of scope. Worth a follow-up if we want the 70% gate back. ## Test plan - [x] `./gradlew assemble` — succeeds (produces `build/libs/nf-quilt-0.9.2-meta.json` + `nf-quilt-0.9.2.zip`) - [x] `./gradlew test` — 226 tests, 222 passing, 14 skipped, 0 failing (locally on JDK 21) - [x] `make test` — green - [ ] CI matrix (ubuntu/macos/windows × JDK 17/19/21) — pending - [ ] Spot-check `make pkg-test WRITE_BUCKET=udp-spec` against real S3 (out of scope for this PR; the existing `nf-quilt/dest-0.9.2` package on `udp-spec` should still satisfy `QuiltPkgTest`) ## Deferred to subsequent PRs - Doc rewrites: `README.md`, `README-DEV.md`, `CHANGELOG.md`. This PR only touches the build layer; references to `plugins/nf-quilt/...` paths in docs were left alone where they aren't actively broken. - Bumping `Plugin-Version` to `1.0.0` and the actual registry release. - Reinstating JaCoCo coverage (if desired). - Validating `releasePlugin` against the registry (no `--dry-run` option exists; needs an API key). <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR migrates the nf-quilt plugin build from the legacy `buildSrc`/`launch.sh`/nested-`build.gradle` layout to the `io.nextflow.nextflow-plugin` Gradle plugin, moving sources to the standard flat layout the new plugin expects and updating the Makefile and CI artifact paths accordingly. - **P1 – `logback-test.xml` in `src/main/resources/`**: this file will be packaged into the production JAR and force `<root level="DEBUG">` logging for every user of the plugin; it must move to `src/test/resources/`. - **P1 – Broken `make coverage` and `make update` targets**: the `jacocoTestReport` and `dependencyUpdates` Gradle tasks referenced by these targets no longer exist in the new `build.gradle`, causing both targets to fail. - **P2 – `s3-test` target missing `-plugins` flag**: unlike all other integration targets, `s3-test` does not pass `-plugins $(PROJECT)@$(VERSION)`, so Nextflow will resolve from the registry instead of the locally installed build. <h3>Confidence Score: 3/5</h3> Not safe to merge as-is: the logback-test.xml placement will pollute production logging for all plugin users, and two Makefile targets will fail at runtime. Two P1s: (1) logback-test.xml in src/main/resources/ ships DEBUG root logging into the production JAR, and (2) `make coverage` and `make update` reference Gradle tasks removed from the new build, causing hard failures. The core Gradle migration and source-move appear correct, but these issues must be fixed before merging. src/main/resources/logback-test.xml (wrong directory — must be src/test/resources/); Makefile (coverage and update targets broken, s3-test missing -plugins flag) <h3>Important Files Changed</h3> | Filename | Overview | |----------|----------| | build.gradle | Root build.gradle rewritten to use io.nextflow.nextflow-plugin 1.0.0-beta.6; declares runtime deps and configures nextflowPlugin block cleanly | | Makefile | Rewritten around assemble/installPlugin/releasePlugin; three P1/P2 issues: `make coverage` calls a removed jacocoTestReport task, `make update` calls a removed dependencyUpdates task, and `s3-test` is missing the `-plugins` flag | | src/main/resources/logback-test.xml | P1: logback-test.xml placed in src/main/resources/ instead of src/test/resources/; will be packaged into the production JAR and force DEBUG root logging for all plugin users | | src/test/groovy/nextflow/quilt/QuiltSpecification.groovy | Base test spec updated to read MANIFEST.MF from build/tmp/jar/ with legacy fallback paths; correct use of TestPluginDescriptorFinder override | | src/test/groovy/nextflow/quilt/QuiltPkgTest.groovy | New test class with manifestVersion() helper; minor P2: FileInputStream not closed in try block | | settings.gradle | Simplified to single rootProject.name line; legacy multi-project include and foojay toolchain resolver removed | | .github/workflows/test.yml | Artifact upload paths updated from plugins/nf-quilt/build/reports/ to build/reports/ for both Windows and Linux/macOS runners | | gradle.properties | Legacy JVM toolchain keys (groovyVersion, jdkVersion, javaLangVersion) removed; now only a comment explaining the new plugin handles these | </details> <h3>Flowchart</h3> ```mermaid %%{init: {'theme': 'neutral'}}%% flowchart TD A[./gradlew assemble] --> B[build/libs/nf-quilt-0.9.2.zip] A --> C[build/tmp/jar/MANIFEST.MF] C --> D[QuiltSpecification.setupSpec\nreads MANIFEST.MF] D --> E[Plugins.init / startIfMissing] E --> F[Test suite runs\n226 tests] G[make install] --> H[./gradlew installPlugin] H --> I[~/.nextflow/plugins/nf-quilt-0.9.2/] I --> J[nextflow run ... -plugins nf-quilt@0.9.2] K[make release] --> L[./gradlew releasePlugin] L --> M[Seqera Plugin Registry] style C fill:#f9f,stroke:#c0c style D fill:#f9f,stroke:#c0c ``` <!-- greptile_failed_comments --> <details open><summary><h3>Comments Outside Diff (1)</h3></summary> 1. `src/main/resources/logback-test.xml`, line 1-31 ([link](https://github.com/quiltdata/nf-quilt/blob/2ad1400b7a52acc759cd005bb8df0a66066562f0/src/main/resources/logback-test.xml#L1-L31)) <a href="#"><img alt="P1" src="https://greptile-static-assets.s3.amazonaws.com/badges/p1.svg?v=7" align="top"></a> **Test-only logback config packaged into production JAR** `logback-test.xml` belongs in `src/test/resources/`, not `src/main/resources/`. By living in `src/main/resources/` it will be included in the production plugin ZIP/JAR. Logback resolves `logback-test.xml` over `logback.xml`, so every Nextflow user who loads this plugin will inherit the `<root level="DEBUG">` configuration, flooding their console with debug output from all loggers at runtime. </details> <!-- /greptile_failed_comments --> <sub>Reviews (1): Last reviewed commit: ["Migrate to new Nextflow plugin build sys..."](2ad1400) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=30247721)</sub> > Greptile also left **4 inline comments** on this PR. <!-- /greptile_comment --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent abfdd28 commit 9125116

49 files changed

Lines changed: 142 additions & 881 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/test.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,13 @@ jobs:
6060
with:
6161
name: nf-quilt-test-reports-${{ matrix.os }}-${{ matrix.java_version }}
6262
path: |
63-
D:\a\nf-quilt\nf-quilt\plugins\nf-quilt\build\reports\
63+
D:\a\nf-quilt\nf-quilt\build\reports\
6464
overwrite: true
6565
- name: Archive production artifacts (Linux and MacOS)
6666
uses: actions/upload-artifact@v4
6767
if: ${{ always() && matrix.os != 'windows-latest' }}
6868
with:
6969
name: nf-quilt-test-reports-${{ matrix.os }}-${{ matrix.java_version }}
7070
path: |
71-
${{ github.workspace }}/plugins/nf-quilt/build/reports/
71+
${{ github.workspace }}/build/reports/
7272
overwrite: true

Makefile

Lines changed: 53 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -2,152 +2,105 @@ sinclude .env # create from example.env
22
PROJECT ?= nf-quilt
33
WRITE_BUCKET ?= write-bucket-not-set
44
FRAGMENT ?= &path=.
5-
NF_DIR ?= ../nextflow
6-
NF_GIT ?= $(NF_DIR)/nextflow
7-
NF_BIN ?= ./launch.sh
8-
PID ?= $$$$
95
QUERY ?= ?Name=$(USER)&Owner=Kevin+Moore&Date=2023-03-07&Type=CRISPR&Notebook+URL=http%3A%2F%2Fexample.com
10-
VERSION ?= $(shell grep 'Plugin-Version' plugins/$(PROJECT)/src/resources/META-INF/MANIFEST.MF | awk '{ print $$2 }')
6+
VERSION ?= $(shell grep "^version" build.gradle | head -1 | awk -F"'" '{ print $$2 }')
117
NXF_VER ?= $(shell cat VERSION)
128
TEST_URI ?= quilt+s3://$(WRITE_BUCKET)$(QUERY)\#package=nf-quilt/dest-$(VERSION)$(FRAGMENT)
139
PIPELINE ?= sarek
14-
PIPE_OUT ?= quilt+s3://$(WRITE_BUCKET)\#package=$(PROJECT)/$(PIPELINE)
15-
NXF_PLUGINS_TEST_REPOSITORY ?= https://github.com/quiltdata/nf-quilt/releases/download/$(VERSION)/nf-quilt-$(VERSION)-meta.json
10+
PIPE_OUT ?= quilt+s3://$(WRITE_BUCKET)\#package=$(PROJECT)/$(PIPELINE)
1611
S3_BASE = s3://$(WRITE_BUCKET)/$(PROJECT)
17-
REPORT ?= ./plugins/$(PROJECT)/build/reports/tests/test/index.html
12+
REPORT ?= ./build/reports/tests/test/index.html
1813

19-
verify: #compile
14+
.PHONY: all assemble clean test test-all check rebuild install package release verify fast \
15+
check-env pkg-test dyn-test s3-overlay s3-test s3-in s3-out \
16+
pkg-fail path-input deps refresh
17+
18+
all: assemble
19+
20+
assemble:
21+
./gradlew assemble
22+
23+
clean:
24+
rm -rf .nextflow*
25+
rm -rf work results null
26+
rm -rf build
27+
./gradlew clean
28+
29+
test:
30+
./gradlew test
31+
32+
check:
33+
./gradlew check --warning-mode all
34+
35+
verify:
2036
echo $(WRITE_BUCKET)
2137
./gradlew test ${ONE} || open $(REPORT)
2238

2339
fast:
2440
./gradlew test ${ONE} --fail-fast || open $(REPORT)
2541
# example: make fast ONE="--tests QuiltProductTest"
42+
2643
check-env:
2744
echo $(VERSION)
2845
echo $(WRITE_BUCKET)
2946
echo "$(TEST_URI)"
30-
echo "Use 'make WRITE_BUCKET=<value>' to override"
47+
echo "Use 'make WRITE_BUCKET=<value>' to override"
3148
printenv MAKEFLAGS
3249

33-
clean:
34-
./gradlew clean
35-
rm -rf null results work
36-
rm -rf build */build */*/build plugins/nf-quilt/bin
37-
rm -f .nextflow.log* .launch*classpath
38-
39-
clean-all: clean
40-
rm -rf .gradle buildSrc/.gradle
41-
4250
rebuild:
4351
./gradlew clean build --refresh-dependencies
4452

45-
compile:
46-
./gradlew compileGroovy exportClasspath
47-
@echo "DONE `date`"
48-
49-
nextflow:
50-
if [ ! -d "$(NF_DIR)" ]; then git clone https://github.com/nextflow-io/nextflow.git "$(NF_DIR)"; fi
51-
cd "$(NF_DIR)"; git checkout && make compile && git restore .; cd ..
52-
53-
compile-all: nextflow compile
54-
55-
check:
56-
./gradlew check --warning-mode all
57-
58-
.PHONY: clean test test-all all pkg-test tower-test
59-
60-
test: clean compile check verifyCoverage
61-
62-
test-nextflow: clean nextflow-git compile check
63-
64-
test-all: clean compile-all check coverage
53+
test-all: clean test
6554

66-
coverage:
67-
./gradlew jacocoTestReport
68-
open plugins/nf-quilt/build/reports/jacoco/test/html/index.html
55+
install: assemble
56+
./gradlew installPlugin
6957

70-
verifyCoverage:
71-
./gradlew jacocoTestCoverageVerification
58+
package:
59+
./gradlew packagePlugin
7260

73-
groovysh:
74-
./gradlew -q --no-daemon --console=plain --init-script groovysh-task.gradle groovysh
61+
release:
62+
./gradlew releasePlugin
7563

7664
#
77-
# Create packages
65+
# Create packages (real-S3 integration tests).
66+
# These run the plugin via `nextflow` directly. Requires `nextflow` on PATH.
7867
#
7968

80-
pkg-test: compile #-all
69+
pkg-test: install
8170
echo "$(TEST_URI)"
82-
$(NF_BIN) run ./main.nf -profile standard -plugins $(PROJECT) --outdir "$(TEST_URI)"
71+
nextflow run ./main.nf -profile standard -plugins $(PROJECT)@$(VERSION) --outdir "$(TEST_URI)"
8372

84-
dyn-test: compile #-all
85-
$(NF_BIN) run wf/main.dynamic.nf -profile standard -plugins $(PROJECT)
73+
dyn-test: install
74+
nextflow run wf/main.dynamic.nf -profile standard -plugins $(PROJECT)@$(VERSION)
8675

87-
s3-overlay: compile
88-
$(NF_BIN) run ./main.nf --plugins $(PROJECT) --outdir "$(S3_BASE)/s3-overlay" --input "$(S3_BASE)/s3-in"
76+
s3-overlay: install
77+
nextflow run ./main.nf --plugins $(PROJECT)@$(VERSION) --outdir "$(S3_BASE)/s3-overlay" --input "$(S3_BASE)/s3-in"
8978

90-
s3-test: compile
91-
$(NF_BIN) run ./main.nf --outdir "$(S3_BASE)/s3-test" --input "$(S3_BASE)/s3-in"
79+
s3-test: install
80+
nextflow run ./main.nf -profile standard -plugins $(PROJECT)@$(VERSION) --outdir "$(S3_BASE)/s3-test" --input "$(S3_BASE)/s3-in"
9281

93-
s3-in: compile
94-
$(NF_BIN) run ./main.nf -profile standard -plugins $(PROJECT) --outdir "$(TEST_URI)" --input "$(S3_BASE)/s3-in"
82+
s3-in: install
83+
nextflow run ./main.nf -profile standard -plugins $(PROJECT)@$(VERSION) --outdir "$(TEST_URI)" --input "$(S3_BASE)/s3-in"
9584

96-
s3-out: compile
97-
$(NF_BIN) run ./main.nf -profile standard -plugins $(PROJECT) --outdir "$(S3_BASE)/s3-out"
85+
s3-out: install
86+
nextflow run ./main.nf -profile standard -plugins $(PROJECT)@$(VERSION) --outdir "$(S3_BASE)/s3-out"
9887

99-
pkg-fail: compile
88+
pkg-fail: install
10089
echo "$(TEST_URI)"
101-
$(NF_BIN) run wf/fail.nf -profile standard -plugins $(PROJECT) --outdir "$(TEST_URI)"
90+
nextflow run wf/fail.nf -profile standard -plugins $(PROJECT)@$(VERSION) --outdir "$(TEST_URI)"
10291

103-
path-input: compile
92+
path-input: install
10493
mkdir -p work
10594
date > work/COPY_THIS.md
10695
echo "$(TEST_URI)"
107-
$(NF_BIN) run wf/main.path.nf -profile standard -plugins $(PROJECT) --outdir "./results"
108-
109-
tower-test: $(NF_BIN)
110-
$(NF_BIN) run "https://github.com/quiltdata/nf-quilt" -name local_einstein -with-tower -r main -latest --pub "$(TEST_URI)"
111-
112-
#
113-
# Production Testing
114-
#
115-
116-
nf-git-ver: $(NF_GIT)
117-
NXF_VER=$(NXF_VER) $(NF_GIT) -v
118-
119-
120-
$(PIPELINE): nf-git-ver
121-
NXF_PLUGINS_TEST_REPOSITORY=$(NXF_PLUGINS_TEST_REPOSITORY) NXF_VER=$(NXF_VER) $(NF_GIT) run nf-core/$(PIPELINE) -r master -profile test,docker -plugins $(PROJECT)@$(VERSION) --outdir "$(PIPE_OUT)"
122-
123-
fetchngs: nf-git-ver
124-
NXF_PLUGINS_TEST_REPOSITORY=$(NXF_PLUGINS_TEST_REPOSITORY) NXF_VER=$(NXF_VER) $(NF_GIT) run nf-core/fetchngs -r master -profile test,docker -plugins $(PROJECT)@$(VERSION) --input ../nf-quilt/wf/ids.csv --outdir s3://$(WRITE_BUCKET)/nf-quilt/fetchngs
96+
nextflow run wf/main.path.nf -profile standard -plugins $(PROJECT)@$(VERSION) --outdir "./results"
12597

12698
#
12799
# Show dependencies
128100
#
129101

130102
deps:
131-
./gradlew -q ${mm}dependencies
132-
133-
update:
134-
./gradlew useLatestVersions
135-
make check
103+
./gradlew -q dependencies
136104

137105
refresh:
138106
./gradlew --refresh-dependencies dependencies
139-
140-
install: compile
141-
./gradlew copyPluginZip
142-
rm -rf ${HOME}/.nextflow/plugins/$(PROJECT)-${VERSION}
143-
cp -r build/plugins/$(PROJECT)-${VERSION} ${HOME}/.nextflow/plugins/
144-
145-
#
146-
# Upload JAR artifacts to Maven Central
147-
#
148-
149-
publish:
150-
echo "Ensure you have set 'github_organization=<owner>' in ~/.gradle/gradle.properties"
151-
ls $(HOME)/.gradle/gradle.properties # create locally or globally if it does not exist
152-
./gradlew :plugins:$(PROJECT):upload
153-
./gradlew :plugins:publishIndex

0 commit comments

Comments
 (0)