Skip to content

Fix #6309: Throw actionable exception on zero or negative version code#6329

Open
thirumani-vihaan wants to merge 1 commit into
oppia:developfrom
thirumani-vihaan:fix/6309-negative-version-code-v2
Open

Fix #6309: Throw actionable exception on zero or negative version code#6329
thirumani-vihaan wants to merge 1 commit into
oppia:developfrom
thirumani-vihaan:fix/6309-negative-version-code-v2

Conversation

@thirumani-vihaan

@thirumani-vihaan thirumani-vihaan commented Jul 16, 2026

Copy link
Copy Markdown

Explanation
Fixes #6309

This prevents shallow Git clone math from silently breaking the build pipeline.

When a user clones with --depth, countCommits returns a small number (e.g., 10). Subtracting the starting commit number (2289) results in a heavily negative value, which Android OS rejects.

This PR adds a check(versionCode > 0) state validation that catches this specific scenario and throws an IllegalStateException instructing the user to run git fetch --unshallow before proceeding. A regression test simulating a 10-commit shallow clone has been added to verify this behavior.

Essential Checklist

  • The PR title starts with "Fix #bugnum: "
  • The explanation section above starts with "Fixes #bugnum: "
  • Any changes to scripts/assets files have their rationale included in the PR explanation.
  • The PR follows the style guide.
  • The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
  • The PR does not contain any unnecessary code changes from Android Studio.
  • The PR is assigned to the appropriate reviewers.

@thirumani-vihaan
thirumani-vihaan requested a review from a team as a code owner July 16, 2026 15:57
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Neer-rn Neer-rn self-assigned this Jul 16, 2026

@Neer-rn Neer-rn 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.

LGTM! Please remove the last part of the PR description. You can simply link the closed PR instead. Thank you for the work. @adhiamboperes PTAL

@oppiabot oppiabot Bot unassigned Neer-rn Jul 16, 2026
@oppiabot

oppiabot Bot commented Jul 16, 2026

Copy link
Copy Markdown

Unassigning @Neer-rn since they have already approved the PR.

@oppiabot

oppiabot Bot commented Jul 16, 2026

Copy link
Copy Markdown

Assigning @manas-yu for code owner reviews. Thanks!

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Results

Number of files assessed: 1
Overall Coverage: 99.21%
Coverage Analysis: PASS

Passing coverage

Files with passing code coverage
File Coverage Lines Hit Status Min Required
TransformAndroidManifest.ktscripts/src/java/org/oppia/android/scripts/build/TransformAndroidManifest.kt
99.21% 125 / 126 70%

To learn more, visit the Oppia Android Code Coverage wiki page

@adhiamboperes adhiamboperes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @thirumani-vihaan! I have left a comment inline, PTAL.

Please note that I have not yet reviewed the test file.

Comment on lines +205 to +212
val versionCode =
BASE_VERSION_CODE + releaseVersionOffset + rcVersionOffset + flavorVersionOffset
check(versionCode > 0) {
"Computed version code ($versionCode) is zero or negative. " +
"This is likely due to using a shallow Git clone. " +
"Please run 'git fetch --unshallow' and try again."
}
return versionCode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@thirumani-vihaan, this fix directly addresses the symptom described in the bug, but tt treats the symptom, not the cause. The check catches a negative version code, but it doesn't verify that the reason is a shallow clone. If the version code goes negative for some other reason (e.g. bad flavor offset config, integer overflow in rcVersionOffset, someone fmistakenly adjusting BASE_VERSION_CODE), running git fetch --unshallow won't fix a config bug.

Additionally, it doesn't handle the false-negative case. A shallow clone with enough depth, or an unlucky commit count, could still produce a small-but-positive version code that's incorrect, but it would sail through check(versionCode > 0) undetected.

A suggestion I have is to detect shallow clones explicitly e.g., via git rev-parse --is-shallow-repository or checking for .git/shallow and fail before even computing the version code, with a message that "you're in a shallow clone" rather than inferred from a downstream symptom.

Ideally, combine both approaches:

  • Detect shallow clone explicitly wherever git history is queried and fail immediately with a precise message.
  • Keep the check(versionCode > 0) as a defensive backstop for any cause of an invalid version code, but make its message generic ("computed version code is invalid, check versioning inputs") rather than assuming the shallow-clone cause, since that assumption won't always hold.

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.

[BUG]: TransformAndroidManifest can compute negative version codes

4 participants