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

Configuration file format specification #417

Open
Leonidas-from-XIV opened this issue Nov 24, 2021 · 3 comments
Open

Configuration file format specification #417

Leonidas-from-XIV opened this issue Nov 24, 2021 · 3 comments
Milestone

Comments

@Leonidas-from-XIV
Copy link
Member

Leonidas-from-XIV commented Nov 24, 2021

Current state

At the moment dune-release uses an ad-hoc config file which doesn't really have any specification, it is just what was simple and easy to do without introducing heavy dependencies. Similarly, dune-release handles dune-project files, which are S-expressions in a somewhat ad-hoc way (see #366).

Previous efforts

There have been efforts to move the dune-release config to a YAML format. That had the advantage of being a standard format thus allowing YAML tools to process the files. Unfortunately, the YAML format is quite complex and the only YAML-library in OCaml while very good depends in a C library which makes this a rather heavyweight dependency.

Similarly, parsing dune-project files ad-hoc is somewhat inelegant and fragile, especially if we need to read more out of it in the future, that code is going to get less and less readable.

Proposals

Switch configuration to S-expressions

This has the advantage that in the OCaml world S-expressions are somewhat of a popular choice for configuration (and unlike JSON they allow comments). That would take care of parsing dune-project files as well as reading and writing our configuration file.

The downside is that there is currently no canonically accepted S-expression parser in the OCaml community:

  1. sexplib depends on parsexp which in turn depends on base is somewhat heavyweight. @emillon looked into this and parsexp could be easily patched to avoid using base.
  2. https://github.com/gpetiot/sexp-parse exists and could be used but its future is uncertain given parseexp could be adapted.

Another point to keep in mind is that changing the format would require writing a converter from the current format to the new one.

Stop parsing dune-project

This proposal suggests that we stop reading the project name from dune-project and instead infer the name out of the name of the git repo. This should be backwards compatible since the names of the tarballs (which is that the name is used for) are overall not particularly important so if the tarball name changes in future releases this is not even a breaking change.

The config file format in this case would stay the same while at the same time reducing the number of ad-hoc parsers in our codebase.

@Leonidas-from-XIV Leonidas-from-XIV added this to the 2.0.0 milestone Nov 24, 2021
@NathanReb
Copy link
Contributor

I think it's worth noting that even if we stop parsing dune-project for the tarball name it might still be good to use sexp for our config. Since we interact with dune it is quite likely that we'll need to parse some dune files at some point and that would still allow us to remove the existing parsing code, which is not the best, in the long run.

@emillon
Copy link
Collaborator

emillon commented Nov 26, 2021

The downside is that there is no canonical S-expression parser:

To remove any ambiguity, this means "no s-expression parser that is canonical", not "parser of canonical s-expressions".

@Leonidas-from-XIV
Copy link
Member Author

Oh wow, I didn't know there was such a thing! 😮 I updated the text above.

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

No branches or pull requests

3 participants