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-copy-closure: Implement in C++. #1023

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

shlevy
Copy link
Member

@shlevy shlevy commented Aug 10, 2016

(on top of #981)

@shlevy shlevy added this to the perl-to-c++ milestone Aug 10, 2016
@edolstra edolstra mentioned this pull request Aug 10, 2016
12 tasks
@@ -552,7 +552,7 @@ void removeTempRoots();
ref<Store> openStore(const std::string & uri = getEnv("NIX_REMOTE"));


void copyPaths(ref<Store> from, ref<Store> to, const Paths & storePaths);
void copyPaths(ref<Store> from, ref<Store> to, const Paths & storePaths, bool substitute = false);
Copy link
Member Author

Choose a reason for hiding this comment

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

@k0001 FYI if this gets merged it will be easy to implement #937 in the new c++ build-remote

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for keeping this in mind @shlevy ! I'm really looking forward to having #937 in master.

@shlevy shlevy mentioned this pull request Aug 12, 2016
@shlevy
Copy link
Member Author

shlevy commented Nov 10, 2016

@edolstra What path do you want to go here? Summarizing from previous discussion, I think the two reasonable options are:

  1. package nix-copy-closure.pl with the nix-perl package, install together by default, but with a configure option to build them separately (or, of course, not build the nix-perl stuff at all), recommend new users use nix copy
  2. Do a dumb rewrite of nix-copy-closure.pl and attendant nix-store --serve code to C++.

I strongly prefer 1, as it will give people who don't want perl a way to get it without breaking compat immediately and requiring a decent chunk of work to port something that will just go away eventually, but I will do either of these.

Tests fail currently because the database is not given proper hashes in the VM
@shlevy shlevy force-pushed the nix-copy-closure-c++ branch from 03a4cbe to bfa41eb Compare January 20, 2017 14:53
@shlevy
Copy link
Member Author

shlevy commented Jan 20, 2017

@edolstra rebased on master. Still need advice re: previous comment.

@copumpkin
Copy link
Member

copumpkin commented Jan 20, 2017

👍 for option 1 😄

@shlevy
Copy link
Member Author

shlevy commented Feb 2, 2017

@edolstra ping

@shlevy
Copy link
Member Author

shlevy commented Feb 6, 2017

@edolstra I would really like to wrap this up. Can you please provide feedback here? If I haven't heard anything in a week or so I will go ahead with option 1 above, please let me know before then if that's not likely to be merged so I don't waste my time.

@edolstra edolstra merged commit bfa41eb into NixOS:master Feb 7, 2017
@edolstra
Copy link
Member

edolstra commented Feb 7, 2017

Thanks, merged.

@shlevy
Copy link
Member Author

shlevy commented Feb 7, 2017

Thank you!

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