Skip to content

protobuf: add new versions#36711

Merged
alalazo merged 2 commits intospack:developfrom
adamjstewart:packages/protobuf
Apr 12, 2023
Merged

protobuf: add new versions#36711
alalazo merged 2 commits intospack:developfrom
adamjstewart:packages/protobuf

Conversation

@adamjstewart
Copy link
Copy Markdown
Member

Extracted out of #36263

@adamjstewart
Copy link
Copy Markdown
Member Author

@spackbot run pipeline

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Apr 9, 2023

I've started that pipeline for you!

hyoklee
hyoklee previously approved these changes Apr 9, 2023
Copy link
Copy Markdown
Contributor

@hyoklee hyoklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update!

@adamjstewart
Copy link
Copy Markdown
Member Author

@danlipsa @vicentebolea @kwryankrattiger @jcftang any idea how to resolve the paraview build failures? The latest version of abseil-cpp requires C++14 or newer, so I forced it to use that, but it seems like paraview is still building with older C++? Is there a way to bump that up? I'm surprised it's linking to abseil-cpp at all since it's not listed as a dep.

@vicentebolea
Copy link
Copy Markdown
Member

@danlipsa @vicentebolea @kwryankrattiger @jcftang any idea how to resolve the paraview build failures? The latest version of abseil-cpp requires C++14 or newer, so I forced it to use that, but it seems like paraview is still building with older C++?

It seems that best course of action is to build the paraview pkg with c++14. ParaView does not set the CMAKE_CXX_STANDARD but VTK does it. However the VTK flag VTK_IGNORE_CMAKE_CXX11_CHECKS allows it.

@adamjstewart
Copy link
Copy Markdown
Member Author

Would I just copy the example from abseil-cpp? Or would any paraview maintainers like to do this themselves? Not sure if abseil-cpp should be a direct dependency or if we need to force both to be built with the same standard.

@vicentebolea
Copy link
Copy Markdown
Member

My bad, lets do it differently: how about making ParaView to depend on a older Alseil, we do not need it as for now. Paraview/VTK will eventually default to C++14, maybe next release? At that time we can support newwer Alseil

@adamjstewart
Copy link
Copy Markdown
Member Author

Should it depend on older abseil-cpp or abseil-cpp cxxstd=11? Like if I build with older abseil-cpp cxxstd=14 will that still work?

@vicentebolea
Copy link
Copy Markdown
Member

Should it depend on older abseil-cpp or abseil-cpp cxxstd=11?

#paraview pkg

depends_on(protobuf ^abseil-cpp@:20220623.0) #(it implicity defaults to cxxstd=11)

@adamjstewart
Copy link
Copy Markdown
Member Author

depends_on statements can't contain ^, so I used a conflict instead. See if the latest commit looks good, hopefully CI will pass now.

Copy link
Copy Markdown
Member

@vicentebolea vicentebolea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paraview-wise

@alalazo alalazo merged commit d79423f into spack:develop Apr 12, 2023
@adamjstewart adamjstewart deleted the packages/protobuf branch April 12, 2023 13:06
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Apr 25, 2023
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