Skip to content

Add NoCompression option to tarball compressor#120

Merged
aramprice merged 2 commits intocloudfoundry:developfrom
mdzhigarov:feature/no-compression-option
Oct 22, 2025
Merged

Add NoCompression option to tarball compressor#120
aramprice merged 2 commits intocloudfoundry:developfrom
mdzhigarov:feature/no-compression-option

Conversation

@mdzhigarov
Copy link
Contributor

This PR adds a new NoCompression option to the tarball compressor that allows creating uncompressed tar files instead of gzipped ones.

Changes

  • Add NoCompression field to CompressorOptions struct
  • Update tarballCompressor to conditionally include -z flag based on NoCompression option
  • Add comprehensive unit tests for NoCompression functionality
  • Update fake compressor to support new option

Testing

  • Tests verify both CompressFilesInDir and CompressSpecificFilesInDir methods
  • Tests check for presence/absence of -z flag instead of exact argument matching
  • Include end-to-end test comparing compressed vs uncompressed tarball sizes
  • All 23 tests pass successfully

Usage

// Create compressed tarball (default behavior)
tarball, err := compressor.CompressFilesInDir(dir, CompressorOptions{})

// Create uncompressed tarball
tarball, err := compressor.CompressFilesInDir(dir, CompressorOptions{NoCompression: true})

Motivation

This feature is useful when you need to create uncompressed tar files, for example when working with already compressed content or when you want to avoid the overhead of compression for small files.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 19, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

- Add NoCompression field to CompressorOptions struct
- Update tarballCompressor to conditionally include -z flag based on NoCompression option
- Add comprehensive unit tests for NoCompression functionality
- Update fake compressor to support new option
- Tests verify both CompressFilesInDir and CompressSpecificFilesInDir methods
- Tests check for presence/absence of -z flag instead of exact argument matching
Copy link

@KauzClay KauzClay left a comment

Choose a reason for hiding this comment

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

I don't have approval powers, but it makes sense to me. Seems to match how CompressorOptions is used in other places in this repo.

Will you then need to pull this into https://github.com/cloudfoundry/bosh-cli/tree/main ?

@mdzhigarov
Copy link
Contributor Author

I don't have approval powers, but it makes sense to me. Seems to match how CompressorOptions is used in other places in this repo.

Will you then need to pull this into https://github.com/cloudfoundry/bosh-cli/tree/main ?

Yes, that's the plan. I have already prepared the changes in bosh-cli locally. Waiting for this to get merged so that I can open a PR in bosh-cli

@aramprice aramprice requested review from a team and rkoster September 25, 2025 14:44
@aramprice aramprice requested review from a team and selzoc and removed request for a team October 2, 2025 14:56
@aramprice aramprice moved this from Inbox to Pending Review | Discussion in Foundational Infrastructure Working Group Oct 2, 2025
@github-project-automation github-project-automation bot moved this from Pending Review | Discussion to Pending Merge | Prioritized in Foundational Infrastructure Working Group Oct 9, 2025
@mdzhigarov
Copy link
Contributor Author

Quite odd... I cannot change the base branch of this PR to get past the check-branch check.
I also tried opening a new PR under cloudfoundry:develop but seems the same check ran again.

Will investigate this tomorrow.
Thanks @rkoster for the review!

@mdzhigarov mdzhigarov changed the base branch from master to develop October 9, 2025 17:11
@mdzhigarov mdzhigarov closed this Oct 9, 2025
@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Oct 9, 2025
@mdzhigarov mdzhigarov reopened this Oct 9, 2025
@mdzhigarov
Copy link
Contributor Author

@rkoster Would you please re-trigger the check-branch job and merge this?

Omitting the -z flag during extraction (-x) is considered best practice as it works for both compressed and non-compressed tars.
@mdzhigarov mdzhigarov force-pushed the feature/no-compression-option branch from 791f4db to 45b9c17 Compare October 9, 2025 17:23
@aramprice aramprice merged commit eb14648 into cloudfoundry:develop Oct 22, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

5 participants