Skip to content

Commit

Permalink
Revert "Revert "fix: use both absolute and relative header paths in h…
Browse files Browse the repository at this point in the history
…eader matching (danmar#362)" (danmar#415)"

This reverts commit 9ce981c.
  • Loading branch information
Tal Hadad committed Feb 16, 2025
1 parent 09a8163 commit c912799
Show file tree
Hide file tree
Showing 6 changed files with 286 additions and 24 deletions.
17 changes: 16 additions & 1 deletion .github/workflows/CI-unixish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,18 @@ 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 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)

Expand All @@ -41,6 +52,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
Expand Down
18 changes: 17 additions & 1 deletion .github/workflows/CI-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand All @@ -48,4 +59,9 @@ jobs:
- name: Selfcheck
run: |
.\${{ matrix.config }}\simplecpp.exe simplecpp.cpp -e || exit /b !errorlevel!
- name: integration test
run: |
set SIMPLECPP_EXE_PATH=.\${{ matrix.config }}\simplecpp.exe
python -m pytest integration_test.py || exit /b !errorlevel!
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,6 @@ testrunner
# CLion
/.idea
/cmake-build-*

# python
__pycache__/
94 changes: 94 additions & 0 deletions integration_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
## 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):
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, 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"""
#undef TEST_H_INCLUDED
#include {format_include(include1, is_include1_sys)}
#include {format_include(include2, is_include2_sys)}
""")
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)

test_file = __test_relative_header_create_source(tmpdir, "test.h", "test.h", is_include1_sys=is_sys, is_include2_sys=is_sys)

args = ([format_include_path_arg(tmpdir)] if is_sys else []) + [test_file]

_, _, stderr = simplecpp(args, cwd=tmpdir)

if with_pragma_once:
assert stderr == ''
else:
assert double_include_error in stderr

@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, inv=inv)

args = [test_file]

_, _, stderr = simplecpp(args, cwd=tmpdir)
assert stderr == ''

@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, is_include1_sys=is_sys, inv=inv)

args = [test_file]

_, _, stderr = simplecpp(args, cwd=tmpdir)

if is_sys:
assert "missing header: Header not found" in stderr
else:
assert stderr == ''

@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=is_sys, inv=inv)

args = [format_include_path_arg(test_subdir), test_file]

_, _, stderr = simplecpp(args, cwd=tmpdir)
assert stderr == ''
121 changes: 99 additions & 22 deletions simplecpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
#ifdef SIMPLECPP_WINDOWS
#include <windows.h>
#undef ERROR
#else
#include <unistd.h>
#endif

#if __cplusplus >= 201103L
Expand Down Expand Up @@ -147,6 +149,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());
Expand Down Expand Up @@ -2681,6 +2688,46 @@ static bool isCpp17OrLater(const simplecpp::DUI &dui)
return !std_ver.empty() && (std_ver >= "201703L");
}


static std::string currentDirectoryOSCalc() {
#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 const 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<std::string, bool> extractRelativePathFromAbsolute(const std::string& absolutepath) {
static const std::string prefix = currentDirectory() + "/";
if (startsWith(absolutepath, prefix)) {
const std::size_t size = prefix.size();
return std::make_pair(absolutepath.substr(size, absolutepath.size() - size), true);
}
// otherwise
return std::make_pair("", false);
}

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)
{
Expand Down Expand Up @@ -3099,9 +3146,12 @@ static std::string openHeader(std::ifstream &f, const std::string &path)

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;
return simplecpp::simplifyPath(path);
}

static std::string openHeaderRelative(std::ifstream &f, const std::string &sourcefile, const std::string &header)
Expand All @@ -3111,7 +3161,7 @@ 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;
std::string path = toAbsolutePath(includePath);
if (!path.empty() && path[path.size()-1U]!='/' && path[path.size()-1U]!='\\')
path += '/';
return path + header;
Expand All @@ -3120,9 +3170,9 @@ static std::string getIncludePathFileName(const std::string &includePath, const
static std::string openHeaderIncludePath(std::ifstream &f, const simplecpp::DUI &dui, const std::string &header)
{
for (std::list<std::string>::const_iterator it = dui.includePaths.begin(); it != dui.includePaths.end(); ++it) {
std::string simplePath = openHeader(f, getIncludePathFileName(*it, header));
if (!simplePath.empty())
return simplePath;
std::string path = openHeader(f, getIncludePathFileName(*it, header));
if (!path.empty())
return path;
}
return "";
}
Expand All @@ -3132,49 +3182,76 @@ static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const
if (isAbsolutePath(header))
return openHeader(f, header);

std::string ret;

if (systemheader) {
ret = openHeaderIncludePath(f, dui, header);
return ret;
// always return absolute path for systemheaders
return toAbsolutePath(openHeaderIncludePath(f, dui, header));
}

std::string ret;

ret = openHeaderRelative(f, sourcefile, header);
if (ret.empty())
return openHeaderIncludePath(f, dui, header);
return toAbsolutePath(openHeaderIncludePath(f, dui, header));// in a similar way to system headers
return ret;
}

static std::string getFileName(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader)
static std::string findPathInMapBothRelativeAndAbsolute(const std::map<std::string, simplecpp::TokenList *> &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<std::string, bool> relativeExtractedResult = extractRelativePathFromAbsolute(path);
if (relativeExtractedResult.second) {
const std::string relativePath = relativeExtractedResult.first;
if (filedata.find(relativePath) != filedata.end()) {
return relativePath;
}
}
} else {
const std::string absolutePath = toAbsolutePath(path);
if (filedata.find(absolutePath) != filedata.end())
return absolutePath;
}
// otherwise
return "";
}

static std::string getFileIdPath(const std::map<std::string, simplecpp::TokenList *> &filedata, const std::string &sourcefile, const std::string &header, const simplecpp::DUI &dui, bool systemheader)
{
if (filedata.empty()) {
return "";
}
if (isAbsolutePath(header)) {
return (filedata.find(header) != filedata.end()) ? 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<std::string>::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, simplecpp::simplifyPath(getIncludePathFileName(*it, header)));
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<std::string, simplecpp::TokenList *> &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<std::string, simplecpp::TokenList*> simplecpp::load(const simplecpp::TokenList &rawtokens, std::vector<std::string> &filenames, const simplecpp::DUI &dui, simplecpp::OutputList *outputList)
Expand Down Expand Up @@ -3530,7 +3607,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;
Expand Down
Loading

0 comments on commit c912799

Please sign in to comment.