Skip to content

Commit

Permalink
Make clear that StorePathWithOutputs is a deprecated type
Browse files Browse the repository at this point in the history
- Add a comment

- Put `OutputsSpec` in a different header (First part of #6815)

- Make a few stray uses of it in new code use `DerivedPath` instead.
  • Loading branch information
Ericson2314 committed Jan 10, 2023
1 parent 1c98daf commit da64f02
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 89 deletions.
4 changes: 2 additions & 2 deletions src/libcmd/installables.hh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include "util.hh"
#include "path.hh"
#include "path-with-outputs.hh"
#include "outputs-spec.hh"
#include "derived-path.hh"
#include "eval.hh"
#include "store-api.hh"
Expand All @@ -20,7 +20,7 @@ namespace eval_cache { class EvalCache; class AttrCursor; }

struct App
{
std::vector<StorePathWithOutputs> context;
std::vector<DerivedPath> context;
Path program;
// FIXME: add args, sandbox settings, metadata, ...
};
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/flake/flakeref.hh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "types.hh"
#include "hash.hh"
#include "fetchers.hh"
#include "path-with-outputs.hh"
#include "outputs-spec.hh"

#include <variant>

Expand Down
1 change: 0 additions & 1 deletion src/libmain/shared.hh
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ void printVersion(const std::string & programName);
void printGCWarning();

class Store;
struct StorePathWithOutputs;

void printMissing(
ref<Store> store,
Expand Down
61 changes: 61 additions & 0 deletions src/libstore/outputs-spec.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#include "outputs-spec.hh"
#include "nlohmann/json.hpp"

#include <regex>

namespace nix {

std::pair<std::string, OutputsSpec> parseOutputsSpec(const std::string & s)
{
static std::regex regex(R"((.*)\^((\*)|([a-z]+(,[a-z]+)*)))");

std::smatch match;
if (!std::regex_match(s, match, regex))
return {s, DefaultOutputs()};

if (match[3].matched)
return {match[1], AllOutputs()};

return {match[1], tokenizeString<OutputNames>(match[4].str(), ",")};
}

std::string printOutputsSpec(const OutputsSpec & outputsSpec)
{
if (std::get_if<DefaultOutputs>(&outputsSpec))
return "";

if (std::get_if<AllOutputs>(&outputsSpec))
return "^*";

if (auto outputNames = std::get_if<OutputNames>(&outputsSpec))
return "^" + concatStringsSep(",", *outputNames);

assert(false);
}

void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec)
{
if (std::get_if<DefaultOutputs>(&outputsSpec))
json = nullptr;

else if (std::get_if<AllOutputs>(&outputsSpec))
json = std::vector<std::string>({"*"});

else if (auto outputNames = std::get_if<OutputNames>(&outputsSpec))
json = *outputNames;
}

void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec)
{
if (json.is_null())
outputsSpec = DefaultOutputs();
else {
auto names = json.get<OutputNames>();
if (names == OutputNames({"*"}))
outputsSpec = AllOutputs();
else
outputsSpec = names;
}
}

}
32 changes: 32 additions & 0 deletions src/libstore/outputs-spec.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#pragma once

#include <variant>

#include "util.hh"

#include "nlohmann/json_fwd.hpp"

namespace nix {

typedef std::set<std::string> OutputNames;

struct AllOutputs {
bool operator < (const AllOutputs & _) const { return false; }
};

struct DefaultOutputs {
bool operator < (const DefaultOutputs & _) const { return false; }
};

typedef std::variant<DefaultOutputs, AllOutputs, OutputNames> OutputsSpec;

/* Parse a string of the form 'prefix^output1,...outputN' or
'prefix^*', returning the prefix and the outputs spec. */
std::pair<std::string, OutputsSpec> parseOutputsSpec(const std::string & s);

std::string printOutputsSpec(const OutputsSpec & outputsSpec);

void to_json(nlohmann::json &, const OutputsSpec &);
void from_json(const nlohmann::json &, OutputsSpec &);

}
54 changes: 0 additions & 54 deletions src/libstore/path-with-outputs.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include "path-with-outputs.hh"
#include "store-api.hh"
#include "nlohmann/json.hpp"

#include <regex>

Expand Down Expand Up @@ -71,57 +70,4 @@ StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std:
return StorePathWithOutputs { store.followLinksToStorePath(path), std::move(outputs) };
}

std::pair<std::string, OutputsSpec> parseOutputsSpec(const std::string & s)
{
static std::regex regex(R"((.*)\^((\*)|([a-z]+(,[a-z]+)*)))");

std::smatch match;
if (!std::regex_match(s, match, regex))
return {s, DefaultOutputs()};

if (match[3].matched)
return {match[1], AllOutputs()};

return {match[1], tokenizeString<OutputNames>(match[4].str(), ",")};
}

