Install bindings along with radare2 #26938

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@dholm
Contributor
dholm commented Feb 23, 2014

This change depends on valabind (#26936).

@MikeMcQuaid MikeMcQuaid and 1 other commented on an outdated diff Feb 23, 2014
Library/Formula/radare2.rb
def install
- system "./configure", "--prefix=#{prefix}"
- system "make"
- system "make install"
+ # Temporarily disable PKG_CONFIG_PATH or build will fail.
+ old_pkg_config_path = ENV['PKG_CONFIG_PATH']
+ ENV['PKG_CONFIG_PATH'] = ''
+
+ system './configure', "--prefix=#{prefix}"
+ system 'make'
@dholm
dholm Feb 23, 2014 Contributor

Without it Clang claims that libewf does not provide x86-64 support. I couldn't figure out why.

@MikeMcQuaid
MikeMcQuaid Feb 23, 2014 Member

I meant the change to single quotes.

@MikeMcQuaid MikeMcQuaid and 1 other commented on an outdated diff Feb 23, 2014
Library/Formula/radare2.rb
depends_on 'libewf'
depends_on 'libmagic'
depends_on 'gmp'
depends_on 'lua'
+ # Fixes:
+ # * r2-bindings/configure using invalid C++ code to detect compiler.
+ def patches
+ DATA
@MikeMcQuaid
MikeMcQuaid Feb 23, 2014 Member

Has this been submitted upstream?

@dholm
dholm Feb 23, 2014 Contributor

Not yet. I have been struggling with getting the latest version to build on OSX but it fails brew audit. The fix will only help the next release and until we can build it properly nobody will be able to use the bindings.

@MikeMcQuaid
MikeMcQuaid Feb 23, 2014 Member

Please submit patches upstream before making PRs for us to use them.

@dholm
dholm Feb 23, 2014 Contributor

It's already fixed upstream apparently.

@MikeMcQuaid
MikeMcQuaid Feb 23, 2014 Member

Any chance of using the upstream commit here as the patch?

@dholm
dholm Feb 23, 2014 Contributor

It might be tricky as it contains incompatible changes.
radare/radare2-bindings@593a2a5#diff-e2d5a00791bce9a01f99bc6fd613a39d

@MikeMcQuaid
MikeMcQuaid Feb 23, 2014 Member

If we can use that patch, great. If not, let's use as close to it as possible.

@MikeMcQuaid
Member

Please read the Ruby Style Guide for any new lines: https://github.com/styleguide/ruby

@MikeMcQuaid
Member

Error: No available formula for vagabond

What tap is it from?

@dholm
Contributor
dholm commented Feb 23, 2014

@mikemcquaid You mean valabind? It's in a separate pull request, #26936.

@adamv
Contributor
adamv commented Mar 7, 2014

@BrewTestBot test this please

@adamv adamv and 1 other commented on an outdated diff Mar 7, 2014
Library/Formula/radare2.rb
def install
+ # Temporarily disable PKG_CONFIG_PATH or build will fail.
@adamv
adamv Mar 7, 2014 Contributor

Fail how? Need more information here.

@dholm
dholm Mar 7, 2014 Contributor

I will need to update this patch for the new radare2 release (0.9.7). I'll check if the workaround is even needed anymore. Right now I can't remember why I needed it for the previous version. ^^

@dholm
dholm Mar 8, 2014 Contributor

@adamv Fixed! That workaround is not needed with the new version.

@MikeMcQuaid
Member

vagabind is failing on Lion it seems. Also, this is failing brew audit as it needs the dylib files renamed properly (and this submitted upstream). Thanks!

@dholm
Contributor
dholm commented Mar 8, 2014

@mikemcquaid I'll have a look at valabind on Lion. The dylib files are installed by the existing radare2 formula and fail brew audit even on master. This patch simply adds the missing bindings.

@MikeMcQuaid
Member

@dholm Given it's a pretty simple rename it would be good if we could clean them up.

@dholm
Contributor
dholm commented Mar 8, 2014

@mikemcquaid Should they be renamed to .so? Do you have a reference as to why .dylib is bad as I'd like to understand the problem?

@MikeMcQuaid
Member

Instead of e.g. libr_anal.dylib.0.9.7 it should be libr_anal.0.9.7.dylib

@dholm
Contributor
dholm commented Mar 8, 2014

@mikemcquaid Oh, that makes more sense. :)
I'll try to fix it and submit upstream + separate pull request.

@MikeMcQuaid
Member

Cheers. Feel free to put in this PR though.

@dholm
Contributor
dholm commented Mar 8, 2014

@mikemcquaid I'm debugging the issue on Lion. The radare2-people likes to use custom made scripts for their build systems (as opposed to i.e. autoconf). Is it possible to get buildbot to run an instrumented build (i.e. me adding some echos in the scripts) without jumping through a ton of hoops?

@MikeMcQuaid
Member

It'll print out the brew install -v output if it fails so: yeh, I guess so.

@dholm
Contributor
dholm commented Mar 8, 2014

Libraries fixed. Next up, valabind on Lion.

