Promote the toolchain feature flag to official feature #11053
Conversation
|
cc @maiste I think the binary release would need to be updated if this is merged. |
ElectreAAS
requested review from
Leonidas-from-XIV,
gridbugs,
maiste,
moyodiallo and
rgrinberg
Done! |
|
Instead of removing it, you could also just flip the default. Not sure if we'd like to keep this mode, but we don't have to delete it right away |
There was a problem hiding this comment.
Depending on if you prefer to go with @rgrinberg solution or not. But otherwise, LGTM.
src/dune_rules/pkg_toolchain.ml
Outdated
| let is_compiler name = | ||
| let module Package_name = Dune_pkg.Package_name in | ||
| let compiler_package_names = | ||
| (* TODO don't hardcode these names here *) |
There was a problem hiding this comment.
You could probably check this by looking at the compiler value in the list of flags: (in the opam file).
There was a problem hiding this comment.
Indeed, that would be cleaner. However I can't find a way to transmute a Dune_lang.Package_name.t into a Dune_pkg.Opam_file.t = OpamParserTypes.FullPos.opamfile, which would allow looking into the flags
As this is not related to the core of this PR, I'll leave the TODO for now
There was a problem hiding this comment.
What you're trying is not possible. A Package_name.t is just the name of a package (basically a fancy way to say string); to get an Opam_file.t you'd need to load an opam repository, look up a Package_name.t and a Package_version.t into a specific OPAM file and then check its flags there.
I also vaguely recall that there was some issue about determining the packages affected from the compiler flag, but I don't recall the details anymore.
ElectreAAS
force-pushed
the
promote-feature-flag
branch
from
b29f3c6 to
28fbd42
Compare
Signed-off-by: Ambre Austen Suhamy <ambre@tarides.com>
Signed-off-by: Ambre Austen Suhamy <ambre@tarides.com>
ElectreAAS
force-pushed
the
promote-feature-flag
branch
from
28fbd42 to
bcbff43
Compare
|
I ended up with only the soft approach: make it enabled by default but not delete it entirely. Also this is rebased on main now |
6 participants
This PR sports
2 commits, the first one being a less-committed approach to the situation, the second a more complete one.just the one commit that makes the flag enabled by default.There seems to be sentiment around the dune team (cc @Leonidas-from-XIV @rgrinberg @gridbugs in particular) that:
Thus, I propose that we
get rid ofpromote the feature flag.Note that this is a BREAKING change for users who use./configure --toolchains disabledobviously, but also./configure --toolchains enabledas the flag will no longer get recognized.Not a breaking change anymore, I just changed the default.