Skip to content

Rip off the bandaid: Format the codebase with clang-format #13108

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.

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 2 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# bulk initial re-formatting with clang-format
9535f8bdc34e8e62d53608a027bc95fd0f07e55f # !autorebase ./maintainers/format.sh --until-stable
463 changes: 0 additions & 463 deletions maintainers/flake-module.nix

Large diffs are not rendered by default.

11 changes: 8 additions & 3 deletions maintainers/format.sh
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
#!/usr/bin/env bash

if ! type -p pre-commit &>/dev/null; then
echo "format.sh: pre-commit not found. Please use \`nix develop\`.";
echo "format.sh: pre-commit not found. Please use \`nix develop -c ./maintainers/format.sh\`.";
exit 1;
fi;
if test -z "$_NIX_PRE_COMMIT_HOOKS_CONFIG"; then
echo "format.sh: _NIX_PRE_COMMIT_HOOKS_CONFIG not set. Please use \`nix develop\`.";
echo "format.sh: _NIX_PRE_COMMIT_HOOKS_CONFIG not set. Please use \`nix develop -c ./maintainers/format.sh\`.";
exit 1;
fi;
pre-commit run --config "$_NIX_PRE_COMMIT_HOOKS_CONFIG" --all-files

while ! pre-commit run --config "$_NIX_PRE_COMMIT_HOOKS_CONFIG" --all-files; do
if [ "${1:-}" != "--until-stable" ]; then
exit 1
fi
done
1 change: 1 addition & 0 deletions packaging/dev-shell.nix
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ pkgs.nixComponents2.nix-util.overrideAttrs (
) pkgs.buildPackages.mesonEmulatorHook
++ [
pkgs.buildPackages.cmake
pkgs.buildPackages.gnused
pkgs.buildPackages.shellcheck
pkgs.buildPackages.changelog-d
modular.pre-commit.settings.package
Expand Down
18 changes: 9 additions & 9 deletions precompiled-headers.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@
#include <unistd.h>

#ifndef _WIN32
# include <grp.h>
# include <netdb.h>
# include <pwd.h>
# include <sys/resource.h>
# include <sys/select.h>
# include <sys/socket.h>
# include <sys/utsname.h>
# include <sys/wait.h>
# include <termios.h>
# include <grp.h>
# include <netdb.h>
# include <pwd.h>
# include <sys/resource.h>
# include <sys/select.h>
# include <sys/socket.h>
# include <sys/utsname.h>
# include <sys/wait.h>
# include <termios.h>
#endif

#include <nlohmann/json.hpp>
80 changes: 36 additions & 44 deletions src/build-remote/build-remote.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <tuple>
#include <iomanip>
#ifdef __APPLE__
#include <sys/time.h>
# include <sys/time.h>
#endif

#include "nix/store/machines.hh"
Expand All @@ -26,8 +26,7 @@
using namespace nix;
using std::cin;

static void handleAlarm(int sig) {
}
static void handleAlarm(int sig) {}

std::string escapeUri(std::string uri)
{
Expand All @@ -42,13 +41,15 @@ static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot)
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri.render()), slot), true);
}

