Skip to content

fix: initialize libgit2 in merge_file - #1315

Merged
weihanglo merged 1 commit into
rust-lang:mainfrom
msuozzo:push-kltyzmsmkuxz
Oct 15, 2026
Merged

weihanglo merged 1 commit into
rust-lang:mainfrom
msuozzo:push-kltyzmsmkuxz

Conversation

@msuozzo

@msuozzo msuozzo commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

merge_file called into libgit2 without crate::init(), so when it ran
before anything else had initialized the library, libgit2's placeholder
allocator failed every allocation and the merge returned an error with
no message.

`merge_file` called into libgit2 without `crate::init()`, so when it ran
before anything else had initialized the library, libgit2's placeholder
allocator failed every allocation and the merge returned an error with
no message.
@rustbot rustbot added the S-waiting-on-review Status: Waiting on review label Oct 11, 2026

@DanielEScherzer DanielEScherzer left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

community review: I would suggest putting the initialization in MergeFileInput::new() instead, which will always be called before this can run, but might also help other code paths

View changes since this review

@msuozzo

msuozzo commented Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! I'd lean toward keeping it in merge_file to match the rest of the crate: crate::init() lives at the entry points that actually do libgit2 work, while constructors like MergeFileOptions::new() and MergeFileInput::new() just fill in structs via the *_init functions, which don't need the library initialized. MergeFileInput only gets used by merge_file anyway, so there aren't really other paths it'd cover.

@DanielEScherzer DanielEScherzer left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense, thanks for explaining

View changes since this review

@weihanglo weihanglo left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the review and the patch, folks!

View changes since this review

@weihanglo
weihanglo added this pull request to the merge queue Oct 15, 2026
Merged via the queue into rust-lang:main with commit 472010f Oct 15, 2026
12 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Waiting on review label Oct 15, 2026
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.

4 participants