More progress on the opam generation #2091

rgrinberg · 2019-04-28T11:45:14Z

Add package stanza
Add x-generated-by header
Move "opam" sub field to the package fields. This is what will enable the correction rules I suppose.

ghost · 2019-04-29T11:35:17Z

If we generate x-generated-by: "dune-<version>", aren't we going to create some noise? I.e. everytime we upgrade dune, everybody will have to commit a new opam file.

Khady · 2019-04-29T11:58:20Z

Would it make sense to have the dune-version as it it in the dune-project file?

rgrinberg · 2019-04-29T12:09:13Z

Would it make sense to have the dune-version as it it in the dune-project file?

I think this bit can always be discovered by looking at the source of the package. Unlike the the dune version used to generate the file

rgrinberg · 2019-04-29T12:25:20Z

I got rid of the generated header.

rgrinberg · 2019-04-29T12:52:34Z

Okay, this is now ready @jonludlam @avsm @diml

dune-project

avsm · 2019-04-29T13:15:03Z

Agreed that the x-generated-by is a bit ugly, and so unnecessary.

rgrinberg · 2019-04-30T08:55:17Z

@andreypopp Am I correct in thinking that you do not want the generation of opam files? you simply want to do without them? If so, then we currently don't have a way to do that.

rgrinberg · 2019-04-30T11:16:03Z

I believe this PR is ready now. It's now possible to work without opam files as well. Doc updates are pending

@avsm @jonludlam review would be appreciated.

jonludlam

This looks good to me.

Might be worth noting what we've still got to do before we can generate dune's opam file completely:

homepage
bug-reports
maintainer
build

of these I think build is going to be the most awkward.

rgrinberg · 2019-05-01T10:56:21Z

homepage
bug-reports
maintainer

I propose that these fields be just set for the project itself.

build

I've sent an email regarding this one.

rgrinberg · 2019-05-01T12:12:58Z

There's now support for those 3 fields. Now we just need to fill in a default for build when possible.

rgrinberg · 2019-05-02T12:48:51Z

@diml @jonludlam this is ready for another round of review:

I removed expected files and promotions
I've added support for filling in the build field
I've added support for an opam.template file as Jeremie requested

ghost · 2019-05-02T13:23:55Z

Ok, I'll have a look shortly

jonludlam · 2019-05-02T14:51:13Z

Looks great! Thanks!

ghost · 2019-05-06T07:44:08Z

The default should indeed be to not generate the files.

src/opam_file.ml

avsm · 2019-05-06T09:16:07Z

This opam 2.1 fix to dynamically query opam files is not coded yet and years out from being something we can depend on (since we have to support opam 2.0 for ages yet). So making the default to not generate means that 99% of users will now have to type it into their dune-project files and keep it there for some time. But I can live with that...

ghost · 2019-05-06T09:22:53Z

We don't need to wait for years. As long as dune-release can ask dune to generate the opam files at release time, then users can start deleting opam files from their repos as soon as opam supports the feature. When doing so, they will prevent their packages from being dev-pinned by opam 2.0 users. However, it seems to me that this choice should be left to individual maintainers.

avsm · 2019-05-06T09:40:57Z

Yes that's true -- and it de-risks the feature in the short term by not generating files by default should there be some oddness that we haven't found yet. I'm ok with the default for generation being off.

rgrinberg · 2019-05-06T11:04:14Z

Okay, I think this is ready. @diml I ended up going with the simple string concatenation with the original template. You are right that it simplifies the implementation.

doc/opam.rst

ghost · 2019-05-06T14:46:17Z

There is one last detail to take care of: before dune would read the package version from the opam files and store this version in generated META files and use it to expand %{version:foo}. Now that dune generate opam files, it no longer does that, which seems natural. However, currently dune-release prepend the version to the various opam files. That means that META files produced by dune will no longer contain version information and %{version:foo} will now be the empty string.

The quickest fix is to change dune-release so that it appends a (version ...) field to the dune-project file. /cc @samoht @NathanReb, does that seem good to you?

NathanReb · 2019-05-06T15:02:38Z

