flutter-intellij icon indicating copy to clipboard operation
flutter-intellij copied to clipboard

Support multi-platform builds

Open firephreek opened this issue 3 years ago • 8 comments

This is a refactor of the flutter-intellij build tool to support multiple platforms including Mac and Windows. Several other bugs and changes were included to streamline and speed up the build process as well. Additionally, a new target, 'artifacts' has been added to aid in identifying needed downloads.

Summary of changes

  • 'unpack' cli arg is moved up used in Setup and Build commands to determine if artifacts should be unpacked again.
  • Artifacts are now unpacked to version tagged directories. This cuts down on disk I/O on repeated or bulk builds and should speed up the process.
  • Artifacts are unpacked using dart:archive package instead of relying on local tar and zip commands.
  • gradle.properties updated to provide IDEA version to builds and also fix a bug with the name property used for determining the plugin name.
  • Housekeeping: more comments, renamed variables, Green text for separators and red text for errors in log output.
  • plugin.bat file created for invocation on Windows platforms.
  • More specific changes/info can be found in commit message history.

Addresses: https://github.com/flutter/flutter-intellij/issues/6052

Pre-launch Checklist

  • [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
  • [x] I read the Flutter Style Guide recently, and have followed its advice.
  • [x] I signed the CLA.
  • [x] I listed at least one issue that this PR fixes in the description above.
  • [x] I updated/added relevant documentation (doc comments with ///).
  • [x] I added new tests to check the change I am making, or this PR is test-exempt.
  • [x] All existing and new tests are passing.

firephreek avatar Sep 16 '22 17:09 firephreek

Picks up from the old PR and goes a bit deeper. This was needed to fully address the multi-platform build, but also some other code smells that I found. Wanted to get this out front for discussion before I put too much more work into it. I realize there might be some edge cases that are used internally for deployment that may have been missed.

firephreek avatar Sep 16 '22 17:09 firephreek

Thanks for getting back to this. I'm going to patch it in and give it a workout :)

stevemessick avatar Sep 19 '22 22:09 stevemessick

The first thing I noticed is the version of the Flutter plugin needs to be changed for the 2022.2 version. It looks like JetBrains removed version 222.3244.4. If you could change that to 222.4167.21 in `product-matrix.json' then the 2022.2 version would probably build. I uploaded the new version to our cloud cache.

It also looks like there is a problem with unit tests. See the presubmit for UNIT_TEST_BOT for details. Do unit tests run locally for you?

stevemessick avatar Sep 20 '22 16:09 stevemessick

Yea, saw the version change, fixed it locally, but didn't want to push it out until you had a chance to see if the other stuff I did made sense.

There are some failing tests locally, going to start working on those. The one I noticed was b/c of a change in the logic I introduced: If an artifact can't be downloaded/located, the tool now fails immediately w/o trying to build anything. The old method relied on system exit codes to determine correctness, but now we're (potentially) moving in a direction of not needing that and being able to bail easier.

A couple other thoughts/changes that I have on a different branch and might suggest (in a different PR): Passing gradle properties via the wrapper instead of constantly re-writing the root gradle.properties file. If that file is edited while the project is loaded in an IDE, it will often cause the project to reload which can cause config/settings to be lost. Also running the runIDE target against Gradle doesn't identify which IDE to run, it just uses the current gradle.properties file, so you have to 'know' to force a re-build/re-gen of that file with the appropriate target.

Also, I noticed a version tag in the tool, I'll see about updating that as well so anyone coming in for a historical perspective will have one more clue to use.

firephreek avatar Sep 21 '22 15:09 firephreek

Yep, both additional topics you mentioned have been a problem. I added the setup command to deal with the second, but you still had to remember to do it, so I changed make to run setup automatically. That's kinda slow, of course. With the versioned artifact dirs it will probably not be so slow. Avoiding the rewrite of the gradle.properties file would be great -- I've trashed my dev project at least once due to that, haha.

stevemessick avatar Sep 21 '22 16:09 stevemessick

I just committed changes that cause a merge conflict for this PR. The changes in product-matrix.json should fix your 2022.2 build problem. The other change was to adapt the artifact downloader to allow for Android Studio file names that no longer include "-ide" in the file name.

stevemessick avatar Sep 22 '22 16:09 stevemessick

I've run into a few problems:

  • On macOS, the Android Studio zip file may unpack into a single directory. The content of that directory needs to be moved up one level. That is, after unpacking, android-studio-2021.1.8 contains a single directory and its content needs to be moved into android-studio-2021.1.8. I think I had included a check for the number of files in the directory and, if just one, assume it needs to have its content moved up.
  • bin/plugin make should ignore the setup channel. It should always do stable and do dev when the option is passed (I have not checked dev so that may be working).
  • The Dart archive package does not handle macOS *.dmg files. It's probably better if we do not download them, but (while working on the setup channel) it did grab ideaIC-2022.1.dmg.
  • When I ran bin/plugin test I noticed it downloaded both android-studio-2022.1.1.6-mac.zip and ideaIC-2022.1.dmg. I think it should only need one of them. Generally, if we're compiling for Android Studio, we're also compiling for IntelliJ. A single distribution file works for both platforms.
  • It isn't clear why we need the platform command line arg. The distribution *.jar files work on any platform. If we're running anything, it is for unit tests, and that uses the current platform.

Anyway, looks very promising. I like your refactoring and general clean up.

stevemessick avatar Sep 22 '22 18:09 stevemessick

Yea, caught the product-matrix.json issue. I'd moved my local to 2022.2.2, figured 'latest and greatest', but already resolved conflicts to your spec.

I had tried to put in some logic to resolve the artifact unpacking issue you mentioned. Old logic relied on the 'bareArchive' param and I thought I had some working checks, but I'll circle back to verify it on my mac.

I had shimmed the platform arg in during some early testing work and it became useful to more explicitly identify which android-studio/IDEA artifacts needed to be downloaded which I figured could be helpful for users working in cross-platform environments to do more explicit builds/tests. e.g. Windows + WSL or Mac ARM/x86 (eventually).

I'll dig into the other issues you mentioned and get back to you.

firephreek avatar Sep 23 '22 15:09 firephreek

Is there any reason that builds on Mac explicitly need the Mac versions of IDEA/Android Studio vs. using the Linux versions? On Windows, if you don't have the Windows versions, compilation fails due to classpaths referencing lib/ffmpeg..-linux...So we might be able to mitigate the dmg issue entirely by short-circuiting the new platform arg to default to Linux when building on a mac.

I have a fix coming for the nested artifact folder issue where we just identify the lib folder and strip out everything before that on each file in the archive when we unpack them.

Thoughts? (At your convenience)

firephreek avatar Sep 26 '22 16:09 firephreek

Using my Mac, I can build distro files using the Linux version of IJ/AS. To run unit tests I need to have the Mac version unpacked in the artifacts directory. I use the setup command to ensure the correct one is present. I don't know why the Windows build would fail due to having Linux files. It's been a while, but I did build using Windows but I had git-bash installed, so all my Linux commands worked. Still, if it was trying to build Linux files, that would have failed, I think.

But yeah, using the Linux platform during make (or build) on a Mac should work. The setup command would need to unpack a Mac version to run the debugger or unit tests.

stevemessick avatar Sep 26 '22 17:09 stevemessick

PR #6322 is going to cause a lot of merge conflicts. Sorry about that. I expect resolution to be straight-forward but tedious.

stevemessick avatar Sep 28 '22 17:09 stevemessick

I haven't abandoned this, just have some deadlines that have been impeded by what has now become actual, detrimental behavior/performance of the flutter/dart tool stack. 😥 I'll keep at this when I can and will resolve any conflicts as appropriate.

firephreek avatar Oct 07 '22 15:10 firephreek

@firephreek Is this still on your radar? (Completely understood if you don't have the bandwidth to get back to it.)

Hixie avatar Jan 31 '23 23:01 Hixie

@firephreek I'm going to close this PR for now to get it off our review queue but please don't hesitate to resume work if you are interested in doing so! Your contributions are definitely welcome and appreciated!

Hixie avatar Apr 25 '23 23:04 Hixie