update: VLC 2.2.0

[RJVB (René Bertin)]

Attached is a port directory for port:VLC@2.2.0, with a number of small improvements notably how VLC finds its config files and plugins. These are not very important for the standard app bundle built by the main port (which found its files through a series of symlinks), but without them 3rd party applications using libVLC didn't work because they failed to find required files.

It's thus also become possible to provide a libVLC subport that provides only the VLC libraries, and this update also provides that subport.

I am actually not sure MacPorts shouldn't keep only port:libVLC (a requirement for port:phonon-backend-vlc which I hope to submit in a very near future).

[RJVB (René Bertin)]

all patchfiles bundled for convenience

[ctreleaven (Craig Treleaven)]

Cc Me!

[rmerpes]

Cc Me!

[ryandesign (Ryan Carsten Schmidt)]

You've made a lot of changes and obviously spend a lot of time on this, so thank you. I see a few problems though.

You've removed the # $Id$ line which we want to keep, and have added nonstandard and in some cases redundant items to the modeline. I would stick with the standard modeline. If you feel the standard modeline is insufficient, you might discuss that on the macports-devel mailing list so that we might consider improving the standard modeline.

You patch does not apply. The # $Id$ line you removed says your changes are based on r126549 from October of 2014, but the port has been updated several times since then. For example, a qt4 variant has already been added.

I don't believe you've declared the port conflicts correctly. In the libVLC subport you've declared correct conflicts on VLC, VLC-devel and libVLC-devel, and in the next block, which takes effect for VLC, you've declared conflicts on libVLC, libVLC-devel and VLC-devel, but none of the preceding actually gets used, because a few lines later you overwrite the conflicts line and set it to just VLC, so now VLC conflicts with itself.

I'm not very comfortable with VLC and libVLC conflicting with each other. If we remove VLC and keep just libVLC, as you suggested, then it's not a problem, but if we keep both, then it sounds like it would make more sense for VLC to depend on libVLC (and for VLC to not install the files that libVLC installs), rather than conflicting with it.

It makes sense to iron out these problems in the VLC port first, but once we're happy with the changes, we would want to make the same changes to the VLC-devel port; -devel and non-devel ports should be kept as similar as reasonably possible.

You've added code to enable hfsCompression during extraction. I think that's something we should do in MacPorts base, not in individual ports.

There remains a backslash at the end of your depends_lib, and at the end of the configure.args in the quartz variant, which should be removed.

You've added qt4 and qt5 variants, but you've marked the qt4 variant as conflicting with the qt5 port, and the qt5 variant as conflicting with the qt4 port. There aren't any ports named qt4 or qt5, and declaring port conflicts inside a variant doesn't work anyway. You probably meant to mark the qt4 and qt5 variants as conflicting with one another. To do that, the conflicts keyword goes in the variant declaration, not on a line of its own, for example

    variant qt4 conflicts qt5 description {Build using Qt4 UI. This will use qt4-mac. Experimental and probably dysfunctional} {

You've disabled FreeRDP support on Snow Leopard, with a comment that FreeRDP does not build on Snow Leopard. Is there a ticket for that? FreeRDP does not build on any system right now, because FreeRDP is not compatible with cmake 3.1 or later (#47389); perhaps that's what you were seeing. Perhaps this would be a good reason not to enable FreeRDP support yet.

You've rearranged some existing lines which use file copy and file delete -force. We may as well clean up those lines: file copy can be replaced with copy, and file delete -force can be replaced with delete. file rename can be changed to move.

You install a file from the files directory which contains the hardcoded path /Applications/MacPorts, then check in the portfile whether ${applications_dir} equals /Applications/MacPorts, and if not, use reinplace to fix it. This works, but is not the way we usually do it. Instead, when there is a value that could vary, we put a placeholder in the file. In this case, the placeholder could be @APPLICATIONS_DIR@. Then in the portfile you would use reinplace to replace that placeholder with the variable, and would not need to conditionalize it.

You've added a post-activate ui_msg if the qt4 or qt5 variants are set. This may be superfluous since the same message is already in the variant descriptions, but if you want to keep it, we typically want to use the notes command instead of manually running ui_msg at post-activation time. In this case, appending to any existing notes, by using notes-append, would probably be good.

[RJVB (René Bertin)]

Replying to ryandesign@…:

You've made a lot of changes and obviously spend a lot of time on this, so thank you. I see a few problems though.

Define few? :)

Some quick reactions before I address matters when I have a moment for that:

You've removed the # $Id$ line which we want to keep, and have added nonstandard and in some cases redundant items to the modeline. I would stick with the standard modeline. If you

I was under the impression the the Id line was added automatically during a commit, and thought it'd help avoid confusion. Should have realised it also helps in comparing versions...

