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

Changed the way the hash is created in hash_inputs() #24

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

Generhr
Copy link
Contributor

@Generhr Generhr commented Aug 24, 2023

I changed the way the hash is created in hash_inputs() because building a project with VSCode and Ninja outputs many thousands of lines of code when running this part of the original code:

    proc = subprocess.Popen(
        co_args,
        stdout=subprocess.PIPE,
        stderr=subprocess.PIPE
    )
    stdout, stderr = proc.communicate()
    if stderr:
        return None

which causes the project to hang indefinately.

I also noticed that stdout and stderr get switched around when compiling with cl.exe vs clang.exe although I can no longer be sure that was the case.

The method now calculates the hash with the time the source file was last modified, the lines within any .clang-tidy config files, and then the args. The last two are more or less the same as the original code although the method I use to find the .clang-tidy is different and requires a new arg to be passed to clang-tidy: --directories_with_clang_tidy=<file1*file2*...>. This can be achieved in CMake with the following code:

cmake_minimum_required(VERSION 3.10)
project(ctcache-example VERSION 1.0)

find_program(CLANG_TIDY_COMMAND clang-tidy)

add_executable(bad01 bad01.cpp)
target_compile_options(bad01 PRIVATE -pedantic;-Wall;-Werror)

if(CLANG_TIDY_COMMAND)
    function(find_all_directories_with_clang_tidy VARIABLE)
        file(GLOB CLANG_TIDY_FILES "${CMAKE_CURRENT_SOURCE_DIR}/.clang-tidy")

        if(CLANG_TIDY_FILES)
            list(APPEND DIRECTORIES_WITH_CLANG_TIDY ${CMAKE_CURRENT_SOURCE_DIR})
        endif()

        file(GLOB ALL_PROJECT_FILES
             LIST_DIRECTORIES true
             "${CMAKE_CURRENT_SOURCE_DIR}/*"
        )

        foreach(DIRECTORY ${ALL_PROJECT_FILES})
            if(IS_DIRECTORY ${DIRECTORY})
                get_filename_component(DIR_NAME ${DIRECTORY} NAME)

                if(NOT DIR_NAME STREQUAL "build")
                    # Check if the directory contains a .clang-tidy file
                    file(GLOB CLANG_TIDY_FILES "${DIRECTORY}/.clang-tidy")

                    # If so, add the directory to `DIRECTORIES_WITH_CLANG_TIDY`
                    if(CLANG_TIDY_FILES)
                        list(APPEND DIRECTORIES_WITH_CLANG_TIDY ${DIRECTORY})
                    endif()
                endif()
            endif()
        endforeach()

        # CMake splits args up with ";" so use "*" to keep all the directories together, folders can't contain "*" in their name so this should be safe
        string(REPLACE ";" "*" DIRECTORIES_WITH_CLANG_TIDY "${DIRECTORIES_WITH_CLANG_TIDY}")

        set(${VARIABLE}
            ${DIRECTORIES_WITH_CLANG_TIDY}
            PARENT_SCOPE
        )
    endfunction()

    find_all_directories_with_clang_tidy(DIRECTORIES_WITH_CLANG_TIDY)

    if(DIRECTORIES_WITH_CLANG_TIDY)
        set_target_properties(bad01 PROPERTIES CXX_CLANG_TIDY ${CLANG_TIDY_COMMAND} --directories_with_clang_tidy=${DIRECTORIES_WITH_CLANG_TIDY})
    else()
        set_target_properties(bad01 PROPERTIES CXX_CLANG_TIDY ${CLANG_TIDY_COMMAND})
    endif()
endif()

I used "*" because (at least on Windows) this character can not be used in a folder name. I have only done limited testing with my hobby projects, there may be other source_file_extensions to consider when identifying the source file clang-tidy is being called on.

Thank you for creating such a cool tool. I spent about a day trying to figure out what the issue was and how to fix it but I'm sure this will save me a great deal of time in the future.

@matus-chochlik matus-chochlik merged commit c45a372 into matus-chochlik:main Aug 25, 2023
@matus-chochlik
Copy link
Owner

thanks for the PR

nicocvn added a commit to nicocvn/ctcache that referenced this pull request Sep 6, 2023
This commit reverses some of the changes introduced by matus-chochlik#24 with the goal to
include the output of Clang preprocessing in the hash computation.

The handling of the compiler arguments is modified to include the `-P` argument
along with `-E`. This removes from the preprocessor output the linemarkers (i.e.
line starting with `#`), which in turn makes it possible to directly use that
output in the hash computation without parsing it line by line (which was one of
the issue in matus-chochlik#24).

The hash computation itself is modified as follows:

- Execute the preprocessor command and add the output to the hash computation.
- Remove the file modification time from the hash computation.

These changes are designed to make the hash depends on any changes in the
translation unit being analyzed.

Closes matus-chochlik#26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants