-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-37266: [CI][C++] Avoid clobbering build type with CMAKE_ARGS #37269
Conversation
Since CMAKE_ARGS could in some circumstances (conda-forge) override CMAKE_BUILD_TYPE, pass it first and let our own choice override it.
|
@github-actions crossbow submit -g cpp |
Revision: f883af5 Submitted crossbow builds: ursacomputing/crossbow @ actions-2927577a55 |
@github-actions crossbow submit conda |
Revision: f883af5 Submitted crossbow builds: ursacomputing/crossbow @ actions-fcac70e6a6 |
@kou Does this look ok? |
Can we use our environment variable such as |
I suspect it might be necessary to set up the correct toolchain and options (such as |
I've tried your solution in #37272 |
It seems like the test-conda-cpp-valgrind failure could be fixed by bumping xxhash (see #37273). At least locally that works. |
If conda recommends using |
Not sure where best to answer across 3 different threads. Yes, we set a bunch of things in I don't understand why the CI for conda builds here ever would need to build non-Release builds, but either ordering CMAKE_ARGS to the beginning (as this PR does) or renaming it should be fine. Does that answer the questions? |
It does, but what should be the preffered solution? Should we use conda's CMAKE_ARGS? |
For conda builds (using conda compilers, with those activation scripts), generally yes, unless you do the (minimum) equivalent bits of the activation scripts yourself (e.g. setting certain paths to binaries). But from my POV, you're not obligated to do it in any particular way in your own CI, and should go with whatever is most understandable / maintainable. I also still don't understand why you're doing debug builds with conda, but assuming that as a given, I'd say it's probably less surprising to rename the respective variable, i.e. follow #37272. |
Why not? We're using conda as a package manager, we don't generate conda packages. |
Sure. As you found out, the compiler stack that comes with that is more geared towards the needs of conda-forge. You can also strip the release flag out of I'm happy to help here, though in this case I don't see an approach that's obviously the best. It's a kind of "off-label" usage, and no variant is perfect. |
Ok, so let's use |
Since CMAKE_ARGS could in some circumstances (conda-forge) override CMAKE_BUILD_TYPE, pass it first and let our own choice override it.