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

melpaPackages: match version of magit-popup to magit #33191

Merged
merged 1 commit into from
Dec 30, 2017

Conversation

benley
Copy link
Member

@benley benley commented Dec 29, 2017

Related to magit/magit#3286

Motivation for this change

I was getting weird errors trying to use magit features that involved magit-popup

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

# version of magit-popup needs to match magit
# https://github.com/magit/magit/issues/3286
magit = super.magit.override {
inherit (self.melpaPackages) magit-popup
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, I see the missing semicolon. Fix coming in a sec.

@lukateras
Copy link
Member

@GrahamcOfBorg eval

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Dec 30, 2017
@lukateras lukateras merged commit 823e191 into NixOS:master Dec 30, 2017
@ttuegel
Copy link
Member

ttuegel commented Dec 30, 2017

A word of warning: this only works as long as the user doesn't install magit-popup explicitly; if they do that, they will get the incompatible version. Actually, Emacs will try to install both versions simultaneously. That's impossible, so I'm not sure what will happen.

@lukateras
Copy link
Member

I've accepted this because melpaPackages already has similar overrides in a few other places:

inherit (self.melpaPackages) powerline;

inherit (self.melpaPackages) easy-kill;

inherit (self.melpaPackages) ess ctable popup;

@ttuegel
Copy link
Member

ttuegel commented Dec 30, 2017

I've accepted this because melpaPackages already has similar overrides in a few other places:

You were right to; this is the best we can do for now!

@benley
Copy link
Member Author

benley commented Dec 30, 2017

A word of warning: this only works as long as the user doesn't install magit-popup explicitly; if they do that, they will get the incompatible version. Actually, Emacs will try to install both versions simultaneously. That's impossible, so I'm not sure what will happen.

I've experimented with that scenario, and it appears that the older incompatible version wins out. It effectively just negates this workaround, I think.

@benley benley deleted the magit-popup-version branch December 30, 2017 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants