GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — perf: download bundletool on demand instead of vendoring it by edusperoni · Pull Request #6097 · NativeScript/nativescript-cli

Conversation

edusperoni commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

PR Checklist

The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/tools/notes/CONTRIBUTING.md#commit-messages.
There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
You have signed the CLA.
All existing tests are passing: https://github.com/NativeScript/nativescript-cli/blob/master/CONTRIBUTING.md#contribute-to-the-code-base
Tests for the changes are included.

No tracking issue — happy to open one if preferred.

What is the current behavior?

vendor/aab-tool/bundletool.jar is checked into the repo and shipped with every install. At 32,515,387 bytes it is the single largest thing in the package — roughly nine tenths of the published tarball.

It earns none of that. AndroidBundleToolService is only reached from AndroidApplicationManager.installApplication() when the artifact being installed is an .aab, i.e. ns run/ns deploy against a bundle. Producing the .aab is gradle's job and never touches the jar, so ns build android --aab doesn't need it either. Every user pays 32MB of download and disk for a path most of them never take.

What is the new behavior?

bundletool is resolved lazily, the first time it is actually invoked, and cached per-user:

NS_BUNDLETOOL_PATH, if set, is used as-is (fails loudly if the file is missing) — the escape hatch for offline, air-gapped or mirrored setups.
Otherwise <profileDir>/bundletool/bundletool-all-<version>.jar, reused when its sha256 matches the pinned value.
Otherwise downloaded from the pinned GitHub release, verified against the pinned sha256, and moved into place with an atomic rename, all under $lockService so two concurrent CLIs neither race nor fetch the same 32MB twice. A tampered or partial cache entry is discarded and re-fetched.

The version lives in the filename, so bumping BUNDLETOOL_VERSION simply fetches the new jar — no staleness logic, and two CLI versions can coexist on one machine. A failed download names the URL and points at NS_BUNDLETOOL_PATH.

The download reports progress through the existing $terminalSpinnerService, once, on first use.

Supporting changes

httpRequest gained a pipeTo option. It previously ran every response through JSON.stringify, which buffers and corrupts a binary payload — streaming was a prerequisite, not a nicety.

The same axios call now also passes httpsAgent. axios selects the agent by protocol, so with httpAgent alone every https:// request was silently bypassing the configured proxy tunnel. That is a pre-existing bug, but the download depends on the fix, so it is included here.

Size

Measured with npm pack --dry-run ./dist on a --release build, before and after:

	packed	unpacked
before	34.0 MB	42.9 MB
after	3.4 MB	10.3 MB

A jar is already-compressed, so the saving survives gzip almost intact: -90% on the published tarball.

vendor/ still ships — vendor/gradle-plugin (148KB) remains vendored and unaffected.

Trade-off

The first .aab install on a machine now needs network access. NS_BUNDLETOOL_PATH covers the cases where that is not acceptable. Given the jar is untouched by every other command, taxing every install to pre-stage it was the worse default.

Verification

Full suite green: 1523 passing, 38 skipped, 102 files.
9 new unit tests for the resolution logic: env-var override and its failure mode, cache hit, resolve-once memoisation, download + verify + atomic rename, re-check under the lock when another process won the race, checksum mismatch, download failure, and eviction of a tampered cache entry.
The download path itself was exercised against the real GitHub release, outside the stubs: 32,515,387 bytes retrieved, redirect to objects.githubusercontent.com followed transparently, progress events delivered with a total, body left undefined on the pipe path, and the resulting sha256 equal to the pinned constant — which is also byte-identical to the jar this PR deletes.

ns doctor needs no changes; it has never had any knowledge of bundletool or vendor/.

Summary by CodeRabbit

New Features
- Android app package generation now automatically downloads and caches the required Bundletool version.
- Added integrity checks to validate downloaded tools and safely recover from incomplete or corrupted downloads.
- Supports using a locally provided Bundletool JAR through an environment variable.
- Improved download progress reporting and prevents duplicate downloads during concurrent operations.
Bug Fixes
- Improved streaming download handling and proxy authentication error messages.
- Ensured invalid cached tools are removed and replaced automatically.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

📝 Walkthrough

Walkthrough

HTTP downloads can now stream into destinations. Android bundletool resolution now supports overrides, cached downloads, checksum validation, locking, progress reporting, and atomic installation, with tests covering these paths.

Changes

Bundletool acquisition and HTTP streaming

Layer / File(s)	Summary
HTTP streaming transport `lib/common/http-client.ts`	Axios requests support streamed responses, download progress, HTTPS agents, pipeline-based destinations, and updated diagnostics.
Cached bundletool resolution and validation `lib/constants.ts`, `lib/services/android/android-bundle-tool-service.ts`, `test/services/android-bundle-tool-service.ts`	Bundletool configuration, environment overrides, cached checksum-verified downloads, locking, atomic replacement, progress reporting, and lifecycle tests are added.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AndroidBundleToolService
  participant IFileSystem
  participant ILockService
  participant HttpClient
  AndroidBundleToolService->>IFileSystem: check cached bundletool JAR
  AndroidBundleToolService->>ILockService: coordinate download
  AndroidBundleToolService->>HttpClient: stream bundletool release
  HttpClient->>IFileSystem: write temporary download
  AndroidBundleToolService->>IFileSystem: verify checksum and rename JAR

