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

Emit better warning for overridden recipes #674

Conversation

progfolio
Copy link
Contributor

@progfolio progfolio commented Jan 12, 2021

See: #523, #500

Note this change required hoisting the build cache code above the recipe registration code in order to satisfy the byte-compiler.

Test Case
(straight-bug-report
  :user-dir "better-warning-recipe-override.straight"
  :pre-bootstrap 
  (setq straight-repository-user "progfolio" straight-repository-branch "feat/better-warning-for-recipe-override")
  :post-bootstrap 
  (straight-use-package 'wikinforg)
  (straight-use-package
   '(wikinfo :type git :host github :repo "user/wikinfo")))
  • Test run at: 2021-01-12 16:37:26
  • system-type: gnu/linux
  • straight-version: prerelease (HEAD -> feat/better-warning-for-recipe-override, fork/feat/better-warning-for-recipe-override) b11b20d 2021-01-12
  • emacs-version: GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.17.4, Xaw3d scroll bars) of 2020-12-31
Output
Bootstrapping straight.el...
Bootstrapping straight.el...done
Rebuilding all packages due to build cache schema change
Looking for gnu-elpa-mirror recipe → Cloning melpa...
Looking for gnu-elpa-mirror recipe → Cloning melpa...done
Looking for emacsmirror-mirror recipe → Cloning gnu-elpa-mirror...
Looking for emacsmirror-mirror recipe → Cloning gnu-elpa-mirror...done
Looking for emacsmirror-mirror recipe → Cloning el-get...
Looking for emacsmirror-mirror recipe → Cloning el-get...done
Looking for straight recipe → Cloning emacsmirror-mirror...
Looking for straight recipe → Cloning emacsmirror-mirror...done
Building straight...
Building straight...done

Test run with version: prerelease (HEAD -> feat/better-warning-for-recipe-override, origin/feat/better-warning-for-recipe-override) b11b20d 2021-01-12
Cloning wikinforg...
Cloning wikinforg...done
Building wikinforg...
Building wikinforg → Cloning wikinfo...
Building wikinforg → Cloning wikinfo...done
Building wikinforg → Building wikinfo...
Building wikinforg → Building wikinfo...done
Building wikinforg → Cloning org...
Building wikinforg → Cloning org...done
Building wikinforg → Building org...
Building wikinforg → Building org...done
Building wikinforg...
Building wikinforg...done

Warning (straight): "wikinfo" recipe ignored: already installed as a dependency of "wikinforg"

@progfolio progfolio force-pushed the feat/better-warning-for-recipe-override branch from 8cf9915 to fe7e167 Compare January 12, 2021 14:33
straight.el Outdated Show resolved Hide resolved
@progfolio progfolio force-pushed the feat/better-warning-for-recipe-override branch from fe7e167 to 4a1aa02 Compare January 12, 2021 14:49
straight.el Show resolved Hide resolved
straight.el Outdated Show resolved Hide resolved
@progfolio progfolio force-pushed the feat/better-warning-for-recipe-override branch from 4a1aa02 to 25ddf4c Compare January 12, 2021 21:17
@progfolio progfolio force-pushed the feat/better-warning-for-recipe-override branch from 25ddf4c to b11b20d Compare January 12, 2021 21:24
@raxod502
Copy link
Member

Well, I definitely can't argue with the new message---it's clear and to the point. One thing I will note is that the behavior is not quite how the message describes. We actually do accept the new recipe, and it overrides the old one. However, this probably doesn't have the effect the user was expecting, since the package will have already been cloned and built using the old recipe. Perhaps we should update the recipe resolution system to actually ignore the new recipe. On the other hand, it is very handy to be able to re-evaluate a straight-use-package form to update its recipe interactively, then e.g. run M-x straight-normalize-package to apply the change. So if we were to try to change the behavior, we should make sure we don't break this kind of use case.

@progfolio
Copy link
Contributor Author

Good points all around. I'll take a look at modifying the recipe resolution to accommodate both use cases.

@progfolio progfolio closed this Jun 11, 2021
@progfolio progfolio deleted the branch radian-software:develop June 11, 2021 18:49
@progfolio progfolio reopened this Jun 11, 2021
@progfolio progfolio closed this Jun 11, 2021
@progfolio progfolio deleted the branch radian-software:develop June 11, 2021 21:25
@progfolio progfolio reopened this Jun 11, 2021
@progfolio progfolio closed this Sep 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants