prim_toPath: Actually make the string a path

[shlevy]

builtins.toPath currently doesn't actually turn something into a path. In fact, while it does check if the argument is indeed a path and canonicalize it, it actually returns a string even the argument was a path! This fixes that.

$ cat test.nix

let
  a = builtins.readFile (builtins.unsafeDiscardStringContext (derivation {
    name = "a";
    builder = "/bin/false";
    system = "sys";
    path = /etc/nixos/configuration.nix;
  }).drvPath);

  b = builtins.readFile (builtins.unsafeDiscardStringContext (derivation {
    name = "b";
    builder = "/bin/false";
    system = "sys";
    path = builtins.toPath "/etc/nixos/configuration.nix";
  }).drvPath);

  c = builtins.readFile (builtins.unsafeDiscardStringContext (derivation {
    name = "c";
    builder = "/bin/false";
    system = "sys";
    path = builtins.toPath /etc/nixos/configuration.nix;
  }).drvPath);
in

import "${(import <nixpkgs> {}).runCommand "paths.nix" {} ''
  echo "'''" > $out
  echo "${a}" | sed 's/.*\(path,[^)]*\)).*/\1/' >> $out
  echo "${b}" | sed 's/.*\(path,[^)]*\)).*/\1/' >> $out
  echo "${c}" | sed 's/.*\(path,[^)]*\)).*/\1/' >> $out
  echo "'''" >> $out
''}"

Before:
$ echo -e $(nix-instantiate test.nix >/dev/null 2>&1 || nix-instantiate test.nix --eval-only | sed 's"\g')

path,/nix/store/ms6xqxb4pvxga4m9wlmj5rp8my418dwz-configuration.nix
path,/etc/nixos/configuration.nix
path,/etc/nixos/configuration.nix

After:
$ echo -e $(nix-instantiate test.nix >/dev/null 2>&1 || nix-instantiate test.nix --eval-only | sed 's"\g')

path,/nix/store/ms6xqxb4pvxga4m9wlmj5rp8my418dwz-configuration.nix
path,/nix/store/ms6xqxb4pvxga4m9wlmj5rp8my418dwz-configuration.nix
path,/nix/store/ms6xqxb4pvxga4m9wlmj5rp8my418dwz-configuration.nix

[shlevy]

Wow, that was a really complicated test-case. I can post a much simpler one if the issue isn't clear.

[edolstra]

Did you see the comment just above toPath that suggests it's obsolete?

[shlevy]

There's a question mark :) Eh, probably anything that can be done with toPath can be done without it, but it seems misleading to have it be called that and still there and not working. It's not documented in the nix manual as being obsolete and there's no warning message printed or anything, so most users probably wouldn't know that it is and are almost certainly expecting a different behavior than what actually happens. IMO even if it is obsolete it should be correct as long as it's still there and usable.

[edolstra]

Merged, thanks.