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

Implement builtins.path #128

Closed
Tracked by #1030
ryantrinkle opened this issue Apr 4, 2018 · 18 comments · Fixed by #1032
Closed
Tracked by #1030

Implement builtins.path #128

ryantrinkle opened this issue Apr 4, 2018 · 18 comments · Fixed by #1032
Labels
builtins help wanted When someone explicitly requests for help

Comments

@ryantrinkle
Copy link
Member

ryantrinkle commented Apr 4, 2018

This Builtin should be added here.

This doesn't have a .exp file; I think that means it just needs to evaluate without error.

Fixes eval-okay-path (remove it from newFailingTests list).

@ryantrinkle ryantrinkle added the help wanted When someone explicitly requests for help label Apr 4, 2018
@ryantrinkle ryantrinkle added this to the Passing all nix language tests milestone Apr 4, 2018
@jwiegley jwiegley added enhancement Betterness, without new explicit feature possibilities challenging May require somewhat complex changes; not recommend if you aren't familiar with the codebase labels Apr 12, 2018
@jwiegley
Copy link
Member

This could be challenging, since I think these "enriched paths" are supposed to be interchangeable with other paths.

@xplat
Copy link

xplat commented Apr 17, 2018

nix actually handles this by figuring out the store path and using that as a plain path object. There are no actual 'enriched path objects' in nix libexpr.

@xplat
Copy link

xplat commented Apr 17, 2018

Actually now that i check more closely, builtins.path returns a string, not a path!

@domenkozar
Copy link
Collaborator

Right:

nix-repl> :t builtins.path { name = "patchelf"; path = ./patchelf; }
a string with context

@jwiegley jwiegley removed challenging May require somewhat complex changes; not recommend if you aren't familiar with the codebase enhancement Betterness, without new explicit feature possibilities labels Apr 23, 2018
@jglukasik
Copy link

I started looking into this today. Haven't made much progress, but I'm getting more familiar with the codebase. Will try again tomorrow.

@jwiegley
Copy link
Member

Great! Once this is closed we’ll be able to turn on the full language tests for Travis!

@mightybyte
Copy link
Member

@jglukasik Definitely ask for some help from someone on this tomorrow. @shlevy will have all the knowledge you need on the nix side of things. It would be great to get this one finished this weekend.

@jglukasik
Copy link

After some discussion with @mightybyte , we've realized this might be a bit too large of a first change for me. I'll let someone else tackle this.

@jwiegley
Copy link
Member

I'm going to work on this on the plane tomorrow.

@shlevy
Copy link
Member

shlevy commented Apr 30, 2018

@jwiegley This requires nar serialization, which @imalsogreg is working on

@jwiegley
Copy link
Member

I realized that soon after looking into it, so I’m going to hold off and leave you guys to it.

@jwiegley
Copy link
Member

@shlevy since were not reimplementing the store, what are you think about binding to libstore through the FFI?

@ryantrinkle
Copy link
Member Author

We could also shell out as I did for derivationStrict; it's dirty, but if it's easier it might be worth it since we should eventually try not to have a dependency at all.

@jwiegley
Copy link
Member

I’m fine with shelling out right now, since it would let us move on to other problems.

@shlevy
Copy link
Member

shlevy commented May 1, 2018

Eh, libnixstore isn't really stable at all and is not a nice interface at all. Not sure why we'd prefer that to implementing the daemon protocol.

@ryantrinkle
Copy link
Member Author

@shlevy Do you think it makes sense to shell out in the immediate future to get this working, and then use the daemon protocol as the next step?

@shlevy
Copy link
Member

shlevy commented May 1, 2018

If that gets us something useful, sure. First step of the required logic to do readonly mode properly (so no need to talk to the daemon) is up at haskell-nix/hnix-store#7

@Anton-Latukha
Copy link
Collaborator

layus was implementing path in #755.
It for ported to the current code in #1032. Passes the basic test (./data/nix/tests/lang//eval-okay-path.nix).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins help wanted When someone explicitly requests for help
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants