VVKot commented on PR #26122: URL: https://github.com/apache/datafusion/pull/26122#issuecomment-6100019478
Thanks for the callouts @toastal! @kosiew - I had a different line of thinking here. (1) is actually a few different things. Yes, flakes are technically experimental. All major upstream projects have a flake by now though: [nix](https://github.com/NixOS/nix/blob/master/flake.nix), [NixOS](https://github.com/NixOS/nixpkgs/blob/master/flake.nix), [home manager](https://github.com/nix-community/home-manager/blob/master/flake.nix), [hardware](https://github.com/NixOS/nixos-hardware/blob/master/flake.nix). [nix-darwin](https://github.com/nix-darwin/nix-darwin/tree/master#getting-started) says `Flakes (Recommended for beginners)`. A few of the newer projects ([devenv](https://github.com/cachix/devenv/blob/main/flake.nix), [cachix](https://github.com/cachix/cachix/blob/master/flake.nix), [lix](https://git.lix.systems/lix-project/lix/src/branch/main/flake.nix)) _only_ use flakes for development, so there is definitely a precedent for that. In short, I'll continue to wait for the day when https://www.youtube.com/watch?v=wWgxmchHSEw becomes true. > should a project should never assume that if nix is on PATH that flakes are used. This will fail for a number of users That one's more important - I did check that `direnv` [enables flakes on invocation](https://github.com/nix-community/nix-direnv/blob/7a8340f7478661c9a28b9014d0575a171f49dbce/direnvrc#L36), but I didn't consider that folks may be running sufficient outdated `nix` binary that doesn't support the flakes in the first place. I can add an extra check! (2) `shell.nix` isn't hermetic / reproducible in the way flakes are. The are also not composable - for example, we could end up with flakes in datafusion-contrib inheriting from the one here. Frankly, as a newer Nix user, I'm familiar with the flakes, so that's what I contributed / happy to continue maintaining. @toastal feel free to raise a PR for `shell.nix` - I'm not comfortable doing so as I don't use the Nix the old way on any of my machines / don't know it well. For example, after a quick skim [flake-compat](https://github.com/NixOS/flake-compat) sounds interesting -- if incomplete -- way to still support `shell.nix` while having flake as a source of truth. (3) Yes, see the issue attached / [direnv guidance](https://github.com/direnv/direnv/blob/e24ea74873aff78d5e371c85061dc7fafdeedd5a/README.md#quick-demo:~:text=Add%20.direnv/%20to%20your%20.gitignore.%20For%20local%20overrides%20that%20you%20do%20not%20commit%2C%20put%20source_env_if_exists%20.envrc.local%20in%20.envrc%20and%20also%20ignore%20.envrc.local.) that I followed. I think it does provide value - as mentioned in the original PR, there is under-appreciated value in having things "just work". Ghostty maintainers say it well here https://ghostty.org/docs/config#zero-configuration-philosophy. As you've said, the security issue is "potential", and the attack vector is "someone is able to modify a file on disk on your machine" which sounds more concerning than lack of checks for loading nested `.envrc.local`. I've subscribed to the latest issue here https://github.com/direnv/direnv/issues/556 to update the setup in DataFusion once it is resolved. @kosiew on (3) specifically - happy to go with `untrack .envrc and add it to .gitignore` if you feel strongly. My preference would be to update the `.envrc` to check for flake support instead. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
