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
{math} [intel/2018b] gap/4.9.3 #7040
Conversation
Test report by @wpoely86 |
|
||
# This tries to install all of the GAP packages. If one fails because | ||
# of missing dependencies, it's skipped automatically | ||
preinstallopts = 'cd pkg && ../bin/BuildPackages.sh && cd .. #' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wpoely86 Please use skipsteps = ['install']
to skip the installation steps, and do the above via buildopts
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be run after the build_step
is done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, yes, you can do that via
buildopts = " && cd pkg && ../bin/BuildPackages.sh"
(the cd ..
at the end is pointless)
It's the trailing #
that I don't like here, this falls in the "hacking" category for me. ;-)
postinstallcmds = ["cd bin && ln -s gap.sh gap && cd -"] | ||
|
||
sanity_check_paths = { | ||
'files': ['bin/gap.sh', 'gap', 'gac'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be bin/gap
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, the real file is gap.sh
. With the postinstallcmds
I add a symlink gap
-> gap.sh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you check for bin/gap
, you make sure the symlink is effectively there, and that it points to an existing file. So why not check both?
# of missing dependencies, it's skipped automatically | ||
preinstallopts = 'cd pkg && ../bin/BuildPackages.sh && cd .. #' | ||
|
||
postinstallcmds = ["cd bin && ln -s gap.sh gap && cd -"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can drop the cd -
, since this command is run in a subshell
|
||
sanity_check_paths = { | ||
'files': ['bin/gap.sh', 'gap', 'gac'], | ||
'dirs': ['pkg'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more specific checks for particular packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a very long list and I don't know if any are more important then others.
And this comes directly from the tarball. It's not because it's there that it will work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fine then :)
toolchain = {'name': 'intel', 'version': '2018b'} | ||
|
||
source_urls = ['http://cgm.cs.mcgill.ca/~avis/C/lrslib/archive/'] | ||
sources = ['%%(name)s-0%s.tar.gz' % version.replace('.', '')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use %(name)s-0%(version_major)s%(version_minor)s.tar.gz
- g++ -Wall -Wno-write-strings -Wno-sign-compare -Wno-unused-variable -I${BOOSTINC} -L${BOOSTLIB} -Wl,-rpath=${BOOSTLIB} -O3 -DPLRS -o plrsmp plrs.cpp lrslib.c lrsmp.c -lboost_thread -lboost_system | ||
+ $(CXX) $(CXXFLAGS) -DGMP -Wall -Wno-write-strings -Wno-sign-compare -I${BOOSTINC} -O3 -DPLRS -DGMP -o plrs plrs.cpp lrslib.c lrsgmp.c -L${BOOSTLIB} -lboost_thread -lboost_system -lgmp | ||
+ $(CXX) $(CXXFLAGS) -Wall -Wno-write-strings -Wno-sign-compare -Wno-unused-variable -I${BOOSTINC} -L${BOOSTLIB} -O3 -DPLRS -DLRSLONG -o plrs1 plrs.cpp lrslib.c lrslong.c -lboost_thread -lboost_system | ||
+ $(CXX) $(CXXFLAGS) -Wall -Wno-write-strings -Wno-sign-compare -Wno-unused-variable -I${BOOSTINC} -L${BOOSTLIB} -O3 -DPLRS -o plrsmp plrs.cpp lrslib.c lrsmp.c -lboost_thread -lboost_system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wpoely86 Remove the hardcoded -O3
? Same below...
|
||
fourier: fourier.c lrslib.h lrslib.c lrsgmp.h lrsgmp.c | ||
- $(CC) $(CFLAGS) -O3 -DTIMES -DSIGNALS -DGMP -I${INCLUDEDIR} fourier.c lrslib.c lrsgmp.c -L${LIBDIR} -lgmp -o fourier | ||
+ $(CC) $(CFLAGS) -O3 -DTIMES -DSIGNALS -DGMP -I${INCLUDEDIR} fourier.c lrslib.c lrsgmp.c -L${LIBDIR} $(LDFLAGS) -lgmp -o fourier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wpoely86 More hardcoded -O3
to remove?
dependencies = [('GMP', '6.1.2')] | ||
|
||
sanity_check_paths = { | ||
'files': ['bin/normaliz', 'bin/Qnormaliz', 'lib/libnormaliz.so', 'lib/libQnormaliz.so'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wpoely86 Please use SHLIB_EXT
rather than hardcoding .so
sanity_check_paths = {
'files': ['bin/normaliz', 'bin/Qnormaliz', 'lib/libnormaliz.%s' % SHLIB_EXT, 'lib/libQnormaliz.%s' % SHLIB_EXT],
'dirs': ['include/libnormaliz']
}
Test report by @wpoely86 |
Test report by @wpoely86 |
Test report by @wpoely86 |
Test report by @wpoely86 |
@wpoely86 You should skip the |
Test report by @wpoely86 |
Test report by @wpoely86 |
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
@wpoely86 Missing dependency on
|
Test report by @wpoely86 |
@boegel can you retry? |
Test report by @boegel |
Test report by @boegel |
Test report by @boegel |
Going in, thanks @wpoely86! |
No description provided.