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.

Sign up for GitHub

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

Jump to bottom

Update GTEST to fix gcc 11.2 issues #914

Closed
wants to merge 1 commit into from
Closed

Update GTEST to fix gcc 11.2 issues #914

wants to merge 1 commit into from

Conversation

MarcelKoch
Copy link
Member

The current GTest version is incompatible with gcc 11.2, which has already been mentioned somewhere, see also [1].
Since GTest now also follows abseil's live at head philosophy (see [2]), they advise using the latest commit in master.
If we decide against that, we should update to the latest release v1.11.0 with e2239ee6043f73722e7aa812a459f54a28552929.

@MarcelKoch
Update GTEST to fix gcc 11.2 issues
d148c73 
The current GTest version is incompatible with gcc 11.2, which has already been mentioned somewhere, see also [2](google/googletest#3219).
Since GTest now also follows abseil's live at head philosophy (see [2](https://abseil.io/about/philosophy#upgrade-support)), they advise using the latest commit in master. 
If we decide against that, we should update to the latest release v1.11.0 with `e2239ee6043f73722e7aa812a459f54a28552929`.
@MarcelKoch MarcelKoch self-assigned this Oct 26, 2021
@MarcelKoch MarcelKoch requested a review from a team October 26, 2021 12:13
@ginkgo-bot ginkgo-bot added the reg:build This is related to the build system. label Oct 26, 2021
@yhmtsai
Copy link
Member

yhmtsai commented Oct 26, 2021

The issue may not be solved in MSVC google/googletest#3311
we encounter this issue when we update gtest with ginkgo 1.4 release

@tcojean
Copy link
Member

tcojean commented Oct 26, 2021 •
edited
Loading

No matter what they say, I'm not sure if I would trust them as we also use GTest in many environments which have proven to create several issues in the past (CUDA, MSVC, and other compilers). I'm fine with updating to the latest version, or the SHA of the current master if it works as expected, though.

@Slaedr
Copy link
Contributor

Slaedr commented Oct 26, 2021

If it works right now, perhaps we could give "live at head" another shot, I'm for that. I'm don't know what can be done about the MSVC issue though. Also, the GTest Github page says they'll soon be adding a dependency on Abseil. I guess that means we'll sooner or later be dependent on it too.

@tcojean
Copy link
Member

tcojean commented Oct 26, 2021

As Mike says, as recently as 2 months ago I wasn't able to bump GTest to a more recent version due to the MSVC issues.
#857

@MarcelKoch
Copy link
Member Author

@yhmtsai @tcojean seems like the bug is still there. I guess that means we still can't update.

@MarcelKoch MarcelKoch closed this Oct 26, 2021
@yhmtsai
Copy link
Member

yhmtsai commented Oct 27, 2021

To keep all things together.
I have created a minimal reproducer for this kind of issue, but I do not know GTest or MSVC will fix this issue.
google/googletest#3631
The quick workaround for us is changing the ranlux generator to different one.

@upsj
Copy link
Member

upsj commented Oct 27, 2021

What is the reason for using ranlux and not default_random_engine anyways?

@yhmtsai
Copy link
Member

yhmtsai commented Oct 27, 2021

No idea, I use ranlux as copy-paste from some existing ginkgo code

@thoasm
Copy link
Member

thoasm commented Oct 27, 2021

Same for me, I don't know the reason why we use randlux specifically. Maybe it is to ensure reproducibility on different platforms (which is not given with default_random_engine)?

@upsj
Copy link
Member

upsj commented Oct 27, 2021

Not sure we should expect any reproducibility qualities from the standard library RNGs, see https://codingnest.com/generating-random-numbers-using-c-standard-library-the-problems/#distributionsarenonportable

@ginkgo-project ginkgo-project deleted a comment Oct 27, 2021
@ginkgo-project ginkgo-project deleted a comment Oct 27, 2021
@ginkgo-project ginkgo-project deleted a comment Oct 27, 2021
@yhmtsai yhmtsai mentioned this pull request Jan 24, 2022
Merged
yhmtsai added a commit that referenced this pull request Feb 7, 2022
@yhmtsai
Merge Change random number generator, Update gtest to v1.11.0
727ce4f 
This PR changes ranlux generator to default generator to avoid MSVC SFIANE issue with gtest.
Update Gtest to release v1.11.0, which also fixes the gcc 11.2 issue as #914 mentioned

Related PR: #954
@tcojean tcojean deleted the gtest-use-master branch February 16, 2022 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@MarcelKoch MarcelKoch

Labels
reg:build This is related to the build system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@MarcelKoch @yhmtsai @tcojean @Slaedr @upsj @thoasm @ginkgo-bot