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

postgresql 12.0 #44875

Open
wants to merge 5 commits into
base: master
from

Conversation

@marksiemers
Copy link

commented Oct 3, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

UPDATE: Looks like I had an issue with my local OS X SDK install. Followed the advice here: petere/homebrew-postgresql#44 (comment)
and now things are working.

First time trying to contribute by bumping to the latest postgresql.

Things were going pretty well, I made the changes so the audit would pass, then building from source did not work:

@BrewTestBot

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

  • New formulae should not have a bottle do block
@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

  • New formulae should not have a bottle do block

What is the new preferred method?

I see this in the readme: https://github.com/Homebrew/brew/blob/master/docs/Formula-Cookbook.md#updating-formulae

@davidfetter

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2019

I'd guess from the error message, leave out the bottle do block.

Formula/postgresql@11.rb Outdated Show resolved Hide resolved
Formula/postgresql@11.rb Outdated Show resolved Hide resolved
@chenrui333

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

I think after the above two changes, it should be good.

Thanks @marksiemers !!

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

I think after the above two changes, it should be good.

Thanks @marksiemers !!

Thank you!

I'm glad the changes are easy (hopefully). I have a couple work meetings this morning then I plan to update the PR later today.

@chenrui333

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

Sounds good!

- Remove bottle do ... end block for postgresql and postgresql@11 formulae

- Remove revision from postgresql@11
@marksiemers marksiemers requested a review from chenrui333 Oct 4, 2019
@chenrui333

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

Thanks @marksiemers !!

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

@chenrui333 - Anything I can do to help with the failing builds? Are those failures expected?

@chenrui333

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

a little bit busy at the moment, will take a look at it later.

From the build log, seems there are possible conflicts which might require revision bump:

14:49:15 Possible conflicting files are:
14:49:15 /usr/local/bin/clusterdb -> /usr/local/Cellar/postgresql@11/11.5/bin/clusterdb
14:49:15 /usr/local/bin/createdb -> /usr/local/Cellar/postgresql@11/11.5/bin/createdb
14:49:15 /usr/local/bin/createuser -> /usr/local/Cellar/postgresql@11/11.5/bin/createuser
14:49:15 /usr/local/bin/dropdb -> /usr/local/Cellar/postgresql@11/11.5/bin/dropdb
14:49:15 /usr/local/bin/dropuser -> /usr/local/Cellar/postgresql@11/11.5/bin/dropuser
14:49:15 /usr/local/bin/ecpg -> /usr/local/Cellar/postgresql@11/11.5/bin/ecpg
14:49:15 /usr/local/bin/initdb -> /usr/local/Cellar/postgresql@11/11.5/bin/initdb
14:49:15 /usr/local/bin/oid2name -> /usr/local/Cellar/postgresql@11/11.5/bin/oid2name
14:49:15 /usr/local/bin/pg_archivecleanup -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_archivecleanup
14:49:15 /usr/local/bin/pg_basebackup -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_basebackup
14:49:15 /usr/local/bin/pg_config -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_config
14:49:15 /usr/local/bin/pg_controldata -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_controldata
14:49:15 /usr/local/bin/pg_ctl -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_ctl
14:49:15 /usr/local/bin/pg_dump -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_dump
14:49:15 /usr/local/bin/pg_dumpall -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_dumpall
14:49:15 /usr/local/bin/pg_isready -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_isready
14:49:15 /usr/local/bin/pg_receivewal -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_receivewal
14:49:15 /usr/local/bin/pg_recvlogical -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_recvlogical
14:49:15 /usr/local/bin/pg_resetwal -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_resetwal
14:49:15 /usr/local/bin/pg_restore -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_restore
14:49:15 /usr/local/bin/pg_rewind -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_rewind
14:49:15 /usr/local/bin/pg_standby -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_standby
14:49:15 /usr/local/bin/pg_test_fsync -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_test_fsync
14:49:15 /usr/local/bin/pg_test_timing -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_test_timing
14:49:15 /usr/local/bin/pg_upgrade -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_upgrade
14:49:15 /usr/local/bin/pg_waldump -> /usr/local/Cellar/postgresql@11/11.5/bin/pg_waldump
14:49:15 /usr/local/bin/pgbench -> /usr/local/Cellar/postgresql@11/11.5/bin/pgbench
14:49:15 /usr/local/bin/postgres -> /usr/local/Cellar/postgresql@11/11.5/bin/postgres
14:49:15 /usr/local/bin/postmaster -> /usr/local/Cellar/postgresql@11/11.5/bin/postmaster
14:49:15 /usr/local/bin/psql -> /usr/local/Cellar/postgresql@11/11.5/bin/psql
14:49:15 /usr/local/bin/reindexdb -> /usr/local/Cellar/postgresql@11/11.5/bin/reindexdb
14:49:15 /usr/local/bin/vacuumdb -> /usr/local/Cellar/postgresql@11/11.5/bin/vacuumdb
14:49:15 /usr/local/bin/vacuumlo -> /usr/local/Cellar/postgresql@11/11.5/bin/vacuumlo
EOS
end

plist_options :manual => "pg_ctl -D #{HOMEBREW_PREFIX}/var/postgres start"

This comment has been minimized.

Copy link
@javian

javian Oct 7, 2019

Member

This also needs to be updated for the versioned formula. If you diff the postgres formula and the postgres@10 there might be more clues as to what changed between the last time we bumped the major version of postgres.

