Skip to content

add apply sys api #531

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

Merged
merged 1 commit into from
Mar 20, 2020
Merged

add apply sys api #531

merged 1 commit into from
Mar 20, 2020

Conversation

extrawurst
Copy link
Contributor

added missing sys calls for missing apply API. is it ok to split this from implementing a safe API or does it have to be in one PR?

(this is related to #403)

seems like enum flags need to be untyped (fix ci)

trying to fix windows ci issue

fix typo

next try fixing windows warning

next windows ci fix
@alexcrichton
Copy link
Member

Looks good to me, thanks!

@alexcrichton alexcrichton merged commit e8347d3 into rust-lang:master Mar 20, 2020
@extrawurst extrawurst deleted the add-apply branch March 20, 2020 16:33
pub delta_cb: git_apply_delta_cb,
pub hunk_cb: git_apply_hunk_cb,
pub payload: *mut c_void,
pub flags: u32,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @alexcrichton thanks for merging!

Out of curiosity: why is it not possible to idiomatically use the enum types in the structs? This only seemed to fail on windows:
https://github.com/rust-lang/git2-rs/runs/521702373

I see many structs using the u32 instead of the actual enum type, but some seem to be able to use them, what am I missing?

Copy link
Member

Choose a reason for hiding this comment

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

I think it has to do with the way MSVC handles enums and whether they're signed or not. I don't remember a ton of specifics but I know it's slightly different from gcc which has caused issues. In any case I'm fine with tweaking things as necessary!

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.

2 participants