Fix empty outputsToInstall for InstallableAttrPath

[tie]

Motivation

OutputsSpec::Names constructor requires a non-empty set of output names.

nix/src/libstore/outputs-spec.hh

Lines 37 to 46 in 802b4e4

	Names(const std::set<OutputName> & s)
	: std::set<OutputName>(s)
	{ assert(!empty()); }
	/**
	* Needs to be "inherited manually"
	*/
	Names(std::set<OutputName> && s)
	: std::set<OutputName>(s)
	{ assert(!empty()); }

Context

Fixes assertion failure if outputsToInstall is empty by defaulting to the "out" output. That is, behavior between the following commands should be consistent:

$ nix build --no-link --json .#nothing-to-install-no-out
error: derivation '/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-nothing-to-install-no-out.drv' does not have wanted outputs 'out'

$ nix build --no-link --file default.nix --json nothing-to-install-no-out
error: derivation '/nix/store/eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee-nothing-to-install-no-out.drv' does not have wanted outputs 'out'

Real-world example of this issue:

$ nix build --json .#.legacyPackages.aarch64-linux.texlive.pkgs.iwona
error: derivation '/nix/store/dj0h6b0pnlnan5nidnhqa0bmzq4rv6sx-iwona-0.995b.drv' does not have wanted outputs 'out'

$ git rev-parse HEAD
eee33247cf6941daea8398c976bd2dda7962b125
$ nix build --json --file . texlive.pkgs.iwona
nix: src/libstore/outputs-spec.hh:46: nix::OutputsSpec::Names::Names(std::set<std::__cxx11::basic_string<char> >&&): Assertion `!empty()' failed.
Aborted (core dumped)

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[roberth]

I think the root cause here is that InstallableAttrPath::toDerivedPaths() doesn't have a parameter that controls whether we query an installable for its installable outputs or all its outputs. Probably nix build should default to all outputs, unlike nix profile.
That would result in more sensible behavior, certainly in the example.

[tie]

Yes, in the long run this seems like a more sensible behavior, although I’m not sure if this would be a breaking change since we already default to out for InstallableFlake (and InstallableAttrPath crashes Nix command-line tool right now).

nix/src/libcmd/installable-flake.cc

Lines 146 to 149 in 8e9fc28

	if (outputsToInstall.empty())
	outputsToInstall.insert("out");
	return OutputsSpec::Names { std::move(outputsToInstall) };

For now, I think that the hot fix for a crash should be good enough and consistent with flake installables.

[roberth]

I've tracked the cause down to 1ddabe1, which just adds the out behavior without any explanation.

It see multiple issues with it:

• out is not necessarily the default output
• we're ignoring the author's intent not to install anything
• this is derivation-based, not output based - unnecessary coupling we want to move away from (Redefine outputs in terms of language-level "package", not necessarily store-level derivation (RFC-92, and multi-drv packages, docs) #6507)
• not well tested or documented

I'm ok to merge this as a hotfix, but this functionality is bad and should be replaced by something more intentional.

[roberth]

Thank you @tie