Remove binary packaging support

[MarijnS95]

Cross-compiling a Windows-rs project from Linux (ie. one of the test crates within this repo) result in the following build error:

$ cargo b -p test_bstr --target x86_64-pc-windows-gnu
...
error: proc macro panicked
 --> tests/bstr/build.rs:4:5
  |
4 | /     windows::build! {
5 | |         Windows::Win32::Foundation::BSTR,
6 | |     };
  | |______^
  |
  = help: message: Path not ending in `;`

The PATH variable on Linux is separated with colons, not semicolons.

It is worrying to see the same undocumented logic and error duplicated across three distinct places in the codebase. What is the first path in PATH supposed to point to? Why are the winmd files copied to ../../.windows/winmd relative to it? Do I need to worry about the bit that copies to %PROFILE%, too?

On a related note, is it worth turning the cargo build for ubuntu-latest in the CI into cargo check --all --target x86_64-pc-windows-gnu, to ensure cross-compiling remains in working order?

[tim-weis]

This issue appears to be causing the windows-app docs build to fail. Though I'm not a big fan of sinking too much time into getting cross-compilation to work, this is probably something that would need to be addressed, on way or another.

[MarijnS95]

@tim-weis It appears these PATH changes (and the 5-or-so yanked releases with it) were made to accommodate broken docs.rs changes in the first place; curious to hear what it's all about (I hope the first path isn't pointing to system32) and how we can address this uniformly.

FWIW I have previously been told that cross-compiling support is a big deal for windows-rs (unfortunately natively using autogenerated COM bindings on Linux is not :|), and am personally rolling it out to quite a few projects as it is - especially on GH Actions - several orders of magnitude faster than natively compiling Rust projects on Windows.

[kennykerr]

This is a know issue. Would appreciate any suggestions as I've not found a satisfactory way to deal with problem. This started with the introduction of packaging support where crates like windows-app need to provide binaries to dependent crates. The difficulty is in determining the target directory so that such crates can copy their binaries to a location where the dependent crates can make use of them.

I have described this problem in more detail here and why the PATH environment variable is being used. I certainly don't like the current solution but haven't found anything better. And yes, it means that the windows-app docs are indefinitely broken.

I burned through a few published crate versions testing packaging support because Cargo behaves differently depending on whether dependencies are published or not and without a reliable way to find the target directory it became quite a problem to test reliably.

[kennykerr]

The PATH variable on Linux is separated with colons, not semicolons.

If the target directory is in the same location on Linux then perhaps that would at least solve the docs and cross-compilation issue.

[MarijnS95]

@kennykerr That seems like an exceptionally annoying problem to have. Thinking out loud, let's go over the call-sites:

crates/macros/src/lib.rs seems to be the most complex, but ultimately results in copying .windows/{target_arch} to the target directory (as well as winmds provided by dependencies). On Windows, DLLs are loaded relative to the started binary. On Linux however, they are loaded relative to the working directory (disregarding standard search paths and RPATH logic). We can possibly just disable this codepath, simply bailing if %PATH% doesn't contain anything satisfactory (after all, this crate is not supposed to be built, or be building anything, for Linux).

Either way, I have not come across a crate that helps dependent crates copy extra libraries to the target folder, especially when that dependent (directly built) crate provides the libraries itself. It should probably have that as its own build step depending on how the crate is debugged and packaged (windows-app-rs being the prime example here).

Then, crates/gen/build.rs on cfg(windows) copies winmd files from crates/gen/.windows/winmd to the target directory, for crates/gen/src/workspace.rs to pick them up. Afaik this was working just fine for dependent crates before, what changed?

The PATH variable on Linux is separated with colons, not semicolons.

If the target directory is in the same location on Linux then perhaps that would at least solve the docs and cross-compilation issue.

The deps folder is not added to PATH anywhere on Linux.

[kennykerr]

Afaik this was working just fine for dependent crates before, what changed?

Previously this worked because the gen crate used include_bytes! rather than packaged files directly. This worked well enough (ignoring the fact that its really slow) for the windows crate (since the build macro knew about it) but not for other crates with binary dependencies. The build! macro needs to be able to discover these other binary dependencies so that it can index any potential winmd files.

[MarijnS95]

Agreed include_bytes! is far from ideal. Perhaps CARGO_MANIFEST_DIR can be used without copying anything, though? As per the documentation this is available in build.rs (where build! is called) and always points to the manifest directory of the current crate being built, not the crate that uses it as a dependency. In theory it should be possible to:

