Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should shim scripts be implemented? #9203

Closed
vitorgalvao opened this issue Jan 24, 2015 · 10 comments
Closed

Should shim scripts be implemented? #9203

vitorgalvao opened this issue Jan 24, 2015 · 10 comments

Comments

@vitorgalvao
Copy link
Member

This issue surfaces from time to time: a cask, after installed, has some limitation on its CLI tools, caused either by some choice from upstream or the nature of how we install things (linking).

Many times this can be resolved (read “worked around”) by a small shell script only a few lines long. So far we have restrained from accepting such solutions, and to an extent that is a good thing. Sometimes upstream takes notice and fixes the problem but those cases are clearly the minority. In the other cases it could be convenient, provide consitency, fix cases that don’t play well with links, or outright facilitate installs and unintalls.

The more I look at it, the less I think it’s a good idea to not support such a capability. Speaking in purely practical terms, I see this as analogous to using :no_check with versioned casks — we can live without it, but it is a worse experience than living with it. Similarly, I do think it should be used sparingly and only as a last resort.

@jawshooah
Copy link
Contributor

👍 Agreed on all points. We would probably want to provide convenience methods like Homebrew's write_exec_script, write_env_script, and write_jar_script.

Or perhaps something like this:

binary :jar => 'foo/bar.jar', :target => 'bar'
binary :env => 'baz/blargh.py', :target => 'blargh', :PYTHONPATH => "baz/lib/site-packages:#{ENV['PYTHONPATH']}"

@phinze
Copy link
Contributor

phinze commented Jan 24, 2015

Also agreed on all points!

@vitorgalvao is this mainly a PR acceptance policy question or mainly a feature question?

Re the latter - It'd be nice if we could provide some basic convenience methods, but I'd worry about going too deep down that rabbit hole to the point where we accidentally encourage shims where they are unnecessary. We should make sure any convenience methods we support properly communicate the "should be used sparingly" part.

@vitorgalvao
Copy link
Member Author

is this mainly a PR acceptance policy question or mainly a feature question?

Mainly about accepting such PRs. However, I agree in full with your comment and do think an officially supported way should exist. The current way to do it is hacky (well, maybe the word should be “inelegant”), and an official solution would go a long way towards readability and maintainability.

We should make sure any convenience methods we support properly communicate the "should be used sparingly" part.

As we do with :target, a comment on why the script is needed with a link back to the discussion seems like a must.

@phinze
Copy link
Contributor

phinze commented Jan 24, 2015

Cool cool - sounds like we're on the same page about the acceptance policy.

Then we just need to decide what the shim script API would look like. @jawshooah gave us a few starting points. Any other ideas on this?

@tapeinosyne
Copy link
Contributor

Quoth @rolandwalker: #4188 (comment). (Nothing particularly articulated.)

I would not rush an expansion to the DSL; the priority is rather low. However, @vitorgalvao is right that we should decide whether to merge such patches or not, as a general policy.

@radeksimko
Copy link
Contributor

👍 Agreed with all points and yes, there should be a decent API to make such things less hacky.

@rolandwalker
Copy link
Contributor

My preferred interface would involve the word "method". Once we refactor symlink/hardlink out of the artifact code, we can think of at least these methods for activating a staged artifact:

  • symlink
  • hardlink
  • dir-hardlink
  • copy
  • shim

I don't know how it would look, exactly, because sometimes an artifact type has an inherent preferred method, sometimes a Cask will want to override the method, and it would be ideal if the user could also

$ brew cask install --method=copy google-chrome

We don't need to do any of those things to implement shims. It's just nice to think generally and be consistent with other desired future functionality.

Homebrew's interface is a mismatch for these goals.

@phinze
Copy link
Contributor

phinze commented Jan 24, 2015

Ooo I like that a lot @rolandwalker - the other word that comes to mind is "strategy". I'll think more on this and get back to you.

@rolandwalker
Copy link
Contributor

"Strategy" is also agreeable because it is abstract.

@vitorgalvao
Copy link
Member Author

#18809.

@miccal miccal removed the discussion label Dec 23, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants