Skip to content
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.

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

Compressing tar files using CompressionStream broke in Deno 1.43.0 #25276

Closed
jespertheend opened this issue Aug 28, 2024 · 6 comments
Closed
Labels
invalid what appeared to be an issue with Deno wasn't

Comments

@jespertheend
Copy link
Contributor

Version: Deno 1.43.0

This is a reduced test case of a script I'm using to archive and compress several files. In Deno 1.42.4 this creates the archive just fine, but in Deno 1.43.0 this always results in a file of 41 bytes.

To reproduce, store this in a file and run with deno run -A compress.ts

import { copy, readableStreamFromReader, readerFromStreamReader } from "https://deno.land/std@0.204.0/streams/mod.ts";
import { Tar } from "https://deno.land/std@0.204.0/archive/tar.ts";
import * as stdPath from "https://deno.land/std@0.221.0/path/mod.ts";

const selfPath = stdPath.fromFileUrl(import.meta.url);
const tar = new Tar();
tar.append("self.ts", {
	filePath: selfPath,
});
const tarPath = stdPath.resolve(stdPath.dirname(selfPath), "compressed.tar.gz");
const writer = await Deno.open(tarPath, {
	create: true,
	truncate: true,
	write: true,
});
const compressionReader = readableStreamFromReader(tar.getReader()).pipeThrough(new CompressionStream("gzip"))
	.getReader();
await copy(readerFromStreamReader(compressionReader), writer);
writer.close();

I think this may have regressed in #21199

@nathanwhit nathanwhit added the bug Something isn't working correctly label Aug 28, 2024
@dsherret
Copy link
Member

dsherret commented Aug 29, 2024

I don't think this one is a bug. copy doesn't guarantee it copies all the bytes—it returns back a number of bytes that it copies: https://deno.land/std@0.204.0/streams/mod.ts?s=copy

I think the code is just not handling that.

@dsherret
Copy link
Member

Nevermind, I looked into the implementation of copy and it loops to write the whole reader to the writer. This does seem like a bug.

@0f-0b
Copy link
Contributor

0f-0b commented Aug 29, 2024

tar.append returns a promise that isn't awaited, so the file might not have been added yet by the time tar.getReader is called.

@lucacasonato
Copy link
Member

lucacasonato commented Aug 29, 2024

I can not reproduce this in Deno 1.46.2-canary (with or without the await). Is there any particular reason you are still on Deno 1.43?

@dsherret dsherret added invalid what appeared to be an issue with Deno wasn't and removed bug Something isn't working correctly labels Aug 29, 2024
@dsherret
Copy link
Member

@0f-0b that's the issue.

I'm able to reproduce on canary, but once I await the append the issue no longer occurs.

Still waiting for the day that TypeScript adds diagnostics to disallow floating promises so that we can all be saved from this.

@dsherret dsherret closed this as not planned Won't fix, can't repro, duplicate, stale Aug 29, 2024
@jespertheend
Copy link
Contributor Author

Is there any particular reason you are still on Deno 1.43?

Not really, that's just the first version where this started failing. I'd like to use the latest version but I can reproduce this in 1.46.2 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid what appeared to be an issue with Deno wasn't
Projects
None yet
Development

No branches or pull requests

5 participants