Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8; format=Flowed
Date: Fri, 22 Mar 2024 12:29:21 -0500
Message-Id: <D00GG8ZIB0HM.2GA5QIFR7ACE5@neon.tech>
Cc: "pgsql-hackers" <pgsql-hackers@postgresql.org>
Subject: Re: make dist using git archive
To: "Peter Eisentraut" <peter@eisentraut.org>
From: "Tristan Partin" <tristan@neon.tech>
References: <40e80f77-a294-4f29-a16f-e21bc7bc75fc@eisentraut.org>
 <CYLI6IZ5RAVR.3RBBF4F3P8MHX@neon.tech>
 <673bbcc2-3e38-44b6-9710-e7c1b2446334@eisentraut.org>
 <CYNXE6A4IB2S.V1YYOX54IVJ@neon.tech>
 <dbff5b4f-21a9-4340-82ee-884b13ee6a3d@eisentraut.org>
 <ac794e4f-28a5-410a-91ec-a7de31de7dc8@eisentraut.org>
 <c9809ea4-5094-4c29-8889-d626043ac112@eisentraut.org>
In-Reply-To: <c9809ea4-5094-4c29-8889-d626043ac112@eisentraut.org>
Archived-At: <https://www.postgresql.org/message-id/D00GG8ZIB0HM.2GA5QIFR7ACE5%40neon.tech>
Precedence: bulk

On Thu Mar 21, 2024 at 3:44 AM CDT, Peter Eisentraut wrote:
> Here is an updated version of this patch set.

You should add 'disabler: true' to the git find_program in Meson. If Git=20
doesn't exist on the system with the way your patch is currently=20
written, the targets would be defined, even though they would never=20
succeed.

You may also want to make sure that we are actually in a Git repository.=20
I don't think git-archive works outside one.

Re the autoclrf, is this something we could throw in a .gitattributes=20
files?

> I have removed the "dirty check" stuff.  It didn't really work well/was=
=20
> buggy under meson, and it failed mysteriously on the Linux CI tasks.  So=
=20
> let's just drop that functionality for now.
>
> I have also added a more complete commit message and some more code=20
> comments.

> Meson has its own distribution building command (meson dist), but we
> are not using that at this point.  The main problem is that the way
> they have implemented it, it is not deterministic in the above sense.
> Also, we want a "make" version for the time being.  But the target
> name "dist" in meson is reserved for that reason, so we call the
> custom target "pgdist" (so call something like "meson compile -C build
> pgdist").

I would suggest poisoning `meson dist` in the following way:

if not meson.is_subproject()
	# Maybe edit the comment...Maybe tell perl to print this message=20
	# instead and then exit non-zero?
	#
	# Meson has its own distribution building command (meson dist), but we
	# are not using that at this point.  The main problem is that the way
	# they have implemented it, it is not deterministic in the above sense.
	# Also, we want a "make" version for the time being.  But the target
	# name "dist" in meson is reserved for that reason, so we call the
	# custom target "pgdist" (so call something like "meson compile -C build
	# pgdist").
	#
	# We don't poison the dist if we are a subproject because it is=20
	# possible that the parent project may want to create a dist using=20
	# the builtin Meson method.
	meson.add_dist_script(perl, '-e', 'exit 1')
endif

> I have extracted the freebsd CI script fix into a separate patch (0002).=
=20
>   I think this is useful even if we don't take the full CI patch (0003).

0002 looks pretty reasonable to me.

> About the 0003 patch: It seems useful in principle to test these things=
=20
> continuously.  The dist script runs about 10 seconds in each task, and=20
> takes a bit of disk space for the artifacts.  I'm not sure to what=20
> degree this might bother someone.

0003 works for me :).

--=20
Tristan Partin
Neon (https://neon.tech)