mrboom 3.9 (new formula) #20746

frranck · 2017-11-17T21:54:45Z

frranck · 2017-11-17T22:01:00Z

Error: 1 problem in 1 formula
mrboom:

C: 9: col 14: Dependency sdl2_mixer should not use option with-libmodplug

why?

ilovezfs · 2017-11-21T18:46:07Z

See Homebrew/brew#2482

frranck · 2017-11-21T21:39:18Z

@ilovezfs So how to fix that ? Does that mean we can't use most features of sdl2_mixer ?

ilovezfs · 2017-11-25T12:09:03Z

Either libmodplug needs to become a default dependency of the sdl2_mixer formula, or sdl2_mixer needs to be vendored into this formula.

frranck · 2017-11-25T14:30:21Z

@ilovezfs What is the best option ?
Shall I make a PR on sdl2_mixer, or do you have an example of vendoring ?

ilovezfs · 2017-11-25T14:56:43Z

@frranck it looks like a tiny dependency so you can make an sdl2_mixer PR :)

ilovezfs · 2017-11-26T10:28:22Z

Formula/mrboom.rb

+
+  def install
+    system "make", "mrboom", "LIBSDL2=1"
+    bin.install "mrboom.out" => "mrboom"


we'll need a make install target

There's one in the Makefile, what is missing here exactly ?

Looks like you you need

system "make", "install", "PREFIX=#{prefix}", "MANDIR=#{man6}"

ilovezfs · 2017-11-26T10:28:45Z

Formula/mrboom.rb

+  end
+
+  test do
+    system "#{bin}/mrboom", "--version"


we'll need a test that exercises and verifies the functionality of the software

It does, it's checking decrunching the data and modplug when running version.

something that loads a config file or similar would work. --version and --help tests aren't allowed for new formulae

OK then I should simply remove it

You must have a test block. Just not one that does --version or --help.

ilovezfs · 2017-11-26T11:08:31Z

Formula/mrboom.rb

@@ -0,0 +1,19 @@
+class Mrboom < Formula
+  desc "Mr.Boom is an 8 player Bomberman clone"
+  homepage "http://mrboom.mumblecore.org"


http://mrboom.mumblecore.org/

Yes ? Any clue whats wrong, it fails on
install -m 555 mrboom.out /usr/local/Cellar/mrboom/3.8/bin/mrboom

Yes ?

should have trailing slash

Any clue whats wrong, it fails on
install -m 555 mrboom.out /usr/local/Cellar/mrboom/3.8/bin/mrboom

It's a Makfile bug. It's assuming BINDIR already exists, which is not the case.

It needs

diff --git a/Makefile b/Makefile index 80a8e95..d795d19 100644 --- a/Makefile +++ b/Makefile @@ -256,7 +256,9 @@ strip: $(STRIP) $(TARGET_NAME).out install: strip + $(INSTALL) -m 0755 -d $(DESTDIR)$(PREFIX)/$(BINDIR) $(INSTALL) -m 555 $(TARGET_NAME).out $(DESTDIR)$(PREFIX)/$(BINDIR)/$(TARGET_NAME) + $(INSTALL) -m 0755 -d $(DESTDIR)$(PREFIX)/$(MANDIR) $(INSTALL) -m 644 Assets/$(TARGET_NAME).6 $(DESTDIR)$(PREFIX)/$(MANDIR) .PHONY: clean

and it's actually MANDIR=share/man/man6 that you need because it does it with relative paths, which is quite weird, but 🤷‍♂️

Thanks, do you have an example on how to modify the formula to include that patch ?

You'll need to commit upstream and then use a patch do block like this:
https://github.com/Homebrew/homebrew-core/blob/master/Formula/tiger-vnc.rb#L21-L27

or just tag a new release

ilovezfs · 2017-11-26T13:21:38Z

Formula/mrboom.rb

+
+  def install
+    system "make", "mrboom", "LIBSDL2=1"
+    system "make", "install", "PREFIX=#{prefix}", "MANDIR=#{man6}"


needs to be with the relative path MANDIR=share/man/man6

that's changed inside the Makefile
you think we should change this to
system "make", "install", "PREFIX=#{prefix}", "MANDIR=share/man/man6" ?

yeah your MANDIR is defined as a relative path https://github.com/Javanaise/mrboom-libretro/blob/master/Makefile#L13

it's changed there https://github.com/Javanaise/mrboom-libretro/blob/master/Makefile#L99

Ah, then don't set it at all here. just PREFIX

ilovezfs · 2017-11-26T13:23:02Z

Formula/mrboom.rb

@@ -0,0 +1,23 @@
+class Mrboom < Formula
+  desc "Mr.Boom is an 8 player Bomberman clone"


desc "Eight player Bomberman clone"

ilovezfs · 2017-11-26T13:23:36Z

Formula/mrboom.rb

+  def test
+    mrboom.out -v
+  end
+


audit won't like this blank line

ilovezfs · 2017-11-26T13:24:15Z

Formula/mrboom.rb

+  end
+
+  def test
+    mrboom.out -v


this will cause a ruby syntax error

ilovezfs · 2017-11-26T13:25:10Z

Formula/mrboom.rb