1. Scan "custom" winmd files in %CARGO_MANIFEST_DIR%/.windows/winmd, for dependent crates;
2. Forward CARGO_MANIFEST_DIR from a build.rs in windows_gen to the built binary, so that it can find the base crates/gen/.windows/winmd files provided by this repository;

Without providing any copy-logic or PATH magic whatsoever. That leaves only the "dll copy" issue to be resolved, which IMO doesn't seem the responsibility of windows-rs.

However, I think you have already explored this solution?

EDIT: FWIW CARGO_MANIFEST_DIR is currently already used in the way described above, just with an intermediary copy to a target folder instead of passing the path to the winmd directly.

[MarijnS95]

master...MarijnS95:winmds @kennykerr Would something like this work for you?

[riverar]

Does this change affect consumers of the windows crate, in that they are now responsible for .windows deployment? That's unacceptable if so.

[MarijnS95]

@riverar not at all. Winmd from win32metadata is still provided by the windows_gen crate, and crate users are still responsible for providing their own winmd and dlls for "external" bindings that are not within the windows SDK as provided by win32metadata.

[riverar]

OK but unless I'm misreading the changes (very possible), it looks like all the logic around copying files out of .windows was removed and that's now a task every single developer has to worry about?

[kennykerr]

Yes, its important to keep the different scenarios in mind. There are basically three.

1. Crate (A) only uses the default metadata provided by the Windows crate (B) in order to call in-box APIs. No problem.

2. Crate (A) also provides its own or additional APIs via a .windows folder local to crate A using the windows::build! macro. No problem again.

3. Crate (A) also depends on additional APIs provided by a third crate (C) where this other crate has a .windows folder. The windows::build! macro must find the .windows contents from crates A, B, and C. It is this three-crate scenario that is the problem. Other variations of this scenario may exist in future. A goal is to avoid having A write extra build macro goo to copy stuff from C.

Other non-Rust build systems make this possible by allowing shared target folders.

[MarijnS95]

OK but unless I'm misreading the changes (very possible), it looks like all the logic around copying files out of .windows was removed and that's now a task every single developer has to worry about?

The goal is to get rid of copies entirely, at least for .winmd files and replace that with passing down the path to these files. That way there's no ambiguity with files parsed in the wrong order (ie. to supersede "builtin" files from windows-rs with a newer version) or identical-named files being overwitten by the order cargo builds the crates in. Nor (the main reason) do we have to figure out what shared target/ folder to use.

dll copies to the target/ folder on the other hand are IMO - since windows-rs doesn't provide any such files anyway - up to the dependent crates, if they need it at all. Otherwise it might be more sensible to provide a helper function like windows::please_copy_dll_files_to_the_right_target_folder_for_me(".windows/").

Scenario 3

Scenario 3 is the one I don't want to encounter, at least not in the implicit way it is now. What happens if I add a crate D, that has nothing to do with B nor C but only wishes to pull things from the .winmd provided by A (windows-rs) and by chance itself?

Would you be opposed to make .winmd inheritance explicit? Crate B could, through the build! macro, reexport its own .windows/winmd path for crate C to use (or not!), or reexport a build! macro that takes care of the windmd's of crate B. Then again we'd be combating the copy-to-shared-target-folder magic, and scan all files in the right order once again - without overlapping names.

A shouldn't have to write extra macro goo to deal with stuff from C. C however, should - to be explicit - need an extra line to specify it wishes to take into account the winmds "exported" (or otherwise explicitly provided) by B (and recursively A). Then one can easily add diamond inheritance, say where C wants to read winmds from B and an imaginary crate E.

[MarijnS95]

Additionally, note that I don't expect crate A to ever see the winmds provided by B nor C; neither should B see the files from C.

[riverar]

dll copies to the target/ folder on the other hand are IMO - since windows-rs doesn't provide any such files anyway - up to the dependent crates, if they need it at all. Otherwise it might be more sensible to provide a helper function like windows::please_copy_dll_files_to_the_right_target_folder_for_me(".windows/").

The windows-app crate has dependencies that need to be deployed with the consuming crate's target app. Most Windows app developers will need to minimally author a fusion manifest (.exe.config), store it in .windows, and deploy it into the target folder post-build. Forcing every developer to write the code needed to copy those files, or explicitly call the same helper function, is not ideal.

Would you be opposed to make .winmd inheritance explicit?

Hm, any further details? Requiring developers to directly specify winmds for import (e.g. import Microsoft.Win32.winmd) means they would have to figure out what winmds a crate offers and figure out how those winmds correlate to the APIs they wish to use. I believe that'll be very confusing and awkward.

[MarijnS95]

dll copies to the target/ folder on the other hand are IMO - since windows-rs doesn't provide any such files anyway - up to the dependent crates, if they need it at all. Otherwise it might be more sensible to provide a helper function like windows::please_copy_dll_files_to_the_right_target_folder_for_me(".windows/").

The windows-app crate has dependencies that need to be deployed with the consuming crate's target app. Most Windows app developers will need to minimally author a fusion manifest (.exe.config), store it in .windows, and deploy it into the target folder post-build. Forcing every developer to write the code needed to copy those files, or explicitly call the same helper function, is not ideal.

This sounds like a feature specific to windows-app and should hence reside in that crate instead?

Either way I'm fine disabling this with #[cfg(windows)] when cross-compiling.

Would you be opposed to make .winmd inheritance explicit?

Hm, any further details? Requiring developers to directly specify winmds for import (e.g. import Microsoft.Win32.winmd) means they would have to figure out what winmds a crate offers and figure out how those winmds correlate to the APIs they wish to use. I believe that'll be very confusing and awkward.

Downstream crates shouldn't have to explicitly specify the winmds they wish to pull in, given that they would probably have to scour the source of their upstream crate to find out - that's confusing and awkward indeed! The name-ambiguity problem as outlined above remains, too.

Rather, downstream crates should specify - for example by means of calling the build! macro on a different crate/module (that is auto-generated) - of what crates they want to use their winmds. In Kenny's third example, crate A could call C::windows::build!, which is an autogenerated macro that passes its own $CARGO_MANIFEST_DIR/.windows/winmds path recursively into B::build!. That way some irrelevant windows crate D will call B::build! (B is this windows crate) without ever seeing the winmds provided by C. At the same time no copies are necessary, because the full path to all source folders containing winmds is known.

Let me know if this is clear enough. If not, I'll try to piece together a little example/test one way or another.

[kennykerr]

In practice, I don't think scenario 3 is going to be very common at all. It's just something we need for the windows-app crate and perhaps one or two others down the road. We'd just like it to work as smoothly as possible. But the vast majority of projects will be well served by scenarios 1 and 2. Ideally, Cargo itself can eventually pick up the slack and provide first-class support for binary packaging.

[MarijnS95]

@kennykerr While it'd be nice for cargo to pick this up it's still a highly specific scenario (collecting a [list of a] bunch of files to take into account during a build.rs step), that can be implemented in a multitude of ways. Being able to find the current target folder as linked before doesn't seem to provide that anyway, but I don't think that's the solution you're looking for either?

Anyway, what is your view on the proposed solution above? It'll get rid of the PATH and copy magic for the time being, enabling cross-compilation again. I doubt cargo shows up with a reasonable solution for this specific problem any time soon.

[kennykerr]

I've decided to just rip out the binary packaging support for now. Its not an urgently-needed feature right now and we can always revisit in future. The Windows crate will thus continue to support the first two scenarios above.

[MarijnS95]

@kennykerr So not just binary packaging support, but also recursively re-providing winmds during build/compilation to support scenario 3?

Anything I can help out with by rebasing and submitting the branch above?

[kennykerr]

I'm going camping so I won't get to this in the next week or so. All the scenario 3 code can disappear.

1. If the consuming crate/workspace doesn't have a .windows folder then the build macro simply uses the winmd files from the windows_gen crate.

2. If the consuming crate/workspace has a .windows folder then the contents of the .windows/<arch> folder (if any) is copied into the target_dir so that cargo run "just works" and the contents of the .windows/winmd folder (if any) is included in the TypeReader.

I'm happy to take care of this upon my return, but feel free to jump start the process if you're confident you know what's required. I just won't be around to spot check. @riverar may be able to help keep an eye on things. He's working on removing the need for this from the windows-app-rs repo.

[MarijnS95]

I'm going camping so I won't get to this in the next week or so. All the scenario 3 code can disappear.

Nice, enjoy and have fun on your week off!

1. If the consuming crate/workspace doesn't have a .windows folder then the build macro simply uses the winmd files from the windows_gen crate.

Yes, that makes the most sense.

2. If the consuming crate/workspace has a .windows folder then the contents of the .windows/<arch> folder (if any) is copied into the target_dir so that cargo run "just works" and the contents of the .windows/winmd folder (if any) is included in the TypeReader.

