Move third party sources outside of base

RESOLVED FIXED in Firefox 27

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: bnicholson, Assigned: bnicholson)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 27
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Having all third party source directly under base (which is generally o.m.g.) is not IDE-friendly, so we should move them outside. This overlaps with bug 890586, but is less ambitious.

Here are the shell commands for all file moves, as requested by rnewman:
git mv httpclientandroidlib/ ../thirdparty/
git mv json-simple/ ../thirdparty/
git mv apache/ ../thirdparty/
git mv braille/ ../thirdparty/
git mv websockets/ ../thirdparty/

Thanks in advance for nalexander or rnewman for syncing these changes with the services branch :)
Blocks: 890586
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
Created attachment 814711 [details] [diff] [review]
Move third party sources outside of base
Attachment #814711 - Flags: review?(rnewman)
(Assignee)

Updated

5 years ago
See Also: → bug 890586
(Assignee)

Comment 2

5 years ago
Oops, dependency works too.
See Also: bug 890586
(Assignee)

Comment 3

5 years ago
Created attachment 814713 [details] [diff] [review]
Use package hierarchy for third party libs

I was going to keep this bug as simple as possible, but we want this change too if we really want to make IDE integration painless.

cd thirdparty/
git mv braille/com/ .
rm -r braille/
mkdir -p org/mozilla
git mv apache/ org/mozilla/
mkdir -p ch/boye
git mv httpclientandroidlib/ ch/boye/
mkdir -p com/codebutler
git mv websockets/ com/codebutler/android_websockets
mkdir org/json
git mv json-simple/ org/json/simple
Attachment #814713 - Flags: review?(rnewman)
Blocks: 924961
In principle, I support this.  And, to my mind, in for a penny, in for a pound: we should follow Java's directory structure everywhere.

I have a few nits with the build system integration, and I'd kind of prefer to land Bug 923306 before doing this rename the world operation, but f+ from me.
Let's wait for the build bug to land before we do this, 'cos I think this is easier to reconstitute. Work for you, Brian?
Depends on: 923306
(Assignee)

Comment 6

5 years ago
WFM
(Assignee)

Comment 7

5 years ago
Created attachment 817379 [details] [diff] [review]
0001-Bug-924738-Move-third-party-sources-outside-of-base.patch, v2

Rebased
Attachment #814711 - Attachment is obsolete: true
Attachment #814711 - Flags: review?(rnewman)
Attachment #817379 - Flags: review?(rnewman)
(Assignee)

Comment 8

5 years ago
Created attachment 817380 [details] [diff] [review]
0002-Bug-924738-Use-package-hierarchy-for-third-party-lib.patch, v2

Rebased
Attachment #814713 - Attachment is obsolete: true
Attachment #814713 - Flags: review?(rnewman)
Attachment #817380 - Flags: review?(rnewman)
Comment on attachment 817380 [details] [diff] [review]
0002-Bug-924738-Use-package-hierarchy-for-third-party-lib.patch, v2

These look fine to me, but needs build peer review.
Attachment #817380 - Flags: review?(rnewman)
Attachment #817380 - Flags: review?(gps)
Attachment #817380 - Flags: review+
Comment on attachment 817379 [details] [diff] [review]
0001-Bug-924738-Move-third-party-sources-outside-of-base.patch, v2

Fold these together?
Attachment #817379 - Flags: review?(rnewman)
Created attachment 817555 [details] [diff] [review]
Part 3

This updates java-third-party-sources.mn, which isn't currently used in the build.
Attachment #817555 - Flags: review+
Upstream copy script change:

https://github.com/mozilla-services/android-sync/pull/365

Verified that with the three hg parts applied, this is a no-op when run.
Comment on attachment 817380 [details] [diff] [review]
0002-Bug-924738-Use-package-hierarchy-for-third-party-lib.patch, v2

Review of attachment 817380 [details] [diff] [review]:
-----------------------------------------------------------------

IMO this counts as trivial and didn't require build peer review.
Attachment #817380 - Flags: review?(gps) → review+
(Assignee)

Comment 14

5 years ago
Nick wants to wait for bug 925185 before we land this one.
Depends on: 925185
https://hg.mozilla.org/mozilla-central/rev/070ebdc4a8e4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Blocks: 1107811
You need to log in before you can comment on or make changes to this bug.