591611 - Clean up FIREFOX_VERSION handling

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Ed Morley [:emorley]]

Kyle, I'm presuming this is referring to bug 591387 comment 21? I'm slighly confused however due to bug 591387 comment 26 and also the fact that there is already:
AC_DEFINE_UNQUOTED(MOZ_UA_FIREFOX_VERSION, "$FIREFOX_VERSION"

...so why do we need this defined again as FIREFOX_VERSION?

I'm happy to do this cleanup, just unclear at present what of:
http://mxr.mozilla.org/mozilla-central/search?string=FIREFOX_VERSION
...needs changing to what.

Thanks!

Comment 2

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

MOZ_UA_FIREFOX_VERSION is something that's semantically different, even if it has the same value (it also has quotes).

I'd like to not touch that, and instead AC_DEFINE_UNQUOTED(FIREFOX_VERSION,$FIREFOX_VERSION).

Once that's done, any makefile that does something like

DEFINES += \
  -DFIREFOX_VERSION=$(FIREFOX_VERSION)

can have that bit removed.

As for bug 591387 comment 26, we can remove the AC_SUBST(MOZ_UA_FIREFOX_VERSION ...) and remove the autoconf.mk.in bit.

I would of course be happy to review.

Comment 3

[Ed Morley [:emorley]]

Attachment: Patch v1

- Removes |AC_SUBST(MOZ_UA_FIREFOX_VERSION)| in both configure.ins and the corresponding |MOZ_UA_FIREFOX_VERSION = @FIREFOX_VERSION@| in autoconf.mk.in , since MOZ_UA_FIREFOX_VERSION is not used in a Makefile anywhere (http://mxr.mozilla.org/mozilla-central/search?string=MOZ_UA_FIREFOX_VERSION)

- Adds |AC_DEFINE_UNQUOTED(FIREFOX_VERSION,$FIREFOX_VERSION)| to /configure.in, and then removes all |-DFIREFOX_VERSION=$(FIREFOX_VERSION)| (http://mxr.mozilla.org/mozilla-central/search?string=DFIREFOX_VERSION) since they are now redundant.

Comment 4

[Ed Morley [:emorley]]

http://dev.philringnalda.com/tbpl/?tree=Try&rev=814ab5d45c97

Comment 5

[Dão Gottwald [:dao]]

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

Comment 6

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0

Is there any way I can verify this?

I looked in the files in the repo:
http://hg.mozilla.org/mozilla-central/file/e0acef471ab2

Is that enough to mark this as VERIFIED FIXED?

Thanks!