Closed
Bug 1183335
Opened 9 years ago
Closed 7 years ago
Set up Intellij JavaScript config on `mach gradle-install`
Categories
(Firefox Build System :: Android Studio and Gradle Integration, defect)
Firefox Build System
Android Studio and Gradle Integration
All
Android
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: mcomella, Assigned: nalexander)
References
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Details |
via [1]:
On Wed, Jul 8, 2015 at 5:47 AM, Sebastian Kaspari <s.kaspari at gmail.com>
wrote:
> Unfortunately some of these settings seem to get lost after a clobber. Has
> anyone been lucky with defining these settings more globally?
>
This is a weakness of the current approach. If we figured out which XML
file in $OBJDIR/mobile/android/gradle/.idea owned the settings, we could
install that file as part of |mach gradle-install|. I don't think I have a
ticket for this, and I don't have time to investigate now. Please file and
investigate.
---
There are three things to consider (via [2]):
1) Enable the JavaScript IDEA plugin - my assumption is this a global IDE config and we can't do this.
2) Set JS language level. I found this in workspace.xml:
<component name="PropertiesComponent">
...
<property name="JavaScriptLanguageLevel" value="ES6" />
3) Register *.jsm as JavaScript - this seems like it may also be a global IDE config.
NI Nick for action items.
[1]: https://mail.mozilla.org/pipermail/mobile-firefox-dev/2015-July/001333.html
[2]: https://mail.mozilla.org/pipermail/mobile-firefox-dev/2015-July/001317.html
Flags: needinfo?(nalexander)
Comment 1•9 years ago
|
||
This is a good start for shared configurations. I hope in the future we find a way to support personal preferences as well. After every clobber I loose some of my settings (logcat colors, window/panel layout, order methods alphabetical in "structure" view) and they are not something that work for everyone.
Assignee | ||
Comment 2•9 years ago
|
||
Part of the technical problem here should be addressed by Bug 1176133, which lets the build system add and remove things from the gradle/.idea folder safely. (Right now, we can't *remove* things, meaning CLOBBER-like problems are possible or even likely.)
A little investigation suggests that workspace.xml should /never/ be checked in to version control. However, $OBJDIR/mobile/android/gradle/.idea/jsLibraryMappings.xml sets the version and could be checked in; that might address part of this. Lots of investigation needed.
Depends on: 1176133
Flags: needinfo?(nalexander)
Assignee | ||
Comment 3•9 years ago
|
||
Bug 1183335 - Install .idea and *.iml during |mach gradle-install|. r?mcomella,sebastian
This makes it possible to "Open" $OBJDIR/mobile/android/gradle (after
|mach build|) without "Importing" it first. There should be no
references outside of the active object directory. In time, we'll run
this as part of the build configuration and remove |mach
gradle-install| entirely.
It's worth noting that the .idea/libraries directory *cannot* be
committed since it contains developer-machine absolute path (to the
Android SDK sources); this means that there's some flakiness around
building with Gradle in IntelliJ before everything works perfectly.
We may need to recommend using View > Tool Windows > Gradle and
refreshing. Testing wanted!
Additional notes:
* This includes an MPL copyright block and a conflicting Class.java
header. I'll clean that before landing. It's not possible to format
the MPL copyright block exactly as we do know (bonkers, I know!):
IntelliJ won't give us
/* First line ...
it will only give
/*
* First line...
or
// First line...
I opted for the latter. We can mass rewrite if we want to keep this
and care enough.
* This includes an ordering for imports, putting org.mozilla ahead of
everything else ahead of java.* and javax.*. Input wanted.
* It's not possible to turn on ECMAScript 2015 since that is
inexplicably stored in .idea/workspace.xml, which can't be checked
into VCS (or reasonably generated). This is an IntelliJ bug, no two
ways about it. (There are many IntelliJ bugs with respect to
sharing configurations.)
Attachment #8651997 -
Flags: review?(s.kaspari)
Attachment #8651997 -
Flags: review?(michael.l.comella)
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → nalexander
Comment 4•9 years ago
|
||
I pulled the commit to test it locally but now ./mach build throws an error:
> File "./mach", line 30, in load_mach
> return mach_bootstrap.bootstrap(dir_path)
> File "/Users/sebastian/Projects/Mozilla/fx-team/build/mach_bootstrap.py", line 330, in bootstrap
> mach.load_commands_from_file(os.path.join(mozilla_dir, path))
> File "/Users/sebastian/Projects/Mozilla/fx-team/python/mach/mach/main.py", line 258, in load_commands_from_file
> imp.load_source(module_name, path)
> File "/Users/sebastian/Projects/Mozilla/fx-team/mobile/android/mach_commands.py", line 164
> patterns = ('.deps/**', '.gradle/**', 'build/**', '*/build/**'):
Flags: needinfo?(nalexander)
Updated•9 years ago
|
Attachment #8651997 -
Flags: review?(s.kaspari)
Comment 5•9 years ago
|
||
Comment on attachment 8651997 [details]
MozReview Request: Bug 1183335 - Install .idea and *.iml during |mach gradle-install|. r?mcomella,sebastian
https://reviewboard.mozilla.org/r/17039/#review15341
I fixed the error in mach_commands.py and opened the project in IntelliJ (After clobber and build/package). IntelliJ modified a bunch of files and they are now showing up as modified files. I uploaded a diff here: https://pastebin.mozilla.org/8843927
It seems all these changes are related to Java 1.7 vs. Java 1.8 and two additional folders (build/reports, build/test-results).
::: mobile/android/gradle/.idea/.name:1
(Diff revision 1)
> +gradle
This is the project name IntelliJ (and Android Studio) show in the list of projects on startup. It always annoyed me that it just said "gradle" and sometimes I edited it but the name was always lost after clobbering. Can we give the project a more descriptive name now that this lives in the tree? :)
::: mobile/android/gradle/app/app.iml:93
(Diff revision 1)
> + <orderEntry type="jdk" jdkName="Android API 22 Platform" jdkType="Android SDK" />
I assume this will need to be updated whenever we change the SDK? But I guess this will happen automatically as we are all using the IDE.
::: mobile/android/mach_commands.py:164
(Diff revision 1)
> - manifest_path],
> + patterns = ('.deps/**', '.gradle/**', 'build/**', '*/build/**'):
The colon at the end of this line is breaking mach.
Assignee | ||
Comment 6•9 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #5)
> Comment on attachment 8651997 [details]
> MozReview Request: Bug 1183335 - Install .idea and *.iml during |mach
> gradle-install|. r?mcomella,sebastian
>
> https://reviewboard.mozilla.org/r/17039/#review15341
>
> I fixed the error in mach_commands.py and opened the project in IntelliJ
> (After clobber and build/package). IntelliJ modified a bunch of files and
> they are now showing up as modified files. I uploaded a diff here:
> https://pastebin.mozilla.org/8843927
Thanks, this is super valuable.
> It seems all these changes are related to Java 1.7 vs. Java 1.8 and two
> additional folders (build/reports, build/test-results).
We can certainly add the two folders. Not sure about 1.7 vs 1.8; I'll have to read the diff.
> ::: mobile/android/gradle/.idea/.name:1
> (Diff revision 1)
> > +gradle
>
> This is the project name IntelliJ (and Android Studio) show in the list of
> projects on startup. It always annoyed me that it just said "gradle" and
> sometimes I edited it but the name was always lost after clobbering. Can we
> give the project a more descriptive name now that this lives in the tree? :)
It's always bugged me too -- but IIRC IJ sets it based on the Gradle project name, which is always extracted from the path in the tree! If it works to set it (i.e., IJ doesn't fight the power), I'd love to change it to 'fennec' or 'Fennec'.
> ::: mobile/android/gradle/app/app.iml:93
> (Diff revision 1)
> > + <orderEntry type="jdk" jdkName="Android API 22 Platform" jdkType="Android SDK" />
>
> I assume this will need to be updated whenever we change the SDK? But I
> guess this will happen automatically as we are all using the IDE.
I expect so. There's a lot here that I don't understand -- part of the reason I'm trying to rope in co-owners :)
> ::: mobile/android/mach_commands.py:164
> (Diff revision 1)
> > - manifest_path],
> > + patterns = ('.deps/**', '.gradle/**', 'build/**', '*/build/**'):
>
> The colon at the end of this line is breaking mach.
Yeah, sorry, fixed locally and forgot to re-push the commit.
Flags: needinfo?(nalexander)
Reporter | ||
Comment 7•9 years ago
|
||
Comment on attachment 8651997 [details]
MozReview Request: Bug 1183335 - Install .idea and *.iml during |mach gradle-install|. r?mcomella,sebastian
https://reviewboard.mozilla.org/r/17039/#review15913
I skimmed the changed code - looks reasonable, although I don't have much context here.
As far as implementation is concerned, I applied the patch and opened "<objdir>/mobile/android/gradle" in Intellij and it opened. However, the support references appear to be not found (notably before, it would not find some references but now it does not find any). Without this regression, the patch looks good.
Note that omg.R is also not found, but I had this issue before.
Attachment #8651997 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 8•9 years ago
|
||
sebastian: very low priority, but could you tell me what IntelliJ settings you tend to customize?
Flags: needinfo?(s.kaspari)
Comment 9•9 years ago
|
||
(In reply to Nick Alexander :nalexander from comment #8)
> sebastian: very low priority, but could you tell me what IntelliJ settings
> you tend to customize?
For example moving the panels around to match my screen setup, setting logcat colors (even though I mostly use pidcat now), ...
Flags: needinfo?(s.kaspari)
Assignee | ||
Comment 10•7 years ago
|
||
Android Studio doesn't really support JS code and we only support AS these days (although IJ should still work, more or less). Therefore this is no longer something we'd address.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Updated•5 years ago
|
Product: Firefox for Android → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•