...yourself become the cheeseburger #724
No reviewers
Labels
No labels
accepted
bug
dependencies
docker
documentation
duplicate
enhancement
github-actions
github_actions
good first issue
gradle
hacktoberfest-accepted
help wanted
invalid
javascript
not-an-issue
npm
question
rejected
wontfix
Working on it
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
Griefed/ServerPackCreator!724
Loading…
Reference in a new issue
No description provided.
Delete branch "develop"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
latestto a digest, freezing every copy of itThe file called "dev" was the slowest way to run this project. It built `docker/Dockerfile`, whose builder stage runs a full Gradle build inside the container with no cache mount, so a one-line change cost a complete from-scratch build with no debugger, no hot-swap and no code sync. And it never published MongoDB's port, so the dev loop this repository already has -- a Spring Boot run configuration with `UpdateClassesAndResources`, and `spring-boot-devtools` already a `developmentOnly` dependency -- could not reach the database at all. The two halves were never connected. It is now dependencies-only by default: `up -d` gives MongoDB with 27017 published plus mongo-express, and the application runs from the IDE. The image build survives behind `profiles: ["app"]`, because it is the only thing that exercises the s6 init script and the `overrides.properties` it composes. Also in the file, each its own small defect: - `version: '3'` is obsolete; `docker compose config` warned on every invocation. - Both compose files use the same service names, so they fought over the same containers. A `name: spc-dev` project and no `container_name:` separates them. - `restart: unless-stopped` resurrected the whole dev stack on every Docker Desktop start. - `depends_on` only orders starts. A `mongosh` healthcheck plus `condition: service_healthy` closes the race that `serverpackcreator-app/CLAUDE.md` records as "the normal first boot". - The database volumes were commented out, so the dev database died with the stack. Named volumes now, and the file says `down -v` is the reset, because `init-mongo.js` only ever runs against an empty /data/db. - mongo-express spent ten retries dialling a host named `mongo` on every start, because its entrypoint parses `ME_CONFIG_MONGODB_URL` -- which was unset -- to decide what to wait for, while the app read the SERVER/PORT/ADMINUSERNAME/ADMINPASSWORD quartet and worked anyway. One URL replaces the four, and the retry loop is gone (verified: 0 retry lines). Digests unpinned, finishing what the deployment compose file started: mongo-express `@sha256:1b23d797...` -> `1.0.2-20-alpine3.19` (that digest is both `latest` and `1.0.2`, May 2024 on Node 18 / alpine 3.18; this is the November 2024 rebuild of the same release, which is still the newest one). MongoDB tracks the deployment file at 8.0.34 rather than lagging it, which on this host means it does not start at all: kernels 6.19.0-7.0.13 hit SERVER-121912 and the image refuses, by design, and Docker Desktop's linuxkit kernel is inside that range. The tag is therefore overridable from an untracked `docker/.env` via `SPC_DEV_MONGO_IMAGE`, so the version stays correct in git while remaining runnable per machine. `.dockerignore` is in this commit because `--profile app` still builds with `context: ..`, and that context was 1025 MB of which almost none was an input. Measured by `du -sm` on the extracted context, identically before and after: **1025 MB -> 112 MB**. The big three were `media/` (789 MB of gitignored install4j output that nothing excluded), `serverpackcreator-homepage/` (373 MB, which has its own context and which `docker/Dockerfile` never touches) and `.git` (119 MB; nothing in the build reads it -- the Dockerfile passes `-Pversion` explicitly). The patterns are `**/`-prefixed now because a single-level `*/build` matched `serverpackcreator-api/build` but not `buildSrc/build`. Verified: `docker compose config --quiet` is silent; the default `up` starts exactly the two dependency services and `--profile app` adds the third; the healthcheck reaches `healthy` and mongo-express waits for it; `mongosh "mongodb://root:1234567890@127.0.0.1:27017/serverpackcreatordb"` from the host returns the database, which is the URI the run configuration will carry; mongo-express answers 401 without and 200 with `mexpress`/`mexpress` and lists the database. The context was checked file-by-file inside an image rather than by reading the ignore file: every input Gradle needs is present (wrapper, catalog, both settings files, all eight module `src/main` trees, buildSrc's Kotlin, `docker/root`, and the frontend's 50 source and 17 test files) and all five excluded trees are genuinely absent. NOT verified, and pre-existing: `--profile app` cannot complete on this machine. The Gradle daemon inside the container is OOM-killed at Docker Desktop's 1.92 GB limit, which has nothing to do with this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>`.runConfigurations/` is the legacy location; modern IntelliJ reads `.run/`, and the split was already visible in the tree -- twenty-one files in the dead directory and one (`Homepage build`) in the live one, added last week. Every one of those twenty-one was invisible in the IDE. Moved with `git mv` so the history follows, which is also what makes the three repairs below legible as repairs rather than as new files. Fixed while moving, each a real defect rather than a tidy-up: - `Debug Fat Jar` had `WORKING_DIRECTORY` of `F:\GitLab\ServerPackCreator\serverpackcreator-app\tests` -- an absolute path from a Windows checkout that cannot resolve anywhere else. Now `$PROJECT_DIR$/serverpackcreator-app/tests`, matching every other configuration here. Its `JAR_PATH` was already correct: `serverpackcreator-app-dev.jar` is what the build produces. - `Test API` ran `:serverpackcreator-api:clean` and `:serverpackcreator-app:test` -- it cleaned one module and tested a different one, so it never once tested the API. Now `:serverpackcreator-api:test`, verified to resolve. - `Build without tests with version` hardcoded `-Pversion=6.3.4`. The project is at 9.3.0 and the value was three majors stale, so the configuration is deleted rather than re-pinned to a number that will go stale again; `Build without tests` plus an explicit `-Pversion` covers the case. Added two, which is the point of the preceding commit: - `Dev stack` runs `docker/docker-compose-dev.yml` from the Services tool window. - `WebService (dev stack)` is the Spring Boot configuration for the fast loop. It carries `--spring.mongodb.uri=...` as a program argument rather than relying on the home directory, because `serverpackcreator-app/tests/serverpackcreator.properties` is generated and untracked and the copy the application writes itself carries no credentials, while the dev database requires them. A comment in the file records that the key is `spring.mongodb.uri` -- `spring.data.mongodb.uri` is retired at error level in Spring Boot 4 and binds nothing -- and that `mongodb://localhost/test` in the log is the symptom of getting it wrong. `misc/Java-References.md` cites `Debug Fat Jar.run.xml` by path for its `ALTERNATIVE_JRE_PATH`, so that link moves with the file; the two remaining repository matches for "runConfigurations" are an unrelated field name in the generated `api-docs.yaml`. The `.dockerignore` entry was swapped from the old directory to the new one in the preceding commit. Verified: all 23 configurations parse as XML, every `name` attribute matches its filename, and no absolute or host-specific path remains anywhere under `.run/`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>.forgejo/from the build context broke the image buildAdds the *Package structure* rule to the root CLAUDE.md, between *Module boundaries* and *Refactor discipline* — it is an architecture rule, and those are its neighbours. Griefed's wording, unabridged. The clause that does the most work in practice is the last pair: use shared packages only for genuinely cross-cutting concerns, and do not impose rigid package structures without a concrete benefit. Both directions are load-bearing, and the audit below leaned on the second more often than on the first. Also records the audit of the five modules the rule was adopted for, in each module's own CLAUDE.md, so the next session reads a verdict instead of re-deriving it: -clientside VIOLATION 37 files in one flat package; split separately -plugin-servertest VIOLATION `core` held two features; split separately -grinder COMPLIANT four feature subpackages; the three base-package residents that look like strays each have a reason, and moving ContainerCandidateVerifier into `container` would make container -> loader -> container -plugin-grinder COMPLIANT one feature; core -> nothing, gui -> core, root -> both, so core/gui is the within-feature domain/ui split the rule sanctions -plugin-selfextract COMPLIANT five files, one feature, nothing to split The two COMPLIANT plugin verdicts are the interesting ones, because `core`/`gui` reads as layering at a glance and is not: the question the rule actually asks is whether the bucket holds one feature or several. -plugin-servertest's `core` held two that never referenced each other; -plugin-grinder's holds one. Each note says how to re-derive its edges rather than asking a reader to trust the sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>All 37 production files sat in `de.griefed.serverpackcreator.clientside`, which is the "organize by global technical layer" case the root CLAUDE.md's new *Package structure* rule exists to prevent — there was no layer and no feature, just a bag. They are now seven feature packages plus the module's API at the root: (root) ClientsideVerifier, ClientsideReportRenderer, ClientsideListEditor boot stage a pack, run a server, keep the evidence 8 files verdict evidence -> conclusion 5 platform Modrinth/CurseForge, the wire, the shared model 6 metadata what a jar says about itself 6 selection which builds get ground 3 dependency attribution and backtracking 2 version version/loader arithmetic, the one shared concern 4 The split was derived from the reference graph, not from names, and the result is acyclic — root -> all, boot -> everything below it, verdict -> platform, selection -> {platform, version}, platform -> metadata -> version, dependency -> version. Acyclicity is the property worth preserving; the layout table in the module's CLAUDE.md records it along with how to re-derive it. Behaviour-preserving. No assertion, expected value or fixture changed. Of 222 changed Kotlin files, 216 differ only in `package`/`import` lines; the other six are reference-only edits of the kind the conventions' `refactor:` carve-out describes — five fully-qualified names (four inside KDoc) and one baked-in source path in `BootVerifierArtifactSinkTest`, which greps BootVerifier's own source and was the single red in the first full run. TWO GUARDS WERE SILENTLY WEAKENED BY THE MOVE, and that is the part worth reading. `JarDownloaderRoutingTest` asserts two things must NOT exist, by `Class.forName` on a hardcoded name: the removed headless-browser `BrowserDownloader`, and the `JarDownloaderKt` facade Kotlin would emit if a top-level routing helper came back. Moving the file left both strings naming the old flat package, so each went on asking about a package the class could never appear in — still green, and green for ever. A "must not exist" guard cannot fail when it is weakened, which is why nothing caught it. Both now name `.platform.`, and both were re-proven red against the thing they exist to catch: adding a top-level `fun` to JarDownloader.kt fails `noRoutingHelperSurvives`, and a stub `class BrowserDownloader` fails `aLockedFileIsRecognisedAndHasNoBrowserFallback`. They were missed on the first pass because the sweep for reflective lookups filtered out `/src/test/` — reflection in a *test* is exactly where a package move does its quiet damage, since production code would not compile. `module.md` is part of this change rather than a follow-up, because nothing shipped in between and it was wrong in the same way the code was. Dokka reads it, and it named the pre-split packages: 14 `[X][de.griefed...clientside.X]` links no longer resolved, and the single `# Package` section still described the whole engine while documenting only the three files left at the root. It now carries one section per feature package. `serverpackcreator-grinder/module.md` referenced `clientside.ServerRunner` too; that one produces no Dokka warning at all, because a cross-module link is not on the other module's doc classpath — tool silence was not evidence, and the repo-wide grep is what found it. All five audited modules now report 0 unresolved links, clientside down from 14. 11 production imports were added purely so a KDoc `[Link]` still resolves across the new seams (13 counting two in tests). They create no bytecode dependency, so the compile-level graph stays acyclic, but a tool reading imports alone will report edges that are not real. Tests mirror their subject's package only where the subject is unambiguous: 42 of 89 moved on a name match against a production file or symbol, and the other 47 are behaviour tests spanning several features, which stay beside the entry point they drive. Green: :serverpackcreator-clientside:test 680, plus :serverpackcreator-grinder:test and :serverpackcreator-app:test, which consume the moved symbols (61 consumer files re-pointed, including four wildcard imports expanded to the symbols each file actually uses). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>corelayer bucket into thepackandsessionfeatures 1c06536e72`core` was named for a layer — its own CLAUDE.md described it as "everything testable without Swing" — which says what the eleven files are *not* rather than what any of them is for. That is the shape the root CLAUDE.md's new *Package structure* rule rejects. The two features were already there and already separable: pack what there is to launch: ServerPackCatalog, StartScriptSelector, ServerPropertiesPatch, PackVariables, GenerationNotifier session running one: PortAllocator, ServerLauncher, ServerSession, SessionState, SessionRegistry `ServerPackCatalog` and `StartScriptSelector` reference each other, so they stay together; `session` depends on `pack` and `pack` on nothing, so the result is acyclic. Verified by reading the imports after the move: pack -> (nothing), session -> pack, gui -> {pack, session, the root model}. `ServerTestSettings` went to the root with the plugin wiring rather than into either feature. It is a typed view over the CommentedConfig SPC owns — a model, not orchestration — and it is read by `session` (port range) and by `gui` (console scrollback) alike, so filing it under either would misstate its scope and a package holding one file would be structure without a benefit. This follows the rule -grinder's base package already documents: a subpackage may import the composition root's domain models, never its orchestration. Behaviour-preserving: package declarations and imports only, no assertion, argument or expected value touched. Tests moved with their subject; the three with no name match (ConcurrentAccessTest, RealPackBootTest, SessionLivenessTest) went to `session` on their reference counts. `module.md` is updated because Dokka reads its `# Package` headings by name and the `core` heading would have documented a package that no longer exists. Green: :serverpackcreator-plugin-servertest:test, 112 tests, 1 skipped — the documented baseline, the skip being the deliberate real boot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>textOrNulland are not duplicates 6b8ceab199The package split landed a landmine saying to collapse `platform.textOrNull(field)` and `BundledJars`' private `textOrNull()` into one function. That advice was wrong, and this commit replaces it with the evidence. The two ask different questions; merging them breaks in both directions, proven by mutation rather than argued: - Narrowing `platform.textOrNull(field)` to `if (isTextual) ... else null` fails FIVE tests — CurseForgeDependencyLineTest (x2), LinkedDependencyTest, PinnedDependencyRefTest and CurseForgePlatformTest. It must render a present non-string, because CurseForge serves `modId` as a number and `12345` has to read as "12345". Its sibling two lines away reads `path("id").asLong()`, which is the tell. - Widening `BundledJars`' reader to that lenient form fails BundledDemandTest.aRangeThatIsNotAPlainStringYieldsNoOpinion. A Fabric/Quilt descriptor field that is not a string is not an id or a range. So the defect was never duplication — it was one identifier meaning two predicates, which is why importing the platform one into BundledJars compiled (they overloaded), sat unused, and was the only thing putting a metadata -> platform cycle into the split. The private one is now `nonBlankStringOrNull()`, which says what it does, and neither can be reached by accident. Both doc comments now name the other and say why they differ. Found while mutating it: the blank half of that reader was NOT pinned. Dropping `takeIf { it.isNotBlank() }` left all 680 tests green, so the guard existed only for the `isTextual` half. `aBlankRangeYieldsNoOpinion` covers it now, written before the check and observed red against exactly that mutation: expected: <[(other, null)]> but was: <[(other, )]> It matters because a blank string IS textual, so it passes the surviving half and hands VersionConstraint an empty pattern to satisfy instead of telling it there is nothing to. Behaviour-preserving: the rename touches a private member and its nine call sites in one file, no assertion or expected value changed, and the one added test asserts behaviour that was already correct. Green: :serverpackcreator-clientside:test 681, full ./gradlew build, and dokkaGeneratePublicationHtml with 0 unresolved links. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>