search

mozilla / cbindgen

A project for generating C bindings from Rust code
Mozilla Public License 2.0
2.37k stars 306 forks source link

Enforce usage of nightly toolchain for macro expansion #813

Closed doxblek closed 1 year ago

doxblek commented 1 year ago

resolves https://github.com/eqrion/cbindgen/issues/812

See the attached issue

jschwe commented 1 year ago

I honestly don't think tools like cbindgen should go ahead and install / update toolchains. I think providing a nice error message if the nightly toolchain is not used and/or not installed would be preferable. (But I'm not a maintainer here, just someone passing by )

doxblek commented 1 year ago

I honestly don't think tools like cbindgen should go ahead and install / update toolchains. I think providing a nice error message if the nightly toolchain is not used and/or not installed would be preferable. (But I'm not a maintainer here, just someone passing by )

I think you are right. Probably it is better to switch to the nightly toolchain but not installing it automatically if it is missing. Along with a descriptive error message in case the toolchain is not installed to guide the user to install it on their own, this might be a good solution.

Could any maintainer give me feedback on that? Then I would update my PR accordingly

emilio commented 1 year ago

Yeah, also assuming rustup exists is ok for dev machines, but not more generally. I think using nightly needs to be an explicit action on the user, so I think the code as-is is probably fine.

You can trivially use:

CARGO=$(rustup +nightly which cargo) cbindgen
jcaden commented 1 year ago

I've submitted an alternative to this PR #818

emilio commented 1 year ago

Yeah, I don't think we want either. There are various ways of making -Zpretty=expanded but we don't want to make people accidentally rely on them. This can be done on the caller quite easily if they want.