search

NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.6k stars 1.51k forks source link

Use portable C++ Pseudorandom number generator #10541

Open Ericson2314 opened 6 months ago

Ericson2314 commented 6 months ago

Avoid calls to rand / srand / random / srandom and other things which are not portable to Windows

_Originally posted by @edolstra in https://github.com/NixOS/nix/pull/8901#discussion_r1568529190_

nixos-discourse commented 6 months ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-on-windows/1113/109

RCoeurjoly commented 6 months ago

I see that random() is called in two places:

src/libstore/unix/gc.cc:44: Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), random()); src/libstore/unix/optimise-store.cc:228: Path tempLink = fmt("%1%/.tmp-link-%2%-%3%", realStoreDir, getpid(), random());

Should we create a PRNG before every call? Or create a PRNG in initNix that somehow gets passed to the functions that need it?

Ericson2314 commented 6 months ago

@RCoeurjoly good question! When in doubt, it is better to avoid global state.

I think for the gc one, it would be good to put the PRNG state in IndirectRootStore, and since LocalStore is a subclass, the optimise-store one can use that too.

Thanks for looking into this!

dottharun commented 6 months ago

I see another random() call in - src/libstore/unix/build/derivation-goal.cc:826 too

Ericson2314 commented 6 months ago

That can also use a per-store PRNG

RCoeurjoly commented 6 months ago

The problem is that random() in gc.cc:44 is called by makeSymlink(const Path & link, const Path & target), a free function, so there is no way to access a per-store PRNG.

I was thinking of the following:

Initialize the PRNG in initNix, as it is done now with srandom(). Use a class with static members.

Use this class wherever is needed.

Thoughts?

dottharun commented 6 months ago

IndirectRootStore Mix-in seems to have a clear purpose of providing its two methods

Ericson2314 commented 6 months ago

@RCoeurjoly After #10556 it will not be a free function

@dottharun And the PRNG will be for one the IndirectRootStore methods; the indirect root symlinks not colliding in the directory is part of the spec! :)

Ericson2314 commented 6 months ago

After https://github.com/NixOS/nix/pull/10556 it will not be a free function

If anyone wants to take this issue, they should feel free to do just redo that part of #10556 in their PR. That is better than waiting for my PR.

Ericson2314 commented 5 months ago

https://github.com/NixOS/nix/pull/10556 is now merged, so this should be a bit easier.