Skip to content

Conversation

@theoparis
Copy link

This is not the best fix atm as it duplicates code.
See #17064

This is not the best fix as it duplicates code.
.handle = iterable_dir.dir,
};
};
defer tmp_directory.closeAndFree(gpa);
Copy link
Member

@squeek502 squeek502 Sep 8, 2023

Choose a reason for hiding this comment

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

Won't this double closeAndFree tmp_directory (since the original defer is still active)? And the first tmp_directory won't ever be closeAndFreed?

// by default, so the same logic applies for buffering the reader as for gzip.
try unpackTarball(gpa, prog_reader.reader(), tmp_directory.handle, std.compress.xz);

tmp_directory = d: {
Copy link
Member

Choose a reason for hiding this comment

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

Why only reopen in the application/x-xz case? Wouldn't this affect all the other cases too?

// by default, so the same logic applies for buffering the reader as for gzip.
try unpackTarball(gpa, prog_reader.reader(), tmp_directory.handle, std.compress.xz);

tmp_directory = d: {
Copy link
Member

@squeek502 squeek502 Sep 8, 2023

Choose a reason for hiding this comment

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

Definitely need an explainer comment detailing why this is needed (and that it's effectively a workaround for a potential IterableDir.Iterator/Walker bug).

@xdBronch xdBronch mentioned this pull request Oct 4, 2023
11 tasks
@andrewrk
Copy link
Member

andrewrk commented Oct 4, 2023

cleaner workaround coming as part of #17392

@andrewrk andrewrk closed this Oct 4, 2023
@theoparis theoparis deleted the theoparis/17064 branch August 1, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants