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.

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

Automatically format with clang-tools #6721

Closed
wants to merge 5 commits into from
Closed
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
BasedOnStyle: LLVM
IndentWidth: 4
BreakBeforeBraces: Custom
BraceWrapping:
AfterStruct: true
AfterClass: true
AfterFunction: true
AfterUnion: true
SplitEmptyRecord: false
PointerAlignment: Middle
FixNamespaceComments: false
SortIncludes: Never
#IndentPPDirectives: BeforeHash
SpaceAfterCStyleCast: true
SpaceAfterTemplateKeyword: false
AccessModifierOffset: -4
AlignAfterOpenBracket: AlwaysBreak
AlignEscapedNewlines: DontAlign
ColumnLimit: 120
BreakStringLiterals: false
BitFieldColonSpacing: None
AllowShortFunctionsOnASingleLine: Empty
AlwaysBreakTemplateDeclarations: Yes
BinPackParameters: false
BreakConstructorInitializers: BeforeComma
EmptyLineAfterAccessModifier: Leave # change to always/never later?
EmptyLineBeforeAccessModifier: Leave
#PackConstructorInitializers: BinPack
Copy link
Member Author

Choose a reason for hiding this comment

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

This might help reduce the diff some, but the nixpkgs pinned in the flake has clang-format 13 and this option is introduced in 14.

BreakBeforeBinaryOperators: NonAssignment
AlwaysBreakBeforeMultilineStrings: true
1 change: 1 addition & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
TODO: Fill in.
2 changes: 1 addition & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@
outputs = [ "out" "dev" "doc" ];

nativeBuildInputs = nativeBuildDeps;
buildInputs = buildDeps ++ propagatedDeps ++ awsDeps;
buildInputs = buildDeps ++ propagatedDeps ++ awsDeps ++ [ buildPackages.clang-tools ];

inherit configureFlags;

Expand Down
3 changes: 3 additions & 0 deletions maintainers/format.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#!/bin/sh

find . -type f -name '*.cc' -o -name '*.hh' | xargs --max-args=8 --max-procs "$(nproc)" clang-format -i
58 changes: 26 additions & 32 deletions src/build-remote/build-remote.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,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 @@ -40,13 +39,15 @@ static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot)
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri), slot), true);
}

