Change app symlinks to point to Contents

[rhendric]

I'm expecting this to be a little controversial. I'm not 100% convinced it's a great idea myself, but boy, do I want a solution to the below problem.

Problem: When adding a symlinked application to the Dock, OS X appears to follow the symlink and actually add the target application bundle to the com.apple.dock.plist, not the source link. As a consequence, when brew-cask upgrades the application to a new version, the Dock icon still launches the old version (or fails if the old version isn't on the system anymore).

Solution: This commit changes the behavior of app linking to create an AppName.app folder containing an AppName.app/Contents symlink, instead of an AppName.app symlink. This gets the Dock to consistently point to the link-containing AppName.app, even when the Contents symlink within is upgraded by brew-cask.

I wrote this with the intent of being backwards-compatible; in particular, installing over either an AppName.app link or an AppName.app/Contents link is allowed, while an AppName.app containing a non-symlink Contents will (still) not be clobbered.

[vitorgalvao]

Pinging @phinze @ndr-qef @radeksimko.

[zmwangx]

I would say this is very counterintuitive. Also,

I wrote this with the intent of being backwards-compatible;

While this might be backwards-compatible with brew-cask's native functionalities, it's certainly possible to break external commands and user scripts. For instance, I have a script that swaps apps with links to meet sandboxing requirements (i.e., some apps have to live under /Applications, e.g., browsers with the 1Password extension), which would certainly break if this change were (silently) introduced.

[vitorgalvao]

Agreed. Our installation should be as close to normal as possible, and this is yet a new point of breakage. Since tere were no further comments, I’ll go agead an close it.

[rhendric]

Does nobody else experience this issue with the Dock? Because in my experience the ‘normal’ thing is that you can install an application and drag it onto the Dock and then upgrade it in place and the Dock icon will refer to the upgraded version. Casks break this, and I find that pretty counterintuitive.

I don't mean to be argumentative; I was just hoping for a little more of a conversation about pros and cons before being closed. If there's a better way to solve this problem that would help the community, I'd like to work on it. If there's a reason why this isn't a problem for the community or shouldn't be solved in this way, I'd like to understand that. Help me work towards the greater good here.

@zmwangx, I don't use 1Password; what specifically is the requirement that your script addresses? I think under some conditions OS X considers the app to live wherever the .app bundle folder lives, so is it possible that with this patch, a brew cask install --appdir=/Applications google-chrome would be sufficiently ‘under /Applications’ for this purpose, so that the script isn't necessary?

[vitorgalvao]

Does nobody else experience this issue with the Dock? (…) If there's a reason why this isn't a problem for the community

First time I remember seeing anyone with the issue.

I don't use 1Password; what specifically is the requirement that your script addresses?

See this caveat.

As a consequence, when brew-cask upgrades the application to a new version

Homebrew-cask does not upgrade to new versions. The feature isn’t done. What you mean is “when you force install a new version”, which is a very different problem. Your use case is clear, but the solution is bad as it introduces a system that will be highly unexpected. Even linking as it is done now already introduces a slew of problems, I can already imagine the ones this solution will bring.

If the problem happens when updating, then it should be fixed on updates. What you’ve done here is not a fix but a workaround with unforeseen disadvantages to fix (what will be) an uncommon action (forcing installation) that serves only to fix errors itself.

[rhendric]

Homebrew-cask does not upgrade to new versions. The feature isn’t done.

Heh. That's frustrating, but a fair response. Thanks for explaining.

I suppose then that the remainder of my input is that I'd like this issue to be covered by the upgrade feature when it is done—applications should be installed such that upgrades don't break the Dock. Does this PR capture that, or should I enter a new issue or add a comment to an existing one or something?

[vitorgalvao]

Many things need to be done before upgrade is done. That is discussion for later on, but the request is understandable, and we should take it into account.

[zmwangx]

@rhendric

so is it possible that with this patch, a brew cask install --appdir=/Applications google-chrome would be sufficiently ‘under /Applications’ for this purpose, so that the script isn't necessary?

Maybe, not sure. But I'd say that's equally bad from a security point of view, and certainly shouldn't be relied upon.

Anyway, I was just providing one example of why this change might break brew's external commands and user scripts. The detail doesn't really matter.

@vitorgalvao

Your use case is clear, but the solution is bad as it introduces a system that will be highly unexpected.

Spot on, that's the biggest problem with this patch.

[scribblemaniac]

First time I remember seeing anyone with the issue.

I'll throw in my two cents here. I have also experienced the issue mentioned above, however since we don't officially support updating, I don't believe it should be expected of us to fix this issue right now. I'm not knowledgeable enough here to weigh in on the pros and cons of this particular solution but this problem definitely needs to be considered for cask upgrade.

I have two potential alternate solutions I should mention, but like I said earlier, I don't know very much about this kind of stuff:

1. Modify the com.apple.dock.plist. The primary issue with this is that it would only solve the issue for the current user because you wouldn't have access to the Library folders of the other users.
2. Add the symlink to the dock. This is the simplest and best solution for now in my mind. If you drag the app symlink to the dock, it follows it to the actually application location as stated above. If you repeat this action however, it appears that you end up with one icon for the actual application and one to the symlink. You can then just remove the non-symlink icon. I haven't tested whether this will solve this issue, but I don't see any reason why it wouldn't work.

[vitorgalvao]

If you’re suggesting homebrew-cask adds the symlink to the dock, no way. Get off my dock, whoever you are. Not every user keeps every app in the dock (I keep almost none), and some apps don’t even make sense in the dock, most of the time. If I install an app automatically via the command-line but then have to remove its icon manually from the dock, that is a crappy experience.

[rhendric]

@scribblemaniac, that symlink workaround sounds excellent but I can't get it to work for me. Is there some additional trick to it? When I drag the symlink over the second time, I don't get a second icon. I tried it both with the application running and not.

@vitorgalvao, yeah, that sounds horribly Windows-esque but I think a saner thing would be only to update existing entries in the plist for the app being upgraded, and do nothing if none are found.

[radeksimko]

I partially agree with @vitorgalvao here, but I can totally see the dock icons being issues between "upgrades" and something that needs to be addressed when implementing the actual upgrade functionality.
I think if user adds app into the dock and that app is being managed by homebrew-cask, we could try and update the location during upgrade.

[rhendric]

(For the record, I tried modifying com.apple.dock.plist before I discovered the Contents-symlink hack, but my original efforts ended up moving the icon in the Dock after it was updated. Probably there's a way to work around that and/or I was just doing something dumb, but it's a thing to watch out for if attempted—I'd be pretty unhappy about a Dock that rearranged itself on a hypothetical brew cask upgrade.)

[scribblemaniac]

If you’re suggesting homebrew-cask adds the symlink to the dock, no way. Get off my dock, whoever you are.

Woah easy there! I'm sorry that I did not make myself clearer, but I was only suggested that it modify existing dock items for apps that are managed by cask. I agree that adding dock items on install is a terrible idea.

@rhendric Hmm strange, it worked right away for me. What mac version do you have? Sadly I am away from my main computer this weekend so I won't be able to test this workaround anymore until late tomorrow or Monday. I'll let you know at that time if I've come up with anything else that may need to be done.

[vitorgalvao]

Woah easy there!

I’m not sure how seriously you took my comment, but just to make sure and stop this from escalating unnecessarily, I want to make clear I was kidding. I mean, yes, I do think adding to the dock automatically is a bad idea, but “[g]et off my dock, whoever you are” was totally in jest, in a “get off my lawn” “old man yells at cloud” kind of way.

[scribblemaniac]

No worries, I got understood where you were going with your comment.

[scribblemaniac]

Okay everyone, I've had a chance to investigate a little further. I was partially mistaken earlier with my dragging to dock technique. It turns out that this only works for aliases and not symlinks. This got me thinking that we should maybe switch from symlinks to aliases. There was a brief discussion about adding an alias command at #5101 however I don't see anything in it that would discourage switching from symlinks to aliases for installation-time linking. Can I hear some thoughts on this idea?

Also I made a quick and dirty hack to the source to use aliases rather than symlinks for installation. The result can be found at scribblemaniac:alias. I have tested this when the alias is added to the dock and this does indeed fix the issue with reinstalls (or eventually upgrades) that the OP is pointing out.

[zmwangx]

@scribblemaniac Is there a way to create aliases without AppleScript? AppleScript is an abomination (I haven't looked at JXA), and osascript was broken big time in the first three stable releases of Yosemite — I'm not sure they won't mess it up again. I don't know about others, but I'm much more comfortable with standard Unix technologies + manipulating plists with defaults if that's necessary.

(I'm not a fan of Mac aliases either.)

[scribblemaniac]

A standard unix technology will definitely not exist for aliases since they are a mac specific features. I would also prefer not to use applescript myself, however this is the only way that I've found to do it. Feel free to do some more looking and if you find something I can definitely update it. Anyway it would not be the first time that hombrew-cask uses applescript; sometimes it's the only reasonable option.

[zmwangx]

By standard Unix technology I mean symlinks...

Anyway it would not be the first time that hombrew-cask uses applescript;

Interesting, I wasn't aware of that.

[jawshooah]

Yup, it's used in quite a few places, actually:

$ git grep osascript
developer/bin/list_running_app_ids:  out, err, status = Open3.capture3('/usr/bin/osascript', '-e', 'tell application "System Events" to get (name, bundle identifier, unix id) of every process')
developer/bin/list_running_app_ids:  %r{^osascript$}.match(app_name)   # this script itself
lib/hbc/artifact/uninstall_base.rb:        num_running = @command.run!('/usr/bin/osascript', :args => ['-e', %Q{tell application "System Events" to count processes whose bundle identifier is "#{id}"}], :sudo => true).stdout.to_i
lib/hbc/artifact/uninstall_base.rb:          @command.run!('/usr/bin/osascript', :args => ['-e', %Q{tell application id "#{id}" to quit}], :sudo => true)
lib/hbc/artifact/uninstall_base.rb:        pid_string = @command.run!('/usr/bin/osascript', :args => ['-e', %Q{tell application "System Events" to get the unix id of every process whose bundle identifier is "#{id}"}], :sudo => true).stdout
test/cask/artifact/uninstall_test.rb:      Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"'], '1')
test/cask/artifact/uninstall_test.rb:      Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application id "my.fancy.package.app" to quit'])
test/cask/artifact/zap_test.rb:      Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"'], '1')
test/cask/artifact/zap_test.rb:      Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application id "my.fancy.package.app" to quit'])
test/cask/cli/zap_test.rb:  #   Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app"'], '1')
test/cask/cli/zap_test.rb:  #   Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application id "my.fancy.package.app" to quit'])
test/cask/cli/zap_test.rb:  #   Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application "System Events" to count processes whose bundle identifier is "my.fancy.package.app.from.uninstall"'], '1')
test/cask/cli/zap_test.rb:  #   Hbc::FakeSystemCommand.stubs_command(['/usr/bin/sudo', '-E', '--', '/usr/bin/osascript', '-e', 'tell application id "my.fancy.package.app.from.uninstall" to quit'])

[zmwangx]

Yeah I found those after learning the fact, but really I would just count the one occurrence (or three if you prefer) under lib.

[vitorgalvao]

There was a brief discussion about adding an alias command at #5101

That issue has nothing to do with the subject at hand, though. It is about a user-facing way of using a custom :target on the fly. It does not pertain to the type of link applications use.

[scribblemaniac]

That issue has nothing to do with the subject at hand, though.

Ah okay, I naturally assumed an alias command would make aliases. Silly me!