Skip to content

cmake does not build curl.1 #13028

Description

@dfandrich

I did this

cmake -Bbuild . && make -C build -j9

This builds the man pages in build/docs/libcurl/ but not build/docs/curl.1 build/docs/curl-config.1 or build/docs/mk-ca-bundle.1 This causes tests 1140 and 1173 to fail.

I expected the following

All the man pages should be built by cmake.

curl/libcurl version

curl-8.7.0-DEV commit 9e2ee70

operating system

Linux x86_64

Activity

  1. bagder commented on Mar 3, 2024

    @bagder
    Member

    So we have no CI builds verifying this or how did this problem sneak in?

  2. bagder commented on Mar 3, 2024

    @bagder
    Member

    We have several cmake CI jobs in GHA macos.yml, but none of them runs the test suite.

    The AppVeyor jobs that use cmake and run tests, have the tests 1140 and 1173 (and more) disabled.

    cmake clearly is a second tier build system for building curl...

  3. bagder commented on Mar 4, 2024

    @bagder
    Member

    This probably (?) never worked.

  4. vszakats commented on Mar 4, 2024

    @vszakats
    Member

    This was broken until maybe 8.6.0 or 8.5.0. Then we fixed it, but ENABLE_CURL_MANUAL stayed OFF by default, because the source tarball already provides these files (possibly even mk-ca-bundle.1, but I haven't followed that). So for most people it makes no sense to rebuild them (which also breaks reproducibility).

    Solution: Maybe skip these tests if the necessary files are missing, then make sure to have perl/nroff around and set ENABLE_CURL_MANUAL=ON where we want these tests to run in CI?

  5. dfandrich commented on Mar 4, 2024

    @dfandrich
    ContributorAuthor

    I've confirmed that ENABLE_CURL_MANUAL=ON causes curl.1 to be built and the tests to pass. But, this is inconsistent because

    1. All 496 of the other man pages are built with ENABLE_CURL_MANUAL=OFF
    2. automake builds build the manual by default (if all dependencies are there)
  6. vszakats commented on Mar 4, 2024

    @vszakats
    Member

    There is a separate control for the 496 libcurl man pages, called BUILD_LIBCURL_DOCS. ON by default. The source tarball doesn't ship them, so building them by default makes sense. Such option was recenly added for autotools too, called --{enable,disable}-docs, in 5413215.

    IMO the autotools default is wrong in rebuilding the existing manuals even if they exist. This is double work (for possibly almost everyone outside curl's own CI), and breaks reproducibility.

    One option would be to verify if the curl manuals exists and set the default accordingly in both cmake and autotools. I've been doing this in curl-for-win builds.

    With CMake there is another related issue when building from a source tarball, where the distributed src/tool_hugehelp.c is missed when doing an out-of-tree build. Have to copy it to the build tree manually. Just noticed this today. I thought it was fixed in 8.6.0, but turns out it worked by chance because src/tool_hugehelp.c is missing from the 8.6.0 source tarball (fixed now), and curl-for-win thus used a manual ENABLE_CURL_MANUAL=ON to build it from scratch.

  7. bagder commented on Mar 4, 2024

    @bagder
    Member

    IMO the autotools default is wrong in rebuilding the existing manuals even if they exist. This is double work (for possibly almost everyone outside curl's own CI), and breaks reproducibility.

    configure generates a makefile, and that makefile rebuilds the target (manpage) if the components are newer, not otherwise. Unless there's a bug.

  8. dfandrich commented on Mar 4, 2024

    @dfandrich
    ContributorAuthor
  9. bagder commented on Mar 4, 2024

    @bagder
    Member

    I'm changing it in #13047 anyway ...

  10. vszakats commented on Mar 4, 2024

    @vszakats
    Member

    I'm changing it in #13047 anyway ...

    That's a nice update! It resolves the reproducibility issue. The double-work remains, which is probably not so bad speaking of these few generated files.

    Maybe someone has an idea how to teach CMake to do a copy instead of a rebuild if these files are already present in the source tree. But just like with autotools, it seems sensitive to corner-cases. (Perhaps an 'AUTO' mode could help with that.)

  11. bagder commented on Mar 4, 2024

    @bagder
    Member

    The double-work remains, which is probably not so bad speaking of these few generated files.

    What double work are you referring to?

  12. vszakats commented on Mar 5, 2024

    @vszakats
    Member

    The double-work remains, which is probably not so bad speaking of these few generated files.

    What double work are you referring to?

    curl's source tarball already ships with curl.1, src/tool_hugehelp.c (at least it will again in 8.7.0). If we change CMake to build these by default, they will be rebuilt for everyone using the source tarball, which I presumable almost all users outside our own CI. This is unnecessary double work, and even after dropping nroff, will introduce a reproducibility issue without a SOURCE_DATE_EPOCH set (presumably not set by many builders).

  13. bagder commented on Mar 5, 2024

    @bagder
    Member

    they will be rebuilt for everyone using the source tarball

    Ok, but that's "just" an area for improvement. If we do dependencies right, it should not rebuild.

    Additionally, I'm changing how hugehelp is made so it will not use the curl.1 file as source anymore.

  14. bagder commented on Mar 6, 2024

    @bagder
    Member

    #13047 is merged. No more nroff in the build process.

  15. vszakats commented on Mar 6, 2024

    @vszakats
    Member

    Pros and con for enabling manuals by default:

    • pro: building the manual is quick (in my tests, slow machine, non-Windows).
    • pro: building the manual supports reproducibility after dropping nroff (subject to double-check tomorrow).
      tool_hugehelp.c is reproducible by default (and no date in it), curl.1 requires SOURCE_DATE_EPOCH.
    • pro: autotools already has this enabled by default.
    • pro: it's not trivial to enable --manual without also building the manual. Can be done with CPPFLAGS=-DUSE_MANUAL, and copying files manually when building out-of-tree. (Something to improve independently.)
    • con: it ignores pre-built manual and builds it anyway (breaking the release date in curl.1). This will affect envs without Perl, those will not have --manual enabled. (Haven't tested this.)

    If not missed anything, it look fine to enable building the manual by default.

    Those who aim for a reproducible man page with cmake, will need to set SOURCE_DATE_EPOCH.

  16. added a commit that references this issue on Mar 6, 2024
    a96bd4b
  17. added a commit that references this issue on Mar 7, 2024
    9b81f1b
  18. dfandrich commented on Mar 7, 2024

    @dfandrich
    ContributorAuthor

    Tests 1140 and 1173 are still failing. curl.1 is now being written to docs/cmdline-opts/curl.1 but the tests expect it in docs/curl.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions