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

[coq.dev] New upstream split package setup. #1923

Merged
merged 1 commit into from
Jun 29, 2022

Conversation

ejgallego
Copy link
Member

@ejgallego ejgallego commented Nov 7, 2021

This should work fine once coq/coq#15560 is merged.

From today, the canonical opam packages live in Coq's repository.

I'm not sure what the best way to keep them in sync is.

@palmskog
Copy link
Contributor

palmskog commented Nov 8, 2021

@ejgallego can you say something about how this draft PR is related to #1801? I would prefer if we only have one draft PR related to splitting of coq.dev.

@ejgallego
Copy link
Member Author

@ejgallego can you say something about how this draft PR is related to #1801?

It implements my suggestion there, tho I'm not sure we are properly testing coq-native in the CI here.

@palmskog
Copy link
Contributor

palmskog commented Nov 8, 2021

Indeed, coq-native testing is yet another PR, #1504, that seems stuck. Maybe you want to include the changes from that PR here?

@ejgallego
Copy link
Member Author

ejgallego commented Nov 8, 2021

Thanks for the pointer @palmskog , let's see if that works.

@ejgallego ejgallego force-pushed the coq+full_dune branch 2 times, most recently from 56f287d to 2e235a4 Compare November 9, 2021 09:44
@gares
Copy link
Member

gares commented Nov 9, 2021

The other PR was stuck because I could not find a failing package to test it...

@ejgallego
Copy link
Member Author

Indeed, seems the native + split + boot rules are broken, it is hard to test this stuff.

@gares
Copy link
Member

gares commented Nov 9, 2021

Here CI fails because of a timeout, you should remove the two commits called "hack" (or at least the unimath one).

@ejgallego
Copy link
Member Author

ejgallego commented Jan 14, 2022

Note that I'm still planning to fix this, it is a bug in how I compute the path for native in Dune, so it could go on 2.9.2 as it'll be a one line fix (hopefully)

@ejgallego ejgallego changed the title [coq.dev] Attempt to split the packages. [coq.dev] New upstream split package setup. Jun 24, 2022
@ejgallego
Copy link
Member Author

Split package will be upstream soon, this PR should be also good once that goes in.

@ejgallego ejgallego marked this pull request as ready for review June 24, 2022 17:10
@RalfJung
Copy link
Contributor

Now that the Coq PR coq/coq#15560 landed, this is rather urgent. Right now, the coq.dev opam package is broken.

@silene
Copy link
Contributor

silene commented Jun 29, 2022

Note that the pull request does not seem correct. For example, the depopts on coq-native is missing, as far as I can tell.

@ejgallego
Copy link
Member Author

Yes I need to fix this, sorry. Give me a minute.

@silene
Copy link
Contributor

silene commented Jun 29, 2022

bwrap: execvp ./configure: No such file or directory

@ejgallego
Copy link
Member Author

I have no idea what is going on.

@gares
Copy link
Member

gares commented Jun 29, 2022

In the logs (saved as artifacts) I don't find the line downloading the sources of coq-core, even if later is unpacks a tgz (which is maybe empty). The url stanza is gone, are we sure this change is OK?

Comment on lines 52 to 41
url {
src: "git+https://github.com/coq/coq.git#master"
}
dev-repo: "git+https://github.com/coq/coq.git"
Copy link
Member

Choose a reason for hiding this comment

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

This looks fishy, and the same holds for the other packages. IMO this opam file is fine for opam install https://github.com/coq/coq but is not good for an opam repository

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, sorry I am not familiar with how this works, I'll amend. Does this mean that we can't share the same opam files for stable opam releases, coq master and coq-core.dev ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that we can't share the same opam files for stable opam releases, coq master and coq-core.dev?

I do not understand. Why would you even want to share them? The build scripts, the required versions of dependencies, the conflicts, the patches, etc, everything at some point is bound to diverge.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why does it have to diverge, since 2.0, opam will update the build definitions from a pin.

For example, any divergence of coq.dev with what we have in master is just a bug. For stable branches, also the canonical files should be the ones in the repos.

What would be an example of such a diverenge? (Other than a patch not yet upstreamed)

Maybe the opam model is just a bit weird when compared to other systems.

Copy link
Contributor

@RalfJung RalfJung Jun 29, 2022

Choose a reason for hiding this comment

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

I agree, they should be in sync. Having the definition of the build process outside the coq git repo is quite annoying, but that's how opam works and at least it should be the same as what it is inside the repo.

Does this mean that we can't share the same opam files for stable opam releases, coq master and coq-core.dev ?

Does opam complain if you just have the url thing in master?
What I do for Iris is to auto-generate the opam repo file from the git file by appending a suitable url block.

Copy link
Contributor

Choose a reason for hiding this comment

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

For stable branches, also the canonical files should be the ones in the repos.

This does not make any sense at all! By definition of "stable", stable releases are not installed from a repository branch but from a tarball (or a tag, but that is frowned upon). There is absolutely no repository you can modify when you need to fix something in the Opam packaging of Coq (except the Opam repository itself, obviously). The only official Opam package for Coq is the one in the Opam repository. Anyone who argues the contrary does not understand how Opam and most package managers work.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the coq.dev package. There is nothing "stable" about it. It builds the latest git master commit. So it needs to be in sync with the build instructions for the latest git master commit.

Copy link
Contributor

@RalfJung RalfJung Jun 29, 2022

Choose a reason for hiding this comment

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

For stable branches, I think what they meant is that at the time the release is made, you want to have the build instructions in sync. Later you might have to fix them, and then they start to diverge, sure.

Or they meant the coq.8.16.dev etc packages, which also should stay in sync with the respective branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

@silene what you say is not consistent with current dune-release practices, which requires opam files in the repository, which are then updated for automatic PR submission to opam repository adding version and url fields.

This is also the workflow for stable branches. Moreover, this workflow is still not good enough so ideally the rewrite step of dune-release will be removed soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, more evidence for this point of view that as of today, opam repo maintainers will request to apply local fixes to the repos with a must.

This should work fine once coq/coq#15560 is merged.

From today, the canonical opam packages live in Coq's repository.

I'm not sure what the best way to keep them in sync is.
@ejgallego ejgallego merged commit f959493 into coq:master Jun 29, 2022
@ejgallego ejgallego deleted the coq+full_dune branch June 29, 2022 17:14
@erikmd
Copy link
Member

erikmd commented Jun 29, 2022

Thanks @ejgallego !

What would be an example of such a diverenge?

Actually I agree with @ejgallego here: that for the specific case of *.dev packages, the only sensible difference between opam-coq-archive's and upstream's (committed in master) opam files should be the line

url {
src: "git+https://github.com/_/_.git#master"
}

AFAICT, storing the opam files apart in opam-coq-archive/{core-dev,extra-dev} should mostly been seen as a convenience (without the need to know in advance the exact full name of the github repository), and the "name resolution algorithm", albeit simple and sensible, was not very well documented TTBOMK, so I had opened this issue FWIW.

Beyond this documentation issue:

in terms of automation, I guess it would be interesting to have a GitHub/GitLab pipeline somewhere, that would check that the difference between opam-coq-archive and upstream specs for selected packages, doesn't diverge… @gares @Zimmi48 @palmskog if you agree with this rough idea, where should we open one such issue? maybe in this repo opam-coq-archive?

@ejgallego
Copy link
Member Author

Actually the opam files here should be identical copies, that's the reason OPAM supports url files, submitting a PR to fix this now.

@mattam82
Copy link
Member

It seems the coqide-server package is missing here.

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.

9 participants