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

Support dune 3.0 #45

Closed
smorimoto opened this issue Mar 15, 2022 · 22 comments
Closed

Support dune 3.0 #45

smorimoto opened this issue Mar 15, 2022 · 22 comments
Labels
enhancement New feature or request

Comments

@smorimoto
Copy link

No description provided.

@booklearner
Copy link

I can start an attempt to do this if it's not already in progress. Feel free to reach out with any tips!

@talex5
Copy link
Contributor

talex5 commented Aug 12, 2022

That would be great! The changes are mostly needed in dune, however.

Dune removed external-lib-deps because it was hard to implement, but this was because it tried to work out the dependencies without building the project first. For opam-dune-lint, that's not a problem.

The dune developers said:

First step for Dune would be to restore external-lib-deps, but as an output of the build itself (so it would become more precise, but at the cost of doing a build, which is fine).

@moyodiallo
Copy link
Collaborator

external-lib-deps is back for the next release of dune, as dune describe external-lib-deps(ocaml/dune#6045). But for the support 3.0, the lib dune-private-libs.dune-lang is removed since dune 3.0 and dune-lang is now private in dune.

@talex5
Copy link
Contributor

talex5 commented Oct 27, 2022

external-lib-deps is back

Great - thanks!

dune-private-libs.dune-lang is removed

Ah. It looks like we just it for modifying the dune-project file with the updated packages. But they're just S-expressions, so we could do it with some other S-expression library. I think they only reason we care about it is to get the right formatting for the output, but perhaps there's a way to make dune fmt sort it out afterwards?

@moyodiallo
Copy link
Collaborator

For formatting dune files, there's a command dune format-dune-file <file>. It works for dune-project file too. But I don't know if it format the way it's needed in opam-dune-lint.

@tmcgilchrist
Copy link
Member

@talex5 is right we can just use a simple s-expression library, maybe https://github.com/ocaml-dune/csexp since it has minimal dependencies. Then formatting via dune format-dune-file <file> 👍🏻

Ideally this should all get moved into dune/opam proper as an extra lint command.

@talex5
Copy link
Contributor

talex5 commented Nov 2, 2022

csexp is a binary(ish) format. https://github.com/janestreet/parsexp is probably what you want.

@moyodiallo
Copy link
Collaborator

Yes, I confirm we need that lib.

@moyodiallo
Copy link
Collaborator

The command dune describe external-lib-deps has not alias like @install or @runtest. It prints out all the external dependencies by dir only. Is it a big deal for linting "with-test"? Because with this command we won't be able to do that.

opam-dune-lint/main.ml

Lines 82 to 83 in fa22308

let build = get_libraries ~pkg ~target:"@install" |> to_opam_set ~project ~index in
let test = get_libraries ~pkg ~target:"@runtest" |> to_opam_set ~project ~index in

@moyodiallo
Copy link
Collaborator

moyodiallo commented Jan 13, 2023

Mentioning this PR #46, it's not far from solving the issue.

@Lupus
Copy link

Lupus commented May 12, 2023

@moyodiallo it seems that there are only couple stylistic remarks in dune related PRs and those await your input. It's painful to match dependencies between opam and dune by hand, would be really awesome if this could go live!

@moyodiallo
Copy link
Collaborator

I admit this is a hack, dune doesn't release the necessary libraries. what we have is, printing out from dune (the PRs) the needed information. This is the same approximation as the current version.

@Lupus
Copy link

Lupus commented May 12, 2023

By current version you mean the master branch? I've just tried installing opam-dune-lint via opam and it suggested to downgrade dune.

@moyodiallo
Copy link
Collaborator

yes the master branch or the lastest release, the downgrade is normal.

@Lupus
Copy link

Lupus commented May 12, 2023

We're using dune 3.x, downgrading is not an option, but If I understand this correctly, the PRs in dune repo can unblock use of this tool with dune 3.x.

@moyodiallo
Copy link
Collaborator

Yes, that is the purpose. we're going to use this branch https://github.com/moyodiallo/dune.git#opam-dune-lint-testing, to test it on our CI ocurrent/ocaml-ci#811.

@smorimoto
Copy link
Author

Do you have any plans to upstream changes to this repository?

@moyodiallo
Copy link
Collaborator

Do you have any plans to upstream changes to this repository?

Yes, #46 (comment)

@tmcgilchrist
Copy link
Member

This also needs a new release to opam-repository @moyodiallo when dune support is in place.

@moyodiallo
Copy link
Collaborator

moyodiallo commented Oct 18, 2023

There is a new release now v0.3. This can be closed, thanks everyone for the effort.

@smorimoto
Copy link
Author

Fantastic!

@tmcgilchrist
Copy link
Member

As always @smorimoto please report any issue found to this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants