#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