static bool allSupportedLocally(Store & store, const StringSet& requiredFeatures) {
static bool allSupportedLocally(Store & store, const StringSet & requiredFeatures)
{
for (auto & feature : requiredFeatures)
if (!store.config.systemFeatures.get().count(feature)) return false;
if (!store.config.systemFeatures.get().count(feature))
return false;
return true;
}

static int main_build_remote(int argc, char * * argv)
static int main_build_remote(int argc, char ** argv)
{
{
logger = makeJSONLogger(getStandardError());
Expand Down Expand Up @@ -85,7 +86,7 @@ static int main_build_remote(int argc, char * * argv)
that gets cleared on reboot, but it wouldn't work on macOS. */
auto currentLoadName = "/current-load";
if (auto localStore = store.dynamic_pointer_cast<LocalFSStore>())
currentLoad = std::string { localStore->config.stateDir } + currentLoadName;
currentLoad = std::string{localStore->config.stateDir} + currentLoadName;
else
currentLoad = settings.nixStateDir + currentLoadName;

Expand All @@ -107,8 +108,11 @@ static int main_build_remote(int argc, char * * argv)

try {
auto s = readString(source);
if (s != "try") return 0;
} catch (EndOfFile &) { return 0; }
if (s != "try")
return 0;
} catch (EndOfFile &) {
return 0;
}

auto amWilling = readInt(source);
auto neededSystem = readString(source);
Expand All @@ -117,10 +121,10 @@ static int main_build_remote(int argc, char * * argv)

/* It would be possible to build locally after some builds clear out,
so don't show the warning now: */
bool couldBuildLocally = maxBuildJobs > 0
&& ( neededSystem == settings.thisSystem
|| settings.extraPlatforms.get().count(neededSystem) > 0)
&& allSupportedLocally(*store, requiredFeatures);
bool couldBuildLocally =
maxBuildJobs > 0
&& (neededSystem == settings.thisSystem || settings.extraPlatforms.get().count(neededSystem) > 0)
&& allSupportedLocally(*store, requiredFeatures);
/* It's possible to build this locally right now: */
bool canBuildLocally = amWilling && couldBuildLocally;

Expand All @@ -139,11 +143,8 @@ static int main_build_remote(int argc, char * * argv)
for (auto & m : machines) {
debug("considering building on remote machine '%s'", m.storeUri.render());

if (m.enabled &&
m.systemSupported(neededSystem) &&
m.allSupported(requiredFeatures) &&
m.mandatoryMet(requiredFeatures))
{
if (m.enabled && m.systemSupported(neededSystem) && m.allSupported(requiredFeatures)
&& m.mandatoryMet(requiredFeatures)) {
rightType = true;
AutoCloseFD free;
uint64_t load = 0;
Expand Down Expand Up @@ -185,8 +186,7 @@ static int main_build_remote(int argc, char * * argv)
if (!bestSlotLock) {
if (rightType && !canBuildLocally)
std::cerr << "# postpone\n";
else
{
else {
// build the hint template.
std::string errorText =
"Failed to find a machine for remote build!\n"
Expand All @@ -205,16 +205,11 @@ static int main_build_remote(int argc, char * * argv)
drvstr = "<unknown>";

auto error = HintFmt::fromFormatString(errorText);
error
% drvstr
% neededSystem
% concatStringsSep<StringSet>(", ", requiredFeatures)
error % drvstr % neededSystem % concatStringsSep<StringSet>(", ", requiredFeatures)
% machines.size();

for (auto & m : machines)
error
% concatStringsSep<StringSet>(", ", m.systemTypes)
% m.maxJobs
error % concatStringsSep<StringSet>(", ", m.systemTypes) % m.maxJobs
% concatStringsSep<StringSet>(", ", m.supportedFeatures)
% concatStringsSep<StringSet>(", ", m.mandatoryFeatures);

Expand Down Expand Up @@ -242,9 +237,7 @@ static int main_build_remote(int argc, char * * argv)
sshStore->connect();
} catch (std::exception & e) {
auto msg = chomp(drainFD(5, false));
printError("cannot build on '%s': %s%s",
storeUri, e.what(),
msg.empty() ? "" : ": " + msg);
printError("cannot build on '%s': %s%s", storeUri, e.what(), msg.empty() ? "" : ": " + msg);
bestMachine->enabled = false;
continue;
}
Expand All @@ -253,7 +246,7 @@ static int main_build_remote(int argc, char * * argv)
}
}

connected:
connected:
close(5);

assert(sshStore);
Expand All @@ -265,13 +258,14 @@ static int main_build_remote(int argc, char * * argv)

AutoCloseFD uploadLock;
{
auto setUpdateLock = [&](auto && fileName){
auto setUpdateLock = [&](auto && fileName) {
uploadLock = openLockFile(currentLoad + "/" + escapeUri(fileName) + ".upload-lock", true);
};
try {
setUpdateLock(storeUri);
} catch (SysError & e) {
if (e.errNo != ENAMETOOLONG) throw;
if (e.errNo != ENAMETOOLONG)
throw;
// Try again hashing the store URL so we have a shorter path
auto h = hashString(HashAlgorithm::MD5, storeUri);
setUpdateLock(h.to_string(HashFormat::Base64, false));
Expand Down Expand Up @@ -315,7 +309,7 @@ static int main_build_remote(int argc, char * * argv)
//
// This condition mirrors that: that code enforces the "rules" outlined there;
// we do the best we can given those "rules".
if (trustedOrLegacy || drv.type().isCA()) {
if (trustedOrLegacy || drv.type().isCA()) {
// Hijack the inputs paths of the derivation to include all
// the paths that come from the `inputDrvs` set. We don’t do
// that for the derivations whose `inputDrvs` is empty
Expand All @@ -330,28 +324,26 @@ static int main_build_remote(int argc, char * * argv)
optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv);
auto & result = *optResult;
if (!result.success())
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg);
throw Error(
"build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg);
} else {
copyClosure(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute);
auto res = sshStore->buildPathsWithResults({
DerivedPath::Built {
.drvPath = makeConstantStorePathRef(*drvPath),
.outputs = OutputsSpec::All {},
}
});
copyClosure(*store, *sshStore, StorePathSet{*drvPath}, NoRepair, NoCheckSigs, substitute);
auto res = sshStore->buildPathsWithResults({DerivedPath::Built{
.drvPath = makeConstantStorePathRef(*drvPath),
.outputs = OutputsSpec::All{},
}});
// One path to build should produce exactly one build result
assert(res.size() == 1);
optResult = std::move(res[0]);
}


auto outputHashes = staticOutputHashes(*store, drv);
std::set<Realisation> missingRealisations;
StorePathSet missingPaths;
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().hasKnownOutputPaths()) {
for (auto & outputName : wantedOutputs) {
auto thisOutputHash = outputHashes.at(outputName);
auto thisOutputId = DrvOutput{ thisOutputHash, outputName };
auto thisOutputId = DrvOutput{thisOutputHash, outputName};
if (!store->queryRealisation(thisOutputId)) {
debug("missing output %s", outputName);
assert(optResult);
Expand Down
62 changes: 21 additions & 41 deletions src/libcmd/built-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,22 @@
namespace nix {

// Custom implementation to avoid `ref` ptr equality
GENERATE_CMP_EXT(
,
std::strong_ordering,
SingleBuiltPathBuilt,
*me->drvPath,
me->output);
GENERATE_CMP_EXT(, std::strong_ordering, SingleBuiltPathBuilt, *me->drvPath, me->output);

// Custom implementation to avoid `ref` ptr equality

// TODO no `GENERATE_CMP_EXT` because no `std::set::operator<=>` on
// Darwin, per header.
GENERATE_EQUAL(
,
BuiltPathBuilt ::,
BuiltPathBuilt,
*me->drvPath,
me->outputs);
GENERATE_EQUAL(, BuiltPathBuilt ::, BuiltPathBuilt, *me->drvPath, me->outputs);

StorePath SingleBuiltPath::outPath() const
{
return std::visit(
overloaded{
[](const SingleBuiltPath::Opaque & p) { return p.path; },
[](const SingleBuiltPath::Built & b) { return b.output.second; },
}, raw()
);
},
raw());
}

StorePathSet BuiltPath::outPaths() const
Expand All @@ -49,13 +39,13 @@ StorePathSet BuiltPath::outPaths() const
res.insert(path);
return res;
},
}, raw()
);
},
raw());
}

SingleDerivedPath::Built SingleBuiltPath::Built::discardOutputPath() const
{
return SingleDerivedPath::Built {
return SingleDerivedPath::Built{
.drvPath = make_ref<SingleDerivedPath>(drvPath->discardOutputPath()),
.output = output.first,
};
Expand All @@ -65,14 +55,10 @@ SingleDerivedPath SingleBuiltPath::discardOutputPath() const
{
return std::visit(
overloaded{
[](const SingleBuiltPath::Opaque & p) -> SingleDerivedPath {
return p;
},
[](const SingleBuiltPath::Built & b) -> SingleDerivedPath {
return b.discardOutputPath();
},
}, raw()
);
[](const SingleBuiltPath::Opaque & p) -> SingleDerivedPath { return p; },
[](const SingleBuiltPath::Built & b) -> SingleDerivedPath { return b.discardOutputPath(); },
},
raw());
}

nlohmann::json BuiltPath::Built::toJSON(const StoreDirConfig & store) const
Expand All @@ -97,16 +83,12 @@ nlohmann::json SingleBuiltPath::Built::toJSON(const StoreDirConfig & store) cons

nlohmann::json SingleBuiltPath::toJSON(const StoreDirConfig & store) const
{
return std::visit([&](const auto & buildable) {
return buildable.toJSON(store);
}, raw());
return std::visit([&](const auto & buildable) { return buildable.toJSON(store); }, raw());
}

nlohmann::json BuiltPath::toJSON(const StoreDirConfig & store) const
{
return std::visit([&](const auto & buildable) {
return buildable.toJSON(store);
}, raw());
return std::visit([&](const auto & buildable) { return buildable.toJSON(store); }, raw());
}

RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const
Expand All @@ -116,20 +98,18 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const
overloaded{
[&](const BuiltPath::Opaque & p) { res.insert(p.path); },
[&](const BuiltPath::Built & p) {
auto drvHashes =
staticOutputHashes(store, store.readDerivation(p.drvPath->outPath()));
for (auto& [outputName, outputPath] : p.outputs) {
if (experimentalFeatureSettings.isEnabled(
Xp::CaDerivations)) {
auto drvHashes = staticOutputHashes(store, store.readDerivation(p.drvPath->outPath()));
for (auto & [outputName, outputPath] : p.outputs) {
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
auto drvOutput = get(drvHashes, outputName);
if (!drvOutput)
throw Error(
"the derivation '%s' has unrealised output '%s' (derived-path.cc/toRealisedPaths)",
store.printStorePath(p.drvPath->outPath()), outputName);
auto thisRealisation = store.queryRealisation(
DrvOutput{*drvOutput, outputName});
assert(thisRealisation); // We’ve built it, so we must
// have the realisation
store.printStorePath(p.drvPath->outPath()),
outputName);
auto thisRealisation = store.queryRealisation(DrvOutput{*drvOutput, outputName});
assert(thisRealisation); // We’ve built it, so we must
// have the realisation
res.insert(*thisRealisation);
} else {
res.insert(outputPath);
Expand Down
Loading
Loading