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

fix: use both absolute and relative header paths in header matching #362

Merged
merged 16 commits into from
Jan 1, 2025
Merged
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
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 @@ -138,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());
Expand Down Expand Up @@ -2658,6 +2665,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 @@ -3077,9 +3124,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 @@ -3089,7 +3139,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 @@ -3098,9 +3148,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 @@ -3110,49 +3160,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 @@ -3508,7 +3585,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