search

crystal-lang / distribution-scripts

40 stars 23 forks source link

Fix: Install `asciidoctor` in linux build image #255

Closed straight-shoota closed 9 months ago

straight-shoota commented 11 months ago

The build job complains about missing asciidoctor for compiling shards' man pages. https://app.circleci.com/pipelines/github/crystal-lang/crystal/12631/workflows/68d1c081-336a-4ecc-b6e8-e4fb3852e8ab/jobs/77518

I don't know why this only became a problem now and hadn't been an issue before. Maybe some other dependency pulled in asciidoctor previously and now doesn't anymore? But I can't tell which one that might be.

straight-shoota commented 11 months ago

Ah the reason why this didn't error before is that the man pages are checked into the repo and make only rebuilds them when they are older then the corresponding .adoc file. That's the case currently (?) which then results in invoking asciidoctor.

straight-shoota commented 11 months ago

Actually, git does not preserve modification time. So it cannot be the reason. I suppose it might perhaps be an edge case that the git checkout is not atomic and one file has a slightly different timestamp than the other? IDK.

Technically, we should not need to rebuilt the manpages. I think we can prevent this by telling make that the manpages are to be considered up to date. However, I think it could also make sense to always rebuilt the manpages in order to get them dated to the build time, not whenever they were last edited. I think we can enforce this by telling make that the manpages are to be rebuilt explicitly.

These paths are pretty much opposite of each other. But either would be viable and better than the current solution in this PR which installs asciidocotor but rebuilding is not enforced.

bcardiff commented 11 months ago

Maybe we can check that the generated files are up to date upon release.

straight-shoota commented 11 months ago

I think "upon release" is the wrong time to check that. At this point it should either be generated fresh, or taken as is (with some ensurance that it must be up to date).

I'm proposing https://github.com/crystal-lang/shards/pull/594 to check the manpages when updating the docs. That would ensure that man/ and docs/ are always in sync.

We can still consider whether we take it as is upon release, or still regenerate the manpages. The latter would only have the purpose of bumping the date to match that of the release. (I suppose we could also do that with a simple sed script and don't need asciidoctor for that).

straight-shoota commented 9 months ago

I don't think this is necessary anymore after https://github.com/crystal-lang/shards/pull/594