Dune upgrade V1 -> V2

[voodoos]

This PR aims to track the ongoing development of a new procedure that would help users to upgrade their projects to dune_lang >=2.0. The hope is that a vast majority of projects can be migrated in a mostly automated way. After the upgrade users will be prompted with a list of hard-to-migrate things that they should check manually.

Progression

Doc updates

• self_build_stubs_archive does not exists since 2.0
• c_names / c_flags / etc do not exist since 2.0

Breaking changes that should be automatically handled

• Get rid of ad-hoc rules for guessing the version. Dune now only
  relies on the version written in the dune-project file and no
  longer read VERSION or similar files.
• The action field in the alias stanza is not available starting lang dune 2.0. The alias field in the rule stanza is a replacement.
• Set up formatting rules by default. They can be configured through a new
  (formatting) stanza in dune-project.
  💡 The upgrader adds (formatting disabled) when the project was not formatted. It adds (formatted (enabled_for ...)) when needed.
• *Change the default modes field of executables to (mode exe). If
  one wants to build a bytecode program, it now needs to be explicitly
  requested via (modes byte exe). *
  💡 The upgrader adds (modes byte exe) to executables without explicit mode.
• Error out when a preprocessor_deps field is present but not
  preprocess field is. It is a warning with Dune 1.x projects.
  💡 The upgrader removes preprocessor_deps field when there is no preprocess field or if the chosen preprocess is no_preprocessing
• no_keep_locs is a no-op for projects that use lang dune older than 2.0. In
  projects where the language is at least 2.0, the field is now forbidden.
• c_flags, c_names and cxx_names are now supported in executable and executables stanzas.
  Note: this feature has been subsequently extended into a separate foreign_stubs field.
  💡 In libraries and executables stanzas, the upgrader replaces the fields c(xx)_names and c(xx)_flags by an appropriate foreign_stubs field.

Breaking changes that the user should be aware of

• Drop support for jbuild and jbuild-ignore files.
• *mli only modules must now be explicitly declared. This was previously a
  warning and is now an error. *
• self_build_stubs_archive was deleted in version 2.0 of the dune language
• Stop installing the ocaml-syntax-shims binary. In order to use
  future_syntax, one now need to depend on the ocaml-syntax-shims
  package.
• Actions which introduce targets where new targets are forbidden (e.g.
  preprocessing) are now an error instead of a warning.
• Stop installing the ocaml-syntax-shims binary. In order to use
  future_syntax, one now need to depend on the ocaml-syntax-shims
  package.
• Do not put the <package>.install files in the source tree unless -p or
  --promote-install-files is passed on the command line
• Library names are now validated in a strict fashion. Previously, invalid names
  would be allowed for unwrapped libraries
• Stricter validation of file names in select. The file names of conditional
  sources must match the prefix and the extension of the resultant filename.
• Modules filtered out from the module list via the Ordered Set Language must
  now be actual modules.
• Stub names are no longer allowed relative paths. This was previously a warning
  and is now an error.
• In (diff? x y) action, require x to exist and register a
  dependency on that file.

Non-breaking changes

• The rest of the changelog 2.0.0 section.

[ghost]

Drop support for jbuild and jbuild-ignore files

I don't think we need to do anything about that. dune >= 2.0.0 binaries cannot read jbuild files at all, whether we are in a dune 1 or dune 2 project.

[voodoos]

I have updated the list of breaking changes that the upgrader should take into account (first post).
@diml, @nojb, @rgrinberg , @emillon do you see anything missing ?

[ghost]

That looks all good to me! I can't think of anything else. Thanks!

[voodoos]

After performing some testing I encountered this change:

Error: 'c_names' was deleted in version 2.0 of the dune language.
Use the (foreign_stubs ...) field instead.

Which was not very clear in the changelog:

c_flags, c_names and cxx_names are now supported in executable and executables stanzas.
(#2562, @nojb) Note: this feature has been subsequently extended into a separate
foreign_stubs field. (#2659, RFC #2650, @snowleopard)

I will add that to the upgrader's work, and correct the doc/changelog.

[emillon]

That's great! With (foreign_stubs) that'll cover most of the uses.

[voodoos]

Ok, I think this PR is ready for review.

All the logic is in upgrader.ml. I initially splitted the upgrader in three files, "common", "v1" and "v2", but decided to merge them back at the end.

• module Common contains code which is common to both upgraders or that should be useful for a future v2 -> v3 upgrader.
• module V1 contains the upgrader from jbuild to dune v1.
• module V2 contains the novel upgrader from dune v1 to v2.
• The toplevel upgrade function makes the list of folders to upgrade and to which version, then calls the corresponding upgraders. The project version "detection" is happening in the detect_project_version function and may still be a bit fragile.

To update an old jbuild project to v2, the upgrader should be run twice.

[rgrinberg]

To update an old jbuild project to v2, the upgrader should be run twice.

Is it difficult to make the tool sequence the upgrades automatically? it's not so important and I think that it probably makes sense for users to inspect each upgrade individually, but for some duniverse like use cases where you have a massive workspace of 3rd party packages, it would be annoying to run $ dune upgrade a few times.

[emillon]

Yes I agree about this point, in particular that ensures that dune upgrade is idempotent (running it twice is equivalent to running it once).

[voodoos]

Is it difficult to make the tool sequence the upgrades automatically?

Ok so I tried to make the updater run twice in two ways:

• Juste make it call itself recursively at the end of the first ugprade if v0-v1 upgrades where detected.
• Schedule two upgrades at the bin/upgrade.ml level

In both cases it looks like the File_tree is not refreshed: the second upgrades tries to perform v0->v1 upgrades again but fails because the jbuild files have already been translated and deleted.

Any idea of how I could manage it ?

[rgrinberg]

Indeed the File_tree is not refreshed. However, this isn’t something that is too tough to fix. If you look at File_tree.Settings.get, you will see that it depends on the current run. I suppose that we’d like for this to depend on a “ref” like value that we can control to reset the file tree after the upgrade finishes. The only trick here is that we need this ref to respect memoization. I’ve copied Arseniy. He can advise us if this is the best way to solve this problem.

…

On Mar 5, 2020, 11:23 AM +0000, Ulysse ***@***.***>, wrote: > Is it difficult to make the tool sequence the upgrades automatically? Ok so I tried to make the updater run twice in two ways: • Juste make it call itself recursively at the end of the first ugprade if v0-v1 upgrades where detected. • Schedule two upgrades at the bin/upgrade.ml level In both cases it looks like the File_tree is not refreshed: the second upgrades tries to perform v0->v1 upgrades again but fails because the jbuild files have already been translated and deleted. Any idea of how I could manage it ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[voodoos]

Thanks ! Memo has a reset function:
If I call this function before the new upgrading attempt it does work !

Is that a bad way to do it ?

[rgrinberg]

It’s not ideal but it should work as well in this case. The only negatives is that it could be confusing to future readers, and that it could possibly be slow things down. I think it’s fine to use here if you leave a comment why it’s necessary.

…

On Mar 5, 2020, 12:39 PM +0000, Ulysse ***@***.***>, wrote: Thanks, Memo has a reset function. If I call this function before the new upgrading attempt it does work ! Is that a bad way to do it ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[aalekseyev]

I think calling reset is reasonable in this case. The incremental computation is not fully implemented yet and untested so we shouldn't try to make it work in this PR.

[emillon]

Hello,

Just checking in: what's the status of this? If it's doing the upgrades in an idempotent way I think it's good to go after a final round of review.

[voodoos]

I added a test illustrating the idempotency of the upgrade procedure.
I also rebased on master. It should be ready for review.