-
Notifications
You must be signed in to change notification settings - Fork 166
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
Conversation
@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 |
It implements my suggestion there, tho I'm not sure we are properly testing coq-native in the CI here. |
Indeed, coq-native testing is yet another PR, #1504, that seems stuck. Maybe you want to include the changes from that PR here? |
5f53b0e
to
1d3b23c
Compare
Thanks for the pointer @palmskog , let's see if that works. |
56f287d
to
2e235a4
Compare
The other PR was stuck because I could not find a failing package to test it... |
Indeed, seems the native + split + boot rules are broken, it is hard to test this stuff. |
Here CI fails because of a timeout, you should remove the two commits called "hack" (or at least the unimath one). |
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) |
Split package will be upstream soon, this PR should be also good once that goes in. |
Now that the Coq PR coq/coq#15560 landed, this is rather urgent. Right now, the |
Note that the pull request does not seem correct. For example, the |
Yes I need to fix this, sorry. Give me a minute. |
|
I have no idea what is going on. |
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? |
core-dev/packages/coq/coq.dev/opam
Outdated
url { | ||
src: "git+https://github.com/coq/coq.git#master" | ||
} | ||
dev-repo: "git+https://github.com/coq/coq.git" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Thanks @ejgallego !
Actually I agree with @ejgallego here: that for the specific case of
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? |
Actually the opam files here should be identical copies, that's the reason OPAM supports |
It seems the coqide-server package is missing here. |
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.