feature: nixpkgs_flake_package #380

rickvanprim · 2023-05-05T17:11:03Z

Introduces a new repository rule nixpkgs_flake_package that invokes nix build <label>#<package> and makes the output available as a Bazel repository. This allows all the Nix version pinning to happen as part of the Nix Flake functionality with flake.lock and simplifies the Bazel side implementation by not having to fetch nixpkgs (or any other Nix repositories) outside of Nix and inject them in.

Contributes to #348.

rickvanprim · 2023-05-05T17:11:47Z

core/nixpkgs.bzl

@@ -1,4 +1,4 @@
-"""<!-- Edit the docstring in `core/nixpkgs.bzl` and run `bazel run //docs:update-README.md` to change this repository's `README.md`. -->
+"""<!-- Edit the docstring in `core/nixpkgs.bzl` and run `bazel run @rules_nixpkgs_docs//:update-readme` to change this repository's `README.md`. -->


Updated to match //nixpkgs/nixpkgs.bzl.

rickvanprim · 2023-05-05T17:11:58Z

core/nixpkgs.bzl

@@ -453,6 +454,7 @@ def _nixpkgs_package_impl(repository_ctx):
    else:
        # No user supplied build file, we may create the default one.
        create_build_file_if_needed = True
+


Auto-added by Buildifier.

rickvanprim · 2023-05-05T17:13:24Z

core/nixpkgs.bzl

+    # Is nix supported on this platform?
+    not_supported = not is_supported_platform(repository_ctx)
+
+    # Should we fail if Nix is not supported?
+    fail_not_supported = repository_ctx.attr.fail_not_supported
+
+    if not_supported and fail_not_supported:
+        fail("Platform is not supported: `nix` not found in PATH. See attribute `fail_not_supported` if you don't want to use Nix.")
+    elif not_supported:
+        return
+
+    # If true, a BUILD file will be created from a template if it does not
+    # exist.
+    # However this will happen AFTER the nix-build command.
+    create_build_file_if_needed = False
+    if repository_ctx.attr.build_file and repository_ctx.attr.build_file_content:
+        fail("Specify one of 'build_file' or 'build_file_content', but not both.")
+    elif repository_ctx.attr.build_file:
+        repository_ctx.symlink(repository_ctx.attr.build_file, "BUILD")
+    elif repository_ctx.attr.build_file_content:
+        repository_ctx.file("BUILD", content = repository_ctx.attr.build_file_content)
+    else:
+        # No user supplied build file, we may create the default one.
+        create_build_file_if_needed = True
+
+        # Workaround to bazelbuild/bazel#4533
+        repository_ctx.path("BUILD")


This could potentially be pulled out of here and nixpkgs_package(). I don't think there's a good way to deduplicate the docstring though?

Factoring out the common parts between nixpkgs_package and nixpkgs_flake_package would be great. There is a fair bit of complexity around invoking Nix that was tricky to get right. I'd be wary of duplicating it and risk it diverging.

rickvanprim · 2023-05-05T17:14:26Z

core/nixpkgs.bzl

+    nix_flake_file_deps = {}
+    for dep_lbl, dep_str in repository_ctx.attr.nix_flake_file_deps.items():
+        nix_flake_file_deps[dep_str] = cp(repository_ctx, dep_lbl)


This could potentially be grouped with the expand_location() call below and pulled out to be shared with nixpkgs_package().

rickvanprim · 2023-05-05T17:14:37Z

core/nixpkgs.bzl

+    expr_args.extend([
+        expand_location(
+            repository_ctx = repository_ctx,
+            string = opt,
+            labels = nix_flake_file_deps,
+            attr = "nixopts",
+        )
+        for opt in repository_ctx.attr.nixopts
+    ])


rickvanprim · 2023-05-05T17:15:34Z

core/nixpkgs.bzl

+    # Create a default BUILD file only if it does not exists and is not
+    # provided by `build_file` or `build_file_content`.
+    if create_build_file_if_needed:
+        p = repository_ctx.path("BUILD")
+        if not p.exists:
+            repository_ctx.template("BUILD", Label("@rules_nixpkgs_core//:BUILD.bazel.tpl"))


Could potentially be pulled into the BUILD file function mentioned above.

rickvanprim · 2023-05-05T17:17:25Z

core/nixpkgs.bzl

+        nix_flake_file,
+        nix_flake_lock_file,
+        nix_flake_file_deps = [],
+        package = None,


Would attribute_path be a better name than package? That would be consistent with nixpkgs_package(). I chose package here initially as it matches the name of one of the known Flake output fields.

rickvanprim · 2023-05-05T21:55:27Z

Looking at //tests, it looks like there's only really one test right now. Should I copy-paste invalid_nixpkgs_package (directory and shell script) to make a test for nixpkgs_flake_package or what's the expectation as far as adding tests? 🙂

aherrmann · 2023-05-08T15:25:46Z

Looking at //tests, it looks like there's only really one test right now.

The tests for core live under testing/core. See #319 for motivation and further details.

Further tests for core should also go there.

aherrmann · 2023-05-08T15:27:15Z

core/nixpkgs.bzl

+      package: Nix Flake package to make available.  The default package will be used if not specified.
+      build_file: The file to use as the BUILD file for this repository.
+
+        Its contents are copied copied into the file `BUILD` in root of the nix output folder. The Label does not need to be named `BUILD`, but can be.


Suggested change

Its contents are copied copied into the file `BUILD` in root of the nix output folder. The Label does not need to be named `BUILD`, but can be.

Its contents are copied into the file `BUILD` in root of the nix output folder. The Label does not need to be named `BUILD`, but can be.

aherrmann · 2023-05-08T15:36:43Z

core/nixpkgs.bzl

+    # Is nix supported on this platform?
+    not_supported = not is_supported_platform(repository_ctx)
+
+    # Should we fail if Nix is not supported?
+    fail_not_supported = repository_ctx.attr.fail_not_supported
+
+    if not_supported and fail_not_supported:
+        fail("Platform is not supported: `nix` not found in PATH. See attribute `fail_not_supported` if you don't want to use Nix.")
+    elif not_supported:
+        return
+
+    # If true, a BUILD file will be created from a template if it does not
+    # exist.
+    # However this will happen AFTER the nix-build command.
+    create_build_file_if_needed = False
+    if repository_ctx.attr.build_file and repository_ctx.attr.build_file_content:
+        fail("Specify one of 'build_file' or 'build_file_content', but not both.")
+    elif repository_ctx.attr.build_file:
+        repository_ctx.symlink(repository_ctx.attr.build_file, "BUILD")
+    elif repository_ctx.attr.build_file_content:
+        repository_ctx.file("BUILD", content = repository_ctx.attr.build_file_content)
+    else:
+        # No user supplied build file, we may create the default one.
+        create_build_file_if_needed = True
+
+        # Workaround to bazelbuild/bazel#4533
+        repository_ctx.path("BUILD")


Factoring out the common parts between nixpkgs_package and nixpkgs_flake_package would be great. There is a fair bit of complexity around invoking Nix that was tricky to get right. I'd be wary of duplicating it and risk it diverging.

benradf · 2023-05-11T12:25:08Z

The workspace CI jobs are failing because the README doesn't match the docstrings in the bzl files. We shouldn't change the README files directly - there are targets to regenerate them from the docstrings. This is minor though - we can fix it when the PR is otherwise ready merge.

The bzlmod CI jobs are failing with this error:

Error in fail: Invalid Nix repository, you must use the nix_repo extension and request a global repository or register a local repository: Unknown repository - no repository named 'flake-hello' available for module 'rules_nixpkgs_core_testing'.

You can reproduce this locally by running the following in the testing/core directory:

bazel run --config=bzlmod //tests:run-flake-hello

I started looking into how the bzlmod code path would need to change to fix this but didn't figure it out yet. I'm guessing nixpkgs_flake_package doesn't do some bzlmod stuff that nixpkgs_package does. Any suggestions @aherrmann?

aherrmann · 2023-05-11T12:50:43Z

testing/core/tests/BUILD.bazel

+        "flake-hello",
+        "flake-hello-with-build-file",


In reply to #380 (comment).
The issue is that these tests were added here. This list comprehension is only valid for tests of packages that are imported using a bzlmod Bazel module extension. However, no such extension exists for the flake part, yet.
You can fix this by adding another instance of the list comprehension like this (unverified code):

[ # All of these tests use the "hello" binary to see # whether different invocations of `nixpkgs_flake_package` # produce a valid bazel repository. sh_test( name = "run-{0}".format(test), timeout = "short", srcs = ["test_bin.sh"], args = ["$(location @{0}//:bin)".format(test)], data = ["@{0}//:bin".format(test)], ) for test in [ "flake-hello", "flake-hello-with-build-file",

(If you go through the Git history of this file, you'll find instances of such code from when not all tests had been converted to use the module extension, yet)

You'll also need to add use_repo entries to MODULE.bazel to import these two repositories from the non_module_deps extension.

Is that just a temporary workaround or the proper fix? It seems like my work here is only half done, as I presumably need to write @rules_nixpkgs_core//extensions:flake_package.bzl (or whatever bikeshed)? This is based on my observation that nix_pkg = use_extension("@rules_nixpkgs_core//extensions:package.bzl", "nix_pkg") seems to be a bzlmod wrapper/validator around nixpkgs_package().

It's a temporary workaround until there is direct bzlmod support for flake with a module extension.
However, I would keep that out of this PR and defer for later. This will require some design work first - right know I don't really have a good picture for what a module extension for flake support should look like. As mentioned in #348 (comment), I don't know how or if we'd want to approach globally unified packages.

Flakes also bring their own dependency management features. There's a question of whether we'd want to integrate those with bzlmod somehow. Part of the bzlmod design is to provide support for package manager integration via module extensions and tags, with a distinct resolution phase before fetching/importing individual deps. So, from that point of view it could make sense to integrate with Nix flake's dependency management features.

bzlmod test fixes pushed. Not sure if they work as the CI jobs don't seem to run automatically?

Yeah, we have to run them manually for contributors from outside the organization. Looks like CI is passing now.

benradf

Good to see _nixpkgs_package_impl being broken up a little - it was getting pretty big for a single macro. Thanks for this contribution @rickvanprim!

feature: nixpkgs_flake_package

7f3ab20

rickvanprim requested a review from benradf as a code owner May 5, 2023 17:11

rickvanprim commented May 5, 2023

View reviewed changes

aherrmann reviewed May 8, 2023

View reviewed changes

rickvanprim added 2 commits May 8, 2023 09:48

Delete flake.lock symlink

7c8bd94

Refactor + Tests

645e6a8

aherrmann reviewed May 11, 2023

View reviewed changes

rickvanprim added 2 commits May 11, 2023 09:40

Documentation fix

23d9c92

bzlmod test fix

aea405f

benradf approved these changes May 16, 2023

View reviewed changes

benradf added the merge-queue merge on green CI label May 16, 2023

benradf merged commit b103523 into tweag:master May 16, 2023

mergify bot removed the merge-queue merge on green CI label May 16, 2023

rickvanprim deleted the jleitch branch May 16, 2023 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: nixpkgs_flake_package #380

feature: nixpkgs_flake_package #380

rickvanprim commented May 5, 2023

rickvanprim May 5, 2023

rickvanprim May 5, 2023

rickvanprim May 5, 2023

aherrmann May 8, 2023

rickvanprim May 5, 2023

rickvanprim May 5, 2023

rickvanprim May 5, 2023

rickvanprim May 5, 2023

rickvanprim commented May 5, 2023

aherrmann commented May 8, 2023

aherrmann May 8, 2023

aherrmann May 8, 2023

benradf commented May 11, 2023

aherrmann May 11, 2023

rickvanprim May 11, 2023

aherrmann May 12, 2023

rickvanprim May 15, 2023

benradf May 16, 2023

benradf left a comment

		@@ -1,4 +1,4 @@
		"""<!-- Edit the docstring in `core/nixpkgs.bzl` and run `bazel run //docs:update-README.md` to change this repository's `README.md`. -->
		"""<!-- Edit the docstring in `core/nixpkgs.bzl` and run `bazel run @rules_nixpkgs_docs//:update-readme` to change this repository's `README.md`. -->

	Its contents are copied copied into the file `BUILD` in root of the nix output folder. The Label does not need to be named `BUILD`, but can be.
	Its contents are copied into the file `BUILD` in root of the nix output folder. The Label does not need to be named `BUILD`, but can be.

feature: nixpkgs_flake_package #380

feature: nixpkgs_flake_package #380

Conversation

rickvanprim commented May 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickvanprim commented May 5, 2023

aherrmann commented May 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benradf commented May 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benradf left a comment

Choose a reason for hiding this comment