Closed Bug 772841 Opened 12 years ago Closed 12 years ago

optimizejars.py doesn't do anything

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox13 affected, firefox14+ fixed, firefox15+ fixed, firefox16+ fixed)

RESOLVED FIXED
mozilla16
Tracking Status
firefox13 --- affected
firefox14 + fixed
firefox15 + fixed
firefox16 + fixed

People

(Reporter: glandium, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

optimizejars.py was broken by the change of rule used to prepare firefox for the profile run from package to stage-package in bug 481815. Stage-package doesn't do omnijar packing, so when the profile runs, there's no omnijar to log accesses to.
Which change specifically caused the regression?
(In reply to Brian R. Bondy [:bbondy] from comment #1)
> Which change specifically caused the regression?

This:
-       $(MAKE) -C $(PGO_OBJDIR) package
+       $(MAKE) -C $(PGO_OBJDIR) stage-package
Attachment #641051 - Flags: review?(ted.mielczarek)
I don't think 481815 made any such change.
(In reply to Brian R. Bondy [:bbondy] from comment #4)
> I don't think 481815 made any such change.

https://hg.mozilla.org/mozilla-central/rev/6b60ebe2cae4
bug 481815 comment 458
ah ok, I thought you meant it was part of the core maintenance service code.  That patch really should have landed in its own bug number.
Comment on attachment 641051 [details] [diff] [review]
Pack omnijar when doing stage-package

Doesn't work :(
https://tbpl.mozilla.org/php/getParsedLog.php?id=13429714&tree=Try
Attachment #641051 - Flags: review?(ted.mielczarek)
This should work better, on top of avoiding zipping the omnijar twice like we currently do.

I'd push to try if try was working:
pushing to ssh://hg.mozilla.org/try
remote: Traceback (most recent call last):
remote:   File "/usr/local/bin/pash.py", line 4, in <module>
remote:     import hg_helper, ldap_helper
remote:   File "/usr/local/bin/hg_helper.py", line 4, in <module>
remote:     from ldap_helper import ldap_connect, get_ldap_attribute
remote:   File "/usr/local/bin/ldap_helper.py", line 50
remote:     if access_time < yesterday:
remote:                               ^
remote: IndentationError: unindent does not match any outer indentation level
Attachment #641101 - Flags: review?(ted.mielczarek)
Attachment #641051 - Attachment is obsolete: true
Previous one was breaking b2g and mobile.
Attachment #641143 - Flags: review?(ted.mielczarek)
Attachment #641101 - Attachment is obsolete: true
Attachment #641101 - Flags: review?(ted.mielczarek)
Comment on attachment 641143 [details] [diff] [review]
Pack omnijar when doing stage-package

Dammit, it breaks osx.
Attachment #641143 - Flags: review?(ted.mielczarek)
Blocks: 773171
It's getting too late in the 14 release process to try to do the right thing. It takes too long to get it not to break mac builds.
Attachment #641462 - Flags: review?(ted.mielczarek)
Attachment #641143 - Attachment is obsolete: true
Attachment #641462 - Attachment is obsolete: true
Attachment #641462 - Flags: review?(ted.mielczarek)
Attachment #641472 - Flags: review?(catlee) → review+
Comment on attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.

Review of attachment 641472 [details] [diff] [review]:
-----------------------------------------------------------------

Would be nice if we could avoid the overhead of actually generating the final package file as well, but not critical.
Attachment #641472 - Flags: review?(ted.mielczarek) → review+
(In reply to Ted Mielczarek [:ted] from comment #13)
> Would be nice if we could avoid the overhead of actually generating the
> final package file as well, but not critical.

That's what i was trying to do, but ran out of time.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
please nominate for uplift with risk assessment so we can evaluate whether we can still take this on beta.
Comment on attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.

[Approval Request Comment]
Regression caused by a part of bug 481815
User impact if declined: Keeping cold startup performance regression we got in either FF12 or FF13 (not entirely clear which one)
Testing completed (on m-c, etc.): Tested on try with pgo ; try logs checked by catlee and myself for correctness. 
Risk to taking this patch (and alternatives if risky): Quite low. It's not introducing new code/functionality, it's restoring the use of code that was mistakenly disabled by a small change from bug 481815.
String or UUID changes made by this patch: None
Attachment #641472 - Flags: approval-mozilla-release?
Attachment #641472 - Flags: approval-mozilla-beta?
Attachment #641472 - Flags: approval-mozilla-aurora?
Attachment #641472 - Flags: approval-mozilla-release?
Comment on attachment 641472 [details] [diff] [review]
Use make package instead of make stage-package before profile run, but avoid package signing.

restoring use of broken code, go ahead and land to branches.
Attachment #641472 - Flags: approval-mozilla-beta?
Attachment #641472 - Flags: approval-mozilla-beta+
Attachment #641472 - Flags: approval-mozilla-aurora?
Attachment #641472 - Flags: approval-mozilla-aurora+
Blocks: 1195331
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.