Closed
Bug 1107811
Opened 10 years ago
Closed 9 years ago
Normalize mobile/android/base/**/*.java to a standard Java location
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Firefox Build System
Android Studio and Gradle Integration
All
Android
Tracking
(firefox45 fixed)
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: nalexander, Assigned: nalexander)
References
Details
Attachments
(2 files, 1 obsolete file)
40 bytes,
text/x-review-board-request
|
mfinkle
:
review+
ritu
:
approval-mozilla-beta-
|
Details |
40 bytes,
text/x-review-board-request
|
mfinkle
:
review+
|
Details |
The Fennec front-end team discussed this in some depth at the Portland All Hands, and we have reached consensus that moving towards a "standard" Java layout for the project is a Good Thing. The principal motivation is to improve our IDE support (principally Gradle/IntelliJ), but we also anticipate (related) that it will help new contributors with an Android/Java background comprehend the project faster. Moving to a standard layour (there are multiple, unfortunately) is also likely to make the *next* tool easier to integrate. Gradle is the current and expected future Android standard build tool; the Gradle standard layout looks like build.gradle src/main/{java,res} src/androidTest/{java,res} This ticket establishes the src/main/java directory. The Java standard is for files to have full package paths, like: src/main/java/org/mozilla/gecko/Class.java (corresponding to class org.mozilla.gecko.Class.java).
Assignee | ||
Comment 1•10 years ago
|
||
Updated•10 years ago
|
Attachment #8532542 -
Flags: review-
Updated•10 years ago
|
Component: Build Config → Build Config & IDE Support
Product: Firefox → Firefox for Android
Comment 2•10 years ago
|
||
Brad: I'd like to understand your concerns here, beyond an r-! I think there's strong impetus to improve our IDE and build support and to go with the Android flow, both for the velocity of the team and to help onboard new contributors. What are you concerned about, and what can we do to address those concerns so that build folks and developer services can keep moving?
Flags: needinfo?(blassey.bugs)
Comment 3•10 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #2) > Brad: I'd like to understand your concerns here, beyond an r-! > > I think there's strong impetus to improve our IDE and build support and to > go with the Android flow, both for the velocity of the team and to help > onboard new contributors. > > What are you concerned about, and what can we do to address those concerns > so that build folks and developer services can keep moving? I don't see any value here. What are the drawbacks of the sym-linked directory structure for IDE support?
Flags: needinfo?(blassey.bugs)
Comment 4•10 years ago
|
||
I understand that you don't see any value in this. You have an existing setup that works, you're not a new contributor, you don't want to use an IDE, and you're not particularly interested in improving the build. Here's my understanding of what this buys us. 1. This isn't just about IDEs; it's also about build system improvements. We want faster builds. We want fine-grained Google Play Services libs (Bug 1115004) to get smaller APKs. We want fine-grained third-party JARs for incremental builds (Bug 1108881). We want pinned versions (Bug 1108782). And so on. I trust Nick when he says that Gradle is the way forward for doing this; after all, he owns this part of the build system, and he's done more research into it than anyone else. 2. But OMG IDEs. About 50% of the large-scale bugs and refactors I've worked on in the past year, and maybe 80% or more of the concurrency issues I've investigated, have only been tractable because of IDE support. This is a multiplier, and I want our team to have an easy time using it. 3. Tools like Gradle (and IDEs for Java developers) shouldn't be second-class citizens. The build system for Fennec will be switching to Gradle, which prefers the standard Java folder hierarchy. It doesn't make sense to keep cobbling together IDE support on top of, and alongside, Makefiles by using hacks like generating directory structures elsewhere. It's 2015; IDEs and modern build tools are necessary. The more steps we introduce between getting a checkout and having a working IDE project, and the more we impede transitions to better build tools, the harder life is for both jobbing front-end devs and new contributors, let alone the poor sods who have to maintain our build system. 4. Let's flip your question around. What's the advantage of using a non-standard directory structure? At the cost of making IDE integration, build tooling, and contributor onboarding harder, we save a few characters in pathnames. (If your shell won't do tab completion when you type "m/a/s/o/m/g/GeckoApp", you need a better shell.) Yes, this transition will make life slightly less pleasant for existing text editor users (unless it improves the editor's jump-to-file functionality; it would for Vim). It will trivially break mxr links -- but then, mxr is going away this quarter anyway, and I'm confident we could ask gps to make dxr follow `hg mv`s. But overall, anything we can do to help the build folks deliver what they want to deliver in 2015 sounds like a win to me. Perhaps Nick can chime in with any context that I missed, or clear up anything I got wrong.
Comment 5•10 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #4) > I understand that you don't see any value in this. You have an existing > setup that works, you're not a new contributor, you don't want to use an > IDE, and you're not particularly interested in improving the build. > > Here's my understanding of what this buys us. > > > 1. This isn't just about IDEs; it's also about build system improvements. We > want faster builds. We want fine-grained Google Play Services libs (Bug > 1115004) to get smaller APKs. We want fine-grained third-party JARs for > incremental builds (Bug 1108881). We want pinned versions (Bug 1108782). And > so on. I trust Nick when he says that Gradle is the way forward for doing > this; after all, he owns this part of the build system, and he's done more > research into it than anyone else. This argument boils down to "I trust Nick", which is fine, but I hope you can see isn't very persuasive. > > > 2. But OMG IDEs. About 50% of the large-scale bugs and refactors I've worked > on in the past year, and maybe 80% or more of the concurrency issues I've > investigated, have only been tractable because of IDE support. This is a > multiplier, and I want our team to have an easy time using it. Again, this is a strawman. We have IDE support. > > > 3. Tools like Gradle (and IDEs for Java developers) shouldn't be > second-class citizens. > > The build system for Fennec will be switching to Gradle, which prefers the > standard Java folder hierarchy. It doesn't make sense to keep cobbling > together IDE support on top of, and alongside, Makefiles by using hacks like > generating directory structures elsewhere. It's 2015; IDEs and modern build > tools are necessary. I don't take it as a given that we should switch to Gradle, which forms the basis of this argument. > > The more steps we introduce between getting a checkout and having a working > IDE project, and the more we impede transitions to better build tools, the > harder life is for both jobbing front-end devs and new contributors, let > alone the poor sods who have to maintain our build system. > > > 4. Let's flip your question around. What's the advantage of using a > non-standard directory structure? I don't want to flip my question around. It has two advantages, the fist it that its how it is currently, so that wins by default. The second is that it is Mozilla standard.
Comment 6•10 years ago
|
||
(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #5) > This argument boils down to "I trust Nick", which is fine, but I hope you > can see isn't very persuasive. Also I want to enable Nick to achieve his goals, which is a broader point. (I'm not unfamiliar with the research and also the current state of affairs, so this isn't blind trust, FWIW.) > Again, this is a strawman. We have IDE support. We have flaky IDE support that requires work to maintain. This is my "second class citizen" argument. > I don't take it as a given that we should switch to Gradle, which forms the > basis of this argument. What's your argument that we shouldn't? > I don't want to flip my question around. It has two advantages, the fist it > that its how it is currently, so that wins by default. The second is that it > is Mozilla standard. Firstly, it's not a Mozilla standard. mobile/android/base is the only oddity. mozStumbler uses a Java directory structure. WebRTC does. Our thirdparty code does. The Search Activity does. http://mxr.mozilla.org/mozilla-central/find?text=&string=org%2F Secondly, "Mozilla standard" is totally meaningless. Literally the entire world disagrees, and Mozilla is not a thought leader here. Seriously, this looks like old-guard "ZOMG CHANGE" to me. What's wrong with moving files that were put in a non-standard place to start with, when the only benefit to leaving them there is inertia?
Comment 7•10 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #6) > Seriously, this looks like old-guard "ZOMG CHANGE" to me. Nope and if you want to resort to this sort dismissivemess, we can end this discuss here. This is a resistance to change for change sake. In the very least there is pain for doing patch uplifts. WRT Gradle, as I pointed out to mfinkle and Nick in other forums, we had platform specific build dependencies before and found that that was terrible. Could you please answer my question? What is wrong with the sym link directory structure?
Comment 8•10 years ago
|
||
The symlink approach is a bandaid. It sends a signal "the Android developer tools are a secondary concern" and that should not be the case. We should be moving ahead to make the Android code/project work better with existing Android tools. This includes build tools and developer tools. Nick has a summary of some of the benefits of embracing existing tools here: http://www.ncalexander.net/blog/2015/01/05/we-should-build-firefox-for-android-with-an-external-build-system/ Brad, you mention platform specific build dependencies from the past. Those were Windows, OS X and Linux specifics. We are not suggesting a change in that direction. We are working to move a "product" (Fennec) not a "platform".
Assignee | ||
Comment 9•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2e3e2ccc810
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1107811 - Part 1: Move Java code. r?mfinkle,margaret #!/bin/bash set -x pushd mobile/android/base # Can't be present in glob below. rm -rf src/org/mozilla/gecko for i in */ ; do # But is necessary to copy into. mkdir -p src/org/mozilla/gecko hg mv $i src/org/mozilla/gecko ; done hg mv *.java src/org/mozilla/gecko # Move back stuff we didn't want to move. hg mv src/org/mozilla/gecko/crashreporter crashreporter hg mv src/org/mozilla/gecko/docs docs hg mv src/org/mozilla/gecko/locales locales hg mv src/org/mozilla/gecko/resources resources popd
Attachment #8696021 -
Flags: review?(mark.finkle)
Attachment #8696021 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 11•9 years ago
|
||
Bug 1107811 - Part 2: Fix moz.build. r?mfinkle,margaret
Attachment #8696022 -
Flags: review?(mark.finkle)
Attachment #8696022 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 12•9 years ago
|
||
mfinkle, margaret: these patches build locally -- we'll see if I missed anything in the try build. This is a perfect time to land such changes: just before a work week, just before the end of the cycle. We get in fewer folks way and get cheap Aurora uplift. There's a bunch of menial work to update the Gradle configurations which I'll push after interviewing this afternoon.
Assignee | ||
Updated•9 years ago
|
Attachment #8532542 -
Attachment is obsolete: true
Comment 13•9 years ago
|
||
Comment on attachment 8696021 [details] MozReview Request: Bug 1107811 - Part 1: Move Java code. r?mfinkle,margaret https://reviewboard.mozilla.org/r/27255/#review24665 Try push looked Green
Attachment #8696021 -
Flags: review?(mark.finkle) → review+
Comment 14•9 years ago
|
||
Comment on attachment 8696022 [details] MozReview Request: Bug 1107811 - Part 2: Fix moz.build. r?mfinkle,margaret https://reviewboard.mozilla.org/r/27257/#review24667
Attachment #8696022 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 15•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/b33e19c498a102dac7d81b08e43fa2ab9c9293dc Bug 1107811 - Part 1: Move Java code. r=mfinkle https://hg.mozilla.org/integration/fx-team/rev/fe2727dce60dd69c15d20221c15adf3c846fde67 Bug 1107811 - Part 2: Fix moz.build. r=mfinkle https://hg.mozilla.org/integration/fx-team/rev/2735a465e5582f716c68d6e12546074ab5e0432b Bug 1107811 - Part 3: Fix Gradle configurations. r=me https://hg.mozilla.org/integration/fx-team/rev/7fb27cc3a41695df71c52db2bf093fdf8cdb43be Bug 1107811 - Part 4: Exclude all but mobile/. r=me
Assignee | ||
Updated•9 years ago
|
Summary: Move mobile/android/base/**/*.java to a standard Java location → Normalize mobile/android/base/**/*.java to a standard Java location
Comment 16•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b33e19c498a1 https://hg.mozilla.org/mozilla-central/rev/fe2727dce60d https://hg.mozilla.org/mozilla-central/rev/2735a465e558 https://hg.mozilla.org/mozilla-central/rev/7fb27cc3a416
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Updated•9 years ago
|
Attachment #8696021 -
Flags: review?(margaret.leibovic)
Updated•9 years ago
|
Attachment #8696022 -
Flags: review?(margaret.leibovic)
Updated•9 years ago
|
Blocks: 1231667
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8696021 [details] MozReview Request: Bug 1107811 - Part 1: Move Java code. r?mfinkle,margaret Approval Request Comment [Feature/regressing bug #]: none. This is purely organizational -- it just makes local development better. Unfortunately, it makes uplifting across the file renames error prone (and we've really seen errors, like https://bugzilla.mozilla.org/show_bug.cgi?id=1232161. Uplifting this will make future uplifts to beta easier. I can prepare the branch patch, since this probably won't apply cleanly. [User impact if declined]: none. [Describe test coverage new/current, TreeHerder]: build-only, on Nightly and Aurora now. [Risks and why]: very low. [String/UUID change made/needed]: none.
Attachment #8696021 -
Flags: approval-mozilla-beta?
Comment on attachment 8696021 [details] MozReview Request: Bug 1107811 - Part 1: Move Java code. r?mfinkle,margaret This seems to big a change to me though I agree that it has value. I would prefer it right the trains. Please.
Attachment #8696021 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Updated•5 years ago
|
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 45 → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•