Remove wrong validation for switch names #3265
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rgrinberg
force-pushed
the
fix-3264
branch
2 times, most recently
from
9349acf to
e6a1965
Compare
rgrinberg
commented
Mar 15, 2020
ghost
mentioned this pull request
ghost
reviewed
Mar 16, 2020
| | Some s -> s | ||
| | None -> ( | ||
| let name = Filename.basename switch ^ Common.fdo_suffix base in | ||
| match Context_name.of_string_opt name with |
There was a problem hiding this comment.
Not specific to this PR, but I remember that once we discussed the naming of these functions and agreed on:
of_string : string -> tfor places where we are just converting between data structures and don't expect errorsparse_string : string -> (t, ...) resultfor places where we are parsing data coming from the outside world (here the...could just beunit)
There was a problem hiding this comment.
Indeed we have. Here's how interface looks like:
val of_string : string -> t
val parse_string_exn : Loc.t * string -> t
So we all need to do is to remove the _exn suffix from parsing.
Previously, switch names were validated to be valid context names. This is wrong as it prevent us from using local switches which are in fact specified by their path. We remove this unnecessary validation and now we can use local switches to define contexts. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Invalid target names will now show proper locations Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
Now, we always require local siwtchse to set the name. Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
rgrinberg
added a commit
to rgrinberg/opam-repository
that referenced
this pull request
Apr 10, 2020
…lugin, dune-private-libs and dune-glob (2.5.0) CHANGES: - Add a `--release` option meaning the same as `-p` but without the package filtering. This is useful for custom `dune` invocation in opam files where we don't want `-p` (ocaml/dune#3260, @diml) - Fix a bug introduced in 2.4.0 causing `.bc` programs to be built with `-custom` by default (ocaml/dune#3269, fixes ocaml/dune#3262, @diml) - Allow contexts to be defined with local switches in workspace files (ocaml/dune#3265, fix ocaml/dune#3264, @rgrinberg) - Delay expansion errors until the rule is used to build something (ocaml/dune#3261, fix ocaml/dune#3252, @rgrinberg, @diml) - [coq] Support for theory dependencies and compositional builds using new field `(theories ...)` (ocaml/dune#2053, @ejgallego, @rgrinberg) - From now on, each version of a syntax extension must be explicitely tied to a minimum version of the dune language. Inconsistent versions in a `dune-project` will trigger a warning for version <=2.4 and an error for versions >2.4 of the dune language. (ocaml/dune#3270, fixes ocaml/dune#2957, @voodoos) - [coq] Bump coq lang version to 0.2. New coq features presented this release require this version of the coq lang. (ocaml/dune#3283, @ejgallego) - Prevent installation of public executables disabled using the `enabled_if` field. Installation will now simply skip such executables instead of raising an error. (ocaml/dune#3195, @voodoos) - `dune upgrade` will now try to upgrade projects using versions <2.0 to version 2.0 of the dune language. (ocaml/dune#3174, @voodoos) - Add a `top` command to integrate dune with any toplevel, not just utop. It is meant to be used with the new `#use_output` directive of OCaml 4.11 (ocaml/dune#2952, @mbernat, @diml) - Allow per-package `version` in generated `opam` files (ocaml/dune#3287, @toots) - [coq] Introduce the `coq.extraction` stanza. It can be used to extract OCaml sources (ocaml/dune#3299, fixes ocaml/dune#2178, @rgrinberg) - Load ppx rewriters in dune utop and add pps field to toplevel stanza. Ppx extensions will now be usable in the toplevel (ocaml/dune#3266, fixes ocaml/dune#346, @stephanieyou) - Add a `(subdir ..)` stanza to allow evaluating stanzas in sub directories. (ocaml/dune#3268, @rgrinberg) - Fix a bug preventing one from running inline tests in multiple modes (ocaml/dune#3352, @diml) - Allow the use of the `%{profile}` variable in the `enabled_if` field of the library stanza. (ocaml/dune#3344, @mrmr1993) - Allow the use of `%{ocaml_version}` variable in `enabled_if` field of the library stanza. (ocaml/dune#3339, @voodoos) - Fix dune build freezing on MacOS when cache is enabled. (ocaml/dune#3249, fixes #ocaml/dune#2973, @artempyanykh)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, switch names were validated to be valid context names. This
is wrong as it prevent us from using local switches which are in fact
specified by their path. We remove this unnecessary validation and now
we can use local switches to define contexts.