+  test do
+    system "#{bin}/mrboom", "--version"
+  end
+


audit won't like this blank line

ilovezfs · 2017-11-26T13:25:23Z

Formula/mrboom.rb

+  end
+
+  test do
+    system "#{bin}/mrboom", "--version"


still need a non --version test

Like what ? --help ?

ilovezfs · 2017-11-26T13:33:01Z

Looks good. So we just need the non --version test.

ilovezfs · 2017-11-26T13:33:46Z

And non --help hehe

frranck · 2017-11-26T13:35:28Z

what do you mean ? those are the only the only two options that don't launch the game...

ilovezfs · 2017-11-26T14:26:15Z

like

  test do
    require "pty"
    PTY.spawn(bin/"mrboom", "-m") do |r, _w, pid|
      begin
        sleep 1
        Process.kill "SIGINT", pid
        assert_match "monster", r.read
      ensure
        Process.wait pid
      end
    end
  end

ilovezfs · 2017-11-26T14:29:13Z

Formula/mrboom.rb

+
+  depends_on "cmake" => :build
+  depends_on "libmodplug"
+  depends_on "lzlib"


I don't think this dependency is used

frranck · 2017-11-26T17:02:33Z

@ilovezfs Did we address all the points ?

ilovezfs · 2017-11-26T17:06:46Z

Thanks for the 🆕 formula @frranck! 💣

frranck · 2017-11-27T18:25:18Z

@ilovezfs: was the sdl2_mixer merged too ? it doesn't seem to work:

$ brew install mrboom
==> Installing dependencies for mrboom: sdl2, libogg, sdl2_mixer
==> Installing mrboom dependency: sdl2
==> Downloading https://homebrew.bintray.com/bottles/sdl2-2.0.7.sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/8e/8e0ed1c42064a78da85f0375fa32e36e2f6e94d33fa1acbf67b7b2777691aeed?__gda__=exp=1511795928~hmac=c67bc08cacd947623f45e79905dde6ccddd1e6ea
######################################################################## 100.0%
==> Pouring sdl2-2.0.7.sierra.bottle.tar.gz
🍺 /usr/local/Cellar/sdl2/2.0.7: 86 files, 3.9MB
==> Installing mrboom dependency: libogg
==> Downloading https://homebrew.bintray.com/bottles/libogg-1.3.3.sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/fe/fedf2c7b4aa2c5051851f47a2131a4f1802791ed3c948446442277ecbabcf32c?__gda__=exp=1511795931~hmac=9a3966b415ef6d90208cd68a7c7461ab95267a25
######################################################################## 100.0%
==> Pouring libogg-1.3.3.sierra.bottle.tar.gz
🍺 /usr/local/Cellar/libogg/1.3.3: 97 files, 460.5KB
==> Installing mrboom dependency: sdl2_mixer
==> Downloading https://homebrew.bintray.com/bottles/sdl2_mixer-2.0.2_1.sierra.bottle.tar.gz
######################################################################## 100.0%
==> Pouring sdl2_mixer-2.0.2_1.sierra.bottle.tar.gz
🍺 /usr/local/Cellar/sdl2_mixer/2.0.2_1: 10 files, 298.6KB
==> Installing mrboom
==> Downloading https://homebrew.bintray.com/bottles/mrboom-3.9.sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/be/be0b87327561ee9744c20f82ed7a75e0e009e19772018c1930a195ac8b291fc7?__gda__=exp=1511795936~hmac=f39ad494ff3354267b822476b6b58eb0d1e81bc9
######################################################################## 100.0%
==> Pouring mrboom-3.9.sierra.bottle.tar.gz
Warning: mrboom dependency libmodplug was built with a different C++ standard
library (libstdc++ from clang). This may cause problems at runtime.
🍺 /usr/local/Cellar/mrboom/3.9: 6 files, 5.6MB
$ man mrboom
$ mrboom -h
Usage: mrboom [options]
Options:
-f , --fx FX volume: from 0 to 10
-h, --help Show summary of options
-l , --level Start in level 0:Candy 1:Pinguine 2:Pink 3:Jungle 4:Board 5:Soccer 6:Sky 7:Aliens
-m, --nomonster No monster mode
-s, --sex Sex team mode
-c, --color Color team mode
-n, --noautofire No autofire for bomb drop
-z, --nomusic No music
-v, --version Display version
$ ./miniconda3
$ mrboom
Mix_LoadMUS("deadfeelings.XM"): Unrecognized audio format: please check SDL2_mixer is compiled --with-libmodplug
Error in init.

frranck · 2017-11-27T18:29:30Z

Could be because of the -disable-music-mod-modplug-shared ?

ilovezfs · 2017-11-27T20:01:24Z

@frranck #21105

ilovezfs · 2017-11-27T20:08:12Z

@frranck it should now be fixed if you brew update and brew upgrade

ilovezfs added the new formula PR adds a new formula to Homebrew/homebrew-core label Nov 18, 2017

frranck mentioned this pull request Nov 25, 2017

sdl2_mixer: make libmodplug required instead of optional #21036

Closed

4 tasks

ilovezfs suggested changes Nov 26, 2017

View reviewed changes