[MacPorts] #47192: update: VLC 2.2.0

[MacPorts]

#47192: update: VLC 2.2.0
--------------------------+--------------------------------
  Reporter:  rjvbertin@…  |      Owner:  macports-tickets@…
      Type:  update       |     Status:  new
  Priority:  Normal       |  Milestone:
 Component:  ports        |    Version:
Resolution:               |   Keywords:  haspatch
      Port:  VLC          |
--------------------------+--------------------------------
Changes (by ryandesign@…):

 * cc: ryandesign@… (added)


Comment:

 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.

-- 
Ticket URL: <https://trac.macports.org/ticket/47192#comment:3>
MacPorts <https://www.macports.org/>
Ports system for OS X