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)

enhancement
Not set
normal

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.
Blocks: 1346026
Blocks: 1367092
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.
This is a totally sensible plan.
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.
Thanks, I'll add it. Yeah, I'd define "vendored" as things whose canonical source lives in another repo (whether Mozilla owned or not).
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
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 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 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 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 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+
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!)
Attachment #8871711 - Attachment is obsolete: true
Attachment #8870812 - Attachment is obsolete: true
Attachment #8871712 - Attachment is obsolete: true
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 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+
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
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
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)
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)
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e6503872bec
Backed out changeset 34b11112f0e3 for bustage
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.
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.
Attachment #8874844 - Attachment is obsolete: true
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
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
https://hg.mozilla.org/mozilla-central/rev/5413302a24e6
https://hg.mozilla.org/mozilla-central/rev/ff64a7889c1f
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Bravo, well done!
Depends on: 1371711
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.