Closed Bug 924343 Opened 8 years ago Closed 8 years ago

Move Preprocessor.py into mozbuild

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 935305

People

(Reporter: gps, Assigned: bokeefe)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
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
Blocks: 935987
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
Assignee: nobody → bokeefe
Status: NEW → ASSIGNED
Attachment #828649 - Flags: review?(gps)
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.
(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 ;)
Ah, right. Which makes me wonder, are there policies for dependencies of js/src/config on mozbuild?
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 935305
Attachment #828649 - Flags: review?(gps)
(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.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.