Hooks are tied to build platform

[ydirson]

Provided hook examples clearly show how platform-dependent they are: if I use a cp to copy additional data in a post_build hook (generated data that cannot be found by <link data-trunk rel="copy-dir" ...), it is working all right on my Linux dev box, but will break on any Windows dev trying to build my app.

Traditionally Cargo has been avoiding this by letting the crate author specify additional build commands in Rust (build.rs). It does not look trivial for the author to write a separate hook script in Rust and get it built for the build architecture, though sorting out how to do that and providing an example could be a first step. Then transparent support in trunk for eg. trunk_hook_post_build.rs and such would be a way to make this easily available to crate authors.

As an alternative, hooks as they stand now should make it possible to declare using cfg-like architecture expressions that a hook is specific to certain build architectures, provide a way to specify implementations for other platforms, and bomb out with "unsupported build platform, please patch Trunk.toml". But while that looks like a more natural evolution, it is IMHO not trivial enough given the value it provides.

[ctron]

I think adding a complete new build system like that would lead to some serious over-engineering. Using local script tools seems like a reasonable approach IMHO.

I do see the benefit of trunks approach for sure. But you could also leverage that for exactly this scenario using the xtask pattern: https://github.com/matklad/cargo-xtask

[ydirson]

The idea is indeed interesting. One thing is a bit tricky however: what I'm after for my app is getting files copied only when a specific feature is requested (the nominal app make fetches from remote URLs, the local-files feature converts this to local URLs for offline development, so this mode must copy the files into dist/, but they must not get there in nominal mode.

My original approach approach was to let build.rs copy (only if the feature is set) the files from the test-data build-dep crate into data/ (which is a bad place, build.rs should not write outside $OUT_DIR, but then the next step cannot today have access to the $OUT_DIR value), and then the post_build script will just copy the files to $TRUNK_STAGING_DIR if data exists, which makes it unnecessary to know about the feature.

Sure, I can keep the same build.rs and have the xtask implement the conditional copy, though:

• having all the code related to this given feature in a single place seems better
• the trick can only ever be used with post_build hooks

So could we make it possible to move all this logic into an xtask, conditioned to a feature? For the xtask to know about the feature, it would be given to cargo run invocation, which would not only require support from Trunk, but some way to select which features to forward this way.
We could instead allow in the hook definition to specify a crate feature (or a feature set) which must be set for the hook to run, like:

[[hooks]]
stage = "post_build"
on_features = ["local-files"]
command = "cargo"
command_arguments = ["run", "--manifest-path", "./xtasks/Cargo.toml", "post_build"]

What do you think?

[ctron]

I am wondering why you don't just use an env-var for this? You can easily check that in an xtask for example.

[ydirson]

I am wondering why you don't just use an env-var for this? You can easily check that in an xtask for example.

Moving the build.rs into an xtask moves the build-deps there. Checking at xtask runtime would still bring the extra build-deps for every build, whereas they are useful only for a portion of dev builds.

[ctron]

Well as xtask actually is just an alias, why not have xtask-feature and provide the --feature there?

[ydirson]

I thought about that (that's the "So could we make it possible to move all this logic into an xtask, conditioned to a feature?" in this post), but it would still require support from Trunk to pass this flag only in certain conditions.

[ctron]

I am sorry, I don't understand why?

[ydirson]

I meant, if I have 2 different xtasks, for feature-on and feature-off, the problem is now "how do I select which xtask to run?". I still fail to see what your idea is to achieve this.

As a concrete basis for discussion, my initial attempt at moving everything in one step can be found in ydirson/generals-familiar#33

[ctron]

Hm ok, maybe you can create a small example repository to illustrate the problem.

[ctron]

I created an example project for experimenting with hooks: https://github.com/trunk-rs/trunk/tree/main/examples/hooks

This has one xtask based hook. And another approach of using the build.rs. That latter has access to the active cargo features. Maybe that helps a bit.