Details on new maintainership #603

NWilson · 2024-12-07T16:52:08Z

It's a start!

Main goals:

Gratefully acknowledge contributions
Inform users of maintainship change
Don't offend anyone

I also want to tidy up some files in git. In a future commit I will probably remove some of the auto-generated files (eg some of those made by PrepareRelease) so that we aren't checking in tool outputs onto the main branch.

I will do whatever renaming (or markdown→text) is required to keep the same files in the release tarball.

PhilipHazel · 2024-12-07T17:01:31Z

The PrepareRelease script makes the HTML files from the man pages. I was explicitly asked to add these to the repo so that those who want to compile from HEAD get the same information. It was easiest just to do it that way - and try to remember to run "PrepareRelease doc" whenever I updated the man pages.

zherczeg · 2024-12-07T17:15:10Z

Nice!

NWilson · 2024-12-07T22:31:56Z

I agree Philip, the docs should be checked-in I think, so that a master checkout is basically usable as-is, equivalent to a tarball release. There are just one or two other files though that I think I could prune (index.md, for a start!)

LICENCE.md

…tidy

NWilson · 2024-12-09T11:12:41Z

maint/PrepareRelease

+perl maint/Detrail \
+  $files \
+  cmake/{*.cmake,*.cmake.in} \
+  m4/* \
+  src/pcre2_*.{h,c} \
+  doc/p* \
+  doc/html/*


I've added some extra files to Detrail. We've added more source files since 10.44, and some of them have some trailing whitespace errors.

NWilson · 2024-12-09T11:13:53Z

maint/PrepareRelease

-# processing of the documentation, detrails files, and creates pcre2.h.generic
-# and config.h.generic (for use by builders who can't run ./configure).


Despite the comment at the top saying it creates {config,pcre2}.h.generic, I can't find any evidence that it has ever done so. I have added a CI job to ensure that these files stay up to date, using the existing make recipe in Makefile.am to generate them.

NWilson · 2024-12-09T21:10:03Z

I've been putting lots of random dist & tarball related tweaks & fixes on this branch.

I'm going to leave it now. I think it's OK to merge.

NWilson · 2024-12-09T21:12:37Z

.github/workflows/dev.yml

+    name: GCC -Os, old Autotools
+    runs-on: ubuntu-20.04


Annoyingly... the changes between Automake 1.15 and 1.16 turn out to be somewhat annoying, and it took several attempts to make my changes to Makefile.am. We didn't have an autoconf build on old Ubuntu, which would have helped me to catch the problems.

NWilson · 2024-12-09T21:13:05Z

.github/workflows/dev.yml

+              echo "Dirty working tree! Affected files:"
+              git status --porcelain || true
+              echo ""
+              echo "Diff:"
+              git diff || true


You'll now get build failures if you update documentation files, but forget to sync the HTML or TXT.

unless we improve the tooling, this sounds a little restrictive.

Truth is that with Philip being the only native English speaker, his "final" touches over the generated documentation were an invaluable part of making sure it is as high quality as it is (including of course, updating the date with the necessary "fixes")

Updating the docs can still be done by me (or other native English speaker). But there seems to be no good use-case for having the different copies of the documentation be out-of-sync with each other. If that ever happens, it's simply because a contributor forgot to sync the HTML or TXT files after updating the man pages.

If you'd prefer, we make it so that the CI job applies the changes and pushes a bot-authored commit to the PR. That seemed like a bit too much infrastructure though, for a simple sanity-check.

The way I saw it was that the "source" files for the documentation were being updated by the developer, and the merge that Philip did which usually included the "documentation fixes" and "generated files" was akin to a compilation and "release" of the documentation.

specially since there is a copy of it that is autogenerated and publicly available in the web site as part of the github.io content.

Oops. I had completely forgotten to mention github.io to Nick. Apologies.

I'm aware of it. It hosts index.md (which is a duplicate of README.md), and the HTML files in docs/.

We could do what some projects do, and have different versions of the docs available (for each library version) so the user has a choice of whether to read the "master" or "10.44" docs.

In a future PR, when I spruce up that website a bit, I'll probably just make it so it shows the docs for the last-released version of PCRE2.

NWilson · 2024-12-09T21:14:16Z

BUILD.bazel

@@ -19,7 +19,7 @@ copy_file(
    out = "src/pcre2_chartables.c",
 )

-# Removed src/pcre2_ucptables.c below because it is #included in 
+# Removed src/pcre2_ucptables.c below because it is #included in


Ensured that Detrail is run on all files that are part of the output tarball (...except the files where we deliberately don't want to run it, eg CRLF files, and any/all tool outputs from autoconf).

Ideally I would like to see that ALL our files get detrail, which necessary implies we need to teach it to work also in Windows style line endings.

I can see there might be a few exceptions (ex: test files that require explicit trailing blank characters), but even those could be exempted at the line number level IMHO.

We just have two nasty old .bat files in the repo with CRLF. They're OK for now (I have a task to convert them to .ps1 files someday - which is vastly better than bat). The test files are treated as "binary" and so I haven't proactively detrailed them. But, all the other files in the repo have now been added to Detrail.

NWilson · 2024-12-09T21:14:57Z

CMakeLists.txt

+file(GLOB html ${PROJECT_SOURCE_DIR}/doc/html/*.html ${PROJECT_SOURCE_DIR}/doc/html/*.txt)
+file(
+  GLOB txts
+  ${PROJECT_SOURCE_DIR}/doc/*.txt
+  AUTHORS.md
+  COPYING
+  ChangeLog
+  LICENCE.md
+  NEWS
+  README
+  SECURITY.md
+)


Fixed discrepancy between cmake --install and ./configure && make install. They should install the same files ... and they do now.

NWilson · 2024-12-09T21:16:52Z

INSTALL

@@ -0,0 +1,368 @@
+Installation Instructions


We should probably edit this file sometime. The default Autoconf one is pretty bland and generic.

I checked it in, in preparation for that, and also to try working around an annoying issue in automake (see other comments on "foreign" flag).

NWilson · 2024-12-09T21:18:13Z

Makefile.am

+EXTRA_DIST += \
+  BUILD.bazel \
+  MODULE.bazel \
+  WORKSPACE.bazel \
+  build.zig


Within reason, I feel that users grabbing the tarball should be getting the same experience as users who check out from git (except that the tarball users don't need to run autogen.sh). So I've added some missing files to the tarball.

Carefully here, since these specific files are usually updated in non backward compatible ways, and are (I presume) used by some external build process that most likely only uses the git files.

NWilson · 2024-12-09T21:18:48Z

Makefile.am

-  PrepareRelease \
-  CheckMan \
-  CleanTxt \
-  Detrail \
-  132html \
-  doc/index.html.src


I don't feel we need to be bundling into the tarball all our maintainer scripts.

Indeed not, nowadays. I think they were originally added way back, certainly before GitHub, probably even before Svn, when tarball was the only way to download anything. Ah yes, its documented as happening in 2007.

NWilson · 2024-12-09T21:20:03Z

README

+  RunTest.bat              a Windows batch file for running tests
+  RunGrepTest.bat          a Windows batch file for pcre2grep tests


I'm satisfied that the tarball contents are correct; and that the README is up to date now with its "manifest" of the tarball contents.

NWilson · 2024-12-09T21:23:55Z

configure.ac

@@ -25,7 +25,7 @@ m4_define(libpcre2_posix_version, [3:5:0])
 AC_PREREQ([2.60])
 AC_INIT([PCRE2],pcre2_major.pcre2_minor[]pcre2_prerelease,[],[pcre2])
 AC_CONFIG_SRCDIR([src/pcre2.h.in])
-AM_INIT_AUTOMAKE([dist-bzip2 dist-zip])
+AM_INIT_AUTOMAKE([dist-bzip2 dist-zip foreign])


This is not ideal... but I ended up adding "foreign" as the path of least resistance.

The problem is, I want to just add some "FOO.md" files to the tarball. That's totally utterly reasonable. And Automake 1.16 endorses this: it special-cases them, according to the fact the latest version of the GNU packaging standards say, "great, use FOO.md instead of FOO files".

But then you have the annoying issue, because Automake 1.15 does NOT special-case them, it turned out to be really awkward to get the same behaviour from both Automake versions, just using the original configure.ac file.

So, this is the route I went down.

Autotools makes me cry (apologies if anyone is a fan...) but there's just so much time wasted every time you try and make modest changes, on every OSS or commercial project I've ever worked on that used Autotools!

NWilson · 2024-12-09T21:25:35Z

maint/CheckTxt

@@ -0,0 +1,64 @@
+#!/usr/bin/perl


New file, to stop us checking in files with CRLF endings, or invalid UTF-8, or anything non-ASCII in files that get processed by the C compiler. (Unicode documentation files are fine.)

Just like existing files in maint/, only runs on relevant parts of the tarball (eg. leaves testdata well alone).

NWilson · 2024-12-09T21:28:14Z

src/config.h.in

@@ -1,470 +0,0 @@
-/* src/config.h.in.  Generated from configure.ac by autoheader.  */


