Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/manual/src/release-notes/rl-next.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,8 @@

Selecting derivation outputs using the attribute selection syntax
(e.g. `nixpkgs#glibc.dev`) no longer works.

* `nix repl` now takes installables on the command line, unifying the usage
with other commands that use `--file` and `--expr`. Primary breaking change
is for the common usage of `nix repl '<nixpkgs>'` which can be recovered with
`nix repl --file '<nixpkgs>'` or `nix repl --expr 'import <nixpkgs>{}'`
Comment thread
tomberek marked this conversation as resolved.
3 changes: 2 additions & 1 deletion src/libcmd/command.hh
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,13 @@ struct InstallablesCommand : virtual Args, SourceExprCommand
InstallablesCommand();

void prepare() override;
Installables load();

virtual bool useDefaultInstallables() { return true; }

std::optional<FlakeRef> getFlakeRefForCompletion() override;

private:
protected:
Comment thread
edolstra marked this conversation as resolved.
Comment thread
edolstra marked this conversation as resolved.

std::vector<std::string> _installables;
};
Expand Down
7 changes: 6 additions & 1 deletion src/libcmd/installables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1025,11 +1025,16 @@ InstallablesCommand::InstallablesCommand()

void InstallablesCommand::prepare()
{
installables = load();
}

Installables InstallablesCommand::load() {
Installables installables;
if (_installables.empty() && useDefaultInstallables())
// FIXME: commands like "nix profile install" should not have a
// default, probably.
_installables.push_back(".");
installables = parseInstallables(getStore(), _installables);
return parseInstallables(getStore(), _installables);
}

std::optional<FlakeRef> InstallablesCommand::getFlakeRefForCompletion()
Expand Down
2 changes: 2 additions & 0 deletions src/libcmd/installables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ struct Installable
const std::vector<std::shared_ptr<Installable>> & installables);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const std::vector<std::shared_ptr<Installable>> & installables);
const Installables & installables);

and there are probably more places that can use the Installables type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I started to make this change, but the compiler was not happy with how I was doing it... we can leave this to another PR? or during hacking session?

};

typedef std::vector<std::shared_ptr<Installable>> Installables;
Comment thread
tomberek marked this conversation as resolved.

struct InstallableValue : Installable
{
ref<EvalState> state;
Expand Down
1 change: 1 addition & 0 deletions src/libutil/experimental-features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ std::map<ExperimentalFeature, std::string> stringifiedXpFeatures = {
{ Xp::RecursiveNix, "recursive-nix" },
{ Xp::NoUrlLiterals, "no-url-literals" },
{ Xp::FetchClosure, "fetch-closure" },
{ Xp::ReplFlake, "repl-flake" },
};

const std::optional<ExperimentalFeature> parseExperimentalFeature(const std::string_view & name)
Expand Down
1 change: 1 addition & 0 deletions src/libutil/experimental-features.hh
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ enum struct ExperimentalFeature
RecursiveNix,
NoUrlLiterals,
FetchClosure,
ReplFlake,
};

/**
Expand Down
95 changes: 70 additions & 25 deletions src/nix/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ extern "C" {
#include "ansicolor.hh"
#include "shared.hh"
#include "eval.hh"
#include "eval-cache.hh"
#include "eval-inline.hh"
#include "attr-path.hh"
#include "store-api.hh"
Expand All @@ -48,10 +49,12 @@ struct NixRepl
#endif
{
std::string curDir;
std::unique_ptr<EvalState> state;
ref<EvalState> state;
Bindings * autoArgs;

Strings loadedFiles;
typedef std::vector<std::pair<Value*,std::string>> AnnotatedValues;
std::function<AnnotatedValues()> getValues;

const static int envSize = 32768;
StaticEnv staticEnv;
Expand All @@ -61,13 +64,15 @@ struct NixRepl

const Path historyFile;

NixRepl(const Strings & searchPath, nix::ref<Store> store);
NixRepl(const Strings & searchPath, nix::ref<Store> store,ref<EvalState> state,
std::function<AnnotatedValues()> getValues);
~NixRepl();
void mainLoop(const std::vector<std::string> & files);
void mainLoop();
StringSet completePrefix(const std::string & prefix);
bool getLine(std::string & input, const std::string &prompt);
StorePath getDerivationPath(Value & v);
bool processLine(std::string line);

void loadFile(const Path & path);
void loadFlake(const std::string & flakeRef);
void initEnv();
Expand All @@ -92,8 +97,10 @@ std::string removeWhitespace(std::string s)
}


NixRepl::NixRepl(const Strings & searchPath, nix::ref<Store> store)
: state(std::make_unique<EvalState>(searchPath, store))
NixRepl::NixRepl(const Strings & searchPath, nix::ref<Store> store, ref<EvalState> state,
std::function<NixRepl::AnnotatedValues()> getValues)
: state(state)
, getValues(getValues)
, staticEnv(false, &state->staticBaseEnv)
, historyFile(getDataDir() + "/nix/repl-history")
{
Expand Down Expand Up @@ -198,16 +205,12 @@ namespace {
}
}

void NixRepl::mainLoop(const std::vector<std::string> & files)
void NixRepl::mainLoop()
{
std::string error = ANSI_RED "error:" ANSI_NORMAL " ";
notice("Welcome to Nix " + nixVersion + ". Type :? for help.\n");

for (auto & i : files)
loadedFiles.push_back(i);

reloadFiles();
if (!loadedFiles.empty()) notice("");

// Allow nix-repl specific settings in .inputrc
rl_readline_name = "nix-repl";
Expand Down Expand Up @@ -630,7 +633,6 @@ bool NixRepl::processLine(std::string line)
return true;
}


void NixRepl::loadFile(const Path & path)
{
loadedFiles.remove(path);
Expand Down Expand Up @@ -684,13 +686,15 @@ void NixRepl::reloadFiles()
Strings old = loadedFiles;
loadedFiles.clear();

bool first = true;
for (auto & i : old) {
if (!first) notice("");
first = false;
notice("Loading '%1%'...", i);
loadFile(i);
}

for (auto & [i, what] : getValues()) {
notice("Loading installable '%1%'...", what);
addAttrsToScope(*i);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this effectively merges the attributes of multiple installables?

I don't like that "loadables" are loaded into the global scope anyway. It makes discovering which attributes exist harder than it should.

Perhaps flakes and installables could be put in a binding that's derived from the installable name?

# nix repl nixpkgs
Loading Installable 'flake:nixpkgs#'...
Added 1 variable.

nix-repl> nixpkgs.legacyPackages.x86_64-linux.emacs.name
"emacs-27.1"

nix-repl> nixpkgs.<TAB>
<UX delight ensues as all top-level flake attrs are listed>

Also interesting

# nix repl . .#inputs.nixpkgs
Added 2 variables

nix-repl> foo.<TAB>

nix-repl> nixpkgs.lib.concatStringsSep ":" ["testing" "it"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The only problem with this is that flakes don't have an obvious name, so it's not entirely straightforward what the top-level attribute name (e.g. nixpkgs) should be.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right. I pulled foo out of thin air.
Most flake refs have a somewhat straightforward candidate for a name.
Using dirname $PWD for . has produced mixed success in the past though.

Maybe the attribute name should be provided by the user.

nix repl --bind foo . --bind nixpkgs .#inputs.nixpkgs

or

nix repl foo=. nixpkgs=.#inputs.nixpkgs

Adding a bare flake or installable to the top-level scope seems ok-ish as long as there's only one. Or maybe it should default to flake= for a flake and a name derived from the installable otherwise?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We do have the previous behavior of multiple --files to worry about.

I wouldn't mind if it just made an array called args, hah! Then don't have to worry about names, and :a can put whatever one likes at the top level.

}
}


Expand Down Expand Up @@ -886,17 +890,32 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
return str;
}

struct CmdRepl : StoreCommand, MixEvalArgs
struct CmdRepl : InstallablesCommand
{
CmdRepl(){
evalSettings.pureEval = false;
}
void prepare()
{
if (!settings.isExperimentalFeatureEnabled(Xp::ReplFlake) && !(file) && this->_installables.size() >= 1) {
warn("future versions of Nix will require using `--file` to load a file");
if (this->_installables.size() > 1)
warn("more than one input file is not currently supported");
auto filePath = this->_installables[0].data();
file = std::optional(filePath);
_installables.front() = _installables.back();
_installables.pop_back();
}
installables = InstallablesCommand::load();
}
std::vector<std::string> files;

CmdRepl()
Strings getDefaultFlakeAttrPaths() override
{
expectArgs({
.label = "files",
.handler = {&files},
.completer = completePath
});
return {""};
}
virtual bool useDefaultInstallables() override
{
return file.has_value() or expr.has_value();
}

std::string description() override
Expand All @@ -913,10 +932,36 @@ struct CmdRepl : StoreCommand, MixEvalArgs

void run(ref<Store> store) override
{
evalSettings.pureEval = false;
auto repl = std::make_unique<NixRepl>(searchPath, openStore());
auto state = getEvalState();
auto getValues = [&]()->NixRepl::AnnotatedValues{
auto installables = load();
NixRepl::AnnotatedValues values;
for (auto & installable: installables){
auto what = installable->what();
if (file){
auto [val, pos] = installable->toValue(*state);
auto what = installable->what();
state->forceValue(*val, pos);
auto autoArgs = getAutoArgs(*state);
auto valPost = state->allocValue();
state->autoCallFunction(*autoArgs, *val, *valPost);
state->forceValue(*valPost, pos);
values.push_back( {valPost, what });
} else {
auto [val, pos] = installable->toValue(*state);
values.push_back( {val, what} );
}
}
return values;
};
auto repl = std::make_unique<NixRepl>(
searchPath,
openStore(),
state,
getValues
);
repl->autoArgs = getAutoArgs(*repl->state);
repl->mainLoop(files);
repl->mainLoop();
}
};

Expand Down
30 changes: 27 additions & 3 deletions src/nix/repl.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,34 @@ R""(
* Interact with Nixpkgs in the REPL:

```console
# nix repl '<nixpkgs>'
# nix repl --file example.nix
Loading Installable ''...
Added 3 variables.

Loading '<nixpkgs>'...
Added 12428 variables.
# nix repl --expr '{a={b=3;c=4;};}'
Loading Installable ''...
Added 1 variables.

# nix repl --expr '{a={b=3;c=4;};}' a
Loading Installable ''...
Added 1 variables.

# nix repl nixpkgs
Comment thread
tomberek marked this conversation as resolved.
Outdated
Loading Installable 'flake:nixpkgs#'...
Added 5 variables.

nix-repl> legacyPackages.x86_64-linux.emacs.name
"emacs-27.1"

nix-repl> legacyPackages.x86_64-linux.emacs.name
"emacs-27.1"

nix-repl> :q

# nix repl --expr 'import <nixpkgs>{}'

Loading Installable ''...
Added 12439 variables.
Comment thread
tomberek marked this conversation as resolved.
Outdated

nix-repl> emacs.name
"emacs-27.1"
Expand Down
55 changes: 51 additions & 4 deletions tests/repl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,17 @@ testRepl
testRepl --store "$TEST_ROOT/store?real=$NIX_STORE_DIR"

testReplResponse () {
local response="$(nix repl <<< "$1")"
echo "$response" | grep -qs "$2" \
local commands="$1"; shift
local expectedResponse="$1"; shift
local response="$(nix repl "$@" <<< "$commands")"
echo "$response" | grep -qs "$expectedResponse" \
|| fail "repl command set:

$1
$commands

does not respond with:

$2
$expectedResponse

but with:

Expand All @@ -71,3 +73,48 @@ testReplResponse '
:a { a = "2"; }
"result: ${a}"
' "result: 2"

testReplResponse '
drvPath
' '".*-simple.drv"' \
$testDir/simple.nix --experimental-features ''
Comment thread
tomberek marked this conversation as resolved.
Outdated

testReplResponse '
drvPath
' '".*-simple.drv"' \
--file $testDir/simple.nix --experimental-features ''

testReplResponse '
drvPath
' '".*-simple.drv"' \
--file $testDir/simple.nix --experimental-features 'repl-flake'
Comment thread
tomberek marked this conversation as resolved.
Outdated

mkdir -p flake && cat <<EOF > flake/flake.nix
{
outputs = { self }: {
foo = 1;
bar.baz = 2;

changingThing = "beforeChange";
};
}
EOF
testReplResponse '
foo + baz
' "3" \
./flake ./flake\#bar --experimental-features 'flakes repl-flake'

# Test the `:reload` mechansim with flakes:
# - Eval `./flake#changingThing`
# - Modify the flake
# - Re-eval it
# - Check that the result has changed
replResult=$( (
echo "changingThing"
sleep 1 # Leave the repl the time to eval 'foo'
sed -i 's/beforeChange/afterChange/' flake/flake.nix
echo ":reload"
echo "changingThing"
) | nix repl ./flake --experimental-features 'flakes repl-flake')
echo "$replResult" | grep -qs beforeChange
echo "$replResult" | grep -qs afterChange