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)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: mcomella, Assigned: nalexander)

References

Details

Attachments

(1 file)

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)
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.
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)
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)
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)
Attachment #8651997 - Flags: review?(s.kaspari)
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.
(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)
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)
sebastian: very low priority, but could you tell me what IntelliJ settings you tend to customize?
Flags: needinfo?(s.kaspari)
(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)
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
Product: Firefox for Android → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: