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

Revise division of labor in deserialization of derivations #3434

Merged
merged 10 commits into from Aug 27, 2020

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Mar 21, 2020

Now:

  • No Store methods are defined in derivations.cc

  • parseDerivation on the underlying string is exposed, since that is the root functionality. (It might be better to make this take an arbitrary std::istringstream at some point.)

  • Store::derivationFromPath and Store::readDerivation are defined less redundantly.

I think it makes more sense to define the data model (derivations), before the operations (store api). This has already been done by other already-merged PRs.

@Ericson2314
Copy link
Member Author

@edolstra This little PR look OK to you? I have a lot of changes I'd like to land so don't want to accumulate too much of a backlog.

This further continues with the dependency inverstion. Also I just went
ahead and exposed `parseDerivation`: it seems like the more proper
building block, and not a bad thing to expose if we are trying to be
less wedded to drv files on disk anywas.
@Ericson2314 Ericson2314 changed the title Flip dependency so store-api.hh includes derivations.hh Define derivations before store Jun 17, 2020
@Ericson2314
Copy link
Member Author

Renamed because this is now a bit more than merely flipping the dependency.

@Ericson2314 Ericson2314 changed the title Define derivations before store WIP: Define derivations before store Jun 17, 2020
@Ericson2314 Ericson2314 changed the title WIP: Define derivations before store Define derivations before store Jul 16, 2020
@Ericson2314 Ericson2314 changed the title Define derivations before store Revise division of labor in deserialization derivations Aug 1, 2020
@Ericson2314 Ericson2314 changed the title Revise division of labor in deserialization derivations Revise division of labor in deserialization of derivations Aug 2, 2020
@edolstra edolstra merged commit eb75282 into NixOS:master Aug 27, 2020
@Ericson2314 Ericson2314 deleted the derivation-header-include-order branch August 27, 2020 14:46
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.

None yet

3 participants