[MacPorts] #47192: update: VLC 2.2.0

MacPorts noreply at macports.org
Fri Apr 10 01:01:02 PDT 2015

#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          |

Comment (by rjvbertin@…):

 Replying to [comment:3 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

 > 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

 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". :)

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

More information about the macports-tickets mailing list