Skip to content

Conversation

@BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Jul 19, 2023

We need this capability to call git mktree.

The temporary-file-less fix to #1144 was authored by https://github.com/klalumiere

@autoantwort
Copy link
Contributor

Maybe we should support a streaming interface for #1043 so that it is not necessary to put the whole file chunk into RAM.

void close_handle_mark_invalid(HANDLE& target) noexcept
{
auto to_close = std::exchange(target, INVALID_HANDLE_VALUE);
if (to_close != INVALID_HANDLE_VALUE && to_close)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this checking to_close for null here -- and should that be a fatal error since it's an invariant violation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Some APIs use 0 as 'invalid handle', others use INVALID_HANDLE_VALUE. The intent is for this to work with either without having to worry about which is which.

@BillyONeal
Copy link
Member Author

Maybe we should support a streaming interface for #1043 so that it is not necessary to put the whole file chunk into RAM.

It seems like 1043 should work by attaching the file directly rather than needing to describe the right concurrency ... thing that would have to go here.

@BillyONeal BillyONeal marked this pull request as ready for review August 1, 2023 07:13
Co-authored-by: Kevin Lalumiere <[email protected]>
@BillyONeal BillyONeal requested a review from klalumiere August 1, 2023 23:27
Copy link
Contributor

@klalumiere klalumiere left a comment

Choose a reason for hiding this comment

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

Thanks for the attribution for the changes in downloads.cpp! 🙂

@BillyONeal BillyONeal merged commit 81c8bed into microsoft:main Aug 12, 2023
@BillyONeal BillyONeal deleted the process-stdin branch August 12, 2023 04:18
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.

5 participants