I thought we wanted to get rid of copying altogether? That shouldn't be necessary to support scenario 2 at all? My solution in master...MarijnS95:winmds simply passes the path from .windows/winmd relative to the currently built crate into the TypeReader, without any copies nor reading PATH.

I'm happy to take care of this upon my return, but feel free to jump start the process if you're confident you know what's required. I just won't be around to spot check.

I'll see what I can do, pending the above regarding removing copies altogether.

[kennykerr]

The winmd files don't need to be copied, but the <arch> files do for obvious reasons. Sorry if that wasn't clear. Thanks!

[MarijnS95]

The winmd files don't need to be copied, but the <arch> files do for obvious reasons. Sorry if that wasn't clear. Thanks!

Ah, I thought that copy behaviour was going to be moved into windows-app-rs. We can probably #[cfg] it for Windows, and leave all the PATH magic it has to perform out of cross-compilation builds until someone actively needs that and can test it.

[riverar]

Every crate dependent on the windows crate relies on files getting copied out of .windows\<arch> for a bag of reasons. No PATH hacks needed for that as that hierarchal winmd mess is getting canned for now. Back to the old days! 🥂

Ref:

windows-rs/crates/macros/src/lib.rs

Lines 89 to 102 in 49ecfbc

	for file in files.filter_map(|file| file.ok()) {
	if let ::std::result::Result::Ok(file_type) = file.file_type() {
	if file_type.is_dir() {
	let mut path = file.path();
	if let ::std::option::Option::Some(filename) = path.file_name() {
	if filename == profile {
	copy(source, &mut path);
	} else {
	copy_to_profile(source, &path, profile);
	}
	}
	}
	}
	}

[MarijnS95]

@riverar Doesn't .windows\<arch> contain specific DLLs for features that are not shipped by default on Windows? Those were all moved to https://github.com/microsoft/windows-samples-rs, and are anyway only used in win2d and webview2 so it doesn't seem like "every crate" needs this?

That bit of code you linked:

windows-rs/crates/macros/src/lib.rs

Line 121 in 49ecfbc

let mut destination : ::std::path::PathBuf = ::std::env::var("OUT_DIR").expect("No `OUT_DIR` env variable set").into();

At least it uses OUT_DIR, so no PATH hacks there 🎉

What's the plan, revert to the state in that commit and then rip out the copy for winmds:

windows-rs/crates/macros/src/lib.rs

Lines 140 to 144 in 49ecfbc

	destination.push(".windows");
	destination.push("winmd");
	source.pop();
	source.push("winmd");
	copy(&source, &mut destination);

In favour of propagating its path using something akin to master...MarijnS95:winmds?

I'm not in a hurry to get this cross-compilation fixed; if my approach isn't close to what you're planning I'll leave this up to you :)

[riverar]

@riverar Doesn't .windows\<arch> contain specific DLLs for features that are not shipped by default on Windows? Those were all moved to https://github.com/microsoft/windows-samples-rs, and are anyway only used in win2d and webview2 so it doesn't seem like "every crate" needs this?

Yep!

The samples are just Rust development samples and don't represent the Windows app ecosystem very well.

There are a zillion reasons to ship files side-by-side with an app, as you can imagine. Could be an application compatibility shim/hook, a localized string or image bucket, libavcodec.dll for video encoding, a manifest to enable access to newer controls and high-DPI support, etc. Nearly all modern Windows apps ship with files at their side.

To reduce developer friction and align with the Rust ecosystem's idiomatic cargo run experience, the crate sacrifices some purity and performs .windows deployment work for the developer.

