public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v24 3/7] Add default_toast_compression GUC
18+ messages / 6 participants
[nested] [flat]
* [PATCH v24 3/7] Add default_toast_compression GUC
@ 2021-01-30 03:37 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Justin Pryzby @ 2021-01-30 03:37 UTC (permalink / raw)
---
src/backend/access/common/tupdesc.c | 2 +-
src/backend/bootstrap/bootstrap.c | 3 +-
src/backend/commands/amcmds.c | 143 +++++++++++++++++++++++++++-
src/backend/commands/tablecmds.c | 4 +-
src/backend/utils/init/postinit.c | 4 +
src/backend/utils/misc/guc.c | 12 +++
src/include/access/amapi.h | 2 +
src/include/access/compressamapi.h | 12 ++-
8 files changed, 176 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index ca26fab487..7afaea000b 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -668,7 +668,7 @@ TupleDescInitEntry(TupleDesc desc,
att->attcollation = typeForm->typcollation;
if (IsStorageCompressible(typeForm->typstorage))
- att->attcompression = DefaultCompressionOid;
+ att->attcompression = GetDefaultToastCompression();
else
att->attcompression = InvalidOid;
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 9b451eaa71..ec3376cf8a 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -732,8 +732,9 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
attrtypes[attnum]->attcacheoff = -1;
attrtypes[attnum]->atttypmod = -1;
attrtypes[attnum]->attislocal = true;
+
if (IsStorageCompressible(attrtypes[attnum]->attstorage))
- attrtypes[attnum]->attcompression = DefaultCompressionOid;
+ attrtypes[attnum]->attcompression = GetDefaultToastCompression();
else
attrtypes[attnum]->attcompression = InvalidOid;
diff --git a/src/backend/commands/amcmds.c b/src/backend/commands/amcmds.c
index 3ad4a61739..1682afd2a4 100644
--- a/src/backend/commands/amcmds.c
+++ b/src/backend/commands/amcmds.c
@@ -13,8 +13,11 @@
*/
#include "postgres.h"
+#include "access/amapi.h"
+#include "access/compressamapi.h"
#include "access/htup_details.h"
#include "access/table.h"
+#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
@@ -27,13 +30,20 @@
#include "parser/parse_func.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
+#include "utils/guc.h"
+#include "utils/inval.h"
#include "utils/rel.h"
#include "utils/syscache.h"
-
static Oid lookup_am_handler_func(List *handler_name, char amtype);
static const char *get_am_type_string(char amtype);
+/* Compile-time default */
+char *default_toast_compression = DEFAULT_TOAST_COMPRESSION; // maybe needs pg_dump support ?
+/* Invalid means need to lookup the text value in the catalog */
+static Oid default_toast_compression_oid = InvalidOid;
+
+static void AccessMethodCallback(Datum arg, int cacheid, uint32 hashvalue);
/*
* CreateAccessMethod
@@ -277,3 +287,134 @@ lookup_am_handler_func(List *handler_name, char amtype)
return handlerOid;
}
+
+/* check_hook: validate new default_toast_compression */
+bool
+check_default_toast_compression(char **newval, void **extra, GucSource source)
+{
+ if (**newval == '\0')
+ {
+ GUC_check_errdetail("%s cannot be empty.",
+ "default_toast_compression");
+ return false;
+ }
+
+ if (strlen(*newval) >= NAMEDATALEN)
+ {
+ GUC_check_errdetail("%s is too long (maximum %d characters).",
+ "default_toast_compression", NAMEDATALEN - 1);
+ return false;
+ }
+
+ /*
+ * If we aren't inside a transaction, or not connected to a database, we
+ * cannot do the catalog access necessary to verify the method. Must
+ * accept the value on faith.
+ */
+ if (IsTransactionState() && MyDatabaseId != InvalidOid)
+ {
+ if (!OidIsValid(get_compression_am_oid(*newval, true)))
+ {
+ /*
+ * When source == PGC_S_TEST, don't throw a hard error for a
+ * nonexistent table access method, only a NOTICE. See comments in
+ * guc.h.
+ */
+ if (source == PGC_S_TEST)
+ {
+ ereport(NOTICE,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("compression access method \"%s\" does not exist",
+ *newval)));
+ }
+ else
+ {
+ GUC_check_errdetail("Compression access method \"%s\" does not exist.",
+ *newval);
+ return false;
+ }
+ }
+ }
+
+ return true;
+}
+
+/*
+ * assign_default_toast_compression: GUC assign_hook for default_toast_compression
+ */
+void
+assign_default_toast_compression(const char *newval, void *extra)
+{
+ /*
+ * Invalidate setting, forcing it to be looked up as needed.
+ * This avoids trying to do database access during GUC initialization,
+ * or outside a transaction.
+ */
+
+ default_toast_compression = NULL;
+fprintf(stderr, "set to null\n");
+}
+
+
+/*
+ * GetDefaultToastCompression -- get the OID of the current toast compression
+ *
+ * This exists to hide and optimize the use of the default_toast_compression
+ * GUC variable.
+ */
+Oid
+GetDefaultToastCompression(void)
+{
+ /* Avoid catalog access during bootstrap, and for default compression */
+ if (strcmp(default_toast_compression, DEFAULT_TOAST_COMPRESSION) == 0)
+ return PGLZ_COMPRESSION_AM_OID;
+
+ /* Cannot call get_compression_am_oid this early */
+ // if (IsBootstrapProcessingMode())
+ // return PGLZ_COMPRESSION_AM_OID;
+ Assert(!IsBootstrapProcessingMode());
+
+ /*
+ * If cached value isn't valid, look up the current default value, caching
+ * the result
+ */
+ if (!OidIsValid(default_toast_compression_oid))
+ default_toast_compression_oid =
+ get_am_type_oid(default_toast_compression, AMTYPE_COMPRESSION,
+ false);
+
+ return default_toast_compression_oid;
+}
+
+/*
+ * InitializeAccessMethods: initialize module during InitPostgres.
+ *
+ * This is called after we are up enough to be able to do catalog lookups.
+ */
+void
+InitializeAccessMethods(void)
+{
+ if (IsBootstrapProcessingMode())
+ return;
+
+ /*
+ * In normal mode, arrange for a callback on any syscache invalidation
+ * of pg_am rows.
+ */
+ CacheRegisterSyscacheCallback(AMOID,
+ AccessMethodCallback,
+ (Datum) 0);
+ /* Force cached default access method to be recomputed on next use */
+ // default_toast_compression_oid = InvalidOid;
+}
+
+/*
+ * AccessMethodCallback
+ * Syscache inval callback function
+ */
+static void
+AccessMethodCallback(Datum arg, int cacheid, uint32 hashvalue)
+{
+ /* Force look up of compression oid on next use */
+ default_toast_compression_oid = false;
+}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dd81d5bf4e..72ba017814 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -11925,7 +11925,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
if (!IsStorageCompressible(tform->typstorage))
attTup->attcompression = InvalidOid;
else if (!OidIsValid(attTup->attcompression))
- attTup->attcompression = DefaultCompressionOid;
+ attTup->attcompression = GetDefaultToastCompression();
}
else
attTup->attcompression = InvalidOid;
@@ -17762,7 +17762,7 @@ GetAttributeCompression(Form_pg_attribute att, char *compression)
/* fallback to default compression if it's not specified */
if (compression == NULL)
- return DefaultCompressionOid;
+ return GetDefaultToastCompression();
amoid = get_compression_am_oid(compression, false);
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index e5965bc517..6a02bbd377 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -25,6 +25,7 @@
#include "access/session.h"
#include "access/sysattr.h"
#include "access/tableam.h"
+#include "access/amapi.h"
#include "access/xact.h"
#include "access/xlog.h"
#include "catalog/catalog.h"
@@ -1060,6 +1061,9 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
/* set default namespace search path */
InitializeSearchPath();
+ /* set callback for changes to pg_am */
+ InitializeAccessMethods();
+
/* initialize client encoding */
InitializeClientEncoding();
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index eafdb1118e..3a2b33fcd1 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -30,6 +30,7 @@
#include <unistd.h>
#include "access/commit_ts.h"
+#include "access/compressamapi.h"
#include "access/gin.h"
#include "access/rmgr.h"
#include "access/tableam.h"
@@ -3915,6 +3916,17 @@ static struct config_string ConfigureNamesString[] =
check_default_table_access_method, NULL, NULL
},
+ {
+ {"default_toast_compression", PGC_USERSET, CLIENT_CONN_STATEMENT,
+ gettext_noop("Sets the default compression for new columns."),
+ NULL,
+ GUC_IS_NAME
+ },
+ &default_toast_compression,
+ DEFAULT_TOAST_COMPRESSION,
+ check_default_toast_compression, assign_default_toast_compression, NULL, NULL
+ },
+
{
{"default_tablespace", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the default tablespace to create tables and indexes in."),
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index d357ebb559..1513cafcf4 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -287,4 +287,6 @@ typedef struct IndexAmRoutine
extern IndexAmRoutine *GetIndexAmRoutine(Oid amhandler);
extern IndexAmRoutine *GetIndexAmRoutineByAmId(Oid amoid, bool noerror);
+void InitializeAccessMethods(void);
+
#endif /* AMAPI_H */
diff --git a/src/include/access/compressamapi.h b/src/include/access/compressamapi.h
index 5a8e23d926..d75a8e9df2 100644
--- a/src/include/access/compressamapi.h
+++ b/src/include/access/compressamapi.h
@@ -17,6 +17,7 @@
#include "catalog/pg_am_d.h"
#include "nodes/nodes.h"
+#include "utils/guc.h"
/*
* Built-in compression method-id. The toast compression header will store
@@ -29,8 +30,17 @@ typedef enum CompressionId
LZ4_COMPRESSION_ID = 1
} CompressionId;
-/* Use default compression method if it is not specified. */
+/* Default compression method if not specified. */
+#define DEFAULT_TOAST_COMPRESSION "pglz"
#define DefaultCompressionOid PGLZ_COMPRESSION_AM_OID
+
+/* GUC */
+extern char *default_toast_compression;
+extern void assign_default_toast_compression(const char *newval, void *extra);
+extern bool check_default_toast_compression(char **newval, void **extra, GucSource source);
+
+extern Oid GetDefaultToastCompression(void);
+
#define IsStorageCompressible(storage) ((storage) != TYPSTORAGE_PLAIN && \
(storage) != TYPSTORAGE_EXTERNAL)
/* compression handler routines */
--
2.17.0
--YZ5djTAD1cGYuMQK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v24-0004-default-to-with-lz4.patch"
^ permalink raw reply [nested|flat] 18+ messages in thread
* make dist using git archive
@ 2024-01-22 07:31 Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-22 07:31 UTC (permalink / raw)
To: pgsql-hackers
One of the goals is to make the creation of the distribution tarball
more directly traceable to the git repository. That is why we removed
the "make distprep" step.
Here I want to take another step in that direction, by changing "make
dist" to directly use "git archive", rather than the custom shell script
it currently runs.
The simple summary is that it would run
git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
postgresql-17devel.tar.gz
(with appropriate version numbers, of course), and that's the tarball we
would ship.
There are analogous commands for other compression formats.
The actual command gets subtly more complicated if you need to run this
in a separate build directory. In my attached patch, the make version
doesn't support vpath at the moment, just so that it's easier to
understand for now. The meson version is a bit hairy.
I have studied and tested this quite a bit, and I have found that the
archives produced this way are deterministic and reproducible, meaning
for a given commit the result file should always be bit-for-bit identical.
The exception is that if you use a git version older than 2.38.0, gzip
records the platform in the archive, so you'd get a different output on
Windows vs. macOS vs. "UNIX" (everything else). In git 2.38.0, this was
changed so that everything is recorded as "UNIX" now. This is just
something to keep in mind. This issue is specific to the gzip format,
it does not affect other compression formats.
Meson has its own distribution building command (meson dist), but opted
against using that. The main problem is that the way they have
implemented it, it is not deterministic in the above sense. (Another
point is of course that we probably want a "make" version for the time
being.)
But the target name "dist" in meson is reserved for that reason, so I
needed to call the custom target "pgdist".
I did take one idea from meson: It runs a check before git archive that
the checkout is clean. That way, you avoid mistakes because of
uncommitted changes. This works well in my "make" implementation. In
the meson implementation, I had to find a workaround, because a
custom_target cannot have a dependency on a run_target. As also
mentioned above, the whole meson implementation is a bit ugly.
Anyway, with the attached patch you can do
make dist
or
meson compile -C build pgdist
and it produces the same set of tarballs as before, except it's done
differently.
The actual build scripts need some fine-tuning, but the general idea is
correct, I think.
From 4b128faca90238d0a0bb6949a8050c2501d1bd67 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 20 Jan 2024 21:54:36 +0100
Subject: [PATCH v0] make dist uses git archive
---
GNUmakefile.in | 34 ++++++++++++----------------------
meson.build | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..3e04785ada2 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
distdir = postgresql-$(VERSION)
dummy = =install=
+GIT = git
+
dist: $(distdir).tar.gz $(distdir).tar.bz2
- rm -rf $(distdir)
-
-$(distdir).tar: distdir
- $(TAR) chf $@ $(distdir)
-
-.INTERMEDIATE: $(distdir).tar
-
-distdir-location:
- @echo $(distdir)
-
-distdir:
- rm -rf $(distdir)* $(dummy)
- for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
- file=`expr X$$x : 'X\./\(.*\)'`; \
- if test -d "$(top_srcdir)/$$file" ; then \
- mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
- else \
- ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
- || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
- fi || exit; \
- done
- $(MAKE) -C $(distdir) distclean
+
+.PHONY: check-dirty-index
+check-dirty-index:
+ $(GIT) diff-index --quiet HEAD
+
+$(distdir).tar.gz: check-dirty-index
+ $(GIT) archive --format tar.gz --prefix $(distdir)/ HEAD -o $@
+
+$(distdir).tar.bz2: check-dirty-index
+ $(GIT) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $@
distcheck: dist
rm -rf $(dummy)
diff --git a/meson.build b/meson.build
index c317144b6bc..f0d870c5192 100644
--- a/meson.build
+++ b/meson.build
@@ -3347,6 +3347,44 @@ run_target('help',
+###############################################################
+# Distribution archive
+###############################################################
+
+git = find_program('git', required: false, native: true, disabler: true)
+bzip2 = find_program('bzip2', required: false, native: true, disabler: true)
+
+distdir = meson.project_name() + '-' + meson.project_version()
+
+check_dirty_index = run_target('check-dirty-index',
+ command: [git, 'diff-index', '--quiet', 'HEAD'])
+
+tar_gz = custom_target('tar.gz',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', 'archive',
+ '--format', 'tar.gz',
+ '--prefix', distdir + '/',
+ '-o', '@BUILD_ROOT@/@OUTPUT@',
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.gz',
+)
+
+tar_bz2 = custom_target('tar.bz2',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command=' + bzip2.path() + ' -c', 'archive',
+ '--format', 'tar.bz2',
+ '--prefix', distdir + '/',
+ '-o', '@BUILD_ROOT@/@OUTPUT@',
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.bz2',
+)
+
+alias_target('pgdist', [check_dirty_index, tar_gz, tar_bz2])
+
+
+
###############################################################
# The End, The End, My Friend
###############################################################
--
2.43.0
Attachments:
[text/plain] v0-0001-make-dist-uses-git-archive.patch (3.2K, ../../[email protected]/2-v0-0001-make-dist-uses-git-archive.patch)
download | inline diff:
From 4b128faca90238d0a0bb6949a8050c2501d1bd67 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 20 Jan 2024 21:54:36 +0100
Subject: [PATCH v0] make dist uses git archive
---
GNUmakefile.in | 34 ++++++++++++----------------------
meson.build | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..3e04785ada2 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
distdir = postgresql-$(VERSION)
dummy = =install=
+GIT = git
+
dist: $(distdir).tar.gz $(distdir).tar.bz2
- rm -rf $(distdir)
-
-$(distdir).tar: distdir
- $(TAR) chf $@ $(distdir)
-
-.INTERMEDIATE: $(distdir).tar
-
-distdir-location:
- @echo $(distdir)
-
-distdir:
- rm -rf $(distdir)* $(dummy)
- for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
- file=`expr X$$x : 'X\./\(.*\)'`; \
- if test -d "$(top_srcdir)/$$file" ; then \
- mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
- else \
- ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
- || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
- fi || exit; \
- done
- $(MAKE) -C $(distdir) distclean
+
+.PHONY: check-dirty-index
+check-dirty-index:
+ $(GIT) diff-index --quiet HEAD
+
+$(distdir).tar.gz: check-dirty-index
+ $(GIT) archive --format tar.gz --prefix $(distdir)/ HEAD -o $@
+
+$(distdir).tar.bz2: check-dirty-index
+ $(GIT) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $@
distcheck: dist
rm -rf $(dummy)
diff --git a/meson.build b/meson.build
index c317144b6bc..f0d870c5192 100644
--- a/meson.build
+++ b/meson.build
@@ -3347,6 +3347,44 @@ run_target('help',
+###############################################################
+# Distribution archive
+###############################################################
+
+git = find_program('git', required: false, native: true, disabler: true)
+bzip2 = find_program('bzip2', required: false, native: true, disabler: true)
+
+distdir = meson.project_name() + '-' + meson.project_version()
+
+check_dirty_index = run_target('check-dirty-index',
+ command: [git, 'diff-index', '--quiet', 'HEAD'])
+
+tar_gz = custom_target('tar.gz',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', 'archive',
+ '--format', 'tar.gz',
+ '--prefix', distdir + '/',
+ '-o', '@BUILD_ROOT@/@OUTPUT@',
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.gz',
+)
+
+tar_bz2 = custom_target('tar.bz2',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command=' + bzip2.path() + ' -c', 'archive',
+ '--format', 'tar.bz2',
+ '--prefix', distdir + '/',
+ '-o', '@BUILD_ROOT@/@OUTPUT@',
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.bz2',
+)
+
+alias_target('pgdist', [check_dirty_index, tar_gz, tar_bz2])
+
+
+
###############################################################
# The End, The End, My Friend
###############################################################
--
2.43.0
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-22 12:10 Junwang Zhao <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Junwang Zhao @ 2024-01-22 12:10 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
Hi,
On Mon, Jan 22, 2024 at 3:32 PM Peter Eisentraut <[email protected]> wrote:
>
> One of the goals is to make the creation of the distribution tarball
> more directly traceable to the git repository. That is why we removed
> the "make distprep" step.
>
> Here I want to take another step in that direction, by changing "make
> dist" to directly use "git archive", rather than the custom shell script
> it currently runs.
>
> The simple summary is that it would run
>
> git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
> postgresql-17devel.tar.gz
>
> (with appropriate version numbers, of course), and that's the tarball we
> would ship.
>
> There are analogous commands for other compression formats.
>
> The actual command gets subtly more complicated if you need to run this
> in a separate build directory. In my attached patch, the make version
> doesn't support vpath at the moment, just so that it's easier to
> understand for now. The meson version is a bit hairy.
>
> I have studied and tested this quite a bit, and I have found that the
> archives produced this way are deterministic and reproducible, meaning
> for a given commit the result file should always be bit-for-bit identical.
>
> The exception is that if you use a git version older than 2.38.0, gzip
> records the platform in the archive, so you'd get a different output on
> Windows vs. macOS vs. "UNIX" (everything else). In git 2.38.0, this was
> changed so that everything is recorded as "UNIX" now. This is just
> something to keep in mind. This issue is specific to the gzip format,
> it does not affect other compression formats.
>
> Meson has its own distribution building command (meson dist), but opted
> against using that. The main problem is that the way they have
> implemented it, it is not deterministic in the above sense. (Another
> point is of course that we probably want a "make" version for the time
> being.)
>
> But the target name "dist" in meson is reserved for that reason, so I
> needed to call the custom target "pgdist".
>
> I did take one idea from meson: It runs a check before git archive that
> the checkout is clean. That way, you avoid mistakes because of
> uncommitted changes. This works well in my "make" implementation. In
> the meson implementation, I had to find a workaround, because a
> custom_target cannot have a dependency on a run_target. As also
> mentioned above, the whole meson implementation is a bit ugly.
>
> Anyway, with the attached patch you can do
>
> make dist
>
> or
>
> meson compile -C build pgdist
I played this with meson build on macOS, the packages are generated
in source root but not build root, I'm sure if this is by design but I think
polluting *working directory* is not good.
Another thing I'd like to point out is, should we also introduce *git commit*
or maybe *git tag* to package name, something like:
git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
postgresql-17devel-`git rev-parse --short HEAD`.tar.gz
git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
postgresql-`git describe --tags`.tar.gz
>
> and it produces the same set of tarballs as before, except it's done
> differently.
>
> The actual build scripts need some fine-tuning, but the general idea is
> correct, I think.
I think this is a good idea, thanks for working on this.
--
Regards
Junwang Zhao
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-22 18:35 Peter Eisentraut <[email protected]>
parent: Junwang Zhao <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-22 18:35 UTC (permalink / raw)
To: Junwang Zhao <[email protected]>; +Cc: pgsql-hackers
On 22.01.24 13:10, Junwang Zhao wrote:
> I played this with meson build on macOS, the packages are generated
> in source root but not build root, I'm sure if this is by design but I think
> polluting *working directory* is not good.
Yes, it's not good, but I couldn't find a way to make it work.
This is part of the complications with meson I referred to. The
@BUILD_ROOT@ placeholder in custom_target() is apparently always a
relative path, but it doesn't know that git -C changes the current
directory.
> Another thing I'd like to point out is, should we also introduce *git commit*
> or maybe *git tag* to package name, something like:
>
> git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
> postgresql-17devel-`git rev-parse --short HEAD`.tar.gz
> git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
> postgresql-`git describe --tags`.tar.gz
I'm not sure why we would need it built-in. It can be done by hand, of
course.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-22 20:04 Tristan Partin <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 2 replies; 18+ messages in thread
From: Tristan Partin @ 2024-01-22 20:04 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Mon Jan 22, 2024 at 1:31 AM CST, Peter Eisentraut wrote:
> From 4b128faca90238d0a0bb6949a8050c2501d1bd67 Mon Sep 17 00:00:00 2001
> From: Peter Eisentraut <[email protected]>
> Date: Sat, 20 Jan 2024 21:54:36 +0100
> Subject: [PATCH v0] make dist uses git archive
>
> ---
> GNUmakefile.in | 34 ++++++++++++----------------------
> meson.build | 38 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 22 deletions(-)
>
> diff --git a/GNUmakefile.in b/GNUmakefile.in
> index eba569e930e..3e04785ada2 100644
> --- a/GNUmakefile.in
> +++ b/GNUmakefile.in
> @@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
> distdir = postgresql-$(VERSION)
> dummy = =install=
>
> +GIT = git
> +
> dist: $(distdir).tar.gz $(distdir).tar.bz2
> - rm -rf $(distdir)
> -
> -$(distdir).tar: distdir
> - $(TAR) chf $@ $(distdir)
> -
> -.INTERMEDIATE: $(distdir).tar
> -
> -distdir-location:
> - @echo $(distdir)
> -
> -distdir:
> - rm -rf $(distdir)* $(dummy)
> - for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
> - file=`expr X$$x : 'X\./\(.*\)'`; \
> - if test -d "$(top_srcdir)/$$file" ; then \
> - mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
> - else \
> - ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
> - || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
> - fi || exit; \
> - done
> - $(MAKE) -C $(distdir) distclean
> +
> +.PHONY: check-dirty-index
> +check-dirty-index:
> + $(GIT) diff-index --quiet HEAD
> +
> +$(distdir).tar.gz: check-dirty-index
> + $(GIT) archive --format tar.gz --prefix $(distdir)/ HEAD -o $@
> +
> +$(distdir).tar.bz2: check-dirty-index
> + $(GIT) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $@
>
> distcheck: dist
> rm -rf $(dummy)
> diff --git a/meson.build b/meson.build
> index c317144b6bc..f0d870c5192 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3347,6 +3347,44 @@ run_target('help',
>
>
>
> +###############################################################
> +# Distribution archive
> +###############################################################
> +
> +git = find_program('git', required: false, native: true, disabler: true)
> +bzip2 = find_program('bzip2', required: false, native: true, disabler: true)
This doesn't need to be a disabler. git is fine as-is. See later
comment. Disablers only work like you are expecting when they are used
like how git is used. Once you call a method like .path(), all bets are
off.
> +distdir = meson.project_name() + '-' + meson.project_version()
> +
> +check_dirty_index = run_target('check-dirty-index',
> + command: [git, 'diff-index', '--quiet', 'HEAD'])
Seems like you might want to add -C here too?
> +
> +tar_gz = custom_target('tar.gz',
> + build_always_stale: true,
> + command: [git, '-C', '@SOURCE_ROOT@', 'archive',
> + '--format', 'tar.gz',
> + '--prefix', distdir + '/',
> + '-o', '@BUILD_ROOT@/@OUTPUT@',
> + 'HEAD', '.'],
> + install: false,
> + output: distdir + '.tar.gz',
> +)
> +
> +tar_bz2 = custom_target('tar.bz2',
> + build_always_stale: true,
> + command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command=' + bzip2.path() + ' -c', 'archive',
> + '--format', 'tar.bz2',
> + '--prefix', distdir + '/',
- '-o', '@BUILD_ROOT@/@OUTPUT@',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
This will generate the tarballs in the build directory. Do the same for
the previous target. Tested locally.
> + 'HEAD', '.'],
> + install: false,
> + output: distdir + '.tar.bz2',
> +)
The bz2 target should be wrapped in an `if bzip2.found()`. It is
possible for git to be found, but not bzip2. I might also define the bz2
command out of line. Also, you may want to add
these programs to meson_options.txt for overriding, even though the
"meson-ic" way is to use a machine file.
> +
> +alias_target('pgdist', [check_dirty_index, tar_gz, tar_bz2])
Are you intending for the check_dirty_index target to prohibit the other
two targets from running? Currently that is not the case. If it is what
you intend, use a stamp file or something to indicate a relationship.
Alternatively, inline the git diff-index into the other commands. These
might also do better as external scripts. It would reduce duplication
between the autotools and Meson builds.
> +
> +
> +
> ###############################################################
> # The End, The End, My Friend
> ###############################################################
I am not really following why we can't use the builtin Meson dist
command. The only difference from my testing is it doesn't use
a --prefix argument.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-23 02:14 Junwang Zhao <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Junwang Zhao @ 2024-01-23 02:14 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Tue, Jan 23, 2024 at 2:36 AM Peter Eisentraut <[email protected]> wrote:
>
> On 22.01.24 13:10, Junwang Zhao wrote:
> > I played this with meson build on macOS, the packages are generated
> > in source root but not build root, I'm sure if this is by design but I think
> > polluting *working directory* is not good.
>
> Yes, it's not good, but I couldn't find a way to make it work.
>
> This is part of the complications with meson I referred to. The
> @BUILD_ROOT@ placeholder in custom_target() is apparently always a
> relative path, but it doesn't know that git -C changes the current
> directory.
>
> > Another thing I'd like to point out is, should we also introduce *git commit*
> > or maybe *git tag* to package name, something like:
> >
> > git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
> > postgresql-17devel-`git rev-parse --short HEAD`.tar.gz
> > git archive --format tar.gz --prefix postgresql-17devel/ HEAD -o
> > postgresql-`git describe --tags`.tar.gz
>
> I'm not sure why we would need it built-in. It can be done by hand, of
> course.
If this is only used by the release phase, one can do this by hand.
*commit id/tag* in package name can be used to identify the git source,
which might be useful for cooperation between QA and dev team,
but surely there are better ways for this, so I do not have a strong
opinion here.
>
--
Regards
Junwang Zhao
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-23 09:30 Peter Eisentraut <[email protected]>
parent: Tristan Partin <[email protected]>
1 sibling, 2 replies; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-23 09:30 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; +Cc: pgsql-hackers
On 22.01.24 21:04, Tristan Partin wrote:
> I am not really following why we can't use the builtin Meson dist
> command. The only difference from my testing is it doesn't use a
> --prefix argument.
Here are some problems I have identified:
1. meson dist internally runs gzip without the -n option. That makes
the tar.gz archive include a timestamp, which in turn makes it not
reproducible.
2. Because gzip includes a platform indicator in the archive, the
produced tar.gz archive is not reproducible across platforms. (I don't
know if gzip has an option to avoid that. git archive uses an internal
gzip implementation that handles this.)
3. Meson does not support tar.bz2 archives.
4. Meson uses git archive internally, but then unpacks and repacks the
archive, which loses the ability to use git get-tar-commit-id.
5. I have found that the tar archives created by meson and git archive
include the files in different orders. I suspect that the Python
tarfile module introduces some either randomness or platform dependency.
6. meson dist is also slower because of the additional work.
7. meson dist produces .sha256sum files but we have called them .sha256.
(This is obviously trivial, but it is something that would need to be
dealt with somehow nonetheless.)
Most or all of these issues are fixable, either upstream in Meson or by
adjusting our own requirements. But for now this route would have some
significant disadvantages.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-24 16:18 Tristan Partin <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2024-01-24 16:18 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Tue Jan 23, 2024 at 3:30 AM CST, Peter Eisentraut wrote:
> On 22.01.24 21:04, Tristan Partin wrote:
> > I am not really following why we can't use the builtin Meson dist
> > command. The only difference from my testing is it doesn't use a
> > --prefix argument.
>
> Here are some problems I have identified:
>
> 1. meson dist internally runs gzip without the -n option. That makes
> the tar.gz archive include a timestamp, which in turn makes it not
> reproducible.
>
> 2. Because gzip includes a platform indicator in the archive, the
> produced tar.gz archive is not reproducible across platforms. (I don't
> know if gzip has an option to avoid that. git archive uses an internal
> gzip implementation that handles this.)
>
> 3. Meson does not support tar.bz2 archives.
>
> 4. Meson uses git archive internally, but then unpacks and repacks the
> archive, which loses the ability to use git get-tar-commit-id.
>
> 5. I have found that the tar archives created by meson and git archive
> include the files in different orders. I suspect that the Python
> tarfile module introduces some either randomness or platform dependency.
>
> 6. meson dist is also slower because of the additional work.
>
> 7. meson dist produces .sha256sum files but we have called them .sha256.
> (This is obviously trivial, but it is something that would need to be
> dealt with somehow nonetheless.)
>
> Most or all of these issues are fixable, either upstream in Meson or by
> adjusting our own requirements. But for now this route would have some
> significant disadvantages.
Thanks Peter. I will bring these up with upstream!
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-24 17:57 Tristan Partin <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2024-01-24 17:57 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Wed Jan 24, 2024 at 10:18 AM CST, Tristan Partin wrote:
> On Tue Jan 23, 2024 at 3:30 AM CST, Peter Eisentraut wrote:
> > On 22.01.24 21:04, Tristan Partin wrote:
> > > I am not really following why we can't use the builtin Meson dist
> > > command. The only difference from my testing is it doesn't use a
> > > --prefix argument.
> >
> > Here are some problems I have identified:
> >
> > 1. meson dist internally runs gzip without the -n option. That makes
> > the tar.gz archive include a timestamp, which in turn makes it not
> > reproducible.
It doesn't look like Python provides the facilities to affect this.
> > 2. Because gzip includes a platform indicator in the archive, the
> > produced tar.gz archive is not reproducible across platforms. (I don't
> > know if gzip has an option to avoid that. git archive uses an internal
> > gzip implementation that handles this.)
Same reason as above.
> > 3. Meson does not support tar.bz2 archives.
Submitted https://github.com/mesonbuild/meson/pull/12770.
> > 4. Meson uses git archive internally, but then unpacks and repacks the
> > archive, which loses the ability to use git get-tar-commit-id.
Because Meson allows projects to distribute arbitrary files via
meson.add_dist_script(), and can include subprojects via `meson dist
--include-subprojects`, this doesn't seem like an easily solvable
problem.
> > 5. I have found that the tar archives created by meson and git archive
> > include the files in different orders. I suspect that the Python
> > tarfile module introduces some either randomness or platform dependency.
Seems likely.
> > 6. meson dist is also slower because of the additional work.
Not easily solvable due to 4.
> > 7. meson dist produces .sha256sum files but we have called them .sha256.
> > (This is obviously trivial, but it is something that would need to be
> > dealt with somehow nonetheless.)
> >
> > Most or all of these issues are fixable, either upstream in Meson or by
> > adjusting our own requirements. But for now this route would have some
> > significant disadvantages.
>
> Thanks Peter. I will bring these up with upstream!
I think the solution to point 4 is to not unpack/repack if there are no
dist scripts and/or subprojects to distribute. I can take a look at
this later. I think this would also solve points 1, 2, 5, and 6 because
at that point meson is just calling git-archive.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-25 06:35 Peter Eisentraut <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-25 06:35 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; +Cc: pgsql-hackers
On 24.01.24 18:57, Tristan Partin wrote:
>> > 4. Meson uses git archive internally, but then unpacks and repacks
>> the > archive, which loses the ability to use git get-tar-commit-id.
>
> Because Meson allows projects to distribute arbitrary files via
> meson.add_dist_script(), and can include subprojects via `meson dist
> --include-subprojects`, this doesn't seem like an easily solvable problem.
git archive has the --add-file option, which can probably do the same
thing. Subprojects are another thing, but obviously are more rarely used.
> I think the solution to point 4 is to not unpack/repack if there are no
> dist scripts and/or subprojects to distribute. I can take a look at this
> later. I think this would also solve points 1, 2, 5, and 6 because at
> that point meson is just calling git-archive.
I think that would be a useful direction.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-25 16:04 Peter Eisentraut <[email protected]>
parent: Tristan Partin <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-25 16:04 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Tristan Partin <[email protected]>
On 22.01.24 21:04, Tristan Partin wrote:
>> +git = find_program('git', required: false, native: true, disabler: true)
>> +bzip2 = find_program('bzip2', required: false, native: true,
>> disabler: true)
>
> This doesn't need to be a disabler. git is fine as-is. See later
> comment. Disablers only work like you are expecting when they are used
> like how git is used. Once you call a method like .path(), all bets are
> off.
ok, fixed
>> +distdir = meson.project_name() + '-' + meson.project_version()
>> +
>> +check_dirty_index = run_target('check-dirty-index',
>> + command: [git, 'diff-index',
>> '--quiet', 'HEAD'])
>
> Seems like you might want to add -C here too?
done
>> +tar_bz2 = custom_target('tar.bz2',
>> + build_always_stale: true,
>> + command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command='
>> + bzip2.path() + ' -c', 'archive',
>> + '--format', 'tar.bz2',
>> + '--prefix', distdir + '/',
>
> - '-o', '@BUILD_ROOT@/@OUTPUT@',
> + '-o', join_paths(meson.build_root(), '@OUTPUT@'),
>
> This will generate the tarballs in the build directory. Do the same for
> the previous target. Tested locally.
Fixed, thanks. I had struggled with this one.
>> + 'HEAD', '.'],
>> + install: false,
>> + output: distdir + '.tar.bz2',
>> +)
>
> The bz2 target should be wrapped in an `if bzip2.found()`.
Well, I think we want the dist step to fail if bzip2 is not there. At
least that is the current expectation.
>> +alias_target('pgdist', [check_dirty_index, tar_gz, tar_bz2])
>
> Are you intending for the check_dirty_index target to prohibit the other
> two targets from running? Currently that is not the case.
Yes, that was the hope, and that's how the make dist variant works. But
I couldn't figure this out with meson. Also, the above actually also
doesn't work with older meson versions, so I had to comment this out to
get CI to work.
> If it is what
> you intend, use a stamp file or something to indicate a relationship.
> Alternatively, inline the git diff-index into the other commands. These
> might also do better as external scripts. It would reduce duplication
> between the autotools and Meson builds.
Yeah, maybe that's a direction.
The updated patch also supports vpath builds with make now.
I have also added a CI patch, for amusement. Maybe we'll want to keep
it, though.
From 13612f9a1c486e8acfe4156a7bc069a66fd30e77 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 12:28:28 +0100
Subject: [PATCH v1 1/2] make dist uses git archive
Discussion: https://www.postgresql.org/message-id/flat/40e80f77-a294-4f29-a16f-e21bc7bc75fc%40eisentraut.org
---
GNUmakefile.in | 34 ++++++++++++----------------------
meson.build | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 22 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..680c765dd73 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
distdir = postgresql-$(VERSION)
dummy = =install=
+GIT = git
+
dist: $(distdir).tar.gz $(distdir).tar.bz2
- rm -rf $(distdir)
-
-$(distdir).tar: distdir
- $(TAR) chf $@ $(distdir)
-
-.INTERMEDIATE: $(distdir).tar
-
-distdir-location:
- @echo $(distdir)
-
-distdir:
- rm -rf $(distdir)* $(dummy)
- for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
- file=`expr X$$x : 'X\./\(.*\)'`; \
- if test -d "$(top_srcdir)/$$file" ; then \
- mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
- else \
- ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
- || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
- fi || exit; \
- done
- $(MAKE) -C $(distdir) distclean
+
+.PHONY: check-dirty-index
+check-dirty-index:
+ $(GIT) -C $(srcdir) diff-index --quiet HEAD
+
+$(distdir).tar.gz: check-dirty-index
+ $(GIT) -C $(srcdir) archive --format tar.gz --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
+
+$(distdir).tar.bz2: check-dirty-index
+ $(GIT) -C $(srcdir) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
distcheck: dist
rm -rf $(dummy)
diff --git a/meson.build b/meson.build
index 55184db2488..43884e7cfdd 100644
--- a/meson.build
+++ b/meson.build
@@ -3348,6 +3348,47 @@ run_target('help',
+###############################################################
+# Distribution archive
+###############################################################
+
+git = find_program('git', required: false, native: true)
+bzip2 = find_program('bzip2', required: false, native: true)
+
+distdir = meson.project_name() + '-' + meson.project_version()
+
+check_dirty_index = run_target('check-dirty-index',
+ command: [git, '-C', '@SOURCE_ROOT@',
+ 'diff-index', '--quiet', 'HEAD'])
+
+tar_gz = custom_target('tar.gz',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', 'archive',
+ '--format', 'tar.gz',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.gz',
+)
+
+tar_bz2 = custom_target('tar.bz2',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command="' + bzip2.path() + '" -c', 'archive',
+ '--format', 'tar.bz2',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.bz2',
+)
+
+# FIXME: would like to add check_dirty_index here, broken(?) before
+# meson 0.60.0
+alias_target('pgdist', [tar_gz, tar_bz2])
+
+
+
###############################################################
# The End, The End, My Friend
###############################################################
base-commit: 46d8587b504170ca14f064890bc7ccbbd7523f81
--
2.43.0
From 3def9a78f25643304aa9cc847fc93376546d35bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 15:35:48 +0100
Subject: [PATCH v1 2/2] WIP: Add dist building to CI
Note: freebsd repartition script didn't copy .git directory
---
.cirrus.tasks.yml | 28 +++++++++++++++++++++++++
src/tools/ci/gcp_freebsd_repartition.sh | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e4e1bcfeb99..6eba3073a35 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -202,6 +202,11 @@ task:
build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop
EOF
+ dist_script: |
+ su postgres -c 'meson compile -C build -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
# if the server continues running, it often causes cirrus-ci to fail
# during upload, as it doesn't expect artifacts to change size
@@ -345,6 +350,10 @@ task:
make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
EOF
+ dist_script: su postgres -c "make dist -j${BUILD_JOBS}"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_ac
@@ -403,6 +412,10 @@ task:
PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS}
EOF
+ dist_script: su postgres -c 'meson compile -C build-32 -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
@@ -496,6 +509,10 @@ task:
ulimit -n 1024 # default is 256, pretty low
meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+ dist_script: meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
@@ -567,6 +584,12 @@ task:
vcvarsall x64
meson test %MTEST_ARGS% --num-processes %TEST_JOBS%
+ dist_script: |
+ vcvarsall x64
+ meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
@@ -627,6 +650,11 @@ task:
test_world_script: |
%BASH% -c "meson test %MTEST_ARGS% --num-processes %TEST_JOBS%"
+ dist_script: |
+ %BASH% -c "meson compile -C build -v pgdist"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
diff --git a/src/tools/ci/gcp_freebsd_repartition.sh b/src/tools/ci/gcp_freebsd_repartition.sh
index 2d5e1738998..edf38e17575 100755
--- a/src/tools/ci/gcp_freebsd_repartition.sh
+++ b/src/tools/ci/gcp_freebsd_repartition.sh
@@ -25,4 +25,4 @@ du -hs $CIRRUS_WORKING_DIR
mv $CIRRUS_WORKING_DIR $CIRRUS_WORKING_DIR.orig
mkdir $CIRRUS_WORKING_DIR
mount -o noatime /dev/da0p3 $CIRRUS_WORKING_DIR
-cp -r $CIRRUS_WORKING_DIR.orig/* $CIRRUS_WORKING_DIR/
+cp -a $CIRRUS_WORKING_DIR.orig/ $CIRRUS_WORKING_DIR/
--
2.43.0
Attachments:
[text/plain] v1-0001-make-dist-uses-git-archive.patch (3.6K, ../../[email protected]/2-v1-0001-make-dist-uses-git-archive.patch)
download | inline diff:
From 13612f9a1c486e8acfe4156a7bc069a66fd30e77 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 12:28:28 +0100
Subject: [PATCH v1 1/2] make dist uses git archive
Discussion: https://www.postgresql.org/message-id/flat/40e80f77-a294-4f29-a16f-e21bc7bc75fc%40eisentraut.org
---
GNUmakefile.in | 34 ++++++++++++----------------------
meson.build | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 22 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..680c765dd73 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
distdir = postgresql-$(VERSION)
dummy = =install=
+GIT = git
+
dist: $(distdir).tar.gz $(distdir).tar.bz2
- rm -rf $(distdir)
-
-$(distdir).tar: distdir
- $(TAR) chf $@ $(distdir)
-
-.INTERMEDIATE: $(distdir).tar
-
-distdir-location:
- @echo $(distdir)
-
-distdir:
- rm -rf $(distdir)* $(dummy)
- for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
- file=`expr X$$x : 'X\./\(.*\)'`; \
- if test -d "$(top_srcdir)/$$file" ; then \
- mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
- else \
- ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
- || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
- fi || exit; \
- done
- $(MAKE) -C $(distdir) distclean
+
+.PHONY: check-dirty-index
+check-dirty-index:
+ $(GIT) -C $(srcdir) diff-index --quiet HEAD
+
+$(distdir).tar.gz: check-dirty-index
+ $(GIT) -C $(srcdir) archive --format tar.gz --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
+
+$(distdir).tar.bz2: check-dirty-index
+ $(GIT) -C $(srcdir) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
distcheck: dist
rm -rf $(dummy)
diff --git a/meson.build b/meson.build
index 55184db2488..43884e7cfdd 100644
--- a/meson.build
+++ b/meson.build
@@ -3348,6 +3348,47 @@ run_target('help',
+###############################################################
+# Distribution archive
+###############################################################
+
+git = find_program('git', required: false, native: true)
+bzip2 = find_program('bzip2', required: false, native: true)
+
+distdir = meson.project_name() + '-' + meson.project_version()
+
+check_dirty_index = run_target('check-dirty-index',
+ command: [git, '-C', '@SOURCE_ROOT@',
+ 'diff-index', '--quiet', 'HEAD'])
+
+tar_gz = custom_target('tar.gz',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', 'archive',
+ '--format', 'tar.gz',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.gz',
+)
+
+tar_bz2 = custom_target('tar.bz2',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command="' + bzip2.path() + '" -c', 'archive',
+ '--format', 'tar.bz2',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.bz2',
+)
+
+# FIXME: would like to add check_dirty_index here, broken(?) before
+# meson 0.60.0
+alias_target('pgdist', [tar_gz, tar_bz2])
+
+
+
###############################################################
# The End, The End, My Friend
###############################################################
base-commit: 46d8587b504170ca14f064890bc7ccbbd7523f81
--
2.43.0
[text/plain] v1-0002-WIP-Add-dist-building-to-CI.patch (3.0K, ../../[email protected]/3-v1-0002-WIP-Add-dist-building-to-CI.patch)
download | inline diff:
From 3def9a78f25643304aa9cc847fc93376546d35bf Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 15:35:48 +0100
Subject: [PATCH v1 2/2] WIP: Add dist building to CI
Note: freebsd repartition script didn't copy .git directory
---
.cirrus.tasks.yml | 28 +++++++++++++++++++++++++
src/tools/ci/gcp_freebsd_repartition.sh | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e4e1bcfeb99..6eba3073a35 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -202,6 +202,11 @@ task:
build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop
EOF
+ dist_script: |
+ su postgres -c 'meson compile -C build -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
# if the server continues running, it often causes cirrus-ci to fail
# during upload, as it doesn't expect artifacts to change size
@@ -345,6 +350,10 @@ task:
make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
EOF
+ dist_script: su postgres -c "make dist -j${BUILD_JOBS}"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_ac
@@ -403,6 +412,10 @@ task:
PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS}
EOF
+ dist_script: su postgres -c 'meson compile -C build-32 -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
@@ -496,6 +509,10 @@ task:
ulimit -n 1024 # default is 256, pretty low
meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+ dist_script: meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
@@ -567,6 +584,12 @@ task:
vcvarsall x64
meson test %MTEST_ARGS% --num-processes %TEST_JOBS%
+ dist_script: |
+ vcvarsall x64
+ meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
@@ -627,6 +650,11 @@ task:
test_world_script: |
%BASH% -c "meson test %MTEST_ARGS% --num-processes %TEST_JOBS%"
+ dist_script: |
+ %BASH% -c "meson compile -C build -v pgdist"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
diff --git a/src/tools/ci/gcp_freebsd_repartition.sh b/src/tools/ci/gcp_freebsd_repartition.sh
index 2d5e1738998..edf38e17575 100755
--- a/src/tools/ci/gcp_freebsd_repartition.sh
+++ b/src/tools/ci/gcp_freebsd_repartition.sh
@@ -25,4 +25,4 @@ du -hs $CIRRUS_WORKING_DIR
mv $CIRRUS_WORKING_DIR $CIRRUS_WORKING_DIR.orig
mkdir $CIRRUS_WORKING_DIR
mount -o noatime /dev/da0p3 $CIRRUS_WORKING_DIR
-cp -r $CIRRUS_WORKING_DIR.orig/* $CIRRUS_WORKING_DIR/
+cp -a $CIRRUS_WORKING_DIR.orig/ $CIRRUS_WORKING_DIR/
--
2.43.0
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-25 16:25 Tristan Partin <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Tristan Partin @ 2024-01-25 16:25 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Thu Jan 25, 2024 at 10:04 AM CST, Peter Eisentraut wrote:
> On 22.01.24 21:04, Tristan Partin wrote:
> >> + 'HEAD', '.'],
> >> + install: false,
> >> + output: distdir + '.tar.bz2',
> >> +)
> >
> > The bz2 target should be wrapped in an `if bzip2.found()`.
The way that this currently works is that you will fail at configure
time if bz2 doesn't exist on the system. Meson will try to resolve
a .path() method on a NotFoundProgram. You might want to define the bz2
target to just call `exit 1` in this case.
if bzip2.found()
# do your current target
else
bz2 = run_target('tar.bz2', command: ['exit', 1])
endif
This should cause pgdist to appropriately fail at run time when
generating the bz2 tarball.
> Well, I think we want the dist step to fail if bzip2 is not there. At
> least that is the current expectation.
>
> >> +alias_target('pgdist', [check_dirty_index, tar_gz, tar_bz2])
> >
> > Are you intending for the check_dirty_index target to prohibit the other
> > two targets from running? Currently that is not the case.
>
> Yes, that was the hope, and that's how the make dist variant works. But
> I couldn't figure this out with meson. Also, the above actually also
> doesn't work with older meson versions, so I had to comment this out to
> get CI to work.
>
> > If it is what
> > you intend, use a stamp file or something to indicate a relationship.
> > Alternatively, inline the git diff-index into the other commands. These
> > might also do better as external scripts. It would reduce duplication
> > between the autotools and Meson builds.
>
> Yeah, maybe that's a direction.
For what it's worth, I run Meson 1.3, and the behavior of generating the
tarballs even though it is a dirty tree still occurred. In the new patch
you seem to say it was fixed in 0.60.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-26 06:28 Peter Eisentraut <[email protected]>
parent: Tristan Partin <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-26 06:28 UTC (permalink / raw)
To: Tristan Partin <[email protected]>; +Cc: pgsql-hackers
On 25.01.24 17:25, Tristan Partin wrote:
> The way that this currently works is that you will fail at configure
> time if bz2 doesn't exist on the system. Meson will try to resolve a
> .path() method on a NotFoundProgram. You might want to define the bz2
> target to just call `exit 1` in this case.
>
> if bzip2.found()
> # do your current target
> else
> bz2 = run_target('tar.bz2', command: ['exit', 1])
> endif
>
> This should cause pgdist to appropriately fail at run time when
> generating the bz2 tarball.
Ok, done that way.
> For what it's worth, I run Meson 1.3, and the behavior of generating the
> tarballs even though it is a dirty tree still occurred. In the new patch
> you seem to say it was fixed in 0.60.
The problem I'm referring to is that before 0.60, alias_target cannot
depend on run_target (only "build target"). This is AFAICT not
documented and might not have been an intentional change, but you can
trace it in the meson source code, and it shows in the PostgreSQL CI.
That's also why for the above bzip2 issue I have to use custom_target in
place of your run_target.
From 48ddd079c1530baaabf92a5650ff9a7bfa7ac3d6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 12:28:28 +0100
Subject: [PATCH v2 1/2] make dist uses git archive
Discussion: https://www.postgresql.org/message-id/flat/40e80f77-a294-4f29-a16f-e21bc7bc75fc%40eisentraut.org
---
GNUmakefile.in | 34 ++++++++++++----------------------
meson.build | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 22 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..680c765dd73 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
distdir = postgresql-$(VERSION)
dummy = =install=
+GIT = git
+
dist: $(distdir).tar.gz $(distdir).tar.bz2
- rm -rf $(distdir)
-
-$(distdir).tar: distdir
- $(TAR) chf $@ $(distdir)
-
-.INTERMEDIATE: $(distdir).tar
-
-distdir-location:
- @echo $(distdir)
-
-distdir:
- rm -rf $(distdir)* $(dummy)
- for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
- file=`expr X$$x : 'X\./\(.*\)'`; \
- if test -d "$(top_srcdir)/$$file" ; then \
- mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
- else \
- ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
- || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
- fi || exit; \
- done
- $(MAKE) -C $(distdir) distclean
+
+.PHONY: check-dirty-index
+check-dirty-index:
+ $(GIT) -C $(srcdir) diff-index --quiet HEAD
+
+$(distdir).tar.gz: check-dirty-index
+ $(GIT) -C $(srcdir) archive --format tar.gz --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
+
+$(distdir).tar.bz2: check-dirty-index
+ $(GIT) -C $(srcdir) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
distcheck: dist
rm -rf $(dummy)
diff --git a/meson.build b/meson.build
index 55184db2488..c245a1818e0 100644
--- a/meson.build
+++ b/meson.build
@@ -3348,6 +3348,53 @@ run_target('help',
+###############################################################
+# Distribution archive
+###############################################################
+
+git = find_program('git', required: false, native: true)
+bzip2 = find_program('bzip2', required: false, native: true)
+
+distdir = meson.project_name() + '-' + meson.project_version()
+
+check_dirty_index = run_target('check-dirty-index',
+ command: [git, '-C', '@SOURCE_ROOT@', 'diff-index', '--quiet', 'HEAD'])
+
+tar_gz = custom_target('tar.gz',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', 'archive',
+ '--format', 'tar.gz',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.gz',
+)
+
+if bzip2.found()
+ tar_bz2 = custom_target('tar.bz2',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command="' + bzip2.path() + '" -c', 'archive',
+ '--format', 'tar.bz2',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.bz2',
+ )
+else
+ tar_bz2 = custom_target('tar.bz2',
+ command: ['sh', '-c', 'exit 1'],
+ output: distdir + '.tar.bz2',
+ )
+endif
+
+# FIXME: would like to add check_dirty_index here, broken(?) before
+# meson 0.60.0
+alias_target('pgdist', [tar_gz, tar_bz2])
+
+
+
###############################################################
# The End, The End, My Friend
###############################################################
base-commit: 46d8587b504170ca14f064890bc7ccbbd7523f81
--
2.43.0
From 65055901d7476ecef67ff96f8218ca8fa344838f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 15:35:48 +0100
Subject: [PATCH v2 2/2] WIP: Add dist building to CI
Note: freebsd repartition script didn't copy .git directory
---
.cirrus.tasks.yml | 28 +++++++++++++++++++++++++
src/tools/ci/gcp_freebsd_repartition.sh | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e4e1bcfeb99..6eba3073a35 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -202,6 +202,11 @@ task:
build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop
EOF
+ dist_script: |
+ su postgres -c 'meson compile -C build -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
# if the server continues running, it often causes cirrus-ci to fail
# during upload, as it doesn't expect artifacts to change size
@@ -345,6 +350,10 @@ task:
make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
EOF
+ dist_script: su postgres -c "make dist -j${BUILD_JOBS}"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_ac
@@ -403,6 +412,10 @@ task:
PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS}
EOF
+ dist_script: su postgres -c 'meson compile -C build-32 -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
@@ -496,6 +509,10 @@ task:
ulimit -n 1024 # default is 256, pretty low
meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+ dist_script: meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
@@ -567,6 +584,12 @@ task:
vcvarsall x64
meson test %MTEST_ARGS% --num-processes %TEST_JOBS%
+ dist_script: |
+ vcvarsall x64
+ meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
@@ -627,6 +650,11 @@ task:
test_world_script: |
%BASH% -c "meson test %MTEST_ARGS% --num-processes %TEST_JOBS%"
+ dist_script: |
+ %BASH% -c "meson compile -C build -v pgdist"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
diff --git a/src/tools/ci/gcp_freebsd_repartition.sh b/src/tools/ci/gcp_freebsd_repartition.sh
index 2d5e1738998..edf38e17575 100755
--- a/src/tools/ci/gcp_freebsd_repartition.sh
+++ b/src/tools/ci/gcp_freebsd_repartition.sh
@@ -25,4 +25,4 @@ du -hs $CIRRUS_WORKING_DIR
mv $CIRRUS_WORKING_DIR $CIRRUS_WORKING_DIR.orig
mkdir $CIRRUS_WORKING_DIR
mount -o noatime /dev/da0p3 $CIRRUS_WORKING_DIR
-cp -r $CIRRUS_WORKING_DIR.orig/* $CIRRUS_WORKING_DIR/
+cp -a $CIRRUS_WORKING_DIR.orig/ $CIRRUS_WORKING_DIR/
--
2.43.0
Attachments:
[text/plain] v2-0001-make-dist-uses-git-archive.patch (3.7K, ../../[email protected]/2-v2-0001-make-dist-uses-git-archive.patch)
download | inline diff:
From 48ddd079c1530baaabf92a5650ff9a7bfa7ac3d6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 12:28:28 +0100
Subject: [PATCH v2 1/2] make dist uses git archive
Discussion: https://www.postgresql.org/message-id/flat/40e80f77-a294-4f29-a16f-e21bc7bc75fc%40eisentraut.org
---
GNUmakefile.in | 34 ++++++++++++----------------------
meson.build | 47 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 22 deletions(-)
diff --git a/GNUmakefile.in b/GNUmakefile.in
index eba569e930e..680c765dd73 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -87,29 +87,19 @@ update-unicode: | submake-generated-headers submake-libpgport
distdir = postgresql-$(VERSION)
dummy = =install=
+GIT = git
+
dist: $(distdir).tar.gz $(distdir).tar.bz2
- rm -rf $(distdir)
-
-$(distdir).tar: distdir
- $(TAR) chf $@ $(distdir)
-
-.INTERMEDIATE: $(distdir).tar
-
-distdir-location:
- @echo $(distdir)
-
-distdir:
- rm -rf $(distdir)* $(dummy)
- for x in `cd $(top_srcdir) && find . \( -name CVS -prune \) -o \( -name .git -prune \) -o -print`; do \
- file=`expr X$$x : 'X\./\(.*\)'`; \
- if test -d "$(top_srcdir)/$$file" ; then \
- mkdir "$(distdir)/$$file" && chmod 777 "$(distdir)/$$file"; \
- else \
- ln "$(top_srcdir)/$$file" "$(distdir)/$$file" >/dev/null 2>&1 \
- || cp "$(top_srcdir)/$$file" "$(distdir)/$$file"; \
- fi || exit; \
- done
- $(MAKE) -C $(distdir) distclean
+
+.PHONY: check-dirty-index
+check-dirty-index:
+ $(GIT) -C $(srcdir) diff-index --quiet HEAD
+
+$(distdir).tar.gz: check-dirty-index
+ $(GIT) -C $(srcdir) archive --format tar.gz --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
+
+$(distdir).tar.bz2: check-dirty-index
+ $(GIT) -C $(srcdir) -c tar.tar.bz2.command='$(BZIP2) -c' archive --format tar.bz2 --prefix $(distdir)/ HEAD -o $(abs_top_builddir)/$@
distcheck: dist
rm -rf $(dummy)
diff --git a/meson.build b/meson.build
index 55184db2488..c245a1818e0 100644
--- a/meson.build
+++ b/meson.build
@@ -3348,6 +3348,53 @@ run_target('help',
+###############################################################
+# Distribution archive
+###############################################################
+
+git = find_program('git', required: false, native: true)
+bzip2 = find_program('bzip2', required: false, native: true)
+
+distdir = meson.project_name() + '-' + meson.project_version()
+
+check_dirty_index = run_target('check-dirty-index',
+ command: [git, '-C', '@SOURCE_ROOT@', 'diff-index', '--quiet', 'HEAD'])
+
+tar_gz = custom_target('tar.gz',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', 'archive',
+ '--format', 'tar.gz',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.gz',
+)
+
+if bzip2.found()
+ tar_bz2 = custom_target('tar.bz2',
+ build_always_stale: true,
+ command: [git, '-C', '@SOURCE_ROOT@', '-c', 'tar.tar.bz2.command="' + bzip2.path() + '" -c', 'archive',
+ '--format', 'tar.bz2',
+ '--prefix', distdir + '/',
+ '-o', join_paths(meson.build_root(), '@OUTPUT@'),
+ 'HEAD', '.'],
+ install: false,
+ output: distdir + '.tar.bz2',
+ )
+else
+ tar_bz2 = custom_target('tar.bz2',
+ command: ['sh', '-c', 'exit 1'],
+ output: distdir + '.tar.bz2',
+ )
+endif
+
+# FIXME: would like to add check_dirty_index here, broken(?) before
+# meson 0.60.0
+alias_target('pgdist', [tar_gz, tar_bz2])
+
+
+
###############################################################
# The End, The End, My Friend
###############################################################
base-commit: 46d8587b504170ca14f064890bc7ccbbd7523f81
--
2.43.0
[text/plain] v2-0002-WIP-Add-dist-building-to-CI.patch (3.0K, ../../[email protected]/3-v2-0002-WIP-Add-dist-building-to-CI.patch)
download | inline diff:
From 65055901d7476ecef67ff96f8218ca8fa344838f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 25 Jan 2024 15:35:48 +0100
Subject: [PATCH v2 2/2] WIP: Add dist building to CI
Note: freebsd repartition script didn't copy .git directory
---
.cirrus.tasks.yml | 28 +++++++++++++++++++++++++
src/tools/ci/gcp_freebsd_repartition.sh | 2 +-
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e4e1bcfeb99..6eba3073a35 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -202,6 +202,11 @@ task:
build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop
EOF
+ dist_script: |
+ su postgres -c 'meson compile -C build -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
# if the server continues running, it often causes cirrus-ci to fail
# during upload, as it doesn't expect artifacts to change size
@@ -345,6 +350,10 @@ task:
make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
EOF
+ dist_script: su postgres -c "make dist -j${BUILD_JOBS}"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_ac
@@ -403,6 +412,10 @@ task:
PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS}
EOF
+ dist_script: su postgres -c 'meson compile -C build-32 -v pgdist'
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
@@ -496,6 +509,10 @@ task:
ulimit -n 1024 # default is 256, pretty low
meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+ dist_script: meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
cores_script: src/tools/ci/cores_backtrace.sh macos "${HOME}/cores"
@@ -567,6 +584,12 @@ task:
vcvarsall x64
meson test %MTEST_ARGS% --num-processes %TEST_JOBS%
+ dist_script: |
+ vcvarsall x64
+ meson compile -C build -v pgdist
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
@@ -627,6 +650,11 @@ task:
test_world_script: |
%BASH% -c "meson test %MTEST_ARGS% --num-processes %TEST_JOBS%"
+ dist_script: |
+ %BASH% -c "meson compile -C build -v pgdist"
+ tar_artifacts:
+ path: "build*/*.tar.*"
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
diff --git a/src/tools/ci/gcp_freebsd_repartition.sh b/src/tools/ci/gcp_freebsd_repartition.sh
index 2d5e1738998..edf38e17575 100755
--- a/src/tools/ci/gcp_freebsd_repartition.sh
+++ b/src/tools/ci/gcp_freebsd_repartition.sh
@@ -25,4 +25,4 @@ du -hs $CIRRUS_WORKING_DIR
mv $CIRRUS_WORKING_DIR $CIRRUS_WORKING_DIR.orig
mkdir $CIRRUS_WORKING_DIR
mount -o noatime /dev/da0p3 $CIRRUS_WORKING_DIR
-cp -r $CIRRUS_WORKING_DIR.orig/* $CIRRUS_WORKING_DIR/
+cp -a $CIRRUS_WORKING_DIR.orig/ $CIRRUS_WORKING_DIR/
--
2.43.0
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-26 19:46 Tristan Partin <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Tristan Partin @ 2024-01-26 19:46 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Fri Jan 26, 2024 at 12:28 AM CST, Peter Eisentraut wrote:
> On 25.01.24 17:25, Tristan Partin wrote:
> > For what it's worth, I run Meson 1.3, and the behavior of generating the
> > tarballs even though it is a dirty tree still occurred. In the new patch
> > you seem to say it was fixed in 0.60.
>
> The problem I'm referring to is that before 0.60, alias_target cannot
> depend on run_target (only "build target"). This is AFAICT not
> documented and might not have been an intentional change, but you can
> trace it in the meson source code, and it shows in the PostgreSQL CI.
> That's also why for the above bzip2 issue I have to use custom_target in
> place of your run_target.
https://github.com/mesonbuild/meson/pull/12783
Thanks for finding these issues.
--
Tristan Partin
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-26 21:18 Eli Schwartz <[email protected]>
parent: Peter Eisentraut <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Eli Schwartz @ 2024-01-26 21:18 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; Tristan Partin <[email protected]>; +Cc: pgsql-hackers
Hello, meson developer here.
On 1/23/24 4:30 AM, Peter Eisentraut wrote:
> On 22.01.24 21:04, Tristan Partin wrote:
>> I am not really following why we can't use the builtin Meson dist
>> command. The only difference from my testing is it doesn't use a
>> --prefix argument.
>
> Here are some problems I have identified:
>
> 1. meson dist internally runs gzip without the -n option. That makes
> the tar.gz archive include a timestamp, which in turn makes it not
> reproducible.
Well, it uses python tarfile which uses python gzip support under the
hood, but yes, that is true, python tarfile doesn't expose this tunable.
> 2. Because gzip includes a platform indicator in the archive, the
> produced tar.gz archive is not reproducible across platforms. (I don't
> know if gzip has an option to avoid that. git archive uses an internal
> gzip implementation that handles this.)
This appears to be https://github.com/python/cpython/issues/112346
> 3. Meson does not support tar.bz2 archives.
Simple enough to add, but I'm a bit surprised as usually people seem to
want either gzip for portability or xz for efficient compression.
> 4. Meson uses git archive internally, but then unpacks and repacks the
> archive, which loses the ability to use git get-tar-commit-id.
What do you use this for? IMO a more robust way to track the commit used
is to use gitattributes export-subst to write a `.git_archival.txt` file
containing the commit sha1 and other info -- this can be read even after
the file is extracted, which means it can also be used to bake the ID
into the built binaries e.g. as part of --version output.
> 5. I have found that the tar archives created by meson and git archive
> include the files in different orders. I suspect that the Python
> tarfile module introduces some either randomness or platform dependency.
Different orders is meaningless, the question is whether the order is
internally consistent. Python uses sorted() to guarantee a stable order,
which may be a different algorithm than the one git-archive uses to
guarantee a stable order. But the order should be stable and that is
what matters.
> 6. meson dist is also slower because of the additional work.
I'm amenable to skipping the extraction/recombination of subprojects and
running of dist scripts in the event that neither exist, as Tristan
offered to do, but...
> 7. meson dist produces .sha256sum files but we have called them .sha256.
> (This is obviously trivial, but it is something that would need to be
> dealt with somehow nonetheless.)
>
> Most or all of these issues are fixable, either upstream in Meson or by
> adjusting our own requirements. But for now this route would have some
> significant disadvantages.
Overall I feel like much of this is about requiring dist tarballs to be
byte-identical to other dist tarballs, although reproducible builds is
mainly about artifacts, not sources, and for sources it doesn't
generally matter unless the sources are ephemeral and generated
on-demand (in which case it is indeed very important to produce the same
tarball each time). A tarball is usually generated once, signed, and
uploaded to release hosting. Meson already guarantees the contents are
strictly based on the built tag.
--
Eli Schwartz
Attachments:
[application/pgp-keys] OpenPGP_0x84818A6819AF4A9B.asc (17.7K, ../../[email protected]/2-OpenPGP_0x84818A6819AF4A9B.asc)
download
[application/pgp-signature] OpenPGP_signature.asc (833B, ../../[email protected]/3-OpenPGP_signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-31 08:03 Peter Eisentraut <[email protected]>
parent: Eli Schwartz <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Peter Eisentraut @ 2024-01-31 08:03 UTC (permalink / raw)
To: Eli Schwartz <[email protected]>; Tristan Partin <[email protected]>; +Cc: pgsql-hackers
On 26.01.24 22:18, Eli Schwartz wrote:
> Hello, meson developer here.
Hello, and thanks for popping in!
>> 3. Meson does not support tar.bz2 archives.
>
> Simple enough to add, but I'm a bit surprised as usually people seem to
> want either gzip for portability or xz for efficient compression.
We may very well end up updating our requirements here before too long,
so I wouldn't bother with this on the meson side. Last time we
discussed this, there were still platforms under support that didn't
have xz easily available.
>> 4. Meson uses git archive internally, but then unpacks and repacks the
>> archive, which loses the ability to use git get-tar-commit-id.
>
> What do you use this for? IMO a more robust way to track the commit used
> is to use gitattributes export-subst to write a `.git_archival.txt` file
> containing the commit sha1 and other info -- this can be read even after
> the file is extracted, which means it can also be used to bake the ID
> into the built binaries e.g. as part of --version output.
It's a marginal use case, for sure. But it is something that git
provides tooling for that is universally available. Any alternative
would be an ad-hoc solution that is specific to our project and would be
different for the next project.
>> 5. I have found that the tar archives created by meson and git archive
>> include the files in different orders. I suspect that the Python
>> tarfile module introduces some either randomness or platform dependency.
>
> Different orders is meaningless, the question is whether the order is
> internally consistent. Python uses sorted() to guarantee a stable order,
> which may be a different algorithm than the one git-archive uses to
> guarantee a stable order. But the order should be stable and that is
> what matters.
(FWIW, I couldn't reproduce this anymore, so maybe it's not actually an
issue.)
> Overall I feel like much of this is about requiring dist tarballs to be
> byte-identical to other dist tarballs, although reproducible builds is
> mainly about artifacts, not sources, and for sources it doesn't
> generally matter unless the sources are ephemeral and generated
> on-demand (in which case it is indeed very important to produce the same
> tarball each time).
The source tarball is, in a way, also an artifact.
I think it's useful that others can easily independently verify that the
produced tarball matches what they have locally. It's not an absolute
requirement, but given that it is possible, it seems useful to take
advantage of it.
In a way, this also avoids the need for signing the tarball, which we
don't do. So maybe that contributes to a different perspective.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-31 15:50 Eli Schwartz <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Eli Schwartz @ 2024-01-31 15:50 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; Tristan Partin <[email protected]>; +Cc: pgsql-hackers
On 1/31/24 3:03 AM, Peter Eisentraut wrote:
>> What do you use this for? IMO a more robust way to track the commit used
>> is to use gitattributes export-subst to write a `.git_archival.txt` file
>> containing the commit sha1 and other info -- this can be read even after
>> the file is extracted, which means it can also be used to bake the ID
>> into the built binaries e.g. as part of --version output.
>
> It's a marginal use case, for sure. But it is something that git
> provides tooling for that is universally available. Any alternative
> would be an ad-hoc solution that is specific to our project and would be
> different for the next project.
mercurial has the "archivemeta" config setting that exports similar
information, but forces the filename ".hg_archival.txt".
The setuptools-scm project follows this pattern by requiring the git
file to be called ".git_archival.txt" with a set pattern mimicking the
hg one:
https://setuptools-scm.readthedocs.io/en/latest/usage/#git-archives
So, I guess you could use this and then it would not be specific to your
project. :)
>> Overall I feel like much of this is about requiring dist tarballs to be
>> byte-identical to other dist tarballs, although reproducible builds is
>> mainly about artifacts, not sources, and for sources it doesn't
>> generally matter unless the sources are ephemeral and generated
>> on-demand (in which case it is indeed very important to produce the same
>> tarball each time).
>
> The source tarball is, in a way, also an artifact.
>
> I think it's useful that others can easily independently verify that the
> produced tarball matches what they have locally. It's not an absolute
> requirement, but given that it is possible, it seems useful to take
> advantage of it.
>
> In a way, this also avoids the need for signing the tarball, which we
> don't do. So maybe that contributes to a different perspective.
Since you mention signing and not as a simple "aside"...
That's a fascinating perspective. I wonder how people independently
verify that what they have locally (I assume from git clones) matches
what the postgres committers have authorized.
I'm a bit skeptical that you can avoid the need to perform code-signing
at some stage, somewhere, somehow, by suggesting that people can simply
git clone, run some commands and compare the tarball. The point of
signing is to verify that no one has acquired an untraceable API token
they should not have and gotten write access to the authoritative server
then uploaded malicious code under various forged identities, possibly
overwriting previous versions, either in git or out of git.
Ideally git commits should be signed, but that requires large numbers of
people to have security-minded git commit habits. From a quick check of
the postgres commit logs, only one person seems to be regularly signing
commits, which does provide a certain measure of protection -- an
attacker cannot attack via `git push --force` across that boundary, and
those commits serve as verifiable states that multiple people have seen.
The tags aren't signed either, which is a big issue for verifiably
identifying the release artifacts published by the release manager. Even
if not every commit is signed, having signed tags provides a known
coordination point of code that has been broadly tested and code-signed
for mass use.
...
In summary, my opinion is that using git-get-tar-commit-id provides zero
security guarantees, and if that's not something you are worried about
then that's one thing, but if you were expecting it to *replace* signing
the tarball, then that's.... very much another thing entirely, and not
one I can agree at all with.
--
Eli Schwartz
Attachments:
[application/pgp-keys] OpenPGP_0x84818A6819AF4A9B.asc (17.7K, ../../[email protected]/2-OpenPGP_0x84818A6819AF4A9B.asc)
download
[application/pgp-signature] OpenPGP_signature.asc (833B, ../../[email protected]/3-OpenPGP_signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: make dist using git archive
@ 2024-01-31 18:12 Robert Haas <[email protected]>
parent: Eli Schwartz <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Robert Haas @ 2024-01-31 18:12 UTC (permalink / raw)
To: Eli Schwartz <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tristan Partin <[email protected]>; pgsql-hackers
On Wed, Jan 31, 2024 at 10:50 AM Eli Schwartz <[email protected]> wrote:
> Ideally git commits should be signed, but that requires large numbers of
> people to have security-minded git commit habits. From a quick check of
> the postgres commit logs, only one person seems to be regularly signing
> commits, which does provide a certain measure of protection -- an
> attacker cannot attack via `git push --force` across that boundary, and
> those commits serve as verifiable states that multiple people have seen.
>
> The tags aren't signed either, which is a big issue for verifiably
> identifying the release artifacts published by the release manager. Even
> if not every commit is signed, having signed tags provides a known
> coordination point of code that has been broadly tested and code-signed
> for mass use.
>
> In summary, my opinion is that using git-get-tar-commit-id provides zero
> security guarantees, and if that's not something you are worried about
> then that's one thing, but if you were expecting it to *replace* signing
> the tarball, then that's.... very much another thing entirely, and not
> one I can agree at all with.
I read this part with interest. I think there's definitely something
to be said for strengthening some of our practices in this area. At
the same time, I think it's reasonable for Peter to want to pursue the
limited goal he stated in the original post, namely reproducible
tarball generation, without getting tangled up in possible policy
changes that might be controversial and might require a bunch of
planning and coordination. "GPG signatures are good" can be true
without "reproducible tarball generation is good" being false; and if
"git archive" allows for that and "meson dist" doesn't, then we're
unlikely to adopt "meson dist".
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2024-01-31 18:12 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-30 03:37 [PATCH v24 3/7] Add default_toast_compression GUC Justin Pryzby <[email protected]>
2024-01-22 07:31 make dist using git archive Peter Eisentraut <[email protected]>
2024-01-22 12:10 ` Re: make dist using git archive Junwang Zhao <[email protected]>
2024-01-22 18:35 ` Re: make dist using git archive Peter Eisentraut <[email protected]>
2024-01-23 02:14 ` Re: make dist using git archive Junwang Zhao <[email protected]>
2024-01-22 20:04 ` Re: make dist using git archive Tristan Partin <[email protected]>
2024-01-23 09:30 ` Re: make dist using git archive Peter Eisentraut <[email protected]>
2024-01-24 16:18 ` Re: make dist using git archive Tristan Partin <[email protected]>
2024-01-24 17:57 ` Re: make dist using git archive Tristan Partin <[email protected]>
2024-01-25 06:35 ` Re: make dist using git archive Peter Eisentraut <[email protected]>
2024-01-26 21:18 ` Re: make dist using git archive Eli Schwartz <[email protected]>
2024-01-31 08:03 ` Re: make dist using git archive Peter Eisentraut <[email protected]>
2024-01-31 15:50 ` Re: make dist using git archive Eli Schwartz <[email protected]>
2024-01-31 18:12 ` Re: make dist using git archive Robert Haas <[email protected]>
2024-01-25 16:04 ` Re: make dist using git archive Peter Eisentraut <[email protected]>
2024-01-25 16:25 ` Re: make dist using git archive Tristan Partin <[email protected]>
2024-01-26 06:28 ` Re: make dist using git archive Peter Eisentraut <[email protected]>
2024-01-26 19:46 ` Re: make dist using git archive Tristan Partin <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox