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

RESOLVED FIXED in Firefox 45

Status

()

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

unspecified
Firefox 45
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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).
Posted patch move_the_world.patch (obsolete) — Splinter Review
Component: Build Config → Build Config & IDE Support
Product: Firefox → Firefox for Android
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)
(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)
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.
Depends on: 924738, 890586
(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.
(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?
(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?
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".
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)
Bug 1107811 - Part 2: Fix moz.build. r?mfinkle,margaret
Attachment #8696022 - Flags: review?(mark.finkle)
Attachment #8696022 - Flags: review?(margaret.leibovic)
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.
Attachment #8532542 - Attachment is obsolete: true
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 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+
Summary: Move mobile/android/base/**/*.java to a standard Java location → Normalize mobile/android/base/**/*.java to a standard Java location
Blocks: 1230848

Updated

4 years ago
Attachment #8696021 - Flags: review?(margaret.leibovic)

Updated

4 years ago
Attachment #8696022 - Flags: review?(margaret.leibovic)
No longer blocks: 1231588
Depends on: 1231588
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-
You need to log in before you can comment on or make changes to this bug.