Consider including xtask/ in the published crates

[musicinmybrain]

In Fedora Linux, we have the following rule:

Projects from crates.io MUST be packaged from the sources that are published there

This has been working well for packaging both the library and the command-line tool, but since #645 (which I agree was a good idea overall), the published crates no longer contain everything necessary to generate the man pages because xtask/* was explicitly excluded.

I can work around this by adding the GitHub source archive as an additional source, which is a bit tedious but workable. However, this is made more difficult by the fact that some of the test files, like issue-141.png, have dubious license/copyright status. By looking at their contents, it is reasonable to suspect they are not covered by oxipng’s MIT license, and indeed to suspect that they may not be redistributable at all. Therefore, I would need to use a script to produce a filtered version of the GitHub source archive for Fedora to use and distribute in source RPMs. This, too, is precedented and possible, but it’s becoming a lot of work to do for a man page!

Would you consider just removing the line

oxipng/Cargo.toml

Line 17 in f8c84c2

"xtask/*",

from package.exclude in Cargo.toml? This would add only three tiny files to the published crates, in exchange for avoiding all of the above packaging issues.

[andrews05]

Apologies for the trouble, it sounds like a pretty reasonable request. I’ll let @AlexTMjugador pick it up though as I’m not sure if there was a reason for excluding that.

[AlexTMjugador]

I chose to explicitly exclude the xtask files because I believed they weren't necessary for crate consumers, and by leaving them out, I achieved two benefits: enforcing the intended implementation detail encapsulation, and slightly reducing the size of the published crate, both of which seemed like a win-win to me.

However, my rationale above did not consider that Linux distributions might actually find these artifacts useful for their packaging needs. Therefore, I too think this is a reasonable request we can apply; there was no reason to exclude these files other than that being "technically neater" in the sense stated above.

[AlexTMjugador]

I've explored several Cargo manifest exclude/include combinations to implement this, but unfortunately, it seems that including the xtask subcrate within the oxipng crate isn't possible due to how cargo publish works.

It'd be technically possible to rename the xtask Cargo.toml manifest to something else so that we're not hit by that limitation, but then that file would need to be renamed back to its original name before that xtask is used. Would that be a good tradeoff for the purposes of packaging?

[musicinmybrain]

I've explored several Cargo manifest exclude/include combinations to implement this, but unfortunately, it seems that including the xtask subcrate within the oxipng crate isn't possible due to how cargo publish works.

Hmm, that’s unfortunate. I didn’t realize that was the case when I opened this issue.

It'd be technically possible to rename the xtask Cargo.toml manifest to something else so that we're not hit by that limitation, but then that file would need to be renamed back to its original name before that xtask is used. Would that be a good tradeoff for the purposes of packaging?

I think that sounds rather awkward and confusing. I would rather not encourage you to commit a “weird” workaround. I can live with needing an auxiliary source archive from GitHub to build the man page, especially since #655 has been dealt with.

[musicinmybrain]

I was just talking about this with @decathorpe, who pointed out that another possible approach would have been to keep building the man page in build.rs, but gate the man page generation and the associated dependencies on a feature flag. I don’t want to push for more churn in the implementation, though. As I said, though, I can live with the auxiliary source, and using an auxiliary source has the advantage that I should be able to copy in tests/ and run them, too.

[AlexTMjugador]

Adding a "private" feature for manpage generation could work, but Cargo features can't truly be private: they can appear in the documentation and can be enabled by any crate that depends on oxipng. Even though in practice naming such a feature in a way that clearly indicates it's not part of the public interface works well enough (and there is a convention for considering features beginning with underscores as private), if you feel that the auxiliary source approach is cleaner overall and not too complicated to implement, let's go with that! 👍

It would also have the added benefit you stated of including the tests folder, which has always been excluded from the published crate due to its large size.

[musicinmybrain]

I’m closing this based on the above discussion. Thanks for investigating!