Merged #1325 into master.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1325#event-3718116926___
Rpm-maint mailing list
@pmatilai approved this pull request.
Works as promised AFAICT
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
@pmatilai Updated, please review.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1325#issuecomment-684947396___
Rpm-maint
@dmnks pushed 2 commits.
82c5af992ed87bb6665de2d382166a563cc7b398 Check for OpenMP version at configure
time
c3af4801917c6cf3d5b5153a02f4cc09f98d6ca2 Bump Lua to 5.2 in configure script
--
You are receiving this because you are subscribed to this thread.
View it on GitHub:
> The right thing to do with an incompatible OpenMP is to silently disable
> OpenMP unless explicitly requested by --enable-openmp. Whether it's worth the
> trouble is a separate question, writing configure.ac logic is ... yeah. No
> cute kittens will be harmed if we set the default to "yes"
Optimally, ./configure without specific arguments is supposed to just figure
out a configuration that will work on this system, and only error out if it
cannot do that - or is explicitly requested to do incompatible things.
The right thing to do is to silently disable OpenMP unless explicitly
@pmatilai Coming back to this PR after a while, I wonder if silently disabling
OpenMP (if the required version isn't available) is really what we want.
Wouldn't it be better to just fail and let the user disable OpenMP explicitly
with `--disable-openmp` if he/she decides so?
--
You are
Thanks, will tweak the PR accordingly. And yeah, I agree otherwise.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
Took more than one night of sleeping over, but here's what I think:
Threading support in general is completely optional, and lack of it doesn't
deprive anybody of any functionality, just a little bit of time. This is stark
contrast to, say, Lua support. So I think we can easily afford to demand
@Conan-Kudo commented on this pull request.
> @@ -761,9 +778,9 @@ AC_ARG_WITH([lua], [AS_HELP_STRING([--with-lua], [build
> with lua support])],
AS_IF([test "$with_lua" != no],[
PKG_CHECK_MODULES([LUA],
-[lua >= 5.1],
+[lua >= 5.2],
Hah, I thought we actually dropped Lua 5.2
@dmnks commented on this pull request.
> @@ -761,9 +778,9 @@ AC_ARG_WITH([lua], [AS_HELP_STRING([--with-lua], [build
> with lua support])],
AS_IF([test "$with_lua" != no],[
PKG_CHECK_MODULES([LUA],
-[lua >= 5.1],
+[lua >= 5.2],
Heh, it's funny how easy is to misread the subject
> Sometimes it's better to test for specifics features, sometimes for versions.
> I don't know how the OpenMP landscape looks like, but sometimes
> implementations only support a subset of a newer standard in which case
> testing for specific features is the friendlier way.
My impression after
Just for a data point, OpenMP 4.5 is supported in clang >= 7 and gcc >= 6, both
from 2018. Which is brand new software barely off its wrappings in terms of rpm
software requirements :smile:
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or
> A more user-friendly way of dealing with this would actually be the opposite,
> i.e. making the use of the priority keyword conditional at preprocessing,
> based on the detected OpenMP version (which is trivial to do as shown in the
> patch) because as you say, all that the keyword really
@pmatilai commented on this pull request.
> @@ -761,9 +778,9 @@ AC_ARG_WITH([lua], [AS_HELP_STRING([--with-lua], [build
> with lua support])],
AS_IF([test "$with_lua" != no],[
PKG_CHECK_MODULES([LUA],
-[lua >= 5.1],
+[lua >= 5.2],
Because 5.2 is the actually required minimum?
@Conan-Kudo requested changes on this pull request.
> @@ -761,9 +778,9 @@ AC_ARG_WITH([lua], [AS_HELP_STRING([--with-lua], [build
> with lua support])],
AS_IF([test "$with_lua" != no],[
PKG_CHECK_MODULES([LUA],
-[lua >= 5.1],
+[lua >= 5.2],
Why aren't we just bumping to Lua 5.3?
That's a very valid point and one that I didn't consider, honestly.
A more user-friendly way of dealing with this would actually be the opposite,
i.e. making the use of the priority keyword conditional at preprocessing, based
on the detected OpenMP version (which is trivial to do as shown in
Looks okay as such, I guess the question here is whether it's reasonable to
disable all openmp-stuff if "priority" is not supported - that's required by
one place only, and even that's just an optimization that doesn't actually
affect correctness (AIUI). On one hand this seems a bit harsh, on
You can view, comment on, or merge this pull request online at:
https://github.com/rpm-software-management/rpm/pull/1325
-- Commit Summary --
* Check document need for correct OpenMP version
* Bump Lua to 5.2 in configure script
-- File Changes --
M INSTALL (6)
M configure.ac
19 matches
Mail list logo
