postgresql: Remove dev dependencies from closure #179962
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| diff --git a/src/bin/pg_config/pg_config.c b/src/bin/pg_config/pg_config.c | ||
| index 62b97af..e5b71e5 100644 | ||
| --- a/src/bin/pg_config/pg_config.c | ||
| +++ b/src/bin/pg_config/pg_config.c | ||
| @@ -112,6 +112,11 @@ advice(void) | ||
| fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname); | ||
| } | ||
|
|
||
| +static char* | ||
| +get_config_val(ConfigData configdata) { | ||
|
There was a problem hiding this comment. Also why is this passed by reference here? There was a problem hiding this comment. It is passed by value, is not it? We could pass a pointer to it if we want to avoid copying the whole struct but I do not think this is worth it – the functions will probably only be called at extension build time and compiler might inline the function anyway. There was a problem hiding this comment. meh, I mixed them up, I meant pass-by-value and wanted to ask why 🙃 |
||
| + return strcmp(configdata.name, "PGXS") == 0 ? "@dev@/lib/pgxs/src/makefiles/pgxs.mk" : configdata.setting; | ||
| +} | ||
| + | ||
| static void | ||
| show_item(const char *configname, | ||
| ConfigData *configdata, | ||
| @@ -122,7 +127,7 @@ show_item(const char *configname, | ||
| for (i = 0; i < configdata_len; i++) | ||
| { | ||
| if (strcmp(configname, configdata[i].name) == 0) | ||
| - printf("%s\n", configdata[i].setting); | ||
| + printf("%s\n", get_config_val(configdata[i])); | ||
| } | ||
| } | ||
|
|
||
| @@ -160,7 +165,7 @@ main(int argc, char **argv) | ||
| if (argc < 2) | ||
| { | ||
| for (i = 0; i < configdata_len; i++) | ||
| - printf("%s = %s\n", configdata[i].name, configdata[i].setting); | ||
| + printf("%s = %s\n", configdata[i].name, get_config_val(configdata[i])); | ||
| exit(0); | ||
| } | ||
|
|
||
| diff --git a/src/common/config_info.c b/src/common/config_info.c | ||
| index aa643b6..3750c14 100644 | ||
| --- a/src/common/config_info.c | ||
| +++ b/src/common/config_info.c | ||
| @@ -42,11 +42,7 @@ get_configdata(const char *my_exec_path, size_t *configdata_len) | ||
| configdata = (ConfigData *) palloc(*configdata_len * sizeof(ConfigData)); | ||
|
|
||
| configdata[i].name = pstrdup("BINDIR"); | ||
| - strlcpy(path, my_exec_path, sizeof(path)); | ||
| - lastsep = strrchr(path, '/'); | ||
| - if (lastsep) | ||
| - *lastsep = '\0'; | ||
| - cleanup_path(path); | ||
| + strlcpy(path, "@out@/bin", sizeof(path)); | ||
| configdata[i].setting = pstrdup(path); | ||
| i++; | ||
|
|
||
| @@ -105,8 +101,7 @@ get_configdata(const char *my_exec_path, size_t *configdata_len) | ||
| i++; | ||
|
|
||
| configdata[i].name = pstrdup("SHAREDIR"); | ||
| - get_share_path(my_exec_path, path); | ||
| - cleanup_path(path); | ||
| + strlcpy(path, "@out@/share", sizeof(path)); | ||
| configdata[i].setting = pstrdup(path); | ||
| i++; | ||
|
|
||
| @@ -117,7 +112,7 @@ get_configdata(const char *my_exec_path, size_t *configdata_len) | ||
| i++; | ||
|
|
||
| configdata[i].name = pstrdup("PGXS"); | ||
| - get_pkglib_path(my_exec_path, path); | ||
| + strlcpy(path, "/dev/null", sizeof(path)); | ||
|
There was a problem hiding this comment. What's the rationale in here to make this There was a problem hiding this comment. IIRC |
||
| strlcat(path, "/pgxs/src/makefiles/pgxs.mk", sizeof(path)); | ||
| cleanup_path(path); | ||
| configdata[i].setting = pstrdup(path); | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| diff --git a/src/port/Makefile b/src/port/Makefile | ||
| index bfe1feb..1f9f0d3 100644 | ||
| --- a/src/port/Makefile | ||
| +++ b/src/port/Makefile | ||
| @@ -152,9 +152,9 @@ pg_config_paths.h: $(top_builddir)/src/Makefile.global | ||
| echo "#define PGBINDIR \"$(bindir)\"" >$@ | ||
jtojnar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| echo "#define PGSHAREDIR \"$(datadir)\"" >>$@ | ||
| echo "#define SYSCONFDIR \"$(sysconfdir)\"" >>$@ | ||
| - echo "#define INCLUDEDIR \"$(includedir)\"" >>$@ | ||
| - echo "#define PKGINCLUDEDIR \"$(pkgincludedir)\"" >>$@ | ||
| - echo "#define INCLUDEDIRSERVER \"$(includedir_server)\"" >>$@ | ||
| + echo "#define INCLUDEDIR \"/dev/null/include\"" >>$@ | ||
|
There was a problem hiding this comment. Doesn't this break with e.g. https://github.com/postgres/postgres/blob/aa210e0c121eb8f58c86d4fcc833a5a6fbb6f5a9/src/interfaces/ecpg/preproc/ecpg.c#L256 since There was a problem hiding this comment. It will return the wrong value but the assumption is that nothing uses the function for anything other than printing it in some build information pages. |
||
| + echo "#define PKGINCLUDEDIR \"/dev/null/include\"" >>$@ | ||
| + echo "#define INCLUDEDIRSERVER \"/dev/null/include/server\"" >>$@ | ||
| echo "#define LIBDIR \"$(libdir)\"" >>$@ | ||
| echo "#define PKGLIBDIR \"$(pkglibdir)\"" >>$@ | ||
| echo "#define LOCALEDIR \"$(localedir)\"" >>$@ | ||
|
There was a problem hiding this comment. We could shave 16M more by removing |
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| --- a/src/Makefile.global.in | ||
| +++ b/src/Makefile.global.in | ||
| @@ -75,7 +75,7 @@ endif # not PGXS | ||
| vpathsearch = `for f in $(addsuffix /$(1),$(subst :, ,. $(VPATH))); do test -r $$f && echo $$f && break; done` | ||
|
|
||
| # Saved arguments from configure | ||
| -configure_args = @configure_args@ | ||
| +configure_args = $(shell echo '@configure_args@' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g') | ||
|
|
||
|
|
||
| ########################################################################## | ||
| diff --git a/src/common/Makefile b/src/common/Makefile | ||
| index 873fbb6..b6476b4 100644 | ||
| --- a/src/common/Makefile | ||
| +++ b/src/common/Makefile | ||
| @@ -29,13 +29,13 @@ STD_CPPFLAGS := $(filter-out -I$(top_srcdir)/src/include -I$(top_builddir)/src/i | ||
| STD_LDFLAGS := $(filter-out -L$(top_builddir)/src/common -L$(top_builddir)/src/port,$(LDFLAGS)) | ||
| override CPPFLAGS += -DVAL_CONFIGURE="\"$(configure_args)\"" | ||
| override CPPFLAGS += -DVAL_CC="\"$(CC)\"" | ||
| -override CPPFLAGS += -DVAL_CPPFLAGS="\"$(STD_CPPFLAGS)\"" | ||
| -override CPPFLAGS += -DVAL_CFLAGS="\"$(CFLAGS)\"" | ||
| -override CPPFLAGS += -DVAL_CFLAGS_SL="\"$(CFLAGS_SL)\"" | ||
| -override CPPFLAGS += -DVAL_LDFLAGS="\"$(STD_LDFLAGS)\"" | ||
| -override CPPFLAGS += -DVAL_LDFLAGS_EX="\"$(LDFLAGS_EX)\"" | ||
| -override CPPFLAGS += -DVAL_LDFLAGS_SL="\"$(LDFLAGS_SL)\"" | ||
| -override CPPFLAGS += -DVAL_LIBS="\"$(LIBS)\"" | ||
| +override CPPFLAGS += -DVAL_CPPFLAGS="\"$(shell echo '$(STD_CPPFLAGS)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
| +override CPPFLAGS += -DVAL_CFLAGS="\"$(shell echo '$(CFLAGS)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
| +override CPPFLAGS += -DVAL_CFLAGS_SL="\"$(shell echo '$(CFLAGS_SL)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
| +override CPPFLAGS += -DVAL_LDFLAGS="\"$(shell echo '$(STD_LDFLAGS)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
| +override CPPFLAGS += -DVAL_LDFLAGS_EX="\"$(shell echo '$(LDFLAGS_EX)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
| +override CPPFLAGS += -DVAL_LDFLAGS_SL="\"$(shell echo '$(LDFLAGS_SL)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
| +override CPPFLAGS += -DVAL_LIBS="\"$(shell echo '$(LIBS)' | sed -E 's~/nix/store/.{32}-~/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-~g')\"" | ||
|
|
||
| override CPPFLAGS := -DFRONTEND $(CPPFLAGS) | ||
| LIBS += $(PTHREAD_LIBS) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate why this is now needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a proper place for this as a library dependency. Ideally, we would remove it from
nativeBuildInputsbut that will probably not work until psycopg/psycopg2#1001. cc @SuperSandro2000 who made this change.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, IIRC they use pg_config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment.