375921 - configure should throw error when APNG not found

Status: RESOLVED FIXED

Description

[Andrew Smith]

A check was added to configure.in so that if the system libpng doesn't have APNG support, the embedded library is used.

It would be nice to have configure through an error rather than silently ignore the --with-system-png option

Once I figure out how to do that I'll post a patch.

Comment 1

[Andrew Smith]

Created attachment 260086 [details] [diff] [review]
use AC_MSG_ERROR

It looks like this works. Will you review it please?

Comment 2

[David Baron :dbaron: ⌚️UTC-8]

I tend to think that we should be consistent in whether these arguments get ignored or give errors when we can't honor them.  This patch would make --with-system-png give an error for one case and be ignored in another.  (Never mind --with-system-zlib, ...jpeg, ...cairo, etc.)

I actually do prefer giving errors, but that's probably something at least Benjamin should be in on deciding.

Comment 3

[Benjamin Smedberg]

Yes, we should throw errors.

Comment 4

[Andrew Smith]

Created attachment 260212 [details] [diff] [review]
use AC_MSG_ERROR twice

I'm not sure this is what anyone asked for, but here is another patch that throws errors for all three system-png problems.

Comment 5

[David Baron :dbaron: ⌚️UTC-8]

Comment on attachment 260212 [details] [diff] [review]
use AC_MSG_ERROR twice

Looks good to me, although we might want Benjamin to double-check.

(It would probably be good to make similar changes for the other system libs -- jpeg, zlib, cairo, maybe nspr.)

Comment 6

[Nickolay_Ponomarev]

Doesn't apply cleanly, could you attach an updated patch?

Comment 7

[Andrew Smith]

Created attachment 264747 [details] [diff] [review]
same as above

this is the same patch as the above except for one character (acTL instead of acTl), that was fixed in another bug.

Comment 8

[Michael Wu [:mwu]]

(In reply to comment #7)
> Created an attachment (id=264747) [details]
> same as above
> 
> this is the same patch as the above except for one character (acTL instead of
> acTl), that was fixed in another bug.
> 
What's with the changes in 
modules/libpr0n/src/imgContainer.cpp?

Comment 9

[Andrew Smith]

Created attachment 268832 [details] [diff] [review]
same thing again

that creeped in from a different patch.

Comment 10

[Michael Wu [:mwu]]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1828; previous revision: 1.1827
done