We're not checking in to git any of the outputs from ./autogen.sh - except this one? I'm not sure why. We do check in config.h.generic, which is reasonable (to me).

This is a very good question. This file always changes.

I guess it's just a historical. It's been in the repo since 2015 - Philip had previous removed it in 2014, then checked it back in again.

Philip added it back alongside src/config.h, which was subsequently removed (again). But config.h.in was left.

I think it should just go. We have config.h.generic for users who want to work with an alternative build system to autoconf.

I might be wrong, but I think the idea was that then you can base your build in this file without having to run auto tools at all.

Agree that is somehow redundant with config.h.generic, but it is because "generic" is a derivative from it (through a Makefile rule)

NWilson

I've added some verbose justification for the changes I've made.

zherczeg · 2024-12-10T09:32:55Z

src/config.h.generic

@@ -58,7 +58,7 @@ sure both macros are undefined; an emulation function will then be used. */
 /* Define this if your compiler supports __attribute__((uninitialized)) */
 /* #undef HAVE_ATTRIBUTE_UNINITIALIZED */

-/* Define to 1 if you have the 'bcopy' function. */
+/* Define to 1 if you have the `bcopy' function. */


Is this a new style?

Interesting, yes, I noticed this too.

autotools-mirror/autoconf@64df9b4#diff-f3bb1e17a70d2e4733de29089bdd13e951406a1a15e4b6b0f5688668f5acbb2bR79

I found a change made a few years ago on the ‎lib/autoconf/functions.m4:80 file:

# Prepare the autoheader snippet for FUNCTION. m4_define([_AH_CHECK_FUNC], [AH_TEMPLATE(AS_TR_CPP([HAVE_$1]), - [Define to 1 if you have the `$1' function.])]) + [Define to 1 if you have the '$1' function.])]) # _AC_CHECK_FUNCS_ONE_U(FUNCTION) # -------------------------------

So apparently the 'straight' quotes are the new style, in Autoconf 2.72 (shipped in Ubuntu 24.10 but not 24.04).

I honestly have no idea how the straight quotes came to be in the existing copy of config.h.generic. It's clearly a quite out-of-date file, not touched since the last PCRE2 release. I hypothesise that Philip perhaps had Autoconf 2.72 installed on his machine which last performed the update.

Let's just go with whatever style is produced by the version of Autoconf in Ubuntu 24.04, since that's what is most convenient to use in CI.

I think it is because Philip runs with LC_ALL=C to avoid introducing "Unicode" quotes like this

It's an ASCII ` backtick character.

I tried re-running export LANG=C LC_ALL=C; ./autogen.sh; ./configure.sh; rm src/config.h.generic; make src/config.h.generic all in a totally clean checkout. No change.

It really is due to the commit I linked above - the Autoconf behaviour has changed.

Philip uses Arch Linux I remember now. They updated to Autoconf 2.72, before Ubuntu did. So that's how Philip was using it this summer when he did the 10.44 release.

carenas · 2024-12-11T09:03:01Z

AUTHORS.md


-The maintainers are grateful for all contributions and participation over the years, whether or not listed here in this recent summary.
+This list is includes names up until the PCRE2 10.44 release. New names will be


I think "is" is not grammatically correct here

!! Well spotted, thanks.

Details on new maintainership

d16d50d

NWilson added 2 commits December 7, 2024 22:19

Run PrepareRelease

e880989

Add job to check for PrepareRelease cleanliness

4045b9f

NWilson force-pushed the user/niwilson/docs-tidy branch 5 times, most recently from 3b892f2 to 734f244 Compare December 8, 2024 19:44

Some small fixes to the CI job

fce20d3

NWilson force-pushed the user/niwilson/docs-tidy branch 2 times, most recently from 8cf8fda to fce20d3 Compare December 8, 2024 19:51

NWilson commented Dec 9, 2024

View reviewed changes

LICENCE.md Outdated Show resolved Hide resolved

NWilson added 4 commits December 9, 2024 11:04

Further dist polishing

54bd8b7

Remove checked-in autoconf outputs

6a9d9fb

Detrail some more files

7c38a2b

Merge remote-tracking branch 'origin/master' into user/niwilson/docs-…

7206737

…tidy

NWilson commented Dec 9, 2024

View reviewed changes

NWilson added 8 commits December 9, 2024 14:40

Add Ubuntu-20.04 autoconf runner

03e25d1

Update Philip's copyright years

06221ad

Tentative override of automake issue

5999827

Make CMake installed files match autoconf

f0ed196

Another attempt to deal with old & new automake

e96c6bf

Readme updates

b2936ba

Run PrepareRelease

3d48d12

More validation of text files

ca1427d

NWilson mentioned this pull request Dec 9, 2024

PCRE2 10.45 release plan #532

Closed

NWilson commented Dec 9, 2024

View reviewed changes

zherczeg reviewed Dec 10, 2024

View reviewed changes

Update acknowledgements

8d21cb7

carenas reviewed Dec 11, 2024

View reviewed changes

Fix typo

f229627

NWilson merged commit ac528f2 into master Dec 11, 2024
23 checks passed

NWilson deleted the user/niwilson/docs-tidy branch December 11, 2024 09:55

		# processing of the documentation, detrails files, and creates pcre2.h.generic
		# and config.h.generic (for use by builders who can't run ./configure).

		RunTest.bat a Windows batch file for running tests
		RunGrepTest.bat a Windows batch file for pcre2grep tests

		@@ -1,470 +0,0 @@
		/* src/config.h.in. Generated from configure.ac by autoheader. */


		The maintainers are grateful for all contributions and participation over the years, whether or not listed here in this recent summary.
		This list is includes names up until the PCRE2 10.44 release. New names will be

Details on new maintainership #603

Details on new maintainership #603

Conversation

NWilson commented Dec 7, 2024

PhilipHazel commented Dec 7, 2024

zherczeg commented Dec 7, 2024

NWilson commented Dec 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NWilson commented Dec 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NWilson Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

carenas Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NWilson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carenas Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NWilson Dec 9, 2024 •

edited

Loading

carenas Dec 10, 2024 •

edited

Loading

carenas Dec 11, 2024 •

edited

Loading