Dunify release by Et7f3 · Pull Request #1361 · ocaml-ppx/ocamlformat

Et7f3 · 2020-05-08T20:51:40Z

In reference of #900 (comment) (opened by @gpetiot) Here metadata are stored with dune who is responsible to build opam metadata (less metadata to update when releasing)

Some notes:

header is removed because now it is generated by dune. I can use ocamlformat.template to inject header but it will be at the end.
opam file differ

-["ocaml" "tools/gen_version.mlt" "lib/Version.ml" version] {pinned}

%%VERSION%% = git describe --always --dirty
the script = git describe --tags --dirty --always
I don't know how should I handle this. I don't get how we can print 0.14.2 for instance and not the commit I have tried with manual git tag my repo but no luck 🤷
maybe #1133 handle correctly ?

The dune-project is formatted with dune (it is sub-optimal because of dune format-dune-file on dune-project is suboptimal ocaml/dune#2291)

I open in draft because I might have broken version field.

jberdine · 2020-05-08T21:04:48Z

It should not be necessary to have a header in a file generated from another file that does have a header. I make no guarantees that the bot is capable of handling that though. But a "header" at the end would not be seen either. Let's not worry about the header until the bot yells at me. It is complaining already and will until #1349 is merged anyhow.

But I expect that this will have to wait for at least #1133. I also could not figure out how to specify the build field of the dune-generated opam file, and getting the correct version when pinning is necessary.

Et7f3 · 2020-05-08T21:14:55Z

Thanks for your quick reply. I have tested to manually pin master and my branch with esy and I get unknown as version. I will test with opam to see if we got the same version or not.

But I expect that this will have to wait for at least #1133. I also could not figure out how to specify the build field of the dune-generated opam file, and getting the correct version when pinning is necessary.

I can force dune to add a specific build command (with the template) but it will not update as dune upgrade and change his cli.

EDIT: With opam: my fork: 0.14.1-5-gd3b5a80 master: 0.14.1

jberdine

This seems like it could be basically okay after a bit more iteration. But I do not understand the motivation for this to be honest. I'm not objecting, but it just seems that we'd copy the contents of the opam file into dune-project and change the syntax. I'd be happy to learn more of the motivation.

ocamlformat.opam

jberdine · 2020-06-23T01:08:01Z

ocamlformat.opam

+    "@install"
+    "@runtest" {with-test}


Is it obvious that these have the same behavior as the version before this PR?

dune build @runtest === dune runtest but with one command that allow us to build also the project
https://dune.readthedocs.io/en/stable/tests.html?highlight=%40install#running-tests and @install just generate file required for the installation

-p pkg is a shorthand for --root . --only-packages pkg --profile release --default-target @install

https://dune.readthedocs.io/en/stable/opam.html#invocation-from-opam I am a liitle bit confused because we have then @install present two times but it is taken from dune opam template. https://github.com/ocaml/dune/blob/ecbee3d4bf8d82b47667fd6a1f74f01deb5e9eb2/src/dune/opam_create.ml#L24

Ok, I hadn't appreciated that {with-test} could be placed on individual arguments instead of whole commands.

Ok, so @install only builds the files to be installed, but does not actually copy them into some other directories, etc. Correct?

I will ask on discord for confirmation.

Yes they are doing almost the same thing. As written the command build all installable libraries and executables (Even if not needed for the tests) and runs the tests. You can add different filters on each string in the list In the opam files, it’s quite convenient at times. @Et7f3 reply above is spot on

Et7f3 · 2020-06-25T07:54:17Z

This seems like it could be basically okay after a bit more iteration. But I do not understand the motivation for this to be honest. I'm not objecting, but it just seems that we'd copy the contents of the opam file into dune-project and change the syntax. I'd be happy to learn more of the motivation.

When dune generate file: dune automatically insert the good dune lower bound so one metadata whe can ignore. Also dune doesn't have such feature yet (or i'm not aware) but it could do more sanity check for instance: only make available dependencies we have listed in our package

jberdine

Ok, thanks for the discussion, this looks good.

One final question: the dune-project file has the look of being auto-formatted. Is it, and how do you do that? AFAIU e.g. dune build @_build/default/fmt formats dune but not dune-project files. Just me?

