Bindgen as a workspace #2284

pvdrz · 2022-09-24T02:44:58Z

This reorganizes the repo into a virtual workspace. The main change is that bindgen as a build-script library and bindgen as a command line utility have been split into the bindgen and bindgen-cli crates respectively.

This means that bindgen no longer depends on clap. Transforming it's cargo tree from this:

bindgen v0.60.1 (/home/christian/Workspace/bindgen/rust-bindgen)
├── bitflags v1.3.2
├── cexpr v0.6.0
│   └── nom v7.0.0
│       ├── memchr v2.5.0
│       └── minimal-lexical v0.1.4
│       [build-dependencies]
│       └── version_check v0.9.3
├── clang-sys v1.3.3
│   ├── glob v0.3.0
│   ├── libc v0.2.126
│   └── libloading v0.7.0
│       └── cfg-if v1.0.0
│   [build-dependencies]
│   └── glob v0.3.0
├── clap v3.2.12
│   ├── atty v0.2.14
│   │   └── libc v0.2.126
│   ├── bitflags v1.3.2
│   ├── clap_lex v0.2.4
│   │   └── os_str_bytes v6.2.0
│   ├── indexmap v1.9.1
│   │   └── hashbrown v0.12.2
│   │   [build-dependencies]
│   │   └── autocfg v1.1.0
│   ├── strsim v0.10.0
│   ├── termcolor v1.1.3
│   └── textwrap v0.15.0
├── env_logger v0.9.0
│   ├── atty v0.2.14 (*)
│   ├── humantime v2.1.0
│   ├── log v0.4.14
│   │   └── cfg-if v1.0.0
│   ├── regex v1.5.5
│   │   ├── aho-corasick v0.7.18
│   │   │   └── memchr v2.5.0
│   │   ├── memchr v2.5.0
│   │   └── regex-syntax v0.6.25
│   └── termcolor v1.1.3
├── lazy_static v1.4.0
├── lazycell v1.3.0
├── log v0.4.14 (*)
├── peeking_take_while v0.1.2
├── proc-macro2 v1.0.43
│   └── unicode-ident v1.0.3
├── quote v1.0.9
│   └── proc-macro2 v1.0.43 (*)
├── regex v1.5.5 (*)
├── rustc-hash v1.1.0
├── shlex v1.0.0
├── syn v1.0.99
│   ├── proc-macro2 v1.0.43 (*)
│   ├── quote v1.0.9 (*)
│   └── unicode-ident v1.0.3
└── which v4.2.2
    ├── either v1.6.1
    └── libc v0.2.126
[dev-dependencies]
├── clap v3.2.12 (*)
├── diff v0.1.12
├── shlex v1.0.0
└── tempfile v3.2.0
    ├── cfg-if v1.0.0
    ├── libc v0.2.126
    ├── rand v0.8.4
    │   ├── libc v0.2.126
    │   ├── rand_chacha v0.3.1
    │   │   ├── ppv-lite86 v0.2.10
    │   │   └── rand_core v0.6.3
    │   │       └── getrandom v0.2.3
    │   │           ├── cfg-if v1.0.0
    │   │           └── libc v0.2.126
    │   └── rand_core v0.6.3 (*)
    └── remove_dir_all v0.5.3

to this:

bindgen v0.60.1 (/home/christian/Workspace/bindgen/rust-bindgen/bindgen)
├── bitflags v1.3.2
├── cexpr v0.6.0
│   └── nom v7.0.0
│       ├── memchr v2.5.0
│       └── minimal-lexical v0.1.4
│       [build-dependencies]
│       └── version_check v0.9.3
├── clang-sys v1.3.3
│   ├── glob v0.3.0
│   ├── libc v0.2.126
│   └── libloading v0.7.0
│       └── cfg-if v1.0.0
│   [build-dependencies]
│   └── glob v0.3.0
├── lazy_static v1.4.0
├── lazycell v1.3.0
├── log v0.4.14
│   └── cfg-if v1.0.0
├── peeking_take_while v0.1.2
├── proc-macro2 v1.0.43
│   └── unicode-ident v1.0.3
├── quote v1.0.9
│   └── proc-macro2 v1.0.43 (*)
├── regex v1.5.5
│   └── regex-syntax v0.6.25
├── rustc-hash v1.1.0
├── shlex v1.0.0
├── syn v1.0.99
│   ├── proc-macro2 v1.0.43 (*)
│   ├── quote v1.0.9 (*)
│   └── unicode-ident v1.0.3
└── which v4.2.2
    ├── either v1.6.1
    └── libc v0.2.126

