If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Set up Intellij JavaScript config on `mach gradle-install`

NEW
Assigned to

Status

()

Firefox for Android
Build Config & IDE Support
2 years ago
2 years ago

People

(Reporter: mcomella, Assigned: nalexander)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.
(Assignee)

Comment 2

2 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

2 years ago
Created attachment 8651997 [details]
MozReview Request: Bug 1183335 - Install .idea and *.iml during |mach gradle-install|. r?mcomella,sebastian

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

2 years ago
Assignee: nobody → nalexander
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.
(Assignee)

Comment 6

2 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

2 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

2 years ago
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)
You need to log in before you can comment on or make changes to this bug.