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

send-pack: build list of oids to sent to pack-objects #127

Closed
wants to merge 3 commits into from

Conversation

kewillford
Copy link
Member

This PR reverts #68 because it was causing issues with pushing to other remotes.

This change builds the list of oids before pack-objects is spawned so that if checking for the oid has to download the object via the read-object process the stdin and stdout handles will not get handled incorrectly.

The one disadvantage of this change is it will take more memory especially when there are a lot of objects to be pushed since it is building the full list first.

Another possible fix is #79 to fix handle inheritance but that change is a lot more invasive and could affect other commands. This change will limit the scope to push.

Copy link

@jeffhostetler jeffhostetler left a comment

Choose a reason for hiding this comment

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

Looks good.

I do wonder if it would be faster to build a single large
strbuf and then fwrite() the whole thing at the child process
rather than feeding it a line at a time.

But that's a bigger change. Feel free to ignore.

@derrickstolee
Copy link

@kewillford are you still working on this PR and/or #79?

@kewillford
Copy link
Member Author

@derrickstolee Nope this was fixed with #128 which went upstream.

@kewillford kewillford closed this Jul 1, 2019
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