Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#47907 closed update (fixed)

libvpx: update to 1.4.0

Reported by: ryandesign (Ryan Schmidt) Owned by: dbevans (David B. Evans)
Priority: Normal Milestone:
Component: ports Version:
Keywords: haspatch Cc: ballapete (Peter Dyballa), larryv (Lawrence Velázquez)
Port: libvpx

Description

I see libvpx 1.4.0 is available here:

http://downloads.webmproject.org/releases/webm/index.html

livecheck didn't see it because it's looking at Google Code, but Google Code is being discontinued and no longer permits new files being uploaded, which is probably why libvpx downloads have moved elsewhere.

Attachments (9)

patch-libvpx-1.4.0.diff (4.6 KB) - added by dbevans (David B. Evans) 4 years ago.
Rough patch to update libvpx to version 1.4.0 for testing, universal support disabled for now.
build-x86_64-darwin13-gcc.log (345.1 KB) - added by dbevans (David B. Evans) 4 years ago.
Successful Mavericks build log using initial test port
main.log (15.8 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.
10.5 ppc main.log
config.log (138.6 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.
10.5 ppc config.log
libvpx-1.4.0-ryandesign.diff (3.9 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.
proposal
patch-build-make-configure.sh.diff (2.7 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.
updated patch
patch-configure.diff (1.7 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.
updated patch
libvpx-1.4.0-ryandesign2.diff (3.6 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.
libvpx-1.4.0-ryandesign3.diff (3.5 KB) - added by ryandesign (Ryan Schmidt) 4 years ago.

Download all attachments as: .zip

Change History (22)

comment:1 Changed 4 years ago by dbevans (David B. Evans)

Status: newassigned

Thanks for the heads up on this. Although Google Code is being discountinued, it appears that libvpx is one of projects that Google is still supporting via Chromium and Google API. There also is an official github mirror.

New download URL is http://storage.googleapis.com/downloads.webmproject.org/releases/webm/

Repository URL is https://chromium.googlesource.com/webm/webm-tools/
Github mirror URL is https://github.com/webmproject/webm-tools/

Update is in progress.

Changed 4 years ago by dbevans (David B. Evans)

Attachment: patch-libvpx-1.4.0.diff added

Rough patch to update libvpx to version 1.4.0 for testing, universal support disabled for now.

comment:2 Changed 4 years ago by dbevans (David B. Evans)

Cc: Peter_Dyballa@… larryv@… added

As requested by ryandesign in #47920, attached is a very rough initial patch to update libvpx to version 1.4.0 for testing.

  • patches that override the port's own per platform targets are disabled
  • attempts to disable dependency on yasm for ppc platforms (#40154)
  • broken unit tests are disabled
  • universal support is temporarily disabled to reduce testing confusion

Please test and report your results, especially on PPC platforms. I'm interested in whether configure correctly selects an appropriate platform and configuration options in this mode. When reporting testing results, either successful or not. please attach your build log so I can see exactly what is happening on your platform.

For example, my (successful) build on Mavericks (full build log attached) shows

:info:configure Configuring for target 'x86_64-darwin13-gcc'
:info:configure   enabling x86_64
:info:configure   enabling mmx
:info:configure   enabling sse
:info:configure   enabling sse2
:info:configure   enabling sse3
:info:configure   enabling ssse3
:info:configure   enabling sse4_1
:info:configure   enabling avx
:info:configure   enabling avx2
:info:configure   using yasm
:info:configure   enabling use_x86inc
:info:configure   enabling webm_io
:info:configure   enabling libyuv
:info:configure Creating makefiles for x86_64-darwin13-gcc libs
:info:configure Creating makefiles for x86_64-darwin13-gcc docs
Last edited 4 years ago by dbevans (David B. Evans) (previous) (diff)

Changed 4 years ago by dbevans (David B. Evans)

Successful Mavericks build log using initial test port

Changed 4 years ago by ryandesign (Ryan Schmidt)

Attachment: main.log added

10.5 ppc main.log

Changed 4 years ago by ryandesign (Ryan Schmidt)

Attachment: config.log added

10.5 ppc config.log

comment:3 Changed 4 years ago by ryandesign (Ryan Schmidt)

universal_variant no doesn't disable the universal variant when the muniversal portgroup is included; comment out PortGroup muniversal 1.0 as well to disable the universal variant.

The only two values that os.arch will ever be are i386 and powerpc, so the conditional [string match "ppc*" ${os.arch}] never matches, so the yasm dependency is always still there. Removing yasm when os.arch is powerpc is probably also not the right idea: what if you want to build universal for ppc and i386 on a ppc machine? Don't you need yasm then? I can test that later when universal builds are working. I think probably the way this should go is something like (untested):

if {[string match "*86*" [get_canonical_archs]]} {
    depends_build-append port:yasm
}

That way, yasm only gets added if you're building for an Intel arch.


It doesn't get past the configure phase on 10.5 ppc. main.log, config.log. It seems it has decided to build universal, even though I did not select that variant, and then failed, possibly because the MacPorts-supplied PowerPC arch flags got added to the project-supplied Intel arch flags, leading to the error "gcc-4.2: -E, -S, -save-temps and -M options are not allowed with multiple -arch flags". Maybe this is a reason why we were specifying the target before.

Last edited 4 years ago by ryandesign (Ryan Schmidt) (previous) (diff)

Changed 4 years ago by ryandesign (Ryan Schmidt)

proposal

comment:4 in reply to:  3 Changed 4 years ago by ryandesign (Ryan Schmidt)

Replying to ryandesign@…:

It seems it has decided to build universal, even though I did not select that variant [...]. Maybe this is a reason why we were specifying the target before.

Oh yeah, there's even a comment in the portfile about that.

I've attached a revised update proposal bringing back the commented-out code for specifying the target. I cleaned up the code a bit and fixed it to use the right target OS version based on the deployment target configured in macports.conf. It builds for me on x86_64 Yosemite (universal i386 and x86_64), and i386 Snow Leopard (non-universal i386, and universal i386 x86_64 ppc). I use --force-target instead of --target to avoid complaints about unknown target ppc32-darwin10-gcc.

I didn't yet look closely at the patchfiles you're no longer using...

comment:5 Changed 4 years ago by ryandesign (Ryan Schmidt)

Keywords: haspatch added

Ok, most of the patches deal with disabusing the hand-rolled configure script of the idea that it should do different things on different OS X versions and that it should pick the SDK and deployment target, and I'm completely in favor of that, so let's keep those patches. The alternative is that the configure script needs to be patched every time a new version of OS X is released, which is a bit mad; we used to do that, until Jeremy fixed it with the current patches.


There's another unrelated part of the patch that modifies two sed invocations. This was added in r70208 to fix comment:ticket:25815:26. It originally made these changes:

  • build/make/configure.sh

    old new  
    375375EOF
    376376
    377377    if enabled rvct; then cat >> $1 << EOF
    378 fmt_deps = sed -e 's;^__image.axf;\$(dir \$@)\$(notdir \$<).o \$@;' #hide
     378fmt_deps = sed -e 's;^__image.axf\$\$;\$(dir \$@)\$(notdir \$<).o \$@;' #hide
    379379EOF
    380380    else cat >> $1 << EOF
    381 fmt_deps = sed -e 's;^\(.*\)\.o;\$(dir \$@)\1\$(suffix \$<).o \$@;' #hide
     381fmt_deps = sed -e 's;^\(.*\)\.o\$\$;\$(dir \$@)\1\$(suffix \$<).o \$@;' #hide
    382382EOF
    383383    fi
    384384

The intention was that after the various quoting is removed, a "$" will have been inserted at the end of the search string, anchoring the search string to the end of the line. The search string is already anchored to the beginning of the line with the leading "^". So whereas before the second invocation would find lines containing ".o", now it only finds lines that end with ".o". This is important because the intention of the second sed invocation was to identify object files whose names end with ".o", but it was finding other things as well, because the default build path in MacPorts contains the string "rsync.macports.org"—which just happens to contain ".o".

The patch was updated in r74608 to accommodate changes between libvpx 0.9.1 and 0.9.5—changes that made our patch unnecessary, and it could have been dropped at that time. But it was kept, and updated a couple times more, most recently in r117553, when it was changed in such a way that the original intention was lost:

  • build/make/configure.sh

    old new  
    442442EOF
    443443
    444444    if enabled rvct; then cat >> $1 << EOF
    445 fmt_deps = sed -e 's;^__image.axf;\${@:.d=.o} \$@;' #hide
     445fmt_deps = sed -e 's;^__image.axf;\$\${@:.d=.o} \$@;' #hide
    446446EOF
    447447    else cat >> $1 << EOF
    448 fmt_deps = sed -e 's;^\([a-zA-Z0-9_]*\)\.o;\${@:.d=.o} \$@;'
     448fmt_deps = sed -e 's;^\([a-zA-Z0-9_]*\)\.o;\$\${@:.d=.o} \$@;'
    449449EOF
    450450    fi
    451451

Now it just doubles the "$" at the beginning of the replacement string, which was not the original intent. We should drop this part of the patch now.


A final part of the patch was added just recently for #47436 and that part can I believe be removed now since that part of the code has changed in 1.4.0. I do not understand the code, but I did not have any problem building universal with Xcode 6.3 without that patch, which was what the patch was intended to fix.

Changed 4 years ago by ryandesign (Ryan Schmidt)

updated patch

Changed 4 years ago by ryandesign (Ryan Schmidt)

Attachment: patch-configure.diff added

updated patch

comment:6 Changed 4 years ago by ryandesign (Ryan Schmidt)

Updated files attached.

I removed the "-clang" lines from patch-configure because they didn't seem to be used anywhere else. I removed the parts of the port that cleared the universal flags, because that didn't seem to be necessary.

Successful builds on x86_64 Yosemite (x86_64, and universal x86_64 i386), i386 Snow Leopard (universal x86_64 i386 ppc), ppc Leopard (universal ppc i386).

Changed 4 years ago by ryandesign (Ryan Schmidt)

comment:7 Changed 4 years ago by dbevans (David B. Evans)

Thanks, Ryan, this all looks reasonable to me particularly if it fixes the various ppc build problems. Would appreciate it if you (or someone else with the appropriate hardward) could test universal and non-universal builds on Tiger as well.

For completeness, can you confirm that these changes do, in fact, fix the issues raised in #40154?

comment:8 Changed 4 years ago by ryandesign (Ryan Schmidt)

As far as I know, there weren't and aren't any ppc build problems. (There was the problem that you could not build for ppc on Snow Leopard, which I already corrected in the update to 1.4.0, since that's a thing I want to be able to do.) The only reported "problem" was #40154 which was just that an unnecessary dependency was added, which is now fixed. I'll attach a revised patch for the remainder of the update that's still needed.

Changed 4 years ago by ryandesign (Ryan Schmidt)

comment:9 in reply to:  8 ; Changed 4 years ago by dbevans (David B. Evans)

Replying to ryandesign@…:

The only reported "problem" was #40154 which was just that an unnecessary dependency was added, which is now fixed. I'll attach a revised patch for the remainder of the update that's still needed.

No need if the change is just to remove the changes you made in r137226. I've already merged those with my copy.

Is there anything else beyond that that needs to be addressed before committing the update?

comment:10 Changed 4 years ago by dbevans (David B. Evans)

Never mind, I see you did it already. Sorry for not paying attention.

comment:11 in reply to:  9 Changed 4 years ago by ryandesign (Ryan Schmidt)

Replying to devans@…:

Is there anything else beyond that that needs to be addressed before committing the update?

Don't think so, I think it's ready to be committed.

comment:12 Changed 4 years ago by dbevans (David B. Evans)

Resolution: fixed
Status: assignedclosed

Right, committed in r137263, thanks for your help with the ppc/universal issues. If you are agreeable, please feel free to add yourself as a co-maintainer for these sort of issues in the future. I just don't have the resources available to test.

comment:13 Changed 4 years ago by ballapete (Peter Dyballa)

Yesterday I found time to upgrade so many packages. libvpx also upgraded (and yasm could be uninstalled).

Note: See TracTickets for help on using tickets.