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

When pressing C-o, I get an error instead of hydra #464

Closed
DamienCassou opened this Issue Apr 6, 2016 · 13 comments

Comments

Projects
None yet
4 participants
@DamienCassou
Contributor

DamienCassou commented Apr 6, 2016

I'm just starting with ivy and reading the manual. In http://oremacs.com/swiper/#introduction, in the discoverability section,

C-o in the minibuffer displays a hydra menu

For me, I get autoload-do-load: Symbol's value as variable is void: hydra-ivy

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Apr 6, 2016

Ideally, you need the hydra package installed before swiper. Then all byte-compile/autoload problems go away. Just re-install them in that order. Or install just hydra and byte-compile ivy-hydra.el to be done faster.

It's arranged this way so that swiper doesn't need to install hydra every time. But there is functionality to self-install hydra when you first press C-o. It probably malfunctioned for you.

@DamienCassou

This comment has been minimized.

Contributor

DamienCassou commented Apr 7, 2016

I'm using a package manager so I can't control load order. What about setting up an error handler which would byte compile ivy-hydra.el if an error is signaled before trying again once outside of the error handler?

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Apr 7, 2016

This issue is just for the first-time installation. Once you install it, it will work fine from then on.

A simple way to do it:

  1. Uninstall swiper and hydra, restart Emacs
  2. Install hydra.
  3. Install swiper.
@sooheon

This comment has been minimized.

Contributor

sooheon commented Apr 18, 2016

I'm seeing this same issue every time I upgrade swiper; I'm installing them through use-package declarations the way spacemacs does it--do you know of a way to programmatically ensure that the installation order is correct using use-package?

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Apr 18, 2016

@sooheon

(use-package hydra
    :ensure t)
(use-package swiper
    :ensure t)
@syl20bnr

This comment has been minimized.

syl20bnr commented May 1, 2016

Unfortunately we cannot do this in Spacemacs because packages are installed and configured in a predictive order which is the alphabetical order, this is an important property of Spacemacs architecture to allow integrating hundreds of packages.

The user should not get the burden of dependency management, this is the job of package.el. I see two possible solutions for this issue:

  1. Moving hydra check from compile time to runtime. Looks to me that a compile time dependency should end up in the package dependencies metadata to be resolved by package.el. Also a runtime check can offer more possibilities and flexibility.
  2. Maybe the cleanest is to create a swiper-hydra package which depends on both swiper and hydra and add the functionality to swiper.

@abo-abo abo-abo closed this in fd4fd36 May 2, 2016

@abo-abo

This comment has been minimized.

Owner

abo-abo commented May 2, 2016

@DamienCassou I've updated the defhydra stub to install and load hydra in a better way. Could you please test it by uninstalling hydra, restarting Emacs and pressing C-o. You'll get a y/n prompt and if you say "yes", the hydra will be installed and ivy-hydra.el will be reloaded.

@syl20bnr

This comment has been minimized.

syl20bnr commented May 2, 2016

Nice ! thank you :-)

Le lundi 2 mai 2016, Oleh Krehel notifications@github.com a écrit :

@DamienCassou https://github.com/DamienCassou I've updated the defhydra
stub to install and load hydra in a better way. Could you please test it
by uninstalling hydra, restarting Emacs and pressing C-o. You'll get a
y/n prompt and if you say "yes", the hydra will be installed and
ivy-hydra.el will be reloaded.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#464 (comment)

-syl20bnr-

@DamienCassou

This comment has been minimized.

Contributor

DamienCassou commented May 4, 2016

@abo-abo thanks for this. Unfortunately, I'm currently back to helm because I had no time to invest in changing right now. I might have a second look later. Sorry about that.

@syl20bnr

This comment has been minimized.

syl20bnr commented May 15, 2016

@abo-abo Unfortunately this is not fixed, we still have the error. I understood why it happens in Spacemacs even if we install in alphabetical order (which means Hydra comes before Ivy). This is because Counsel (and soon many others) depends on Ivy so Ivy ends up being installed before Counsel and thus before Hydra.

I think the current approach is not viable because it puts the burden of package dependency on the users shoulder. Any package declaring a new dependency on Ivy can trigger this error so basically we should tell people to install Hydra first. Now imagine if other package maintainers start to play this game, we end up with a dependency hell for the user where any upgrade can break their configuration. This is simply not practical.

What I recommend in this particular case:

  • remove ivy-hydra.el from the Ivy recipe on MELPA
  • add the dependencies on both Hydra and Ivy in this file
  • create a new recipe on MELPA named ivy-hydra including this file.

This way it respects POLA principle, saves some documentation, some user's time and make their configuration more reliable. Also package.el makes it easily discoverable.

@syl20bnr

This comment has been minimized.

syl20bnr commented May 15, 2016

Also the current approach to ask for Hydra installation for the user is not very handy, lots of people manage their package list themselves. If they want the Ivy Hydra they just add the package that adds this functionality to their config.

Actually this approach can work but it can install the ivy-hydra package instead of hydra (hydra will be installed correctly by package.el). So all in all there is no loss of functionality.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented May 16, 2016

@syl20bnr

Any package declaring a new dependency on Ivy can trigger this error so basically we should tell people to install Hydra first.

What is the error you're referring to?

I think the current approach is not viable because it puts the burden of package dependency on the users shoulder.

Ultimately, the burden is always there.

create a new recipe on MELPA named ivy-hydra including this file.

This can work. Still, the problem still needs a Package-Recommends: functionality in package.el to be truly solved.

abo-abo added a commit that referenced this issue May 16, 2016

@syl20bnr

This comment has been minimized.

syl20bnr commented May 17, 2016

What is the error you're referring to?

Oops sorry, this is a different error than OP, in ivy config we (require 'ivy-hydra) and we get the following error:

Symbol's value as variable is void: hydra-ivy

Recompiling ivy-hydra.el fixes the issue. This is due to the fact that Ivy is installed very early as a dependency of Counsel and Hydra is installed later on (Spacemacs sorts the package list before installing them).

Since it looks like Hydra must be compiled before ivy-hydra.el I guess that we can rely on package.el for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment