mkvtoolnix: upgrade to version 6.8.0

[dgonyier (Dwaine Gonyier)]

​http://www.bunkus.org/videotools/mkvtoolnix/index.html

03 Mar 2014: Released v6.8.0

[dgonyier (Dwaine Gonyier)]

Cc Me!

[mojca (Mojca Miklavec)]

The port never worked for me, but I can commit a patch. (As a reporter you get emails without being cc-ed.)

[mojca (Mojca Miklavec)]

Unrelated, but probably worth fixing – the build fails for me on a slightly "cleaner" machine:

--->  Building mkvtoolnix
rake aborted!
Ruby 1.9.x or newer is required for building

I had to fix rake.d/vendor/drake-0.9.2.0.3.1/bin/drake to launch the right ruby version.

[mojca (Mojca Miklavec)]

I will be offline for a while. I would commit the patch, but I suspect that building will fail on buildbots where default ruby < 1.9 without an additional patch for drake. So I suggest that someone fixes this ruby issue and commits it together with the attached patch.

[cooljeanius (Eric Gallager)]

mkvtoolnix already has a dependency on the ruby port, so I doubt that what the system ruby is on the buildbots matters... If ruby 1.9 is needed, the dependency on the ruby port should be switched to the ruby19 port. Also, there is a rb19-rake port that could probably be used instead of the vendored-in drake script... (idk for sure though)

[cooljeanius (Eric Gallager)]

Cc Me!

[mojca (Mojca Miklavec)]

Replying to egall@…:

mkvtoolnix already has a dependency on the ruby port, so I doubt that what the system ruby is on the buildbots matters...

It does because the file is using #!/usr/bin/env ruby.

[cooljeanius (Eric Gallager)]

Replying to mojca@…:

Replying to egall@…:

mkvtoolnix already has a dependency on the ruby port, so I doubt that what the system ruby is on the buildbots matters...

It does because the file is using #!/usr/bin/env ruby.

I thought that the MacPorts ruby would appear first in the path that /usr/bin/env searches though? Actually I suppose that would only be the case if the user made symlinks with the ruby_select port though... I guess the current dependency on the ruby port must just be a leftover from before the ruby_select port was introduced then...

[mojca (Mojca Miklavec)]

I didn't check, but it is possible that previous version worked with ruby 1.8 and nobody noticed any problem because OS X ships with ruby anyway.

For me the ruby port installs /opt/local/bin/ruby1.8, so /usr/bin/env ruby would not work both because the executable has the wrong name and also because it's at the wrong version.

[dgonyier (Dwaine Gonyier)]

For what it is worth. I was able to manually work around the ruby 1.8 issue in my manual, local build of mkvtoolnix by

# port install ruby19 ruby_select
# port select --set ruby ruby19

then ran configure... followed by ./drake in the git clone tree for mkvtoolnix 6.8.0 I pulled.

The drake tool bundled with mkvtoolnix then appears to have automagically grabbed /opt/local/bin/ruby-1.9 as the ruby version used.

[mopihopi]

Cc Me!

[mojca (Mojca Miklavec)]

I added a patch for 7.0.0 which is the same as 6.8.0, just a different version. (Ruby issue not solved yet though.)

[pixilla (Bradley Giesbrecht)]

mkvtoolnix 7.0.0 mp ruby

[pixilla (Bradley Giesbrecht)]

My patch is for 7.0.0 and mp ruby21.

App still crashes.

[mojca (Mojca Miklavec)]

So many patch files seem like an overkill. Why not merging all the ruby patches into a single patch file? It will be *a lot* easier to maintain.

Also: any special reason for removing the patch for clang?

[pixilla (Bradley Giesbrecht)]

Replying to mojca@…:

So many patch files seem like an overkill. Why not merging all the ruby patches into a single patch file? It will be *a lot* easier to maintain.

I use a script to generate patch files that conform to MP standards of patch-path.diff.

Also: any special reason for removing the patch for clang?

It compiled without the patch.

[mojca (Mojca Miklavec)]

Replying to pixilla@…:

Replying to mojca@…:

So many patch files seem like an overkill. Why not merging all the ruby patches into a single patch file? It will be *a lot* easier to maintain.

I use a script to generate patch files that conform to MP standards of patch-path.diff.

Can we nonetheless make an exception here?

