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

Adapt to coq/coq#19927 (coq-core renamed to rocq-runtime) #883

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

SkySkimmer
Copy link
Collaborator

There are many more occurrences of coq-core in this repo but I don't know what they're doing. This patch is just enough to fix Coq's CI.

@SkySkimmer
Copy link
Collaborator Author

Doesn't quite work, I get

-   - 2 Coq path directory bindings in scope; 27 Coq plugin directory bindings in scope
+   - 2 Coq path directory bindings in scope; 0 Coq plugin directory bindings in scope

eg https://gitlab.inria.fr/coq/coq/-/jobs/5062976

@ejgallego
Copy link
Owner

I'll have a look tomorrow and try to fix it.

@ejgallego
Copy link
Owner

@SkySkimmer you missed the occurence in coq/args.ml which the important one.

Anyways this code can be removed as we don't use the findlib method anymore, I actually have a coq-lsp branch that does that but it will take a while for me to return back to work unfortunately.

@ejgallego
Copy link
Owner

@SkySkimmer you mind bumping the vendor/coq submodule to I can use coq-lsp CI to check things are right?

(And adapt to other changes like the worker build, etc...)

Also serlib needs (an easy) fix for its dune files, that basically covers it all.

Note that I plan to make the loader not so hacky, also got a branch for that somewhere.

@SkySkimmer
Copy link
Collaborator Author

@SkySkimmer you mind bumping the vendor/coq submodule to I can use coq-lsp CI to check things are right?

IDK how to do that

@SkySkimmer SkySkimmer mentioned this pull request Dec 12, 2024
@SkySkimmer SkySkimmer changed the title Adapt to coq/coq#19764 (coq-core renamed to rocq-runtime) Adapt to coq/coq#19927 (coq-core renamed to rocq-runtime) Dec 12, 2024
@ejgallego
Copy link
Owner

IDK how to do that

pushd vendor/coq
git checkout $the_branch_you_want
popd
git add vendor/coq && git commit ...

@SkySkimmer
Copy link
Collaborator Author

But the vendored git repo only knows about coq/coq, my branch is in skyskimmer/coq

@ejgallego
Copy link
Owner

Good questions, I'm not sure I make it work, as I use hub and gh command line tools. Maybe git submodule set-url is the new recommended way?

Never mind if that's too hard.

@SkySkimmer
Copy link
Collaborator Author

I edited .gitmodules to point at my fork

@ejgallego
Copy link
Owner

Updated rest of the system, including the missing change to coq/args that made Coq CI fail.

Let's see how it goes.

@ejgallego
Copy link
Owner

ejgallego commented Dec 13, 2024

  • 2 Coq path directory bindings in scope; 0 Coq plugin directory bindings in scope

As I've mentioned, this will change to "X findlib paths in scope" for Rocq 9.0

Copy link
Owner

@ejgallego ejgallego left a comment

Choose a reason for hiding this comment

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

CI green, local and JS tests OK too.

@SkySkimmer SkySkimmer merged commit c5caa1d into ejgallego:main Dec 18, 2024
13 of 14 checks passed
@SkySkimmer SkySkimmer deleted the rocq-cli branch December 18, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants