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

rewrite disko in C++ (WIP) #887

Draft
wants to merge 4 commits into
base: disko-rewrite-python
Choose a base branch
from

Conversation

Sk7Str1p3
Copy link
Contributor

@iFreilicht as i promised, my current code. for now, only basic functions implemented. it feels bit shitty for me, waiting for response and advice , thx.

return nix_Args;
return dry_Run;
return omit_Check;
return verbose;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are multi-returns like this in C++ even an actual feature? I would expect the first return to immediately end the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, now function adds variables into array and returns it.

@iFreilicht
Copy link
Contributor

I think you might have misunderstood what "rewrite" means in the context of #789. A rewrite of just the CLI entrypoint in another language will solve nothing. We need to re-implement all the _create options in all the lib/types/*.nix files. These contain the bash code that's so hard to maintain. In addition, I want to use this opportunity to change to a different execution strategy that allows a user to verify which steps are executed and why before running them.

If you want to make a case for C++ being a good language for this rewrite, you need to show a PoC of that. This is a high ask, I know, but I've been working through multiple iterations of the Python rewrite and haven't even reached that point yet; I myself have not yet shown that a rewrite in Python is even feasible.

@Sk7Str1p3
Copy link
Contributor Author

I think you might have misunderstood what "rewrite" means in the context of #789. A rewrite of just the CLI entrypoint in another language will solve nothing. We need to re-implement all the _create options in all the lib/types/*.nix files.

No, i understood it correctly. CLI entrypoint is just the beginning of this, I'm going to reimplement those functions and make other minor changes.

@iFreilicht
Copy link
Contributor

Okay, good. But in that case I'll unsubscribe from this PR and would ask you to mention me again if anything interesting comes up. I don't want to get pinged every time you push a commit.

@Mic92
Copy link
Member

Mic92 commented Nov 20, 2024

If I am honest, I don't feel comfortable maintaining disko if it was written in C++. I maintain a few C++ project and I run into way too many footguns and we would have the issue that we have to compile inside an installer when used in nixos-anywhere beyond just the entrypoint CLI.
If you want to go this route, I would suggest forking the project. We can link to it from here.

@Sk7Str1p3
Copy link
Contributor Author

Wouldn't we have to compile inside an installer in any way? I don't think this is C++ specific problem. Also (at least for now) maintaining C++ code feels not so bad and hard.

However, if you insist, I can rewrite this in other language. But which one?

@Enzime
Copy link
Contributor

Enzime commented Nov 20, 2024

My preference would be Rust over C++, however the lower bar to entry in Python is also nice

If we just passed a JSON blob to a compiled program then you wouldn’t need to build it locally if it was cached by Hydra/packaged in Nixpkgs

@Mic92
Copy link
Member

Mic92 commented Nov 20, 2024

The rust toolchain is also too big to be downloaded in an in-memory installer. Further details in #789

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.

4 participants