svalinn / DAGMC

Direct Accelerated Geometry Monte Carlo Toolkit
https://svalinn.github.io/DAGMC
Other
100 stars 63 forks source link

Windows Build Failure with Gtest #885

Closed ahnaf-tahmid-chowdhury closed 1 year ago

ahnaf-tahmid-chowdhury commented 1 year ago

Issue Description:

Overview: I and @gonuke have encountered a persistent build failure on the Windows platform in our workflow file (#860, #869). Despite updating Gtest to version 1.13.0 and re-running the workflows, the problem persists. Interestingly, the Linux and Mac workflows have been successful with the latest Gtest version.

Affected Workflow:

Expected Behavior: The Windows build should successfully pass, just like the Linux and Mac builds, after updating Gtest to v1.13.0.

Observed Behavior: The Windows build is failing consistently, indicating there might be an issue specific to the Windows environment.

Error Message: The error message encountered during the Windows builds can be found in the workflow log linked above.

Related Logs:

We would appreciate any assistance or insights from the community to identify and resolve this issue so that the Windows build can proceed successfully with Gtest v1.13.0. Thank you for your support!

Tagging: @makeclean (who mentioned the Gtest-related issue).

gonuke commented 1 year ago

Are you testing locally on Windows, or only via our GitHub actions workflows?

My best guess is that something has changed with the GitHub build environment and not the code itself.

ahnaf-tahmid-chowdhury commented 1 year ago

I haven't tested it on my local Windows. The tests were conducted only through our GitHub actions workflows.

Yes, I am also guessing the same thing. I have found a conflicting definitions of the same class which located in two different files.

D:\a\DAGMC\DAGMC\src\gtest\gtest-1.13.0\include\gtest/gtest-assertion-result.h
C:\Miniconda\Library\include\gtest/gtest.h
gonuke commented 1 year ago

The gtest.h headers are installed as part of the yaml-cpp conda package that is present by default. I don't think we need this package, or any of the things that depend upon it, so I am trying to build after removing that package. stay tuned

gonuke commented 1 year ago

More information: The Windows build currently installed yaml-cpp v0.7.0 by default. It is not clear when that changed from the previous version or why:

Notably, the previous version (v0.6.3) used gtest v1.8.0 which is the same as we use. The current version of yaml-cpp uses gtest v1.11.0.

Long term solutions include some combination of:

ahnaf-tahmid-chowdhury commented 1 year ago

The gtest.h headers are installed as part of the yaml-cpp conda package that is present by default. I don't think we need this package, or any of the things that depend upon it, so I am trying to build after removing that package. stay tuned

Amazing!!! It worked!!! I tried various methods, but it turns out that yaml-cpp was the culprit causing the issue. Removing that package made all the difference. Thank you so much for your help. Cheers! 🎉

gonuke commented 1 year ago

I've raised an issue on the yaml-cpp conda-forge feedstock project - we'll see what opinions they have.

ahnaf-tahmid-chowdhury commented 1 year ago

I believe the most robust approach is to remove the yaml-cpp package during testing. This way, we can consistently use our exact version of gtest every time. However, I attempted to prioritize our own header path ahead of the system header paths by modifying the CMake configuration, but unfortunately, it didn't yield the desired results.

I tried the following method:

# Add the include directories for Google Test to the gtest target
target_include_directories(gtest PRIVATE ${GTEST_DIR})
target_include_directories(gtest PRIVATE ${GTEST_DIR}/include)

Despite these changes, the custom header path didn't take precedence as expected. Maybe there are some other ways available.