Closed
Bug 924343
Opened 11 years ago
Closed 11 years ago
Move Preprocessor.py into mozbuild
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 935305
People
(Reporter: gps, Assigned: bokeefe)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
The text preprocessor in Preprocessor.py should be in the mozbuild package.
Since Preprocessor.py is referenced throughout the tree in more locations than I care to update, I'm thinking we'll just keep a stub config/Preprocessor.py around that imports and runs the new location.
Comment 1•11 years ago
|
||
And there are even external users, guilty as charged ;-). https://github.com/Pike/font-tool/blob/master/buildfonts.py#L38 is how I use it from an external python script,
sys.path.append(os.path.join(moz_base, 'config'))
from Preprocessor import preprocess
Assignee | ||
Comment 2•11 years ago
|
||
I needed the preprocessor to be somewhere other than /config for bug 935987, so I moved it into mozbuild.action. I had to move Expression.py too, because try couldn't find it on Windows (never mind that it worked on my machine), and since I was already doing that, I moved the tests too.
I left a forwarder stub for Preprocessor.py (as strange as it looks, it does work), but I didn't bother leaving one for Expression.py
I didn't touch the indenting, as unpleasant as it is. The only changes I made to the moved scripts were to fix up the imports.
Green try run: https://tbpl.mozilla.org/?tree=Try&rev=63ec57f73dbb
Comment 3•11 years ago
|
||
Comment on attachment 828649 [details] [diff] [review]
Move preprocessor into mozbuild
Review of attachment 828649 [details] [diff] [review]:
-----------------------------------------------------------------
Could we make Preprocessor.py not require mozbuild to be in it's import path? i.e., make that hack the sys.path itself?
I'm suprised to see the removal of Expression.py in the patch explicitly, I had assumed that hg mv doesn't do that?
I don't expect to actually miss Expression.py in its current location, fwiw.
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #3)
> Could we make Preprocessor.py not require mozbuild to be in it's import
> path? i.e., make that hack the sys.path itself?
That sounds like a good idea. Should be simple enough to add (though I'll wait for any comments Greg has first).
> I'm suprised to see the removal of Expression.py in the patch explicitly, I
> had assumed that hg mv doesn't do that?
I hg mv'ed config/Expression.py, but there's an identical copy in js/src/config/Expression.py that I hg rm'ed. The joys of check-sync-dirs ;)
Comment 5•11 years ago
|
||
Ah, right. Which makes me wonder, are there policies for dependencies of js/src/config on mozbuild?
Updated•11 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Updated•11 years ago
|
Attachment #828649 -
Flags: review?(gps)
Comment 7•11 years ago
|
||
(In reply to Axel Hecht [:Pike] from comment #5)
> Ah, right. Which makes me wonder, are there policies for dependencies of
> js/src/config on mozbuild?
FWIW, js/src already doesn't build without mozbuild and without the virtualenv.
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•