Skip to content

Commit

Permalink
Fix include and subninja paths
Browse files Browse the repository at this point in the history
The paths specified by `include` and `subninja` statements are relative
to the current ninja build file and we weren't taking into account the
relative path between this ninja build file and our current working
directory.

This commit changes the snapshot tests to not run from the same
directory as the ninja build file and adds the relative path from the
current working directory on to new paths we need to follow.
  • Loading branch information
elliotgoodrich committed Sep 22, 2024
1 parent 3478434 commit 7d6e819
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 19 deletions.
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ set_tests_properties(
foreach(TEST ${TRIMJA_TESTS})
add_test(
NAME trimja.snapshot.${TEST}
COMMAND trimja -f build.ninja --expected expected.ninja --affected changed.txt --explain
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests/${TEST}/
COMMAND trimja -f ${TEST}/build.ninja --expected ${TEST}/expected.ninja --affected ${TEST}/changed.txt --explain
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tests
)
set_tests_properties(
trimja.snapshot.${TEST}
Expand Down
14 changes: 10 additions & 4 deletions src/manifestparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,14 @@ const EvalString& IncludeReader::path() {
return *m_storage;
}

const std::filesystem::path& IncludeReader::parent() const {
return m_lexer->getFilename();
}

const std::filesystem::path& SubninjaReader::parent() const {
return m_lexer->getFilename();
}

ManifestReader::iterator::iterator(Lexer* lexer, EvalString* storage)
: detail::BaseReader{lexer, storage},
m_value{PoolReader{nullptr, nullptr, nullptr}} {
Expand Down Expand Up @@ -344,10 +352,8 @@ bool operator!=(const ManifestReader::iterator& iter,

ManifestReader::ManifestReader(const std::filesystem::path& ninjaFile,
const std::string& ninjaFileContents)
: m_lexer(ninjaFileContents.c_str()),
m_storage(),
m_filename(ninjaFile.string()) {
m_lexer.Start(m_filename, ninjaFileContents);
: m_lexer(), m_storage() {
m_lexer.Start(ninjaFile, ninjaFileContents);
}

ManifestReader::iterator ManifestReader::begin() {
Expand Down
10 changes: 9 additions & 1 deletion src/manifestparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,24 @@ class IncludeReader : public detail::BaseReaderWithStart {
public:
using detail::BaseReaderWithStart::BaseReaderWithStart;
const EvalString& path();

// Return the path passed in to the `ManifestReader` constructor. Note that
// this method can be called before or after `path`.
const std::filesystem::path& parent() const;
};

class SubninjaReader : public detail::BaseReaderWithStart {
public:
using detail::BaseReaderWithStart::BaseReaderWithStart;

// Return the path passed in to the `ManifestReader` constructor. Note that
// this method can be called before or after `path`.
const std::filesystem::path& parent() const;
};

class ManifestReader {
Lexer m_lexer;
EvalString m_storage;
std::string m_filename;

public:
class sentinel {};
Expand Down
18 changes: 11 additions & 7 deletions src/trimutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,18 +338,22 @@ struct BuildContext {
}

void operator()(IncludeReader& r) {
const EvalString& pathEval = r.path();
std::string path;
evaluate(path, pathEval, fileScope);
const std::filesystem::path file = [&] {
const EvalString& pathEval = r.path();
std::string path;
evaluate(path, pathEval, fileScope);
return std::filesystem::path(r.parent()).remove_filename() / path;
}();

if (!std::filesystem::exists(path)) {
throw std::runtime_error(std::format("Unable to find {}!", path));
if (!std::filesystem::exists(file)) {
throw std::runtime_error(
std::format("Unable to find {}!", file.string()));
}
std::stringstream ninjaCopy;
std::ifstream ninja(path);
std::ifstream ninja(file);
ninjaCopy << ninja.rdbuf();
fileContents.push_front(ninjaCopy.str());
parse(path, fileContents.front());
parse(file, fileContents.front());
}

void operator()(SubninjaReader&) {
Expand Down
6 changes: 3 additions & 3 deletions thirdparty/ninja/lexer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ bool Lexer::Error(const std::string_view& message, std::string* err) {
int col = last_token_ ? (int)(last_token_ - line_start) : 0;

char buf[1024];
snprintf(buf, sizeof(buf), "%s:%d: ", std::string(filename_).c_str(), line);
snprintf(buf, sizeof(buf), "%s:%d: ", filename_.string().c_str(), line);
*err = buf;
*err += message;
*err += "\n";
Expand Down Expand Up @@ -66,8 +66,8 @@ Lexer::Lexer(const char* input) {
Start("input", input);
}

void Lexer::Start(std::string_view filename, std::string_view input) {
filename_ = filename;
void Lexer::Start(std::filesystem::path filename, std::string_view input) {
filename_ = std::move(filename);
input_ = input;
ofs_ = input_.data();
last_token_ = NULL;
Expand Down
10 changes: 8 additions & 2 deletions thirdparty/ninja/lexer.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,13 @@
// * Add `const char* position()` to return the current position in the
// ninja file
// * Change 'ReadIdent' to return the key as a `string_view`
// * Store the filename as a `std::filesystem::path` and add `getFilename`
// accessor

#ifndef NINJA_LEXER_H_
#define NINJA_LEXER_H_

#include <filesystem>
#include <string>
#include <string_view>

Expand Down Expand Up @@ -72,7 +75,7 @@ struct Lexer {
std::string_view DescribeLastError();

/// Start parsing some input.
void Start(std::string_view filename, std::string_view input);
void Start(std::filesystem::path filename, std::string_view input);

/// Read a Token from the Token enum.
Token ReadToken();
Expand Down Expand Up @@ -114,10 +117,13 @@ struct Lexer {
/// Read a $-escaped string.
bool ReadEvalString(EvalString* eval, bool path, std::string* err);

std::string_view filename_;
std::filesystem::path filename_;
std::string_view input_;
const char* ofs_;
const char* last_token_;

public:
const std::filesystem::path& getFilename() const { return filename_; }
};

#endif // NINJA_LEXER_H_

0 comments on commit 7d6e819

Please sign in to comment.