Opened 9 years ago
Closed 7 years ago
#50288 closed update (fixed)
UPDATE: gpsd-3.16
Reported by: | fhgwright (Fred Wright) | Owned by: | ryandesign (Ryan Carsten Schmidt) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | haspatch | Cc: | mojca (Mojca Miklavec), g5pw (Aljaž Srebrnič), mp@… |
Port: | gpsd |
Description
This is just a placeholder for now. I'm in the process of testing a 3.16 port, and will upload files when ready.
Attachments (7)
Change History (26)
comment:1 Changed 9 years ago by mojca (Mojca Miklavec)
Cc: | mojca@… added |
---|
Changed 9 years ago by fhgwright (Fred Wright)
Complete Portfile for 3.16_0
Changed 9 years ago by fhgwright (Fred Wright)
Attachment: | Portfile-gpsd.diff added |
---|
Portfile diff relative to 3.14_4
Changed 9 years ago by fhgwright (Fred Wright)
Attachment: | leopard-IPV6_TCLASS.patch added |
---|
Patch for IPv6 on Leopard
Changed 9 years ago by fhgwright (Fred Wright)
Attachment: | patch-SConstruct.diff added |
---|
Patch for SConstruct build script
Changed 9 years ago by fhgwright (Fred Wright)
Attachment: | patch-driver_rtcm2.c.diff added |
---|
Patch for endianness check on 10.5 (and earlier)
Changed 9 years ago by fhgwright (Fred Wright)
Attachment: | gpsd-3.16_0.tgz added |
---|
Complete tarball of gpsd-3.16_0
comment:2 Changed 9 years ago by fhgwright (Fred Wright)
Here are the files for the 3.16 port. I supplied both a diff of the Portfile against the previous published version, and the complete new Portfile, which is more readable (and smaller). The patch files are presented as is, rather than readability-challenged diffs of diffs, and I also included a complete tarball of all the files with correct timestamps (though the repository doesn't seem to be set up to preserve timestamps).
Changes from a file perspective:
The two pc.in patchfiles are obsoleted by upstream fixes, and have been removed.
The two remaining code patchfiles have been updated to match the updated sources, but are functionally unchanged (other than adding a comment line to the rtcm2 patch to clarify why the order matters).
The SConstruct patchfile has been updated both because of substantial upstream changes to SConstruct since 3.14, and to add a couple of tweaks.
The Portfile has changed substantially.
Changes from a functional perspective:
The universal variant has been disabled, since it didn't work (and would probably require significant work on the SConstruct script to implement).
A new "xgps" variant has been added to determine whether the xgps and xgpsspeed X11 programs are included. This is because they both require gobject and gtk, expanding recursively to over 150 (previously unexpressed) dependencies that aren't otherwise needed. This is plumbed via a new SConstruct option.
There are now three Qt-related variants - qt4, qt5, and qt. The first two build against the specified Qt version, and the last is equivalent to whichever qtN variant is appropriate for the OS version. This is qt4 for OSX < 10.8 and qt5 for OSX >= 10.8, corresponding (mostly) to the Qt5 supported OS list. Qt5 doesn't consider 10.11 to be a supported OS, but this is presumably temporary, and isn't reflected in the selection. This is similar to what was suggested in #47739, though with a more aggressive move to Qt5. Since the only Qt module used here is QtNetwork, and since none of the items listed as incompatible changes in QtNetwork appears to be used, it's reasonable to expect that the Qt5 substitution will work without any code changes. However, the name change from QtNetwork to Qt5Network required adding a new option to SConstruct.
There's a workaround for #50347 in the Qt4 setup.
The variants now correctly control the build options and the dependencies, which wasn't entirely the case previously.
The "LIBPATH='.'" arguments in the SConstruct script, which only served to break linking against libdbus-1, have been patched out.
The test phase is now implemented, running all the provided regression tests (with a hack to keep them from being ridiculously slow - see the comment).
Because 3.16 changes all the Python shebang lines to refer to "python2", this now explicitly depends on python2_select, and includes both a note to suggest using it if "python2" isn't found in the command path, and a similar fatal error at the start of the test phase (since the tests require "python2").
The bad permissions on gpsprof (in the 3.16 release tarball) are corrected.
The long description has been expanded and cosmetically cleaned up.
Testing:
Build/installs have been tested for no variants, maximal variants, and the non-default Qt variant, as well as running the regression tests. This was done on a Mac Pro running 10.9.5, a PowerBook running 10.5.8, and x86_64 VMs running all OS versions from 10.5 through 10.11. The Qt5 builds failed on 10.5 and 10.6, due to the Qt5 build itself (intentionally) failing, but everything else worked. Note that Qt wasn't tested beyond testing the builds, since there aren't any Qt tests included.
comment:3 Changed 9 years ago by fhgwright (Fred Wright)
Ping.
BTW, I only belatedly ran "port lint" with the "--nitpick" option, and it complained about the naming of "leopard-IPV6_TCLASS.patch". I'm disinclined to fix this, since:
1) That's a preexisting patch file (even predating 3.14 AFAICT) that I merely updated without renaming it.
2) That fix has now been accepted upstream, so that patch (as well as the rtcm2 patch) will be obsolete in future versions.
comment:4 Changed 9 years ago by mf2k (Frank Schima)
Cc: | ryandesign@… removed |
---|---|
Keywords: | haspatch added |
Owner: | changed from macports-tickets@… to ryandesign@… |
Version: | 2.3.4 |
comment:5 Changed 9 years ago by fhgwright (Fred Wright)
Ping again.
FYI - a summary of the affected files is as follows:
MacPro:macports-trunk fw$ svn status M dports/net/gpsd/Portfile M dports/net/gpsd/files/leopard-IPV6_TCLASS.patch M dports/net/gpsd/files/patch-SConstruct.diff M dports/net/gpsd/files/patch-driver_rtcm2.c.diff D dports/net/gpsd/files/patch-libgps.pc.in.diff D dports/net/gpsd/files/patch-libgpsd.pc.in.diff
If Subversion had a way to generate a "commit file", I'd post it.
comment:6 follow-up: 7 Changed 9 years ago by g5pw (Aljaž Srebrnič)
Cc: | g5pw@… added |
---|
Wow, that's great work! Just a small nitpick: there's no need to explicitly depend on python2_select since python2.7 has a runtime dependency on it. With that corrected I'll happily commit it.
Another thing: if you cd
in the port directory and run svn diff > port.patch
svn should output a patch of everything in one, making applying it easier.
Changed 9 years ago by fhgwright (Fred Wright)
Attachment: | Port-gpsd.patch added |
---|
Complete patchfile for 3.14_4->3.16_0 update
comment:7 Changed 9 years ago by fhgwright (Fred Wright)
Replying to g5pw@…:
Wow, that's great work! Just a small nitpick: there's no need to explicitly depend on python2_select since python2.7 has a runtime dependency on it. With that corrected I'll happily commit it.
I thought about that, but I disagree, since I think it's bad practice to rely on an indirect dependency for something which is a direct requirement.
For example, if I have a C program that uses something in foo.h, it will of course include foo.h. If foo.h needs something from bar.h, it will include bar.h. But if my code also *directly* uses something from bar.h, then it should also include bar.h explicitly, rather than relying on the internal include within foo.h.
In this case, since the Python gpsd
code needs a python2
command, it's appropriate for it to explicitly depend on python2_select
(which is imperfect in that it doesn't guarantee that python2_select
has been *used*, but it's the best we can do with dependencies). And strictly speaking, there's no reason in principle why python27
needs a dependency on python2_select
, since it probably has no python2
commands of its own. That dependency is presumably only there for convenience, given that it's fairly inexpensive, making it particularly dangerous to rely on it.
Once gpsd
's Python code has been updated to be compatible with Python 3, the python2
issue will become moot.
Another thing: if you
cd
in the port directory and runsvn diff > port.patch
svn should output a patch of everything in one, making applying it easier.
I've attached a diff of that form.
comment:8 follow-up: 9 Changed 9 years ago by mojca (Mojca Miklavec)
While what you say about bar.h
is true, depending on python2_select
doesn't guarantee you in any way that the python2
command exists. The package needs to reinplace
the lines calling python2
with ${prefix}/bin/python2.7
. You should not ask users to run port select python2
manually.
comment:9 Changed 9 years ago by fhgwright (Fred Wright)
Replying to mojca@…:
While what you say about
bar.h
is true, depending onpython2_select
doesn't guarantee you in any way that thepython2
command exists.
Hence the (dynamically conditional) port notes
when it doesn't (and the error if one tries to run port test
without python2
).
The package needs to
reinplace
the lines callingpython2
with${prefix}/bin/python2.7
. You should not ask users to runport select python2
manually.
While I agree that that would be more convenient for users that haven't previously used port select python2
, it does have some disadvantages:
1) It requires patching 8 more files that otherwise don't need it. I'm trying to *reduce* the number of MacPorts-specific patches, not increase it. :-)
2) It unnecessarily breaks the ability to use Python 2.6. The only reason I made the port dependency specifically on python27
is that I didn't want to go to the trouble of creating Python version variants (and also that the required scons
port depends specifically on python27
, presumably for a similar reason); the code itself will work just fine if python2
maps to python26
.
3) Any user interested in PEP0394
compliance should run port select python2
, anyway.
comment:10 follow-up: 11 Changed 9 years ago by mojca (Mojca Miklavec)
I do sympathize with this to some extent, but it's against the policy and general practice of most other ports.
Running reinplace
is simple enough and doesn't require writing explicit patches. For example:
fs-traverse f ${worksrcpath} { if {[string match "*.py" "$f"]} { reinplace "s|/usr/bin/env python2|${python_bin}|" $f } }
On top of that, if the user indeed picked python2.6 with port select
and gpsd
depends on py-pygtk
, but the port only installed py27-pygtk
, it will fail to work because the port doesn't guarantee in any way that py26-pygtk
is present. Worse yet, let's assume that user has selected port select python2
, but has another instance of python2
binary that comes before python provided by MacPorts. It would fail just as well because that other python2
won't have the required dependencies.
I would suggest you to raise the question on the macports-dev
mailing list and ask other developers for opinion.
comment:11 Changed 9 years ago by fhgwright (Fred Wright)
Replying to mojca@…:
I do sympathize with this to some extent, but it's against the policy and general practice of most other ports.
It's difficult to quantify "most" without some fairly painstaking analysis, but it's certainly not "all".
Running
reinplace
is simple enough and doesn't require writing explicit patches. For example:fs-traverse f ${worksrcpath} { if {[string match "*.py" "$f"]} { reinplace "s|/usr/bin/env python2|${python_bin}|" $f } }
A patch is still a patch, whether it's by a patchfile or inline in the Portfile. Every patch makes the port harder to maintain by requiring more analysis to move to a new upstream version. As it is (since I've now gotten most of the patches incorporated upstream), there's a fairly good chance that by the time 3.17 is released, it can be completely patch-free, and I'd prefer to keep it that way.
Not to mention all the testing (see above) that I'd have to redo if I changed anything. :-)
On top of that, if the user indeed picked python2.6 with
port select
andgpsd
depends onpy-pygtk
, but the port only installedpy27-pygtk
, it will fail to work because the port doesn't guarantee in any way thatpy26-pygtk
is present. Worse yet, let's assume that user has selectedport select python2
, but has another instance ofpython2
binary that comes before python provided by MacPorts. It would fail just as well because that otherpython2
won't have the required dependencies.
That's no different from what happens with a program that specifies python
and one does a port select python
without having the required dependencies. And putting another python
or python2
ahead of the MacPorts position in the command path breaks the corresponding selector entirely, regardless of which one it is. So none of that has anything to do with python2
vs. python
.
Just among the ports I have installed here, I count 13 non-gpsd
executables specifying the generic python
in the shebang line (which is fine by those of us who find gratuitous monoversionism annoying). Up through version 3.15, gpsd
also used the generic python
in all its shebang lines. It changed them to python2
in 3.16 because at least one Linux distro has switched to Python3 as the default Python, and the code isn't Python3-ready. The only *real* difference between python2
and python
is that Apple provides the latter regardless of MacPorts, while the former is missing from a base install.
Assuming /usr/local/bin
exists and is in the command path, then sudo ln -s /usr/bin/python /usr/local/bin/python2
is sufficient to make python2
shebang lines work just as well as python
shebang lines in all cases (at least until such time as Apple moves to a Python3 default, which hasn't happened yet, and which ought to be accompanied by a proper python2
if and when it happens). It's not a bad idea to do that for consistency anyway, regardless of what one does with python2_select
, but I thought it was best to warn users who hadn't set up any form of python2
(though some gpsd
users might not even care about the Python components; it's primarily a C package).
Although gpsd
may be the first port where this issue has appeared (or at least the first one where anyone has noticed), it's almost certainly not the last, as additional non-Python3-ready code gets tweaked to defend against Python3 as the default. Does it really make sense to inflict a growing number of maintainability-reducing patches on a growing number of ports, just to let some subset of users avoid a one-time use of a one-line command that they should probably be doing anyway?
BTW, it would probably be a good idea to expand the port notes for the Python 2.x ports to suggest using port select python2
, in addition to the current suggestion for port select python
(given the likely future increase in the use of python2
). If that had been in place for a while, I wouldn't have bothered doing anything about it here.
comment:12 Changed 9 years ago by mojca (Mojca Miklavec)
Please bring that discussion up on the macports-dev
mailing list. In this particular ticket it's just me and you (and two others). The wider community of developers won't read the ticket unless someone brings it up on the mailing list. (You may explain (copy-paste) your thoughts again in the email rather than just pointing to the ticket.)
comment:13 Changed 8 years ago by mp@…
This patch works beautifully on Mavericks. Thanks a zillion for the effort!
(I've actually spent most of today – along with several attempts during the past months – trying to fix this port myself, and just as I gave up I found this patch. That it's been available and just not committed because of some nitpicking is sort of hilarious.)
comment:15 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
Status: | new → assigned |
---|
comment:16 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
Please remove the requirement that the user use python_select
to select python2x, as Mojca already explained above.
Please don't use system
to call chmod
to change a file's permissions; use file attributes -permissions
instead.
comment:17 follow-up: 18 Changed 7 years ago by fhgwright (Fred Wright)
This is now obsolete, since 3.17 has been released. There's a new pull request for 3.17:
comment:18 Changed 7 years ago by fhgwright (Fred Wright)
Replying to fhgwright:
This is now obsolete, since 3.17 has been released. There's a new pull request for 3.17:
The 3.17 PR has been merged, so this ticket is now *really* obsolete.
comment:19 Changed 7 years ago by mf2k (Frank Schima)
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Cc Me!