Opened 8 years ago
Closed 6 years ago
#54125 closed update (fixed)
portaudio upgrade
Reported by: | RJVB (René Bertin) | Owned by: | RJVB (René Bertin) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | haspatch | Cc: | mojca (Mojca Miklavec) |
Port: | portaudio |
Description
port:portaudio is significantly out of date, this patch brings it to the current version.
Bonus modifications:
- avoids opportunistic dependencies on port:jack
- adds a variant to install test and example binaries
Attachments (6)
Change History (12)
Changed 8 years ago by RJVB (René Bertin)
Attachment: | patch-configure added |
---|
Changed 8 years ago by RJVB (René Bertin)
Attachment: | patch-ltmain.sh.diff added |
---|
Changed 8 years ago by RJVB (René Bertin)
Attachment: | patch-src__common__pa_types.h added |
---|
Changed 8 years ago by RJVB (René Bertin)
Attachment: | patch-audacity-portmixer.diff added |
---|
Changed 8 years ago by RJVB (René Bertin)
Attachment: | patch-src__hostapi__coreaudio__pa_mac_core.c added |
---|
Changed 8 years ago by RJVB (René Bertin)
Attachment: | portaudio.diff added |
---|
comment:1 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
comment:2 Changed 8 years ago by RJVB (René Bertin)
Changing the advertised would mean adapting the distname calculation as well as the livecheck. Not a big deal, but why bother using a different version, or a different version rewritten this way? Why not 19.6.0.2016.10.30
while we're at it, for instance? Upstream must have had a reason to pick the version string they did and it's not like MacPorts needs standardised version numbers for us in depspecs or the like ;)
The 2 make calls in the tools
variant are to ensure that the respective targets are built. They're not run by the current build system, so there's no interference with port test
. You're right that I could add the targets via build.target-append
.
Good to know about the expansion operator. Is there an official policy concerning its use? I can see how it can be useful but I usually avoid obfuscated syntax like this.
comment:3 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
Software developers often obfuscate their projects' version numbers in the distname, as the developers of portaudio have done here. If you look through the source files you'll see they refer to this version as "19.6.0" not "190600". MacPorts portfiles should strive to put the projects' actual version numbers into the version field, not whatever obfuscation the developers may have used in the distfile name.
Does make tests
build something that gets installed by the tools variant, or does it (as I assumed) run tests? If it runs tests, then I wouldn't do it automatically because that's not what we usually do. If it builds something that gets installed, that's fine, and I guess it's just unfortunate that the developers have used a confusing target name.
The expansion operator is not obfuscation; it's simplification. We didn't used to use it because it requires Tcl 8.5 or greater; now that MacPorts bundles a copy of Tcl 8.5 there's no reason not to use the available features of the language.
comment:4 Changed 8 years ago by RJVB (René Bertin)
I understand your reasoning with the version, but I cannot help but think that most users don't care about the actual version number reported, if anything possible only whether livecheck reports that it's up to date. Note that this is a library, not an executable that reports a different version somewhere - and that the online documentation already talks about PortAudio 2 (fortunately they don't use a scheme where a pre-release version has a higher number than the upcoming release version). I'll look into it, but this is not high-priority for me. I'm not planning on taking over maintainership.
Re: make tests: no, that doesn't actually run any tests. As I said, neither of the make commands run their targets. I wouldn't have included the test executables in the variant otherwise.
The expansion operator is not obfuscation; it's simplification.
With all due respect, I think it's both, and very much along the lines of the old obfuscated C jokes. It doesn't help understanding (readability) to the casual port (Tcl) developer. It means extra work the moment you need to do something more inside the loop.
How does it even work, xinstall
doesn't accept multiple file arguments to my knowledge so does the operator have an RPN aspect to it?
comment:5 Changed 6 years ago by mojca (Mojca Miklavec)
I forgot about this ticket and only checked if there were any open tickets left after spending some time trying to close the oldest open pull request.
I quickly checked the attachments and the only two remaining "patches" seem to be the two new variants. Please let me know if we are missing anything else. Note that ./configure --help
mentions
--with-jack Enable support for JACK [autodetect]
so I'm a bit confused about the usage of --disable-jack
.
Parallel build did not work for me. I tried to remove that line, but then the build broke (I'm on APFS, no clue if that's related in any way).
In the Makefile
the target all
has both tests
and examples
as dependency:
all: lib/$(PALIB) all-recursive tests examples selftests
so even if we run make
again or if we add additional targets, all we gain is
make: Nothing to be done for `tests'. make: Nothing to be done for `examples'.
I opened a pull request: https://github.com/macports/macports-ports/pull/2634
comment:6 Changed 6 years ago by RJVB (René Bertin)
Owner: | set to RJVB |
---|---|
Resolution: | → fixed |
Status: | new → closed |
I think the port should advertise its version as "19.6.0.20161030" not "190600_20161030". Note this would require increasing the epoch.
I'm unclear why you're running
make tests
in a post-build block. Usually we run tests in the test phase, not the build phase.Rather than running
make examples
in a post-build block, wouldn't it suffice to usebuild.target-append examples
?You don't need
foreach
loops to delete or install multiple files. Use the expansion operator ({*}
) to turn the list result from theglob
into individual arguments thatdelete
andxinstall
understand. e.g.xinstall {*}[glob ${build.dir}/bin/.libs/pa*] ${utildir}