Add Builder functions for including folder(s) and headers

[aatifsyed]

I find myself frequently extending bindgen::Builder with the following functions:

    /// Add multiple input C/C++ headers.
    pub fn headers<T: AsRef<Path>>(
        mut self,
        headers: impl IntoIterator<Item = T>,
    ) -> Builder { ... }

    /// Include a folder for system header file resolution
    pub fn include(self, path: impl AsRef<Path>) -> Builder {
        self.clang_arg(format!("-I{}", path.as_ref().to_string_lossy()))
    }

    /// Include multiple folders for system header file resolution
    pub fn includes<T: AsRef<Path>>(
        mut self,
        paths: impl IntoIterator<Item = T>,
    ) -> Builder { ... }

This pull request adds them.

Points to consider

• use of Path::to_string_lossy
• use of -I vs --include-directory=
• Adding documentation to refer to pkg-config, a motivating usecase for includes

[aatifsyed]

@emilio for your review or referral to a reviewer

[pvdrz]

@aatifsyed it has been a while so the logs are gone. Do you happen to know why CI was failing?

Regarding the PR itself. I'd use impl AsRef<str> instead of impl AsRef<Path> as there's no consensus about how to handle invalid utf-8 paths yet.

[aatifsyed]

@pvdrz Thanks for resurrecting this!

• I've made your suggested change from Path to str
• I've rebased to the latest master, with gratitude to this guide
• I'm afraid I don't remember why the pipelines were failing

[pvdrz]

Looks good to me!

Maybe we can change includes by something less similar to include like include_folder or something? I'm bad at naming things so deferring to @emilio or @kulp for a review.

[pvdrz]

Maybe we can change includes by something less similar to include like include_folder or something? I'm bad at naming things so deferring to @emilio or @kulp for a review.

I am probably not better than @pvdrz at naming things. I agree that include and includes are too similar; but the new function headers is too close to the existing function header, too.

The only thing that comes to mind is include_multiple and header_multiple but I do not really like those either.

I am not standing in the way of this but for the moment I am a little wary of cluttering the API.

maybe we can use include_directory? It'd be clear enough I think

[bors-servo]

:umbrella: The latest upstream changes (presumably #2284) made this pull request unmergeable. Please resolve the merge conflicts.

[aatifsyed]

Hello all, thanks for your feedback :) I'm away from my PC for a couple of weeks, so I'll catch up and amend when I'm back.

I'd be interesting in feedback on the following suggestion:

• #[doc(hidden)] the existing header function. Reduces API clutter without deprecating
• add headers(self, i: impl IntoIterator<Item = impl Into<String>>)
• add include_directories(self, i: impl IntoIterator<Item = impl Into<String>>)

So the documented API remains smaller?

We could even #[deprecated] header, but I'm wary of that

[pvdrz]

I'd favor include_directories(self, i: impl IntoIterator<Item = impl Into<String>>). We would have to document clearly that people are supposed to do include_directories([dir]) for a single directory.

[bors-servo]

:umbrella: The latest upstream changes (presumably 2034a0fda98cb8a5180d4f14b8c0d7949c6cb146) made this pull request unmergeable. Please resolve the merge conflicts.

[kulp]

See #2743.

[aatifsyed]

:/

[kulp]

Possibly I was too picky about the naming of things. The bindgen project has varying amounts of volunteer support; sometimes one person gets lucky, and another unlucky. Still, I am sorry, @aatifsyed, that your PR did not get merged.