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

CI check suggestion: dune-project and opam files are in sync #220

Closed
hannesm opened this issue Jul 16, 2020 · 6 comments
Closed

CI check suggestion: dune-project and opam files are in sync #220

hannesm opened this issue Jul 16, 2020 · 6 comments
Labels
type/enhancement New feature or request

Comments

@hannesm
Copy link
Contributor

hannesm commented Jul 16, 2020

some projects generate their opam file from dune-project, but keep the opam files in the git repository for technical reasons (to keep opam pin --dev working).

the generated opam file contain a comment "please do not edit, generated by ..." - that is fine. but already more than once it happened to me that I modify (out of habit) the opam file, get the PR merged, and discover only at a later point that the dune-project and opam are now out of sync.

it would be great if the ocaml CI could check that after a dune build the git diff is empty (i.e. there are no modified files). thanks.

@avsm
Copy link
Member

avsm commented Jul 19, 2020

Yes, this sounds like a good thing to add. I think a general 'lint promote' step to check that no promotions occur would be useful. This has to also do a git diff, since opam file generation is a special case that is not a dune promote candidate.

avsm added a commit to avsm/ocaml-ci that referenced this issue Jul 26, 2020
This will catch the case of modifying a dune-project file but
forgetting to commit the corresponding opam file (or vice versa).

Fixes ocurrent#220
@avsm avsm added the type/enhancement New feature or request label Jul 26, 2020
@avsm
Copy link
Member

avsm commented Jul 26, 2020

We don't have a git checkout, so it's not quite as simple as git --exit-code to implement this.

One cheap and cheerful way would be to chmod the source directory to be read-only. It looks like BuildKit got support for this 24 days ago, so need to check if it's made its way into a docker release near us first... moby/buildkit#1492

@hannesm
Copy link
Contributor Author

hannesm commented Jul 27, 2020

We don't have a git checkout

I've no clue about the internals, but my intuition is that ocaml-ci/ocurrent uses a git clone of a remote repository for conducting its work. Now, if you don't have a git checkout, I fail to understand how ocaml-ci is supposed to work. You could copy the directory before build and do a recursive diff after build.

@avsm
Copy link
Member

avsm commented Jul 28, 2020

ocaml-ci builds a Dockerfile that's incremental, so it copies in the source tree as it builds up layers. For example it starts with the opam files, analyses metadata, installs depexts, then adds the sources, then does a dune build. This way if you just modify OCaml code, the depext phase and earlier can be cached since they will not have changed.

It therefore never has a full git checkout in the Dockerfile. I ran a quick test and chmod -R 400 of the source tree works fine. You get a permission denied error when dune tries to modify the source tree -- the only time it does this is to write a .merlin file or to update opam files, so it should be safe for the CI to do and won't disturb caching.

@talex5
Copy link
Contributor

talex5 commented Dec 15, 2020

This will be fixed as a side-effect of #285.

@craigfe
Copy link
Member

craigfe commented Feb 22, 2021

Looks like this one can be closed thanks to opam-dune-lint :-)

@talex5 talex5 closed this as completed Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants