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

baseNameOf: Don't copy paths to the store first #376

Merged
merged 2 commits into from Jan 29, 2015

Conversation

shlevy
Copy link
Member

@shlevy shlevy commented Oct 19, 2014

This is technically a breaking change, but I doubt anyone relies on the existing semantics. I'd expect baseNameOf ./foo to be "foo", not "somehash-foo". Currently I'm doing baseNameOf (toString ./foo) everywhere I need baseNameOf.

@shlevy
Copy link
Member Author

shlevy commented Oct 19, 2014

cc @edolstra

@vcunat
Copy link
Member

vcunat commented Oct 19, 2014

+1 for your semantics. Nix manual mentions only how it affects strings (not paths), so this seems a conservative extension.

If accepted, it should be mentioned in docs for baseNameOf, I think.

@edolstra
Copy link
Member

This is probably fine. Though I'm not sure if we should do it before or after the 1.8 release. It's probably better to do it after, so we have some time to notice if anything breaks.

@shlevy
Copy link
Member Author

shlevy commented Oct 19, 2014

Sure, sounds good.

@ehmry
Copy link
Contributor

ehmry commented Oct 28, 2014

+1 This is has been annoying me for some time.

@copumpkin
Copy link
Member

This just bit me too

@shlevy shlevy merged commit c9bd6a1 into NixOS:master Jan 29, 2015
zolodev pushed a commit to zolodev/nix that referenced this pull request Jan 1, 2024
* Rewrite "Set up development environment"

The previous version is specific to Python as it uses
`buildPythonApplication` instead of `mkShell`.

This one is more general but maintains the same Python Flask web
application example.

* Update pinning-nixpkgs to use 22.11

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
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

5 participants