Last Comment Bug 646489 - Require yasm 1.0.1 instead of yasm 1.1.0 for libjpeg-turbo on Linux
: Require yasm 1.0.1 instead of yasm 1.1.0 for libjpeg-turbo on Linux
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Linux
: -- normal with 1 vote (vote)
: mozilla5
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
: 647081 (view as bug list)
Depends on:
Blocks: 573948
  Show dependency treegraph
 
Reported: 2011-03-30 08:46 PDT by Justin Lebar (not reading bugmail)
Modified: 2011-04-14 13:57 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (3.52 KB, patch)
2011-04-06 10:05 PDT, Justin Lebar (not reading bugmail)
khuey: review+
Details | Diff | Splinter Review
Followup (2.36 KB, patch)
2011-04-12 09:21 PDT, Justin Lebar (not reading bugmail)
khuey: review+
Details | Diff | Splinter Review

Description Justin Lebar (not reading bugmail) 2011-03-30 08:46:28 PDT
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 1 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-03-31 18:08:20 PDT
*** Bug 647081 has been marked as a duplicate of this bug. ***
Comment 2 Bill Gianopoulos [:WG9s] 2011-03-31 18:13:02 PDT
I verified this and commented on the original bug that decided to enforce 1.1 and was completely ignored.
Comment 3 Bill Gianopoulos [:WG9s] 2011-03-31 18:14:23 PDT
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] 2011-03-31 18:23:32 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-03-31 18:24:53 PDT
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] 2011-03-31 18:31:20 PDT
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 pissed 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) 2011-03-31 22:26:39 PDT
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] 2011-04-03 11:38:19 PDT
(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) 2011-04-06 10:05:46 PDT
Created attachment 524205 [details] [diff] [review]
Patch v1
Comment 10 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-04-06 10:08:07 PDT
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) 2011-04-06 10:12:45 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-04-06 10:17:40 PDT
sgtm
Comment 13 Justin Lebar (not reading bugmail) 2011-04-09 16:58:18 PDT
Fixed in Cedar: http://hg.mozilla.org/projects/cedar/rev/028a07f2fae2
Comment 15 Serge Gautherie (:sgautherie) 2011-04-12 03:11:34 PDT
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) 2011-04-12 09:21:43 PDT
Created attachment 525402 [details] [diff] [review]
Followup
Comment 17 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-04-12 09:22:55 PDT
Comment on attachment 525402 [details] [diff] [review]
Followup

rs=me
Comment 18 :Ehsan Akhgari 2011-04-14 13:57:41 PDT
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!

Note You need to log in before you can comment on or make changes to this bug.