Skip to content

Build: keep generated version state reproducible - #519

Merged
Narflex merged 1 commit into
google:masterfrom
opensagetv-vibe:sagetv-review/build-source-clean
Sep 29, 2026
Merged

Narflex merged 1 commit into
google:masterfrom
opensagetv-vibe:sagetv-review/build-source-clean

Conversation

@jzhvymetal

@jzhvymetal jzhvymetal commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • restore generated SageConstants.java build-version state after builds
  • prevent ordinary Gradle builds from leaving tracked source modified

Why

The build-number task rewrites a tracked Java source file. Without deterministic restoration, a successful build can leave a dirty worktree and accidentally include generated version state in a later commit.

Reproduction

  1. Start from a clean checkout.
  2. Run a Gradle task that reaches compileJava without completing sageJar.
  3. Inspect git status, SageConstants.java, and SageConstants.java.bak.

Before this change, restoration is finalized only by sageJar, and backup deletion is deferred with deleteOnExit. Other successful compile paths can therefore leave generated state or its backup behind until JVM exit or later.

After this change, compileJava.finalizedBy restoreSageConstants restores the tracked source after every compile path. Backup deletion occurs immediately and fails visibly if it cannot complete.

Validation

  • focused gradlew compileJava --no-daemon completed successfully
  • SageConstants.java SHA-256 was identical before and after compilation
  • SageConstants.java.bak did not remain after the build
  • previously recorded complete Java suite and sageJar gates passed
  • canonical check-changes and cla/google pass

Scope and risk

This PR now changes only build.gradle: four inserted lines and two removed lines. The unrelated .gitattributes file and all line-ending policy have been removed. It does not change runtime Java behavior, packaging, deployment, Vibe workflows, or container policy.

Review state

Small maintenance change; ready for individual review.

Current-Ubuntu container dependency

This is one of four independent Core approvals needed before proposing the current-Ubuntu runtime image to OpenSageTV/sagetv-dockers: #516 launcher behavior, #519 reproducible source-clean builds, #528 current GCC/64-bit native compatibility, and the separable Ubuntu/ImageLoader portions of #529. This PR is a build-integrity prerequisite rather than a runtime/GPU behavior change.

@google-cla

google-cla Bot commented Sep 29, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Narflex Narflex left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one I've noticed can be a problem, so I'm fine with this change...but the line endings modification shouldn't be part of that (and I'm not aware of an actual problem with line endings, so I'm not a fan of that change).

Comment thread .gitattributes Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line endings has nothing to do with this actual change, remove this new file please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

• The branch now contains only the build.gradle lifecycle fix; .gitattributes and the unrelated trailing-line change are gone. I’m running the focused compileJava reproduction to prove the generated Sage constants are restored and no backup or tracked-source modification remains. I’m auditing every remaining open PR now and will remove or isolate unrelated line-ending changes anywhere they are not essential to that PR’s demonstrated problem. The focused #519 build is still completing.

@jzhvymetal
jzhvymetal force-pushed the sagetv-review/build-source-clean branch from f6bd5c3 to 089984e Compare September 29, 2026 20:42
@jzhvymetal

Copy link
Copy Markdown
Contributor Author

Agreed. I removed the new .gitattributes file and all line-ending claims from this PR.

The PR now changes only build.gradle (4 insertions, 2 deletions) for the generated-version restoration lifecycle. I reran the focused reproduction with gradlew compileJava --no-daemon: the build passed, SageConstants.java had the identical SHA-256 before and after, and no .bak file remained.

I also audited every other open PR for the same issue. Only the broader draft #529 contained a smaller .gitattributes policy, and I removed it there as well so line-ending policy is not bundled into another change.

@Narflex
Narflex merged commit 1888f18 into google:master Sep 29, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants