Wbhart update cxxwrap #191
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…into wbhart-update_cxxwrap
fingolfin
reviewed
Feb 5, 2020
deps/build.jl
Outdated
| @@ -203,8 +203,14 @@ push!(Libdl.DL_LOAD_PATH, joinpath(prefixpath, "lib")) | |||
|
|
|||
| cd(oldwdir) | |||
|
|
|||
| println("here") | |||
There was a problem hiding this comment.
Suggested change
| println("here") |
deps/build.jl
Outdated
| jlcxx_cmake_dir = realpath(joinpath(dirname(pathof(CxxWrap)), "..", "deps", "usr", "lib", "cmake", "JlCxx")) | ||
|
|
||
| #jlcxx_cmake_dir = ENV["JLCXX_DIR"] | ||
|
|
||
| println("jlcxx_cmake_dir = $jlcxx_cmake_dir") |
There was a problem hiding this comment.
I assume we can also get rid of this?
There was a problem hiding this comment.
Yes, we can get rid of the commented line and the println. At least, I think so. We should be going back to whatever was there before, I believe, and that would seem to do it.
|
On Wed, Feb 05, 2020 at 06:00:17AM -0800, Max Horn wrote:
fingolfin commented on this pull request.
Seems plausible though before we switch to CxxWrap 0.9, we should
coordinate with Polymake.jl / @kalmarek to avoid annoyances.
We need to - polymake does not work with 0.9. They also have a patch or
many waiting. It needs to happen at the same time...
> @@ -15,8 +15,7 @@ BinaryProvider = "b99e7846-7c00-51b0-8f62-c81ae34c0232"
[compat]
AbstractAlgebra = "^0.8.0"
Nemo = "^0.16.0"
-julia = "1.0, 1.1, 1.2"
-CxxWrap = "^0.8.1"
+julia = "≥ 1.0.0"
The commit with this change says "Add dependency on 0.9.0 of CxxWrap" but that's not what happens here. Suggestion:
```suggestion
julia = "≥ 1.0.0"
CxxWrap = "^0.9"
```
Yep. This was an attempt to get it to finally actually use 0.9 - I ran
out of patience about the absolute denial to acknowledge 0.9
> @@ -203,8 +203,14 @@ push!(Libdl.DL_LOAD_PATH, joinpath(prefixpath, "lib"))
cd(oldwdir)
+println("here")
```suggestion
```
> jlcxx_cmake_dir = realpath(joinpath(dirname(pathof(CxxWrap)), "..", "deps", "usr", "lib", "cmake", "JlCxx"))
+#jlcxx_cmake_dir = ENV["JLCXX_DIR"]
+
+println("jlcxx_cmake_dir = $jlcxx_cmake_dir")
I assume we can also get rid of this?
Honestly: no idea. I do not understand why this is neccessary. The web
seems to suggest that this should/could be handled different in cmake.
It works.
clearly the prints can be removed.
…
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#191 (review)
|
Codecov Report
@@ Coverage Diff @@
## master #191 +/- ##
===========================================
+ Coverage 0% 77.62% +77.62%
===========================================
Files 37 37
Lines 3169 3110 -59
===========================================
+ Hits 0 2414 +2414
+ Misses 3169 696 -2473
Continue to review full report at Codecov.
|
fingolfin
force-pushed
the
wbhart-update_cxxwrap
branch
from
636e2b8
to
6634c7f
Compare
|
You should also update the version of Singular to 0.2.0 as this is technically breaking (for Oscar). That would make this independent of the Polymake PR. |
wbhart
suggested changes
Feb 5, 2020
deps/build.jl
Outdated
| @@ -203,8 +203,14 @@ push!(Libdl.DL_LOAD_PATH, joinpath(prefixpath, "lib")) | |||
|
|
|||
| cd(oldwdir) | |||
|
|
|||
| println("here") | |||
deps/build.jl
Outdated
| jlcxx_cmake_dir = realpath(joinpath(dirname(pathof(CxxWrap)), "..", "deps", "usr", "lib", "cmake", "JlCxx")) | ||
|
|
||
| #jlcxx_cmake_dir = ENV["JLCXX_DIR"] | ||
|
|
||
| println("jlcxx_cmake_dir = $jlcxx_cmake_dir") |
There was a problem hiding this comment.
Yes, we can get rid of the commented line and the println. At least, I think so. We should be going back to whatever was there before, I believe, and that would seem to do it.
|
I don't see where the changes mentioned at the bottom of this thread: have been made. |
|
Also, there are a lot of CI failures which look real. |
|
what is the ETA on this? |
…em/Singular.jl into wbhart-update_cxxwrap
|
On Wed, Feb 12, 2020 at 01:04:23PM -0800, kalmarek wrote:
what is the ETA on this?
no idea - the tests are bizarrly working on MacOS, but not on linux. Its
failing with syntatical errors...
…
--
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#191 (comment)
|
|
Linux and MacOS don't share the compiler; |
gcc 7 has better support for C++17 (needed for cxxwrap 0.9.0)
update travis dist to ubuntu bionic for gcc 7
wbhart
approved these changes
Feb 20, 2020
fingolfin
pushed a commit
to fingolfin/Singular.jl
that referenced
this pull request
Jun 6, 2023
…xxwrap Wbhart update cxxwrap
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
should be merged with other pull request - but I lack to git-fu