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

gtest: 1.12.1 -> 1.13.0 #228209

Merged
merged 1 commit into from
May 5, 2023
Merged

Conversation

cbourjau
Copy link
Contributor

Description of changes

Update gtest (googletest) to its latest release (changelog).

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

pkgs/development/libraries/gtest/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/gtest/default.nix Outdated Show resolved Hide resolved
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
@trofi
Copy link
Contributor

trofi commented May 5, 2023

Bisect says 2bb0f80 gtest: 1.12.1 -> 1.13.0 broke protobuf on staging as:

$ nix build --no-link -f. -L protobuf
...
protobuf> /build/source/third_party/googletest/googletest/include/gtest/internal/gtest-port.h:270:2: error: #error C++ versions less than C++14 are not supported.
protobuf>   270 | #error C++ versions less than C++14 are not supported.
protobuf>       |  ^~~~~

Looks like googletest wants at least c++14 while protobuf forces c++11 .

@trofi
Copy link
Contributor

trofi commented May 5, 2023

Probably fixed upstream in protocolbuffers/protobuf@7df94e2

@SuperSandro2000
Copy link
Member

Do we want to add protobuf to gtest passthru.tests?

@cbourjau
Copy link
Contributor Author

cbourjau commented May 8, 2023

Do we want to add protobuf to gtest passthru.tests?

I'm afraid I'm not sure what this entails.

@SuperSandro2000
Copy link
Member

That ofborg will try to build it and we catch regressions early, nothing else.

@vcunat
Copy link
Member

vcunat commented May 10, 2023

I believe we should revert this PR from 23.05. Just one glance at the announcement shows breaking change which caused this issue:

GoogleTest now requires at least C++14

Breaking changes aren't allowed now (and weren't around merge time), as seen in #223562

/cc #231026

@vcunat
Copy link
Member

vcunat commented May 10, 2023

Packages broken by this, for future reference (will be edited):

vcunat added a commit that referenced this pull request May 10, 2023
This reverts commit 3c75a19, reversing
changes made to cfd8765.

Doesn't seem suitable for nixpkgs 23.05 anymore, etc.  See
#228209 (comment)
@vcunat
Copy link
Member

vcunat commented May 11, 2023

I reverted the update. We can merge it again after forking off NixOS 23.05 (scheduled to May 22), though at once with fixes for these known regressions at least.

@cbourjau
Copy link
Contributor Author

I reverted the update. We can merge it again after forking off NixOS 23.05 (scheduled to May 22), though at once with fixes for these known regressions at least.

Is there a reasonable way forward to get gtest up-to-day? Nixpkgs supports several versions of protobuf.Does this mean/is the way forward to also support various versions of gtest for the time being?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants