Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove extraneous #if in postgis.sql.in #706

Closed
wants to merge 1 commit into from

Conversation

rjuju
Copy link
Contributor

@rjuju rjuju commented Sep 15, 2022

Oversight in commit 3e38e0b.

For the record the CI does notice this problem but keeps going. For instance https://github.com/postgis/postgis/actions/runs/3057481917/jobs/4932676006 in the "Build & Test" item line 646:

postgis.sql.in:4436: error: unterminated #if
4436 | #if POSTGIS_PGSQL_VERSION >= 120
|

@rjuju rjuju changed the title Add missing #endif in postgis.sql.in Remove extraneous #if in postgis.sql.in Sep 15, 2022
@strk
Copy link
Member

strk commented Sep 15, 2022 via email

@rjuju
Copy link
Contributor Author

rjuju commented Sep 15, 2022

This happens in any build process. I just happened to saw it after a "make -s" while pretty much everything else was already compiled so it was kind of obvious.

Is comes from https://github.com/postgis/postgis/blob/master/postgis/Makefile.in#L239-L241:

%.sql: %.sql.in
	$(SQLPP) -I@top_srcdir@/libpgcommon -I@builddir@ $< | grep -v '^#' | \
	$(PERL) -lpe "s'MODULE_PATHNAME'\$(MODULEPATH)'g;s'@extschema@\.''g" > $@

the SQLPP command itselfs errors out, but since the output is piped make sees the result of the last command, which doesn't fail.

I'm not really sure how to fix it. I could add this after:

test ${PIPESTATUS[0]} -eq 0

but I don't know how portable it's (especially on windows).

Note that the same pattern is used in many other makefiles.

@strk
Copy link
Member

strk commented Sep 15, 2022 via email

strk added a commit that referenced this pull request Sep 15, 2022
@rjuju
Copy link
Contributor Author

rjuju commented Sep 15, 2022

It works too :)

Isn't it missing the rm -f $@.tmp in the other makefiles?

Note that for postgis.sql it leaks the .tmp file in case of error, but it will eventually be cleaned up once the source file is fixed so it doesn't seem like a problem.

@strk
Copy link
Member

strk commented Sep 15, 2022 via email

@strk
Copy link
Member

strk commented Sep 16, 2022

This was merged as 229eeb7

@strk strk closed this Sep 16, 2022
@strk
Copy link
Member

strk commented Sep 16, 2022

I love our testsuite. THere IS actually a bot running distcheck:
https://gitlab.com/postgis/postgis/-/jobs/3042375176#L5376

My .tmp files didn't go unnoticed there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants