Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thirumani-vihaan wants to merge 1 commit into
base: develop
Choose a base branch
from
+47 −1
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion scripts/src/java/org/oppia/android/scripts/build/TransformAndroidManifest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,14 @@ private class TransformAndroidManifest(
val releaseVersionOffset = possibleReleaseCount * VERSION_CODES_PER_RELEASE
val rcVersionOffset = (releaseCandidateNumber - 1) * MAX_FLAVORS_PER_RC
val flavorVersionOffset = buildFlavor.index
return BASE_VERSION_CODE + releaseVersionOffset + rcVersionOffset + flavorVersionOffset
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
Comment on lines +205 to +212

@adhiamboperes adhiamboperes Jul 23, 2026

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.

}

// The format here is defined as part of the app's release process.
Expand Down
39 changes: 39 additions & 0 deletions scripts/src/javatests/org/oppia/android/scripts/build/TransformAndroidManifestTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,37 @@ class TransformAndroidManifestTest {
)
}

@Test
fun testUtility_developBranch_shallowClone_negativeVersionCode_throwsException() {
initializeShallowGitRepositoryWithHistory()

val exception = assertThrows<IllegalStateException>() {
runScript(
tempFolder.root.absolutePath,
tempFolder.newFile(TEST_MANIFEST_FILE_NAME).apply {
writeText(TEST_MANIFEST_CONTENT_WITHOUT_VERSIONS)
}.absolutePath,
File(tempFolder.root, TRANSFORMED_MANIFEST_FILE_NAME).absolutePath,
BUILD_FLAVOR,
MAJOR_VERSION,
MINOR_VERSION,
APPLICATION_RELATIVE_QUALIFIED_CLASS,
"false",
"false"
)
}

assertThat(exception)
.hasMessageThat()
.contains("Computed version code")
assertThat(exception)
.hasMessageThat()
.contains("zero or negative")
assertThat(exception)
.hasMessageThat()
.contains("git fetch --unshallow")
}

/** Runs the transform_android_manifest utility. */
private fun runScript(vararg args: String) {
main(args.toList().toTypedArray())
Expand All @@ -751,6 +782,14 @@ class TransformAndroidManifestTest {
testGitRepository.createRemoteBranchRef("origin/develop")
}

private fun initializeShallowGitRepositoryWithHistory() {
// Initialize the git repository with a small number of commits to simulate a shallow clone.
testGitRepository.init()
testGitRepository.setUser(email = "test@oppia.org", name = "Test User")
testGitRepository.initializeHistoricalCommits(commitCount = 10)
testGitRepository.createRemoteBranchRef("origin/develop")
}

private fun getMostRecentCommitOnCurrentBranch(): String {
// See https://stackoverflow.com/a/949391 for a reference to validate that this is correct.
return commandExecutor.executeCommand(
Expand Down
Loading