search

RJVB / macstrop

RJVB's repository of alternative macports, with ports missing from or overriding those in the standard collection, including a set of KF5 ports.
20 stars 9 forks source link

VLC-test: properly build VLC.app #17

Closed michaellass closed 5 years ago

michaellass commented 5 years ago

This change adresses three issues leading to a failure during built of VLC.app:

  1. The make target in destroot needs to be VLC.app.
  2. The VLC.app target does not obey the DESTROOT variable. To work around this, we need to set destroot.destdir.
  3. The tarball is missing the folder share/hrtfs. It is present in the github tree though, so we can fetch is from there.

Fixes #16

Regarding no. 3: This issue was mentioned at https://forum.videolan.org/viewtopic.php?t=141470#p464626 and should have been fixed. Probably we need to file a bugreport upstream about that. For the time being, I don't know how important this file is and if it's worth the effort of pulling it from github. We may also try to delete the corresponding line from the Makefile.

I guess the code for copying the file to its correct place would be better suited for a post-extract section. Also we may use xinstall. So this PR is probably not ready to be merged. However, I wanted to get some feedback before continuing.

RJVB commented 5 years ago

Thanks for this!

Quick reply, I'll try to look at your changes tomorrow: Looks good esp. since things build and work for you now.

Did you look at what that missing share/hrtfs folder contains, if it serves a purpose? Apparently the files are not included in the official build process, so we could probably just as well leave them out. Searching for 'hrtfs' in the source directory turns up a few hits that suggest it might be a Win32 only thing (possibly relating to Head-Related Transfer Functions).

michaellass commented 5 years ago

I had a closer look on this. The file is required for using the spatialaudio audio filter. It creates a virtual 3D sound when using headphones. However, we don't include this plugin in our build anyway, and if we wanted we would need to create a port for the spatialaudio library first. So the file can simply go.

I'll update this PR in a few minutes.

michaellass commented 5 years ago

From my point of view this is now ready. Two non-related things I noticed but won't attack myself:

  1. The port fails to build when using the -t parameter (trace mode). I have no clue what's happening there but it shouldn't be related to this change.
  2. Why does the Portfile contain several platform darwin sections when darwin is the only supported platform anyway? I have not much experience with writing Portfiles so there may be a good reason I just don't see.
RJVB commented 5 years ago
  1. Why does the Portfile contain several platform darwin sections when darwin is the only supported platform anyway? I have not much experience with writing Portfiles so there may be a good reason I just don't see.

Maintainer privilege ;) I have a version of the port for Linux, in a separate tree. Having these platform sections makes it easier to keep the 2 in sync with side-by-side diffs.

You'll find other portfiles (from other authors) that have this kind of section for no apparent reason that outsiders can see.

I won't be addressing the -t issue either; I have no experience with that mode (partly because, IIRC, ports often fail with it).

RJVB commented 5 years ago

Merged in commit #d4fc436

Thanks for figuring this out for me!