Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

mbelib: 1.2.5 - mbelib, P25 Phase 1 and ProVoice vocoder #21792

Closed
wants to merge 1 commit into from
Closed

mbelib: 1.2.5 - mbelib, P25 Phase 1 and ProVoice vocoder #21792

wants to merge 1 commit into from

Conversation

grempe
Copy link
Contributor

@grempe grempe commented Aug 10, 2013

Needed to compile DSD which will be the subject of a
future pull request.

See:
https://github.com/szechyjs/mbelib
https://github.com/szechyjs/dsd

@mistydemeo
Copy link
Member

Looks like the prefix isn't passed anywhere, so presumably this isn't being installed to the correct prefix?

Also, re: the issue you linked to this from, is this working on OS X yet?

@grempe
Copy link
Contributor Author

grempe commented Aug 12, 2013

It does appear to be installed to the correct location. However I think you are right in that the uninstallation is not working cleanly. Thanks for catching that.

...
built target mbe-static
Install the project...
-- Install configuration: ""
-- Installing: /usr/local/lib/libmbe.a
-- Installing: /usr/local/lib/libmbe.1.0.dylib
-- Installing: /usr/local/lib/libmbe.1.dylib
-- Installing: /usr/local/lib/libmbe.dylib
-- Installing: /usr/local/include/mbelib.h
==> Cleaning
==> Finishing up
ln -s ../../Cellar/mbelib/1.2.5 mbelib
ln -s ../Cellar/mbelib/1.2.5 mbelib
==> Summary
 /usr/local/Cellar/mbelib/1.2.5: 4 files, 16K, built in 2 seconds
➜  Formula git:(mbelib) ✗ ls -la /usr/local/lib/libmb*
-rwxr-xr-x  1 glenn  staff   99260 Aug 12 09:05 /usr/local/lib/libmbe.1.0.dylib
lrwxr-xr-x  1 glenn  staff      16 Aug 12 09:05 /usr/local/lib/libmbe.1.dylib -> libmbe.1.0.dylib
-rw-r--r--  1 glenn  staff  100648 Aug 12 09:05 /usr/local/lib/libmbe.a
lrwxr-xr-x  1 glenn  staff      14 Aug 12 09:05 /usr/local/lib/libmbe.dylib -> libmbe.1.dylib
➜  Formula git:(mbelib) ✗ brew uninstall mbelib
Uninstalling /usr/local/Cellar/mbelib/1.2.5...
➜  Formula git:(mbelib) ✗ ls -la /usr/local/lib/libmb*
-rwxr-xr-x  1 glenn  staff   99260 Aug 12 09:05 /usr/local/lib/libmbe.1.0.dylib
lrwxr-xr-x  1 glenn  staff      16 Aug 12 09:05 /usr/local/lib/libmbe.1.dylib -> libmbe.1.0.dylib
-rw-r--r--  1 glenn  staff  100648 Aug 12 09:05 /usr/local/lib/libmbe.a
lrwxr-xr-x  1 glenn  staff      14 Aug 12 09:05 /usr/local/lib/libmbe.dylib -> libmbe.1.dylib

I'll look into that and fix.

Regarding your second questions about the referenced bug. That was for the 'dsd' installation which I have not yet submitted to homebrew as a pull request. dsd has a dependency on this mbe library the two can be separated from each other. I just wanted to get the mbelib portion of the install out of the way first. This will also be helpful for the 'dsd' developers who will have an easier time getting this dependency.

@mistydemeo
Copy link
Member

Homebrew actually installs software to isolated prefixes, then symlinks them into /usr/local. For example, with mbelib, the prefix would actually be /usr/local/Cellar/mbelib/1.2.5

These are versioned, and also make it possible for users to temporarily unlink them, switch between versions, etc.

The prefix method in the formula returns the correct prefix for the given formula. For cmake formulae, we also provide the std_cmake_args helper method that returns an array with several standard cmake arguments (including the prefix). For example, when you call cmake, you could do this instead:

system "cmake", "..", *std_cmake_args

@mistydemeo
Copy link
Member

These things are documented in the Formula Cookbook, so if you haven't already give it a read.

depends_on 'cmake' => :build

def install
remove_dir('build', true)
Copy link
Member

Choose a reason for hiding this comment

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

Why are you removing the build directory, then recreating it?

Updated to use *std_cmake_args and stop removal and then immediate
rebuild of build dir.

This lib is Needed to compile DSD which will be the subject of a
future pull request.

See:
https://github.com/szechyjs/mbelib
https://github.com/szechyjs/dsd
@grempe
Copy link
Contributor Author

grempe commented Aug 13, 2013

Thanks for the feedback Misty. This is why Homebrew is awesome. :-)

I've made the changes you suggested (all made sense) and everything seems to be working now. I've force pushed an amended commit on top of my old pull request commit. You should be able to see the updated code here:

https://github.com/grempe/homebrew/commit/ff074588d8546d74ba375c40a787f9d8ded934f9

I had read the formula page, but '*std_cmake_args' was not documented at all on that page (it was only a commented out line of code in one of the examples).

I think its good to go now.

Thanks,

Glenn

depends_on 'cmake' => :build

def install
mkdir 'build'
Copy link
Contributor

Choose a reason for hiding this comment

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

mkdir also has a block form, so you can do mkdir 'build' do ...

@ghost ghost assigned adamv Sep 23, 2013
@adamv adamv closed this in 371a75b Sep 24, 2013
@grempe
Copy link
Contributor Author

grempe commented Sep 24, 2013

Thanks adamv.

handyman5 pushed a commit to handyman5/homebrew that referenced this pull request Oct 7, 2013
Closes Homebrew#21792.

Signed-off-by: Adam Vandenberg <flangy@gmail.com>
@Homebrew Homebrew 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants