Closed Bug 636215 Opened 9 years ago Closed 9 years ago

browser/locales/Makefile.in cleanup, remove prepare-repackages and prepare-upload-*

Categories

(Firefox Build System :: General, defect)

x86
Windows XP
defect
Not set

Tracking

(status2.0 wontfix)

RESOLVED FIXED
Tracking Status
status2.0 --- wontfix

People

(Reporter: Callek, Assigned: Callek)

Details

(Whiteboard: fixed-in-bs)

Attachments

(1 file, 1 obsolete file)

Attached patch just clean them up. (obsolete) — Splinter Review
prepare-repackages-% and prepare-upload-% are currently unused in browser/locales/Makefile.in

None of our release engineering scripts, nor any existing build rules use them. Just get rid of them, and make life easier to understand.
Attachment #514547 - Flags: review?(armenzg)
Comment on attachment 514547 [details] [diff] [review]
just clean them up.

wow no wonder I barely remember these targets.
They are from back in 2008.
http://hg.mozilla.org/mozilla-central/rev/ccce859a94aa

As we looked around they are not in used anymore.

I see APP_VERSION being used on toolkit files but I can't tell if it is the same one as you are removing.
http://mxr.mozilla.org/mozilla-central/search?string=%25APP_VERSION%25&find=toolkit&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

Axel, do you know?
(In reply to comment #1)
> I see APP_VERSION being used on toolkit files but I can't tell if it is the
> same one as you are removing.
> http://mxr.mozilla.org/mozilla-central/search?string=%25APP_VERSION%25&find=toolkit&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
> 
> Axel, do you know?

the test/ ones don't expand anything from a makefile, and its app-code that expands them.

The XPIProvider.jsm and plugin*Datasource.js use regex to expand these based on variables local to their respective scopes. None of the makefiles included uses this var as defined here. (fwiw Thunderbird also includes l10n.mk and does not define APP_VERSION in its locales/Makefile.in)
Per a chat with armen in IRC, we're keeping the APP_VERSION (with my XXX Comment) just to be extra safe in this end-game. The rest is perfectly safe, and simply unused cleanup.

For added safety once this lands I'll verify with a few repacks from the following nightly, and/or get one or two manually kicked immediately after I land this.
Attachment #514547 - Attachment is obsolete: true
Attachment #514547 - Flags: review?(armenzg)
Attachment #514605 - Flags: review+
Attachment #514605 - Flags: approval2.0?
(In reply to comment #3)
> Created attachment 514605 [details] [diff] [review]
> just clean them up. v2

O yea, that r+ is r=armenzg via IRC
Comment on attachment 514605 [details] [diff] [review]
just clean them up. v2

I can't see why we'd take this now, please renominate if you really need this for some reason that isn't in the bug.
Attachment #514605 - Flags: approval2.0? → approval2.0-
status2.0: --- → wontfix
FTR, if you're asking questions to some Axel guy, it might make sense to CC one.
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.