570440 - disable-javaxpcom not recognized after configure.in change

Status: RESOLVED FIXED

Description

[Micah Gersten]

Created attachment 449572 [details] [diff] [review]
Flip enable javaxpcom check to disable javaxpcom check

When this commit happened, the explicit empty string for MOZ_JAVAXPCOM was removed:
http://hg.mozilla.org/mozilla-central/rev/21f0727c27a6

This seems to enable javaxpcom even though I have --disable-javaxpcom explicitly set.

I flipped the logic to check for it disabled and that seems to allow me to build again.  I don't know if that's the proper solution, but I am submitting the patch for review.

Comment 1

[Micah Gersten]

Created attachment 449573 [details] [diff] [review]
Flip enable javaxpcom check to disable javaxpcom check v2

Sorry, I left some debug output code in there.

Comment 2

[Micah Gersten]

Comment on attachment 449573 [details] [diff] [review]
Flip enable javaxpcom check to disable javaxpcom check v2

I'm withdrawing the review request until I verify that it builds.

Comment 4

[Jacek Caban]

Created attachment 449594 [details] [diff] [review]
fix v1.0

From bug 570463:

Since the patch from bug 557824 is committed, MOZ_ARG_ENABLE_BOOL(javaxpcom,
...) no longer overwrites MOZ_JAVAXPCOM variable when --disable-javaxpcom is
passed. MOZ_AVAXPCOM is set in xulrunner/confvars.sh for xulrunner, so it's
impossible to disable it (it's useful for crosscompiling).

Due to the way AC_ARG_ENABLE works (not passing argument is equal to false
condition), I don't see any clear way to fix it. In my proposed patch I use
MOZ_ARG_ENABLE_BOOL or MOZ_ARG_DISABLE_BOOL depending on initial MOZ_JAVAXPCOM
value (the second macro has no healp string to not duplicate the first).

Comment 5

[Benjamin Smedberg]

Comment on attachment 449594 [details] [diff] [review]
fix v1.0

This shouldn't be necessary... ENABLE_ and DISABLE_ are mirrors of eachother, and can take three arguments, so it should be possible to do:

MOZ_ARG_ENABLE_BOOL(javaxpcom, --enable-javaxpcom, MOZ_JAVAXPCOM=1, MOZ_JAVAXPCOM=)

Comment 6

[Jacek Caban]

You're right. I had an other impression after reading AC_ARG_ENABLE documentation (that the else condition is invoked when no explicit arg is passed, so it would overwrite the default value). I've checked that it's not true. A patch, that partially reverts the patch from bug 557824, will do the trick then. Now I don't see the reason for similar changes in http://hg.mozilla.org/mozilla-central/rev/21f0727c27a6 and I expect more similar problems with other configs.

Comment 7

[Jacek Caban]

Created attachment 449625 [details] [diff] [review]
fix v1.1

Comment 8

[Micah Gersten]

Reassigning to Jacek.  I realized later that the simplest solution is usually
best and was going that add that patch.  Thanks for catching that.

Comment 11

[Matt Brubeck (:mbrubeck)]

There are several similar changes made by bug 557824 (e.g. NSS_DISABLE_DBM).  Should those be reverted too?

Comment 12

[Matt Brubeck (:mbrubeck)]

Created attachment 449915 [details] [diff] [review]
alternate patch - revert all similar changes

Restore all of the MOZ_ARG_*_BOOL arguments removed by attachment 449184 [details] [diff] [review].

Comment 13

[Takanori MATSUURA]

(In reply to comment #12)
> Created an attachment (id=449915) [details]
Does this patch obsolete attachment 449573 [details] [diff] [review] and attachment 449625 [details] [diff] [review]?

Comment 14

[Jacek Caban]

Yes, it does.

Comment 16

[Mitchell Field [:Mitch]]

Pushed with conflicts fixed:
http://hg.mozilla.org/mozilla-central/rev/7043a72809a8