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

lib/types.nix: fix missing inherit #101743

Merged
merged 1 commit into from Oct 26, 2020

Conversation

kini
Copy link
Member

@kini kini commented Oct 26, 2020

I think there was a silent (i.e. semantic) merge conflict between PR #101139 and
PR #100456. This commit should fix the error, which manifests as follows:

error: undefined variable 'boolToString' at /home/kkini/src/nixpkgs/lib/types.nix:552:42
Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

I think there was a silent (i.e. semantic) merge conflict between PR NixOS#101139 and
PR NixOS#100456.  This commit should fix the error, which manifests as follows:

  error: undefined variable 'boolToString' at /home/kkini/src/nixpkgs/lib/types.nix:552:42
@xaverdh
Copy link
Contributor

xaverdh commented Oct 26, 2020

I can confirm that this fixes nixpkgs eval (locally on my machine).

@oyren
Copy link
Contributor

oyren commented Oct 26, 2020

I can confirm that this fixes nixpkgs eval (locally on my machine).

+1

@roberth roberth merged commit d210cbf into NixOS:master Oct 26, 2020
@kini kini deleted the boolToString-merge-conflict branch October 26, 2020 09:30
@roberth
Copy link
Member

roberth commented Oct 26, 2020

It's not clear to me why ofborg failed. The fix looks good.

@kini
Copy link
Member Author

kini commented Oct 26, 2020

Thanks for merging.

I'm not really sure, but I think in the last few months ofborg has started marking commits as failed while they're in its queue and then changing them to succeeded once the evaluations succeed, as opposed to the former behavior where it would log itself as having "started" the check even though it was actually just queued in the backlog.

I assumed the change was made because some kind of problem was occurring when github checks were in the running state for too long, or something. But I guess it also means that now if you merge a PR before ofborg gets to it, the commit will be marked as failing forever (?). Oh well.

@SuperSandro2000
Copy link
Member

It's not clear to me why ofborg failed. The fix looks good.

I think this is because eval on master failed but don't quote me on that.

@cole-h
Copy link
Member

cole-h commented Oct 27, 2020

@SuperSandro2000 That's correct. The ofborg eval failed because master failed to eval at that point in time (despite this PR fixing that).

@kini This is because of proponents of GitHub Actions. Without this immediate failure, the editorconfig check used to give PRs a green checkmark, making people think they are good to merge, even though they're still working through ofborg. Whether this is good or bad is subjective, but that's why it is. (Note: it's not ofborg marking the PR as failed (unless it should be), but a GitHub Action.)

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.

None yet

6 participants