search

nagisa / rust_libloading

Bindings around the platform's dynamic library loading primitives with greatly improved memory safety.
https://docs.rs/libloading
ISC License
1.24k stars 102 forks source link

Honor Cargo's target directory when building LIBPATH #100

Closed jamessan closed 2 years ago

jamessan commented 2 years ago

Running the tests with “cargo test --target-dir ” fails because "target/" doesn't exist.

Rust 1.57.0 added $CARGO_TARGET_TMPDIR as a scratch directory for integration tests to use, which Cargo itself won't touch.

Therefore, use $CARGO_TARGET_TMPDIR if it's available, $CARGO_TARGET_DIR otherwise, and fallback to "target" as a last resort.

nagisa commented 2 years ago

Hm, libloading has a MSRV preceding 1.57 by a lot. If we wanted to support running tests with --target-dir, wouldn't it be more straightforward for the test to create the target/ directory if it doesn't yet exist? target/ is .gitignored and it is unclear to me it would be a huge deal if $PWD/target was created by a test even if --target-dir flag was set.

jamessan commented 2 years ago

I only mentioned 1.57 as the reason for honoring $CARGO_TARGET_TEMPDIR. It will still work with versions prior to that. However, $CARGO_TARGET_DIR has existed for much longer (since Cargo 0.28.0).

wouldn't it be more straightforward for the test to create the target/ directory if it doesn't yet exist?

That won't work if the reason --target-dir is being used is because the source tree is read-only.

jamessan commented 2 years ago

I only mentioned 1.57 as the reason for honoring $CARGO_TARGET_TEMPDIR. It will still work with versions prior to that. However, $CARGO_TARGET_DIR has existed for much longer (since Cargo 0.28.0).

wouldn't it be more straightforward for the test to create the target/ directory if it doesn't yet exist?

That won't work if the reason --target-dir is being used is because the source tree is read-only.

Does this satisfy your concerns, @nagisa?

nagisa commented 2 years ago

Thanks, merged in https://github.com/nagisa/rust_libloading/commit/8009f51a9fc95c06d34cf12cb3897791ecd9dc92

Sorry for the delay ^^