Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a new overrideInputs on {default,shell}Nix #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ckiee
Copy link

@ckiee ckiee commented Jan 1, 2023

This allows the user of this non-flake to have similar functionality to follows.

Example (default.nix):

nix-repl> (import ./.).default
«derivation /nix/store/axgvq22kyb8ymchzq1mjayms4jdl6ni6-ledc-0.1.0.drv»

nix-repl> ((import ./.).overrideInputs { nixpkgs = <nixpkgs>; }).default
«derivation /nix/store/r8zw693hpg91yx6f57hyx1gk4zdiq2lm-ledc-0.1.0.drv»

It's currently only able to override our flake's direct inputs. Sorry for the big diff.

This allows the user of this non-flake to have similar functionality
to `follows`.

Example (default.nix):

    nix-repl> (import ./.).default
    «derivation /nix/store/axgvq22kyb8ymchzq1mjayms4jdl6ni6-ledc-0.1.0.drv»

    nix-repl> ((import ./.).overrideInputs { nixpkgs = <nixpkgs>; }).default
    «derivation /nix/store/r8zw693hpg91yx6f57hyx1gk4zdiq2lm-ledc-0.1.0.drv»
Accept other forks also with type="github".

This is enough to be a bit more convenient in a few more
cases once this propagates into new future projects.
Copy link

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense overall, and the diff isn't even that big when whitespace is ignored :)

in "${toString y'}${pad (toString m)}${pad (toString d)}${pad (toString hours)}${pad (toString minutes)}${pad (toString seconds)}";

rootOverrides =
mapAttrs'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason to use mapAttrs' and not mapAttrs here?

if key == lockFile.root
then rootSrc
else
if rootOverrides.${key} != null then

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if rootOverrides.${key} != null then
if (rootOverrides.${key} or null) != null then

@ckiee
Copy link
Author

ckiee commented Sep 18, 2023

@lheckemann some work continues in https://github.com/ElvishJerricco/flake-compat/tree/add-overrideInputs. i don't currently have the spoons for a review cycle, this may change by next week.

if you're looking for this to get merged, it'd probably be wise to make a PR against https://github.com/nix-community/flake-compat 💜

@DavHau
Copy link

DavHau commented Sep 30, 2023

The current head of the PR is broken, it is missing inputs, no matter if overrideInputs is used or not.
Errror:

error: attribute 'nixpkgs_2' missing

       at /nix/store/dp0f4gy50va2lbrz970lx9v65qb4qv70-source/default.nix:159:20:

          158|               else
          159|                 if rootOverrides.${key} != null then
             |                    ^
          160|                   { type = "path";
       Did you mean nixpkgs?

The PR at the nix-community flake-compat does work fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants