add a stage-qa target for USES=gem runtime dependencies

[lifanov]

Summary

This finds unsatisfied runtime dependencies so that Vagrant, Metasploit, etc. don't break so often:

====> Running Q/A tests (stage-qa)
Error: RubyGem dependency archive-tar-minitar = 0.5.2 is not satisfied.
*** Error code 1

Stop.
make: stopped in /usr/home/lifanov/src/svn/freebsd/ports/head/archivers/rubygem-fpm

Test Plan

it works - needs review, exp-run, and fixing fallout

[mat]

Mk/Scripts/qa.sh

828

rewrite this to use heredocs like the rest of the file does.

829

. ?

830

s/-print0 | xargs -0/-exec/
s/dependency/dependency {} +/

[lifanov]

o address mat's feedback
o use pipe continuation style consistent with the rest of the file

[mat]

Please, do not do | while, it runs all your code in a subshell, use a heredocs, something like:

while read -r l; do
  # No results presents a blank line from heredoc.
  [ -z "${l}" ] && continue
  ...
end <<-EOT
$(find ${STAGEDIR}${PREFIX} -type f \
	​-path '*/specifications/*\.gemspec' \
	​-exec grep -a add_runtime_dependency {} + \
	​| sed 's|.*<\(.*\)>.*\[\(.*\)\])|\1 \2|')
EOT

and fill the msg variable directly.

Also, put all your variables in brackets, do ${msg}, not $msg

[mat]

[16:19:44] <swills> mat: it might be helpful to understand why subshells should be avoided and maybe document that
[16:23:43] <mat> because you have to use $() to extract values, you mask many errors, and it's a bitch to debug :-)
[16:24:44] <swills> mind pasting that into the review?
[16:25:12] <mat> and forks are bad, every fork you add end up costing time when you run bulk -a

[lifanov]

address mat's feedback:
o feed while loops with heredocs
o use ${msg} instead of $msg

I'm not sure what you mean by fill the msg variable directly...

[mat]

I mean remove the msg=$(, and the corresponding ).

And fill the msg variable directly instead of doing a printf:

msg="RubyGem dependency <${name} ["${v}"]> is not satisfied"

Mk/Scripts/qa.sh

838

You could probably use ruby -e "puts BAD..." possibly with a heredoc.

840

-n "${bar}"

856

-n "${msg}"

[lifanov]

address mat's feedback

o use ruby -e instead of pipe
o don't implicitly test for empty strings
o populate ${msg} directly

[mat]

Mk/Scripts/qa.sh

828

should probably be local msg one line before. No need to set it to the empty string then.

[lifanov]

address mat's feedback:
make ${msg} local

This is not POSIX, but all interesting shells support it.

[lifanov]

Is this close enough to ask for an exp-run at this point?

[lifanov]

I submitted an exp-run request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220605

[sunpoet]

Thanks. I'll be glad to help to fix the failure caught by exp-run.

[mat]

Mk/Scripts/qa.sh

833

There are still variables here that are not inside braces.

838–840

Here too, $vers should be ${vers}.

844–846

I wonder.

Would not this be simpler:

grep -a add_runtime_dependency ${STAGEDIR}${PREFIX}/lib/ruby/gems/*/specifications/*.gemspec

Or are there occurrences of ruby gems where the files are not installed there ?

[lifanov]

address mat's feedback:

o more consistent use of ${var}
o be more specific about gemspec to test

[mat]

Mk/Scripts/qa.sh

844

Well, this is STAGEDIR, there should be only one gemspec file in there, ever.
I am not certain being this specific is useful.

[lifanov]

There *should* be, but that's not the case everywhere in the tree.
For example, www/rubygem-merb-core bundles a bunch of crap, including an older version of itself.

I considered making this fatal, but after discussing it with swills, we decided that it's OK-ish?

[mat]

Sure, it was just a thought. (approval here means it's ok, you still need to do the exp-run dance and fix fallout before committing this.)

[lifanov]

Thank you!

[lifanov]

handle situation where multiple local versions of the same gem are installed

This was found from exp-run by sysutils/rubygem-chef, which has dependency
chain that includes multiple versions of mime-types

This needs review again, sorry mat...

[mat]

I never reviewed the code itself, only the style, so it still works for me :-)

[lifanov]

Great, thank you! As long as the style is still OK...

[lifanov]

o fix case with no runtime dependencies (devel/rubygem-extlib)
o speed up by deduplicating a list of version strings to check

[lifanov]

don't skip testing ports with a suffix (like www/rubygem-rack16)

[sunpoet]

1. QA messages should be handled via notice(), warn() or err(). Please use err() instead of $msg.
2. Remove unnecessary <[]> and add a period.

That changes

RubyGem dependency <foo [~> 1.2.3]> is not satisfied

to

Error: RubyGem dependency foo ~> 1.2.3 is not satisfied.

Mk/Scripts/qa.sh

855

Remove this line since err() already shows the error message.

[lifanov]

address sunpoet's feedback:

o use err() and change message format
o use rc status instead of ${msg}

[sunpoet]

LGTM. Thank you!

[lifanov]

fix finding .gemspec for PKGBASE with numeric suffix

This is found by www/rubygem-rails4, which has the suffix in the package name
and it regressed limiting the scope of the search to just the one spec
in order to allow ports that bundle gems that can't possibly work to work.

[mat]

In D11491#244577, @lifanov wrote:

fix finding .gemspec for PKGBASE with numeric suffix

This is found by www/rubygem-rails4, which has the suffix in the package name
and it regressed limiting the scope of the search to just the one spec
in order to allow ports that bundle gems that can't possibly work to work.

Are there no gem names ending with a number?

[mat]

Like, say, net/rubygem-omniauth-google-oauth2 which name is omniauth-google-oauth2.

[mat]

You might want to use PORTNAME and not fiddle with PKGBASE for it.

[lifanov]

address mat's feedback:

Yes, you're right about PORTNAME. Getting gem name from PKGBASE didn't even work correctly. I switched to using PORTNAME here, which does the right thing.

If it was up to me, I would just check everything under
specifications/*.gemspec, but apparently we want to allow broken
bundled gemspecs to be installed.

[lifanov]

just export PORTNAME instead of "make -VPORTNAME -C../.." dance

[mat]

In D11491#244608, @lifanov wrote:

just export PORTNAME instead of "make -VPORTNAME -C../.." dance

I'm curious about what ../.. was referring to.

[lifanov]

qa.sh runs in STAGEDIR

[mat]

In D11491#244636, @lifanov wrote:

qa.sh runs in STAGEDIR

Mmm, ok, so, ../.. would be ${WRKDIRPREFIX}${.CURDIR}/work/stage/../.., so, there is a good chance nothing is in that directory except a directory called work :-)

[lifanov]

Yeah, that's right. Just adding PORTNAME to QA_ENV works though.

[lifanov]

What do you think of the current approach?

[mat]

If this is for me, I approved the patch a while back :-)

[lifanov]

This is done, but I accidentally put the wrong revision in commit message.