gpetiot · 2020-06-30T16:43:21Z

I rebased and updated the constraint on base according to #1396. I don't think there is a way to format the dune-project file though but it's not an issue.

Et7f3 · 2020-07-01T12:50:58Z

dune format-dune-file FILE This format any dune file.

gpetiot · 2020-07-01T13:42:07Z

dune format-dune-file FILE This format any dune file.

Yes but it's not integrated the same way dune files are when running dune @fmt with the promote feature. We would need to make an inplace transformation in the Makefile. I wouldn't mind waiting until they are automatically formatted by dune.

jberdine · 2020-07-01T13:52:28Z

FWIW it could be tmp=$(mktemp -t 'dune-format'); dune format-dune-file dune-project > tmp; mv -f tmp dune-project

emillon · 2020-07-02T08:47:40Z

@jberdine a motivation for this for example is that when you change (lang dune ...) and use opam file generation, the bound in the opam file will be updated automatically.

emillon · 2020-07-02T08:49:57Z

ocamlformat.opam.template

@@ -0,0 +1,13 @@
+build: [


If we merge #1133 before this, we can remove this file before merging

gpetiot · 2020-09-09T14:35:55Z

Rebased onto master, should be compatible with the dune-build-info changes.

facebook-github-bot added the CLA Signed label May 8, 2020

gpetiot requested a review from emillon May 11, 2020 11:56

jberdine requested changes Jun 9, 2020

View reviewed changes

ocamlformat.opam Outdated Show resolved Hide resolved

ocamlformat.opam Show resolved Hide resolved

Et7f3 force-pushed the dunify-release branch from d3b5a80 to a26e8f9 Compare June 14, 2020 15:27

Et7f3 marked this pull request as ready for review June 14, 2020 15:41

gpetiot added the no changelog set this to bypass the CI check for changelog entries label Jun 22, 2020

jberdine reviewed Jun 23, 2020

View reviewed changes

jberdine approved these changes Jun 25, 2020

View reviewed changes

gpetiot force-pushed the dunify-release branch from a26e8f9 to dd340ba Compare June 30, 2020 16:39

gpetiot approved these changes Jun 30, 2020

View reviewed changes

gpetiot force-pushed the dunify-release branch from a9ca8a1 to eb9f5f0 Compare July 1, 2020 12:56

emillon reviewed Jul 2, 2020

View reviewed changes

emillon mentioned this pull request Jul 6, 2020

Use dune-build-info to compute version number #1133

Merged

jberdine added this to the 0.16.0 milestone Jul 11, 2020

emillon mentioned this pull request Aug 24, 2020

Specify package version in opam file #1445

Closed

Et7f3 and others added 7 commits September 9, 2020 21:21

chore: Fill metadata in dune-project.

b081317

chore: dune build ocamlformat.opam

09bd58f

update base and stdio version constraint

d465236

update ocaml and omp version constraint

742b447

autoformat dune-project

869cf90

remove template file

660fead

add missing dune-build-info dep and omp upper bound

c344d0c

gpetiot force-pushed the dunify-release branch from bfca6ac to c344d0c Compare September 9, 2020 14:35

gpetiot requested review from Julow and emillon September 9, 2020 14:36

gpetiot merged commit c59717d into ocaml-ppx:master Sep 11, 2020

Conversation

Et7f3 commented May 8, 2020

Uh oh!

jberdine commented May 8, 2020

Uh oh!

Et7f3 commented May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jberdine left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jberdine Jun 23, 2020

Choose a reason for hiding this comment

Uh oh!

Et7f3 Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

jberdine Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

Et7f3 Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

mseri Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Et7f3 commented Jun 25, 2020

Uh oh!

jberdine left a comment

Choose a reason for hiding this comment

Uh oh!

gpetiot commented Jun 30, 2020

Uh oh!

Et7f3 commented Jul 1, 2020

Uh oh!

gpetiot commented Jul 1, 2020

Uh oh!

jberdine commented Jul 1, 2020

Uh oh!

emillon commented Jul 2, 2020

Uh oh!

emillon Jul 2, 2020

Choose a reason for hiding this comment

Uh oh!

gpetiot commented Sep 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Et7f3 commented May 8, 2020 •

edited

Loading

mseri Jun 25, 2020 •

edited

Loading