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

nix: init #264

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

nix: init #264

wants to merge 6 commits into from

Conversation

jhvst
Copy link

@jhvst jhvst commented Nov 28, 2024

Hi there,

I have packaged the pulse project with Nix while targeting the earlier release branch made earlier the year. I'm opening this PR to gauge interest in upstreaming these configurations -- I noticed that FStar and Karamel had these already, but pulse did not.

Are you interested in accepting Nix packaging for pulse?

To elaborate on this, this adds a flake.nix with devenv integration that uses the upstream FStar Nix flake to build a pulse artifact that compiles. Currently, you still need to run make and make install manually, but I plan to properly package pulse later on.

I think it makes sense to make a bash wrapper for pulse that automatically populates the basic library files, and then allow additional arguments to be passed as $2 bash argument, where $1 is the filename.

Some things that are still missing:

  • [] Karamel integration
  • [] pulse2rust integration
  • [] getting pulse to compile against the latest master branch
  • getting pulse to compile using a makefile

I plan to have at least the pulse2rust be built as well, since that is what I want to target.

Before that, I am marking this as draft.

Let me know if there is something else to consider!

@nikswamy
Copy link
Contributor

Thanks! Much appreciated. Just a heads up that most of us maintainers are on holiday until Monday. So, please expect that it'll take us until then to give you some proper feedback.

@jhvst
Copy link
Author

jhvst commented Nov 30, 2024

So, I got pulse to compile using the Makefile in 381c5e6. Luckily, no ugly hacks were involved! While doing this, I also created the dune package, which I am not sure is redundant or not. Then, I created the wrapper script in 8a590d9 which uses the paths in the Nix store to run fstar with the pulse libraries loaded. Extra arguments can be passed as normal to fstar.

To continue with packaging pulse2rust, Nix would really want you to commit the Cargo.lock file as part of the project. This avoids some edge-cases where a package version gets re-released (has happened to me before...). Alternatively, I can include whatever version I get out as part of the PR.

--include ${config.packages.pulse}/lib/pulse/lib/pledge \
--load_cmxs pulse \
"$@"
'';
Copy link
Member

Choose a reason for hiding this comment

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

A nit here, given that lib/fstar.include exists, the only thing needed is --include ${config.packages.pulse}/lib.

(Also F* will try to automatically load plugins when there's is a #lang-X directive, so the --load_cmxs is mostly redundant, but still required for some files currently, so best to keep it.)

Copy link
Author

Choose a reason for hiding this comment

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

OK, I have to figure out how to get the lib/fstar.include to be found. For some reason I could not get the command to work unless the paths are exhaustively and explicitly included.

Copy link
Member

Choose a reason for hiding this comment

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

Ah.. interesting. Then this is fine and we can later try to reduce it, fstar.include is rather new so no worries.

flake.nix Outdated
stdint
yojson
zarith
]);
Copy link
Member

Choose a reason for hiding this comment

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

This list is here to specify only a subset of the F* ocaml packages, right? Would this break if F* took more dependencies?

Copy link
Author

Choose a reason for hiding this comment

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

True, addressed in c2ff0ea

@mtzguido
Copy link
Member

mtzguido commented Dec 2, 2024

Hi Juuso, thanks for this! I left a question (total non-Nix-expert here).

Could we add a Nix CI job too? I think this workflow https://github.com/FStarLang/FStar/blob/master/.github/workflows/nix.yaml should just work.

@jhvst
Copy link
Author

jhvst commented Dec 9, 2024

Could we add a Nix CI job too? I think this workflow https://github.com/FStarLang/FStar/blob/master/.github/workflows/nix.yaml should just work.

Changes done in a9b8313 should enable all to work (nix build -L stands for default, so now that's added).

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