Move manpages generation to an xtask #645
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR #596 brought forward automatic generation of Linux manual pages for Oxipng, which is executed every time Oxipng is built. However, while building manpages on every build is convenient for Oxipng development and doing so didn't catch my attention initially, it introduces noticeable inefficiencies for crates using Oxipng as a library: during their build, Oxipng manpages are also built, even though most dependent crates won't use such artifacts, as they are not considered part of the public Oxipng crate API or even appropriate for non-human consumption. Moreover, generating manpages depends on `clap`, which is a heavyweight dependency: according to a fresh `cargo build --timings --release` on my development workstation, its `clap_builder` dependency is the third most time consuming unit to build, totalling 1.5 s (out of 11.7 s, or 12.8%). And there is no way for dependent crates to turn this off: [`build-dependencies` cannot be conditional on crate features](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies). Potentially using other `cfg` hacks to either enable or disable manpage generation is unergonomic, if not outright disallowed. Besides reducing their compilation time cost, dependent crates may also want to trim the size of their dependency tree, avoiding unnecessary dependency downloads in the process. Therefore, a better solution to conditionally build manpages in a way convenient for both Oxipng maintainers and downstream consumers is needed. My proposal implemented in this PR is to leverage the [`cargo-xtask`](https://github.com/matklad/cargo-xtask) convention to define an auxiliary crate to move the manpage generation logic and dependencies to, which is to be used exclusively by Oxipng maintainers and not part of the `oxipng` crate published on `crates.io`. That way Oxipng maintainers and packagers can still generate manpages at request with ease, without any automation being noticeable to uninterested crate consumers. And as a side benefit, Oxipng maintainers can also benefit from slightly faster iteration times due to the lack of a build script for the main crate. The new `mangen` xtask can be run at any time with `cargo xtask mangen`. The generated manpages are now available at `target/xtask/mangen/manpages`. Existing deployment scripts were updated accordingly.
AlexTMjugador
added
T-Techdebt
andrews05
reviewed
Nov 23, 2024
There was a problem hiding this comment.
Thanks @AlexTMjugador, great to see this done! I think xtask was always the better solution, I just didn't have any experience with it.
No bashisms are needed here, so target portability accross Unix systems.
andrews05
approved these changes
Nov 24, 2024
|
Yes, I think we can proceed with a new patch or minor release. However, I'd like to get #642 merged before doing the release, as it's likely to be a pretty welcome improvement for those using Docker. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #596 brought forward automatic generation of Linux manual pages for Oxipng, which is executed every time Oxipng is built. However, while building manpages on every build is convenient for Oxipng development and doing so didn't catch my attention initially, it introduces noticeable inefficiencies for crates using Oxipng as a library: during their build, Oxipng manpages are also built, even though most dependent crates won't use such artifacts, as they are not considered part of the public Oxipng crate API or even appropriate for non-human consumption.
Moreover, generating manpages depends on
clap, which is a heavyweight dependency: according to a freshcargo build --timings --releaseon my development workstation, itsclap_builderdependency is the third most time consuming unit to build, totalling 1.5 s (out of 11.7 s, or 12.8%). And there is no way for dependent crates to turn this off:build-dependenciescannot be conditional on crate features. Potentially using othercfghacks to either enable or disable manpage generation is unergonomic, if not outright disallowed. Besides reducing their compilation time cost, dependent crates may also want to trim the size of their dependency tree, avoiding unnecessary dependency downloads in the process.Therefore, a better solution to conditionally build manpages in a way convenient for both Oxipng maintainers and downstream consumers is needed. My proposal implemented in this PR is to leverage the
cargo-xtaskconvention to define an auxiliary crate to move the manpage generation logic and dependencies to, which is not part of theoxipngcrate published oncrates.io. That way Oxipng maintainers and packagers can still generate manpages at request with ease, without any automation being noticeable to uninterested crate consumers. And as a side benefit, Oxipng maintainers can also benefit from slightly faster iteration times due to the lack of a build script for the main crate.The new
mangenxtask can be run at any time withcargo xtask mangen. The generated manpages are now available attarget/xtask/mangen/manpages. Existing deployment scripts were updated accordingly.