Possibly related PRs

NativeScript/nativescript-cli#6092: Updates the same asset-copy helper and its copyFile() logic.

Suggested reviewers: nathanwalker

Poem

I hop through streams where bundletools fly,
Cache jars beneath the Android sky.
Checksums guard each downloaded byte,
Locks keep the rabbits’ paths just right.
With progress dots, we cheer and sing—
Atomic jars make springtime spring!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: bundletool is now downloaded on demand instead of being vendored.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          perf: download bundletool on demand instead of vendoring it

bae97e9

The 32MB bundletool jar shipped with every install, but it is only used
to convert and install .aab artifacts on a device - a path most users
never hit, since building an aab is gradle's job.

Resolve it lazily instead: on first use fetch the pinned release into
<profileDir>/bundletool/, verify its sha256, and move it into place
atomically under a lock so concurrent CLIs neither race nor download it
twice. NS_BUNDLETOOL_PATH overrides the whole flow for offline and
air-gapped setups.

This needs a streaming download, so httpRequest grows a pipeTo option -
it previously ran every response through JSON.stringify, which would
corrupt a binary payload. The same call now also sets httpsAgent, since
axios selects the agent by protocol and https requests were silently
bypassing the configured proxy tunnel.

Release package drops from 34.0MB to 3.4MB (42.9MB to 10.3MB unpacked).

edusperoni force-pushed the feat/lazy-bundletool branch from 3b9492f to bae97e9 Compare

July 29, 2026 17:30

NathanWalker approved these changes

Jul 29, 2026

View reviewed changes

coderabbitai Bot reviewed

Jul 29, 2026

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

lib/common/http-client.ts (2)
41-59: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Retrying a pipeTo request reuses an already-destroyed destination stream.

