Skip to content

feat(dyngen): preconvert path to OsStr when opening the library - #3315

Merged
emilio merged 1 commit into
rust-lang:mainfrom
nagisa:libloading-compat
Nov 10, 2025
Merged

emilio merged 1 commit into
rust-lang:mainfrom
nagisa:libloading-compat

Conversation

@nagisa

@nagisa nagisa commented Nov 10, 2025 •

Copy link
Copy Markdown
Member

In libloading 0.9 the argument bound changed from AsRef<OsStr> to libloading::AsFilename. This was necessary in order to enable compilation of libloading for usecases that cannot depend on libstd. libloading::AsFilename supports most of the same types, but cannot be implemented generally for any T: AsRef<OsStr> due to the orphan rule.

This makes the generated code support both 0.8 and 0.9 versions of libloading transparently, and at the same time it also reduces the number of different instantiations of Library::new.

When bindgen is adjusted to work with libloading 0.9 only, it will be worthwhile to adjust the generation of from_library to use c"string" literals over b"string\0" ones as those can forgo any runtime checking of the argument(s). However we can't do it right while libloading 0.8 is still supported.

In libloading 0.9 the argument bound changed from `AsRef<OsStr>` to
`libloading::AsFilename`. This was necessary in order to enable
compilation of libloading for usecases that cannot depend on libstd.
`libloading::AsFilename` supports most of the same types, but cannot be
implemented generally for any `T: AsRef<OsStr>` due to the orphan rule.

This makes the generated code support both 0.8 and 0.9 versions of
libloading transparently, and at the same time it also reduces the
number of different instantiations of `Library::new`.

When bindgen is adjusted to work with libloading 0.9 only it will be
worthwhile to adjust the generation of `from_library` to use `c"string"`
literals over `b"string\0"` ones as those can forgo any runtime checking
of the argument(s). However we can't do it right while libloading 0.8 is
still supported.
@emilio
emilio added this pull request to the merge queue Nov 10, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 10, 2025
@emilio
emilio added this pull request to the merge queue Nov 10, 2025
@emilio

emilio commented Nov 10, 2025

Copy link
Copy Markdown
Contributor

I think the failure was spurious, trying again.

Merged via the queue into rust-lang:main with commit 5813198 Nov 10, 2025
51 checks passed
@WardBrian

Copy link
Copy Markdown
Contributor

Hi -- does bindgen follow a set release cycle? It would be great if this could show up on crates.rs to unblock updates to libloading

@nagisa
nagisa deleted the libloading-compat branch November 24, 2025 08:29
Umio-Yasuno added a commit to Umio-Yasuno/libdrm-amdgpu-sys-rs that referenced this pull request Jan 17, 2026
okayyoumay added a commit to okayyoumay/nekoguruma that referenced this pull request Oct 8, 2026
…3.2 (#79)

## Summary

Before: both `-sys` crates use `libloading` 0.8 with bindings generated
by bindgen 0.72.1. Dependabot's bump to `libloading` 0.9 (#77) does not
compile: 0.9's `Library::new` takes an `AsFilename` path, and the
generated loader passes its generic `P: AsRef<OsStr>` straight through.

After: both crates use `libloading` 0.9. The bindings are regenerated
with bindgen 0.73.2, whose loader first converts the path with
`as_ref()` (rust-lang/rust-bindgen#3315). `&OsStr` is accepted by 0.9.
This replaces #77, which can be closed once this merges.

## Changes

- `iso22900-sys`, `j2534-0404-sys`: `libloading` 0.8.0 -> 0.9.0,
build-dependency `bindgen` 0.72.1 -> 0.73.2.
- All 18 committed binding files (9 targets x 2 crates) regenerated with
`--features bindgen`. Each changes in two lines only: the generator
version comment and `let path = path.as_ref();` in the loader.
- `j2534-0404-sys/src/abi.rs`: the hand-written `Api::new` loader calls
`Library::new(path.as_ref())`.
- `Cargo.lock`: bindgen 0.73.2 accepts `syn` 2 or 3, and Cargo picked 3
(already in the graph through `serde_derive` and `async-trait`). But
bindgen hands its `syn` tree to `prettyplease` 0.2, which is built on
`syn` 2 (and `prost-build` keeps `prettyplease` at 0.2), so the
`bindgen` feature failed to compile. The lockfile now resolves bindgen's
`syn` to 2.0.119; `cargo check --locked` accepts it. The normal build
uses the committed files and does not build bindgen.
- `docs/worker-crates.md`: the regeneration notes now cover cross Linux
sysroots (`BINDGEN_EXTRA_CLANG_ARGS_<target>=--sysroot=...` with the
`libc6-dev-*-cross` packages) and the `syn` 2 pin.

## Documentation

- [x] `README.md` workspace table / `docs/worker-crates.md`
(regeneration notes)

## Checks

- [x] No standard text is copied anywhere in this PR, including commit
messages and this description.
- [x] No secrets or private infrastructure details.
- [x] Generated code: FFI bindings regenerated with the build script
(not hand-edited) for every committed target; the maintainer approved
the regeneration in the project thread.
- [x] Write, flash and routine-control jobs: N/A.

## Test plan

- [x] `cargo fmt --all -- --check` and `cargo clippy --workspace
--all-targets --locked` clean locally
- [x] `scripts/check-work-refs.sh`, `scripts/check-adr-index.sh`,
`scripts/check-backlog.sh` pass
- [x] `cargo check --workspace --all-targets --locked`; `cargo test` for
`iso22900-sys`, `j2534-0404-sys`, `iso22900`, `j2534-0404` pass
- [ ] CI green (worker-check and abi-roundtrip load real libraries
through the new loader)
- [x] Other: `git diff` of the 18 binding files contains only the two
lines named above, 18 times each

## Open items

None

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_013y25opBEPUVRoGagjhCZNF

---
_Generated by [Claude
Code](https://claude.ai/code/session_013y25opBEPUVRoGagjhCZNF)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants