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

add option not to use pcrecpp #12

Merged
merged 1 commit into from
May 2, 2015
Merged

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Dec 27, 2014

I still have trouble on #7 and it seems something trouble if we load collada-dom vid dlopen and host program already have liked with pcre library
This PR rewrite function using pcrecpp, and confirmed it works on test code fixed in #11

@rdiankov
Copy link
Owner

intesting way of getting around this issue ;0)
using liburiparser is fine, but if you make it default, then existing users might have trouble compiling.

how about looking for liburiparser first, and then if that isn't found, falling back to using pcre?

@k-okada
Copy link
Contributor Author

k-okada commented Dec 28, 2014

I'm not familiar with liburiparser, but it seems very stable library, it is supported from 12.04 to 15.04
http://packages.ubuntu.com/search?lang=es&keywords=liburiparser&searchon=names

My concern is I'm not confident this will get "exactly" same results as pcre provides, I have tested on test code, and it seems ok, but might be something that I missed.

@rdiankov
Copy link
Owner

i think it is a great idea to use liburiparser.
i'll look into it more to see if behavior is the same. give me a week for this ;0)

@130s
Copy link

130s commented May 1, 2015

ping @rdiankov

@k-okada
Copy link
Contributor Author

k-okada commented May 1, 2015

this can be resolved in ros/robot_model#108

◉ Kei Okada

On Fri, May 1, 2015 at 1:34 PM, Isaac I.Y. Saito notifications@github.com
wrote:

ping @rdiankov https://github.com/rdiankov


Reply to this email directly or view it on GitHub
#12 (comment).

rdiankov added a commit that referenced this pull request May 2, 2015
add option not to use pcrecpp
@rdiankov rdiankov merged commit f9bae8e into rdiankov:master May 2, 2015
@rdiankov
Copy link
Owner

rdiankov commented May 2, 2015

oops, i thought i had merged this. thanks for the pull request!

i just made a minor commit to fallback on pcrecpp

5939c33

@k-okada
Copy link
Contributor Author

k-okada commented May 4, 2015

thanks, merging this it's self is ok, but to be effective on ros users,

  1. update openrave launchpad
  2. ask tully? to copy that launch pad deb to package.ros.org

◉ Kei Okada

On Sat, May 2, 2015 at 11:30 AM, Rosen Diankov notifications@github.com
wrote:

oops, i thought i had merged this. thanks for the pull request!

i just made a minor commit to fallback on pcrecpp

5939c33
5939c33


Reply to this email directly or view it on GitHub
#12 (comment).

@k-okada k-okada deleted the remove_pcre branch June 20, 2015 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants