Update safe properties in .status.el (whitelist approach) #2238
Conversation
|
Not sure about the backlist idea here... the current tradeoff is towards making it as easy as possible to author recipes, at the expenses of loose control and sometimes harder to debug situations... that said, I'm not doing enough debugging here to have a horse in this race, so I'll trust you to make the right choice! |
Previously, the recipe in .status.el was only updated on init, since only some properties are safe for change on init the unsafe ones were never updated. Therefore, we classify addtional properties as safe fo change on update, and update .status.el after el-get-update too. * el-get-core.el (el-get-as-string): Handle number argument. * el-get-status.el (el-get-status-init-whitelist): Rename from el-get-status-recipe-update-whitelist. (el-get-status-update-whitelist): New constant. (el-get-classify-new-properties): Add OPERATION argument to choose white whitelist to use. The :builtin property is safe to change if it doesn't change the :type. (el-get-diagnosis-properties, el-get-merge-properties-into-status): Add OPERATION argument. * el-get.el (el-get-do-init): Call el-get-merge-properties-into-status with OPERATION 'init. (el-get-post-update-build): Call el-get-merge-properties-into-status with OPERATION 'update.
* el-get-status.el (el-get-classify-new-properties): Consider :url changes update safe for :type http*, builtin. Consider :builtin changes safe if not crossing current Emacs version.
npostavs
force-pushed
the
update-safe-props-v2
branch
from
9450cdb
to
11c7d23
Compare
npostavs
force-pushed
the
update-safe-props-v2
branch
from
11c7d23
to
d774318
Compare
Make it say exactly which operation(s) are required to update the recipe. * el-get-status.el (el-get-classify-new-properties): Return a partition of updates for each operation instead of (dis)allowed lists for a given operation. (el-get-diagnosis-properties): Return the required operation(s) instead of update-p. (el-get-merge-properties-into-status): Make the warning only suggest to update if that would actually succeed given the properties needing updating.
npostavs
force-pushed
the
update-safe-props-v2
branch
from
300473b
to
5be0271
Compare
Changing to a blacklist shouldn't affect recipe authoring at all. The difficulty would be in writing the blacklist itself, which has to be different per el-get Or maybe it's better to just add a manual override to To realize that is safe would require some url parsing, which really seems like overkill... |
|
Another thing to consider is updating only some of the properties, currently it's all-or-nothing so some property changes deemed "unsafe" can hold the rest "hostage". I just hit this now after updating magit, the |
with-editor is registered as an individual package on MELPA but it is provided by magit. The version from MELPA complains because the el-get version of dash is installed instead of the MELPA version. By having magit provide with-editor we avoid the issues of having MELPA's version installed.
Fixes #1817.
This extends the whitelist with a 2nd update whitelist, with special cases for
:builtin(safe if we haven't "crossed" the Emac version) and:url(safe for some recipe types).I'm actually considering switching to a blacklist approach instead. Consider that removing (or adding) properties like
:providethat are unknown to el-get is obviously perfectly safe. Essentially, properties that are not used by a type's update method are safe to change.The downside of blacklists is it's more dangerous if we get the list wrong of course. But, leaving a stale recipe is also bad if we want to use it (eg a solution for #2232 might want to do that).
Thoughts?