Merge pull request tweag#33 from Profpatsch/deprecate-path

Deprecate path

CHANGELOG.md

@@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ## [Unreleased]
 
+### Removed
+
+* The `path` attribute has been removed. See `Migration` section
+  in `README.md` for instructions.
+
 ### Changed
 
 * `nixpkgs_packages` does not accept implicit `<nixpkgs>` version. See

README.md

@@ -63,7 +63,7 @@ nixpkgs_git_repository(name, revision, sha256)
       <td><code>name</code></td>
       <td>
         <p><code>Name; required</code></p>
-        <p>A unique name for this target</p>
+        <p>A unique name for this repository.</p>
       </td>
     </tr>
     <tr>
@@ -101,11 +101,11 @@ Make the content of a Nixpkgs package available in the Bazel workspace.
 ```bzl
 nixpkgs_package(
     name, attribute_path, nix_file, nix_file_deps, nix_file_content,
-    path, repository, build_file, build_file_content,
+    repositories, build_file, build_file_content,
 )
 ```
 
-If neither `repository` or `path` are specified, you must provide a
+If `repositories` is not specified, you must provide a
 nixpkgs clone in `nix_file` or `nix_file_content`.
 
 <table class="table table-condensed table-bordered table-params">
@@ -164,20 +164,19 @@ nixpkgs clone in `nix_file` or `nix_file_content`.
       </td>
     </tr>
     <tr>
-      <td><code>repositorie</code></td>
+      <td><code>repositories</code></td>
       <td>
         <p><code>Label-keyed String dict; optional</code></p>
-        <p>A dictionary mapping repositoriy labels to `NIX_PATH` entries.
-        Specify one of `path` or `repositories`.</p>
-      </td>
-    </tr>
-    <tr>
-      <td><code>path</code></td>
-      <td>
-        <p><code>String; optional</code></p>
-        <p>The path to the directory containing Nixpkgs, as
-           interpreted by `nix-build`. Specify one of `path` or
-		   `repositories`.</p>
+        <p>A dictionary mapping repositoriy labels to `NIX_PATH` entries.</p>
+        <p>Setting it to <pre>
+repositories = { "myrepo" : "//:myrepo" }
+</pre>
+           for example would replace all instances
+           of <code>&lt;myrepo&gt;</code> in the called nix code by the
+           path to the target <code>"//:myrepo"</code>. See the
+           <a href="https://nixos.org/nix/manual/#env-NIX_PATH">relevant
+           section in the nix manual</a> for more information.</p>
+        <p>Specify one of `path` or `repositories`.</p>
       </td>
     </tr>
     <tr>
@@ -198,3 +197,32 @@ nixpkgs clone in `nix_file` or `nix_file_content`.
     </tr>
   </tbody>
 </table>
+
+
+## Migration
+
+### `path` Attribute
+
+`path` was an attribute from the early days of `rules_nixpkgs`, and
+its ability to reference arbitrary paths a danger to build hermeticity.
+
+Replace it with either `nixpkgs_git_repository` if you need
+a specific version of `nixpkgs`. If you absolutely *must* depend on a
+local folder, use bazel’s
+[`local_repository` workspace rule](https://docs.bazel.build/versions/master/be/workspace.html#local_repository).
+Both approaches work well with the `repositories` attribute of `nixpkgs_package`.
+
+```bzl
+local_repository(
+  name = "local-nixpkgs",
+  path = "/path/to/nixpkgs",
+)
+
+nixpkgs_package(
+  name = "somepackage",
+  repositories = {
+    "nixpkgs": "@local-nixpkgs//:default.nix",
+  },
+  …
+)
+```

WORKSPACE

@@ -4,7 +4,23 @@ load("//nixpkgs:nixpkgs.bzl", "nixpkgs_git_repository", "nixpkgs_package")
 
 # For tests
 
-nixpkgs_package(name = "hello", repositories = { "nixpkgs": "//:nix/default.nix" })
+nixpkgs_git_repository(
+  name = "remote_nixpkgs",
+  remote = "https://github.com/NixOS/nixpkgs",
+  revision = "18.09",
+  sha256 = "6451af4083485e13daa427f745cbf859bc23cb8b70454c017887c006a13bd65e",
+)
+
+nixpkgs_package(
+    name = "nixpkgs-git-repository-test",
+    repositories = { "nixpkgs": "@remote_nixpkgs//:default.nix" },
+    attribute_path = "hello",
+)
+
+nixpkgs_package(
+  name = "hello",
+  repositories = { "nixpkgs": "//:nix/default.nix" }
+)
 
 nixpkgs_package(
   name = "expr-test",

nixpkgs/nixpkgs.bzl

@@ -40,7 +40,7 @@ def _nixpkgs_package_impl(ctx):
     ctx.template("BUILD", Label("@io_tweag_rules_nixpkgs//nixpkgs:BUILD.pkg"))
 
   strFailureImplicitNixpkgs = (
-     "One of 'path', 'repositories', 'nix_file' or 'nix_file_content' must be provided. "
+     "One of 'repositories', 'nix_file' or 'nix_file_content' must be provided. "
      + "The NIX_PATH environment variable is not inherited.")
 
   expr_args = []
@@ -50,7 +50,7 @@ def _nixpkgs_package_impl(ctx):
     ctx.symlink(ctx.attr.nix_file, "default.nix")
   elif ctx.attr.nix_file_content:
     expr_args = ["-E", ctx.attr.nix_file_content]
-  elif not (ctx.attr.path or repositories):
+  elif not repositories:
     fail(strFailureImplicitNixpkgs)
   else:
     expr_args = ["-E", "import <nixpkgs> {}"]
@@ -75,20 +75,16 @@ def _nixpkgs_package_impl(ctx):
     "--out-link", "bazel-support/nix-out-link"
   ])
 
-  # If neither repositories or path are set, leave empty which will use
-  # default value from NIX_PATH, which will fail unless a pinned nixpkgs is
-  # set in the 'nix_file' attribute.
+  # If repositories is not set, leave empty so nix will fail
+  # unless a pinned nixpkgs is set in the `nix_file` attribute.
   nix_path = ""
-  if repositories and ctx.attr.path:
-      fail("'repositories' and 'path' attributes are mutually exclusive.")
-  elif repositories:
+  if repositories:
     # XXX Another hack: the repository label typically resolves to
     # some top-level package in the external workspace. So we use
     # dirname to get the actual workspace path.
-    nix_path = ":".join([(path_name + "=" + str(ctx.path(path).dirname)) \
-                         for (path, path_name) in repositories.items()])
-  elif ctx.attr.path:
-    nix_path = str(ctx.attr_path)
+    nix_path = ":".join(
+      [(path_name + "=" + str(ctx.path(path).dirname))
+         for (path, path_name) in repositories.items()])
   elif not (ctx.attr.nix_file or ctx.attr.nix_file_content):
     fail(strFailureImplicitNixpkgs)
 
@@ -123,7 +119,6 @@ _nixpkgs_package = repository_rule(
     "nix_file": attr.label(allow_single_file = [".nix"]),
     "nix_file_deps": attr.label_list(),
     "nix_file_content": attr.string(),
-    "path": attr.string(),
     "repositories": attr.label_keyed_string_dict(),
     "repository": attr.label(),
     "build_file": attr.label(),

tests/BUILD

@@ -13,4 +13,5 @@ package(default_testonly = 1)
     "expr-attribute-test",
     "nix-file-test",
     "nix-file-deps-test",
+    "nixpkgs-git-repository-test",
 ]]