add --extra-nixpkgs-config

[figsoda]

closes #314

[SomeoneSerge]

Is there anything to be done with list_packages?

nixpkgs-review/nixpkgs_review/review.py

Lines 333 to 347 in 6ffaffa

	def list_packages(
	path: str,
	system: str,
	allow: AllowedFeatures,
	check_meta: bool = False,
	) -> List[Package]:
	cmd = [
	"nix-env",
	"--extra-experimental-features",
	"" if allow.url_literals else "no-url-literals",
	"--option",
	"system",
	system,
	"-f",
	path,

Maybe it's easier to implement --nixpkgs-option config { allowUnfree = true; } than --extra-nixpkgs-config { config.allowUnfree = true; }, because it maps directly into the already familiar nix-env's --arg or --option

[figsoda]

I'm not sure. There are a few things we can do

• use --extra-nixpkgs-config and do nothing to nix-env (what we are doing right now)
• use --extra-nixpkgs-arg (and --extra-nixpkgs-argstr) instead, similar to nix-env's --arg and --argstr
• use --extra-nixpkgs-config and pass it to nix-env (with -E?)

[SomeoneSerge]

IIUC, we need either of the last two options, otherwise review.differences will miss the affected attributes. Either is good, and I don't know which is easier to maintain

[Mic92]

I will skip reviewing this feature but I think we should have some way of passing nixpkgs configuration into nixpkgs-review. Just merge if you come to a consensus @SuperSandro2000 @figsoda

[SomeoneSerge]

If extra-nixpkgs-config contains config as an attribute, we're going to loose allowUnfree. We either need a recursive merge or we stick to --nixpkgs-arg[str] instead

[SomeoneSerge]

Here's another bit, I think:

nixpkgs-review/nixpkgs_review/buildenv.py

Lines 38 to 43 in 6ffaffa

	self.nixpkgs_config = NamedTemporaryFile()
	self.nixpkgs_config.write(
	str.encode(
	f"""{{
	allowUnfree = true;
	allowBroken = true;

[SomeoneSerge]

Here's a preview of the current status: https://gist.github.com/SomeoneSerge/745681c4d577fac2b2e71b6349cc2503 with this quick solution to differences(): SomeoneSerge@9c3c9fe

At a glance, I think the list of packages looks good

[Mic92]

When this PR is merged I will make a new release.

[figsoda]

One issue I have currently is that nix-env doesn't do recursive updates. Would any config change the outputs of nix-env? I'm thinking we should just ignore nix-env and implement recursive updates for the regular eval I think we should go with --extra-nixpkgs-config instead so we don't have to handle multiple arguments

[SomeoneSerge]

I think (I hope) I'm observing some false-positive "updates": https://gist.github.com/SomeoneSerge/6cc00b41964e43f725fc12046778532d Sorry, it works alright, just accidentally opened a portal to hell. I'll run without global cudaSupport and post again when it's over

[SomeoneSerge]

Is it expect behaviour that nixpkgs-review pr doesn't always print the "X packages updated" part? E.g. here it didn't mention nix-env and just went ahead to nix build: https://gist.github.com/5c5ceccb1add211d551a9813ef8be960

[figsoda]

It is expected I believe: e704d6a

[SomeoneSerge]

The current version works for me (example: NixOS/nixpkgs#218265 (comment)) and I'm happy with the UX. I find it confusing that for some PRs the tool doesn't show in advance which packages it's going to build, but I'm not even sure if it's related to this change. The rest is in Sandro's scope, I suppose:)

[figsoda]

@SuperSandro2000

[figsoda]

bors r+

I messaged sandro privately. He is currently prioritizing on other things and is not against merging this

[bors]

Build succeeded:

• build-flake
• tests

[nviets]

Can I use this argument to cap resources with max-jobs and cores? By default nixpkgs-review tries to build with everything.

[SuperSandro2000]

You want to set those settings in your nix.conf.