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.

Sign up for GitHub

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

Jump to bottom

A different take on git submodule support for flakes #5497

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

A different take on git submodule support for flakes #5497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
03e71c5
implement passing submodules information when tree is not dirty
Nov 4, 2021
4d6b738
parse the cat-file output to find the submodules
cleverca22 Oct 16, 2021
73bbc24
serialize submodule info into the cache
Oct 19, 2021
d54d8ec
Fix fetchGit.sh tests
Oct 20, 2021
2c9ec9d
Get flake.sh tests to pass
Oct 20, 2021
1ca14d5
Fix wrong path
Oct 20, 2021
f5fa4b0
Fix dirty tree scenario
Oct 20, 2021
28d6715
Explicitly add file:// when bare path is given
Oct 21, 2021
2ff9b20
Add test to demonstrate
Nov 4, 2021
b2bf6cc
Add tests for unclean submodule state
Nov 16, 2021
594a51e
Factor out some logic I might need
Nov 16, 2021
f55b99f
Implement support for unclean submodule scenario
Nov 16, 2021
ce081f0
printTalkative spam
Nov 30, 2021
124e703
Fix wrong repo path
Nov 30, 2021
50d13e7
Support specifying `branch` in `.gitmodules`
Nov 30, 2021
551cb98
Test recomposing source with submodules
Nov 30, 2021
c2cde79
Add the submodule info to flake fingerprint
Dec 6, 2021
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
3 changes: 2 additions & 1 deletion src/libexpr/flake/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,9 +727,10 @@ Fingerprint LockedFlake::getFingerprint() const
// and we haven't changed it, then it's sufficient to use
// flake.sourceInfo.storePath for the fingerprint.
return hashString(htSHA256,
fmt("%s;%s;%d;%d;%s",
fmt("%s;%s;%s;%d;%d;%s",
flake.sourceInfo->storePath.to_string(),
flake.lockedRef.subdir,
flake.lockedRef.input.getModules().value_or(""),
flake.lockedRef.input.getRevCount().value_or(0),
flake.lockedRef.input.getLastModified().value_or(0),
lockFile));
Expand Down
20 changes: 17 additions & 3 deletions src/libexpr/primops/fetchTree.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <iomanip>
#include <regex>

#include <nlohmann/json.hpp>

namespace nix {

void emitTreeAttrs(
Expand All @@ -20,8 +22,7 @@ void emitTreeAttrs(
bool forceDirty)
{
assert(input.isImmutable());

state.mkAttrs(v, 8);
state.mkAttrs(v, 9);

auto storePath = state.store->printStorePath(tree.storePath);

Expand All @@ -34,9 +35,22 @@ void emitTreeAttrs(
mkString(*state.allocAttr(v, state.symbols.create("narHash")),
narHash->to_string(SRI, true));

if (input.getType() == "git")
if (input.getType() == "git") {
Value *modules = state.allocAttr(v, state.symbols.create("modules"));

auto modulesJson = fetchers::getStrAttr(input.attrs, "modules");
auto modulesInfo = fetchers::jsonToAttrs(nlohmann::json::parse(modulesJson));

state.mkAttrs(*modules, modulesInfo.size());
for (auto & [path, url] : modulesInfo) {
Value *vUrl = state.allocValue();
mkString(*vUrl, std::get<string>(url).c_str());
modules->attrs->push_back(Attr(state.symbols.create(path), vUrl));
}
modules->attrs->sort();
mkBool(*state.allocAttr(v, state.symbols.create("submodules")),
fetchers::maybeGetBoolAttr(input.attrs, "submodules").value_or(false));
}

if (!forceDirty) {

Expand Down
7 changes: 7 additions & 0 deletions src/libfetchers/fetchers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,13 @@ std::optional<time_t> Input::getLastModified() const
return {};
}

std::optional<std::string> Input::getModules() const
{
if (auto s = maybeGetStrAttr(attrs, "modules"))
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that this works... Isn't modules an attrset? Doesn't maybeGetStrAttr throw if the attribute isn't a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already stored as JSON. fetchTree reconstructs it into an attrset here: https://github.com/NixOS/nix/pull/5497/files#diff-e473b01a82cc31e51baa89b26b4fe5083e56df27cd834b9722789841ef9d5620R38-R50

not the prettiest thing in the world but it works 🤷‍♀️

Copy link
Member

Choose a reason for hiding this comment

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

😱

Does the process producing the JSON sort keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, you're right. we only sort in fetchTree 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... looks like nlohmann takes care of it, unless I'm missing something: https://json.nlohmann.me/features/object_order/

return *s;
return {};
}

ParsedURL InputScheme::toURL(const Input & input)
{
throw Error("don't know how to convert input '%s' to a URL", attrsToJSON(input.attrs));
Expand Down
1 change: 1 addition & 0 deletions src/libfetchers/fetchers.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public:
std::optional<Hash> getRev() const;
std::optional<uint64_t> getRevCount() const;
std::optional<time_t> getLastModified() const;
std::optional<std::string> getModules() const;
};


Expand Down
Loading