There are some questions that I'd like to resolve before moving forward with this:

Is CI working correctly? There are many test crates like quickcheck and expect but I'm not sure if my changes to the ci scripts are actually running everything they should.
How do we distribute bindgen-cli? RIght now you can test cargo install running the command
```
cargo install --git=https://github.com/ferrous-systems/rust-bindgen --branch=bindgen-as-workspace bindgen-cli
```
Which installs a bindgen-cli binary instead of just bindgen. I haven't found out how to change the name of the binary back to just bindgen.

bors-servo · 2022-09-27T19:14:55Z

☔ The latest upstream changes (presumably #2282) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2022-09-29T02:39:02Z

☔ The latest upstream changes (presumably #2287) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2022-10-02T23:43:30Z

☔ The latest upstream changes (presumably 8b69ba0) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2022-10-03T16:20:25Z

☔ The latest upstream changes (presumably #2285) made this pull request unmergeable. Please resolve the merge conflicts.

emilio · 2022-10-04T15:19:02Z

This looks great, though it'd be great to not change the name of the CLI tool. It seems something like this would do?

diff --git a/bindgen-cli/Cargo.toml b/bindgen-cli/Cargo.toml
index 2aef9b5b..ebeceacc 100644
--- a/bindgen-cli/Cargo.toml
+++ b/bindgen-cli/Cargo.toml
@@ -21,6 +21,10 @@ include = [
   "build.rs",
 ]
 
+[[bin]]
+path = "src/main.rs"
+name = "bindgen"
+
 [badges]
 travis-ci = { repository = "rust-lang/rust-bindgen" }

Also, we need to update the docs that mention cargo install and so on.

emilio

Other thoughts:

Maybe we can flatten the directory structure a bit by using Cargo.toml magic (e.g. don't require bindgen/src, bindgen-cli/src and so on).
It seems some of the test scripts need a bit of work.
The naming of the cli crate I believe should probably be kept, I think what I posted earlier should work but lmk if it doesn't for any reason.

Other than that, this looks pretty good!

bindgen-tests/Cargo.toml

bindgen-cli/build.rs

bindgen-integration/build.rs

emilio · 2022-10-04T15:21:43Z

bindgen-tests/tests/expectations/struct_with_anon_struct_array_float.rs

@@ -0,0 +1 @@
+


this seems to be a merge conflict casualty

It is not 🙃 the file is just there since 2017: https://github.com/rust-lang/rust-bindgen/blob/a900f8f863d1313ad76603234aaeea22bb9ba7b3/tests/expectations/struct_with_anon_struct_array_float.rs

Apparently the file was created here and it was empty from the start. I'll remove it in an upcoming PR: fitzgen@b4895d9

ci/test.sh

pvdrz · 2022-10-04T15:53:01Z

@emilio the [[bin]] section on the manifest worked. You still have to install bindgen-cli but get a bindgen executable.

emilio

Let's get this merged sooner rather than later to avoid all the conflicts.

Can you squash your commits before landing please? Thank you for doing this!

emilio

A couple last minute comments.

bindgen-tests/Cargo.toml

bindgen-cli/Cargo.toml

remove `clap` dependency 🎉 update the book installation instructions

pvdrz · 2022-10-05T02:26:21Z

Hi everyone! If you are reading this message because this PR broke yours, I'm sorry 😬. For most of the cases it is enough to rebase against master as there won't be any conflicts, just files being moved here and there

Update bindgen to v0.61.0. See: - rust-lang/rust-bindgen#2284

Update bindgen to v0.61.0. See: - rust-lang/rust-bindgen#2284 - artichoke/docker-artichoke-nightly#127

In LLVM 16, anonymous items may return names like `(unnamed union at ..)` rather than empty names [1], which breaks Rust-enabled builds because bindgen assumed an empty name instead of detecting them via `clang_Cursor_isAnonymous` [2]: $ make rustdoc LLVM=1 CLIPPY=1 -j$(nproc) RUSTC L rust/core.o BINDGEN rust/bindings/bindings_generated.rs BINDGEN rust/bindings/bindings_helpers_generated.rs BINDGEN rust/uapi/uapi_generated.rs thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9 ... thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9 ... This was fixed in bindgen 0.62.0. Therefore, upgrade bindgen to a more recent version, 0.65.1, to support LLVM 16. Since bindgen 0.58.0 changed the `--{white,black}list-*` flags to `--{allow,block}list-*` [3], update them on our side too. In addition, bindgen 0.61.0 moved its CLI utility into a binary crate called `bindgen-cli` [4]. Thus update the installation command in the Quick Start guide. Moreover, bindgen 0.61.0 changed the default functionality to bind `size_t` to `usize` [5] and added the `--no-size_t-is-usize` flag to not bind `size_t` as `usize`. Then bindgen 0.65.0 removed the `--size_t-is-usize` flag [6]. Thus stop passing the flag to bindgen. Finally, bindgen 0.61.0 added support for the `noreturn` attribute (in its different forms) [7]. Thus remove the infinite loop in our Rust panic handler after calling `BUG()`, since bindgen now correctly generates a `BUG()` binding that returns `!` instead of `()`. Link: llvm/llvm-project@19e984e [1] Link: rust-lang/rust-bindgen#2319 [2] Link: rust-lang/rust-bindgen#1990 [3] Link: rust-lang/rust-bindgen#2284 [4] Link: rust-lang/rust-bindgen@cc78b6f [5] Link: rust-lang/rust-bindgen#2408 [6] Link: rust-lang/rust-bindgen#2094 [7] Signed-off-by: Aakash Sen Sharma <aakashsensharma@gmail.com> Closes: #1013 Tested-by: Ariel Miculas <amiculas@cisco.com> Reviewed-by: Gary Guo <gary@garyguo.net> Link: https://lore.kernel.org/r/20230612194311.24826-1-aakashsensharma@gmail.com [ Reworded commit message. Mentioned the `bindgen-cli` binary crate change, linked to it and updated the Quick Start guide. Re-added a deleted "as" word in a code comment and reflowed comment to respect the maximum length. ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

bindgen 0.65.1 will be the next version supported by the kernel [1]. Therefore, add it to the latest build environment. This also requires updating the name of the package from `bindgen` to `bindgen-cli`, since bindgen 0.61.0 moved its CLI utility into a binary crate called `bindgen-cli` [2]. Link: Rust-for-Linux/linux@08ab786 [1] Link: rust-lang/rust-bindgen#2284 [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

In LLVM 16, anonymous items may return names like `(unnamed union at ..)` rather than empty names [1], which breaks Rust-enabled builds because bindgen assumed an empty name instead of detecting them via `clang_Cursor_isAnonymous` [2]: $ make rustdoc LLVM=1 CLIPPY=1 -j$(nproc) RUSTC L rust/core.o BINDGEN rust/bindings/bindings_generated.rs BINDGEN rust/bindings/bindings_helpers_generated.rs BINDGEN rust/uapi/uapi_generated.rs thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9 ... thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9 ... This was fixed in bindgen 0.62.0. Therefore, upgrade bindgen to a more recent version, 0.65.1, to support LLVM 16. Since bindgen 0.58.0 changed the `--{white,black}list-*` flags to `--{allow,block}list-*` [3], update them on our side too. In addition, bindgen 0.61.0 moved its CLI utility into a binary crate called `bindgen-cli` [4]. Thus update the installation command in the Quick Start guide. Moreover, bindgen 0.61.0 changed the default functionality to bind `size_t` to `usize` [5] and added the `--no-size_t-is-usize` flag to not bind `size_t` as `usize`. Then bindgen 0.65.0 removed the `--size_t-is-usize` flag [6]. Thus stop passing the flag to bindgen. Finally, bindgen 0.61.0 added support for the `noreturn` attribute (in its different forms) [7]. Thus remove the infinite loop in our Rust panic handler after calling `BUG()`, since bindgen now correctly generates a `BUG()` binding that returns `!` instead of `()`. Link: llvm/llvm-project@19e984e [1] Link: rust-lang/rust-bindgen#2319 [2] Link: rust-lang/rust-bindgen#1990 [3] Link: rust-lang/rust-bindgen#2284 [4] Link: rust-lang/rust-bindgen@cc78b6f [5] Link: rust-lang/rust-bindgen#2408 [6] Link: rust-lang/rust-bindgen#2094 [7] Signed-off-by: Aakash Sen Sharma <aakashsensharma@gmail.com> Closes: Rust-for-Linux#1013 Tested-by: Ariel Miculas <amiculas@cisco.com> Reviewed-by: Gary Guo <gary@garyguo.net> Link: https://lore.kernel.org/r/20230612194311.24826-1-aakashsensharma@gmail.com [ Reworded commit message. Mentioned the `bindgen-cli` binary crate change, linked to it and updated the Quick Start guide. Re-added a deleted "as" word in a code comment and reflowed comment to respect the maximum length. ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

pvdrz force-pushed the bindgen-as-workspace branch from ca31321 to 35aa2a2 Compare September 27, 2022 18:25

pvdrz requested review from amanjeev, emilio and kulp and removed request for amanjeev and kulp September 27, 2022 18:53

pvdrz changed the title ~~[WIP] Bindgen as a workspace~~ Bindgen as a workspace Sep 27, 2022

pvdrz force-pushed the bindgen-as-workspace branch 2 times, most recently from a5ee75e to 1130dd7 Compare September 27, 2022 19:39

pvdrz force-pushed the bindgen-as-workspace branch from 1130dd7 to ed90466 Compare September 29, 2022 18:52

pvdrz force-pushed the bindgen-as-workspace branch from ed90466 to fa0c34a Compare October 3, 2022 14:57

pvdrz force-pushed the bindgen-as-workspace branch from fa0c34a to 384aec2 Compare October 3, 2022 19:24

emilio requested changes Oct 4, 2022

View reviewed changes

pvdrz force-pushed the bindgen-as-workspace branch from 384aec2 to b231d7e Compare October 4, 2022 15:50

pvdrz requested a review from emilio October 4, 2022 16:23

pvdrz mentioned this pull request Oct 4, 2022

Disable feature logging and clap on default #2238

Closed

emilio approved these changes Oct 5, 2022

View reviewed changes

emilio reviewed Oct 5, 2022

View reviewed changes

bindgen-tests/Cargo.toml Show resolved Hide resolved

bindgen-tests/Cargo.toml Outdated Show resolved Hide resolved

bindgen-cli/Cargo.toml Outdated Show resolved Hide resolved

split the repo into a workspace

0296f9e

remove `clap` dependency 🎉 update the book installation instructions

pvdrz force-pushed the bindgen-as-workspace branch from 8a87299 to 0296f9e Compare October 5, 2022 01:47

pvdrz merged commit 73ce4bc into rust-lang:master Oct 5, 2022

pvdrz deleted the bindgen-as-workspace branch October 5, 2022 01:56

This was referenced Oct 17, 2022

Turn bin/lib into subcrates within workspace? #1970

Closed

bindgen v0.61.0 has no binaries rust-lang/cargo#11249

Closed

lopopolo added a commit to artichoke/docker-artichoke-nightly that referenced this pull request Oct 22, 2022

Install bindgen from bindgen-cli package

060d77c

Update bindgen to v0.61.0. See: - rust-lang/rust-bindgen#2284

lopopolo mentioned this pull request Oct 22, 2022

Install bindgen from bindgen-cli package artichoke/docker-artichoke-nightly#127

Merged

lopopolo added a commit to artichoke/artichoke that referenced this pull request Oct 22, 2022

Install bindgen from bindgen-cli package

f4cb202

Update bindgen to v0.61.0. See: - rust-lang/rust-bindgen#2284 - artichoke/docker-artichoke-nightly#127

lopopolo mentioned this pull request Oct 22, 2022

Install bindgen from bindgen-cli package artichoke/artichoke#2222

Merged

michaellass mentioned this pull request Oct 25, 2022

Disable bindgen default features michaellass/e2p-sys#3

Merged

kgrech mentioned this pull request Nov 22, 2022

Update dependencies Absolucy/tracing-oslog#3

Merged

kulp mentioned this pull request Nov 20, 2024

Move CLI options to bindgen #2983

Merged

Bindgen as a workspace #2284

Bindgen as a workspace #2284

Uh oh!

Conversation

pvdrz commented Sep 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors-servo commented Sep 27, 2022

Uh oh!

bors-servo commented Sep 29, 2022

Uh oh!

bors-servo commented Oct 2, 2022

Uh oh!

bors-servo commented Oct 3, 2022

Uh oh!

emilio commented Oct 4, 2022

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilio Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

pvdrz Oct 4, 2022

Choose a reason for hiding this comment

Uh oh!

pvdrz Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvdrz Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pvdrz commented Oct 4, 2022

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

emilio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pvdrz commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pvdrz commented Sep 24, 2022 •

edited

Loading

pvdrz Oct 4, 2022 •

edited

Loading

pvdrz Oct 5, 2022 •

edited

Loading

pvdrz commented Oct 5, 2022 •

edited

Loading