Refactor CreateDmg and BundleMacosJdk: use Property<..>, migrate to Kotlin #70

vlsi · 2020-07-16T15:41:07Z

fixes #69

The PR includes the following changes:

Use Gradle's Property API so the tasks support lazily generated inputs. For instance, jdk dependency is downloaded only in case the task is actually executed
Migrate to Kotlin
File.createTempDir is replaced with build directory to better conform with Gradle conventions
README cleanup: added Kotlin DSL samples, fixed that JDK dependency requires .zip rather than .tgz
Added cleanTemporaryDirectories option for debugging purposes (if set to false it would skip removing the temporary directories used for preparing the output artifact)
Added integration test to verify CreateDmg and BundleMacosJdk tasks

vlsi · 2020-07-17T12:32:36Z

The createDmg test passes locally, however it fails in GitHub Actions :(

arch: posix_spawnp: perl5.18: Bad CPU type in executable

arimer · 2020-07-23T06:55:16Z

Hello Vladimir. Thanks for contributing! 🎉
Do you mind adding a short discription to this PR to just sumup your idea. This is mostly useful for documentation purposes and might help others during code review.

vlsi · 2020-07-23T07:59:03Z

@arimer , well, I thought It was summarized in the PR title. I duplicated it to the description as well.

arimer

Thanks a lot for this contribution I have still some open questions and points which are commented on the code.

arimer · 2020-07-23T09:54:35Z

build.gradle.kts

@@ -1,5 +1,6 @@
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
 import java.net.URI
+import org.gradle.api.tasks.testing.logging.TestExceptionFormat



Due the switch to the property API users of this plugins would face problems if they encode a version like 1.4.+ in their project setup.
Therefore I would opt for at least increase the minor verion in L.24.

Maybe @coolya or @sergej-koscejev have an other oppinion.

Frankly speaking, the version is puzzling here, so I need your input here.
Of course, I can bump minor version to 5

arimer · 2020-07-23T09:55:17Z

src/main/kotlin/de/itemis/mps/gradle/BaseBundleMacOsTask.kt

+import org.gradle.kotlin.dsl.property
+import java.io.File
+
+abstract class BaseBundleMacOsTask(


I really like the idea of having a common super class 👍

src/main/kotlin/de/itemis/mps/gradle/BundleMacosJdk.kt

src/main/kotlin/de/itemis/mps/gradle/CreateDmg.kt

arimer · 2020-07-23T10:07:29Z

src/test/kotlin/de/itemis/mps/gradle/test/CreateDmgTest.kt

@@ -0,0 +1,161 @@
+package de.itemis.mps.gradle.test


I really like the idea having a minimal RCP test which can be executed.
Could you maybe just drop some lines in the readme about the test idea and the setup?
This would be great and could help others in the future to update/enrich the test if required.

I'm puzzled here. Can you please clarify what is not clear?

I'm using a standard approach to use Gradle plugins: org.gradle.testkit.runner.GradleRunner
The test creates Gradle project within a temporary directory and executes a single task there.

sergej-koscejev

I can't give a good review on this because I don't use these tasks, they run on our build server to produce a DMG for mbeddr RCP and either they work properly or nobody seems to care about DMGs. I also don't understand if there's a particular problem that this PR is supposed to solve.

@vlsi do you intend to maintain these tasks in the future? Have you had problems with these tasks?

Otherwise I'm tending against merging this PR because I'd like to avoid fixing what isn't broken.

sergej-koscejev

Sorry, I overlooked that this fixes #69. In that case let's merge this, but please bump version to 1.5 as @arimer mentioned.

…otlin This change bumps the version to 1.5 since it updates APIs for CreateDmg and BundleMacosJdk.

vlsi · 2021-04-18T22:47:25Z

@sergej-koscejev , sorry for the delay. I've rebased the PR and I bumped the version.

Do you think it can be merged?

sergej-koscejev · 2021-04-21T16:56:31Z

I can't merge it because the macOS builds fail. @vlsi any idea why?

vlsi · 2021-04-21T17:06:19Z

macOS builds fail

I think the build fails since macOS scripts are outdated (see #99)
However, I was not sure if that is GitHub's issue or if the scripts went out of date indeed.

vlsi · 2021-04-21T17:19:18Z

So I guess the ways to proceed are:

temporarily skip macOS from build matrix (or mark it to ignore errors)
update macOS scripts

WDYT?

sergej-koscejev · 2021-04-22T06:43:14Z

I'm fine with updating the macOS scripts. @coolya do you have any opinion?

coolya · 2021-04-23T06:54:46Z

I'm fine with updating the macOS scripts. @coolya do you have any opinion?

I'm fine with updating the files.

vlsi · 2021-05-11T13:32:45Z

Well, I did check the scripts from JetBrains, and it looks like they are worse than the scripts in mps-gradle-plugin :((
They use positional arguments instead of named ones, they hard-code certain assumptions (e.g. JetBrains, .png suffix, and so on).

As far as I understand, there's no point in building .dmg without bundling JDK. Java evolves at extreme pace nowadays, so it looks like the only sane way to release MPS-based IDEs (and .dmg in particular) is to bundle the workable JRE right into the distribution.

generic.zip might be helpful, however, it looks like "always bundle JRE" is the way to go for macOS, Windows, Linux.

So I suggest:

update mps-gradle-plugins to accommodate macOS notarization
heal somehow the Perl failure (e.g. borrow the relevant bits from JetBrains Platform)
drop bundleJdk task altogether and let createDmg do everything (create the dmg with jre inside and sign it). An alternative option is to split createDmg (basically avoid shell scripts) into smaller tasks (perform "create dmg", "sign", "notarize" in different Gradle tasks), however, it would probably require more effort, and the gains are not obvious.

WDYT?

sergej-koscejev · 2021-05-20T07:39:03Z

@vlsi I'm fine with all your suggestions, I just don't want to be the one to do this.

Yes, there's no point in building a .dmg without a JDK because the JetBrains JDK contains some improvements that help their IDEs work properly. The IDEs might crash or misbehave on a standard JDK, AFAIK.

vlsi force-pushed the createdmg branch from 8d785b3 to 9e2db16 Compare July 17, 2020 12:35

vlsi marked this pull request as ready for review July 17, 2020 12:48

coolya requested review from arimer and sergej-koscejev July 22, 2020 21:08

arimer requested changes Jul 23, 2020

View reviewed changes

vlsi mentioned this pull request Jul 23, 2020

Upgrade to JUnit 5 #72

Open

arimer requested a review from wsafonov August 18, 2020 19:22

sergej-koscejev reviewed Nov 22, 2020

View reviewed changes

sergej-koscejev self-requested a review November 22, 2020 15:32

sergej-koscejev requested changes Nov 22, 2020

View reviewed changes

Execute GitHub Actions on PRs

0f64e82

vlsi force-pushed the createdmg branch from 859df92 to 3c49fc2 Compare April 18, 2021 22:20

Refactor CreateDmg and BundleMacosJdk: use Property<..>, migrate to K…

022272c

…otlin This change bumps the version to 1.5 since it updates APIs for CreateDmg and BundleMacosJdk.

vlsi force-pushed the createdmg branch from 3c49fc2 to 022272c Compare April 18, 2021 22:23

vlsi mentioned this pull request Apr 21, 2021

macOS: update scripts for producing and signing dmg #99

Open

vlsi mentioned this pull request May 18, 2021

WDYT of adding a "bundle your own standalone IDE" feature? JetBrains/intellij-platform-gradle-plugin#663

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor CreateDmg and BundleMacosJdk: use Property<..>, migrate to Kotlin #70

Refactor CreateDmg and BundleMacosJdk: use Property<..>, migrate to Kotlin #70

vlsi commented Jul 16, 2020 •

edited

Loading

vlsi commented Jul 17, 2020

arimer commented Jul 23, 2020

vlsi commented Jul 23, 2020

arimer left a comment •

edited

Loading

arimer Jul 23, 2020

arimer Jul 23, 2020

vlsi Jul 23, 2020

arimer Jul 23, 2020

arimer Jul 23, 2020

vlsi Jul 23, 2020

sergej-koscejev left a comment

sergej-koscejev left a comment

vlsi commented Apr 18, 2021

sergej-koscejev commented Apr 21, 2021

vlsi commented Apr 21, 2021

vlsi commented Apr 21, 2021

sergej-koscejev commented Apr 22, 2021

coolya commented Apr 23, 2021

vlsi commented May 11, 2021

sergej-koscejev commented May 20, 2021

Refactor CreateDmg and BundleMacosJdk: use Property<..>, migrate to Kotlin #70

Are you sure you want to change the base?

Refactor CreateDmg and BundleMacosJdk: use Property<..>, migrate to Kotlin #70

Conversation

vlsi commented Jul 16, 2020 • edited Loading

vlsi commented Jul 17, 2020

arimer commented Jul 23, 2020

vlsi commented Jul 23, 2020

arimer left a comment • edited Loading

Choose a reason for hiding this comment

arimer Jul 23, 2020

Choose a reason for hiding this comment

arimer Jul 23, 2020

Choose a reason for hiding this comment

vlsi Jul 23, 2020

Choose a reason for hiding this comment

arimer Jul 23, 2020

Choose a reason for hiding this comment

arimer Jul 23, 2020

Choose a reason for hiding this comment

vlsi Jul 23, 2020

Choose a reason for hiding this comment

sergej-koscejev left a comment

Choose a reason for hiding this comment

sergej-koscejev left a comment

Choose a reason for hiding this comment

vlsi commented Apr 18, 2021

sergej-koscejev commented Apr 21, 2021

vlsi commented Apr 21, 2021

vlsi commented Apr 21, 2021

sergej-koscejev commented Apr 22, 2021

coolya commented Apr 23, 2021

vlsi commented May 11, 2021

sergej-koscejev commented May 20, 2021

vlsi commented Jul 16, 2020 •

edited

Loading

arimer left a comment •

edited

Loading