I don't think I changed the standard modeline. I added one for a different editor, one I happen to be using (in fact I use KDevelop). I think it can be moved to EOF if that's less invasive, I'll want to keep it as long as my name is in the maintainer list. I don't think it'd be possible (nor a good idea) to merge the 2 modelines.

BTW, the vi modeline somehow interferes with my own vim settings, causing files to be marked "modified" as soon as I open them in the editor. I've asked about this on the ML but never got an answer, so I'm re-raising the issue here. It's rather annoying.

You patch does not apply. The # $Id$ line you removed says your changes are based on r126549 from October of 2014, but the port has been updated several times since then. For example, a qt4 variant has already been added.

The Portfile.diff patch? Evidently that one applied when I uploaded my attachments, and parallel changes to the reference are a reason I don't like to upload only diffs.

I don't believe you've declared the port conflicts correctly. In the libVLC subport you've declared correct conflicts on VLC, VLC-devel and libVLC-devel, and in the next block, which takes effect for VLC, you've declared conflicts on libVLC, libVLC-devel and VLC-devel, but none of the preceding actually gets used, because a few lines later you overwrite the conflicts line and set it to just VLC, so now VLC conflicts with itself.

That'd be an oversight...

I'm not very comfortable with VLC and libVLC conflicting with each other. If we remove VLC and keep just libVLC, as you suggested, then it's not a problem, but if we keep both, then it sounds like it would make more sense for VLC to depend on libVLC (and for VLC to not install the files that libVLC installs), rather than conflicting with it.

That would be the other approach. The reason I didn't follow it is that, as you probably saw, there is no way to build just libVLC, and just the player. Things are conceived to build the whole thing, and for port:libVLC I simply throw away the unused things in the post-destroot. All of which means that if someone wants to install port:VLC and for some reason has to build from source, s/he will find it takes about twice as long if we make VLC depend on libVLC.

It makes sense to iron out these problems in the VLC port first, but once we're happy with the changes, we would want to make the same changes to the VLC-devel port; -devel and non-devel ports should be kept as similar as reasonably possible.

I can't recall nor check right now if I uploaded my own VLC-devel port, which now is a fork of this port and builds VLC 3 from git.

You've added code to enable hfsCompression during extraction. I think that's something we should do in MacPorts base, not in individual ports.

I agree. And I'll promise to remove the block as soon as it becomes redundant, but in the meantime there's no harm in keeping it, right?

You've disabled FreeRDP support on Snow Leopard, with a comment that FreeRDP does not build on Snow Leopard. Is there a ticket for that? FreeRDP does not build on any system right now, because FreeRDP is not compatible with cmake 3.1 or later (#47389); perhaps that's what you were seeing. Perhaps this would be a good reason not to enable FreeRDP support yet.

I didn't file a ticket. I apparently tried to build libVLC on my 10.6 VM but if so that was just to satisfy a dependency so I didn't spend more time on it than strictly necessary. It's perfectly possible that I ran into the cmake issue (I built FreeRDP on 10.9 using cmake 3.0.2). FreeRDP support is probably rather ... esoteric in libVLC anyway so yeah, I'll just disable it.

You've rearranged some existing lines which use file copy and file delete -force. We may as well clean up those lines: file copy can be replaced with copy, and file delete -force can be replaced with delete. file rename can be changed to move.

The guide isn't explicit on this: does delete have an implicit -force?

You install a file from the files directory which contains the hardcoded path /Applications/MacPorts, then check in the portfile whether ${applications_dir} equals /Applications/MacPorts, and if not, use reinplace to fix it. This works, but is not the way we usually do it. Instead, when there is a value that could vary, we put a placeholder in the file. In this case, the placeholder could be @APPLICATIONS_DIR@. Then in the portfile you would use reinplace to replace that placeholder with the variable, and would not need to conditionalize it.

I'm not really sure why I'm conditionalising it in the first place; reinplace is a noop the 2nd time it's called, no? I know the official way is to use a placeholder, but the pragmatist I am sees just 2 perfectly equivalent replacement patterns where one can in fact be tested "as is". :)

[RJVB (René Bertin)]

I refreshed the attached diff, and took into account most of the critiques above. For now I've kept the conflicting VLC and libVLC subports. I haven't checked the new Portfile other than with port lint, btw.

I've also updated my git repo so that it shows the diffs against the current release version.

[larryv (Lawrence Velázquez)]

Replying to rjvbertin@…:

I was under the impression the the Id line was added automatically during a commit, and thought it'd help avoid confusion. Should have realised it also helps in comparing versions...

The “Id” keyword anchor is not added automatically, but it is replaced automatically by the svn client if configured properly.

​http://svnbook.red-bean.com/nightly/en/svn.advanced.props.special.keywords.html

