Skip to content

make internal bin the second entry in bundles#1024

Merged
trunk-io[bot] merged 1 commit intomainfrom
dylan/make-internal-bin-second-entry-to-bundles
Feb 4, 2026
Merged

make internal bin the second entry in bundles#1024
trunk-io[bot] merged 1 commit intomainfrom
dylan/make-internal-bin-second-entry-to-bundles

Conversation

@dfrankland
Copy link
Copy Markdown
Member

@dfrankland dfrankland commented Feb 4, 2026

depends on:

This greatly improves our ability to extract only what's needed from byte streams

@trunk-io
Copy link
Copy Markdown

trunk-io Bot commented Feb 4, 2026

😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details.

@dfrankland dfrankland force-pushed the dylan/update-to-rust-edition-2024 branch from aa07767 to 7356854 Compare February 4, 2026 04:07
@dfrankland dfrankland force-pushed the dylan/make-internal-bin-second-entry-to-bundles branch from 073e460 to 53eec9a Compare February 4, 2026 04:08
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 93.57798% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.66%. Comparing base (f024d97) to head (ec667b2).

Files with missing lines Patch % Lines
bundle/src/bundler.rs 93.95% 13 Missing ⚠️
bundle/src/bundle_meta.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1024      +/-   ##
==========================================
+ Coverage   81.33%   81.66%   +0.33%     
==========================================
  Files          69       69              
  Lines       14526    14527       +1     
==========================================
+ Hits        11815    11864      +49     
+ Misses       2711     2663      -48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dfrankland dfrankland force-pushed the dylan/update-to-rust-edition-2024 branch from 7356854 to 363806c Compare February 4, 2026 04:27
@dfrankland dfrankland force-pushed the dylan/make-internal-bin-second-entry-to-bundles branch from 53eec9a to b8c16c1 Compare February 4, 2026 04:29
@dfrankland dfrankland force-pushed the dylan/update-to-rust-edition-2024 branch from 363806c to 57f1b88 Compare February 4, 2026 04:46
@dfrankland dfrankland force-pushed the dylan/make-internal-bin-second-entry-to-bundles branch from b8c16c1 to 31f21f9 Compare February 4, 2026 04:46
@dfrankland dfrankland force-pushed the dylan/update-to-rust-edition-2024 branch from 57f1b88 to bb29ba9 Compare February 4, 2026 04:51
@dfrankland dfrankland force-pushed the dylan/make-internal-bin-second-entry-to-bundles branch from 31f21f9 to d6c56a5 Compare February 4, 2026 04:51
@trunk-staging-io
Copy link
Copy Markdown

trunk-staging-io Bot commented Feb 4, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Comment thread bundle/src/bundle_meta.rs
Comment on lines -340 to +343
pub fn internal_bundled_file(&self) -> Option<BundledFile> {
pub fn internal_bundled_file(&self) -> Option<&BundledFile> {
match self {
Self::V0_7_8(data) => data.internal_bundled_file.clone(),
Self::V0_7_7(data) => data.internal_bundled_file.clone(),
Self::V0_7_8(data) => data.internal_bundled_file.as_ref(),
Self::V0_7_7(data) => data.internal_bundled_file.as_ref(),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure why this was cloned before, but from what I can see only a reference is needed

Comment thread bundle/src/bundler.rs
Comment on lines +71 to +77
// Add the internal binary file if it exists.
if let Some(bundled_file) = self.meta.internal_bundled_file.as_ref() {
let path = std::path::Path::new(&bundled_file.original_path);
let mut file = File::open(path)?;
tar.append_file(&bundled_file.path, &mut file)?;
total_bytes_in += std::fs::metadata(path)?.len();
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

moved up from below

Comment thread bundle/src/bundler.rs
Comment on lines -176 to -178
/// Reads and decompresses a .tar.zstd file from an input stream into just a `meta.json` file.
/// This assumes that the `meta.json` file will be the first entry in the tarball.
pub async fn parse_meta_from_tarball<R: AsyncBufRead>(input: R) -> anyhow::Result<VersionedBundle> {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

moved down to be closer to the other parse_ functions

Comment thread bundle/src/bundler.rs
Comment on lines -216 to -217
/// Reads and decompresses a .tar.zstd file from an input stream into just the internal bin file.
pub async fn parse_internal_bin_from_tarball<R: AsyncBufRead>(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

also moved down to be closer to other parse_ functions

Comment thread bundle/src/bundler.rs
Err(anyhow::anyhow!("No internal.bin file found in the tarball"))
}

#[cfg(test)]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

tests were getting a bit noisy, so I tried to clean them up a bit and make them clearer

Comment thread bundle/src/bundler.rs

#[tokio::test]
pub async fn test_nondefault_internal_bin_path() {
pub async fn test_internal_bin_is_second_entry() {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added this test new to check that the generated bundle's second entry is the internal bin

Comment thread bundle/src/bundler.rs
#[test]
pub fn test_no_duplicate_internal_file() {
#[tokio::test]
pub async fn test_internal_bin_is_backwards_compatible_with_last_entry() {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

also added this test to ensure that parsing the internal bin works even if it's not the second entry

@dfrankland dfrankland marked this pull request as ready for review February 4, 2026 05:41
@trunk-io
Copy link
Copy Markdown

trunk-io Bot commented Feb 4, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Comment thread bundle/src/bundler.rs
Comment on lines +200 to +214
async fn parse_meta_entry<R: AsyncBufRead>(
entry: &mut Entry<Archive<ZstdDecoder<Pin<Box<R>>>>>,
) -> anyhow::Result<Option<VersionedBundle>> {
if let Some(path_str) = entry.path()?.to_str()
&& path_str == META_FILENAME
{
let mut meta_bytes = Vec::new();
entry.read_to_end(&mut meta_bytes).await?;
Ok(Some(parse_meta(meta_bytes)?))
} else {
Ok(None)
}
}

anyhow::Result::Err(anyhow::anyhow!(
"No {} file found in the tarball",
target_name
))
async fn parse_meta_from_first_entry<R: AsyncBufRead>(
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.

which should be used? parse_meta_from_first_entry?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Neither parse_meta_entry nor parse_meta_from_first_entry are public, they are only used internally here. Additionally, they take different arguments:

  • parse_meta_entry: takes a &mut Entry<T> (object containing file bytes)
  • parse_meta_from_first_entry: takes a &mut Entries<T> (stream of files)

parse_meta_entry is for parsing the entry itself while parse_meta_from_first_entry is for ensuring the first entry of the stream parses

@dfrankland dfrankland changed the base branch from dylan/update-to-rust-edition-2024 to main February 4, 2026 17:05
@dfrankland dfrankland force-pushed the dylan/make-internal-bin-second-entry-to-bundles branch from d6c56a5 to ec667b2 Compare February 4, 2026 17:06
@trunk-io trunk-io Bot merged commit ed25a09 into main Feb 4, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants