Skip to content

Better name detection#1361

Merged
rgrinberg merged 4 commits intoocaml:masterfrom
rgrinberg:better-name-detection
Oct 2, 2018
Merged

Better name detection#1361
rgrinberg merged 4 commits intoocaml:masterfrom
rgrinberg:better-name-detection

Conversation

@rgrinberg
Copy link
Copy Markdown
Member

@rgrinberg rgrinberg commented Sep 30, 2018

The old name detection is confusing and also incorrectly detects situations where the prefix is unintended. This new scheme is more resilient and understands that many packages have a - as a separator.

Fix #1360

@rgrinberg rgrinberg requested review from a user and emillon September 30, 2018 18:38
@ghost
Copy link
Copy Markdown

ghost commented Oct 1, 2018

Note that this heuristic doesn't work for xenstore_transport, since it defines two packages xenstore-tool and xenstore_transport.

@emillon
Copy link
Copy Markdown
Collaborator

emillon commented Oct 1, 2018

If that's the only package in the list for which that heuristic doesn't work, it seems acceptable to just fix that one by hand.

@ghost
Copy link
Copy Markdown

ghost commented Oct 1, 2018

I guess. TBH I'm a bit wary about such automatic name detection altogether and I think it's better if the user explicitly specifies a project name in the dune-project file. That said, this is only when using the jbuilder binary which is now deprecated, so it seems fine if it helps minimizing the churn for already released packages.

@rgrinberg
Copy link
Copy Markdown
Member Author

I can improve it for xenstore as well. - are more common, but _ should be supported as well.

@rgrinberg rgrinberg force-pushed the better-name-detection branch from 1bb876b to d0bd008 Compare October 1, 2018 15:52
Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
Rather than rely on the shortest package being a prefix of the rest, we detect
the prefix and ensure that it doesn't have a trailing `-`

Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
Signed-off-by: Rudi Grinberg <rudi.grinberg@gmail.com>
@rgrinberg rgrinberg force-pushed the better-name-detection branch from d0bd008 to c369ec1 Compare October 2, 2018 14:26
@rgrinberg rgrinberg merged commit 2d1e663 into ocaml:master Oct 2, 2018
@rgrinberg rgrinberg deleted the better-name-detection branch October 2, 2018 14:30
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Oct 4, 2018
CHANGES:

- Do not fail if the output of `ocamlc -config` doesn't include
  `standard_runtime` (ocaml/dune#1326, @diml)

- Let `Configurator.V1.C_define.import` handle negative integers
  (ocaml/dune#1334, @Chris00)

- Re-execute actions when a target is modified by the user inside
  `_build` (ocaml/dune#1343, fix ocaml/dune#1342, @diml)

- Pass `--set-switch` to opam (ocaml/dune#1341, fix ocaml/dune#1337, @diml)

- Fix bad interaction between multi-directory libraries the `menhir`
  stanza (ocaml/dune#1373, fix ocaml/dune#1372, @diml)

- Integration with automatic formatters (ocaml/dune#1252, fix ocaml/dune#1201, @emillon)

- Better error message when using `(self_build_stubs_archive ...)` and
  `(c_names ...)` or `(cxx_names ...)` simultaneously.
  (ocaml/dune#1375, fix ocaml/dune#1306, @nojb)

- Improve name detection for packages when the prefix isn't an actual package
  (ocaml/dune#1361, fix ocaml/dune#1360, @rgrinberg)

- Support for new menhir rules (ocaml/dune#863, fix ocaml/dune#305, @fpottier, @rgrinberg)

- Do not remove flags when compiling compatibility modules for wrapped mode
  (ocaml/dune#1382, fix ocaml/dune#1364, @rgrinberg)

- Fix reason support when using `staged_pps` (ocaml/dune#1384, @charlesetc)

- Add support for `enabled_if` in `rule`, `menhir`, `ocamllex`,
  `ocamlyacc` (ocaml/dune#1387, @diml)

- Exit gracefully when a signal is received (ocaml/dune#1366, @diml)

- Load all defined libraries recursively into utop (ocaml/dune#1384, fix ocaml/dune#1344,
  @rgrinberg)

- Allow to use libraries `bytes`, `result` and `uchar` without `findlib`
  installed (ocaml/dune#1391, @nojb)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Oct 10, 2018
CHANGES:

- Do not fail if the output of `ocamlc -config` doesn't include
  `standard_runtime` (ocaml/dune#1326, @diml)

- Let `Configurator.V1.C_define.import` handle negative integers
  (ocaml/dune#1334, @Chris00)

- Re-execute actions when a target is modified by the user inside
  `_build` (ocaml/dune#1343, fix ocaml/dune#1342, @diml)

- Pass `--set-switch` to opam (ocaml/dune#1341, fix ocaml/dune#1337, @diml)

- Fix bad interaction between multi-directory libraries the `menhir`
  stanza (ocaml/dune#1373, fix ocaml/dune#1372, @diml)

- Integration with automatic formatters (ocaml/dune#1252, fix ocaml/dune#1201, @emillon)

- Better error message when using `(self_build_stubs_archive ...)` and
  `(c_names ...)` or `(cxx_names ...)` simultaneously.
  (ocaml/dune#1375, fix ocaml/dune#1306, @nojb)

- Improve name detection for packages when the prefix isn't an actual package
  (ocaml/dune#1361, fix ocaml/dune#1360, @rgrinberg)

- Support for new menhir rules (ocaml/dune#863, fix ocaml/dune#305, @fpottier, @rgrinberg)

- Do not remove flags when compiling compatibility modules for wrapped mode
  (ocaml/dune#1382, fix ocaml/dune#1364, @rgrinberg)

- Fix reason support when using `staged_pps` (ocaml/dune#1384, @charlesetc)

- Add support for `enabled_if` in `rule`, `menhir`, `ocamllex`,
  `ocamlyacc` (ocaml/dune#1387, @diml)

- Exit gracefully when a signal is received (ocaml/dune#1366, @diml)

- Load all defined libraries recursively into utop (ocaml/dune#1384, fix ocaml/dune#1344,
  @rgrinberg)

- Allow to use libraries `bytes`, `result` and `uchar` without `findlib`
  installed (ocaml/dune#1391, @nojb)

- Take argument to self_build_stubs_archive into account. (ocaml/dune#1395, @nojb)

- Fix bad interaction between `env` customization and vendored
  projects: when a vendored project didn't have its own `env` stanza,
  the `env` stanza from the enclosing project was in effect (ocaml/dune#1408,
  @diml)

- Fix stop early bug when scanning for watermarks (ocaml/dune#1423, @diml)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

jbuilder subst -p name leads to an error

2 participants