std::string printOutputsSpec(const OutputsSpec & outputsSpec)
{
if (std::get_if<DefaultOutputs>(&outputsSpec))
return "";

if (std::get_if<AllOutputs>(&outputsSpec))
return "^*";

if (auto outputNames = std::get_if<OutputNames>(&outputsSpec))
return "^" + concatStringsSep(",", *outputNames);

assert(false);
}

void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec)
{
if (std::get_if<DefaultOutputs>(&outputsSpec))
json = nullptr;

else if (std::get_if<AllOutputs>(&outputsSpec))
json = std::vector<std::string>({"*"});

else if (auto outputNames = std::get_if<OutputNames>(&outputsSpec))
json = *outputNames;
}

void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec)
{
if (json.is_null())
outputsSpec = DefaultOutputs();
else {
auto names = json.get<OutputNames>();
if (names == OutputNames({"*"}))
outputsSpec = AllOutputs();
else
outputsSpec = names;
}
}

}
30 changes: 7 additions & 23 deletions src/libstore/path-with-outputs.hh
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
#pragma once

#include <variant>

#include "path.hh"
#include "derived-path.hh"
#include "nlohmann/json_fwd.hpp"

namespace nix {

/* This is a deprecated old type just for use by the old CLI, and older
versions of the RPC protocols. In new code don't use it; you want
`DerivedPath` instead.
`DerivedPath` is better because it handles more cases, and does so more
explicitly without devious punning tricks.
*/
struct StorePathWithOutputs
{
StorePath path;
Expand All @@ -33,25 +38,4 @@ StorePathWithOutputs parsePathWithOutputs(const Store & store, std::string_view

StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std::string_view pathWithOutputs);

typedef std::set<std::string> OutputNames;

struct AllOutputs {
bool operator < (const AllOutputs & _) const { return false; }
};

struct DefaultOutputs {
bool operator < (const DefaultOutputs & _) const { return false; }
};

typedef std::variant<DefaultOutputs, AllOutputs, OutputNames> OutputsSpec;

/* Parse a string of the form 'prefix^output1,...outputN' or
'prefix^*', returning the prefix and the outputs spec. */
std::pair<std::string, OutputsSpec> parseOutputsSpec(const std::string & s);

std::string printOutputsSpec(const OutputsSpec & outputsSpec);

void to_json(nlohmann::json &, const OutputsSpec &);
void from_json(const nlohmann::json &, OutputsSpec &);

}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "path-with-outputs.hh"
#include "outputs-spec.hh"

#include <gtest/gtest.h>

Expand Down
23 changes: 18 additions & 5 deletions src/nix/app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,19 @@ UnresolvedApp Installable::toApp(EvalState & state)
if (type == "app") {
auto [program, context] = cursor->getAttr("program")->getStringWithContext();

std::vector<StorePathWithOutputs> context2;
for (auto & [path, name] : context)
context2.push_back({path, {name}});
std::vector<DerivedPath> context2;
for (auto & [path, name] : context) {
context2.push_back(name != "" || path.isDerivation()
? (DerivedPath) DerivedPath::Built {
.drvPath = path,
.outputs = name != ""
? StringSet { name }
: StringSet { },
}
: (DerivedPath) DerivedPath::Opaque {
.path = path,
});
}

return UnresolvedApp{App {
.context = std::move(context2),
Expand All @@ -105,7 +115,10 @@ UnresolvedApp Installable::toApp(EvalState & state)
: DrvName(name).name;
auto program = outPath + "/bin/" + mainProgram;
return UnresolvedApp { App {
.context = { { drvPath, {outputName} } },
.context = { DerivedPath::Built {
.drvPath = drvPath,
.outputs = {outputName},
} },
.program = program,
}};
}
Expand All @@ -123,7 +136,7 @@ App UnresolvedApp::resolve(ref<Store> evalStore, ref<Store> store)

for (auto & ctxElt : unresolved.context)
installableContext.push_back(
std::make_shared<InstallableDerivedPath>(store, ctxElt.toDerivedPath()));
std::make_shared<InstallableDerivedPath>(store, ctxElt));

auto builtContext = Installable::build(evalStore, store, Realise::Outputs, installableContext);
res.program = resolveString(*store, unresolved.program, builtContext);
Expand Down
2 changes: 1 addition & 1 deletion src/nix/develop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "common-args.hh"
#include "shared.hh"
#include "store-api.hh"
#include "path-with-outputs.hh"
#include "outputs-spec.hh"
#include "derivations.hh"
#include "progress-bar.hh"
#include "run.hh"
Expand Down
2 changes: 1 addition & 1 deletion src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include "get-drvs.hh"
#include "store-api.hh"
#include "derivations.hh"
#include "path-with-outputs.hh"
#include "outputs-spec.hh"
#include "attr-path.hh"
#include "fetchers.hh"
#include "registry.hh"
Expand Down

0 comments on commit da64f02

Please sign in to comment.