Add support for automatically updating Unicode derived files

On Tue, Oct 29, 2019 at 6:06 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

Continuing the discussion from [0] and [1], here is a patch that
automates the process of updating Unicode derived files. Summary:

- Edit UNICODE_VERSION and/or CLDR_VERSION in src/Makefile.global.in
- Run make update-unicode
- Commit

Hi Peter,

I gave "make update-unicode" a try. It's unclear to me what the state
of the build tree should be when a maintainer runs this, so I'll just
report what happens when running naively (on MacOS).

After only running configure, "make update-unicode" gives this error
at normalization-check:

ld: library not found for -lpgcommon
clang: error: linker command failed with exit code 1 (use -v to see invocation)

After commenting that out, the next command "$(MAKE) -C
contrib/unaccent $@" failed, seemingly because $(PYTHON) is empty
unless --with-python was specified at configure time.

Open questions that are currently not handled consistently:

- Should the downloaded files be listed in .gitignore?

These files are transient byproducts of a build, and we don't want
them committed, so they seem like a normal candidate for .gitignore.

- Should the downloaded files be cleaned by make clean (or distclean or
maintainer-clean or none)?

It seems one would want to make clean without removing these files,
and maintainer clean is for removing things that are preserved in
distribution tarballs. So I would go with distclean.

- Should the generated files be excluded from pgindent? Currently, the
generated files will not pass pgindent unchanged, so that could cause
annoying whitespace battles when these files are updated and re-indented
around release time.

I see what you mean in the norm table header. I think generated files
should not be pgindent'd, since creating them is already a consistent,
mechanical process, and their presentation is not as important as
other code.

Other comments:

+print "/* generated by
src/common/unicode/generate-unicode_combining_table.pl, do not edit
*/\n\n";

I would print out the full boilerplate like for other generated headers.

Lastly, src/common/unicode/README is outdated (and possibly no longer
useful at all?).

--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2019-12-19 23:48, John Naylor wrote:

I gave "make update-unicode" a try. It's unclear to me what the state
of the build tree should be when a maintainer runs this, so I'll just
report what happens when running naively (on MacOS).

Yeah, that wasn't fully thought through, it appears.

After only running configure, "make update-unicode" gives this error
at normalization-check:

ld: library not found for -lpgcommon
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Fixed by adding more make dependencies.

After commenting that out, the next command "$(MAKE) -C
contrib/unaccent $@" failed, seemingly because $(PYTHON) is empty
unless --with-python was specified at configure time.

I'm not sure whether that's worth addressing.

Open questions that are currently not handled consistently:

- Should the downloaded files be listed in .gitignore?

These files are transient byproducts of a build, and we don't want
them committed, so they seem like a normal candidate for .gitignore.

OK done

- Should the downloaded files be cleaned by make clean (or distclean or
maintainer-clean or none)?

It seems one would want to make clean without removing these files,
and maintainer clean is for removing things that are preserved in
distribution tarballs. So I would go with distclean.

also done

- Should the generated files be excluded from pgindent? Currently, the
generated files will not pass pgindent unchanged, so that could cause
annoying whitespace battles when these files are updated and re-indented
around release time.

I see what you mean in the norm table header. I think generated files
should not be pgindent'd, since creating them is already a consistent,
mechanical process, and their presentation is not as important as
other code.

I've left it alone for now because the little indentation problem
currently present might actually go away with my Unicode normalization
support patch.

Other comments:

+print "/* generated by
src/common/unicode/generate-unicode_combining_table.pl, do not edit
*/\n\n";

I would print out the full boilerplate like for other generated headers.

Hmm, you are probably comparing with
src/common/unicode/generate-unicode_norm_table.pl, but other file
generating scripts around the tree print out a small header in the style
that I have. I'd rather adjust the output of
generate-unicode_norm_table.pl to match those. (It's also not quite
correct to make copyright claims about automatically generated output.)

Lastly, src/common/unicode/README is outdated (and possibly no longer
useful at all?).

updated

new patch attached

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Dec 26, 2019 at 12:39 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-12-19 23:48, John Naylor wrote:

I would print out the full boilerplate like for other generated headers.

Hmm, you are probably comparing with
src/common/unicode/generate-unicode_norm_table.pl, but other file
generating scripts around the tree print out a small header in the style
that I have. I'd rather adjust the output of
generate-unicode_norm_table.pl to match those. (It's also not quite
correct to make copyright claims about automatically generated output.)

Hmm, the scripts I'm most familiar with have full headers. Your point
about copyright makes sense, and using smaller file headers would aid
readability of the scripts, but I also see how others may feel
differently.

v2 looks good to me, marked ready for committer.

--
John Naylor https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-01-03 15:13, John Naylor wrote:

On Thu, Dec 26, 2019 at 12:39 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 2019-12-19 23:48, John Naylor wrote:

I would print out the full boilerplate like for other generated headers.

Hmm, you are probably comparing with
src/common/unicode/generate-unicode_norm_table.pl, but other file
generating scripts around the tree print out a small header in the style
that I have. I'd rather adjust the output of
generate-unicode_norm_table.pl to match those. (It's also not quite
correct to make copyright claims about automatically generated output.)

Hmm, the scripts I'm most familiar with have full headers. Your point
about copyright makes sense, and using smaller file headers would aid
readability of the scripts, but I also see how others may feel
differently.

v2 looks good to me, marked ready for committer.

Committed, thanks.

I have added a little tweak so that it works also without --with-python,
to avoid gratuitous annoyances.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

Committed, thanks.

This patch is making src/tools/pginclude/headerscheck unhappy:

./src/include/common/unicode_combining_table.h:3: error: array type has incomplete element type

I guess that header needs another #include, or else you need to
move some declarations around.

regards, tom lane

On 2020-01-15 01:37, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

Committed, thanks.

This patch is making src/tools/pginclude/headerscheck unhappy:

./src/include/common/unicode_combining_table.h:3: error: array type has incomplete element type

I guess that header needs another #include, or else you need to
move some declarations around.

Hmm, this file is only meant to be included inside one particular
function. Making it standalone includable would seem to be unnecessary.
What should we do?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-01-15 01:37, Tom Lane wrote:

This patch is making src/tools/pginclude/headerscheck unhappy:
./src/include/common/unicode_combining_table.h:3: error: array type has incomplete element type

Hmm, this file is only meant to be included inside one particular
function. Making it standalone includable would seem to be unnecessary.
What should we do?

Well, we could make it a documented exception in headerscheck and
cpluspluscheck.

regards, tom lane

On 2020-01-20 16:43, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2020-01-15 01:37, Tom Lane wrote:

This patch is making src/tools/pginclude/headerscheck unhappy:
./src/include/common/unicode_combining_table.h:3: error: array type has incomplete element type

Hmm, this file is only meant to be included inside one particular
function. Making it standalone includable would seem to be unnecessary.
What should we do?

Well, we could make it a documented exception in headerscheck and
cpluspluscheck.

OK, done.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

I have committed the first Unicode data update using this new "make
update-unicode" facility.

CLDR is released regularly every 6 months, so around this time every
year would be the appropriate time to pull in the latest updates in
preparation for our own release.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services