#51411 closed enhancement (fixed)
mlt: qt5 subport
| Reported by: | RJVB (René Bertin) | Owned by: | mkae (Marko Käning) |
|---|---|---|---|
| Priority: | Normal | Milestone: | |
| Component: | ports | Version: | |
| Keywords: | haspatch | Cc: | dbevans (David B. Evans), ddennedy (Dan Dennedy) |
| Port: | mlt |
Description
This patch adds the Qt5 subport that got lost in a previous ticket.
I called the subport qt5-mlt earlier, but as you may have seen on macports-devel I now wonder if we shouldn't use a suffix rather than a prefix for ports depending on Qt5 (= mlt-qt5) and reserve the prefix to ports providing Qt5 (or its components).
As a bonus I bumped the provided version to 6.2.0 .
Attachments (5)
Change History (23)
Changed 8 years ago by RJVB (René Bertin)
comment:1 Changed 8 years ago by mf2k (Frank Schima)
| Cc: | dan@… removed |
|---|---|
| Keywords: | haspatch added; has_patch removed |
| Owner: | changed from macports-tickets@… to dan@… |
| Type: | submission → enhancement |
| Version: | 2.3.4 |
comment:2 Changed 8 years ago by ddennedy (Dan Dennedy)
comment:3 Changed 8 years ago by RJVB (René Bertin)
Great. I agree about the comment...
I suggest we wait a little bit to see if there's any feedback on the ML. I have a vague memory that there was a preference for using a prefix a while back, I think in line with the python and perl ports.
comment:5 Changed 8 years ago by RJVB (René Bertin)
Updating to 6.4.1 and still introducing the still missing Qt5 subport (now called mlt-qt5 to align with Qt5 (sub)ports).
I'd like to request a maintainer timeout on this one <<<
Changed 8 years ago by RJVB (René Bertin)
| Attachment: | mlt.2.diff added |
|---|
update to 6.4.1, mlt-qt5 subport
Changed 8 years ago by RJVB (René Bertin)
| Attachment: | mlt.4.diff added |
|---|
mlt-qt5 depends on a Qt5 component that's not installed by default by port:qt5 and port:qt55
comment:6 Changed 8 years ago by mkae (Marko Käning)
| Cc: | dan@… added |
|---|---|
| Owner: | changed from dan@… to mkae |
| Status: | new → accepted |
I am picking this up, since I hope to be forced to have to use this with Qt5. If you're not happy with this, simply revert this! I'll file a PR if needed.
comment:7 Changed 8 years ago by mkae (Marko Käning)
Dan, would you be fine with co-maintainership and also allowing openmaintainer for this port?
comment:8 follow-up: 11 Changed 8 years ago by ddennedy (Dan Dennedy)
Yes, I agree with this. I appreciate the help. Especially, I doubt I could have come up with the changes in mlt.4.diff - much appreciated!
comment:9 Changed 8 years ago by mkae (Marko Käning)
| Status: | accepted → assigned |
|---|
OK, Dan.
Will take this on as soon as we hit kf5-kdenlive, which is hopefully going to happen soonish. :-)
comment:10 Changed 8 years ago by mkae (Marko Käning)
| Cc: | mkae removed |
|---|
comment:11 Changed 8 years ago by RJVB (René Bertin)
Replying to ddennedy:
Yes, I agree with this. I appreciate the help. Especially, I doubt I could have come up with the changes in mlt.4.diff - much appreciated!
You're very welcome and I appreciate the quick reaction! It would help us if you could test the new version on your end (even if you don't have Qt5 installed), and push it if it works for you. I don't think there's much point in waiting for the introduction of kf5-kdenlive, esp. not for a complex port like that one. Rather, we should allow users to start weaning off Qt4.
I see I put myself first in the maintainer list which isn't very nice in retrospect and probably just a bit of force of habit. You can evidently keep principle maintainership...
comment:12 follow-up: 14 Changed 8 years ago by ddennedy (Dan Dennedy)
I am trying to test this, but when I run portindex with multimedia/mlt/Portfile patched with mlt.4.diff, I get
Failed to parse file multimedia/mlt/Portfile with subport 'mlt-qt5': can't read "os.major": no such variable
I see os.major documented in the Guide, but I do not know why it is failing. I did run port selfupdate prior to starting this, and it succeeded. I am running 10.11 El Capitan
Macbooks-MacBook-Pro:ports ddennedy$ port version Version: 2.3.5
Any thoughts?
comment:13 Changed 8 years ago by ddennedy (Dan Dennedy)
Despite the above issue, the patched local mlt port (not mlt-qt5) upgraded fine, melt --version shows the new version 6.4.1 and some basic tests work.
comment:14 Changed 8 years ago by RJVB (René Bertin)
Replying to ddennedy:
I am trying to test this,
Thanks, I appreciate it! Good news port:mlt still works (not that there was any reason why it wouldn't ... :) )
Failed to parse file multimedia/mlt/Portfile with subport 'mlt-qt5': can't read "os.major": no such variable
...
Any thoughts?
Yes, my bad, I missed a global declaration for os.major: I'm quite certain your error will disappear when you add this declaration:
proc qt5.depends_component {first args} {
+ global os.major
Let me know if this works and you want me to upload an updated patch.
R.
Changed 8 years ago by ddennedy (Dan Dennedy)
| Attachment: | mlt-Portfile-6.4.1.diff added |
|---|
update to 6.4.1 and add mlt-qt5 subport
comment:15 Changed 8 years ago by ddennedy (Dan Dennedy)
Yes, the global declaration fixed it. I do not have push permission; so, I uploaded a new patch file that includes everything: mlt-Portfile-6.4.1.diff. I also successfully built mlt-qt5 and did some basic testing of it. I approve this change to be committed.
comment:16 Changed 8 years ago by RJVB (René Bertin)
Perfect, thanks!
David, if you're seeing this, maybe you can commit the patch like you did the 2 previous changes (if Marko doesn't beat you to it)?
comment:17 Changed 8 years ago by mkae (Marko Käning)
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
I approve, but I also agree that "qt5" works better as a suffix than a prefix. Also, I am not sure the comment
# NB: qt-includedir can and should be set to ${qt_includes_dir}, # not ${qt_includes_dir}/Qt !!makes much sense after "/Qt" has been removed (outside of viewing revision log).