RFC: Add a simple feature checking mechanism. #48764
Conversation
maleadt
added
the
speculative
| @test Base.has_feature("features") | ||
| f() = Base.has_feature("features") | ||
| code = sprint(code_warntype, f, Tuple{}) | ||
| @test occursin("(\"features\")::Core.Const(true)", code) |
There was a problem hiding this comment.
There probably is a better way to check for const-inferred results.
There was a problem hiding this comment.
symbols for example would be a lot better here.
There was a problem hiding this comment.
Why? That wouldn't change the outcome of the call being folded away (i.e., it'd still contain a const-inferred call to Base.has_features, now just passing :features instead of "features"). Or do you mean returning a symbol instead of a const boolean?
There was a problem hiding this comment.
strings generally have worse effects than symbols since they're technically mutable
|
I love this idea and I'm surprised we didn't add this ages ago. I guess you have to remember to add the feature flag, but we already have a "checklist" of sorts we go thru when adding new stuff (tests, docs, news item, etc.). |
|
I like this in principle, but I wonder if the use-case is a bit narrow and may in practice be too much overhead for what it adds? Just thinking about how small some new features could be, or how they could change in dev builds; and then once released, they won't be removed. It would make more sense to me if there were components sometimes present and sometimes not, e.g. due to running on different platforms. If the main issue is how packages interact with PRs to Julia in dev builds, maybe there's a better solution that just addresses that specifically? Perhaps dev builds could have a list of merged PR numbers (that gets reset on release?), and you could check for the presence of that PR number? Just brainstorming here. |
Sure, that would be fine, but how do you automate that in a reliable way? It also needs to work for builds of an unmerged PR, and I don't think there's a way to determine the PR number in such a situation without using the GitHub API. It also needs to work outside of GitHub, so we can't rely on there being a merge commit. |
This PR adds a simple mechanism to check if a build of Julia has certain features:
The purpose of this function is to use this to drive conditional code in packages. Currently, we use:
VERSIONchecks, e.g.if VERSION >= v"1.10.0-DEV.649", which doesn't tell me much if I read this codeif isdefined(Base, :get_extension)The other problem with version checks is that they need the Julia PR to be merged before we can adapt packages. For example, in #48611 PkgEval revealed quite some breakage. I'd like to fix packages without having to merge the (re-landed version of) that PR, but there isn't any easy way to check for the changed behavior without looking deeply into compiler internals, which is ugly. In fact, I think the
isdefined(Base, :get_extension)is ugly too, and a call toBase.has_feature("extensions")would be much easier on the eyes.Currently, the set of
featuresis hard-coded, just as a starting point for this PR. I considered automating its generation during build, e.g. making it possible to check for PR numbers, but AFAIK that would require use of the GitHub API during build which isn't great.