gamerson / workspace-demos

0 stars 1 forks source link

Feedback - bundleChecksumMD5 #1

Closed sustacek closed 3 years ago

sustacek commented 3 years ago

Hi @gamerson,

here're my thoughts.

Default checksum

The way I understand it, right now, the default checksum is read by fetching a URL. This URL is computed based on bundleURL by appending .MD5 to it. I'd make the value of the checksum explicit in the ".product_json" metadata file. Or put the checksum URL there, but make it explicit and not hidden in the impl of the plugin. The URL might be obfuscated the same way as the bundleURL to keep it semi-hidden and not reveal the URL of each bundle.

Configuration

Instead of this: https://github.com/gamerson/workspace-demos/blob/ab0f542912882e41b7e6a97ddabb549f9bd39459/bundleChecksumMD5_custom/build.gradle#L1-L4

I'd encourage all configurations to be done in portal.properties, not inside build.properties. People already configure many things for the workspace using gradle.properties (your examples do it as well), so why not put the custom MD5 sum there as well? I know build.gradle is more flexible (you can compute the values using Groovy / Kotlin), but I'd at least give users the option to use gradle.properties if it's enough for them.

For example, support reading project property liferay.workspacec.bundle.checksum.md5 while still allowing users to use gradle.liferayWorkspace extension to set the value -- in case they need to compute the value for some reason (like fetch the value by reading content sent on a computed URL. The docs might look like this:

To configure custom MD5 sum (as a static value), you can use:

# gradle.properties
liferay.workspacec.bundle.checksum.md5 = 123jjh234134

Or you can compute the value inside the build script, for example:

// build.gradle
gradle.liferayWorkspace {
   // uses http://docs.groovy-lang.org/latest/html/groovy-jdk/java/net/URL.html#getText()
    bundleChecksumMD5  = new URL("${bundleURL}.md5sum").text
}
gamerson commented 3 years ago

Hey @sustacek thanks for the feedback, we will incorporate this into another workspace version very soon and get you an updated release. No ETA right now, but hopefully this week or next.

gamerson commented 3 years ago

Hey @sustacek

I've pushed a new update. Now with the latest workspace (3.4.3) we have a new property

liferay.workspace.bundle.checksum.md5

If you specify your own bundle url, there is no default md5. you will have to explicitly set it with the above property (or you can use the extension object)

gradle.liferayWorkspace {
    bundleChecksumMD5 = "..."
}

I've updated the 3 examples in this repo accordingly.

Also if you are using the liferay.workspace.product=... key, all of our product infos now include the bundle checksums, so if you just do liferay.workspace.product=dxp-7.2-sp2 then the bundleChecksumMD5 property will get added and the verifyBundle task will get used.

Please let me know if you have any more thoughts or issues with this feature. And thanks for the original suggestion.

sustacek commented 3 years ago

Hi Greg,

thanks for making the changes. I've tested this in our project and works as expected. I don't have any other suggestions for now.

I think this will be a great addition and make the development using bundles way easier and reliable. Thanks for putting this in!