static bool allSupportedLocally(Store & store, const std::set<std::string>& requiredFeatures) {
static bool allSupportedLocally(Store & store, const std::set<std::string> & requiredFeatures)
{
for (auto & feature : requiredFeatures)
if (!store.systemFeatures.get().count(feature)) return false;
if (!store.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(*logger);
Expand Down Expand Up @@ -82,7 +83,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->stateDir } + currentLoadName;
currentLoad = std::string{localStore->stateDir} + currentLoadName;
else
currentLoad = settings.nixStateDir + currentLoadName;

Expand All @@ -104,18 +105,21 @@ 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);
drvPath = store->parseStorePath(readString(source));
auto requiredFeatures = readStrings<std::set<std::string>>(source);

auto canBuildLocally = amWilling
&& ( neededSystem == settings.thisSystem
|| settings.extraPlatforms.get().count(neededSystem) > 0)
&& allSupportedLocally(*store, requiredFeatures);
auto canBuildLocally =
amWilling
&& (neededSystem == settings.thisSystem || settings.extraPlatforms.get().count(neededSystem) > 0)
&& allSupportedLocally(*store, requiredFeatures);

/* Error ignored here, will be caught later */
mkdir(currentLoad.c_str(), 0777);
Expand All @@ -134,12 +138,9 @@ static int main_build_remote(int argc, char * * argv)

if (m.enabled
&& (neededSystem == "builtin"
|| std::find(m.systemTypes.begin(),
m.systemTypes.end(),
neededSystem) != m.systemTypes.end()) &&
m.allSupported(requiredFeatures) &&
m.mandatoryMet(requiredFeatures))
{
|| std::find(m.systemTypes.begin(), m.systemTypes.end(), neededSystem)
!= m.systemTypes.end())
&& m.allSupported(requiredFeatures) && m.mandatoryMet(requiredFeatures)) {
rightType = true;
AutoCloseFD free;
uint64_t load = 0;
Expand Down Expand Up @@ -181,8 +182,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 @@ -201,16 +201,11 @@ static int main_build_remote(int argc, char * * argv)
drvstr = "<unknown>";

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

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

Expand Down Expand Up @@ -239,9 +234,8 @@ static int main_build_remote(int argc, char * * argv)

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

connected:
connected:
close(5);

std::cerr << "# accept\n" << storeUri << "\n";
Expand Down Expand Up @@ -303,7 +297,7 @@ static int main_build_remote(int argc, char * * argv)
if (settings.isExperimentalFeatureEnabled(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(result.builtOutputs.count(thisOutputId));
Expand Down
91 changes: 44 additions & 47 deletions src/libcmd/command.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include <nlohmann/json.hpp>

extern char * * environ __attribute__((weak));
extern char ** environ __attribute__((weak));

namespace nix {

Expand All @@ -20,7 +20,8 @@ nix::Commands RegisterCommand::getCommandsFor(const std::vector<std::string> & p
if (name.size() == prefix.size() + 1) {
bool equal = true;
for (size_t i = 0; i < prefix.size(); ++i)
if (name[i] != prefix[i]) equal = false;
if (name[i] != prefix[i])
equal = false;
if (equal)
res.insert_or_assign(name[prefix.size()], command);
}
Expand All @@ -33,9 +34,7 @@ nlohmann::json NixMultiCommand::toJSON()
return MultiCommand::toJSON();
}

StoreCommand::StoreCommand()
{
}
StoreCommand::StoreCommand() {}

ref<Store> StoreCommand::getStore()
{
Expand Down Expand Up @@ -110,17 +109,15 @@ ref<EvalState> EvalCommand::getEvalState()
{
if (!evalState) {
evalState =
#if HAVE_BOEHMGC
std::allocate_shared<EvalState>(traceable_allocator<EvalState>(),
searchPath, getEvalStore(), getStore())
#else
std::make_shared<EvalState>(
searchPath, getEvalStore(), getStore())
#endif
#if HAVE_BOEHMGC
std::allocate_shared<EvalState>(traceable_allocator<EvalState>(), searchPath, getEvalStore(), getStore())
#else
std::make_shared<EvalState>(searchPath, getEvalStore(), getStore())
#endif
;

if (startReplOnEvalErrors) {
evalState->debugRepl = &runRepl;
evalState->debugRepl = &runRepl;
};
}
return ref<EvalState>(evalState);
Expand Down Expand Up @@ -183,8 +180,7 @@ void BuiltPathsCommand::run(ref<Store> store)

StorePathsCommand::StorePathsCommand(bool recursive)
: BuiltPathsCommand(recursive)
{
}
{}

void StorePathsCommand::run(ref<Store> store, BuiltPaths && paths)
{
Expand All @@ -211,60 +207,58 @@ Strings editorFor(const Path & file, uint32_t line)
{
auto editor = getEnv("EDITOR").value_or("cat");
auto args = tokenizeString<Strings>(editor);
if (line > 0 && (
editor.find("emacs") != std::string::npos ||
editor.find("nano") != std::string::npos ||
editor.find("vim") != std::string::npos ||
editor.find("kak") != std::string::npos))
if (line > 0
&& (editor.find("emacs") != std::string::npos || editor.find("nano") != std::string::npos
|| editor.find("vim") != std::string::npos || editor.find("kak") != std::string::npos))
args.push_back(fmt("+%d", line));
args.push_back(file);
return args;
}

MixProfile::MixProfile()
{
addFlag({
.longName = "profile",
.description = "The profile to update.",
.labels = {"path"},
.handler = {&profile},
.completer = completePath
});
addFlag(
{.longName = "profile",
Copy link
Member

Choose a reason for hiding this comment

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

The change to the formatting of designated initializers causes a huge diff, but I don't see a way to keep the old style...

Copy link
Contributor

Choose a reason for hiding this comment

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

.description = "The profile to update.",
.labels = {"path"},
.handler = {&profile},
.completer = completePath});
}

void MixProfile::updateProfile(const StorePath & storePath)
{
if (!profile) return;
if (!profile)
return;
auto store = getStore().dynamic_pointer_cast<LocalFSStore>();
if (!store) throw Error("'--profile' is not supported for this Nix store");
if (!store)
throw Error("'--profile' is not supported for this Nix store");
auto profile2 = absPath(*profile);
switchLink(profile2,
createGeneration(
ref<LocalFSStore>(store),
profile2, storePath));
switchLink(profile2, createGeneration(ref<LocalFSStore>(store), profile2, storePath));
}

void MixProfile::updateProfile(const BuiltPaths & buildables)
{
if (!profile) return;
if (!profile)
return;

std::vector<StorePath> result;

for (auto & buildable : buildables) {
std::visit(overloaded {
[&](const BuiltPath::Opaque & bo) {
result.push_back(bo.path);
std::visit(
overloaded{
[&](const BuiltPath::Opaque & bo) { result.push_back(bo.path); },
[&](const BuiltPath::Built & bfd) {
for (auto & output : bfd.outputs) {
result.push_back(output.second);
}
},
},
[&](const BuiltPath::Built & bfd) {
for (auto & output : bfd.outputs) {
result.push_back(output.second);
}
},
}, buildable.raw());
buildable.raw());
}

if (result.size() != 1)
throw UsageError("'--profile' requires that the arguments produce a single store path, but there are %d", result.size());
throw UsageError(
"'--profile' requires that the arguments produce a single store path, but there are %d", result.size());

updateProfile(result[0]);
}
Expand All @@ -274,7 +268,8 @@ MixDefaultProfile::MixDefaultProfile()
profile = getDefaultProfile();
}

MixEnvironment::MixEnvironment() : ignoreEnvironment(false)
MixEnvironment::MixEnvironment()
: ignoreEnvironment(false)
{
addFlag({
.longName = "ignore-environment",
Expand All @@ -300,14 +295,16 @@ MixEnvironment::MixEnvironment() : ignoreEnvironment(false)
});
}

void MixEnvironment::setEnviron() {
void MixEnvironment::setEnviron()
{
if (ignoreEnvironment) {
if (!unset.empty())
throw UsageError("--unset does not make sense with --ignore-environment");

for (const auto & var : keep) {
auto val = getenv(var.c_str());
if (val) stringsEnv.emplace_back(fmt("%s=%s", var.c_str(), val));
if (val)
stringsEnv.emplace_back(fmt("%s=%s", var.c_str(), val));
}

vectorEnv = stringsToCharPtrs(stringsEnv);
Expand Down
Loading