SOLR-14847: Create Solr Server TGZ

[madrob]

No description provided.

[HoustonPutman]

Two issues I see currently.

• The package is named solr-packaging-<version>.tgz not solr-<version>.tgz. This should be easily fixed by: archiveBaseName = "solr"
• The tar contains the output contents, not the output directory. The current solr tars have the contents all stored under solr-<version>/. The problem exists because toDir.outputs is the list of contents of the release directory, not the release directory itself. The same issue pops up in the docker PR, which I have made a workaround for.

[HoustonPutman]

I think I have addressed both of the issues I raised.

[dweiss]

Yes, correct. Remove excessive lines maybe?

[dweiss]

Please take a look at the link I provided earlier - it contains a matcher on file name so only one from clause is needed.

[dweiss]

from(..., {
filesMatching("**/*.sh", {
fileMode 0755
})
})

[HoustonPutman]

from(..., {
filesMatching("**/*.sh", {
fileMode 0755
})
})

Sorry, forgot to mention this before. I tried that initially and it resulted in all files having the executable bit added.

  from(toDir.outputs, {
    filesMatching(["**/bin/*", "**/*.sh"], {
      fileMode 0755
    })
  })

drwxr-xr-x 1 houston houston    512 Sep  9 11:11 bin
-rwxr-xr-x 1 houston houston 946673 Sep  9 17:11 CHANGES.txt
drwxr-xr-x 1 houston houston    512 Sep  9 11:11 contrib
drwxr-xr-x 1 houston houston    512 Sep  9 11:11 dist
drwxr-xr-x 1 houston houston    512 Sep  9 11:11 example
drwxr-xr-x 1 houston houston    512 Sep  9 11:11 licenses
-rwxr-xr-x 1 houston houston  12646 Sep  9 17:11 LICENSE.txt
-rwxr-xr-x 1 houston houston 786774 Sep  9 17:11 LUCENE_CHANGES.txt
-rwxr-xr-x 1 houston houston  27951 Sep  9 17:11 NOTICE.txt
-rwxr-xr-x 1 houston houston   7470 Sep  9 17:11 README.md
drwxr-xr-x 1 houston houston    512 Sep  9 11:11 server

[madrob]

Doing more research on this, I suspect that we should be using the https://docs.gradle.org/current/userguide/distribution_plugin.html built in distribution plugin instead of trying to roll all of this ourselves. I'll spend some time trying to piece this together

[HoustonPutman]

So it turns out that fileMode isn't an option for FileCopyDetails, so it was applying it to the whole Copy operation instead. Changing it to setMode fixed everything.

Hopefully this helps in your migration to that plugin Mike.

[madrob]

@HoustonPutman since I don't have a windows machine, can you check the behaviour of the the distribution plugin for me one more time? ./gradlew -p solr/packaging distTar should create the correct file. We probably will need to still set the file mode like you did, but I wanted to verify it once more before committing to it.

[madrob]

Most of the changes at this point are indentation, so the PR is much smaller than it appears.

[dweiss]

I am not keen on using the plugin. I like knowing what's going on behind the scenes - a bit of verboseness is good here, actually.

[dweiss]

Could you please revert to using a task instead of the plugin? Including that executable bits block Houston added.

[dweiss]

Perhaps I should clarify my reluctance to use the plugin - these plugins are built-in and ship with gradle. It's a moving target when you upgrade the build system - their behavior changes, your build silently does different things. I've been scalded by this before and I hate it. Versioned plugins are different in that you have some control over their upgrades.

[dweiss]

Give me some time to digest it, @madrob I'll take a look at what the distribution plugin does behind the scenes and how often it changes.

[dweiss]

So it turns out that fileMode isn't an option for FileCopyDetails, so it was applying it to the whole Copy operation instead. Changing it to setMode fixed everything.

Ah. Great catch! This is one of the biggest gotchas in gradle - unscoped variables sometimes reach out to unexpected places. Thanks for finding it.

[dweiss]

I've added a file permissions block and verified what the distribution plugin does. I still have mixed feelings about it but I admit I am conservative about using third-party plugins (spent too much time debugging such stuff...).

Let it be though. +1 from me.

[HoustonPutman]

I might be using it wrong, but I don't think the distTar task overwrites previously generated tars. I've tried changing the executable bit paths, and the tar that already exists is unnaffected.

If this is the case, we might want to switch back to the manual task. Though this seems like I'm doing something wrong, because this surely can't be the default behavior of the plugin.

[dweiss]

I think those bits are not part of the task's inputs so it's not re-run. Try --rerun-tasks or cleanDistTar

[HoustonPutman]

Ahh ok, thank for clearing that up. Whenever I cleaned it up beforehand, the task worked fine. I've also tested changing actual files and the task unsurprisingly works correctly.

Since we won't be adding executables in weird places too often, I think this should be good to go. +1

[dweiss]

I think the timestamps/ execution bits are not part of inputs on purpose (to allow distributed artifact caches). This is on the border of my gradle foo knowledge, you'd have to dig in yourself if you want to know more. Dawid

…

On Thu, Sep 10, 2020 at 4:19 PM Houston Putman ***@***.***> wrote: Ahh ok, thank for clearing that up. Whenever I cleaned it up beforehand, the task worked fine. I've also tested changing actual files and the task unsurprisingly works correctly. Since we won't be adding executables in weird places too often, I think this should be good to go. +1 — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#1844 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABQWLXKZR3NVGBZ74RLVETSFDN6LANCNFSM4RANVA4A> .

[madrob]

Does this need a CHANGES entry? We haven't been doing that for other gradle build things, but wanted to confirm.

[dweiss]

I don't know. It's on master only and pretty much everything falls under "gradle build" umbrella. Feel free to add it if you like though!