Exclude test DLL's from published package

[weiznich]

During a dependency review we noticed that the libloading crate includes various dll files used for testing. These shouldn't be there as building the crate itself doesn't require these files. As of now they prevent any downstream user from enabling the [bans.build.executable option] option of cargo deny.

I opted for using an explicit include list instead of an exclude list to prevent these files from being included in the published packages to make sure that everything that's included is an conscious choice.

[nagisa]

Thanks. On one hand this makes some sense, on the other hand this presents a problem that the crate is no longer self-sufficient for development anymore, which can matter in e.g. vendoring scenarios.

Picking breadcrumbs it may now be possible to compile the relevant test files from rust (ref.

rust_libloading/tests/windows.rs

Lines 6 to 14 in dab97c5

	// The ordinal DLL contains exactly one function (other than DllMain, that is) with ordinal number
	// 1. This function has the sugnature `fn() -> *const c_char` and returns a string "bunny\0" (in
	// reference to WindowsBunny).
	//
	// Both x86_64 and x86 versions of the .dll are functionally the same. Ideally we would compile the
	// dlls with well known ordinals from our own testing helpers library, but rustc does not allow
	// specifying a custom .def file (https://github.com/rust-lang/rust/issues/35089)
	//
	// The DLLs were kindly compiled by WindowsBunny (aka. @retep998).

and rust-lang/rust#35089) but somebody will have to experiment with it...

[weiznich]

I pushed 23f9e31 which introduces a new windows_test_dll crate (name up to discussion). This crate is added as dev-dependency to libloading and builds the relevant DLL files.

Now I do not have access to a windows machine for development so I "only" run tests via cargo xwin + wine but everything seems to work there. At least the tests are passing. So I'm hopeful that this should also work on real windows machines.

As a side note: I did not add anything about ordinals or def files as mentioned in the comment, but the test still worked. So maybe rustc already generates a for the test good result there?

[nagisa]

There's already a test helpers that we could use here with all the relevant compilation on demand infrastructure present: https://github.com/nagisa/rust_libloading/blob/master/src/test_helpers.rs and

rust_libloading/tests/functions.rs

Lines 16 to 39 in dab97c5

	fn make_helpers() {
	static ONCE: ::std::sync::Once = ::std::sync::Once::new();
	ONCE.call_once(|| {
	if std::env::var_os("PRECOMPILED_TEST_HELPER").is_some() {
	//I can't be asked to make rustc work in wine.
	//I can call it myself from my linux host and then just move the file here this allows me to skip this.
	eprintln!("WILL NOT COMPILE TEST HELPERS, PROGRAM WILL ASSUME THAT {} EXISTS AND WAS EXTERNALLY PRE COMPILED", lib_path().display());
	return;
	}
	let rustc = std::env::var_os("RUSTC").unwrap_or_else(|| "rustc".into());
	let mut cmd = ::std::process::Command::new(rustc);
	cmd.arg("src/test_helpers.rs").arg("-o").arg(lib_path());
	if let Some(target) = std::env::var_os("TARGET") {
	cmd.arg("--target").arg(target);
	} else {
	eprintln!("WARNING: $TARGET NOT SPECIFIED! BUILDING HELPER MODULE FOR NATIVE TARGET.");
	}
	assert!(cmd
	.status()
	.expect("could not compile the test helpers!")
	.success());
	});
	}

.

While I wouldn't be surprised if ordinals get assigned by default nowadays, I'm not quite sure if the order of those ordinals is going to be defined. IMO we should still specify a .def file, but that should actually be quite straightforward with just a -Clink-arg=foo.def with the def file looking probably something like

LIBRARY   test_helpers
EXPORTS
   test   @1

Could you try doing it this way and seeing if it works?

[weiznich]

Should be ready now. CI also passed in the fork

[nagisa]

This is amazing, thank you!

[nagisa]

This is no longer necessary, right?