Skip to content

Commit

Permalink
Merge pull request #583 from LibrePCB/add-cli-strict-mode
Browse files Browse the repository at this point in the history 
CLI: Add "--strict" option
(cherry picked from commit ffa210c)
  • Loading branch information
@ubruhin
ubruhin committed Nov 25, 2019
1 parent 51141a1 commit 143c6ea
Show file tree
Hide file tree
Showing 7 changed files with 300 additions and 60 deletions.
138 changes: 95 additions & 43 deletions apps/librepcb-cli/commandlineinterface.cpp
View file
Expand Up @@ -131,6 +131,10 @@ int CommandLineInterface::execute() noexcept {
QCommandLineOption saveOption(
"save",
tr("Save project before closing it (useful to upgrade file format)."));
QCommandLineOption prjStrictOption(
"strict", tr("Fail if the project files are not strictly canonical, i.e. "
"there would be changes when saving the project. Note that "
"this option is not available for *.lppz files."));

// Define options for "open-library"
QCommandLineOption libAllOption(
Expand All @@ -139,6 +143,9 @@ int CommandLineInterface::execute() noexcept {
QCommandLineOption libSaveOption(
"save", tr("Save library (and contained elements if '--all' is given) "
"before closing them (useful to upgrade file format)."));
QCommandLineOption libStrictOption(
"strict", tr("Fail if the opened files are not strictly canonical, i.e. "
"there would be changes when saving the library elements."));

// First parse to get the supplied command (ignoring errors because the parser
// does not yet know the command-dependent options).
Expand Down Expand Up @@ -166,6 +173,7 @@ int CommandLineInterface::execute() noexcept {
parser.addOption(pcbFabricationSettingsOption);
parser.addOption(boardOption);
parser.addOption(saveOption);
parser.addOption(prjStrictOption);
} else if (command == "open-library") {
parser.clearPositionalArguments();
parser.addPositionalArgument(command, commands[command].first,
Expand All @@ -174,6 +182,7 @@ int CommandLineInterface::execute() noexcept {
tr("Path to library directory (*.lplib)."));
parser.addOption(libAllOption);
parser.addOption(libSaveOption);
parser.addOption(libStrictOption);
} else if (!command.isEmpty()) {
printErr(QString(tr("Unknown command '%1'.")).arg(command), 2);
print(parser.helpText(), 0);
Expand Down Expand Up @@ -234,17 +243,19 @@ int CommandLineInterface::execute() noexcept {
parser.isSet(exportPcbFabricationDataOption), // export PCB fab. data
parser.value(pcbFabricationSettingsOption), // PCB fab. settings
parser.values(boardOption), // boards
parser.isSet(saveOption) // save project
parser.isSet(saveOption), // save project
parser.isSet(prjStrictOption) // strict mode
);
} else if (command == "open-library") {
if (positionalArgs.count() != 1) {
printErr(tr("Wrong argument count."), 2);
print(parser.helpText(), 0);
return 1;
}
cmdSuccess = openLibrary(positionalArgs.value(0), // library directory
parser.isSet(libAllOption), // all elements
parser.isSet(libSaveOption) // save
cmdSuccess = openLibrary(positionalArgs.value(0), // library directory
parser.isSet(libAllOption), // all elements
parser.isSet(libSaveOption), // save
parser.isSet(libStrictOption) // strict mode
);
} else {
printErr(tr("Internal failure."));
Expand All @@ -267,7 +278,7 @@ bool CommandLineInterface::openProject(
const QStringList& exportSchematicsFiles, const QStringList& exportBomFiles,
const QStringList& exportBoardBomFiles, const QString& bomAttributes,
bool exportPcbFabricationData, const QString& pcbFabricationSettingsPath,
const QStringList& boards, bool save) const noexcept {
const QStringList& boards, bool save, bool strict) const noexcept {
try {
bool success = true;
QMap<FilePath, int> writtenFilesCounter;
Expand Down Expand Up @@ -295,6 +306,31 @@ bool CommandLineInterface::openProject(
new TransactionalDirectory(projectFs)),
projectFileName); // can throw

// Check for non-canonical files (strict mode)
if (strict) {
print(tr("Check for non-canonical files..."));
if (projectFp.getSuffix() == "lppz") {
printErr(" " % tr("ERROR: The option '--strict' is not available for "
"*.lppz files!"));
success = false;
} else {
project.save(); // can throw
QStringList paths = projectFs->checkForModifications(); // can throw
// ignore user config files
paths = paths.filter(QRegularExpression("^((?!\\.user\\.lp).)*$"));
// sort file paths to increases readability of console output
std::sort(paths.begin(), paths.end());
foreach (const QString& path, paths) {
printErr(
QString(" - Non-canonical file: %1")
.arg(prettyPath(projectFs->getAbsPath(path), projectFile)));
}
if (paths.count() > 0) {
success = false;
}
}
}

// ERC
if (runErc) {
print(tr("Run ERC..."));
Expand Down Expand Up @@ -501,7 +537,7 @@ bool CommandLineInterface::openProject(
}

bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
bool save) const noexcept {
bool save, bool strict) const noexcept {
try {
bool success = true;

Expand All @@ -513,6 +549,8 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(libFp, save); // can throw
Library lib(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(libFs))); // can throw
processLibraryElement(libDir, *libFs, lib, save, strict,
success); // can throw

// Open all component categories
if (all) {
Expand All @@ -526,11 +564,8 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(fp, save); // can throw
ComponentCategory element(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(fs))); // can throw
if (save) {
qInfo() << QString(tr("Save '%1'...")).arg(prettyPath(fp, libDir));
element.save(); // can throw
fs->save(); // can throw
}
processLibraryElement(libDir, *fs, element, save, strict,
success); // can throw
}
}

Expand All @@ -546,11 +581,8 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(fp, save); // can throw
PackageCategory element(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(fs))); // can throw
if (save) {
qInfo() << QString(tr("Save '%1'...")).arg(prettyPath(fp, libDir));
element.save(); // can throw
fs->save(); // can throw
}
processLibraryElement(libDir, *fs, element, save, strict,
success); // can throw
}
}

Expand All @@ -565,11 +597,8 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(fp, save); // can throw
Symbol element(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(fs))); // can throw
if (save) {
qInfo() << QString(tr("Save '%1'...")).arg(prettyPath(fp, libDir));
element.save(); // can throw
fs->save(); // can throw
}
processLibraryElement(libDir, *fs, element, save, strict,
success); // can throw
}
}