This comment has been minimized.

Copy link
@marksiemers

marksiemers Oct 8, 2019

Author

I'll continue to work through the issues.

I'm now seeing the commands that are running in CI that show failing tests, it looks like for at least some of them I can reproduce locally and shorten the feedback loop.

When I did the audit and build from source (when originally opening the PR) it was all for postgres 12 - I didn't run any real tests for postgresql@11 (at that time)

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 8, 2019

For these missing libraries:

Missing libraries:
  /usr/local/lib/libecpg.6.dylib
  /usr/local/lib/libpgtypes.3.dylib
  /usr/local/lib/libpq.5.dylib

Should libpq be listed as a dependency for postgresql@11? Or does it need to be available as a native library on the system (without using brew)?

Even with that, it might needs some compiler flags:

libpq is keg-only, which means it was not symlinked into /usr/local,
because conflicts with postgres formula.

If you need to have libpq first in your PATH run:
  echo 'export PATH="/usr/local/opt/libpq/bin:$PATH"' >> ~/.bash_profile

For compilers to find libpq you may need to set:
  export LDFLAGS="-L/usr/local/opt/libpq/lib"
  export CPPFLAGS="-I/usr/local/opt/libpq/include"

For pkg-config to find libpq you may need to set:
  export PKG_CONFIG_PATH="/usr/local/opt/libpq/lib/pkgconfig"
@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 8, 2019

This may be dependent on citus 9.0.0 and some others libs upgrading to support pg 12.

citusdata/citus#3081

@hanefi

This comment has been minimized.

Copy link

commented Oct 8, 2019

I did not test building citus from source using the HomeBrew PostgreSQL. AFAIK, libpq is not needed, and it suffices to have pg_config in the $PATH (or the path of pg_configin $PG_CONFIG).

I will try to test it later today, and inform you on this thread.

@hanefi

This comment has been minimized.

Copy link

commented Oct 8, 2019

I confirmed my theory. It is sufficient to have pg_config, and libpq is not necessary to compile citus.

Even if it were, I think it is not the responsiblity of the Postgres package to have that dependency just because a postgres extension may need it.

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 8, 2019

I confirmed my theory. It is sufficient to have pg_config, and libpq is not necessary to compile citus.

Even if it were, I think it is not the responsiblity of the Postgres package to have that dependency just because a postgres extension may need it.

@hanefi - Agreed. I still need to get the CI build working for citus and the other extensions.

I'll take a look at the postgresql (v12) formula to see if it is missing some steps for the pg_config settings

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 8, 2019

This is the error that I'm struggling to get past:

Testing citus
==> /usr/local/opt/postgresql/bin/initdb /private/tmp/citus-test-20191008-6167-1ksbyh1/test
2019-10-08 09:08:33.697 PDT [6193] FATAL:  could not load library "/usr/local/lib/postgresql/citus.so": dlopen(/usr/local/lib/postgresql/citus.so, 10): Symbol not found: _AllocSetContextCreateExtended
	  Referenced from: /usr/local/lib/postgresql/citus.so
	  Expected in: /usr/local/opt/postgresql/bin/postgres
	 in /usr/local/lib/postgresql/citus.so
2019-10-08 09:08:33.697 PDT [6193] LOG:  database system is shut down
==> /usr/local/opt/postgresql/bin/createdb -p 55561 test
Last 15 lines from /Users/mark/Library/Logs/Homebrew/citus/test.02.createdb:
2019-10-08 09:08:35 -0700

/usr/local/opt/postgresql/bin/createdb
-p
55561
test

createdb: error: could not connect to database template1: could not connect to server: No such file or directory
	Is the server running locally and accepting
	connections on Unix domain socket "/tmp/.s.PGSQL.55561"?

And the postgres binary does exist there:

ls -la /usr/local/opt/postgresql/bin/postgres
-r-xr-xr-x  1 mark  admin  6524756 Oct  7 22:13 /usr/local/opt/postgresql/bin/postgres

I'm thinking there was a breaking change in postgres 12 for the function _AllocSetContextCreateExtended and I'm assuming citus 9.x accommodates the change.

@chenrui333

This comment has been minimized.

Copy link
Member

commented Oct 9, 2019

@marksiemers just added a help wanted tag so that we can collaborate on this.

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 10, 2019

Citus 9.0.0 has been merged to master but no release yet.

There is a chance that bumping the formula for citus will resolve one of the errors in CI.

If that is the case, will a Formula be needed for citus@8 to be compatible with some of the older postgres versions?

Or is citus 9.0.0 compatible with postgres 10 and 11 as well? @hanefi - can you comment on citus 9.x backwards compatibility?

@MatthiasWinkelmann

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2019

Postgres 10 support was just removed in citusdata/citus@74cb168. Note that this happened immediately after the 9.0 release, which should, therefore, support PG 10 and 11.

@marksiemers

This comment has been minimized.

Copy link
Author

commented Oct 15, 2019

I haven't been able to spend much time with this recently.

I'm on Catalina now, and trying a fresh postgresql (12) with brew - install gives me problems with perl headers. It seems like this happens regularly where XCode moves the location of perl.h - so if anyone has a good idea of how to handle this in the postgresql Formula, it would be great to incorporate.

I can do the install --without-perl but that seems to cause downstream issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.