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

ifdefs on elpi version in source code #693

Merged
merged 4 commits into from
Oct 2, 2024
Merged

Conversation

gares
Copy link
Contributor

@gares gares commented Sep 20, 2024

No description provided.

@gares
Copy link
Contributor Author

gares commented Sep 20, 2024

@vbgl I need some nix expertise here.
It seems that %{version:elpi} does not work on nix, it seems to expand to the empty string.
Is it expected? It should be the version written in the META file, in turn generated by dune...

@vbgl
Copy link
Contributor

vbgl commented Sep 20, 2024

I think the problem comes from the fact that elpi in nixpkgs is built from the git repository rather than from the released tarball (which contains correct version information, I guess when produced by dune-release).

@gares
Copy link
Contributor Author

gares commented Sep 20, 2024

Hem, ok, thanks for explaining.

Next question is: how can I fix that?

Is using the tarball generated by dune-release a problem for nix?

@vbgl
Copy link
Contributor

vbgl commented Sep 20, 2024

I’ll propose a patch for nixpkgs asap. What is suppose to happen to your dune scripts (in coq-elpi) when using a development version (i.e., not a release with a proper version string) of elpi?

Comment on lines 7 to 12
(* sanitization *)
let l =
match l with
| l when List.for_all is_number l -> l
| ["%%VERSION_NUM%%"] -> ["99";"99";"99"]
| _ -> Printf.eprintf "version_parser: cannot parse: %s\n" v; exit 1 in
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vbgl this was my "attempt" to support development versions. I'm happy to improve on that

etc/version_parser.ml Outdated Show resolved Hide resolved
@vbgl
Copy link
Contributor

vbgl commented Sep 23, 2024

The fix has been merged into nixpkgs master branch. I hope this addresses you problems. Please ping again if it’s not the case. Kudos to Cyril for the patch.

@gares
Copy link
Contributor Author

gares commented Sep 24, 2024

@CohenCyril opam works on master, but not nix. Since I can't run nix on my machine, is there any chance to get verbose logs in CI?

@gares
Copy link
Contributor Author

gares commented Sep 24, 2024

I mean, the CI file is generated, and it contains 2>&1 > /dev/null. Can I disable that "feature"?

@gares gares merged commit 5403138 into master Oct 2, 2024
82 of 84 checks passed
@gares gares deleted the support-multiple-elpi-version branch October 2, 2024 11:44
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.

2 participants