Skip to content

Fix for #2990#3457

Merged
rgrinberg merged 3 commits intoocaml:masterfrom
greedy:fix-2990
May 6, 2020
Merged

Fix for #2990#3457
rgrinberg merged 3 commits intoocaml:masterfrom
greedy:fix-2990

Conversation

@greedy
Copy link
Copy Markdown
Contributor

@greedy greedy commented May 5, 2020

The second argument of a 'diff?' action is removed from the set of
targets as the action consumes (i.e. deletes) the file.

I'm not 100% confident that the implementation of ( +<- ) is right for all the cases since I'm not very familiar with these abstractions around the different ASTs in dune.

@greedy greedy force-pushed the fix-2990 branch 3 times, most recently from 9443c00 to 32de58b Compare May 5, 2020 23:54
greedy and others added 3 commits May 5, 2020 20:46
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
The second argument of a 'diff?' action is removed from the set of
targets as the action consumes (i.e. deletes) the file.

Signed-off-by: Geoffrey Reedy <geoff@programmer-monk.net>
The second argument must be in the build dir, as we cannot produce
targets outside the build dir.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg
Copy link
Copy Markdown
Member

Thanks for your contribution. It was a great PR already, but I made a few minor adjustments:

  • Added a CHANGES entry to make sure users are aware of the bug fix.
  • Split the test into its own commit so that we can see how your commit fixes the bug.
  • Made the type of the diff action more precise. Since the second argument to diff can be written to, it must be a Path.Build.t rather than a Path.t.

Keep the PR's coming!

@rgrinberg rgrinberg merged commit 9e62f10 into ocaml:master May 6, 2020
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 6, 2020
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 7, 2020
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Jun 7, 2020
…lugin, dune-private-libs and dune-glob (2.6.0)

CHANGES:

- Fix a bug where valid lib names in `dune init exec --libs=lib1,lib2`
  results in an error. (ocaml/dune#3444, fix ocaml/dune#3443, @bikallem)

- Add and `enabled_ if` field to the `install` stanza. Enforce the same variable
  restrictions for `enabled_if` fields in the `executable` and `install` stanzas
  than in the `library` stanza. When using dune lang < 2.6, the usage of
  forbidden variables in executables stanzas with only trigger a warning to
  maintain compatibility. (ocaml/dune#3408 and ocaml/dune#3496, fixes ocaml/dune#3354, @voodoos)

- Insert a constraint one the version of dune when the user explicitly
  specify the dependency on dune in the `dune-project` file (ocaml/dune#3434 ,
  fixes ocaml/dune#3427, @diml)

- Generate correct META files for sub-libraries (of the form `lib.foo`) that
  contain .js runtime files. (ocaml/dune#3445, @hhugo)

- Add a `(no-infer ...)` action that prevents inference of targets and
  dependencies in actions. (ocaml/dune#3456, fixes ocaml/dune#2006, @roddyyaga)

- Correctly infer targets for the `diff?` action. (ocaml/dune#3457, fixes ocaml/dune#2990, @greedy)

- Fix `$ dune print-rules` crashing (ocaml/dune#3459, fixes ocaml/dune#3440, @rgrinberg)

- Simplify js_of_ocaml rules using js_of_ocaml.3.6 (ocaml/dune#3375, @hhugo)

- Add a new `ocaml-merlin` subcommand that can be used by Merlin to get
  configuration directly from dune instead of using `.merlin` files. (ocaml/dune#3395,
  @voodoos)

- Remove experimental variants feature and make default implementations part of
  the language (ocaml/dune#3491, fixes ocaml/dune#3483, @rgrinberg)
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.

2 participants