1107811 - Normalize mobile/android/base/**/*.java to a standard Java location

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Nick Alexander :nalexander [he/him]]

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).

Comment 1

[Nick Alexander :nalexander [he/him]]

Attachment: move_the_world.patch

Comment 2

[Richard Newman [:rnewman]]

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?

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

(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?

Comment 4

[Richard Newman [:rnewman]]

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

[Brad Lassey [:blassey] (use needinfo?)]

(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

[Richard Newman [:rnewman]]

(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

[Brad Lassey [:blassey] (use needinfo?)]

(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

[Mark Finkle (:mfinkle) (use needinfo?)]

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".

Comment 9

[Nick Alexander :nalexander [he/him]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2e3e2ccc810

Comment 10

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1107811 - Part 1: Move Java code. r?mfinkle,margaret

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

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1107811 - Part 2: Fix moz.build. r?mfinkle,margaret

Bug 1107811 - Part 2: Fix moz.build. r?mfinkle,margaret

Comment 12

[Nick Alexander :nalexander [he/him]]

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.

Comment 13

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8696022 [details]
MozReview Request: Bug 1107811 - Part 2: Fix moz.build. r?mfinkle,margaret

https://reviewboard.mozilla.org/r/27257/#review24667

Comment 15

[Nick Alexander :nalexander [he/him]]

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

Comment 16

[Carsten Book [:Tomcat]]

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

Comment 17

[Nick Alexander :nalexander [he/him]]

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.

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.