[pixilla (Bradley Giesbrecht)]

Replying to mojca@…:

Replying to pixilla@…:

Replying to mojca@…:

So many patch files seem like an overkill. Why not merging all the ruby patches into a single patch file? It will be *a lot* easier to maintain.

I use a script to generate patch files that conform to MP standards of patch-path.diff.

Can we nonetheless make an exception here?

Sure, I thought someone might be interested in testing with a newer ruby so I shared my quick hack of a patch. You can concat the patches into a single file if you like.

[mojca (Mojca Miklavec)]

I would split the dependency on ruby into depends_run and depends_build (when upgrading ruby, there is no need to revbump this port, I guess)? Other than that (and the splitting) it looks ok. I didn't test, but I don't have much to test as long as it keeps crashing.

If you are willing to merge the files for ruby into a single file, I would say "just go with it".

When you say that it compiled ok without a patch: are you saying that it compiled just fine with gcc or did you actually compile it with clang? (Personally I would leave that clang patch in the repository, even if you remove it from the Portfile, at least for a while.)

[mopihopi]

I tried patch-mkvtoolnix-7.0.0.diff on Snow Leopard. It builds (using macports-gcc-4.7, as chosen by the Portfile), but mkvextract crashes. Apparently this is a longstanding known issue on all pre-Mavericks MacPorts builds of recent mkvtoolnix, caused by a mismatched C++ runtime (#34806/#40231).

[mojca (Mojca Miklavec)]

A while ago someone also submitted a patch for private boost and icu. I don't know why these solutions have never been considered/implemented.

[mojca (Mojca Miklavec)]

I went ahead and committed r120972 (mostly Bradley's patches, slightly modified).

Replying to pixilla@…:

I use a script to generate patch files that conform to MP standards of patch-path.diff.

So just a provocation then: what's your excuse for using (long) path prefixes?

--- /opt/local/var/macports/sources/svn.macports.org/trunk/dports/multimedia/mkvtoolnix/Portfile	(revision 120935)
+++ /opt/local/var/macports/sources/svn.macports.org/trunk/dports/multimedia/mkvtoolnix/Portfile	(working copy)

--- a/Rakefile	2014-06-11 19:57:12.000000000 -0700
+++ b/Rakefile	2014-06-11 19:58:30.000000000 -0700

[mojca (Mojca Miklavec)]

Replying to pixilla@…:

Replying to mojca@…:

Also: any special reason for removing the patch for clang?

It compiled without the patch.

The ​buildbot seems to disagree:

In file included from src/common/command_line.cpp:14:
In file included from src/common/common_pch.h:4:
In file included from src/common/common.h:29:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/vector:1900:21: fatal error: object of type 'boost::filter_iterator<<lambda at src/common/command_line.cpp:301:74>, boost::transform_iterator<boost::range_detail::select_first<std::__1::map<std::__1::basic_string<char>, std::__1::basic_string<char>, std::__1::less<std::__1::basic_string<char> >, std::__1::allocator<std::__1::pair<const std::__1::basic_string<char>, std::__1::basic_string<char> > > > >, std::__1::__map_const_iterator<std::__1::__tree_const_iterator<std::__1::__value_type<std::__1::basic_string<char>, std::__1::basic_string<char> >, std::__1::__tree_node<std::__1::__value_type<std::__1::basic_string<char>, std::__1::basic_string<char> >, void *> *, long> >, boost::use_default, boost::use_default> >' cannot be assigned because its copy assignment operator is implicitly deleted
                __m = __first;
                    ^

[mojca (Mojca Miklavec)]

I added the clang patches back in r120975.

(Purely theoretically this could warrant a new revbump or one could only apply the patch conditionally, but I really don't think that it makes any real difference with or without that patch.)

[pixilla (Bradley Giesbrecht)]

Replying to mojca@…:

I went ahead and committed r120972 (mostly Bradley's patches, slightly modified).

Replying to pixilla@…:

I use a script to generate patch files that conform to MP standards of patch-path.diff.

So just a provocation then: what's your excuse for using (long) path prefixes?

--- /opt/local/var/macports/sources/svn.macports.org/trunk/dports/multimedia/mkvtoolnix/Portfile	(revision 120935)
+++ /opt/local/var/macports/sources/svn.macports.org/trunk/dports/multimedia/mkvtoolnix/Portfile	(working copy)

--- a/Rakefile	2014-06-11 19:57:12.000000000 -0700
+++ b/Rakefile	2014-06-11 19:58:30.000000000 -0700

That is the output of "svn diff".

[pixilla (Bradley Giesbrecht)]

Replying to mojca@…:

Replying to pixilla@…:

Replying to mojca@…:

Also: any special reason for removing the patch for clang?

It compiled without the patch.

The ​buildbot seems to disagree:

In file included from src/common/command_line.cpp:14:
In file included from src/common/common_pch.h:4:
In file included from src/common/common.h:29:
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/vector:1900:21: fatal error: object of type 'boost::filter_iterator<<lambda at src/common/command_line.cpp:301:74>, boost::transform_iterator<boost::range_detail::select_first<std::__1::map<std::__1::basic_string<char>, std::__1::basic_string<char>, std::__1::less<std::__1::basic_string<char> >, std::__1::allocator<std::__1::pair<const std::__1::basic_string<char>, std::__1::basic_string<char> > > > >, std::__1::__map_const_iterator<std::__1::__tree_const_iterator<std::__1::__value_type<std::__1::basic_string<char>, std::__1::basic_string<char> >, std::__1::__tree_node<std::__1::__value_type<std::__1::basic_string<char>, std::__1::basic_string<char> >, void *> *, long> >, boost::use_default, boost::use_default> >' cannot be assigned because its copy assignment operator is implicitly deleted
                __m = __first;
                    ^

I failed to noticed that on Mountain Lion the port was using gcc. Some good news, mkvtoolnix does not crash on Mavericks.

[mojca (Mojca Miklavec)]

Please don't take this as a critique, just as a teaser:

Whatever you do, the result is not following the MP rules. So saying that you split the files just in order to be compliant, doesn't sound convincing.

How on earth do you get the whole /opt/local/var/macports/sources/svn.macports.org/trunk/dports/multimedia/mkvtoolnix with svn diff? I have a local sparse checkout in my home dir, I go to that folder and svn diff prints just Portfile.

For creating patches I usually use

# cd <port dir>
sudo port extract
# cd <port work>
sudo git init .
sudo git add -A
# edit
git diff

and then search-and-replace "--- a/" with "--- " to get rid of the top level a/ and b/.

Can you please test whether +wxwidgets does anything useful?

[pixilla (Bradley Giesbrecht)]

Replying to mojca@…:

Please don't take this as a critique, just as a teaser:

Whatever you do, the result is not following the MP rules. So saying that you split the files just in order to be compliant, doesn't sound convincing.

The {a,b} patches in files/ is a fairly common method. The full path in the Portfile patch is an anomaly, see below for my best guess on that one.

How on earth do you get the whole /opt/local/var/macports/sources/svn.macports.org/trunk/dports/multimedia/mkvtoolnix with svn diff? I have a local sparse checkout in my home dir, I go to that folder and svn diff prints just Portfile.

I don't know, I may have been in a hurry and did something like "svn diff $(port dir mkvtoolnix)".

For creating patches I usually use

# cd <port dir>
sudo port extract
# cd <port work>
sudo git init .
sudo git add -A
# edit
git diff

and then search-and-replace "--- a/" with "--- " to get rid of the top level a/ and b/.

I copy the sources to {a,b} so I don't loose changes if I forget to add "port -k -o" after a Portfile edit.

Can you please test whether +wxwidgets does anything useful?

I'll try again, the gui did not work for me a few days ago.

[pixilla (Bradley Giesbrecht)]

I have mkvextract working on Mountain Lion after installing "boost +gcc49" and force upgrading flac, gettext and pcre with macports-gcc-4.9. Previously flac, gettext and pcre linked with:

/usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 56.0.0)

and now:

/opt/local/lib/libgcc/libstdc++.6.dylib (compatibility version 7.0.0, current version 7.20.0)

[mojca (Mojca Miklavec)]

I would suggest to further discuss this in #34806 (which was closed, but I reopened it).

We either need to compile all the dependencies with gcc4X or against libc++. The question is whether we could perhaps introduce some standard location where all such "duplicated ports" could be installed. (Rather than having mkvtoolnix.boost it would make sense to have something like boost.libc or boost.gcc.)