Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Pgtap #25903

Closed
wants to merge 12 commits into from
Closed

Pgtap #25903

wants to merge 12 commits into from

Conversation

theory
Copy link
Contributor

@theory theory commented Jan 14, 2014

First commit updates pgTAP to v0.94.0. Second adds a method that installs pg_prove from CPAN, which simplifies the running of pgTAP tests.

@MikeMcQuaid
Copy link
Member

Needs rebased on master.

@theory
Copy link
Contributor Author

theory commented Jan 14, 2014

Oops, sorry, thought I just forked, but I guess I had an old fork. Rebased now.


depends_on :postgresql
depends_on :cpanminus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no symbol version of this dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks!

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Hrm, looking at this failure and wondering if it worked before.

ENV.remove_from_cflags(/-msse\d?/)

# Install TAP::Parser::SourceHandler::pgTAP.
system "cpanm --local-lib '#{prefix}' --notest TAP::Parser::SourceHandler::pgTAP"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this interact with ~/.cpan at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it will leave files in ~/.cpan.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't modify the home directory during installation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm. Should I add a line to rm -rf ~/.cpan? How does one usually install CPAN modules? @miyagawa, is there a cpanm flag to use a different directory? I couldn't find anything in a quick scan of the man page.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See clipsafe.rb.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ha! Switched to that approach in eaef0db. Wouldn't want to do that for something like Sqitch, which requires a shitload of modules, but for this narrow case, I think it makes a lot of sense. Thanks!

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

We still have this issue:

mkdir: /usr/share/doc/postgresql: Permission denied
mkdir: /usr/share/doc/postgresql: No such file or directory

What are the permissions used by the Postgres build? Did this work for previous versions of pgTAP? Do we need to do sudo make install, perhaps?

@MikeMcQuaid
Copy link
Member

I would guess there's a hardcoded path that needs fixed.

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Hrm. I have postgresql-9.2.3 installed. I ran the build on my box and it worked. The Perl stuff was included in /usr/local/Cellar/pgtap/0.94.0/, while the pgTAP extension was installed in /usr/local/Cellar/postgresql/9.2.3/share/postgresql/. No error creating the doc directory, which PGXS determines from pg_config --docdir. I'm mystified what the permission issue could be about on the test host(s).

@MikeMcQuaid
Copy link
Member

There's no issue on the test host; it does a clean install every time.

@mistydemeo
Copy link
Member

The non-server edition of OS X includes a postgres client (but not a server), which includes a /usr/bin/pg_config. Possibly that's ending up in the PATH before the postgres the build is meant to be using.

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Hey @mistydemeo. Hrm. Maybe this line doesn't do what I think it should?

ENV.prepend_path 'PATH', Formula.factory('postgresql').bin

@mistydemeo
Copy link
Member

Nope, that looks right. Drop in a which pg_config just to see if you're getting it instead?

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Hrm. The scripts are not getting linked, either. That is, these three apps:

> ll /usr/local/Cellar/pgtap/0.94.0/libexec/bin 
total 96
-r-xr-xr-x  1 david  admin    19K Jan 15 13:59 pg_prove*
-r-xr-xr-x  1 david  admin    11K Jan 15 13:59 pg_tapgen*
-r-xr-xr-x  1 david  admin    13K Jan 15 13:59 prove*

Do I need to do something else to tell brew to link those?

@mistydemeo
Copy link
Member

Those are in libexec, which is usually not exposed publicly.

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Oh. So should I use INSTALL_BASE=%{lib} or something instead?

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

On my box, system 'which', 'pg_config' outputs /usr/local/bin/pg_config, which is correct. Want me to push that?

@mistydemeo
Copy link
Member

Might as well, just to see what the bot is picking up.

@mistydemeo
Copy link
Member

/bin/sh /usr/lib/postgresql/pgxs/src/makefiles/../../config/install-sh -c -d '/usr/share/postgresql/extension'
/bin/sh /usr/lib/postgresql/pgxs/src/makefiles/../../config/install-sh -c -d '/usr/share/doc/postgresql/extension'
mkdir: /usr/share/doc/postgresql: Permission denied
mkdir: /usr/share/doc/postgresql: No such file or directory

Wait, I see here: looks like it's picking up on a postgres in /usr/lib.

@mistydemeo
Copy link
Member

Ping @MikeMcQuaid - are the bots running OS X server?

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

  • 58ea9cf: Using prefix instead of libexec. That gets things linked up.
  • 577fbfa: Try to find out what pg_config we're using.

@MikeMcQuaid
Copy link
Member

One of them has it installed.

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Yeah, this failure clearly shows that it is not picking up the right pg_config, even with the path prepend. WTF? I can pass PG_CONFIG to make instead.

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Oh-ho! I switched to this:

pg_config = File.join(Formula.factory('postgresql').bin, 'pg_config')
system 'make', "PG_CONFIG=#{pg_config}"
system 'make', "PG_CONFIG=#{pg_config}", 'install'

And now get this complaint:

/bin/sh: /usr/local/Cellar/postgresql/9.3.2/bin/pg_config: No such file or directory

This is because I have 9.2 installed, not 9.3. So getting the bin dir from the postgresql formula is not reliable. Should I just use /usr/local/bin/pg_config?

@mistydemeo
Copy link
Member

Use:

pg_config = Formula.factory('postgresql').opt_prefix/'bin/pg_config'

(The path objects you return from Formula instances are Pathname objects, which support concatenation using the / operator.)

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

Okay, c818ec2 should get it right!

@theory
Copy link
Contributor Author

theory commented Jan 15, 2014

So much for that idea:

/bin/sh: /usr/local/opt/postgresql/bin/pg_config: No such file or directory

Also need to figure out how to get Test::Harness linked up before building TAP::Parser::SourceHandler::pgTAP, on which it depends.

@mistydemeo
Copy link
Member

Oh, of course.

This is a consequence of the symbol-form postgres dependency, which is using an existing postgres install if present. So, on those VMs, it's not actually installing the Homebrew postgres. That's why prepending the path did nothing! The Homebrew postgres wasn't installed.

Rather than using cpanm. Idea borrowed from `clipsafe.rb`, with thanks to
@MikeMcQuaid for the pointer.
So that the apps (prove, pg_prove, pg_tapgen) will be properly linked.
Because the latter allows the system postgres, but we cannot install into its directories.
The reason for setting $PERL5LIB is so that, when
TAP::Parser::SourceHandler::pgTAP is built, it can find Test::Harness, which
will have been built, but not yet symlinked.
@theory
Copy link
Contributor Author

theory commented Jan 16, 2014

Rebased.

ehershey pushed a commit to ehershey/homebrew that referenced this pull request Apr 4, 2014
Closes Homebrew#25903.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants