Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#22814 closed enhancement (fixed)

Update cmake to 3.8.0

Reported by: Thierry Monteil Owned by:
Priority: major Milestone: sage-8.0
Component: packages: experimental Keywords: days86, sdl
Cc: Isuru Fernando, François Bissey Merged in:
Authors: Thierry Monteil Reviewers: Travis Scrimshaw, François Bissey
Report Upstream: N/A Work issues:
Branch: 8471784 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

Change History (34)

comment:1 Changed 6 years ago by Thierry Monteil

Branch: u/tmonteil/update_cmake_to_3_8_0

comment:2 Changed 6 years ago by Thierry Monteil

Cc: Isuru Fernando François Bissey added
Commit: 4ded8e24cfa768a309d0c122dda0b2eb394a8eaa
Keywords: days86 added
Status: newneeds_review

With 3.2.3:

  • on 64bit, the build was OK and self-tests lead to 1 error (CTestTestUpload)
  • on 32bit, the build failed at configure time

With 3.8.0:

  • on 64bit, the build is OK and self-tests lead to 2 errors (CTestTestUpload and RunCMake.ctest_submit), but the second test did not exist on 3.2.3.
  • on 32bit, the build is OK and 7 self-tests failed out of 442 (98% tests passed)

So, regarding GNU/Linux, the state of this package is strictly better than before.

However, i had to remove the osx10.10.patch patch since it does not apply anymore. So, if you have access to such machines, please have a look to OSX builds.


New commits:

4ded8e2#22814 : update cmake to 3.8.0

comment:3 Changed 6 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM.

comment:4 in reply to:  3 Changed 6 years ago by Thierry Monteil

Replying to tscrim:

LGTM.

Does it compile/test well on OSX ?

comment:5 Changed 6 years ago by François Bissey

Tested on OS X on top of #12426, tests returned 3 errors:

        Start 190: CTestTestUpload
190/445 Test #190: CTestTestUpload ......................................***Failed  Required regular expression not found.Regex=[Upload\.xml
] 78.11 sec
        Start 349: RunCMake.ctest_submit
349/445 Test #349: RunCMake.ctest_submit ................................***Failed  388.42 sec
        Start 383: RunCMake.Framework
383/445 Test #383: RunCMake.Framework ...................................***Failed   11.32 sec

Details [CTestTestUpload]

[ERROR_MESSAGE]
   Error when uploading file: /Users/fbissey/build/sage-clang/local/var/tmp/sage/build/cmake-3.8.0/src/Tests/CTestTestUpload/Testing/20170415-2318/Build.xml

   Error message was: Failed to connect to 192.0.2.0 port 5187: Operation timed out
   Problems when submitting via HTTP

[RunCMake.ctest_submit]

[ERROR_MESSAGE]
   Error when uploading file: /Users/fbissey/build/sage-clang/local/var/tmp/sage/build/cmake-3.8.0/src/Tests/RunCMake/ctest_submit/FailDrop-https-build/Testing/20170415-2$

   Error message was: Failed to connect to 192.0.2.0 port 5187: Operation timed out
   Problems when submitting via HTTP

Looks like a pattern... [RunCMake.Framework]: OS X specific, it looks like multiple errors trying to build stuff for iDevices...

comment:6 Changed 6 years ago by François Bissey

I'd say the OS X failure in framework is because we set MACOSX_DEPLOYMENT_TARGET in sage-env. When cmake tries to build executable for iOS there is an option on the command line that tells it to produce OS X stuff (x86_64) instead. Boom when you try to link.

comment:7 Changed 6 years ago by François Bissey

Yup, confirmed. Unsetting MACOSX_DEPLOYMENT_TARGET make the framework test pass. Only those two tests that probably try to contact to an internal kitware server are now failing.

comment:8 Changed 6 years ago by git

Commit: 4ded8e24cfa768a309d0c122dda0b2eb394a8eaaeb3ae47afee781ff2e9de249baeabab07d26fa6a
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

eb3ae47#22814 : unset MACOSX_DEPLOYMENT_TARGET.

comment:9 in reply to:  7 Changed 6 years ago by Thierry Monteil

Replying to fbissey:

Yup, confirmed. Unsetting MACOSX_DEPLOYMENT_TARGET make the framework test pass. Only those two tests that probably try to contact to an internal kitware server are now failing.

Thanks fot the investigation, does the last commit fixes that issue ?

comment:10 Changed 6 years ago by François Bissey

Not what I have done or my recommendation. I am also working on silencing the other two tests. I am acting only on spkg-check

#!/usr/bin/env bash

cd src
unset MACOSX_DEPLOYMENT_TARGET
ctest -E "(CTestTestStopTime|TestUpload)"

That made the framework test pass and silenced the TestUpload one. Gentoo silence the TestUpload one too amongst other. I still have to do something about ctest_submit. Have to find the right command.

Last edited 6 years ago by François Bissey (previous) (diff)

comment:11 Changed 6 years ago by François Bissey

That should take care of everything on 64bit. Cannot do anything about the 32bit tests without knowing more about them

#!/usr/bin/env bash

cd src
unset MACOSX_DEPLOYMENT_TARGET
ctest --extra-verbose --output-on-failure -E "(CTestTestStopTime|TestUpload|ctest_submit)"

And I like things to be verbose, that makes debugging so much easier.

comment:12 Changed 6 years ago by git

Commit: eb3ae47afee781ff2e9de249baeabab07d26fa6a96cae4e257a59bba9a6f6fcb46a20cb715bec6ad

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

96cae4e#22814 : make the self tests pass + fix dependencies

comment:13 Changed 6 years ago by Thierry Monteil

Thanks for the hints, the tests now pass on 64bit arch. I took the opportunity to add a missing dependencies file. Regarding the 32bit issues, i might have some problems with my current VM (see #22782 that i seem to be the only one to produce), so i will rebuild a fresh one to only get the meaningful errors.

comment:14 in reply to:  2 ; Changed 6 years ago by Isuru Fernando

Replying to tmonteil:

However, i had to remove the osx10.10.patch patch since it does not apply anymore. So, if you have access to such machines, please have a look to OSX builds.

Since sage is changing the compiler to clang on OSX, the patch is not needed at all.

Replying to tmonteil:

Thanks for the hints, the tests now pass on 64bit arch. I took the opportunity to add a missing dependencies file.

I see that curl is a sage package now. Since it was not when the previous cmake version was added, I added logic such that curl was used from OSX system and it was built from source on Linux (cmake has a bundled curl). Can you change the spkg-install such that cmake finds sage's curl package?

comment:15 in reply to:  14 ; Changed 6 years ago by François Bissey

Reviewers: Travis ScrimshawTravis Scrimshaw, François Bissey
Status: needs_reviewpositive_review

Replying to isuruf:

Replying to tmonteil:

However, i had to remove the osx10.10.patch patch since it does not apply anymore. So, if you have access to such machines, please have a look to OSX builds.

Since sage is changing the compiler to clang on OSX, the patch is not needed at all.

Replying to tmonteil:

Thanks for the hints, the tests now pass on 64bit arch. I took the opportunity to add a missing dependencies file.

I see that curl is a sage package now. Since it was not when the previous cmake version was added, I added logic such that curl was used from OSX system and it was built from source on Linux (cmake has a bundled curl). Can you change the spkg-install such that cmake finds sage's curl package?

Thierry did put curl in the dependencies, so it is taken care of. I am adding myself as a reviewer and put it back to positive review.

comment:16 in reply to:  15 Changed 6 years ago by Isuru Fernando

Replying to fbissey:

Thierry did put curl in the dependencies, so it is taken care of. I am adding myself as a reviewer and put it back to positive review.

Yes, but on Linux, cmake builds the bundled curl right?

comment:17 Changed 6 years ago by François Bissey

Status: positive_reviewneeds_work

You are right spkg-install needs cleaning up. The whole section for using or not system curl needs updating, most of it will have to go and be simplified. Back to need_works.

comment:18 Changed 6 years ago by François Bissey

I was trying to find the origin of the 192.0.2.0 address in the cmake test suite. It is not in the source code. cmake does something weird with its test suite. At some point it links one of the sub-folders in the Tests to the Applications folder on the mac. In my case one of the App in Applications is libreoffice. That weird address is found inside libreoffice's python code....

This is just bizarre.

I'll clean spkg-install there is other stuff in there like MACOSX_DEPLOYMENT_TARGET that may have been an early attempt at fixing some stuff (or needed to be used with the patch) that has no business here.

comment:19 Changed 6 years ago by François Bissey

Branch: u/tmonteil/update_cmake_to_3_8_0u/fbissey/update_cmake_to_3_8_0
Commit: 96cae4e257a59bba9a6f6fcb46a20cb715bec6ad1d18a976daf3a1bba374cb69d08e44b390924620

Isuru, can you double check my changes, add your name to the reviewers and put it to positive review?


New commits:

1d18a97Simplify spkg-install and make it always use a system (sage) provided curl

comment:20 Changed 6 years ago by François Bissey

Status: needs_workneeds_review

comment:21 Changed 6 years ago by Thierry Monteil

Sorry for the delay, i had the exact same commit, so it is fine for me, except that i let the unset MACOSX_DEPLOYMENT_TARGET, since i am not sure whether it is still useful.

Also, looking at the bootstrap script, i was wondering whether we should add xz as a dependency and add the --system-liblzma option. Does this makes sense ?

Last edited 6 years ago by Thierry Monteil (previous) (diff)

comment:22 Changed 6 years ago by François Bissey

If we have it (and I guess it is a requirement for R nowadays) then yes go ahead.

I tested the removal of unset MACOSX_DEPLOYMENT_TARGET and I didn't observe any problem nor did I expect any. Further if we want to take cmake from experimental to optional and may be even standard in time, then it has to go. We cannot make portable binaries if it is unset.

comment:23 in reply to:  22 Changed 6 years ago by Thierry Monteil

Replying to fbissey:

If we have it (and I guess it is a requirement for R nowadays) then yes go ahead.

OK, xz is standard spkg, i am trying this right now.

I tested the removal of unset MACOSX_DEPLOYMENT_TARGET and I didn't observe any problem nor did I expect any. Further if we want to take cmake from experimental to optional and may be even standard in time, then it has to go. We cannot make portable binaries if it is unset.

OK, great, iirc cgal (still to be packaged) and csympy rely on cmake, so it might be interesting in a near future.

comment:24 Changed 6 years ago by Thierry Monteil

Branch: u/fbissey/update_cmake_to_3_8_0u/tmonteil/update_cmake_to_3_8_0

comment:25 Changed 6 years ago by git

Commit: 1d18a976daf3a1bba374cb69d08e44b3909246208471784d3d245e41405cc03ffa235f9238f2bdb1

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8471784Use xz/liblzma provided by Sage

comment:26 Changed 6 years ago by Thierry Monteil

Tests still pass with Sage's xz.

comment:27 Changed 6 years ago by François Bissey

Good, and presumably

./sage -sh
ldd -r local/bin/cmake

Doesn't report anything out of place.

comment:28 Changed 6 years ago by Thierry Monteil

I got:

	linux-vdso.so.1 (0x00007ffddb538000)
	libz.so.1 => /opt/sagemath/sage-source/local/lib/libz.so.1 (0x00007f814f4e4000)
	liblzma.so.5 => /opt/sagemath/sage-source/local/lib/liblzma.so.5 (0x00007f814f2bf000)
	libbz2.so.1 => /opt/sagemath/sage-source/local/lib/libbz2.so.1 (0x00007f814f0af000)
	libcurl.so.4 => /opt/sagemath/sage-source/local/lib/libcurl.so.4 (0x00007f814ee48000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f814ec2b000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f814ea27000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f814e81f000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f814e514000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f814e213000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f814dffd000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f814dc52000)
	libssl.so.1.0.0 => /usr/lib/x86_64-linux-gnu/libssl.so.1.0.0 (0x00007f814d9f1000)
	libcrypto.so.1.0.0 => /usr/lib/x86_64-linux-gnu/libcrypto.so.1.0.0 (0x00007f814d5f5000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f814f6fe000)

comment:29 Changed 6 years ago by François Bissey

Status: needs_reviewpositive_review

Looks all right to me. Let's move to the next stage.

comment:30 Changed 6 years ago by Volker Braun

Branch: u/tmonteil/update_cmake_to_3_8_08471784d3d245e41405cc03ffa235f9238f2bdb1
Resolution: fixed
Status: positive_reviewclosed

comment:31 Changed 6 years ago by Dima Pasechnik

Commit: 8471784d3d245e41405cc03ffa235f9238f2bdb1

It perhaps makes sense to immediately go to cmake 3.8.1, released 2 weeks ago.

comment:32 Changed 6 years ago by François Bissey

Well this ticket is already merged (8.0.beta5) so it would have to go in a new ticket. But sure let's do that if you also want to push it optional.

comment:33 in reply to:  32 Changed 6 years ago by Dima Pasechnik

Replying to fbissey:

Well this ticket is already merged (8.0.beta5) so it would have to go in a new ticket. But sure let's do that if you also want to push it optional.

OK, this is now #22999, and yes, let us make it optional package, indeed.

comment:34 Changed 3 years ago by Thierry Monteil

Keywords: sdl added
Note: See TracTickets for help on using tickets.