That seems reasonable to me although I wonder if a command line option on the dune side, which dune-release would use to pass the version wouldn't be a better or at least a less hacky solution.

ghost · 2019-05-06T15:09:26Z

@NathanReb when would dune-release pass this option to dune?

Which makes me thinks: we need to make dune subst append the (version ...) field as well to the dune-project file. Otherwise we'll have the same issue when the package is dev-pinned.

NathanReb · 2019-05-06T15:18:17Z

I was thinking when generating the .opam file but that won't do the trick for the %{version:foo} unless we also change the generated build field of the opam file so that version is passed to the build command but we can't really do that since we want to allow custom values for this field.

It seems pretty fragile anyway so we indeed can't do that.

If dune subst does append the (version ...) field to the dune-project then dune-release could use that instead of appending the field on its own which feels better.

Would that work or am I missing something again?

ghost · 2019-05-06T15:36:41Z

Does dune-release call dune subst though? I thought it was editing the files on its own. Maybe that's something that should change though, so that the behaviors could be better aligned.

* remove correction of existing opam files. the opam file is now generated * add missing fields such as homepage, maintainer, bug_reports * add a dune-project option to explicitly enable the generation of opam files * Add .template file for users to provide their own custom fields Signed-off-by: Rudi Grinberg <[email protected]>

Signed-off-by: Jeremie Dimino <[email protected]>

rgrinberg · 2019-05-07T03:15:53Z

Now that dune generate opam files, it no longer does that, which seems natural.

Did I break this accidentally somewhere? Seems like dune can still read the version information from the opam file.

I will merge this PR to prevent rebase headaches down the line, but we'll need to solve this issue before releasing.

@aalekseyev