Expand All @@ -584,11 +613,8 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(fp, save); // can throw
Package element(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(fs))); // can throw
if (save) {
qInfo() << QString(tr("Save '%1'...")).arg(prettyPath(fp, libDir));
element.save(); // can throw
fs->save(); // can throw
}
processLibraryElement(libDir, *fs, element, save, strict,
success); // can throw
}
}

Expand All @@ -603,11 +629,8 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(fp, save); // can throw
Component element(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(fs))); // can throw
if (save) {
qInfo() << QString(tr("Save '%1'...")).arg(prettyPath(fp, libDir));
element.save(); // can throw
fs->save(); // can throw
}
processLibraryElement(libDir, *fs, element, save, strict,
success); // can throw
}
}

Expand All @@ -622,28 +645,57 @@ bool CommandLineInterface::openLibrary(const QString& libDir, bool all,
TransactionalFileSystem::open(fp, save); // can throw
Device element(std::unique_ptr<TransactionalDirectory>(
new TransactionalDirectory(fs))); // can throw
if (save) {
qInfo() << QString(tr("Save '%1'...")).arg(prettyPath(fp, libDir));
element.save(); // can throw
fs->save(); // can throw
}
processLibraryElement(libDir, *fs, element, save, strict,
success); // can throw
}
}

// Save library
if (save) {
print(QString(tr("Save library '%1'...")).arg(prettyPath(libFp, libDir)));
lib.save(); // can throw
libFs->save(); // can throw
}

return success;
} catch (const Exception& e) {
printErr(QString(tr("ERROR: %1")).arg(e.getMsg()));
return false;
}
}

void CommandLineInterface::processLibraryElement(const QString& libDir,
TransactionalFileSystem& fs,
LibraryBaseElement& element,
bool save, bool strict,
bool& success) const {
// Save element to transactional file system, if needed
if (strict || save) {
element.save(); // can throw
}

// Check for non-canonical files (strict mode)
if (strict) {
qInfo() << QString(tr("Check '%1' for non-canonical files..."))
.arg(prettyPath(fs.getPath(), libDir));

QStringList paths = fs.checkForModifications(); // can throw
// sort file paths to increases readability of console output
std::sort(paths.begin(), paths.end());
foreach (const QString& path, paths) {
printErr(QString(" - Non-canonical file: %1")
.arg(prettyPath(fs.getAbsPath(path), libDir)));
}
if (paths.count() > 0) {
success = false;
}
}

// Save element to file system, if needed
if (save) {
qInfo()
<< QString(tr("Save '%1'...")).arg(prettyPath(fs.getPath(), libDir));
fs.save(); // can throw
}

// Do not propagate changes in the transactional file system to the
// following checks
fs.discardChanges();
}

