Last Comment Bug 774584 - Remove $(shell) from browser/locales/Makefile.in
: Remove $(shell) from browser/locales/Makefile.in
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Build Config (show other bugs)
: 11 Branch
: All All
: -- normal (vote)
: Firefox 17
Assigned To: Edmund Wong (:ewong)
:
Mentors:
Depends on:
Blocks: 769390
  Show dependency treegraph
 
Reported: 2012-07-16 23:54 PDT by Gregory Szorc [:gps]
Modified: 2012-08-11 19:54 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove $(shell) from browser/locales/Makefile.in (1.01 KB, patch)
2012-08-09 02:19 PDT, Edmund Wong (:ewong)
mh+mozilla: review+
Details | Diff | Splinter Review
Remove $(shell) from browser/locales/Makefile.in (v2) (1.05 KB, patch)
2012-08-09 18:57 PDT, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Splinter Review
Remove $(shell) from browser/locales/Makefile.in (v3) (1.07 KB, patch)
2012-08-09 18:58 PDT, Edmund Wong (:ewong)
ewong: review+
Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2012-07-16 23:54:03 PDT
Makefile.in files should not have $(shell) per bug 769390.

browser/locales/Makefile.in has a $(shell) on line 44 (setting APP_VERSION).

There is a nice comment saying this might be unused. Other parts of the build system use:

 APP_VERSION = @MOZ_APP_VERSION@

We should switch to that or something similar (assuming this is still needed, which it may not be).
Comment 1 Axel Hecht [pto-Aug-30][:Pike] 2012-07-17 00:55:34 PDT
Yeah, this might be unused, historic raisins are http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/browser/locales&command=DIFF_FRAMESET&file=Makefile.in&rev2=1.3&rev1=1.2, and the DEFINES part is now in l10n.mk and used MOZ_APP_VERSION.
Comment 2 Edmund Wong (:ewong) 2012-08-09 02:19:08 PDT
Created attachment 650474 [details] [diff] [review]
Remove $(shell) from browser/locales/Makefile.in
Comment 3 Mike Hommey [:glandium] 2012-08-09 02:26:30 PDT
Comment on attachment 650474 [details] [diff] [review]
Remove $(shell) from browser/locales/Makefile.in

AFAICT, everything that is included from this makefile use MOZ_APP_VERSION directly. Just remove the variable (and the comment). r+ if you do that.
Comment 4 Edmund Wong (:ewong) 2012-08-09 18:57:13 PDT
Created attachment 650753 [details] [diff] [review]
Remove $(shell) from browser/locales/Makefile.in (v2)

Note to all:  While it does say to Remove $(shell); there are two instances in this file.  The first one, I just removed with this patch.  The second one, as far as I've been told, isn't easy to remove (if it is at all possible to remove or made into a Variable. (my Makefile-fu is beginners).
Comment 5 Edmund Wong (:ewong) 2012-08-09 18:58:43 PDT
Created attachment 650754 [details] [diff] [review]
Remove $(shell) from browser/locales/Makefile.in (v3)

w/ review attribution...
Comment 6 Edmund Wong (:ewong) 2012-08-10 22:50:34 PDT
Pushed to mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1022af7974c9
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-08-11 19:54:31 PDT
https://hg.mozilla.org/mozilla-central/rev/1022af7974c9

Note You need to log in before you can comment on or make changes to this bug.