Closed Bug 591611 Opened 9 years ago Closed 9 years ago

Clean up FIREFOX_VERSION handling

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
mozilla7

People

(Reporter: khuey, Assigned: emorley)

References

Details

Attachments

(1 file)

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.
Attached patch Patch v1Splinter Review
- 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+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/d1fb4ff8c16e
Status: ASSIGNED → RESOLVED
Closed: 9 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
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.