Skip to content
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

docs: preserve the modification date when copying the prebuilt man page #12199

Closed
wants to merge 1 commit into from

Conversation

mkauf
Copy link
Contributor

@mkauf mkauf commented Oct 25, 2023

The previously built man page "curl.1" must be copied with the original modification date, otherwise the man page is never updated.

This fixes a bug that has been introduced with commit 2568441.

@github-actions github-actions bot added the build label Oct 25, 2023
@mkauf
Copy link
Contributor Author

mkauf commented Oct 25, 2023

Fix this autobuild problem: https://curl.se/dev/log.cgi?id=20231025023847-2167007#prob3

CC @dfandrich

@dfandrich
Copy link
Contributor

dfandrich commented Oct 26, 2023 via email

@mkauf
Copy link
Contributor Author

mkauf commented Oct 26, 2023

A different bugfix that uses the touch command:

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 5454e8330..bb8241e72 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -111,7 +111,8 @@ SUFFIXES = .1 .html .pdf
 # have changed.
 $(abs_builddir)/curl.1:
        if test "$(top_builddir)x" != "$(top_srcdir)x" -a -e "$(srcdir)/curl.1"; then \
-               $(INSTALL_DATA) "$(srcdir)/curl.1" $@; fi
+               $(INSTALL_DATA) "$(srcdir)/curl.1" $@; \
+               touch -r "$(srcdir)/curl.1" $@; fi
        cd cmdline-opts && $(MAKE)
 
 html: $(HTMLPAGES)

@bagder
Copy link
Member

bagder commented Oct 26, 2023

A different bugfix that uses the touch command:

I would expect touch -r have the same problems or perhaps worse than cp -p ...

@dfandrich
Copy link
Contributor

dfandrich commented Oct 26, 2023 via email

@dfandrich
Copy link
Contributor

I think the touch proposal three comments up has the best chance of working. We're not using touch anywhere else in the build system yet but it's a standard POSIX command so it should be available everywhere.

The previously built man page "curl.1" must be copied with the original
modification date, otherwise the man page is never updated.

This fixes a bug that has been introduced with commit 2568441.
@mkauf
Copy link
Contributor Author

mkauf commented Oct 28, 2023

Ok, I have updated the pull request, now it uses touch -r.

@mkauf
Copy link
Contributor Author

mkauf commented Oct 28, 2023

I would expect touch -r have the same problems or perhaps worse than cp -p ...

cp -p also changes file ownership information, touch -r only changes the file's timestamp. So I think that it should work.

@dfandrich
Copy link
Contributor

dfandrich commented Oct 28, 2023 via email

@mkauf
Copy link
Contributor Author

mkauf commented Nov 2, 2023

@bagder OK to merge this?

@mkauf mkauf closed this in 2c8f4c8 Nov 4, 2023
@mkauf mkauf deleted the fix_curl.1_bug branch November 4, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants