Closed
Bug 525047
Opened 11 years ago
Closed 11 years ago
'make clean' deletes automationutils.py
Categories
(Firefox Build System :: General, defect)
Tracking
(status1.9.2 .2-fixed, status1.9.1 .9-fixed)
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: ennofennema, Assigned: neil)
References
Details
(Keywords: verified1.9.1, verified1.9.2)
Attachments
(1 file, 4 obsolete files)
2.71 KB,
patch
|
ted
:
review+
dveditz
:
approval1.9.2.2+
dveditz
:
approval1.9.1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.21) Gecko/20090410 SUSE/1.1.16-1.1 SeaMonkey/1.1.16 Build Identifier: SEAMONKEY_2_0rc2_RELEASE 'make clean' deletes mozilla/build/automationutils.py which is a proper and necessary source file and not eg. generated from automationutils.py.in Should not happen Reproducible: Always
Comment 1•11 years ago
|
||
I've run into this problem also with SeaMonkey 2.0 release on Mac OS X
Comment 2•11 years ago
|
||
I assume you're doing a srcdir build?
Assignee | ||
Comment 3•11 years ago
|
||
build/automation-build.mk copies automation.py to the objdir (why?) and adds it to the garbage. This is of course unhelpful for a srcdir build ;-)
Assignee | ||
Comment 4•11 years ago
|
||
Assignee: nobody → neil
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #409058 -
Flags: review?(ted.mielczarek)
Comment 5•11 years ago
|
||
(In reply to comment #3) > build/automation-build.mk copies automation.py to the objdir (why?) and adds it > to the garbage. This is of course unhelpful for a srcdir build ;-) Because PYTHONPATH mangling can be a bit of a pain, and our preprocessing of Python files doesn't make it any easier. Ideally we wouldn't preprocess any of our Python, and we could stop copying these files around.
(In reply to comment #2) > I assume you're doing a srcdir build? Yes, if that means not using objdir
I know little about the python preprocessing but could a solution be introducing a automationutils.py.in with nill reprocessing?
Comment 8•11 years ago
|
||
No. I don't want that.
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to comment #5) > (In reply to comment #3) > > build/automation-build.mk copies automation.py to the objdir (why?) and adds it > > to the garbage. This is of course unhelpful for a srcdir build ;-) > Because PYTHONPATH mangling can be a bit of a pain, and our preprocessing of > Python files doesn't make it any easier. Ideally we wouldn't preprocess any of > our Python, and we could stop copying these files around. How does automation-utils.py relate to the preprocessing of automation.py?
Updated•11 years ago
|
Version: unspecified → 1.9.1 Branch
Comment 10•11 years ago
|
||
Comment on attachment 409058 [details] [diff] [review] Possible patch I'm pretty sure this will break Mochitest/reftest. When running them in-tree, they rely on having automationutils.py copied to the local directory, then copied again to $objdir/_test/whatever.
Assignee | ||
Comment 11•11 years ago
|
||
(In reply to comment #10) > (From update of attachment 409058 [details] [diff] [review]) > I'm pretty sure this will break Mochitest/reftest. When running them in-tree, > they rely on having automationutils.py copied to the local directory, then > copied again to $objdir/_test/whatever. Oh I see, I need to change the references from $(CURDIR)/automationutils.py to $(topsrcdir)/build/automationutils.py, to avoid the unnecessary copy.
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #409058 -
Attachment is obsolete: true
Attachment #412563 -
Flags: review?(ted.mielczarek)
Attachment #409058 -
Flags: review?(ted.mielczarek)
Comment 13•11 years ago
|
||
Comment on attachment 412563 [details] [diff] [review] Possible patch Ok, seems reasonable.
Attachment #412563 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 14•11 years ago
|
||
Pushed changeset dbdbe3ad0234 to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: [wanted on m-1.9.2/m-1.9.1 too...]
Target Milestone: --- → mozilla1.9.3a1
Version: 1.9.1 Branch → Trunk
Comment 15•11 years ago
|
||
Backed out, since this caused clobbered leak test machines to fail with: ImportError: No module named automationutils e.g.: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1260932775.1260937785.2108.gz&fulltext=1 https://hg.mozilla.org/mozilla-central/rev/4bbe26190c7d
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 16•11 years ago
|
||
(In reply to comment #15) > Backed out, since this caused clobbered leak test machines to fail with: > > ImportError: No module named automationutils > > e.g.: > http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1260932775.1260937785.2108.gz&fulltext=1 Weird. That log shows automationutils.py being installed into _leaktest: /builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox/config/nsinstall -R automation.py /builds/moz2_slave/mozilla-central-linux-debug/build/build/automationutils.py leaktest.py /builds/moz2_slave/mozilla-central-linux-debug/build/build/bloatcycle.html /builds/moz2_slave/mozilla-central-linux-debug/build/build/pgo/server-locations.txt ../_leaktest And python leaktest.py -- -register certainly succeeds locally...
Assignee | ||
Comment 17•11 years ago
|
||
My bad, this works on Windows because it copies instead of symlinks...
Assignee | ||
Comment 18•11 years ago
|
||
(In reply to comment #11) > Oh I see, I need to change the references from $(CURDIR)/automationutils.py to > $(topsrcdir)/build/automationutils.py, to avoid the unnecessary copy. And I need to not forgot to prefix $(topsrcdir)/build/ !
Assignee | ||
Comment 19•11 years ago
|
||
And I need to remember not to typo the checkin comment again too... Pushed changeset 5a2692b1abee to mozilla-central.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Comment 20•11 years ago
|
||
(In reply to comment #19) > Pushed changeset 5a2692b1abee to mozilla-central. http://hg.mozilla.org/mozilla-central/rev/d86a2cf35ca8 backout
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 21•11 years ago
|
||
(In reply to comment #18) > (In reply to comment #11) > > Oh I see, I need to change the references from $(CURDIR)/automationutils.py to > > $(topsrcdir)/build/automationutils.py, to avoid the unnecessary copy. > And I need to not forgot to prefix $(topsrcdir)/build/ ! That's not it, since if you're building from $(CURDIR)/build/ then $(topsrcdir)/build/ is in your $(VPATH)...
Assignee | ||
Comment 22•11 years ago
|
||
Attachment #412563 -
Attachment is obsolete: true
Attachment #418843 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 23•11 years ago
|
||
Attachment #418907 -
Flags: review?(ted.mielczarek)
Comment 24•11 years ago
|
||
(In reply to comment #22) Ftr, > Created an attachment (id=418843) [details] This one uses multiple |NSDISTMODE = copy|. (In reply to comment #23) > Created an attachment (id=418907) [details] This one uses |sys.path.insert(0, "");|
Comment 25•11 years ago
|
||
Comment on attachment 418843 [details] [diff] [review] Stupid Python Yeah, I'd rather keep the symlinks, they make development easier.
Attachment #418843 -
Flags: review?(ted.mielczarek) → review-
Comment 26•11 years ago
|
||
Comment on attachment 418907 [details] [diff] [review] Stupid Python >diff --git a/build/automation.py.in b/build/automation.py.in >--- a/build/automation.py.in >+++ b/build/automation.py.in >@@ -45,17 +45,17 @@ import re > import re > import select > import shutil > import signal > import subprocess > import sys > import threading > import tempfile >- >+sys.path.insert(0, ""); What does this wind up doing, adding the script's directory to sys.path? Could we make this clearer using os.path.dirname(__file__) or something? (I tested, and dirname(__file__) returns the containing dir even if __file__ is a symlink.)
Attachment #418907 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 27•11 years ago
|
||
We already have a variable with the appropriate path in question :-)
Attachment #418843 -
Attachment is obsolete: true
Attachment #418907 -
Attachment is obsolete: true
Attachment #421035 -
Flags: review?(ted.mielczarek)
Comment 28•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] Yeah, that's the ticket. For the record, when Neil says "stupid Python" he means "when Python adds the script directory to sys.path, if the script is a symlink, it actually adds the directory of the original file".
Attachment #421035 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 29•11 years ago
|
||
Pushed changeset fa0a5a38b7f4 to mozilla-central. (In reply to comment #28) > For the record, when Neil says "stupid Python" he means "when Python adds the > script directory to sys.path, if the script is a symlink, it actually adds the > directory of the original file". Which IMHO is still a stupid thing to do...
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Comment 30•11 years ago
|
||
Not disagreeing, just clarifying! (since it wasn't mentioned anywhere else on the bug)
Updated•11 years ago
|
status1.9.1:
--- → ?
status1.9.2:
--- → ?
Comment 31•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] "approval1.9.2.2=?": Zero risk, build config only.
Attachment #421035 -
Flags: approval1.9.2.2?
Comment 32•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] Approved for 1.9.2.2, a=dveditz for release-drivers
Attachment #421035 -
Flags: approval1.9.2.2? → approval1.9.2.2+
Updated•11 years ago
|
Comment 33•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] "approval1.9.1.9=?": Zero risk, build config only.
Attachment #421035 -
Attachment description: Even better → Even better
[Checkin: Comment 29 & 33]
Attachment #421035 -
Flags: approval1.9.1.9?
Updated•11 years ago
|
Attachment #421035 -
Attachment description: Even better
[Checkin: Comment 29 & 33] → Even better
[Checkin: Comment 29 & 34]
Comment 34•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] http://hg.mozilla.org/releases/mozilla-1.9.2/rev/08f2d33c06f6
Updated•11 years ago
|
Comment 35•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] Approved for 1.9.1.9, a=dveditz for release-drivers
Attachment #421035 -
Flags: approval1.9.1.9? → approval1.9.1.9+
Comment 36•11 years ago
|
||
Comment on attachment 421035 [details] [diff] [review] Even better [Checkin: Comment 29 & 34 & 36] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4fdc73ee8729
Attachment #421035 -
Attachment description: Even better
[Checkin: Comment 29 & 34] → Even better
[Checkin: Comment 29 & 34 & 36]
Updated•11 years ago
|
Comment 37•11 years ago
|
||
Verified that patches pushed in comment#34 and comment#36 landed.
Keywords: verified1.9.1,
verified1.9.2
Updated•3 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•