Jasper Lievisse Adriaanse <jas...@openbsd.org> writes:
> On Tue, Aug 11, 2015 at 01:20:24PM -0500, attila wrote:
>> Hello tech@,
>>
>> On the 6 Aug snap I ran into this issue:
>>
>> $ pkg_info | grep libevent
>> libevent-2.0.22 event notification library
>> $ pkg-config --atleast-version=2.0.1 libevent && echo yes
>> Argument "22-stabl" isn't numeric in numeric eq (==) at
>> /usr/bin/pkg-config line 662.
>> yes
>>
>> So it works but spits out that error. The issue is that the libevent
>> port reports its version as 2.0.22-stable, which does not follow the
>> convention used by the compare() sub in pkg-config. The attached
>> patch gets rid of the error albeit in a very pointilistic way; perhaps
>> a more general solution is desired, but I'm not sure what the best
>> approach is, especially given that there is something called
>> pkg-config available pretty much everywhere but based on very
>> different implementations...
>>
>> It doesn't appear as though my patch introduces any regressions,
>> although I'm still sussing out how the regression tests work so I'm
>> not sure if I'm doing anything wrong... it appears that five of
>> pkg-config's regression tests fail regardless of my patch:
>>
>> FAIL usr.bin/pkg-config/static-cflags2
>> FAIL usr.bin/pkg-config/static-libs2
>> FAIL usr.bin/pkg-config/static-libs3
>> FAIL usr.bin/pkg-config/static-libs4
>> FAIL usr.bin/pkg-config/missing-req-2
>>
>> In all cases it looks like the difference is ordering, except for the
>> last where two errors are produced but only one is expected. I'll
>> work on a patch to fix these failures next.
> That's indeed the case and patches to fix that are welcome.
>
>> Feedback, comments most welcome.
> Could you please add regress tests for this issue you're solving too?
This turned out to be not so much difficult as cognitive
dissonance-inducing. The details are numbingly boring but I'll
happily discuss if there's an issue. In the end I have done what you
asked.
The first patch attached modifies pkg-config itself in the following
ways:
* Adds a (looks like libevent-specific) fix to compare() so that
libevent's version number in e.g. libevent.pc (2.0.22-stable) does
not cause warnings;
* Hoists the code that redirected stderr -> stdout when
--errors-to-stdout was specified from say_msg() into the main
program, so that such warnings (if they ever appear again) appear in
the .got files produced by regression tests;
* Cleans up a couple stylistic nits (add a space between ")" and "{")
in a few places to be consistent with how all the other Perl code
looks.
The second patch attached cleans up the failing regression tests.
I've verified that our pkg-config produces the same output as the(a?)
one on at least Linux given our test cases, and manually examined all
the output. It looks right to me.
As always feedback, comments, beatings most welcome.
Pax, -A
--
http://trac.haqistan.net | att...@stalphonsos.com | 0xE6CC1EDB
cvs server: Diffing .
Index: pkg-config
===================================================================
RCS file: /cvs/src/usr.bin/pkg-config/pkg-config,v
retrieving revision 1.85
diff -u -p -r1.85 pkg-config
--- pkg-config 17 Nov 2014 22:16:23 -0000 1.85
+++ pkg-config 13 Aug 2015 21:02:55 -0000
@@ -109,13 +109,24 @@ GetOptions( 'debug' => \$mode{debug},
'define-variable=s' => $variables,
);
+# If --errors-to-stdout was given, close STDERR (to be safe),
+# then dup the output to STDOUT and delete the key from %mode so we
+# won't keep checking it. STDERR stays dup'ed.
+# We do this here instead of in say_msg() to also redirect warn()
+# output to stdout to make a regression test happy (dash-stable).
+if ($mode{estdout}) {
+ close(STDERR);
+ open(STDERR, ">&STDOUT") or die "Can't dup STDOUT: $!";
+ delete($mode{estdout});
+}
+
# Unconditionally switch to static mode on static arches as --static
# may not have been passed explicitly, but we don't want to re-order
# and simplify the libs like we do for shared architectures.
{
my @static_archs = qw(vax);
my $machine_arch = $Config{'ARCH'};
- if (grep { $_ eq $machine_arch } @static_archs){
+ if (grep { $_ eq $machine_arch } @static_archs) {
$mode{static} = 1;
}
}
@@ -156,12 +167,12 @@ my $top_config = [];
# When we got here we're supposed to have had at least one
# package as argument.
-if (!@ARGV){
+if (!@ARGV) {
say_error("No package name(s) specified.");
exit 1;
}
-while (@ARGV){
+while (@ARGV) {
my $p = shift @ARGV;
my $op = undef;
my $v = undef;
@@ -233,7 +244,7 @@ if ($mode{variable}) {
my $dep_cfg_list = $cfg_full_list;
-if ($mode{static}){
+if ($mode{static}) {
$dep_cfg_list = [reverse(@$cfg_full_list)];
} else {
$dep_cfg_list = simplify_and_reverse($cfg_full_list);
@@ -439,7 +450,7 @@ sub do_modversion
if (defined $cfg) {
my $value = $cfg->get_property('Version', $variables);
if (defined $value) {
- if (defined($mode{printprovides})){
+ if (defined($mode{printprovides})) {
print "$p = " . stringize($value) . "\n";
return undef;
} else {
@@ -472,7 +483,7 @@ sub do_cflags
return 0;
}
});
- if (defined($a) && defined($variables->{pc_sysrootdir})){
+ if (defined($a) && defined($variables->{pc_sysrootdir})) {
$a =~ s/[\w]?-I/$&$variables->{pc_sysrootdir}/g;
}
@@ -512,7 +523,7 @@ sub do_libs
}
});
- if (defined($variables->{pc_sysrootdir})){
+ if (defined($variables->{pc_sysrootdir})) {
$a =~ s/[\w]?-[lL]/$&$variables->{pc_sysrootdir}/g;
}
@@ -625,16 +636,16 @@ sub compare
# is there a valid non-numeric suffix to deal with later?
# accepted are (in order): a(lpha) < b(eta) < rc < ' '.
# suffix[0] is the 'alpha' part, suffix[1] is the '1' part in 'alpha1'.
- if ($a =~ s/(rc|beta|b|alpha|a)(\d+)$//) {
- say_debug("valid suffix $1$2 found in $a$1$2.");
- $suffix_a[0] = $1;
- $suffix_a[1] = $2;
+ if ($a =~ s/(|-)(stable|rc|beta|b|alpha|a)(|\d+)$//) {
+ say_debug("valid suffix $2$3 found in $a$2$3.");
+ $suffix_a[0] = $2;
+ $suffix_a[1] = $3;
}
- if ($b =~ s/(rc|beta|b|alpha|a)(\d+)$//) {
- say_debug("valid suffix $1$2 found in $b$1$2.");
- $suffix_b[0] = $1;
- $suffix_b[1] = $2;
+ if ($b =~ s/(|-)(stable|rc|beta|b|alpha|a)(|\d+)$//) {
+ say_debug("valid suffix $2$3 found in $b$2$3.");
+ $suffix_b[0] = $2;
+ $suffix_b[1] = $3;
}
# The above are standard suffixes; deal with single alphabetical
@@ -660,7 +671,7 @@ sub compare
} else {
# extended comparison.
if (((@a == 1) || (@b == 1)) &&
- ($a[0] == $b[0])){
+ ($a[0] == $b[0])) {
# one of the arrays has reached the last element,
# compare the suffix.
@@ -824,14 +835,6 @@ sub say_msg
{
my $str = shift;
- # If --errors-to-stdout was given, close STDERR (to be safe),
- # then dup the output to STDOUT and delete the key from %mode so we
- # won't keep checking it. STDERR stays dup'ed.
- if ($mode{estdout}) {
- close(STDERR);
- open(STDERR, ">&STDOUT") or die "Can't dup STDOUT: $!";
- delete($mode{estdout});
- }
print STDERR $str . "\n";
}
cvs server: Diffing OpenBSD
cvs server: Diffing .
Index: Makefile
===================================================================
RCS file: /cvs/src/regress/usr.bin/pkg-config/Makefile,v
retrieving revision 1.49
diff -u -p -r1.49 Makefile
--- Makefile 2 Nov 2014 10:34:52 -0000 1.49
+++ Makefile 13 Aug 2015 21:06:00 -0000
@@ -79,7 +79,8 @@ REGRESS_TARGETS=cmp-vers1-1 \
variables-2 \
variables-3 \
variables-4 \
- variables-5
+ variables-5 \
+ dash-stable
PKG_CONFIG?= pkg-config
PCONFIG = PKG_CONFIG_PATH=${.CURDIR}/pcdir/ ${PKG_CONFIG}
@@ -375,7 +376,7 @@ static-cflags1:
static-cflags2:
# Test grabbing Cflags (with Requires.private)
- @echo "-I/usr/local/include/foo -I/usr/local/include" > ${WANT}
+ @echo "-I/usr/local/include -I/usr/local/include/foo" > ${WANT}
@${VPCONFIG} --cflags --static static2
@diff -u ${WANT} ${GOT}
@@ -387,19 +388,19 @@ static-libs1:
static-libs2:
# Test grabbing Libs.private from Requires in order
- @echo "-L/usr/local/lib -lc -lm -ll -lutil -lz" > ${WANT}
+ @echo "-L/usr/local/lib -lutil -lz -lc -lm -ll" > ${WANT}
@${VPCONFIG} --libs --static static2
@diff -u ${WANT} ${GOT}
static-libs3:
# Test grabbing Libs.private from Requires.private in order
- @echo "-L/tmp/lib -L/tmp/lib/foo -L/usr/local/lib -lbaz\ quux -lc -lm -ll -lutil -lz" > ${WANT}
+ @echo "-L/usr/local/lib -L/tmp/lib -L/tmp/lib/foo -lutil -lz -lc -lm -ll -lbaz\ quux" > ${WANT}
@${VPCONFIG} --libs --static static3
@diff -u ${WANT} ${GOT}
static-libs4:
# Test Requires.private
- @echo "-L/public-dep/lib -L/private-dep/lib -L/requires-test/lib -lpublic-dep -lprivate-dep -lrequires-test" > ${WANT}
+ @echo "-L/requires-test/lib -L/private-dep/lib -L/public-dep/lib -lrequires-test -lprivate-dep -lpublic-dep" > ${WANT}
@${VPCONFIG} --libs --static requires-test
@diff -u ${WANT} ${GOT}
@@ -419,7 +420,8 @@ missing-req-1:
missing-req-2:
# Test for missing packages in Requires (cflags)
- @echo "Package nonexisting was not found in the pkg-config search path" > ${WANT}
+ @echo "Package nonexisting2 was not found in the pkg-config search path" > ${WANT}
+ @echo "Package nonexisting was not found in the pkg-config search path" >> ${WANT}
@if ${VPCONFIG} --cflags missing-req; then false; fi
@diff -u ${WANT} ${GOT}
@@ -553,6 +555,12 @@ variables-5:
# Test --variable
@echo ' -lfoo-1' > ${WANT}
@${VPCONFIG} --libs variables
+ @diff -u ${WANT} ${GOT}
+
+dash-stable:
+ # Test Version: x.y.z-stable w/compare()
+ @touch ${WANT}
+ @${VPCONFIG} --atleast-version=2.0.1 dash-stable
@diff -u ${WANT} ${GOT}
clean:
cvs server: Diffing pcdir
Index: pcdir/dash-stable.pc
===================================================================
RCS file: pcdir/dash-stable.pc
diff -N pcdir/dash-stable.pc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ pcdir/dash-stable.pc 13 Aug 2015 21:06:00 -0000
@@ -0,0 +1,3 @@
+Name: dash-stable
+Description: pkg-config(1) regress file
+Version: 2.0.22-stable
