Re: Add support for automatically updating Unicode derived files

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