-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
introducing "curldown" for libcurl man page format #12730
Conversation
e67a9d8
to
fe96ca5
Compare
fe96ca5
to
0139d7f
Compare
The |
0139d7f
to
272b028
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
57f7358
to
ddbf3f7
Compare
ddbf3f7
to
2380072
Compare
This comment was marked as resolved.
This comment was marked as resolved.
2380072
to
dd0a3d5
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
b8c1be7
to
bd659c5
Compare
An interesting side-effect of switching to |
my $blankline = 0; | ||
my $header = 0; | ||
|
||
push @desc, ".\\\" generated by cd2nroff $cd2nroff from $f\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a local test run (I used CMake) $f
remains empty for all output files:
.\" generated by cd2nroff 0.1 from
.TH CURLINFO_APPCONNECT_TIME 3 "December 14 2023" libcurl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the full command line used when cd2nroff
is invoked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see why
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff --git a/docs/libcurl/CMakeLists.txt b/docs/libcurl/CMakeLists.txt
index fc1283a53..0d52a7e56 100644
--- a/docs/libcurl/CMakeLists.txt
+++ b/docs/libcurl/CMakeLists.txt
@@ -34,11 +34,11 @@ function(add_manual_pages _listname)
else()
string(REPLACE ".3" ".md" _mdfile "${CMAKE_CURRENT_SOURCE_DIR}/${_file}")
endif()
add_custom_command(OUTPUT "${_rofffile}"
- COMMAND ${PROJECT_SOURCE_DIR}/scripts/cd2nroff < ${_mdfile} > ${_rofffile}
+ COMMAND ${PROJECT_SOURCE_DIR}/scripts/cd2nroff ${_mdfile} > ${_rofffile}
DEPENDS "${_mdfile}"
VERBATIM
)
endforeach()
endfunction()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They look like this:
cd ./curl/_x64-win-ucrt-cmake-llvm-bld/docs/libcurl/opts && \
./curl/scripts/cd2nroff \
< ./curl/docs/libcurl/opts/CURLINFO_APPCONNECT_TIME.md \
> ./curl/_x64-win-ucrt-cmake-llvm-bld/docs/libcurl/opts/CURLINFO_APPCONNECT_TIME.3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, my diff above changes that command line, to pass the file name instead < [filename]
which is what made it blank there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! That fixed it ...but now there is another issue: The filename embedded contains the whole (absolute) path of the source filename, which breaks reproducibility and may also leak local info. (This may be CMake-specific, as CMake loves absolute paths, causing this class of issues everywhere.)
.\" generated by cd2nroff 0.1 from /<home-dir>/<path-to>/curl/docs/libcurl/opts/CURLINFO_CONNECT_TIME_T.md
.TH CURLINFO_CONNECT_TIME_T 3 "December 14 2023" libcurl
Perhaps the simplest fix is to omit this info from the generated output? Or strip all path from it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not break reproducibility, you just have to rebuild with the same paths. But yes, I'll make the script cut off the dir name. I want the file name mentioned there to help users find the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Patch to drop the source filename:
diff --git a/scripts/cd2nroff b/scripts/cd2nroff
index 08fbd768e..734cbf8f3 100755
--- a/scripts/cd2nroff
+++ b/scripts/cd2nroff
@@ -186,7 +186,7 @@ sub single {
my $blankline = 0;
my $header = 0;
- push @desc, ".\\\" generated by cd2nroff $cd2nroff from $f\n";
+ push @desc, ".\\\" generated by cd2nroff $cd2nroff\n";
push @desc, ".TH $title $section \"$date\" $source\n";
while(<$fh>) {
$line++;
To strip path:
diff --git a/scripts/cd2nroff b/scripts/cd2nroff
index 08fbd768e..d9938b6e9 100755
--- a/scripts/cd2nroff
+++ b/scripts/cd2nroff
@@ -30,6 +30,8 @@ Converts a curldown file to nroff (man page).
=end comment
=cut
+use File::Basename;
+
my $cd2nroff = "0.1"; # to keep check
my $dir;
my $extension;
@@ -186,7 +188,7 @@ sub single {
my $blankline = 0;
my $header = 0;
- push @desc, ".\\\" generated by cd2nroff $cd2nroff from $f\n";
+ push @desc, ".\\\" generated by cd2nroff $cd2nroff from " . basename($f) . "\n";
push @desc, ".TH $title $section \"$date\" $source\n";
while(<$fh>) {
$line++;
Adding a CMake option to disable building docs: diff --git a/CMakeLists.txt b/CMakeLists.txt
index 70ca457a9..18d8d085b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -306,6 +306,7 @@ endif()
find_package(Perl)
+option(BUILD_DOCS "to build manual pages" ON)
option(ENABLE_MANUAL "to provide the built-in manual" OFF)
if(ENABLE_MANUAL AND PERL_FOUND)
diff --git a/docs/libcurl/CMakeLists.txt b/docs/libcurl/CMakeLists.txt
index fc1283a53..c719c9764 100644
--- a/docs/libcurl/CMakeLists.txt
+++ b/docs/libcurl/CMakeLists.txt
@@ -54,11 +54,13 @@ add_custom_command(OUTPUT libcurl-symbols.md
VERBATIM
)
-add_manual_pages(man_MANS)
-add_custom_target(man ALL DEPENDS ${man_MANS})
-if(NOT CURL_DISABLE_INSTALL)
- install(FILES "$<LIST:TRANSFORM,${man_MANS},PREPEND,${CMAKE_CURRENT_BINARY_DIR}/>"
- DESTINATION ${CMAKE_INSTALL_MANDIR}/man3)
-endif()
+if(BUILD_DOCS)
+ add_manual_pages(man_MANS)
+ add_custom_target(man ALL DEPENDS ${man_MANS})
+ if(NOT CURL_DISABLE_INSTALL)
+ install(FILES "$<LIST:TRANSFORM,${man_MANS},PREPEND,${CMAKE_CURRENT_BINARY_DIR}/>"
+ DESTINATION ${CMAKE_INSTALL_MANDIR}/man3)
+ endif()
-add_subdirectory(opts)
+ add_subdirectory(opts)
+endif() (Edited to be enabled by default.) |
They are built with autotools by default and there is no option to switch that off... |
On my machine, building all libcurl ~500 man pages takes 800 milliseconds. It is a pretty quick operation, even if my machine might be above average CPU and disk speed wise. |
I takes almost as long as the build itself here, but I'm on fairly old gear. I'd appreciate an option to turn this off, as this will increase greatly the time for each build. When doing them by the hundreds, it adds up. (Even building curl.1/hugehelp takes a fair amount, but I can tweak that locally.) For some reason autotools never built man pages with the curl-for-win script, I don't know why, with Edit: with this PR applied, autotools quits with:
|
Fair enough! |
Re: autotools, it did have this working before this patch, but was unnoticably fast (being just a handful of But, for now autotools builds quit with this error (with this PR applied), so I couldn't actually test it:
|
We use autotools in numerous CI builds, including windows (on appveyor) and none of them fail like that... I'm not saying there is no problem left, but I might not consider that a blocker for merging this. |
This is the biggest PR I/we have done in a long time. I completely expect that there will be some issues popping up once we merge this, but I don't think they are worse than we can just fix them. I believe this is a good PR. |
No worries, re-run it in a clean sandbox, and that resolved it. |
This
But this works fine:
And so does my good old StrawBerry Perl (v5.18.4 from 2014). Edit: Cygwin's Perl doesn't work at all. Hence using StrawBerry Perl. |
It built in several Windows CI jobs, but I'm sure we can improve it. |
Since all those CI jobs checks out with |
No, I can't see anything that sets
Whenever I try to guess how something works with text files on Windows, I seem to guess wrong. So I don't know. |
I bet you can fix that by changing/setting some locale environment variable(s)... |
- cmake: enable `BUILD_DOCS` by default (this controls converting and installing `.3` files from `.md` sources) - cmake: speed up generating `.3` files by using a single command per directory, instead of a single command per file. This reduces external commands by about a thousand. (There remains some CMake logic kicking in resulting in 500 -one per file- external `-E touch_nocreate` calls.) - cd2nroff: add ability to process multiple input files. - cd2nroff: add `-k` option to use the source filename to form the output filename. (instead of the default in-file `Title:` line.) Follow-up to 3f08d80 Follow-up to ea0b575 #12753 Follow-up to eefcc1b #12730 Closes #12762
That I cannot explain because to me it looks like the generate the date strings identically! |
It was accidentally added in curl#12730 Follow-up to eefcc1b
It was accidentally added in curl#12730 Co-authored-by: Lukáš Zaoral <lzaoral@redhat.com> Signed-off-by: Jan Macku <jamacku@redhat.com> Follow-up to eefcc1b
overview
curldown is this new file format for libcurl man pages. It is markdown inspired with differences:
their man page section is specified)
tools:
This setup generates .3 versions of all the curldown versions at build time.
CI
Since the documentation is now technically markdown in the eyes of many
things, the CI runs many more tests and checks on this documentation,
including proselint, link checkers and tests that make sure we capitalize the
first letter after a period...
tasks
.XX
instructions in nroff files are handled by nroff2cd.md
sourcesdocs/CURLDOWN.md
documents the format and involved tools.md
file extensionsThe tests that rely on the man pages to be present will be disabled for the CI jobs that use cmake on Appveyor for now.