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

Deprecate --caskroom flag #21858

Closed
vitorgalvao opened this issue Jun 11, 2016 · 12 comments
Closed

Deprecate --caskroom flag #21858

vitorgalvao opened this issue Jun 11, 2016 · 12 comments

Comments

@vitorgalvao
Copy link
Member

In a similar vein to #21372, in merging with Homebrew and moving the Caskroom inside their directory by default, why would you ever want your Caskroom in a separate location?

We would simplify the code by removing the flag. Even when creating the Caskroom for the first time, this (26 lines with multiple sudos) could become this (9 lines, no sudos):

def self.init
  odebug 'Creating directories'
  HOMEBREW_CACHE.mkpath unless HOMEBREW_CACHE.exist?
  HOMEBREW_CACHE_CASKS.mkpath unless HOMEBREW_CACHE_CASKS.exist?
  unless caskroom.exist?
    ohai "Creating Caskroom at #{caskroom}"
    system '/bin/mkdir', '--', caskroom
  end
end
@claui
Copy link
Contributor

claui commented Jun 12, 2016

@vitorgalvao I am very much in favor of this – as long as the particular minority I’m lobbying for is not affected; and it indeed is not. (I would make for a terrible politician!)

Removing things no one really needs anymore is exactly what keeps a product from becoming a big ball of mud. 👍

jawshooah pushed a commit that referenced this issue Jun 12, 2016
This commit fixes an error occurring with both `install` and `install --force` where either would fail when encountering a broken symlink.

Users _might_ run into that issue in the wake of PR #21857, and certainly _will_ be bitten by it once PR #21858 is merged.

This commit changes the `install` case so it handles the condition gracefully. It also fixes `install --force` so before moving an app, it removes any broken symlinks which might stand in the way.
@jawshooah
Copy link
Contributor

In light of the feedback we've been getting after #21857 (e.g. #21894), I think we should hold off on this for a while. I feel like we jumped the gun, and should have made sure to notify users of the change and ensure that their old Caskroom would remain intact, at least for a specified transitional period.

I'm away from my computer at the moment, but unless someone beats me to it (@claui has been killing it lately), I'm going to write up a PR this evening that will use the old path if it exists, warn users that the default has changed, and encourage them to add --caskroom=/opt/homebrew-cask/Caskroom to HOMEBREW_CASK_OPTS to prevent data duplication and installation issues.

@vitorgalvao
Copy link
Member Author

@jawshooah Made an official answer before we start getting bashed by the less friendly users.

As for the fix, I think we should both encourage adding --caskroom=/opt/homebrew-cask/Caskroom or moving the Caskroom. You suggestions from the other issue, basically.

jawshooah added a commit that referenced this issue Jun 13, 2016
If a Caskroom exists at the old location of /opt/homebrew-cask/Caskroom,
use it and warn the user that the default location has changed.

Refs #21858, #21857, #21894
chizmw pushed a commit to chizmw/homebrew-cask that referenced this issue Jun 15, 2016
This commit fixes an error occurring with both `install` and `install --force` where either would fail when encountering a broken symlink.

Users _might_ run into that issue in the wake of PR Homebrew#21857, and certainly _will_ be bitten by it once PR Homebrew#21858 is merged.

This commit changes the `install` case so it handles the condition gracefully. It also fixes `install --force` so before moving an app, it removes any broken symlinks which might stand in the way.
chizmw pushed a commit to chizmw/homebrew-cask that referenced this issue Jun 15, 2016
If a Caskroom exists at the old location of /opt/homebrew-cask/Caskroom,
use it and warn the user that the default location has changed.

Refs Homebrew#21858, Homebrew#21857, Homebrew#21894
@vitorgalvao vitorgalvao changed the title Discard --caskroom flag? Deprecate --caskroom flag Feb 2, 2017
@vitorgalvao vitorgalvao added core Issue with Homebrew itself rather than with a specific cask. ready to implement and removed discussion labels Feb 2, 2017
@vitorgalvao
Copy link
Member Author

Changed title to Deprecate --caskroom flag as deprecation doesn’t automatically make it stop working.

@miccal miccal removed core Issue with Homebrew itself rather than with a specific cask. ready to implement labels May 25, 2017
@balupton
Copy link
Contributor

balupton commented Jul 7, 2017

Started seeing this:

Warning: Calling `brew cask` with the `--caskroom` flag is deprecated and will be disabled on 2017-10-31!

Where exactly is the new location?

Here is my brew config output:

> brew config
HOMEBREW_VERSION: 1.2.4
ORIGIN: https://github.com/Homebrew/brew
HEAD: b5529084906af89827f6d9befd613457a1615918
Last commit: 5 days ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: ff3cf9dfb84d229e1f5fa02728d518f6622ce74e
Core tap last commit: 7 hours ago
HOMEBREW_PREFIX: /Users/balupton/.homebrew
HOMEBREW_REPOSITORY: /Users/balupton/.homebrew
HOMEBREW_CELLAR: /Users/balupton/.homebrew/Cellar
HOMEBREW_BOTTLE_DOMAIN: https://homebrew.bintray.com
CPU: quad-core 64-bit broadwell
Homebrew Ruby: 2.0.0-p648
Clang: 8.1 build 802
Git: 2.13.2 => /Users/balupton/.homebrew/bin/git
Perl: /usr/bin/perl
Python: /Users/balupton/.homebrew/bin/python => /Users/balupton/.homebrew/Cellar/python/2.7.13/Frameworks/Python.framework/Versions/2.7/bin/python2.7
Ruby: /Users/balupton/.homebrew/bin/ruby => /Users/balupton/.homebrew/Cellar/ruby/2.4.1_1/bin/ruby
Java: N/A
macOS: 10.12.5-x86_64
Xcode: 8.3.3
CLT: N/A
X11: N/A

Configured via: https://github.com/balupton/dotfiles/blob/7cf12331de5462ad3f377609deaa9c9cd101c83e/.scripts/sources/mac.sh

@reitermarkus
Copy link
Member

Where exactly is the new location?

The new location is $(brew --prefix)/Caskroom.

@balupton
Copy link
Contributor

balupton commented Jul 7, 2017

Okay cool! Any plan for a brew --caskroomdir flag or so?

google-cloud-sdk needs some dotfile adjustments at the caskroom folder, so having a flag like that would make it easy to get the location

@vitorgalvao
Copy link
Member Author

Okay cool! Any plan for a brew --caskroomdir flag or so?

For what? $(brew --prefix)/Caskroom already does that and is dynamic. It isn’t worth it to add an extra option.

@philBrown
Copy link
Contributor

Warning: Calling brew cask with the --caskroom flag is deprecated and will be disabled on 2017-10-31!

Started seeing this too. What does it mean? All I executed was brew cask list

@reitermarkus
Copy link
Member

reitermarkus commented Aug 9, 2017

Do you have set HOMEBREW_CASK_OPTS? If so, remove the --caskroom flag from it.

@philBrown
Copy link
Contributor

Ah yes, that's it. Thank you @reitermarkus

@cdesch
Copy link

cdesch commented Nov 21, 2017

Remove --caskroom=/usr/local/Caskroom from the .bash_profile worked for me

before:
export HOMEBREW_CASK_OPTS="--appdir=~/Applications --caskroom=/usr/local/Caskroom"

after:
export HOMEBREW_CASK_OPTS="--appdir=~/Applications"

@Homebrew Homebrew locked and limited conversation to collaborators Nov 21, 2017
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

8 participants