On a stuck request/response, httpRequestCore is retried with the same options object (Line 49). When options.pipeTo is set, the first attempt's failure runs pipeline(result.data, options.pipeTo) (or the request never even reaches that point), and Node's stream.pipeline destroys both source and destination streams on error. Retrying with the same destroyed write stream will either throw (ERR_STREAM_DESTROYED) or produce a corrupted/incomplete file, defeating the purpose of the retry.
🔧 Suggested fix: don't retry when a destination stream is involved
 			) {
+				if (options.pipeTo) {
+					// the destination stream from the failed attempt is already
+					// destroyed or partially written; retrying would corrupt it
+					throw err;
+				}
 				// Retry the request immediately because there are at least 10 seconds between the two requests.
 				this.$logger.warn(
 					"%s Retrying request to %s...",
 					err.message,
 					options.url || options,
 				);
 				const retryResult = await this.httpRequestCore(options, proxySettings);
This directly affects the new bundletool download path (downloadBundleTool in lib/services/android/android-bundle-tool-service.ts), which passes a fs.createWriteStream as pipeTo.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/http-client.ts` around lines 41 - 59, Update the retry branch in
httpRequestCore to skip the immediate retry whenever options.pipeTo is set,
allowing the original error to propagate instead of reusing a destroyed
destination stream. Preserve the existing retry behavior for requests without a
pipeTo destination, including its logging and response handling.
111-140: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Close the download stream before deleteFile runs.

When options.pipeTo is used, pipeline() is not called in the .catch handler, so a failed HTTP stream leaves options.pipeTo open. downloadBundleTool then calls fs.unlinkSync(tempPath) on that open file handle, which can fail on Windows and leak the descriptor on all platforms.
Suggested fix: destroy the destination stream on any request failure
 		}).catch((err) => {
 			this.$logger.trace("An error occurred while sending the request:", err);
+			if (options.pipeTo && typeof options.pipeTo.destroy === "function") {
+				options.pipeTo.destroy();
+			}
 			if (err.response) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/http-client.ts` around lines 111 - 140, Update the request error
handler in the axios call to destroy or otherwise close the destination stream
referenced by options.pipeTo before rethrowing the error. Ensure this cleanup
runs for every failed piped request, while preserving existing error-message
handling and behavior for non-stream requests.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/common/http-client.ts`:
- Around line 44-48: Update the retry warning in the HTTP client’s logger.warn
call to avoid passing the full options object when options.url is falsy. Log
only a safe, bounded request identifier or fallback value that excludes headers,
credentials, and request bodies, while preserving the existing URL logging
behavior when available.

In `@lib/services/android/android-bundle-tool-service.ts`:
- Around line 119-124: Update getBundleToolPath to clear bundleToolPathPromise
when resolveBundleToolPath rejects, while retaining the cached promise after
successful resolution. Ensure subsequent installApks or buildApks calls can
retry resolution after a transient failure.

---

Outside diff comments:
In `@lib/common/http-client.ts`:
- Around line 41-59: Update the retry branch in httpRequestCore to skip the
immediate retry whenever options.pipeTo is set, allowing the original error to
propagate instead of reusing a destroyed destination stream. Preserve the
existing retry behavior for requests without a pipeTo destination, including its
logging and response handling.
- Around line 111-140: Update the request error handler in the axios call to
destroy or otherwise close the destination stream referenced by options.pipeTo
before rethrowing the error. Ensure this cleanup runs for every failed piped
request, while preserving existing error-message handling and behavior for
non-stream requests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d982bbf-bd80-4939-af56-08c523f8de76

📥 Commits

Reviewing files that changed from the base of the PR and between ddd39b8 and bae97e9.

⛔ Files ignored due to path filters (1)

vendor/aab-tool/bundletool.jar is excluded by !**/*.jar

📒 Files selected for processing (7)

lib/common/http-client.ts
lib/constants.ts
lib/services/android/android-bundle-tool-service.ts
scripts/copy-assets.js
test/services/android-bundle-tool-service.ts
vendor/aab-tool/LICENSE
vendor/aab-tool/README.txt

💤 Files with no reviewable changes (3)

vendor/aab-tool/README.txt
vendor/aab-tool/LICENSE
scripts/copy-assets.js

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

lib/common/http-client.ts

Comment on lines 44 to 48

		this.$logger.warn(
		"%s Retrying request to %s...",
		err.message,
		options.url \|\| options
		options.url \|\| options,
		);

coderabbitai Bot Jul 29, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Retry-warning fallback can log the entire options object, including headers/credentials.

options.url || options will log the full options object (potentially containing Authorization headers, proxy credentials, or request bodies) whenever url is falsy. Prefer logging a safe, bounded subset.

🔧 Suggested fix

 				this.$logger.warn(
 					"%s Retrying request to %s...",
 					err.message,
-					options.url || options,
+					options.url || options.method || "unknown request",
 				);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	this.$logger.warn(
	"%s Retrying request to %s...",
	err.message,
	options.url \|\| options
	options.url \|\| options,
	);
	this.$logger.warn(
	"%s Retrying request to %s...",
	err.message,
	options.url \|\| options.method \|\| "unknown request",
	);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/common/http-client.ts` around lines 44 - 48, Update the retry warning in
the HTTP client’s logger.warn call to avoid passing the full options object when
options.url is falsy. Log only a safe, bounded request identifier or fallback
value that excludes headers, credentials, and request bodies, while preserving
the existing URL logging behavior when available.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

lib/services/android/android-bundle-tool-service.ts

Comment on lines +119 to +124

		private getBundleToolPath(): Promise<string> {
		this.bundleToolPathPromise =
		this.bundleToolPathPromise \|\| this.resolveBundleToolPath();

		return this.bundleToolPathPromise;
		}

coderabbitai Bot Jul 29, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

A failed resolution is cached forever for the life of the process.

bundleToolPathPromise is memoized unconditionally. If resolveBundleToolPath() rejects once (transient network blip, momentary checksum mismatch, etc.), every subsequent installApks/buildApks call in the same process will immediately re-reject with the same stale error — there's no way to retry without restarting the CLI process.

🔧 Suggested fix: clear the cached promise on failure

 	private getBundleToolPath(): Promise<string> {
-		this.bundleToolPathPromise =
-			this.bundleToolPathPromise || this.resolveBundleToolPath();
+		if (!this.bundleToolPathPromise) {
+			this.bundleToolPathPromise = this.resolveBundleToolPath().catch(
+				(err) => {
+					this.bundleToolPathPromise = null;
+					throw err;
+				},
+			);
+		}

 		return this.bundleToolPathPromise;
 	}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	private getBundleToolPath(): Promise<string> {
	this.bundleToolPathPromise =
	this.bundleToolPathPromise \|\| this.resolveBundleToolPath();

	return this.bundleToolPathPromise;
	}
	private getBundleToolPath(): Promise<string> {
	if (!this.bundleToolPathPromise) {
	this.bundleToolPathPromise = this.resolveBundleToolPath().catch(
	(err) => {
	this.bundleToolPathPromise = null;
	throw err;
	},
	);
	}

	return this.bundleToolPathPromise;
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/services/android/android-bundle-tool-service.ts` around lines 119 - 124,
Update getBundleToolPath to clear bundleToolPathPromise when
resolveBundleToolPath rejects, while retaining the cached promise after
successful resolution. Ensure subsequent installApks or buildApks calls can
retry resolution after a transient failure.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

NathanWalker merged commit 1eae2f6 into main

Jul 29, 2026

12 checks passed

Uh oh!

There was an error while loading. Please reload this page.

NathanWalker deleted the feat/lazy-bundletool branch

July 29, 2026 17:53

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

NathanWalker

NathanWalker approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

edusperoni commented Jul 29, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

What is the current behavior?

What is the new behavior?

Supporting changes

Size

Trade-off

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

edusperoni commented Jul 29, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.