Ideally, we'd move all this code elsewhere but I don't believe Cargo has any post-build hooks to latch onto. (Post-build scripts, lifecycle events, etc. are hot asks in the Cargo community/repository but I haven't seen any major RFC movement in that area.) So we're kind of stuck with what we got or "tell the developer to do it themselves / run a script", with the latter being antithetical to creating a great developer experience.

Open to other ideas!

What's the plan, revert to the state in that commit and then rip out the copy for winmds [...]

If you want to tackle this while Kenny is away, I would say the plan is: revert back to what we had, prior to the PATH/winmd hacks, and maintain the .windows deployment behavior (for now!). I believe this is generally referred to as "supporting scenario 1 and 2" earlier in the thread. That should fix/close out this issue.

[...] In favour of propagating its path using something akin to master...MarijnS95:winmds? [...]

The approach you had was great/solid but I don't think we need it now. The winmd dependency propagation logic is getting removed. The windows-app-rs crate will likely become a cargo-generate template instead.

Make sense? Did I miss anything?

[MarijnS95]

The samples are just Rust development samples and don't represent the Windows app ecosystem very well.

There are a zillion reasons to ship files side-by-side with an app, as you can imagine. Could be an application compatibility shim/hook, a localized string or image bucket, libavcodec.dll for video encoding, a manifest to enable access to newer controls and high-DPI support, etc. Nearly all modern Windows apps ship with files at their side.

To reduce developer friction and align with the Rust ecosystem's idiomatic cargo run experience, the crate sacrifices some purity and performs .windows deployment work for the developer.

Hmm, I had assumed there are also quite a few applications that wish to use windows-rs as a replacement for winapi just to access the many APIs that are available default, but catering to apps that need something extra doesn't hurt those that don't.

Ideally, we'd move all this code elsewhere but I don't believe Cargo has any post-build hooks to latch onto. (Post-build scripts, lifecycle events, etc. are hot asks in the Cargo community/repository but I haven't seen any major RFC movement in that area.) So we're kind of stuck with what we got or "tell the developer to do it themselves / run a script", with the latter being antithetical to creating a great developer experience.

Open to other ideas!

For now I'm relying on Linux where libraries are generally easily installable through a package manager and end up in well-known locations that are searched by linkers, instead of having build scripts copy them to the target folder to sit next to an executable (which is only a problem for the few custom libraries one does not install to the system), so I can't really comment on a proper solution. Again, it's fine to revert this to the old system without PATH magic so that it doesn't panic cross-compilation builds anymore.

What's the plan, revert to the state in that commit and then rip out the copy for winmds [...]

If you want to tackle this while Kenny is away, I would say the plan is: revert back to what we had, prior to the PATH/winmd hacks, and maintain the .windows deployment behavior (for now!). I believe this is generally referred to as "supporting scenario 1 and 2" earlier in the thread. That should fix/close out this issue.

I would have submitted that branch (with the deployment behaviour reverted to the old state instead of being removed), but won't be able to test anything on Windows so that might be weird (but we have CI 🎉). Also, see below:

[...] In favour of propagating its path using something akin to master...MarijnS95:winmds? [...]

The approach you had was great/solid but I don't think we need it now. The winmd dependency propagation logic is getting removed. The windows-app-rs crate will likely become a cargo-generate template instead.

This branch doesn't contain "the propagation logic" as discussed above, just "propagating" the path to <windows_gen>/.windows/winmds in such a way that the build macros can find it. I don't exactly remember why I did it this way, as it was working before. Will have to check how it was working before, afaik it was include_bytes which was found to be slow:

windows-rs/crates/gen/src/workspace.rs

Lines 83 to 94 in 8e7ce3d

	// TODO: include_bytes is very slow - it takes an extra 60ms compared with memory mapped files.
	// https://github.com/rust-lang/rust/issues/65818
	if !result.iter().any(|file| file.name.starts_with("Windows.")) {
	result.push(File::from_bytes(
	"Windows.Win32.winmd".to_string(),
	include_bytes!("../default/Windows.Win32.winmd").to_vec(),
	));
	result.push(File::from_bytes(
	"Windows.WinRT.winmd".to_string(),
	include_bytes!("../default/Windows.WinRT.winmd").to_vec(),
	));

If we don't want to go back to include_bytes there should be some way to acquire the path to windows_gen when it's sitting somewhere in ~/.cargo/registry/src/../windows_gen-xxx/ or ~/.cargo/git/...`, which that branch would provide. I guess it makes sense to submit (a cleaned version of) it after all and see where we end up?

[riverar]

If we don't want to go back to include_bytes there should be some way to acquire the path to windows_gen when it's sitting somewhere in ~/.cargo/registry/src/../windows_gen-xxx/ or ~/.cargo/git/...`, which that branch would provide. I guess it makes sense to submit (a cleaned version of) it after all and see where we end up?

Agreed. I think the only blocker for your branch was us wanting to support winmds from a bunch of other crates (scenario 3). But since that's out now, I think your changes would be compatible/welcome. I'm happy to test on Windows when ready.

I'm sure if we missed something, Kenny will figure out how to yell at us from afar with just access to rocks and flora.

[MarijnS95]

Agreed. I think the only blocker for your branch was us wanting to support winmds from a bunch of other crates (scenario 3). But since that's out now, I think your changes would be compatible/welcome. I'm happy to test on Windows when ready.

Sounds good, I'll get that set up in the coming days!

I'm sure if we missed something, Kenny will figure out how to yell at us from afar with just access to rocks and flora.

Will look for smoke signals in the meantime :)