I added one for a different editor, one I happen to be using (in fact I use KDevelop). I think it can be moved to EOF if that's less invasive, I'll want to keep it as long as my name is in the maintainer list.

Moving it to the end should be fine. Modeline use is up to the maintainer.

All of which means that if someone wants to install port:VLC and for some reason has to build from source, s/he will find it takes about twice as long if we make VLC depend on libVLC.

Is libVLC supposed to be something that other software can use? If VLC and libVLC conflict, any software that wants to use libVLC would have to accept both ports as a dependency, which makes things more complicated.

How long does VLC take to compile? I expect most users would receive a binary archive, so compile time should not be a major concern.

You've added code to enable hfsCompression during extraction. I think that's something we should do in MacPorts base, not in individual ports.

I agree. And I'll promise to remove the block as soon as it becomes redundant, but in the meantime there's no harm in keeping it, right?

Ports should only override base functionality if they really have to, and HFS compression is not required to extract VLC. Ryan is right; we should do this in base or not at all. The HFS compression support should not be committed.

You've rearranged some existing lines which use file copy and file delete -force. We may as well clean up those lines: `file copy can be replaced with copy, and file delete -force` can be replaced with delete. file rename can be changed to move.

The guide isn't explicit on this: does delete have an implicit -force?

Yes: source:tags/release_2_3_3/base/src/port1.0/portutil.tcl#L1068​

[RJVB (René Bertin)]

Replying to larryv@…:

All of which means that if someone wants to install port:VLC and for some reason has to build from source, s/he will find it takes about twice as long if we make VLC depend on libVLC.

Is libVLC supposed to be something that other software can use?

Evidently, there's little point in providing a port for a library that isn't supposed to be used, eh? :)

If VLC and libVLC conflict, any software that wants to use libVLC would have to accept both ports as a dependency, which makes things more complicated.

True, though that's what path: style dependencies are for.

How long does VLC take to compile? I expect most users would receive a binary archive, so compile time should not be a major concern.

With the autoreconfig, I'd say between 15 and 30 minutes. Long enough not to want to do the same thing twice in a row.

%> port-check-distributable.tcl -v VLC
"VLC" is not distributable because its license "GPL" conflicts with license "OpenSSL" of dependency "openssl"

If there's a way to build port:libVLC, "cache" the stuff that port doesn't use and then install that for port:VLC the issue goes away. Alternatively, one could think of providing only port:libVLC, with a +player variant for those who want the full monty.

Ports should only override base functionality if they really have to, and HFS compression is not required to extract VLC. Ryan is right; we should do this in base or not at all. The HFS compression support should not be committed.

Sigh. We're talking about *extending* base functionality with a feature that's completely transparent if you disregard the 1 extra (and highly useful) extract dependency and the reduced footprint. It's not very important for VLC, but for ports like Qt5-mac and Calligra I'd argue it's required for anyone running off an SSD.

Of course this is something that can be implemented in base in a matter of seconds, esp. now that there's a tested example...

[dbevans (David B. Evans)]

Looks like things are at a stand still here.

In the interest of moving things along and getting this port updated, without taking a stand one way or the other, I would suggest decoupling the various issues and submitting a modified version of the patch that just brings the port up to date in a vanilla fashion without the HFS compression and the subport.

This would make the update easier to understand and you can address the additional issues in subsequent updates as you gain some consensus on their utility. For instance, this would provide a baseline against which possible performance enhancements could be measured.

I note that VLC has now been updated to version 2.2.1, a bug fix release that addresses some regressions in 2.2.0 relative to the 2.1 release branch.

[RJVB (René Bertin)]

portfile without the libVLC subport nor hfs compression

[RJVB (René Bertin)]

Replying to devans@…:

Looks like things are at a stand still here.

And they were a bit longer, as I was on a trip.

In the interest of moving things along and getting this port updated, without taking a stand one way or the other, I would suggest decoupling the various issues and submitting a modified version of the patch that just brings the port up to date in a vanilla fashion without the HFS compression and the subport.

Done, as well as the update to 2.2.1 . And maintaining a version that does have the libVLC subport, in hope that that's the version that will be accepted eventually. I continue to think that there's more justification for a port:libVLC than for port:VLC even if for the moment I know of only 1 3rd-party project that actually uses the library.

gain some consensus on their utility. For instance, this would provide a baseline against which possible performance enhancements could be measured.

Where do those performance enhancements come from? In building the port?

[mdeaudelin (Mathieu Deaudelin-Lemay)]

Cc Me!

[mojca (Mojca Miklavec)]

Cc Me!

[mojca (Mojca Miklavec)]

I would assume that #49051 supersedes this ticket, so I'm closing this as fixed (r146098). If any detail was forgotten, please open a new ticket.