Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow . in c_names and cxx_names #2036

Merged
merged 2 commits into from
Apr 9, 2019
Merged

Conversation

rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Apr 9, 2019

Let's get a confirmation fro m @diml that this should be allowed. The fix itself is trivial.

Fix #2033

@rgrinberg rgrinberg requested review from a user and aalekseyev April 9, 2019 08:48
@rgrinberg rgrinberg changed the title Fix 2033 Fix #2033 Apr 9, 2019
@rgrinberg rgrinberg changed the title Fix #2033 Allow . in c_names and cxx_names Apr 9, 2019
Copy link
Collaborator

@aalekseyev aalekseyev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason the test is negative (showing the error message) rather than positive (showing a successful compilation)?

Assuming the compilation succeeds when the file is added, this change seems obviously good to me.

@rgrinberg
Copy link
Member Author

The reason is that I forgot to promote. Thanks for noticing.

@rgrinberg
Copy link
Member Author

Sorry @ejgallego I must have added you as a reviewer accidentally

@rgrinberg rgrinberg force-pushed the fix-2033 branch 3 times, most recently from 2cb6a98 to 6bc722b Compare April 9, 2019 11:24
@ghost
Copy link

ghost commented Apr 9, 2019

I can't think of a reason why this shouldn't be allowed. I can't remember either why it wasn't allowed in the first place.

@rgrinberg rgrinberg merged commit d300cde into ocaml:master Apr 9, 2019
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 9, 2019
CHANGES:

- Warn when generated `.merlin` does not reflect the preprocessing
  specification. This occurs when multiple stanzas in the same directory use
  different preprocessing specifications. This warning can now be disabled with
  `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg)

- Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956,
  @emillon)

- Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg)

- Add support for library variants and default implementations. (ocaml/dune#1900,
  @TheLortex)

- Add experimental `$ dune init` command. This command is used to create or
  update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder)

- Experimental Coq support (fix ocaml/dune#1466, @ejgallego)

- Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix
  ocaml/dune#1973 @rgrinberg)

- Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997,
  @rgrinberg)

- Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008,
  @rgrinberg)

- Warn instead of failing if an opam file fails to parse. This opam file can
  still be used to define scope. (ocaml/dune#2023, @rgrinberg)

- Do not crash if unable to read a directory when traversing to find root
  (ocaml/dune#2024, @rgrinberg)

- Do not exit dune if some source directories are unreadable. Instead, warn the
  user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg)

- Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent
  `binaries` fields would be ignored, but instead they should be combined.
  (ocaml/dune#2029, @rgrinberg)

- Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Apr 9, 2019
CHANGES:

- Warn when generated `.merlin` does not reflect the preprocessing
  specification. This occurs when multiple stanzas in the same directory use
  different preprocessing specifications. This warning can now be disabled with
  `allow_approx_merlin` (ocaml/dune#1947, fix ocaml/dune#1946, @rgrinberg)

- Watch mode: display "Success" in green and "Had errors" in red (ocaml/dune#1956,
  @emillon)

- Fix glob dependencies on installed directories (ocaml/dune#1965, @rgrinberg)

- Add support for library variants and default implementations. (ocaml/dune#1900,
  @TheLortex)

- Add experimental `$ dune init` command. This command is used to create or
  update project boilerplate. (ocaml/dune#1448, fixes ocaml/dune#159, @shonfeder)

- Experimental Coq support (fix ocaml/dune#1466, @ejgallego)

- Install .cmi files of private modules in a `.private` directory (ocaml/dune#1983, fix
  ocaml/dune#1973 @rgrinberg)

- Fix `dune subst` attempting to substitute on directories. (ocaml/dune#2000, fix ocaml/dune#1997,
  @rgrinberg)

- Do not list private modules in the generated index. (ocaml/dune#2009, fix ocaml/dune#2008,
  @rgrinberg)

- Warn instead of failing if an opam file fails to parse. This opam file can
  still be used to define scope. (ocaml/dune#2023, @rgrinberg)

- Do not crash if unable to read a directory when traversing to find root
  (ocaml/dune#2024, @rgrinberg)

- Do not exit dune if some source directories are unreadable. Instead, warn the
  user that such directories need to be ignored (ocaml/dune#2004, fix ocaml/dune#310, @rgrinberg)

- Fix nested `(binaries ..)` fields in the `env` stanza. Previously, parent
  `binaries` fields would be ignored, but instead they should be combined.
  (ocaml/dune#2029, @rgrinberg)

- Allow "." in `c_names` and `cxx_names` (ocaml/dune#2036, fix ocaml/dune#2033, @rgrinberg)

- Format rules: if a dune file uses OCaml syntax, do not format it.
  (ocaml/dune#2014, fix ocaml/dune#2012, @emillon)
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.

dune does not find C files with a dot in the name
3 participants