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

Add dev packages for Coq Platform packages which don't have one as yet #2080

Merged
merged 3 commits into from
Jan 26, 2022

Conversation

MSoegtropIMC
Copy link
Contributor

This PR adds .dev packages for those packages in Coq Platform which don't have one as yet.

The packages are all derived from and/or checked against the latest released packages and adjusted.

@palmskog
Copy link
Contributor

@MSoegtropIMC what's the reasoning behind having coq-dpdgraph depend on conf-automake rather than conf-autoconf? This is not what we are testing in the coq-dpdgraph CI.

@MSoegtropIMC
Copy link
Contributor Author

The released packages have automake/autoconf already run. If you build from git you need to run automake.

@MSoegtropIMC
Copy link
Contributor Author

That is the release opam packages don't have and don't need the ["autoreconf" "-i" "-s"] step.

@MSoegtropIMC
Copy link
Contributor Author

But possibly I messed it up - what is the right conf package for autoreconf? As far as I understand this, autoreconf first calls automake and then autoconf and the automake package included the autoconf package.

@palmskog
Copy link
Contributor

I don't personally understand the difference between autoreconf and autoconf. I just followed the lead from packages such as Flocq and Interval, and used the same idiom for build dependencies (i.e., conf-autoconf).

@silene
Copy link
Contributor

silene commented Jan 25, 2022

As far as I understand this, autoreconf first calls automake and then autoconf and the automake package included the autoconf package.

autoreconf calls automake only if there is a file Makefile.am. If there is only a file Makefile.in, it should directly call autoconf. But take that with a grain of salt, since I am not completely sure what would happen if you have a file configure.ac (instead of configure.in) but no file Makefile.am.

@palmskog
Copy link
Contributor

I don't personally understand the difference between autoreconf and autoconf. I just followed the lead from packages such as Flocq and Interval, and used the same idiom for build dependencies (i.e., conf-autoconf).

@MSoegtropIMC
Copy link
Contributor Author

@silene : thanks for sharing these details! I guess it is safe to switch to autoconf then.

@palmskog : I had a lot of trouble with packages committing to git automake output but not updating this always, so that autoconf usually works, but not always. I spent sufficient time debugging this (debugging m4 is a lot fo fun) that at some point I decided that it simplifies my life to always use autoreconf -i -s which never failed me to get things (a makefile) done.

@MSoegtropIMC
Copy link
Contributor Author

Btw.: what does the dev criterion mean in ["autoconf"] {dev}? That source is from git?

@silene
Copy link
Contributor

silene commented Jan 25, 2022

what does the dev criterion mean in ["autoconf"] {dev}? That source is from git?

As a rule of thumb, yes. I think there are actually three different cases:

  1. The package is pinned to a local repository. (opam pin /path/to/repo)
  2. The url field was ignored and the dev-repo field was used instead. (opam pin coq-foo --dev-repo)
  3. The url field was used but it does not point to a tarball. (opam install coq-foo.dev)

@palmskog
Copy link
Contributor

I see the following for gappa.dev:

# bwrap: execvp bison: No such file or directory

Is there a missing dependency on Bison?

@MSoegtropIMC
Copy link
Contributor Author

Is there a missing dependency on Bison?

It does have "conf-bison" {build} already.

Co-authored-by: Karl Palmskog <palmskog@gmail.com>
@palmskog
Copy link
Contributor

OK, it looks like you will have to add the system packages for bison and flex to the CI configuration for the gappa.dev package to build.

@MSoegtropIMC
Copy link
Contributor Author

OK, it looks like you will have to add the system packages for bison and flex to the CI configuration for the gappa.dev package to build.

Why is that? CI doesn't have depext support?

@palmskog
Copy link
Contributor

I generally try to stay out of the CI setup, but no, it doesn't have depext support. You'll have to add the packages here in the .gitlab-ci.yml file: https://github.com/coq/opam-coq-archive/blob/76553b80984336c7ec21efd9ae49f65338cc0640/.gitlab-ci.yml#L22

@MSoegtropIMC
Copy link
Contributor Author

Are there known difficulties with depext? With opam 2.1 it should be quite easy to do (at least if you have permissions to install system packages without password).

Co-authored-by: Karl Palmskog <palmskog@gmail.com>
@gares
Copy link
Member

gares commented Jan 26, 2022

There is no depext support because we never added it. I think it is the right thing to do, your are welcome to hack the scripts:
https://github.com/coq/opam-coq-archive/blob/1f0f0ec84c738d88a8ee550494917e9387975ebb/scripts/opam-coq-install-remove#L60-L77

The quick fix is to install bison and the other deps upfront.

@MSoegtropIMC
Copy link
Contributor Author

@gares:

There is no depext support because we never added it.

As far as I know in opam 2.1 it is built in. So is CI on opam 2.0 or did you some tricks to disable it?

@gares
Copy link
Member

gares commented Jan 26, 2022

Not really a trick: https://github.com/coq/opam-coq-archive/blob/1f0f0ec84c738d88a8ee550494917e9387975ebb/.gitlab-ci.yml#L16

Maybe it's time to switch to 2.1 (it is not beta anymore, right?)

@MSoegtropIMC
Copy link
Contributor Author

Maybe it's time to switch to 2.1 (it is not beta anymore, right?)

No, it is released. It 2.1.2 now (https://opam.ocaml.org/blog/opam-2-1-2/).

@MSoegtropIMC
Copy link
Contributor Author

@palmskog : can we merge this now? I don't think we need to wait for CI for the commit of your changes - except for the gappa depext issues it did run through before and you anyway suggested to do this PR without CI and that "extra-dev is Wild-West".

Or do you want me to fix the bison/flex dependencies? I would rather enable depext but do this after the Coq Platform release.

I can also keep the dev packages in the Coq Platform local opam repo.

@MSoegtropIMC
Copy link
Contributor Author

Btw.: we can also remove the gappa.dev package from here. Please note that the release package lives in the main opam repo, but since they don't have the concept of a dev repo I put the dev package here. But I can also keep it local to Coq platform (or remove it entirely and always use gappa releases).

@palmskog
Copy link
Contributor

Fine by me to have the gappa.dev package here. @gares should we merge this now or fix the opam version and ci?

@MSoegtropIMC
Copy link
Contributor Author

@palmskog : I need this to be merged before 1 PM German time or I will keep the packages in the Coq Platform patch repo in the release. I don't think it is worth it to delay the Coq Platform release just for the opam cleanup.

@palmskog palmskog merged commit 5a72913 into coq:master Jan 26, 2022
@MSoegtropIMC
Copy link
Contributor Author

Thanks! I do the final Coq Platform PR right away.

@@ -0,0 +1,43 @@
opam-version: "2.0"
maintainer: "Ralf Jung <jung@mpi-sws.org>"
Copy link
Contributor

@RalfJung RalfJung Aug 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate if this package would be removed. We deliberately did not put a coq-stdpp.dev (or coq-iris.dev) package into this repository. The 'dev' version is meant for local pinning only. This is not just cosmetic; I noticed these packages because they lead to strange behavior of our CI.

I am surprised that the people maintaining this software (and supposedly even maintaining this package!) did not get pinged...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is not the first time this happened, there were "rogue" coq-iris.dev packages already in 2017 and they have been removed again (#191 -- though those versions really made no sense, this time at least the dev packages are actually properly tracking git).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2256 proposes to remove these packages again.

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

Successfully merging this pull request may close these issues.

5 participants