646489 - Require yasm 1.0.1 instead of yasm 1.1.0 for libjpeg-turbo on Linux

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

yasm 1.1.0 is hard to come by on Linux distributions, since it's pretty new.  If 1.0.1 works just as well, we could require that version instead.

smaug verified that Firefox compiled and passed the jpeg reftests (/modules/libpr0n/test/reftest/jpeg) on Fedora 14 x86-64 using yasm 1.0.1.  But we still need to verify that it works on x86-32 using yasm 1.0.1.

Comment 2

[Bill Gianopoulos [:WG9s]]

I verified this and commented on the original bug that decided to enforce 1.1 and was completely ignored.

Comment 3

[Bill Gianopoulos [:WG9s]]

Well now I am not sure it was on the bug, but was definitely on the IRC conversation about this issue.

Comment 4

[Bill Gianopoulos [:WG9s]]

I guess since you were so quick to dupe this, that means you are not interested int he patch I intended to apply to my bug.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I would r+ and land a patch to drop the version requirement down to 1.0.1.  I don't really want to just change the error to a warning though.

Comment 6

[Bill Gianopoulos [:WG9s]]

Yes, but somehow now that my bug has been duped, I feel OK now the person who duped it is on the hook for developing such a patch. I am still **** off that this landed in the first place since before it did I had pointed out to those pushing for it the version 1.0.1 worked just fine.

Comment 7

[Justin Lebar (not reading bugmail)]

Bill, I'm sorry I missed your comment about yasm 1.0.1 working, and I'm sorry this has frustrated you.  I just searched through my IRC logs, and I don't see messages from you containing "1.0.1" or "yasm" in #developers, so you must have spoken to someone else.

If you can confirm for me that yasm 1.0.1 passes all the reftests in modules/libpr0n/test/reftest/jpeg on Linux x86-32?  If so, I'll develop the requisite patch.

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to comment #7)
> Bill, I'm sorry I missed your comment about yasm 1.0.1 working, and I'm sorry
> this has frustrated you.  I just searched through my IRC logs, and I don't see
> messages from you containing "1.0.1" or "yasm" in #developers, so you must have
> spoken to someone else.

Actually, it is me should should be apologizing form my frustrations in my real job from spilling over here.

> 
> If you can confirm for me that yasm 1.0.1 passes all the reftests in
> modules/libpr0n/test/reftest/jpeg on Linux x86-32?  If so, I'll develop the
> requisite patch.

The yasm version I am running passes all the reftests in modules/libpr0n/test/reftest/jpeg, however it is not strictly speaking 1.0.1.  yasm --version returns the following:

yasm 1.0.1.2326
Compiled on May 23 2010.
Copyright (c) 2001-2010 Peter Johnson and other Yasm developers.
Run yasm --license for licensing overview and summary.

I suspect the .2326 part might be significant here.

Comment 9

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

Comment 10

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 524205 [details] [diff] [review]
Patch v1

r+ assuming we actually do need yasm 1.1 on non-Linux.

Comment 11

[Justin Lebar (not reading bugmail)]

I'm pretty sure we actually do need yasm 1.1 on non-Linux, as bug 573948 comment 65 was made two months after the 1.0.1 release [1].

[1] https://github.com/yasm/yasm/commits/1.0.1

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

sgtm

Comment 13

[Justin Lebar (not reading bugmail)]

Fixed in Cedar: http://hg.mozilla.org/projects/cedar/rev/028a07f2fae2

Comment 14

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/028a07f2fae2

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 524205 [details] [diff] [review]
Patch v1

>+        if test "$_YASM_MAJOR_VERSION" -lt "1" -o \( "$_YASM_MAJOR_VERSION" -eq "1" -a "$_YASM_MINOR_VERSION" -eq "0" -a "$_YASM_RELEASE" -lt "1" \) ; then
>+            AC_MSG_ERROR([yasm 1.0.1 or greater is required to build with libjpeg-turbo's optimized JPEG decoding routines, but you appear to have version $_YASM_MAJOR_VERSION.$_YASM_MINOR_VERSION.$_YASM_MINOR_VERSION.  Upgrade to the newest version or configure with --disable-libjpeg-turbo to use the pure C JPEG decoder.  See https://developer.mozilla.org/en/YASM for more details.])

This should be $_YASM_MAJOR_VERSION.$_YASM_MINOR_VERSION.$_YASM_RELEASE .

Comment 16

[Justin Lebar (not reading bugmail)]

Attachment: Followup

Comment 17

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 525402 [details] [diff] [review]
Followup

rs=me

Comment 18

[(no longer active)]

The followup patch was landed:

http://hg.mozilla.org/mozilla-central/rev/c89cfb2997dc

I mistakenly backed it out <http://hg.mozilla.org/mozilla-central/rev/4090fb78bbac> but immediately restored it <http://hg.mozilla.org/mozilla-central/rev/ecfed11869b2>.  Sorry!