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

version bump: dev-libs/hiredis 0.13.1 #136

Closed
wants to merge 15 commits into from
Closed

version bump: dev-libs/hiredis 0.13.1 #136

wants to merge 15 commits into from

Conversation

jbergstroem
Copy link

Bump including a few fixes and features.

question: does insinto/doins require ${ED}?

@jbergstroem
Copy link
Author

Not sure about returning 0/1 vs die in src_test. Would appreciate some feedback on improving that as well.

@jbergstroem
Copy link
Author

Also, how do we handle the dep to dev-db/redis for the other arches?

@jbergstroem
Copy link
Author

bringing @neurogeek (maintainer) in here.

@jbergstroem
Copy link
Author

ping @mgorny

static void test_blocking_connection_errors(void) {
redisContext *c;

+ /*
Copy link
Member

Choose a reason for hiding this comment

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

Safer to do #if 0. Won't break on comments in the middle of it :).

@jbergstroem
Copy link
Author

So, how do we handle KEYWORDS between hiredis and redis?

@mgorny
Copy link
Member

mgorny commented Jun 3, 2015

Well, obviously you drop the keywords that are not on redis.

@mgorny mgorny changed the title version bump: dev-libs/hidredis 0.13.1 version bump: dev-libs/hiredis 0.13.1 Jun 3, 2015
@mgorny
Copy link
Member

mgorny commented Jun 3, 2015

Now, if you really believe someone used hiredis on those arches, we need a new keywordreq for both packages.

@jbergstroem
Copy link
Author

@mgorny ..or expand dev-db/redis? I can't vouch wider than x86/amd64. Keywords seems to been around for a while (besides alpha) so it'd be great to get @neurogeek's opinion.

@jbergstroem
Copy link
Author

Just added an attempt to fix the paths in the pkg-config file. I feel like it isn't quite enough though;

  • how would I handle prefix installs?

  • the generated .pc seems to hardcode lib path, should i rewrite that to use libdir instead?

    $  pkg-config --libs hiredis                                             
    -L/usr/lib -lhiredis 

@mgorny
Copy link
Member

mgorny commented Jun 5, 2015

I guess you'd pass ${EPREFIX}/usr on Prefix but I'm not sure how exactly PREFIX works in the package. And yep, you gotta fix the libdir in pkg-config file.

@mgorny
Copy link
Member

mgorny commented Jun 5, 2015

Just to be clear, if you're not sure about Prefix support, you can leave it out. Prefix is pretty much 'if we can, we do it, if we can't, we leave it to prefix@ to fix :)'.

local REDIS_PID="${T}"/hiredis.pid
local REDIS_SOCK="${T}"/hiredis.sock
local REDIS_PORT=56379
local REDIS_TEST_CONFIG=$"daemonize yes
Copy link
Member

Choose a reason for hiding this comment

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

Well, $"" implies parsing escapes like \n. If you don't use them anymore, use plain "" :).

Copy link
Author

Choose a reason for hiding this comment

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

The config assumes each option is on a newline

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but you don't need $"" for the inline newlines.

@mgorny
Copy link
Member

mgorny commented Jun 12, 2015

@jbergstroem, are you still there? Also you need to rebase :P.

@jbergstroem
Copy link
Author

Rebased and fixed outstanding issues.

@mgorny
Copy link
Member

mgorny commented Jun 15, 2015

+*hiredis-0.13.1 (15 Jun 2015)
+
+  15 Jun 2015; Michał Górny <mgorny@gentoo.org>
+  +files/hiredis-0.13.1-disable-network-tests.patch, +hiredis-0.13.1.ebuild:
+  Version bump with some fixes. Install a pkg-config file. Run tests against a
+  isolated redis instance.

Also:

  KEYWORDS.dropped              1
   dev-libs/hiredis/hiredis-0.13.1.ebuild: alpha arm ia64 ppc sparc

Now you go file a keywordreq :).

@mgorny mgorny closed this Jun 15, 2015
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