SLING-13253 - update-local-site: fix problems found running the website update - #50
Merged
Merged
Conversation
…te update Three problems that only a real run against a real checkout shows. ensureRepo hard-reset whatever branch happened to be checked out. Because the checkout location is configurable it may be a checkout someone else is using, whose branch would then be reset and the release committed onto it, so the published branch is now checked out explicitly first. Commits set the committer as well as the author: the container has no git identity, so JGit derived one from the process user and hostname, producing commits committed by root@<container id>. An entry that already carried the released version was reported as missing from the downloads page, because the result could not distinguish an absent entry from an up-to-date one. Re-running finalize therefore raised a false alarm; the two cases are now separate. Also silences JGit's filesystem timestamp resolution warning, which it emits when it cannot measure the resolution of the directory holding the checkout. It looks like a failure mid-release and there is nothing to act on.
Checking out the published branch was not preceded by discarding the working tree, so it threw CheckoutConflictException whenever the configured checkout was on another branch with an uncommitted change to a file whose content differs between that branch and the published one - exactly the case the branch handling exists for. The working tree is now reset before switching. The previous test did not catch this because it branched at the same commit, making the switch a no-op; the new one commits a divergent change first. The downloads page outcome was decided on counters summed over all of a release's artifact ids, so a single already-current entry hid siblings that were missing altogether and the message claimed ids were listed when they were not. It also made the other-major message unreachable whenever anything was current. Each artifact id is now classified on its own and every category reported with the ids that actually belong to it. Also closes the Git returned by the clone, which leaked the repository and its pack handles for the rest of the process.
royteeuwen
force-pushed
the
feature/SLING-13253-site-update-fixes
branch
from
August 14, 2026 19:25
34b037a to
a6afb9a
Compare
Contributor
Author
|
Rebased onto master (which now carries the image rename from #41) and pushed two further fixes found while reviewing this branch:
Plus closing the |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack: 1 of 2 — #48 (shallow clone) builds on this.
Split out of #48 per review feedback: those were fixes rather than part of the shallow-clone change, and belong lower in the stack. All three are problems that only showed up running the website update against a real checkout, not in the unit tests as they were.
ensureReporeset the wrong branch. It hard-reset whatever branch happened to be checked out. Since the checkout location is configurable it may be one someone else is using, whose branch would be reset and the release then committed onto it. The published branch is now checked out explicitly first.Commits recorded the wrong committer. Only the author was set, so JGit derived the committer from the process user and hostname —
root@<container id>inside the image. Both are now set.A false "add it by hand" warning. An entry that already carried the released version was reported as missing from the downloads page, because the result could not distinguish an absent entry from an up-to-date one. Re-running
finalizetherefore raised a false alarm on every re-run; the two cases are now separate.Also silences JGit's filesystem-timestamp-resolution warning, which it emits when it cannot measure the resolution of the directory holding the checkout. It reads like a failure mid-release and there is nothing to act on.
Verified: full build green on its own (176 tests). The committer fix in particular was caught by the real-repository tests added in #40 — the previous mocked version asserted against itself and could not see it.