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 dependency to coq-elpi in coq-corn.dev #3247

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

MSoegtropIMC
Copy link
Contributor

No description provided.

@MSoegtropIMC
Copy link
Contributor Author

Since this is "dev" which has been called "pretty much wild west" before I will self merge as soon as CI passes unless someone protests until then.

@SkySkimmer
Copy link
Contributor

Looks like elpi is failing

- File "theories/dune", line 5, characters 11-20:
- 5 |  (theories elpi_elpi))
-                ^^^^^^^^^
- Theory "elpi_elpi" has not been found.
- -> required by theory elpi in theories/dune:2
- -> required by theory elpi_examples in examples/dune:3
- -> required by _build/default/examples/example_abs_evars.vo
- -> required by
-    _build/install/default/lib/coq/user-contrib/elpi_examples/example_abs_evars.vo
- -> required by _build/default/coq-elpi.install
- -> required by alias install

cc @gares

@MSoegtropIMC
Copy link
Contributor Author

@SkySkimmer : what I don't quite understand: elpi is on coq-ci - does fail there on its own as well?

@SkySkimmer
Copy link
Contributor

I guess it's missing LPCIC/coq-elpi@069018b here

@MSoegtropIMC
Copy link
Contributor Author

I guess it's missing LPCIC/coq-elpi@069018b here

This has been merged 2 days ago. How can it fail today lacking this?

@SkySkimmer
Copy link
Contributor

SkySkimmer commented Dec 9, 2024

because it's a opam file change so it needs to be copied to this repo ("here")

@gares
Copy link
Member

gares commented Dec 9, 2024

Right, but I cannot do it today

@MSoegtropIMC
Copy link
Contributor Author

because it's a opam file change so it needs to be copied to this repo ("here")

Ah yes, sorry.

Shall I piggy pack it to this PR?

@gares
Copy link
Member

gares commented Dec 9, 2024

Yes please

@MSoegtropIMC
Copy link
Contributor Author

Done.

@MSoegtropIMC
Copy link
Contributor Author

I now get while building coq-elpi.dev:

# +  File "./test.v", line 1, characters 0-39:
# +  Error: Cannot load Stdlib.Strings.String: no physical path bound to
# +  Stdlib.Strings

@gares ?

@gares
Copy link
Member

gares commented Dec 10, 2024

Looks like the dune files for the tests miss the stdlib dep. I Don't get why I did not figure it out upstream

@MSoegtropIMC
Copy link
Contributor Author

Thanks! Please ping me as soon as the up stream fix is done.

@gares
Copy link
Member

gares commented Dec 15, 2024

should work now

@MSoegtropIMC
Copy link
Contributor Author

Thanks. I will rebase the PR to trigger a new CI.

@MSoegtropIMC
Copy link
Contributor Author

@gares : So I guess the "stdlib-shims" need to remain and I should just remove the second commit from this PR?

@gares
Copy link
Member

gares commented Dec 16, 2024

yes, please remove your second commit

@MSoegtropIMC MSoegtropIMC merged commit 2a2ed16 into coq:master Dec 16, 2024
3 checks passed
@MSoegtropIMC
Copy link
Contributor Author

As announced last Monday I self merge as soon as CI passes => merge.

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.

3 participants