@dholm
Contributor
dholm commented Mar 8, 2014

@mikemcquaid The reason valabind fails on Lion is because vala appears to be broken:

http://bot.brew.sh/job/Homebrew%20Pull%20Requests/8362/version=lion/testReport/junit/brew-test-bot/lion/install_valabind/
strings: object: /usr/local/opt/vala/bin/valac malformed object (unknown load command 17)

This looks unrelated to both valabind and radare2.

@adamv
Contributor
adamv commented Apr 7, 2014

@BrewTestBot test this please

@dholm
Contributor
dholm commented Apr 12, 2014

@BrewTestBot test this please

@adamv adamv self-assigned this Apr 12, 2014
@adamv
Contributor
adamv commented Apr 12, 2014

"test this please" only works for a maintainer, but pushing new commits to an existing pull request will also trigger the build bot

@adamv
Contributor
adamv commented Apr 14, 2014

@BrewTestBot test this please

@adamv
Contributor
adamv commented May 4, 2014

@BrewTestBot test this please

@adamv
Contributor
adamv commented May 4, 2014

Copying in Lion build error:

==> Installing radare2 dependency: valabind
==> Downloading https://github.com/radare/valabind/archive/0.8.0.tar.gz
Already downloaded: /Library/Caches/Homebrew/valabind-0.8.0.tar.gz
==> Verifying valabind-0.8.0.tar.gz checksum
tar xf /Library/Caches/Homebrew/valabind-0.8.0.tar.gz
==> make
strings: object: /usr/local/opt/vala/bin/valac malformed object (unknown load command 17)
Cannot find valac version string
fatal: Not a git repository (or any of the parent directories): .git
Generating config.vala
mkdir -p build
Generating main.vala -> build/main.vapi
Generating valabindwriter.vala -> build/valabindwriter.vapi
Generating nodeffiwriter.vala -> build/nodeffiwriter.vapi
Generating utils.vala -> build/utils.vapi
Generating girwriter.vala -> build/girwriter.vapi
Generating swigwriter.vala -> build/swigwriter.vapi
Generating cxxwriter.vala -> build/cxxwriter.vapi
Generating ctypeswriter.vala -> build/ctypeswriter.vapi
Generating dlangwriter.vala -> build/dlangwriter.vapi
Generating config.vala -> build/config.vapi
Compiling config.vala main.vala valabindwriter.vala nodeffiwriter.vala utils.vala girwriter.vala swigwriter.vala cxxwriter.vala ctypeswriter.vala dlangwriter.vala -> valabind
valac -g --cc="clang"  -o valabind --pkg posix --pkg  --save-temps  config.vala main.vala valabindwriter.vala nodeffiwriter.vala utils.vala girwriter.vala swigwriter.vala cxxwriter.vala ctypeswriter.vala dlangwriter.vala 
error: Package `--save-temps' not found in specified Vala API directories or GObject-Introspection GIR directories
Compilation failed: 1 error(s), 0 warning(s)
make: *** [valabind] Error 1
Error: valabind 0.8.0 did not build
@adamv
Contributor
adamv commented May 4, 2014

There's a 0.24.0 version of Vala; is every version stable or does Vala have that even/odd versioning scheme?

@adamv adamv removed their assignment May 13, 2014
@adamv
Contributor
adamv commented May 14, 2014

@BrewTestBot test this please

@adamv
Contributor
adamv commented May 14, 2014

Is valabind expected to work on Lion or no?

@adamv
Contributor
adamv commented May 14, 2014

MacPorts doesn't seem to be doing anything special: https://trac.macports.org/browser/trunk/dports/devel/valabind/Portfile

@dholm
Contributor
dholm commented May 17, 2014

@adamv I still think it's an issue in vala on Lion. I am planning to push an update to the vala recipe with a better test case than the current in order to try to verify this is the case.

@dholm
Contributor
dholm commented May 17, 2014

Hmm, I'm reevaluating the build output of valabind on Lion and it actually looks like one of the package parameters to valac is messed up, triggering the build error:

valac -g --cc="clang"  -o valabind --pkg posix --pkg  --save-temps

The second package name is missing so it is interpreting --save-temps as a package name and not another option. Digging deeper..

@dholm
Contributor
dholm commented May 17, 2014

Issue identified and fix submitted upstream.

@adamv
Contributor
adamv commented May 20, 2014

@BrewTestBot test this please

@adamv adamv added a commit that closed this pull request May 20, 2014
@dholm @adamv dholm + adamv radare2: install bindings
Closes #26938.

Signed-off-by: Adam Vandenberg <flangy@gmail.com>
6a99dfd
@adamv adamv closed this in 6a99dfd May 20, 2014
@wjlroe wjlroe added a commit to wjlroe/homebrew that referenced this pull request Jun 15, 2014
@dholm @wjlroe dholm + wjlroe radare2: install bindings
Closes #26938.

Signed-off-by: Adam Vandenberg <flangy@gmail.com>
509d9de
@dholm dholm deleted the dholm:radare2-bindings branch Nov 2, 2014
@xu-cheng xu-cheng 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.