Add TeamPostgreSQL Cask

[jawshooah]

This is a weird one. It uses a graphical installer, and a graphical uninstaller which is found in the install directory.

Since there's no support for graphical uninstallers yet, I'm not entirely sure what to use for the uninstall stanza.

Also, the graphical uninstaller removes the install directory contents but not the directory itself, which is why I added "#{Cask.appdir}/teampostgresql" to the zap stanza. This won't work if the user selects a different location in the graphical installer, but I figured it would cover most cases. Is there a better way to handle this?

[vitorgalvao]

This should be temporary and managed by the system. It shouldn’t be zapped.

[vitorgalvao]

I'm not entirely sure what to use for the uninstall stanza.

Answer is “the best you can”. Here’s a similar case.

This won't work if the user selects a different location in the graphical installer, but I figured it would cover most cases. Is there a better way to handle this?

Being discussed.

[jawshooah]

@vitorgalvao Is there a documented way of killing a process by doing something like ps -ef | grep dbexplorer.TeamPostgreSQL | awk '{print $2}' | xargs pkill -9?

If so, we can use that in the uninstall stanza and manually delete all the app files, most of which are already in the zap stanza.

Reason being that there's no app bundle id, no launchctl process, but after the installer is run it kicks off this java process.

Pinging @rolandwalker on this too, since it involves potentially undocumented or experimental DSL features.

[vitorgalvao]

uninstall :quit or (uninstall :signal, in extreme cases) should be what you’re looking for.

[jawshooah]

That won't work unfortunately, as the bundle id (com.install4j.0115-9748-2388-7305.31565) is for the installer. The service that is installed doesn't seem to have a bundle id.

[jawshooah]

Hmm, actually the service can be stopped by running

'/Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service' stop

Is it acceptable to add an uninstall_preflight stanza with the following line?

@command.run!('/Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service', :args => ['stop'])

[jawshooah]

Just FYI, adding the [WIP] tag somehow gave me the option to cancel or confirm the merge, which should be a power limited to maintainers.

Edit: Gone now. Weird.

[vitorgalvao]

Installing this to test, I’ve noticed it has a Java dependency, so it’ll need a caveat to that effect. I’ll defer that part to @radeksimko, since he has been taking care of the java parts.

[jawshooah]

The script in Library/StartupItems just delegates to a script in the install directory, teampostgresql/misc/teampostgresql-service.

When uninstalling, should we assume that the user has not removed the script in Library/StartupItems and run that, or run the one in the install directory, since we warn them about atypical install locations in the caveats?

[jawshooah]

Also, attempting to uninstall or zap the Cask when the service is not running results in an error message:

Error: Command failed to execute!

==> Failed command:
["/Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service", "stop"]

==> Output of failed command:
Shutting down teampostgresql-service
The daemon is not running.


==> Exit status of failed command:
#<Process::Status: pid 18940 exit 1>

Can we easily intercept and ignore this message, or is there a better way to handle it?

[vitorgalvao]

When uninstalling, should we assume that the user has not removed the script in Library/StartupItems and run that

If that script changes its target depending on the chosen install location, then yes, we should use that one.

Also, attempting to uninstall or zap the Cask when the service is not running results in an error message

That message may or may not be problematic. It depends on if failure from uninstall_preflight blocks other actions from uninstall or zap. @rolandwalker should know if thats’s an issue.

[rolandwalker]

HI!

@command.run is an internal interface. Within a Cask, system is better for now.

But shutting down a launchctl job shouldn't be done from within uninstall_preflight at all: that is covered by uninstall :launchtl example.

The last I can recall, :launchctl needs some work on the backend. Yosemite may have changed some parameters, and I'd like it to accept a path as well as an ID. However, it knows not to stop the uninstall process when the job isn't loaded.

[jawshooah]

I'm not sure that this is a launchctl job. Nothing related to this Cask shows up when I run list_loaded_launchjob_ids or list_installed_launchjob_ids.

[rolandwalker]

My bad! I didn't read closely enough. /Library/StartupItems is indeed something different, though it is strange to see it still in use. So run your command this way:

uninstall :script => {
                      :executable => '/Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service',
                      :args => %w{stop}
                     },
          :delete => [
                      '/Library/StartupItems/TeamPostgreSQL Service',
                      "#{Cask.appdir}/teampostgresql"
                     ]

I'm not sure it's correct that I have TeamPostgreSQL Service/TeamPostgreSQL Service (repeated twice in the path). That's just copied from your snippet above.

[jawshooah]

Yup, that's correct.

There's still the issue of the uninstall failing if the daemon at /Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service is not running.

Here's a dump of the terminal output:

$ brew cask uninstall teampostgresql
==> Running uninstall process for teampostgresql; your password may be necessary
Password:
==> Shutting down teampostgresql-service
==> The daemon is not running.
Error: Command failed to execute!

==> Failed command:
["/usr/bin/sudo", "-E", "--", "#<Pathname:/Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service>", "stop"]