CHANGES: - Restricted the set of variables available for expansion in the destination filename of `install` stanza to simplify implementation and avoid dependency cycles. (ocaml/dune#2073, @aalekseyev, @diml) - [menhir] call menhir from context root build_dir (ocaml/dune#2067, @ejgallego, review by @diml, @rgrinberg) - [coq] Add `coq.pp` stanza to help with pre-processing of grammar files (ocaml/dune#2054, @ejgallego, review by @rgrinberg) - Add a new more generic form for the *promote* mode: `(promote (until-clean) (into <dir>))` (ocaml/dune#2068, @diml) - Allow to promote only a subset of the targets via `(promote (only <pred>))`. For instance: `(promote (only *.mli))` (ocaml/dune#2068, @diml) - Improve the behavior when a strict subset of the targets of a rule is already in the source tree for projects using the dune language < 1.10 (ocaml/dune#2068, fixes ocaml/dune#2061, @diml) - With lang dune >= 1.10, rules in standard mode are no longer allowed to produce targets that are present in the source tree. This has been a warning for long enough (ocaml/dune#2068, @diml) - Allow %{...} variables in pps flags (ocaml/dune#2076, @mlasson review by @diml and @aalekseyev). - Add a 'cookies' option to ppx_rewriter/deriver flags in library stanzas. This allow to specify cookie requests from variables expanded at each invocation of the preprocessor. (ocaml/dune#2106, @mlasson @diml) - Add more opam metadata and use it to generate `.opam` files. In particular, a `package` field has been added to specify package specific information. (ocaml/dune#2017, ocaml/dune#2091, @avsm, @jonludlam, @rgrinberg) - Clean up the special support for `findlib.dynload`. Before, Dune would simply match on the library name. Now, we only match on the findlib package name when the library doesn't come from Dune. Someone writing a library called `findlib.dynload` with Dune would have to add `(special_builtin_support findlib_dynload)` to trigger the special behavior. (ocaml/dune#2115, @diml) - Install the `future_syntax` preprocessor as `ocaml-syntax-shims.exe` (ocaml/dune#2125, @rgrinberg) - Hide full command on errors and warnings in development and show them in CI. (detected using the `CI` environment variable). Commands for which the invocation might be omitted must output an error prefixed with `File `. Add an `--always-show-command-line` option to disable this behavior and always show the full command. (ocaml/dune#2120, fixes ocaml/dune#1733, @rgrinberg) - In `dune-workspace` files, add the ability to choose the host context and to create duplicates of the default context with different settings. (ocaml/dune#2098, @TheLortex, review by @diml, @rgrinberg and @aalekseyev) - Add support for hg in `dune subst` (ocaml/dune#2135, @diml) - Don't build documentation for implementations of virtual libraries (ocaml/dune#2141, fixes ocaml/dune#2138, @jonludlam) - Fix generation of the `-pp` flag in .merlin (ocaml/dune#2142, @rgrinberg) - Make `dune subst` add a `(version ...)` field to the `dune-project` file (ocaml/dune#2148, @diml) - Add the `%{os_type}` variable, which is a short-hand for `%{ocaml-config:os_type}` (ocaml/dune#1764, @diml) - Allow `enabled_if` fields in `library` stanzas, restricted to the `%{os_type}`, `%{model}`, `%{architecture}`, `%{system}` variables (ocaml/dune#1764, ocaml/dune#2164 @diml, @rgrinberg) - Fix `chdir` on external and source paths. Dune will also fail gracefully if the external or source path does not exist (ocaml/dune#2165, fixes ocaml/dune#2158, @rgrinberg) - Support the `.cc` extension fro C++ sources (ocaml/dune#2195, fixes ocaml/dune#83, @rgrinberg) - Run `ocamlformat` relative to the context root. This improves the locations of errors. (ocaml/dune#2196, fixes ocaml/dune#1370, @rgrinberg) - Fix detection of `README`, `LICENSE`, `CHANGE`, and `HISTORY` files. These would be undetected whenever the project was nested in another workspace. (ocaml/dune#2194, @rgrinberg) - Fix generation of `.merlin` whenever there's more than one stanza with the same ppx preprocessing specification (ocaml/dune#2209 ,fixes ocaml/dune#2206, @rgrinberg) - Fix generation of `.meriln` in the presence of the `copy_files` stanza and preprocessing specifications of other stanazs. (ocaml/dune#2211, fixes ocaml/dune#2206, @rgrinberg)

@aalekseyev

CHANGES: - Restricted the set of variables available for expansion in the destination filename of `install` stanza to simplify implementation and avoid dependency cycles. (ocaml/dune#2073, @aalekseyev, @diml) - [menhir] call menhir from context root build_dir (ocaml/dune#2067, @ejgallego, review by @diml, @rgrinberg) - [coq] Add `coq.pp` stanza to help with pre-processing of grammar files (ocaml/dune#2054, @ejgallego, review by @rgrinberg) - Add a new more generic form for the *promote* mode: `(promote (until-clean) (into <dir>))` (ocaml/dune#2068, @diml) - Allow to promote only a subset of the targets via `(promote (only <pred>))`. For instance: `(promote (only *.mli))` (ocaml/dune#2068, @diml) - Improve the behavior when a strict subset of the targets of a rule is already in the source tree for projects using the dune language < 1.10 (ocaml/dune#2068, fixes ocaml/dune#2061, @diml) - With lang dune >= 1.10, rules in standard mode are no longer allowed to produce targets that are present in the source tree. This has been a warning for long enough (ocaml/dune#2068, @diml) - Allow %{...} variables in pps flags (ocaml/dune#2076, @mlasson review by @diml and @aalekseyev). - Add a 'cookies' option to ppx_rewriter/deriver flags in library stanzas. This allow to specify cookie requests from variables expanded at each invocation of the preprocessor. (ocaml/dune#2106, @mlasson @diml) - Add more opam metadata and use it to generate `.opam` files. In particular, a `package` field has been added to specify package specific information. (ocaml/dune#2017, ocaml/dune#2091, @avsm, @jonludlam, @rgrinberg) - Clean up the special support for `findlib.dynload`. Before, Dune would simply match on the library name. Now, we only match on the findlib package name when the library doesn't come from Dune. Someone writing a library called `findlib.dynload` with Dune would have to add `(special_builtin_support findlib_dynload)` to trigger the special behavior. (ocaml/dune#2115, @diml) - Install the `future_syntax` preprocessor as `ocaml-syntax-shims.exe` (ocaml/dune#2125, @rgrinberg) - Hide full command on errors and warnings in development and show them in CI. (detected using the `CI` environment variable). Commands for which the invocation might be omitted must output an error prefixed with `File `. Add an `--always-show-command-line` option to disable this behavior and always show the full command. (ocaml/dune#2120, fixes ocaml/dune#1733, @rgrinberg) - In `dune-workspace` files, add the ability to choose the host context and to create duplicates of the default context with different settings. (ocaml/dune#2098, @TheLortex, review by @diml, @rgrinberg and @aalekseyev) - Add support for hg in `dune subst` (ocaml/dune#2135, @diml) - Don't build documentation for implementations of virtual libraries (ocaml/dune#2141, fixes ocaml/dune#2138, @jonludlam) - Fix generation of the `-pp` flag in .merlin (ocaml/dune#2142, @rgrinberg) - Make `dune subst` add a `(version ...)` field to the `dune-project` file (ocaml/dune#2148, @diml) - Add the `%{os_type}` variable, which is a short-hand for `%{ocaml-config:os_type}` (ocaml/dune#1764, @diml) - Allow `enabled_if` fields in `library` stanzas, restricted to the `%{os_type}`, `%{model}`, `%{architecture}`, `%{system}` variables (ocaml/dune#1764, ocaml/dune#2164 @diml, @rgrinberg) - Fix `chdir` on external and source paths. Dune will also fail gracefully if the external or source path does not exist (ocaml/dune#2165, fixes ocaml/dune#2158, @rgrinberg) - Support the `.cc` extension fro C++ sources (ocaml/dune#2195, fixes ocaml/dune#83, @rgrinberg) - Run `ocamlformat` relative to the context root. This improves the locations of errors. (ocaml/dune#2196, fixes ocaml/dune#1370, @rgrinberg) - Fix detection of `README`, `LICENSE`, `CHANGE`, and `HISTORY` files. These would be undetected whenever the project was nested in another workspace. (ocaml/dune#2194, @rgrinberg) - Fix generation of `.merlin` whenever there's more than one stanza with the same ppx preprocessing specification (ocaml/dune#2209 ,fixes ocaml/dune#2206, @rgrinberg) - Fix generation of `.merlin` in the presence of the `copy_files` stanza and preprocessing specifications of other stanazs. (ocaml/dune#2211, fixes ocaml/dune#2206, @rgrinberg) - Run `refmt` from the context's root directory. This improves error messages in case of syntax errors. (ocaml/dune#2223, @rgrinberg) - In .merlin files, don't pass `-dump-ast` to the `future_syntax` preprocessor. Merlin doesn't seem to like it when binary AST is generated by a `-pp` preprocessor. (ocaml/dune#2236, @aalekseyev) - `dune install` will verify that all files mentioned in all .install files exist before trying to install anything. This prevents partial installation of packages (ocaml/dune#2230, @rgrinberg)

rgrinberg requested review from avsm, jonludlam and a user April 28, 2019 11:45

rgrinberg requested a review from emillon as a code owner April 28, 2019 11:45

rgrinberg force-pushed the dune-opam branch from 5956d53 to d87cea8 Compare April 29, 2019 12:24

avsm reviewed Apr 29, 2019

View reviewed changes

dune-project Show resolved Hide resolved

avsm mentioned this pull request Apr 29, 2019

Improve CI tarides/opam-monorepo#18

Merged

3 tasks

rgrinberg force-pushed the dune-opam branch from 4af27be to 90179b0 Compare April 30, 2019 07:55

rgrinberg force-pushed the dune-opam branch 2 times, most recently from 1540306 to 1f01125 Compare May 1, 2019 10:08

jonludlam approved these changes May 1, 2019

View reviewed changes

rgrinberg force-pushed the dune-opam branch 2 times, most recently from e7adbba to 6c2d80f Compare May 1, 2019 12:11

rgrinberg force-pushed the dune-opam branch 2 times, most recently from edf4612 to d4bf448 Compare May 2, 2019 12:47

rgrinberg force-pushed the dune-opam branch from d4bf448 to 23ce5ae Compare May 2, 2019 12:58

rgrinberg force-pushed the dune-opam branch from 23ce5ae to 43332c7 Compare May 2, 2019 15:54

ghost suggested changes May 6, 2019

View reviewed changes

src/opam_file.ml Outdated Show resolved Hide resolved

rgrinberg force-pushed the dune-opam branch from 3a2e66e to 747dcba Compare May 6, 2019 10:58

rgrinberg force-pushed the dune-opam branch from 747dcba to 32efad5 Compare May 6, 2019 11:12

rgrinberg requested a review from a user May 6, 2019 12:18

rgrinberg force-pushed the dune-opam branch from 32efad5 to 5ea188b Compare May 6, 2019 12:20

avsm reviewed May 6, 2019

View reviewed changes

doc/opam.rst Outdated Show resolved Hide resolved

avsm reviewed May 6, 2019

View reviewed changes

doc/opam.rst Outdated Show resolved Hide resolved

avsm reviewed May 6, 2019

View reviewed changes

doc/opam.rst Outdated Show resolved Hide resolved

avsm approved these changes May 6, 2019

View reviewed changes

ghost approved these changes May 6, 2019

View reviewed changes

rgrinberg and others added 2 commits May 7, 2019 11:00

Simplify Package.hash

94c5cb5

Signed-off-by: Jeremie Dimino <[email protected]>

rgrinberg force-pushed the dune-opam branch from 493a4f4 to 94c5cb5 Compare May 7, 2019 03:00

rgrinberg merged commit de46f23 into ocaml:master May 7, 2019

rgrinberg deleted the dune-opam branch May 7, 2019 03:19

rgrinberg mentioned this pull request May 30, 2019

[new release] dune (1.10.0) ocaml/opam-repository#14215

Closed

rgrinberg mentioned this pull request Jun 4, 2019

[new release] dune (1.10.0) ocaml/opam-repository#14239

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More progress on the opam generation #2091

More progress on the opam generation #2091

rgrinberg commented Apr 28, 2019 •

edited

Loading

ghost commented Apr 29, 2019

Khady commented Apr 29, 2019

rgrinberg commented Apr 29, 2019

rgrinberg commented Apr 29, 2019

rgrinberg commented Apr 29, 2019

avsm commented Apr 29, 2019

rgrinberg commented Apr 30, 2019

rgrinberg commented Apr 30, 2019

jonludlam left a comment

rgrinberg commented May 1, 2019

rgrinberg commented May 1, 2019

rgrinberg commented May 2, 2019

ghost commented May 2, 2019

jonludlam commented May 2, 2019

ghost commented May 6, 2019

avsm commented May 6, 2019

ghost commented May 6, 2019

avsm commented May 6, 2019

rgrinberg commented May 6, 2019

ghost commented May 6, 2019

NathanReb commented May 6, 2019

ghost commented May 6, 2019

NathanReb commented May 6, 2019

ghost commented May 6, 2019

rgrinberg commented May 7, 2019

More progress on the opam generation #2091

More progress on the opam generation #2091

Conversation

rgrinberg commented Apr 28, 2019 • edited Loading

ghost commented Apr 29, 2019

Khady commented Apr 29, 2019

rgrinberg commented Apr 29, 2019

rgrinberg commented Apr 29, 2019

rgrinberg commented Apr 29, 2019

avsm commented Apr 29, 2019

rgrinberg commented Apr 30, 2019

rgrinberg commented Apr 30, 2019

jonludlam left a comment

Choose a reason for hiding this comment

rgrinberg commented May 1, 2019

rgrinberg commented May 1, 2019

rgrinberg commented May 2, 2019

ghost commented May 2, 2019

jonludlam commented May 2, 2019

ghost commented May 6, 2019

avsm commented May 6, 2019

ghost commented May 6, 2019

avsm commented May 6, 2019

rgrinberg commented May 6, 2019

ghost commented May 6, 2019

NathanReb commented May 6, 2019

ghost commented May 6, 2019

NathanReb commented May 6, 2019

ghost commented May 6, 2019

rgrinberg commented May 7, 2019

rgrinberg commented Apr 28, 2019 •

edited

Loading