From 907c44354a9bcf2509bc073ec30d8fd84e0b7026 Mon Sep 17 00:00:00 2001 From: Tal500 Date: Tue, 6 Aug 2024 12:15:00 +0300 Subject: [PATCH 01/15] fix: always use absolute (and simplified) header path whenever possible This fix an issue of importing the same header by different addresses. The issue is trickier than seems, because if a user used the include dir flag `-I...` with an absolute path, then every matched header was detected to be absolute, but if the import was a relative import to the source file and the source file isn't with an absolute path, than the header is identified by the combined relative path. See https://sourceforge.net/p/cppcheck/discussion/general/thread/c01f80556b/ --- simplecpp.cpp | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/simplecpp.cpp b/simplecpp.cpp index 3cce780..d3ee9ce 100755 --- a/simplecpp.cpp +++ b/simplecpp.cpp @@ -43,6 +43,8 @@ #ifdef SIMPLECPP_WINDOWS #include #undef ERROR +#else +#include #endif #if __cplusplus >= 201103L @@ -3087,11 +3089,29 @@ static std::string openHeader(std::ifstream &f, const std::string &path) return ""; } +static std::string currentDirectory() { +#ifdef SIMPLECPP_WINDOWS + TCHAR NPath[MAX_PATH]; + GetCurrentDirectory(MAX_PATH, NPath); + return NPath; +#else + const std::size_t size = 1024; + char the_path[size]; + getcwd(the_path, size); + return the_path; +#endif +} + static std::string getRelativeFileName(const std::string &sourcefile, const std::string &header) { + std::string path; if (sourcefile.find_first_of("\\/") != std::string::npos) - return simplecpp::simplifyPath(sourcefile.substr(0, sourcefile.find_last_of("\\/") + 1U) + header); - return simplecpp::simplifyPath(header); + path = sourcefile.substr(0, sourcefile.find_last_of("\\/") + 1U) + header; + else + path = header; + if (!isAbsolutePath(path)) + path = currentDirectory() + "/" + path; + return simplecpp::simplifyPath(path); } static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header) @@ -3102,6 +3122,8 @@ static std::string openHeaderRelative(std::ifstream &f, const std::string &sourc static std::string getIncludePathFileName(const std::string &includePath, const std::string &header) { std::string path = includePath; + if (!isAbsolutePath(includePath)) + path = currentDirectory() + "/" + path; if (!path.empty() && path[path.size()-1U]!='/' && path[path.size()-1U]!='\\') path += '/'; return path + header; @@ -3141,7 +3163,8 @@ static std::string getFileName(const std::map Date: Thu, 14 Nov 2024 22:00:21 +0200 Subject: [PATCH 02/15] respect relative paths, but check both absolute and relative path id instead --- simplecpp.cpp | 157 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 114 insertions(+), 43 deletions(-) diff --git a/simplecpp.cpp b/simplecpp.cpp index 5cfa33c..afc8cf9 100755 --- a/simplecpp.cpp +++ b/simplecpp.cpp @@ -140,6 +140,11 @@ static unsigned long long stringToULL(const std::string &s) return ret; } +static bool startsWith(const std::string &s, const std::string &p) +{ + return (s.size() >= p.size()) && std::equal(p.begin(), p.end(), s.begin()); +} + static bool endsWith(const std::string &s, const std::string &e) { return (s.size() >= e.size()) && std::equal(e.rbegin(), e.rend(), s.rbegin()); @@ -2660,6 +2665,59 @@ static bool isCpp17OrLater(const simplecpp::DUI &dui) return !std_ver.empty() && (std_ver >= "201703L"); } + +static std::string currentDirectoryCalc() { +#ifdef SIMPLECPP_WINDOWS + TCHAR NPath[MAX_PATH]; + GetCurrentDirectory(MAX_PATH, NPath); + return NPath; +#else + const std::size_t size = 1024; + char the_path[size]; + getcwd(the_path, size); + return the_path; +#endif +} + +static const std::string& currentDirectory() { + static std::string curdir = currentDirectoryCalc(); + return curdir; +} + +static std::pair extractRelativePathFromAbsolute(const std::string& absolutepath) { + static const std::string prefix = currentDirectory() + "/"; + if (startsWith(absolutepath, prefix)) { + std::size_t size = prefix.size(); + return std::make_pair(absolutepath.substr(size, absolutepath.size() - size), true); + } + // otherwise + return std::make_pair("", false); +} + +struct FilePathAndAbsolute { + std::string path; + std::string absolutePath; + + FilePathAndAbsolute() {} + explicit FilePathAndAbsolute(const std::string& path, const std::string& absolutePath = "") : path(path), absolutePath(absolutePath) {} + + static FilePathAndAbsolute makeFromRelativePath(const std::string& path) { + return FilePathAndAbsolute(path, currentDirectory() + "/" + path); + } + + static FilePathAndAbsolute makeFromAbsolutePath(const std::string& path) { + return FilePathAndAbsolute(path, path); + } + + static FilePathAndAbsolute makeFromGenericPath(const std::string& path) { + if (isAbsolutePath(path)) { + return makeFromAbsolutePath(path); + } + // otherwise + return makeFromRelativePath(path); + } +}; + static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const std::string &sourcefile, const std::string &header, bool systemheader); static void simplifyHasInclude(simplecpp::TokenList &expr, const simplecpp::DUI &dui) { @@ -3061,7 +3119,7 @@ static NonExistingFilesCache nonExistingFilesCache; #endif -static std::string openHeader(std::ifstream &f, const std::string &path) +static FilePathAndAbsolute openHeader(std::ifstream &f, const std::string &path) { std::string simplePath = simplecpp::simplifyPath(path); #ifdef SIMPLECPP_WINDOWS @@ -3070,24 +3128,11 @@ static std::string openHeader(std::ifstream &f, const std::string &path) #endif f.open(simplePath.c_str()); if (f.is_open()) - return simplePath; + return FilePathAndAbsolute::makeFromGenericPath(simplePath); #ifdef SIMPLECPP_WINDOWS nonExistingFilesCache.add(simplePath); #endif - return ""; -} - -static std::string currentDirectory() { -#ifdef SIMPLECPP_WINDOWS - TCHAR NPath[MAX_PATH]; - GetCurrentDirectory(MAX_PATH, NPath); - return NPath; -#else - const std::size_t size = 1024; - char the_path[size]; - getcwd(the_path, size); - return the_path; -#endif + return FilePathAndAbsolute(); } static std::string getRelativeFileName(const std::string &sourcefile, const std::string &header) @@ -3097,12 +3142,10 @@ static std::string getRelativeFileName(const std::string &sourcefile, const std: path = sourcefile.substr(0, sourcefile.find_last_of("\\/") + 1U) + header; else path = header; - if (!isAbsolutePath(path)) - path = currentDirectory() + "/" + path; return simplecpp::simplifyPath(path); } -static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header) +static FilePathAndAbsolute openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header) { return openHeader(f, getRelativeFileName(sourcefile, header)); } @@ -3111,71 +3154,99 @@ static std::string getIncludePathFileName(const std::string &includePath, const { std::string path = includePath; if (!isAbsolutePath(includePath)) - path = currentDirectory() + "/" + path; + path = currentDirectory() + "/" + path;// TODO: Export this to a function if (!path.empty() && path[path.size()-1U]!='/' && path[path.size()-1U]!='\\') path += '/'; return path + header; } -static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header) +static FilePathAndAbsolute openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header) { for (std::list::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) { - std::string simplePath = openHeader(f, getIncludePathFileName(*it, header)); - if (!simplePath.empty()) - return simplePath; + FilePathAndAbsolute simplePathInfo = openHeader(f, getIncludePathFileName(*it, header)); + if (!simplePathInfo.path.empty()) + return simplePathInfo; } - return ""; + return FilePathAndAbsolute(); } static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const std::string &sourcefile, const std::string &header, bool systemheader) { if (isAbsolutePath(header)) - return openHeader(f, header); - - std::string ret; + return openHeader(f, header).path; if (systemheader) { - ret = openHeaderIncludePath(f, dui, header); - return ret; + // always return absolute path for systemheaders + return openHeaderIncludePath(f, dui, header).absolutePath; } + FilePathAndAbsolute ret; + ret = openHeaderRelative(f, sourcefile, header); - if (ret.empty()) - return openHeaderIncludePath(f, dui, header); - return ret; + if (ret.path.empty()) + return openHeaderIncludePath(f, dui, header).absolutePath;// in a similar way to system headers + return ret.path; } -static std::string getFileName(const std::map &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader) +static std::string findPathInMapBothRelativeAndAbsolute(const std::map &filedata, const std::string& path) { + // here there are two possibilities - either we match this from absolute path or from a relative one + if (filedata.find(path) != filedata.end()) {// try first to respect the exact match + return path; + } + // otherwise - try to use the normalize to the correct representation + if (isAbsolutePath(path)) { + const std::pair relativeExtractedResult = extractRelativePathFromAbsolute(path); + if (relativeExtractedResult.second) { + const std::string relativePath = relativeExtractedResult.first; + if (filedata.find(relativePath) != filedata.end()) { + return relativePath; + } + } + } else { + const FilePathAndAbsolute filePathInfo = FilePathAndAbsolute::makeFromGenericPath(path); + const std::string absolutePath = filePathInfo.absolutePath; + if (filedata.find(absolutePath) != filedata.end()) + return absolutePath; + } + // otherwise + return ""; +} + +static std::string getFileIdPath(const std::map &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader) { if (filedata.empty()) { return ""; } if (isAbsolutePath(header)) { - const std::string simplifiedHeaderPath = simplecpp::simplifyPath(header); + const std::string simplifiedHeaderPath = simplecpp::simplifyPath(header); return (filedata.find(simplifiedHeaderPath) != filedata.end()) ? simplifiedHeaderPath : ""; } if (!systemheader) { - const std::string relativeFilename = getRelativeFileName(sourcefile, header); - if (filedata.find(relativeFilename) != filedata.end()) - return relativeFilename; + const std::string relativeOrAbsoluteFilename = getRelativeFileName(sourcefile, header);// unknown if absolute or relative, but always simplified + const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, relativeOrAbsoluteFilename); + if (!match.empty()) { + return match; + } } for (std::list::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) { std::string s = simplecpp::simplifyPath(getIncludePathFileName(*it, header)); - if (filedata.find(s) != filedata.end()) - return s; + const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, s); + if (!match.empty()) { + return match; + } } if (systemheader && filedata.find(header) != filedata.end()) - return header; + return header;// system header that its file wasn't found in the included paths but alreasy in the filedata - return this as is return ""; } static bool hasFile(const std::map &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader) { - return !getFileName(filedata, sourcefile, header, dui, systemheader).empty(); + return !getFileIdPath(filedata, sourcefile, header, dui, systemheader).empty(); } std::map simplecpp::load(const simplecpp::TokenList &rawtokens, std::vector &filenames, const simplecpp::DUI &dui, simplecpp::OutputList *outputList) @@ -3531,7 +3602,7 @@ void simplecpp::preprocess(simplecpp::TokenList &output, const simplecpp::TokenL const bool systemheader = (inctok->str()[0] == '<'); const std::string header(realFilename(inctok->str().substr(1U, inctok->str().size() - 2U))); - std::string header2 = getFileName(filedata, rawtok->location.file(), header, dui, systemheader); + std::string header2 = getFileIdPath(filedata, rawtok->location.file(), header, dui, systemheader); if (header2.empty()) { // try to load file.. std::ifstream f; From f156111b9ce350ff15124a6b999bb8e945932297 Mon Sep 17 00:00:00 2001 From: Tal Hadad Date: Sun, 17 Nov 2024 12:22:13 +0200 Subject: [PATCH 03/15] simplify path usage functions to be just strings --- simplecpp.cpp | 76 ++++++++++++++++++++------------------------------- 1 file changed, 30 insertions(+), 46 deletions(-) diff --git a/simplecpp.cpp b/simplecpp.cpp index afc8cf9..d1fd310 100755 --- a/simplecpp.cpp +++ b/simplecpp.cpp @@ -2666,7 +2666,7 @@ static bool isCpp17OrLater(const simplecpp::DUI &dui) } -static std::string currentDirectoryCalc() { +static std::string currentDirectoryOSCalc() { #ifdef SIMPLECPP_WINDOWS TCHAR NPath[MAX_PATH]; GetCurrentDirectory(MAX_PATH, NPath); @@ -2680,10 +2680,21 @@ static std::string currentDirectoryCalc() { } static const std::string& currentDirectory() { - static std::string curdir = currentDirectoryCalc(); + static std::string curdir = simplecpp::simplifyPath(currentDirectoryOSCalc()); return curdir; } +static std::string toAbsolutePath(const std::string& path) { + if (path.empty()) { + return path;// preserve error file path that is indicated by an empty string + } + if (!isAbsolutePath(path)) { + return currentDirectory() + "/" + path; + } + // otherwise + return path; +} + static std::pair extractRelativePathFromAbsolute(const std::string& absolutepath) { static const std::string prefix = currentDirectory() + "/"; if (startsWith(absolutepath, prefix)) { @@ -2694,30 +2705,6 @@ static std::pair extractRelativePathFromAbsolute(const std::s return std::make_pair("", false); } -struct FilePathAndAbsolute { - std::string path; - std::string absolutePath; - - FilePathAndAbsolute() {} - explicit FilePathAndAbsolute(const std::string& path, const std::string& absolutePath = "") : path(path), absolutePath(absolutePath) {} - - static FilePathAndAbsolute makeFromRelativePath(const std::string& path) { - return FilePathAndAbsolute(path, currentDirectory() + "/" + path); - } - - static FilePathAndAbsolute makeFromAbsolutePath(const std::string& path) { - return FilePathAndAbsolute(path, path); - } - - static FilePathAndAbsolute makeFromGenericPath(const std::string& path) { - if (isAbsolutePath(path)) { - return makeFromAbsolutePath(path); - } - // otherwise - return makeFromRelativePath(path); - } -}; - static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const std::string &sourcefile, const std::string &header, bool systemheader); static void simplifyHasInclude(simplecpp::TokenList &expr, const simplecpp::DUI &dui) { @@ -3119,7 +3106,7 @@ static NonExistingFilesCache nonExistingFilesCache; #endif -static FilePathAndAbsolute openHeader(std::ifstream &f, const std::string &path) +static std::string openHeader(std::ifstream &f, const std::string &path) { std::string simplePath = simplecpp::simplifyPath(path); #ifdef SIMPLECPP_WINDOWS @@ -3128,11 +3115,11 @@ static FilePathAndAbsolute openHeader(std::ifstream &f, const std::string &path) #endif f.open(simplePath.c_str()); if (f.is_open()) - return FilePathAndAbsolute::makeFromGenericPath(simplePath); + return simplePath; #ifdef SIMPLECPP_WINDOWS nonExistingFilesCache.add(simplePath); #endif - return FilePathAndAbsolute(); + return ""; } static std::string getRelativeFileName(const std::string &sourcefile, const std::string &header) @@ -3145,47 +3132,45 @@ static std::string getRelativeFileName(const std::string &sourcefile, const std: return simplecpp::simplifyPath(path); } -static FilePathAndAbsolute openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header) +static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header) { return openHeader(f, getRelativeFileName(sourcefile, header)); } static std::string getIncludePathFileName(const std::string &includePath, const std::string &header) { - std::string path = includePath; - if (!isAbsolutePath(includePath)) - path = currentDirectory() + "/" + path;// TODO: Export this to a function + std::string path = toAbsolutePath(includePath); if (!path.empty() && path[path.size()-1U]!='/' && path[path.size()-1U]!='\\') path += '/'; return path + header; } -static FilePathAndAbsolute openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header) +static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header) { for (std::list::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) { - FilePathAndAbsolute simplePathInfo = openHeader(f, getIncludePathFileName(*it, header)); - if (!simplePathInfo.path.empty()) - return simplePathInfo; + std::string path = openHeader(f, getIncludePathFileName(*it, header)); + if (!path.empty()) + return path; } - return FilePathAndAbsolute(); + return ""; } static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const std::string &sourcefile, const std::string &header, bool systemheader) { if (isAbsolutePath(header)) - return openHeader(f, header).path; + return openHeader(f, header); if (systemheader) { // always return absolute path for systemheaders - return openHeaderIncludePath(f, dui, header).absolutePath; + return toAbsolutePath(openHeaderIncludePath(f, dui, header)); } - FilePathAndAbsolute ret; + std::string ret; ret = openHeaderRelative(f, sourcefile, header); - if (ret.path.empty()) - return openHeaderIncludePath(f, dui, header).absolutePath;// in a similar way to system headers - return ret.path; + if (ret.empty()) + return toAbsolutePath(openHeaderIncludePath(f, dui, header));// in a similar way to system headers + return ret; } static std::string findPathInMapBothRelativeAndAbsolute(const std::map &filedata, const std::string& path) { @@ -3203,8 +3188,7 @@ static std::string findPathInMapBothRelativeAndAbsolute(const std::map Date: Sun, 17 Nov 2024 14:18:44 +0200 Subject: [PATCH 04/15] tidy fix --- simplecpp.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/simplecpp.cpp b/simplecpp.cpp index d1fd310..c4e38c4 100755 --- a/simplecpp.cpp +++ b/simplecpp.cpp @@ -2680,7 +2680,7 @@ static std::string currentDirectoryOSCalc() { } static const std::string& currentDirectory() { - static std::string curdir = simplecpp::simplifyPath(currentDirectoryOSCalc()); + static const std::string curdir = simplecpp::simplifyPath(currentDirectoryOSCalc()); return curdir; } @@ -2698,7 +2698,7 @@ static std::string toAbsolutePath(const std::string& path) { static std::pair extractRelativePathFromAbsolute(const std::string& absolutepath) { static const std::string prefix = currentDirectory() + "/"; if (startsWith(absolutepath, prefix)) { - std::size_t size = prefix.size(); + const std::size_t size = prefix.size(); return std::make_pair(absolutepath.substr(size, absolutepath.size() - size), true); } // otherwise @@ -3215,8 +3215,7 @@ static std::string getFileIdPath(const std::map::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) { - std::string s = simplecpp::simplifyPath(getIncludePathFileName(*it, header)); - const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, s); + const std::string match = findPathInMapBothRelativeAndAbsolute(filedata, simplecpp::simplifyPath(getIncludePathFileName(*it, header))); if (!match.empty()) { return match; } From 2893b68b9420d3c4b137e3a484ddd3976ce4649d Mon Sep 17 00:00:00 2001 From: Tal Hadad Date: Tue, 10 Dec 2024 18:15:10 +0200 Subject: [PATCH 05/15] add integration tests --- .gitignore | 3 + integration_test.py | 131 ++++++++++++++++++++++++++++++++++++++++++++ testutils.py | 54 ++++++++++++++++++ 3 files changed, 188 insertions(+) create mode 100644 integration_test.py create mode 100644 testutils.py diff --git a/.gitignore b/.gitignore index 183545f..113cf36 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,6 @@ testrunner # CLion /.idea /cmake-build-* + +# python +__pycache__/ diff --git a/integration_test.py b/integration_test.py new file mode 100644 index 0000000..875c9ea --- /dev/null +++ b/integration_test.py @@ -0,0 +1,131 @@ +## test with python -m pytest integration_test.py + +import os +from testutils import simplecpp, format_include_path_arg, format_include + +def __test_relative_header_create_header(dir, with_pragma_once=True): + header_file = os.path.join(dir, 'test.h') + with open(header_file, 'wt') as f: + f.write(f""" + {"#pragma once" if with_pragma_once else ""} + #ifndef TEST_H_INCLUDED + #define TEST_H_INCLUDED + #else + #error header_was_already_included + #endif + """) + return header_file, "error: #error header_was_already_included" + +def __test_relative_header_create_source(dir, include1, include2, is_include1_sys=False, is_include2_sys=False): + src_file = os.path.join(dir, 'test.c') + with open(src_file, 'wt') as f: + f.write(f""" + #undef TEST_H_INCLUDED + #include {format_include(include1, is_include1_sys)} + #include {format_include(include2, is_include2_sys)} + """) + return src_file + + +def test_relative_header_0_rel(tmpdir): + _, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=False) + + test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h") + + args = [test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert double_include_error in stderr + +def test_relative_header_0_sys(tmpdir): + _, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=False) + + test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=True, is_include2_sys=True) + + args = [format_include_path_arg(tmpdir), test_file] + + _, _, stderr = simplecpp(args) + assert double_include_error in stderr + +def test_relative_header_1_rel(tmpdir): + __test_relative_header_create_header(tmpdir) + + test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h") + + args = [test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert stderr == '' + +def test_relative_header_1_sys(tmpdir): + __test_relative_header_create_header(tmpdir) + + test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=True, is_include2_sys=True) + + args = [format_include_path_arg(tmpdir), test_file] + + _, _, stderr = simplecpp(args) + assert stderr == '' + +## TODO: the following tests should pass after applying simplecpp#362 + +def test_relative_header_2(tmpdir): + header_file, _ = __test_relative_header_create_header(tmpdir) + + test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file) + + args = [test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert stderr == '' + +def test_relative_header_3(tmpdir): + test_subdir = os.path.join(tmpdir, "test_subdir") + os.mkdir(test_subdir) + header_file, _ = __test_relative_header_create_header(test_subdir) + + test_file = __test_relative_header_create_source(tmpdir, "test_subdir/test.h", header_file) + + args = [test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert stderr == '' + +def test_relative_header_3_inv(tmpdir): + test_subdir = os.path.join(tmpdir, "test_subdir") + os.mkdir(test_subdir) + header_file, _ = __test_relative_header_create_header(test_subdir) + + test_file = __test_relative_header_create_source(tmpdir, header_file, "test_subdir/test.h") + + args = [test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert stderr == '' + + +def test_relative_header_4(tmpdir): + test_subdir = os.path.join(tmpdir, "test_subdir") + os.mkdir(test_subdir) + header_file, _ = __test_relative_header_create_header(test_subdir) + + test_file = __test_relative_header_create_source(tmpdir, header_file, "test.h", is_include2_sys=True) + + args = [format_include_path_arg(test_subdir), test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert stderr == '' + + + +def test_relative_header_5(tmpdir): + test_subdir = os.path.join(tmpdir, "test_subdir") + os.mkdir(test_subdir) + __test_relative_header_create_header(test_subdir) + + test_file = __test_relative_header_create_source(tmpdir, "test.h", "test_subdir/test.h", is_include1_sys=True) + + args = [format_include_path_arg(test_subdir), test_file] + + _, _, stderr = simplecpp(args, cwd=tmpdir) + assert stderr == '' diff --git a/testutils.py b/testutils.py new file mode 100644 index 0000000..1237f8f --- /dev/null +++ b/testutils.py @@ -0,0 +1,54 @@ +import os +import subprocess +import json + +def __run_subprocess(args, env=None, cwd=None, timeout=None): + p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env, cwd=cwd) + + try: + stdout, stderr = p.communicate(timeout=timeout) + return_code = p.returncode + p = None + except subprocess.TimeoutExpired: + import psutil + # terminate all the child processes + child_procs = psutil.Process(p.pid).children(recursive=True) + if len(child_procs) > 0: + for child in child_procs: + child.terminate() + try: + # call with timeout since it might be stuck + p.communicate(timeout=5) + p = None + except subprocess.TimeoutExpired: + pass + raise + finally: + if p: + # sending the signal to the process groups causes the parent Python process to terminate as well + #os.killpg(os.getpgid(p.pid), signal.SIGTERM) # Send the signal to all the process groups + p.terminate() + stdout, stderr = p.communicate() + p = None + + stdout = stdout.decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') + stderr = stderr.decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n') + + return return_code, stdout, stderr + +def simplecpp(args = [], cwd = None): + dir_path = os.path.dirname(os.path.realpath(__file__)) + simplecpp_path = os.path.join(dir_path, "simplecpp") + return __run_subprocess([simplecpp_path] + args, cwd = cwd) + +def quoted_string(s): + return json.dumps(str(s)) + +def format_include_path_arg(include_path): + return f"-I{str(include_path)}" + +def format_include(include, is_sys_header=False): + if is_sys_header: + return f"<{quoted_string(include)[1:-1]}>" + else: + return quoted_string(include) From 196a498f2b4d7b2e175287417519ccaa9b636c24 Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 18:43:52 +0200 Subject: [PATCH 06/15] run integraion tests on CI-unixish.yml --- .github/workflows/CI-unixish.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index 050e1d7..9fc5682 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -30,7 +30,17 @@ jobs: run: | sudo apt-get update sudo apt-get install libc++-18-dev - + + - name: Install missing software on macos + if: contains(matrix.os, 'macos') + run: | + brew install python3 + + - name: Install missing Python packages + run: | + python3 -m pip install pip --upgrade + python3 -m pip install pytest + - name: make simplecpp run: make -j$(nproc) @@ -41,6 +51,10 @@ jobs: run: | make -j$(nproc) selfcheck + - name: integration test + run: | + python3 -m pytest integration_test.py + - name: Run CMake run: | cmake -S . -B cmake.output From 52c77b9a824a17425a53e68f3dc8baba63cc1ec8 Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 18:48:49 +0200 Subject: [PATCH 07/15] Use `pip --user` to avoid macos14 permission issues in CI-unixish.yml --- .github/workflows/CI-unixish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index 9fc5682..f82eaaf 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -38,8 +38,8 @@ jobs: - name: Install missing Python packages run: | - python3 -m pip install pip --upgrade - python3 -m pip install pytest + python3 -m pip --user install pip --upgrade + python3 -m pip --user install pytest - name: make simplecpp run: make -j$(nproc) From ef7837a5be7be0053dd89929180224ab38dfbe7b Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 18:51:32 +0200 Subject: [PATCH 08/15] Move the flag `--user` in the pip install row CI-unixish.yml --- .github/workflows/CI-unixish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index f82eaaf..aa9cfa5 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -38,8 +38,8 @@ jobs: - name: Install missing Python packages run: | - python3 -m pip --user install pip --upgrade - python3 -m pip --user install pytest + python3 -m pip install pip --upgrade --user + python3 -m pip install pytest --user - name: make simplecpp run: make -j$(nproc) From c2e9b673ecdda0a29babc68b428a5f03c77340bb Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 18:54:43 +0200 Subject: [PATCH 09/15] Try the flag `--break-system-packages` instead of `--user` in CI-unixish.yml --- .github/workflows/CI-unixish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index aa9cfa5..241d533 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -38,8 +38,8 @@ jobs: - name: Install missing Python packages run: | - python3 -m pip install pip --upgrade --user - python3 -m pip install pytest --user + python3 -m pip install --break-system-packages pip --upgrade + python3 -m pip install --break-system-packages pytest - name: make simplecpp run: make -j$(nproc) From 26ee4d926d10cce64699ab0decc0500bccd07dfb Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 18:57:40 +0200 Subject: [PATCH 10/15] Set in a more poratable way break-system-packages to be true CI-unixish.yml --- .github/workflows/CI-unixish.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index 241d533..9a0e8d8 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -38,8 +38,9 @@ jobs: - name: Install missing Python packages run: | - python3 -m pip install --break-system-packages pip --upgrade - python3 -m pip install --break-system-packages pytest + python3 -m pip config set global.break-system-packages true + python3 -m pip install pip --upgrade + python3 -m pip install pytest - name: make simplecpp run: make -j$(nproc) From 3b6fc9cf9dff670e4041ee90bda2fb023b9eff08 Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 19:03:04 +0200 Subject: [PATCH 11/15] Pytest for Windows in CI-windows.yml --- .github/workflows/CI-windows.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/CI-windows.yml b/.github/workflows/CI-windows.yml index 1f78876..4e6891d 100644 --- a/.github/workflows/CI-windows.yml +++ b/.github/workflows/CI-windows.yml @@ -26,7 +26,18 @@ jobs: - name: Setup msbuild.exe uses: microsoft/setup-msbuild@v2 - + + - name: Set up Python 3.13 + uses: actions/setup-python@v5 + with: + python-version: '3.13' + check-latest: true + + - name: Install missing Python packages + run: | + python -m pip install pip --upgrade || exit /b !errorlevel! + python -m pip install pytest || exit /b !errorlevel! + - name: Run cmake if: matrix.os == 'windows-2019' run: | @@ -48,4 +59,8 @@ jobs: - name: Selfcheck run: | .\${{ matrix.config }}\simplecpp.exe simplecpp.cpp -e || exit /b !errorlevel! + + - name: integration test + run: | + python -m pytest integration_test.py || exit /b !errorlevel! From 9db5d609388b3e9d2f895974805f59025bd1f06a Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 19:24:22 +0200 Subject: [PATCH 12/15] Update testutils.py: allow custom SIMPLECPP_EXE_PATH for integration tests --- testutils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/testutils.py b/testutils.py index 1237f8f..55a2686 100644 --- a/testutils.py +++ b/testutils.py @@ -38,7 +38,10 @@ def __run_subprocess(args, env=None, cwd=None, timeout=None): def simplecpp(args = [], cwd = None): dir_path = os.path.dirname(os.path.realpath(__file__)) - simplecpp_path = os.path.join(dir_path, "simplecpp") + if 'SIMPLECPP_EXE_PATH' in os.environ: + simplecpp_path = os.environ['SIMPLECPP_EXE_PATH'] + else: + simplecpp_path = os.path.join(dir_path, "simplecpp") return __run_subprocess([simplecpp_path] + args, cwd = cwd) def quoted_string(s): From 47853fb4cc65d14082ce0950d149e475cdb0be92 Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 19:26:27 +0200 Subject: [PATCH 13/15] Update CI-windows.yml: pass SIMPLECPP_EXE_PATH env var --- .github/workflows/CI-windows.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/CI-windows.yml b/.github/workflows/CI-windows.yml index 4e6891d..88909d8 100644 --- a/.github/workflows/CI-windows.yml +++ b/.github/workflows/CI-windows.yml @@ -62,5 +62,6 @@ jobs: - name: integration test run: | + set SIMPLECPP_EXE_PATH=".\${{ matrix.config }}\simplecpp.exe" python -m pytest integration_test.py || exit /b !errorlevel! From c0262191d560e50f4f369a459ece5aadf6072a13 Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 15 Dec 2024 19:29:58 +0200 Subject: [PATCH 14/15] Update CI-windows.yml: fix the set command --- .github/workflows/CI-windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/CI-windows.yml b/.github/workflows/CI-windows.yml index 88909d8..50d5a84 100644 --- a/.github/workflows/CI-windows.yml +++ b/.github/workflows/CI-windows.yml @@ -62,6 +62,6 @@ jobs: - name: integration test run: | - set SIMPLECPP_EXE_PATH=".\${{ matrix.config }}\simplecpp.exe" + set SIMPLECPP_EXE_PATH=.\${{ matrix.config }}\simplecpp.exe python -m pytest integration_test.py || exit /b !errorlevel! From a402c7918b316a54a7a51f2ed694d26d57f5bc0c Mon Sep 17 00:00:00 2001 From: Tal500 Date: Sun, 22 Dec 2024 12:04:09 +0200 Subject: [PATCH 15/15] use parametrized tests and improve coverage in integration_test.py --- integration_test.py | 105 ++++++++++++++------------------------------ 1 file changed, 34 insertions(+), 71 deletions(-) diff --git a/integration_test.py b/integration_test.py index 875c9ea..0b2b0b3 100644 --- a/integration_test.py +++ b/integration_test.py @@ -1,6 +1,7 @@ ## test with python -m pytest integration_test.py import os +import pytest from testutils import simplecpp, format_include_path_arg, format_include def __test_relative_header_create_header(dir, with_pragma_once=True): @@ -16,7 +17,11 @@ def __test_relative_header_create_header(dir, with_pragma_once=True): """) return header_file, "error: #error header_was_already_included" -def __test_relative_header_create_source(dir, include1, include2, is_include1_sys=False, is_include2_sys=False): +def __test_relative_header_create_source(dir, include1, include2, is_include1_sys=False, is_include2_sys=False, inv=False): + if inv: + return __test_relative_header_create_source(dir, include1=include2, include2=include1, is_include1_sys=is_include2_sys, is_include2_sys=is_include1_sys) + ## otherwise + src_file = os.path.join(dir, 'test.c') with open(src_file, 'wt') as f: f.write(f""" @@ -26,104 +31,62 @@ def __test_relative_header_create_source(dir, include1, include2, is_include1_sy """) return src_file +@pytest.mark.parametrize("with_pragma_once", (False, True)) +@pytest.mark.parametrize("is_sys", (False, True)) +def test_relative_header_1(tmpdir, with_pragma_once, is_sys): + _, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=with_pragma_once) -def test_relative_header_0_rel(tmpdir): - _, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=False) - - test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h") + test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=is_sys, is_include2_sys=is_sys) - args = [test_file] + args = ([format_include_path_arg(tmpdir)] if is_sys else []) + [test_file] _, _, stderr = simplecpp(args, cwd=tmpdir) - assert double_include_error in stderr - -def test_relative_header_0_sys(tmpdir): - _, double_include_error = __test_relative_header_create_header(tmpdir, with_pragma_once=False) - - test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=True, is_include2_sys=True) - - args = [format_include_path_arg(tmpdir), test_file] - _, _, stderr = simplecpp(args) - assert double_include_error in stderr + if with_pragma_once: + assert stderr == '' + else: + assert double_include_error in stderr -def test_relative_header_1_rel(tmpdir): - __test_relative_header_create_header(tmpdir) - - test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h") - - args = [test_file] - - _, _, stderr = simplecpp(args, cwd=tmpdir) - assert stderr == '' - -def test_relative_header_1_sys(tmpdir): - __test_relative_header_create_header(tmpdir) - - test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=True, is_include2_sys=True) - - args = [format_include_path_arg(tmpdir), test_file] - - _, _, stderr = simplecpp(args) - assert stderr == '' - -## TODO: the following tests should pass after applying simplecpp#362 - -def test_relative_header_2(tmpdir): +@pytest.mark.parametrize("inv", (False, True)) +def test_relative_header_2(tmpdir, inv): header_file, _ = __test_relative_header_create_header(tmpdir) - test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file) + test_file = __test_relative_header_create_source(tmpdir, "test.h", header_file, inv=inv) args = [test_file] _, _, stderr = simplecpp(args, cwd=tmpdir) assert stderr == '' -def test_relative_header_3(tmpdir): +@pytest.mark.parametrize("is_sys", (False, True)) +@pytest.mark.parametrize("inv", (False, True)) +def test_relative_header_3(tmpdir, is_sys, inv): test_subdir = os.path.join(tmpdir, "test_subdir") os.mkdir(test_subdir) header_file, _ = __test_relative_header_create_header(test_subdir) - test_file = __test_relative_header_create_source(tmpdir, "test_subdir/test.h", header_file) + test_file = __test_relative_header_create_source(tmpdir, "test_subdir/test.h", header_file, is_include1_sys=is_sys, inv=inv) args = [test_file] _, _, stderr = simplecpp(args, cwd=tmpdir) - assert stderr == '' -def test_relative_header_3_inv(tmpdir): - test_subdir = os.path.join(tmpdir, "test_subdir") - os.mkdir(test_subdir) - header_file, _ = __test_relative_header_create_header(test_subdir) - - test_file = __test_relative_header_create_source(tmpdir, header_file, "test_subdir/test.h") - - args = [test_file] + if is_sys: + assert "missing header: Header not found" in stderr + else: + assert stderr == '' - _, _, stderr = simplecpp(args, cwd=tmpdir) - assert stderr == '' - - -def test_relative_header_4(tmpdir): +@pytest.mark.parametrize("use_short_path", (False, True)) +@pytest.mark.parametrize("is_sys", (False, True)) +@pytest.mark.parametrize("inv", (False, True)) +def test_relative_header_4(tmpdir, use_short_path, is_sys, inv): test_subdir = os.path.join(tmpdir, "test_subdir") os.mkdir(test_subdir) header_file, _ = __test_relative_header_create_header(test_subdir) + if use_short_path: + header_file = "test_subdir/test.h" - test_file = __test_relative_header_create_source(tmpdir, header_file, "test.h", is_include2_sys=True) - - args = [format_include_path_arg(test_subdir), test_file] - - _, _, stderr = simplecpp(args, cwd=tmpdir) - assert stderr == '' - - - -def test_relative_header_5(tmpdir): - test_subdir = os.path.join(tmpdir, "test_subdir") - os.mkdir(test_subdir) - __test_relative_header_create_header(test_subdir) - - test_file = __test_relative_header_create_source(tmpdir, "test.h", "test_subdir/test.h", is_include1_sys=True) + test_file = __test_relative_header_create_source(tmpdir, header_file, "test.h", is_include2_sys=is_sys, inv=inv) args = [format_include_path_arg(test_subdir), test_file]