Clean up FIREFOX_VERSION handling

VERIFIED FIXED in mozilla7

Status

VERIFIED FIXED
9 years ago
a year ago

People

(Reporter: khuey, Assigned: emorley)

Tracking

Trunk
mozilla7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Comment 1

8 years ago
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!
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.
(Assignee)

Comment 3

8 years ago
Created attachment 535422 [details] [diff] [review]
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.
Assignee: khuey → bmo
Status: NEW → ASSIGNED
Attachment #535422 - Flags: review?(khuey)
Attachment #535422 - Flags: review?(khuey) → review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/d1fb4ff8c16e
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
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!
Status: RESOLVED → VERIFIED

Updated

a year ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.