QString CommandLineInterface::prettyPath(const FilePath& path,
const QString& style) noexcept {
if (QFileInfo(style).isAbsolute()) {
Expand Down
14 changes: 12 additions & 2 deletions apps/librepcb-cli/commandlineinterface.h
View file
Expand Up @@ -32,6 +32,11 @@ namespace librepcb {

class Application;
class FilePath;
class TransactionalFileSystem;

namespace library {
class LibraryBaseElement;
}

namespace cli {

Expand Down Expand Up @@ -61,8 +66,13 @@ class CommandLineInterface final {
const QStringList& exportBoardBomFiles,
const QString& bomAttributes, bool exportPcbFabricationData,
const QString& pcbFabricationSettingsPath,
const QStringList& boards, bool save) const noexcept;
bool openLibrary(const QString& libDir, bool all, bool save) const noexcept;
const QStringList& boards, bool save, bool strict) const
noexcept;
bool openLibrary(const QString& libDir, bool all, bool save,
bool strict) const noexcept;
void processLibraryElement(const QString& libDir, TransactionalFileSystem& fs,
library::LibraryBaseElement& element, bool save,
bool strict, bool& success) const;
static QString prettyPath(const FilePath& path,
const QString& style) noexcept;
static void print(const QString& str, int newlines = 1) noexcept;
Expand Down
44 changes: 38 additions & 6 deletions libs/librepcb/common/fileio/transactionalfilesystem.cpp
View file
Expand Up @@ -267,6 +267,44 @@ void TransactionalFileSystem::exportToZip(const FilePath& fp) const {
}
}

void TransactionalFileSystem::discardChanges() noexcept {
mModifiedFiles.clear();
mRemovedFiles.clear();
mRemovedDirs.clear();
}

QStringList TransactionalFileSystem::checkForModifications() const {
QStringList modifications;

// removed directories
foreach (const QString& dir, mRemovedDirs) {
FilePath fp = mFilePath.getPathTo(dir);
if (fp.isExistingDir()) {
modifications.append(dir);
}
}

// removed files
foreach (const QString& filepath, mRemovedFiles) {
FilePath fp = mFilePath.getPathTo(filepath);
if (fp.isExistingFile()) {
modifications.append(filepath);
}
}

// new or modified files
foreach (const QString& filepath, mModifiedFiles.keys()) {
FilePath fp = mFilePath.getPathTo(filepath);
QByteArray content = mModifiedFiles.value(filepath);
if ((!fp.isExistingFile()) ||
(FileUtils::readFile(fp) != content)) { // can throw
modifications.append(filepath);
}
}

return modifications;
}

void TransactionalFileSystem::autosave() {
saveDiff("autosave"); // can throw
}
Expand Down Expand Up @@ -449,12 +487,6 @@ void TransactionalFileSystem::removeDiff(const QString& type) {
FileUtils::removeDirRecursively(dir); // can throw
}

void TransactionalFileSystem::discardChanges() noexcept {
mModifiedFiles.clear();
mRemovedFiles.clear();
mRemovedDirs.clear();
}

/*******************************************************************************
* End of File
******************************************************************************/
Expand Down
14 changes: 8 additions & 6 deletions libs/librepcb/common/fileio/transactionalfilesystem.h
View file
Expand Up @@ -80,7 +80,8 @@ class TransactionalFileSystem final : public FileSystem {
virtual ~TransactionalFileSystem() noexcept;

// Getters
bool isWritable() const noexcept { return mIsWritable; }
const FilePath& getPath() const noexcept { return mFilePath; }
bool isWritable() const noexcept { return mIsWritable; }
bool isRestoredFromAutosave() const noexcept { return mRestoredFromAutosave; }

// Inherited from FileSystem
Expand All @@ -95,10 +96,12 @@ class TransactionalFileSystem final : public FileSystem {
virtual void removeDirRecursively(const QString& path = "") override;

// General Methods
void loadFromZip(const FilePath& fp);
void exportToZip(const FilePath& fp) const;
void autosave();
void save();
void loadFromZip(const FilePath& fp);
void exportToZip(const FilePath& fp) const;
void discardChanges() noexcept;
QStringList checkForModifications() const;
void autosave();
void save();

// Static Methods
static std::shared_ptr<TransactionalFileSystem> open(
Expand Down Expand Up @@ -126,7 +129,6 @@ class TransactionalFileSystem final : public FileSystem {
void saveDiff(const QString& type) const;
void loadDiff(const FilePath& fp);
void removeDiff(const QString& type);
void discardChanges() noexcept;

private: // Data
FilePath mFilePath;
Expand Down
3 changes: 0 additions & 3 deletions tests/cli/open-project/test_save.py
View file
Expand Up @@ -9,9 +9,6 @@
Test command "open-project --save"
"""

PROJECT_LPP = 'data/Empty Project/Empty Project.lpp'
PROJECT_LPPZ = 'data/Empty Project.lppz'


@pytest.mark.parametrize("project", [
params.EMPTY_PROJECT_LPP_PARAM,
Expand Down

0 comments on commit 143c6ea

Please sign in to comment.