Closed
Bug 1346025
Opened 7 years ago
Closed 7 years ago
Move vendored python modules from /python to /third_party/python
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(2 files, 3 obsolete files)
Not sure where else to file this bug. Currently vendored python modules are included under /python. They are mixed in freely with in-house modules, which makes it tricky to tell what is and isn't ok to modify. Vendored rust crates have been going into /third_party/rust, we should follow their lead and put our vendored python modules there too.
Comment 1•7 years ago
|
||
Not answering this issue itself but I have been maintaining this list: https://hg.mozilla.org/mozilla-central/file/tip/tools/rewriting/ThirdPartyPaths.txt which also includes Python thirdparty libs.
Comment 2•7 years ago
|
||
This is a totally sensible plan.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8870812 [details] Bug 1346025 - Split vendored modules in python/moz.build to third_party/python/moz.build, https://reviewboard.mozilla.org/r/142378/#review146464 ::: build/virtualenv_packages.txt:6 (Diff revision 1) > mozilla.pth:python/mozboot > mozilla.pth:python/mozbuild > mozilla.pth:python/mozlint > mozilla.pth:python/mozversioncontrol > -mozilla.pth:python/blessings > mozilla.pth:python/compare-locales Depending on the strict definition of "vendored", compare-locales probably also belongs into the vendored bucket. Its upstream repo is hg.m.o/l10n/compare-locales, and I only occasionally deploy a release here to be used in the builds.
Assignee | ||
Comment 8•7 years ago
|
||
Thanks, I'll add it. Yeah, I'd define "vendored" as things whose canonical source lives in another repo (whether Mozilla owned or not).
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment 13•7 years ago
|
||
I don't agree with moving the l10n docs to third-party, they're original m-c content. PS: those docs will get edits soon as part of bug 1165906.
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8870528 [details] Bug 1346025 - Move vendored python modules from /python to /third_party/python, https://reviewboard.mozilla.org/r/141980/#review149832
Attachment #8870528 -
Flags: review?(ted) → review+
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8870812 [details] Bug 1346025 - Split vendored modules in python/moz.build to third_party/python/moz.build, https://reviewboard.mozilla.org/r/142378/#review149834
Attachment #8870812 -
Flags: review?(ted) → review+
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8871711 [details] Bug 1346025 - Merge 'compare-locales' mach command with python/mach_commands.py, https://reviewboard.mozilla.org/r/143198/#review149836
Attachment #8871711 -
Flags: review?(ted) → review+
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8871712 [details] Bug 1346025 - Update references to moved python modules, https://reviewboard.mozilla.org/r/143200/#review149838 ::: testing/mozharness/configs/builds/taskcluster_firefox_win32_clang.py:28 (Diff revision 1) > ], > 'exes': { > 'virtualenv': [ > sys.executable, > os.path.join( > - os.getcwd(), 'build', 'src', 'python', 'virtualenv', 'virtualenv.py' > + os.getcwd(), 'build', 'src', 'third_party', 'python', 'virtualenv', 'virtualenv.py' Can you file a followup on getting rid of the duplication here? ::: tools/rewriting/ThirdPartyPaths.txt:70 (Diff revision 1) > security/nss/ > security/sandbox/chromium/ > testing/gtest/gmock/ > testing/gtest/gtest/ > testing/talos/talos/tests/dromaeo/ > +third_party/python/blessings/ Might be worth filing a follow-up to change whatever consumes this file to blanket-ignore all of third_party.
Attachment #8871712 -
Flags: review?(ted) → review+
Comment 18•7 years ago
|
||
I assume you're going to squash these for landing, since they aren't really separable in practice? (Thanks for splitting them out for review though, that made it much easier!)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8871711 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8870812 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8871712 -
Attachment is obsolete: true
Assignee | ||
Comment 21•7 years ago
|
||
I squashed the latter 3 commits (no further review required on your part ted!). Axel, I moved the compare-locales docs and mach command to tools/compare-locales. I've tested the docs still get generated as part of the in-tree docs, and the mach command seems to work.
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8874844 [details] Bug 1346025 - Move 'python/compare-locales' documentation and mach command to 'tools/compare-locales', https://reviewboard.mozilla.org/r/146236/#review150614 Thanks, r=me.
Attachment #8874844 -
Flags: review?(l10n) → review+
Assignee | ||
Comment 23•7 years ago
|
||
I wasn't really sure what subtle ways this might break something, so here's a fairly comprehensive and good looking try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ca90e4a7d30d96ac34e1e303d94a54e201f342a
Comment 24•7 years ago
|
||
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5413302a24e6 Move 'python/compare-locales' documentation and mach command to 'tools/compare-locales', r=Pike https://hg.mozilla.org/integration/autoland/rev/34b11112f0e3 Move vendored python modules from /python to /third_party/python, r=ted
Comment 25•7 years ago
|
||
backed out the /python to third_party part for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=105174629&repo=autoland
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 26•7 years ago
|
||
This was clearly working in yesterday's try run. I *think* I might just need to touch the CLOBBER file, but I'm not sure how to test whether that would actually fix it or not (since try always does a clobber).
Flags: needinfo?(ahalberstadt)
Comment 27•7 years ago
|
||
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8e6503872bec Backed out changeset 34b11112f0e3 for bustage
Assignee | ||
Comment 28•7 years ago
|
||
Ah, I found: https://api.pub.build.mozilla.org/clobberer/ After clobbering from there, re-triggering the failed builds seemed to get past the ImportError: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=34b11112f0e3b342c149295ea8e8eac01cdf646c&filter-searchStr=os%20x%2010.7%20opt One of them still failed hitting the taskcluster rest API.. I'm not sure why, probably unrelated? To be safe I also reproduced this on a local Windows build (hit the ImportError, clobber, rebuild, fixed). So I'll push up a new patch that touches CLOBBER.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 31•7 years ago
|
||
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8874844 -
Attachment is obsolete: true
Comment 33•7 years ago
|
||
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ff64a7889c1f Move vendored python modules from /python to /third_party/python, r=ted
Assignee | ||
Comment 34•7 years ago
|
||
Comment on attachment 8874844 [details] Bug 1346025 - Move 'python/compare-locales' documentation and mach command to 'tools/compare-locales', Had to push just the single review up to reviewboard to appease autoland (since only the second patch got backed out).
Attachment #8874844 -
Attachment is obsolete: false
Comment 35•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5413302a24e6 https://hg.mozilla.org/mozilla-central/rev/ff64a7889c1f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 36•7 years ago
|
||
Bravo, well done!
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•