==> Output of failed command:
Shutting down teampostgresql-service
The daemon is not running.


==> Exit status of failed command:
#<Process::Status: pid 6258 exit 1>

[rolandwalker]

There's still the issue of the uninstall failing

Oh right. So you should add :must_succeed => false:

uninstall :script => {
                      :executable => '/Library/StartupItems/TeamPostgreSQL Service/TeamPostgreSQL Service',
                      :args => %w{stop},
                      :must_succeed => false
                     },

[jawshooah]

Excellent, that did the trick! Making the change and squashing now.

I'm investigating a way to run this installer without launching the GUI. This looks promising, so I tried the following:

sudo '/opt/homebrew-cask/Caskroom/teampostgresql/latest/TeamPostgreSQL Installer.app/Contents/MacOS/JavaApplicationStub' -q -dir '/Applications'

and everything goes fine until the installer tries to start the system service. Then it spits out this error

There was a problem starting the TeamPostgreSQL service. Installation will be aborted.
Rolling back changes..

I've contacted the devs about this, and they said it could be a permissions issue. I doubt it, since the same thing happens when I try it after executing sudo su.

Ideally, we could bypass the GUI and not depend on the user to install the Cask somewhere expected. I'll keep looking into it.

[jawshooah]

@rolandwalker Is there a blessed way of creating a wrapper script to be symlinked into $(brew --prefix)/bin?

The reason I ask is that this software is also available in a "cross-platform archive", which is just a .zip archive containing the app's resources and a launcher script teampostgresql-run.sh.

The problem is that the launcher script depends on having the app's Java resources in the working directory, so we can't just symlink it with a binary 'teampostgresql/teampostgresql-run.sh', :target => 'teampostgresql' artifact.

But what we can do is just as simple: create a wrapper script that executes the launcher and symlink that into $(brew --prefix)/bin.

[rolandwalker]

So far we have gotten along without creating any shim scripts, though it does come up from time to time. Currently there is only binary to link a particular binary into /usr/local/bin.

[jawshooah]

So there's no clean, documented way to do it, then.

How about a dirty, undocumented way?

[jawshooah]

@rolandwalker Alright, I've added a quick-and-dirty implementation. Will this fly, or is there a cleaner way of doing it?

[fanquake]

@jawshooah Could you squash your commits together?

@rolandwalker @vitorgalvao Any feedback or this?

[jawshooah]

@fanquake Done. I'd certainly welcome feedback, though.

[vitorgalvao]

As said in this issue and in the jenkis one, we prefer to not create those types of scripts.

The problem is that the launcher script depends on having the app's Java resources in the working directory.

Which means that limitation isn’t due to our installation, but the app itself. There’s not much reason for us to create that workaround, then. Users should do it only if they want, just like they would if they installed it manually.

[jawshooah]

@vitorgalvao But isn't reducing the frequency of such tedious manual installations one of the primary benefits of using brew-cask?

The other problem is that, without that binary stanza, this cask has no artifacts. I could revert to using the graphical installer for Mac, but that's exactly what I was hoping to avoid with this cross-platform archive.

[vitorgalvao]

Yes, but the goal is to do it in a standard way. This means that a cask install shouldn’t have any surprises: it should mostly lead to the result you were expecting to achieve if doing it manually — a wrapper script is definitely not included in those.

In principle most of the cases that require a wrapper script could be solved by a fix upstream, and incentivising those is something I see as a good thing¹. In practice, however, these scripts could come in handy as a last resource, and as @rolandwalker mentioned, this does come up from time to time.

Perhaps we should open a discussion for this in another issue. If we do end up deciding in favor of such capability, it should be both officially supported (no need for hacky solutions) and discouraged².

---

¹ It happens very often that submissions try to work around some peculiarity in open-source software via homebrew-cask instead of trying to resolve it upstream instead, where it could and should be solved most of the time. Programming sucks in part because we’re constantly building workarounds on top of workarounds, when sometimes the solution is to fix what’s broken.

² sha256 :no_check in a cask with a set version is an example of a feature in the same standing.

[jawshooah]

I've gone ahead and submitted a pull request on Homebrew for this archive, as they're a little more lax in terms of style.

I plan to come back to this soon and hopefully get the Mac installer working without having to go through the GUI.

[vitorgalvao]

Like I said, I do see a divergence in this issue regarding the differences in principle and in practice, and perhaps we should open a discussion issue for this. Would you care to do it? I’d like to see opinions from other maintainers as well.

It’s the second cask you envision working with a wrapper, and another one has cropped up a few hours ago, so we could use those as examples. The factor.rb cask is also an example in that similarly a shell script was suggested as a workaround but reaching out to the developers turned out to be the right solution, since it was a bug that has since been fixed.

[jawshooah]

Closing this as I no longer use this software, and there doesn't seem to be a demand. We can revisit if someone else needs it.