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

Fixes an outrageous number of bugs and warnings #90

Merged
merged 23 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 14 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
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ endif()
# Set a default build type if none was specified
# This is required for Conan 2.0 to work properly
if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
message(STATUS "Setting build type to 'Release' as none was specified.")
message(STATUS "Setting build type to 'Debug' as none was specified.")
set(CMAKE_BUILD_TYPE
Debug
CACHE STRING "Choose the type of build." FORCE)
Expand Down
3 changes: 3 additions & 0 deletions cmake/CompilerWarnings.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ function(
-Wduplicated-branches # warn if if / else branches have duplicated code
-Wlogical-op # warn about logical operations being used where bitwise were probably wanted
-Wuseless-cast # warn if you perform a cast to the same type
-Wno-unused-variable
-Wno-unused-function
-Wno-unused-parameter
)
endif()

Expand Down
16 changes: 16 additions & 0 deletions cmake/StaticAnalyzers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ macro(cloysterhpc_enable_cppcheck WARNINGS_AS_ERRORS CPPCHECK_OPTIONS)
# style should enable the other 3, but we'll be explicit just in case
set(CMAKE_CXX_CPPCHECK
${CPPCHECK}
--quiet
--template=${CPPCHECK_TEMPLATE}
--enable=style,performance,warning,portability
--inline-suppr
Expand All @@ -26,6 +27,21 @@ macro(cloysterhpc_enable_cppcheck WARNINGS_AS_ERRORS CPPCHECK_OPTIONS)
# ignores code that cppcheck thinks is invalid C++
--suppress=syntaxError
--suppress=preprocessorErrorDirective
# FIXME: The below warnings were disabled to underflow the devopment
# they should be enabled again
--suppress=functionStatic
--suppress=functionConst
--suppress=funcArgNamesDifferent
--suppress=unusedPrivateFunction
--suppress=constVariable
--suppress=missingOverride
--suppress=useStlAlgorithm
--suppress=constParameter
--suppress=noExplicitConstructor
--suppress=duplicateBreak
--suppress=variableScope
--suppress=unreadVariable
--suppress=shadowFunction
--inconclusive)
else()
# if the user provides a CPPCHECK_OPTIONS with a template specified, it will override this template
Expand Down
2 changes: 1 addition & 1 deletion conanfile.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
cli11/2.3.2
# libfmt version will be automatically selected by spdlog
fmt/10.2.1
spdlog/[>=1.14.0]
spdlog/[=1.14.0]
boost/1.82.0
magic_enum/0.9.2
gsl-lite/0.40.0
Expand Down
6 changes: 3 additions & 3 deletions include/cloysterhpc/dbus_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ class DBusClient : public MessageBus {
public:
DBusClient(std::string bus, std::string object)
: m_proxy(
sdbus::createProxy(std::move(sdbus::createSystemBusConnection()),
std::move(sdbus::ServiceName { bus }),
std::move(sdbus::ObjectPath { object })))
sdbus::createProxy(std::move(sdbus::createSystemBusConnection()),
std::move(sdbus::ServiceName { bus }),
std::move(sdbus::ObjectPath { object })))
{
}

Expand Down
4 changes: 2 additions & 2 deletions include/cloysterhpc/diskImage.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class DiskImage {
* @param path Filesystem path to the disk image to check.
* @return True if the disk image is known, false otherwise.
*/
bool isKnownImage(const std::filesystem::path& path);
static bool isKnownImage(const std::filesystem::path& path);

/**
* @brief Checks if the given disk image has a verified checksum.
*
Expand Down
8 changes: 8 additions & 0 deletions include/cloysterhpc/functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,14 @@ std::string getCurrentTimestamp();
std::string findAndReplace(const std::string_view& source,
const std::string_view& find, const std::string_view& replace);

/**
* @brief Copies a file, skip copying if it exists
*
* @param source The source file to copy.
* @param destination The path where the source file will be copied.
*/
void copyFile(std::filesystem::path source, std::filesystem::path destination);

} /* namespace cloyster */

#endif // CLOYSTERHPC_FUNCTIONS_H_
6 changes: 3 additions & 3 deletions include/cloysterhpc/messagebus.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* tell this to the user
*/
template <typename T>
concept MessageReturnable = std::is_nothrow_default_constructible_v<
T> && std::is_nothrow_move_constructible_v<T>;
concept MessageReturnable = std::is_nothrow_default_constructible_v<T>
&& std::is_nothrow_move_constructible_v<T>;

/**
* A more or less "generic"-ish way to refer to a message reply
Expand Down Expand Up @@ -107,7 +107,7 @@ class MessageBusMethod {
virtual MessageReply callMethod() = 0;

public:
void addParams()
static void addParams()
{
// end case for the variadic template below
}
Expand Down
3 changes: 2 additions & 1 deletion include/cloysterhpc/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class Node : public Server {
const std::string& getMACAddress() const;
void setMACAddress(const std::string& macAddress);
const std::optional<std::string>& getNodeRootPassword() const;
void setNodeRootPassword(const std::optional<std::string>& ndeRootPassword);
void setNodeRootPassword(
const std::optional<std::string>& nodeRootPassword);
};

#endif // CLOYSTERHPC_NODE_H_
22 changes: 12 additions & 10 deletions include/cloysterhpc/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
#ifndef CLOYSTERHPC_OS_H_
#define CLOYSTERHPC_OS_H_

#include "services/dnf.h"

#include <cloysterhpc/const.h>
#include <cloysterhpc/services/package_manager.h>
#include <gsl/gsl-lite.hpp>
#include <memory>
#include <string>
#include <variant>

/**
* @class OS
Expand Down Expand Up @@ -50,18 +50,18 @@ class OS {
enum class Distro { RHEL, OL, Rocky, AlmaLinux };

private:
Arch m_arch;
Family m_family;
Platform m_platform;
Distro m_distro;
std::variant<std::monostate, Arch> m_arch;
std::variant<std::monostate, Family> m_family;
std::variant<std::monostate, Platform> m_platform;
std::variant<std::monostate, Distro> m_distro;
std::string m_kernel;
unsigned m_majorVersion {};
unsigned m_minorVersion {};
// BUG: The package_manager should be a unique_ptr;
// however repos.h needs to be rewritten to support it.
// 'OS::os()' is implicitly deleted because the default definition
// would be ill-formed
std::shared_ptr<package_manager> m_packageManager {};
std::shared_ptr<package_manager> m_packageManager;

private:
void setMajorVersion(unsigned int majorVersion);
Expand All @@ -74,7 +74,10 @@ class OS {
* @param line The key-value pair string.
* @return The value extracted from the key-value pair string.
*/
std::string getValueFromKey(const std::string& line);
static std::string getValueFromKey(const std::string& line);

std::shared_ptr<package_manager> factoryPackageManager(
OS::Platform platform);

public:
OS();
Expand Down Expand Up @@ -119,8 +122,7 @@ class OS {
[[nodiscard]] unsigned int getMajorVersion() const;
[[nodiscard]] unsigned int getMinorVersion() const;

std::shared_ptr<package_manager> createPackageManager(
OS::Platform platform);
gsl::not_null<package_manager*> packageManager() const;

#ifndef NDEBUG
/**
Expand Down
6 changes: 3 additions & 3 deletions include/cloysterhpc/queuesystem/slurm.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class SLURM : public QueueSystem {
/**
* @brief Installs the SLURM server package on the system.
*/
void installServer();
static void installServer();

/**
* @brief Configures the SLURM server.
Expand All @@ -32,12 +32,12 @@ class SLURM : public QueueSystem {
/**
* @brief Enables the SLURM server to start at boot.
*/
void enableServer();
static void enableServer();

/**
* @brief Starts the SLURM server.
*/
void startServer();
static void startServer();
};

#endif // CLOYSTERHPC_SLURM_H_
5 changes: 3 additions & 2 deletions include/cloysterhpc/repos.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class RepoManager {
void loadFiles(const std::filesystem::path& basedir = "/etc/yum.repos.d");
void loadCustom(inifile& file, const std::filesystem::path& path);

// BUG: Enable and EnableMultiple are the same method. Overload it.
void enable(const std::string& id);
void enableMultiple(std::vector<std::string> ids);
void disable(const std::string& id);
Expand All @@ -65,11 +66,11 @@ class RepoManager {
private:
std::vector<repository> m_repos;
BaseRunner& m_runner;
OS m_os;
const OS& m_os;

void createFileFor(std::filesystem::path path);

std::vector<repository> buildCloysterTree(
const std::vector<repository> buildCloysterTree(
const std::filesystem::path& basedir);

void loadSingleFile(std::filesystem::path source);
Expand Down
2 changes: 1 addition & 1 deletion include/cloysterhpc/repos/el9/cloyster.repo
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ gpgkey=https://yum.oracle.com/RPM-GPG-KEY-oracle-ol9

[{0}-Rocky-BaseOS]
name=Rocky Linux $releasever - BaseOS
baseurl=http://dl.rockylinux.org/$contentdir/$releasever/BaseOS/$basearch/os/
baseurl=http://dl.rockylinux.org/pub/rocky/$releasever/BaseOS/$basearch/os/
enabled=0
gpgcheck=1
gpgkey=https://mirror.versatushpc.com.br/rocky/linux/RPM-GPG-KEY-Rocky-9
Expand Down
21 changes: 10 additions & 11 deletions include/cloysterhpc/services/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include <cloysterhpc/cluster.h>
#include <cloysterhpc/services/execution.h>
#include <cloysterhpc/services/provisioner.h>

/**
* @class Shell
Expand Down Expand Up @@ -71,9 +70,9 @@ class Shell : public Execution {
*
* This function disables the DNS override feature of NetworkManager.
*/
void disableNetworkManagerDNSOverride(); // This should be on Network
static void disableNetworkManagerDNSOverride(); // This should be on Network

void deleteConnectionIfExists(std::string_view connectionName);
static void deleteConnectionIfExists(std::string_view connectionName);

/**
* @brief Configures network connections.
Expand All @@ -83,7 +82,7 @@ class Shell : public Execution {
*
* @param connections A list of network connections to configure.
*/
void configureNetworks(const std::list<Connection>&);
static void configureNetworks(const std::list<Connection>&);

/**
* @brief Runs a system update.
Expand All @@ -98,22 +97,22 @@ class Shell : public Execution {
* This function installs all the necessary packages for the system to
* operate.
*/
void installRequiredPackages();
static void installRequiredPackages();

/**
* @brief Disallows SSH root password login.
*
* This function configures SSH to disallow root login using password
* authentication.
*/
void disallowSSHRootPasswordLogin();
static void disallowSSHRootPasswordLogin();

/**
* @brief Installs the OpenHPC base packages.
*
* This function installs the base packages required for OpenHPC.
*/
void installOpenHPCBase();
static void installOpenHPCBase();

/**
* @brief Configure repositories
Expand All @@ -131,7 +130,7 @@ class Shell : public Execution {
* @param connections A list of network connections to use for time service
* configuration.
*/
void configureTimeService(const std::list<Connection>&);
static void configureTimeService(const std::list<Connection>&);

/**
* @brief Configures the queue system.
Expand All @@ -153,21 +152,21 @@ class Shell : public Execution {
*
* This function removes any limits on memory locking.
*/
void removeMemlockLimits();
static void removeMemlockLimits();
/**
* @brief Installs development components.
*
* This function installs the necessary development tools and libraries.
*/
void installDevelopmentComponents();
static void installDevelopmentComponents();

/* Ancillary functions */
/**
* @brief Disables SELinux.
*
* This function completely disables SELinux enforcement.
*/
void disableSELinux();
static void disableSELinux();

public:
// FIXME: Guideline: Don’t use a const unique_ptr& as a parameter;
Expand Down
Loading
Loading