Skip to content

Commit

Permalink
Fix issues found when bootstrapping trimja
Browse files Browse the repository at this point in the history
When installing `trimja-action` on this repository there were a few
issues found:

  * CMake unity build creates the source files at the configure step and
    not within the Ninja build file.  This means that we lose the
    connection and we cannot use trimja with unity builds
  * Affected files are not converted in an intuitive manner from
    relative to absolute.  CMake generates absolute file paths in the
    `${build-dir}/build.ninja` file.  If we run `git diff --name-only`
    in the root directory we will get paths in the format
    `src/header.h`.  `trimja` will convert these to absolute paths using
    the Ninja build file directory as the base, where we probably need
    to use the current working directory.
  • Loading branch information
elliotgoodrich committed Oct 10, 2024
1 parent a160c50 commit 9a3cb2c
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 9 deletions.
8 changes: 3 additions & 5 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,13 @@ jobs:
-DCMAKE_C_COMPILER=${{ matrix.environment.c }}
${{ format('-D{0}={1}', matrix.environment.name == 'Windows' && 'CMAKE_CONFIGURATION_TYPES' || 'CMAKE_BUILD_TYPE', matrix.build_type) }}
${{ matrix.environment.name != 'Windows' && '-G Ninja' || ''}}
-DCMAKE_UNITY_BUILD=ON
- name: Build
run: cmake --build build --config ${{ matrix.build_type }}
run: cmake --build ${{ steps.strings.outputs.build-output-dir }} --config ${{ matrix.build_type }}
- name: Test
working-directory: build
run: ctest --build-config ${{ matrix.build_type }} --output-on-failure
run: ctest --test-dir ${{ steps.strings.outputs.build-output-dir }} --build-config ${{ matrix.build_type }} --output-on-failure
- name: Install
if: startsWith(github.ref, 'refs/tags/') && matrix.build_type == 'Release'
run: cmake --build build --config ${{ matrix.build_type }} --target package
run: cmake --build ${{ steps.strings.outputs.build-output-dir }} --config ${{ matrix.build_type }} --target package
- name: Generate Changelog
run: git show -s --format='%b' > build/CHANGELOG.txt
if: startsWith(github.ref, 'refs/tags/') && matrix.build_type == 'Release'
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Microsoft Visual Studio supports opening CMake projects directly.
A standard workflow to build a Debug version into the `build` folder would look
like:

1. configure: `cmake -B build -DCMAKE_BUILD_TYPE=Debug -DCMAKE_UNITY_BUILD=ON`
1. configure: `cmake -B build -DCMAKE_BUILD_TYPE=Debug`
2. build: `cmake --build build --config Debug`
3. test: `ctest --test-dir build --build-config Debug --output-on-failure`
4. package: `cmake --build build --config Debug --target package`
Expand Down
1 change: 1 addition & 0 deletions src/builddirutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "manifestparser.h"

#include <fstream>
#include <sstream>
#include <variant>

namespace trimja {
Expand Down
2 changes: 2 additions & 0 deletions src/fixed_string.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

#include "fixed_string.h"

#include <algorithm>

namespace trimja {

const fixed_string& fixed_string::make_temp(
Expand Down
1 change: 1 addition & 0 deletions src/graph.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "fixed_string.h"

#include <numeric>
#include <optional>
#include <set>
#include <string>
Expand Down
5 changes: 2 additions & 3 deletions src/trimutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -655,8 +655,7 @@ void TrimUtil::trim(std::ostream& output,
// If that does not indicate a path, try the absolute path
std::filesystem::path p(line);
if (!p.is_absolute()) {
const std::filesystem::path absolute =
std::filesystem::absolute(ninjaFileDir / p);
const std::filesystem::path absolute = std::filesystem::absolute(p);
std::string absoluteStr = absolute.string();
const std::optional<std::size_t> index = graph.findPath(absoluteStr);
if (index.has_value()) {
Expand All @@ -673,7 +672,7 @@ void TrimUtil::trim(std::ostream& output,
// If neither indicates a path, then try the path relative to the ninja
// file
if (!p.is_relative()) {
const std::filesystem::path relative = p.lexically_relative(ninjaFileDir);
const std::filesystem::path relative = std::filesystem::relative(p);
std::string relativeStr = relative.string();
const std::optional<std::size_t> index = graph.findPath(relativeStr);
if (index.has_value()) {
Expand Down

0 comments on commit 9a3cb2c

Please sign in to comment.