Handling absolute paths?

[ghutchis]

This looks fantastic and I appreciate all the work that went into it.

I'm trying to figure out the details.. my most recent builds (https://github.com/OpenChemistry/avogadrolibs/pull/551/files) fail with:

RuntimeError: compile_commands.json contains absolute paths that I don't know how to deal with: '/home/runner/work/avogadrolibs/avogadrolibs/build/thirdparty/spglib-src'

Is there a good way to ensure only relative paths are stored in the JSON?

Could I add a parameter to the script/action to remove part of the absolute paths and have Python rewrite them to relative paths?

[ghutchis]

To be clear, I simply want to transform:
/home/runner/work/avogadrolibs/avogadrolibs/build/thirdparty/spglib-src
into
thirdparty/spglib-src

So

        basedir = original_directory[:build_dir_index]
        newbasedir = os.getcwd()

become something like:

        basedir = original_directory[len(args.prefix_path):build_dir_index]
        newbasedir = os.getcwd()

[ZedThree]

I don't think there's a way to get CMake or bear to use relative paths in compile_commands.json. CMake, for instance, is just not designed to work like that.

But if you know in advance what the absolute path is, then we could take that as an argument and do as you suggest.

[ghutchis]

Just to make sure I'm doing the right transformation, can you give me an example of what basedir should look like after transformation?
(I have a good idea, but want to ensure I don't introduce a subtle break.)

I can try a pull request today.

[ZedThree]

Sure, here's the output from one our recent runs:

Replacing 'Replacing '/home/runner/work/BOUT-dev/BOUT-dev' with '/github/workspace'' with '/github/workspace'

This results from:

• args.build_dir: build
• original_directory: /home/runner/work/BOUT-dev/BOUT-dev/build
• basedir: /home/runner/work/BOUT-dev/BOUT-dev
• newbasedir: /github/workspace

original_directory is the "directory" key for each element in compile_commands.json. Currently, it assumes every file has the same directory -- or at least has the same basedir part that needs replacing.

[bwrsandman]

Here's an entry which causes issues for me:

[
{
  "directory": "/__w/openblack/openblack/build/externals",
  "command": "/usr/bin/clang++  -I/__w/openblack/openblack/externals/imgui -I/__w/openblack/openblack/externals/imgui_user -isystem /usr/include/SDL2 -g -std=gnu++17 -o CMakeFiles/imgui.dir/imgui/imgui.cpp.o -c /__w/openblack/openblack/externals/imgui/imgui.cpp",
  "file": "/__w/openblack/openblack/externals/imgui/imgui.cpp"
}
]

Where build_dir would be "build" and the workspace root is /__w/openblack/openblack

It is like this because there is a sub CMake in externals:

• CMakeLists.txt
• src/CMakeLists.txt
• externals/CMakeLists.txt

So not every directory will be terminating in build_dir, some will have subpaths.

Could you compute relative paths using https://docs.python.org/3/library/pathlib.html#pathlib.PurePath.relative_to ?

[ZedThree]

@bwrsandman That's a very good point about the subpaths. I'm not quite sure how to handle that just yet -- I've not touched this in some time, so I need to remember how it works. I think the issue we're dealing with is that we don't necessarily know what the original directory is. This means that we can't even transform the paths to be relative, because we don't know which parts of the path we need to transform.

Currently, there's some logic to work out what the original directory is, which works by assuming that the first entry in compile_commands.json has a directory which is just /original_directory/build_dir.

One fix would be to add an optional argument allowing the user to specify the original directory. That could default to pwd, assuming we can get that from the github context.

[tanneberger]

@ZedThree So what is the recommended way of handling the error now ?

I currently generate my compile databass with:

cmake . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=1

[ZedThree]

I've got a branch, cmake-commands, that now lets you run CMake inside the Action's docker container. This means we no longer need to fix the paths in compile_commands.json.

@ghutchis @bwrsandman @revol-xut If you'd like to test this for your use case, that would be very helpful!

[bwrsandman]

I can test this within an action on one of my repos. How can I use the cmake-commands branch?

[ZedThree]

Thanks!

You can use it by specifying the branch instead of a tag, and then pass something to the cmake_command argument:

- uses: ZedThree/clang-tidy-review@cmake-command
  with:
    build_dir: build
    cmake_command: cmake . -B build

[bwrsandman]

The issues that I'm running into is that it seems like it's running cmake from within a container.
The version of cmake in your container (3.16) and my project requires 3.20.
Also I have system dependencies such as SDL2 which aren't installed.
I should note that this is new behavior. It wasn't like that before.
Before (with manual fix using sed): https://github.com/openblack/openblack/runs/4218160573?check_suite_focus=true
Now: https://github.com/openblack/openblack/runs/5014007413?check_suite_focus=true

[ZedThree]

Thanks for checking @bwrsandman.

It looks like it'll need a different fix for your project. I can install the latest cmake for the Action, and it's possible to install additional apt packages too, but that won't help if you're installing things not available in the ubuntu version used in the Action.

I'll see if I can get a better fix for your use case

[ZedThree]

@bwrsandman Please could you try the fix-absolute-paths branch? I'm hoping the following will work:

      - name: Create compile_commands.json
        run: |
          git submodule update --init externals/imgui
          cmake . -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=On

      - name: Run clang-tidy
        uses: ZedThree/clang-tidy-review@fix-absolute-paths
        id: review
        with:
          build_dir: build

It's not completely clear to me if GITHUB_WORKSPACE is the current directory for you. If it's not, you can set the base_dir argument manually:

      - name: Run clang-tidy
        uses: ZedThree/clang-tidy-review@fix-absolute-paths
        id: review
        with:
          build_dir: build
          base_dir: $PWD

although the docs don't make it clear if environment variables work in that context. Worst case scenario you have to set it via a separate step:

      - name: Set the value
        id: step_one
        run: |
          echo "cwd=$(pwd)" >> $GITHUB_ENV

      - name: Run clang-tidy
        uses: ZedThree/clang-tidy-review@fix-absolute-paths
        id: review
        with:
          build_dir: build
          base_dir: ${{ env.cwd }}

[bwrsandman]

Started a run here https://github.com/openblack/openblack/runs/5044771511?check_suite_focus=true

[bwrsandman]

The configure seems to have worked but looks like clang-tidy crashed???

[ZedThree]

I fixed a couple of silly bugs. Please try restarting the Action

[ZedThree]

I've confirmed that setting base_dir via an environment variable as below works:

      - name: Set the value
        id: step_one
        run: |
          echo "base_dir=$(pwd)" >> $GITHUB_ENV

      - name: Run clang-tidy
        uses: ZedThree/clang-tidy-review@fix-absolute-paths
        id: review
        with:
          base_dir: ${{ env.base_dir }}

[bwrsandman]

Any idea why clang is crashing here?
https://github.com/openblack/openblack/runs/5102794841?check_suite_focus=true#step:12:38

[ZedThree]

Sorry, yours is one of the cases where this action can't automatically work out the base_dir, so you'll need to set it manually like this:

      - name: Set the base directory for clang-tidy
        run: |
          echo "base_dir=$(pwd)" >> $GITHUB_ENV

      - name: Run clang-tidy
        uses: ZedThree/clang-tidy-review@v0.8.0
        id: review
        with:
          build_dir: build
          base_dir: ${{ env.base_dir }}

This is because you're running in an external container and $(pwd) isn't the same as ${GITHUB_WORKSPACE}, and from inside the Action it seems to be impossible to tell the real absolute path of the workflow running clang-tidy-review.