Thread: [Rdkit-devel] [PATCH 1/2] Do not use absolute paths on RDKit_*Dir variables.
Open-Source Cheminformatics and Machine Learning
Brought to you by:
glandrum
|
From: Gianluca S. <gi...@gm...> - 2010-07-11 23:18:11
|
This allows installation in arbitrary locations by using:
make install DESTDIR=path
Use LIB_SUFFIX to properly install libs in 64bit systems
Remove unused RDKit_PythonDir
---
CMakeLists.txt | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8183c27..4590e3c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -20,9 +20,8 @@ option(RDK_BUILD_COMPRESSED_SUPPLIERS "build in support for compressed MolSuppli
# Config variables:
set(RDKit_CodeDir "${CMAKE_SOURCE_DIR}/Code")
set(RDKit_ExternalDir "${CMAKE_SOURCE_DIR}/External")
-set(RDKit_BinDir "${CMAKE_SOURCE_DIR}/bin")
-set(RDKit_LibDir "${CMAKE_SOURCE_DIR}/lib")
-set(RDKit_PythonDir "${CMAKE_SOURCE_DIR}/rdkit")
+set(RDKit_BinDir "bin")
+set(RDKit_LibDir "lib${LIB_SUFFIX}")
# defines macros: rdkit_python_extension, rdkit_test
include(RDKitUtils)
--
1.7.1.1
|
|
From: Gianluca S. <gi...@gm...> - 2010-07-11 23:18:17
|
---
Code/cmake/Modules/RDKitUtils.cmake | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Code/cmake/Modules/RDKitUtils.cmake b/Code/cmake/Modules/RDKitUtils.cmake
index 3bfcd6f..f2dcbe6 100644
--- a/Code/cmake/Modules/RDKitUtils.cmake
+++ b/Code/cmake/Modules/RDKitUtils.cmake
@@ -17,7 +17,7 @@ macro(rdkit_library)
#IF(RDKLIB_SHARED)
add_library(${RDKLIB_NAME} SHARED ${RDKLIB_SOURCES})
INSTALL(TARGETS ${RDKLIB_NAME}
- DESTINATION ${RDKit_BinDir}/${RDKLIB_DEST})
+ DESTINATION ${RDKit_LibDir}/${RDKLIB_DEST})
#ELSE(RDKLIB_SHARED)
# add_library(${RDKLIB_NAME} ${RDKLIB_SOURCES})
# INSTALL(TARGETS ${RDKLIB_NAME}
--
1.7.1.1
|
|
From: Greg L. <gre...@gm...> - 2010-07-12 05:38:18
|
Dear Gianluca, Thanks for these patches! I created a branch in svn for testing the modifications; https://rdkit.svn.sourceforge.net/svnroot/rdkit/branches/Packaging_12July2010 I haven't yet done much testing of the installation changes, but I can confirm that the code at least still builds on windows and linux. Best Regards, -greg On Mon, Jul 12, 2010 at 1:17 AM, Gianluca Sforna <gi...@gm...> wrote: > --- > Code/cmake/Modules/RDKitUtils.cmake | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/Code/cmake/Modules/RDKitUtils.cmake b/Code/cmake/Modules/RDKitUtils.cmake > index 3bfcd6f..f2dcbe6 100644 > --- a/Code/cmake/Modules/RDKitUtils.cmake > +++ b/Code/cmake/Modules/RDKitUtils.cmake > @@ -17,7 +17,7 @@ macro(rdkit_library) > #IF(RDKLIB_SHARED) > add_library(${RDKLIB_NAME} SHARED ${RDKLIB_SOURCES}) > INSTALL(TARGETS ${RDKLIB_NAME} > - DESTINATION ${RDKit_BinDir}/${RDKLIB_DEST}) > + DESTINATION ${RDKit_LibDir}/${RDKLIB_DEST}) > #ELSE(RDKLIB_SHARED) > # add_library(${RDKLIB_NAME} ${RDKLIB_SOURCES}) > # INSTALL(TARGETS ${RDKLIB_NAME} > -- > 1.7.1.1 > > > ------------------------------------------------------------------------------ > This SF.net email is sponsored by Sprint > What will you do first with EVO, the first 4G phone? > Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first > _______________________________________________ > Rdkit-devel mailing list > Rdk...@li... > https://lists.sourceforge.net/lists/listinfo/rdkit-devel > |
|
From: Greg L. <gre...@gm...> - 2010-07-14 04:38:41
|
Dear Gianluca, I made a couple of modifications to the packaging scripts to be sure that some of the "extra" directories like $RDBASE/Data also get installed. These are in svn on the packaging branch. The current state builds and produces a useable system with "make install" on both linux and the mac. On windows everything builds, but the python files are installed in a less than ideal location (c:/program files/RDKit/Lib/site-packages instead of c:/python2.6/Lib/site-packages). I need to track this down and see if it can be fixed. At the moment the system still needs the RDBASE environment variable so that the python code can find some of its extra data; I'm not quite sure what to do about this. Would you mind checking to see if the current state of things is still useable for packaging? Best, -greg |
|
From: Gianluca S. <gi...@gm...> - 2010-07-14 14:19:09
|
On Wed, Jul 14, 2010 at 6:38 AM, Greg Landrum <gre...@gm...> wrote: > Dear Gianluca, > > I made a couple of modifications to the packaging scripts to be sure > that some of the "extra" directories like $RDBASE/Data also get > installed. These are in svn on the packaging branch. Great, I just cloned the code from SVN to have a look the latest changes > > At the moment the system still needs the RDBASE environment variable > so that the python code can find some of its extra data; I'm not quite > sure what to do about this. Yeah, RDBASE is my next target. While technically nothing prevents packages from adding env variables, it's better to not pollute the system if we can avoid it. Other than for tests (where I guess it's fine to leave it as is) do you know what RDBASE is used for at runtime? I already started patching the code to remove the runtime dependency, but I'd like to hear your opinion on the topic before digging further in this task. Cheers G. -- Gianluca Sforna http://morefedora.blogspot.com http://www.linkedin.com/in/gianlucasforna |
|
From: Greg L. <gre...@gm...> - 2010-07-14 15:43:34
|
On Wed, Jul 14, 2010 at 4:18 PM, Gianluca Sforna <gi...@gm...> wrote:
> On Wed, Jul 14, 2010 at 6:38 AM, Greg Landrum <gre...@gm...> wrote:
>>
>> At the moment the system still needs the RDBASE environment variable
>> so that the python code can find some of its extra data; I'm not quite
>> sure what to do about this.
>
> Yeah, RDBASE is my next target. While technically nothing prevents
> packages from adding env variables, it's better to not pollute the
> system if we can avoid it.
>
> Other than for tests (where I guess it's fine to leave it as is) do
> you know what RDBASE is used for at runtime?
>
It's really only used to find parameter files. It's not even used
directly most of the time, since the python code uses values found in
$RDBASE/rdkit/RDConfig.py, which uses $RDBASE to construct the paths
of the Data directory.
Here's a scan of where $RDBASE appears (I ignore unit test files since
those are not needed when packaging a runtime):
~/RDKit_Packaging/rdkit > pygrep RDBASE | grep -v UnitTest
./Chem/ChemUtils/AlignDepict.py:# This file is part of RDKit and
covered by $RDBASE/license.txt
./qtGui/Search3D/Search3D.py:
os.environ['RDBASE']=os.path.dirname(sys.executable)
./qtGui/Search3D/Search3D.py:if not os.environ.has_key('RDBASE') or
not os.environ['RDBASE']:
./qtGui/Search3D/Search3D.py: os.environ['RDBASE']=os.getcwd()
./RDConfig.py:if not os.environ.has_key('RDBASE'):
./RDConfig.py: raise ImportError,'please set the RDBASE environment
variable before using RDKit code'
./RDConfig.py:RDBaseDir=os.environ['RDBASE']
./RDToDo/RDToDo.py:if not env.has_key('RDBASE'):
./RDToDo/RDToDo.py: env['RDBASE']=os.getcwd()
the stuff in qtGui and RDToDo is deprecated and will soon be removed,
so no need to worry about those. The more interesting question is
where the RDDataDir variable (built in RDConfig.py) is used:
~/RDKit_Packaging/rdkit > pygrep RDDataD | grep -v UnitTest
./Chem/BuildFragmentCatalog.py: groupFileName =
os.path.join(RDConfig.RDDataDir,"FunctionalGroups.txt")
./Chem/ChemUtils/SDFToCSV.py: fName =
os.path.join(RDConfig.RDDataDir,'NCI','first_200.props.sdf')
./Chem/ChemUtils/SDFToCSV.py: fName =
os.path.join(RDConfig.RDDataDir,'NCI','first_200.props.sdf')
./Chem/Crippen.py:defaultPatternFileName =
os.path.join(RDConfig.RDDataDir,'Crippen.txt')
./Chem/Features/ShowFeats.py:parser.add_option('-f','--fdef',default=os.path.join(RDConfig.RDDataDir,'BaseFeatures.fdef'),
./Chem/Fragments.py:defaultPatternFileName =
os.path.join(RDConfig.RDDataDir,'FragmentDescriptors.csv')
./Chem/Pharm2D/__init__.py: fdefFile =
os.path.join(RDConfig.RDDataDir,'BaseFeatures.fdef')
./Chem/Pharm2D/LazyGenerator.py: inD =
open(RDConfig.RDDataDir+"/NCI/first_5K.smi",'r').readlines()[:nToDo]
./Chem/tests/BuildDescrsTestSet.py: inFileName =
os.path.join(RDConfig.RDDataDir,'PhysProp',
./ML/Descriptors/CompoundDescriptors.py: dbName = RDConfig.RDDataDatabase
./qtGui/GuiLib/ClusterWindow.py:
widg.loadPickledClusterTree(os.path.join(RDConfig.RDDataDir,'Ferromag','clusts.pkl'))
./qtGui/GuiLib/DecTreeWindow.py:
widg.loadPickledTree(os.path.join(RDConfig.RDDataDir,'Ferromag','ferromag_tree.pkl'))
./RDConfig.py:RDDataDir=os.path.join(RDBaseDir,'Data')
./RDConfig.py: RDDataDatabase='::RDData'
./RDConfig.py: RDTestDatabase=os.path.join(RDDataDir,"RDTests.sqlt")
./RDConfig.py: RDDataDatabase=os.path.join(RDDataDir,"RDData.sqlt")
./RDConfig.py: RDDataDatabase=None
./utils/chemutils.py: _atomDbName =
os.path.join(RDConfig.RDDataDir,'atomdb.gdb')
Some of that is deprecated, some is internal testing, and some is real
identification of data files.
If there were a way to detect at runtime where the Data directory has
been installed, then RDConfig.py could use in situations when $RDBASE
isn't defined. Unfortunately, this I really have no idea how to do.
> I already started patching the code to remove the runtime dependency,
> but I'd like to hear your opinion on the topic before digging further
> in this task.
Assuming that you mean the runtime dependency on the $RDBASE variable:
If there's some way to remove it that works on linux and windows (the
mac normally works if linux does), and there's still a way to use
$RDBASE if it's set, I think it would be excellent if $RDBASE were no
longer required.
-greg
|
|
From: Gianluca S. <gi...@gm...> - 2010-07-18 23:07:37
|
Hi Greg, thanks for your reply
On Wed, Jul 14, 2010 at 5:43 PM, Greg Landrum <gre...@gm...> wrote:
> If there were a way to detect at runtime where the Data directory has
> been installed, then RDConfig.py could use in situations when $RDBASE
> isn't defined. Unfortunately, this I really have no idea how to do.
>
>> I already started patching the code to remove the runtime dependency,
>> but I'd like to hear your opinion on the topic before digging further
>> in this task.
>
> Assuming that you mean the runtime dependency on the $RDBASE variable:
Yeah :)
> If there's some way to remove it that works on linux and windows (the
> mac normally works if linux does), and there's still a way to use
> $RDBASE if it's set, I think it would be excellent if $RDBASE were no
> longer required.
A short term solution could be to hardcode the RDBASE value in
RDConfig.py at build time (since we know where stuff will land in the
installed system) while allowing override from the env variable at
runtime.
Lastly, I pulled the packaging branch from SVN and attempted the
build; I noticed the destination directories are repeated twice as in:
/usr/share/RDKit/Data/Data/SmartsLib/RLewis_smarts.txt
The following patch looks like fixing it
===
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 68e96df..7c68ea5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -74,14 +74,9 @@ if(CMAKE_COMPILER_IS_GNUCXX)
ADD_DEFINITIONS("-Wno-deprecated -Wno-unused-function
-fno-strict-aliasing -fPIC")
endif()
-install(DIRECTORY Data DESTINATION ${RDKit_ShareDir}/Data
- PATTERN ".svn" EXCLUDE)
-install(DIRECTORY Docs DESTINATION ${RDKit_ShareDir}/Docs
- PATTERN ".svn" EXCLUDE)
-install(DIRECTORY Projects DESTINATION ${RDKit_ShareDir}/Projects
- PATTERN ".svn" EXCLUDE)
-install(DIRECTORY Contrib DESTINATION ${RDKit_ShareDir}/Contrib
- PATTERN ".svn" EXCLUDE)
+install(DIRECTORY Data Docs Projects Contrib
+ DESTINATION ${RDKit_ShareDir}
+ PATTERN ".svn" EXCLUDE)
install(FILES README license.txt DESTINATION ${RDKit_ShareDir})
===
Cheers
G.
--
Gianluca Sforna
http://morefedora.blogspot.com
http://www.linkedin.com/in/gianlucasforna
|
|
From: Greg L. <gre...@gm...> - 2010-07-21 04:00:02
|
Dear Gianluca, On Mon, Jul 19, 2010 at 1:07 AM, Gianluca Sforna <gi...@gm...> wrote: > > On Wed, Jul 14, 2010 at 5:43 PM, Greg Landrum <gre...@gm...> wrote: > >> If there's some way to remove it that works on linux and windows (the >> mac normally works if linux does), and there's still a way to use >> $RDBASE if it's set, I think it would be excellent if $RDBASE were no >> longer required. > > A short term solution could be to hardcode the RDBASE value in > RDConfig.py at build time (since we know where stuff will land in the > installed system) while allowing override from the env variable at > runtime. I guess I don't see how we're going to know at build time where things will be installed. That's not known until the user does "make install" (or, in the case of the package, installs the package). Are you suggesting we take sensible defaults (by operating system) and just use those? > Lastly, I pulled the packaging branch from SVN and attempted the > build; I noticed the destination directories are repeated twice as in: > /usr/share/RDKit/Data/Data/SmartsLib/RLewis_smarts.txt doh! Fixed in svn... thanks. -greg |
|
From: Gianluca S. <gi...@gm...> - 2010-07-21 10:46:09
|
On Wed, Jul 21, 2010 at 5:59 AM, Greg Landrum <gre...@gm...> wrote: > I guess I don't see how we're going to know at build time where things > will be installed. That's not known until the user does "make install" It is true files are installed after the build step. However, the various installation paths are interpolated and defined when you run cmake; in particular, the %cmake macro used while building the RPM expands to: cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_INSTALL_LIBDIR:PATH=/usr/lib64 -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64 -DCMAKE_SKIP_RPATH:BOOL=ON -DBUILD_SHARED_LIBS:BOOL=ON and at this point we are able to tell where things will land with a "make install". I will try to implement something in the next few days so we can see if it's going to be clean and simple as I would like it to be. Cheers G. -- Gianluca Sforna http://morefedora.blogspot.com http://www.linkedin.com/in/gianlucasforna |
|
From: Riccardo V. <ric...@gm...> - 2010-07-25 22:55:08
Attachments:
patch.txt
|
Hello,
On Wed, Jul 21, 2010 at 12:45 PM, Gianluca Sforna <gi...@gm...> wrote:
> On Wed, Jul 21, 2010 at 5:59 AM, Greg Landrum <gre...@gm...> wrote:
>> I guess I don't see how we're going to know at build time where things
>> will be installed. That's not known until the user does "make install"
>
>
> It is true files are installed after the build step. However, the
> various installation paths are interpolated and defined when you run
> cmake; in particular, the %cmake macro used while building the RPM
> expands to:
>
> cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr
> -DCMAKE_INSTALL_LIBDIR:PATH=/usr/lib64
> -DINCLUDE_INSTALL_DIR:PATH=/usr/include
> -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc
> -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64
> -DCMAKE_SKIP_RPATH:BOOL=ON -DBUILD_SHARED_LIBS:BOOL=ON
>
> and at this point we are able to tell where things will land with a
> "make install".
following this idea I tried to assemble a first small patch. Some
additional code in $RDBASE/rdkit/CMakeLists.txt generates a few-lines
python module (RDPaths.py) with some hard-coded path values and
installs it in the top python directory (now assigned to
${RDKit_PythonDir} by the top CMakeLists.txt file). If the $RDBASE
environment variable is set, RDConfig.py should behave as usual,
otherwise the hard-coded values are imported from RDPaths. At present
the main issue I faced has been with the path value represented by
$RDBASE itself (aka RDConfig.RDBaseDir) that I'm not sure has a unique
correspondence in a system-wide installation and I therefore left it
unassigned.
Please, have a look at the attached file, maybe it's not complete, but
I think it could be quite close to what was suggested.
Regards,
Riccardo
|
|
From: Greg L. <gre...@gm...> - 2010-07-27 05:00:16
|
Dear Riccardo,
On Mon, Jul 26, 2010 at 12:55 AM, Riccardo Vianello
<ric...@gm...> wrote:
>
> following this idea I tried to assemble a first small patch. Some
> additional code in $RDBASE/rdkit/CMakeLists.txt generates a few-lines
> python module (RDPaths.py) with some hard-coded path values and
> installs it in the top python directory (now assigned to
> ${RDKit_PythonDir} by the top CMakeLists.txt file). If the $RDBASE
> environment variable is set, RDConfig.py should behave as usual,
> otherwise the hard-coded values are imported from RDPaths. At present
> the main issue I faced has been with the path value represented by
> $RDBASE itself (aka RDConfig.RDBaseDir) that I'm not sure has a unique
> correspondence in a system-wide installation and I therefore left it
> unassigned.
>
> Please, have a look at the attached file, maybe it's not complete, but
> I think it could be quite close to what was suggested.
Thanks for the patch. I tested it this morning and it seems to work
well (at least after some simple testing). It's checked in on the
Packaging branch now.
Best Regards,
-greg
|
|
From: Riccardo V. <ric...@gm...> - 2010-07-30 16:14:53
Attachments:
soversion.diff
versions.h.cmake
|
Dear Greg,
On Tue, Jul 27, 2010 at 6:59 AM, Greg Landrum <gre...@gm...> wrote:
> Thanks for the patch. I tested it this morning and it seems to work
> well (at least after some simple testing). It's checked in on the
> Packaging branch now.
thanks for including that patch. I prepared an additional one, that
would introduce some changes to the way the package version is
managed. This way, installing the C++ shared libraries with more
unix-friendly sonames linking to the real (versioned) library names, I
think becomes easier.
With this patch applied to the Packaging branch, the release
identifiers are set by the top CMakeLists.txt,
set(RDKit_Year "2010")
set(RDKit_Quarter "3")
set(RDKit_Revision "1pre")
set(RDKit_ABI "1")
and used inside RDKitUtils.cmake to set the output name and version
properties of library targets. Moreover (to avoid having the release
string defined in multiple places) the Code/RDGeneral/versions.h
header is created at build-time starting from a corresponding
Code/RDGeneral/versions.h.cmake template (attached, it's supposed to
replace the current header file versions.h).
${RDKit_ABI} is new and used to build the libraries soname, the
overall release name didn't change (rdkit.rdBase.rdkitVersion is still
'2010Q3_1pre') and the naming of the unix shared library results in
something like the following:
libDepictor.so -> libDepictor.so.1
libDepictor.so.1 -> libDepictor.so.1.20103.1pre
I couldn't yet verify the behaviour of the windows counterpart to
library versioning in RDKitUtils.cmake, that part will probabily
strongly benefit a review (but I think it can be eventually just
commented out), so this is more a proposal and a starting point than
anything else, but I think a more formal versioning could be useful to
the system-wide installation.
Regards,
Riccardo
|
|
From: Greg L. <gre...@gm...> - 2010-08-02 19:21:14
|
Dear all,
On Fri, Jul 30, 2010 at 6:14 PM, Riccardo Vianello
<ric...@gm...> wrote:
>
> thanks for including that patch. I prepared an additional one, that
> would introduce some changes to the way the package version is
> managed. This way, installing the C++ shared libraries with more
> unix-friendly sonames linking to the real (versioned) library names, I
> think becomes easier.
>
> With this patch applied to the Packaging branch, the release
> identifiers are set by the top CMakeLists.txt,
>
> set(RDKit_Year "2010")
> set(RDKit_Quarter "3")
> set(RDKit_Revision "1pre")
> set(RDKit_ABI "1")
>
> and used inside RDKitUtils.cmake to set the output name and version
> properties of library targets. Moreover (to avoid having the release
> string defined in multiple places) the Code/RDGeneral/versions.h
> header is created at build-time starting from a corresponding
> Code/RDGeneral/versions.h.cmake template (attached, it's supposed to
> replace the current header file versions.h).
>
> ${RDKit_ABI} is new and used to build the libraries soname, the
> overall release name didn't change (rdkit.rdBase.rdkitVersion is still
> '2010Q3_1pre') and the naming of the unix shared library results in
> something like the following:
>
> libDepictor.so -> libDepictor.so.1
> libDepictor.so.1 -> libDepictor.so.1.20103.1pre
>
> I couldn't yet verify the behaviour of the windows counterpart to
> library versioning in RDKitUtils.cmake, that part will probabily
> strongly benefit a review (but I think it can be eventually just
> commented out), so this is more a proposal and a starting point than
> anything else, but I think a more formal versioning could be useful to
> the system-wide installation.
In principle these sound find to me, and I was able to apply them to
the packaging branch and get working code (which I've checked in).
I still need to test on Windows, which I will do tomorrow sometime,
and the mac, which I assume will just work.
A comment about the way this is going: it currently seems to be pretty
easy to get a system-wide installation (i.e. something in /opt or
/usr/local), which is great of packaging. But as a developer, I would
like to be able to have multiple copies of the code, so it would be
very nice to have current (trunk) "make install" behavior available:
i.e. copy the binaries back up into the current source tree. I will
look into this a bit over the next couple of days, but if anyone has
tips on how this can be accomplished I'd love to hear them.
-greg
|
|
From: Greg L. <gre...@gm...> - 2010-08-03 05:06:27
|
Quick followup: On Mon, Aug 2, 2010 at 9:20 PM, Greg Landrum <gre...@gm...> wrote: > >> >> I couldn't yet verify the behaviour of the windows counterpart to >> library versioning in RDKitUtils.cmake, that part will probabily >> strongly benefit a review (but I think it can be eventually just >> commented out), so this is more a proposal and a starting point than >> anything else, but I think a more formal versioning could be useful to >> the system-wide installation. > > I still need to test on Windows, which I will do tomorrow sometime, > and the mac, which I assume will just work. This morning I pulled a copy of the packaging branch on windows (including my changes to allow in-source installs): things build and install successfully and the tests all pass. Best, -greg |
|
From: Gianluca S. <gi...@gm...> - 2010-08-02 20:42:15
|
On Mon, Aug 2, 2010 at 9:20 PM, Greg Landrum <gre...@gm...> wrote:
> Dear all,
>
> On Fri, Jul 30, 2010 at 6:14 PM, Riccardo Vianello
> <ric...@gm...> wrote:
>>
>> thanks for including that patch. I prepared an additional one, that
>> would introduce some changes to the way the package version is
>> managed. This way, installing the C++ shared libraries with more
>> unix-friendly sonames linking to the real (versioned) library names, I
>> think becomes easier.
>>
>> With this patch applied to the Packaging branch, the release
>> identifiers are set by the top CMakeLists.txt,
>>
>> set(RDKit_Year "2010")
>> set(RDKit_Quarter "3")
>> set(RDKit_Revision "1pre")
>> set(RDKit_ABI "1")
>>
>> and used inside RDKitUtils.cmake to set the output name and version
>> properties of library targets. Moreover (to avoid having the release
>> string defined in multiple places) the Code/RDGeneral/versions.h
>> header is created at build-time starting from a corresponding
>> Code/RDGeneral/versions.h.cmake template (attached, it's supposed to
>> replace the current header file versions.h).
>>
>> ${RDKit_ABI} is new and used to build the libraries soname, the
>> overall release name didn't change (rdkit.rdBase.rdkitVersion is still
>> '2010Q3_1pre') and the naming of the unix shared library results in
>> something like the following:
>>
>> libDepictor.so -> libDepictor.so.1
>> libDepictor.so.1 -> libDepictor.so.1.20103.1pre
>>
>> I couldn't yet verify the behaviour of the windows counterpart to
>> library versioning in RDKitUtils.cmake, that part will probabily
>> strongly benefit a review (but I think it can be eventually just
>> commented out), so this is more a proposal and a starting point than
>> anything else, but I think a more formal versioning could be useful to
>> the system-wide installation.
>
> In principle these sound find to me, and I was able to apply them to
> the packaging branch and get working code (which I've checked in).
>
> I still need to test on Windows, which I will do tomorrow sometime,
> and the mac, which I assume will just work.
>
> A comment about the way this is going: it currently seems to be pretty
> easy to get a system-wide installation (i.e. something in /opt or
> /usr/local), which is great of packaging. But as a developer, I would
> like to be able to have multiple copies of the code, so it would be
> very nice to have current (trunk) "make install" behavior available:
> i.e. copy the binaries back up into the current source tree. I will
> look into this a bit over the next couple of days, but if anyone has
> tips on how this can be accomplished I'd love to hear them.
I don't know if that's easily doable. However, rdkit is the only
project I worked on with a that kind of behaviour for "make install";
how exactly is that useful to you?
--
Gianluca Sforna
http://morefedora.blogspot.com
http://www.linkedin.com/in/gianlucasforna
|
|
From: Greg L. <gre...@gm...> - 2010-08-03 04:24:20
|
Dear al, On Mon, Aug 2, 2010 at 10:41 PM, Gianluca Sforna <gi...@gm...> wrote: > On Mon, Aug 2, 2010 at 9:20 PM, Greg Landrum <gre...@gm...> wrote: >> >> A comment about the way this is going: it currently seems to be pretty >> easy to get a system-wide installation (i.e. something in /opt or >> /usr/local), which is great of packaging. But as a developer, I would >> like to be able to have multiple copies of the code, so it would be >> very nice to have current (trunk) "make install" behavior available: >> i.e. copy the binaries back up into the current source tree. I will >> look into this a bit over the next couple of days, but if anyone has >> tips on how this can be accomplished I'd love to hear them. > > I don't know if that's easily doable. However, rdkit is the only > project I worked on with a that kind of behaviour for "make install"; > how exactly is that useful to you? That's an easy one. There are several reasons: 1) I typically have several different versions of the RDKit active at any one point on a machine (last release, current synced trunk copy, a branch, the copy I'm working in, etc.). It's very convenient to be able to switch between these purely via environment variables. 2) for people who just want to try the software out without having to globally install anything, it's nice if there's a method for building and installing that doesn't require a system directory. 3) while doing development it's nice to have the svn checkout of the python files together with the built .so files. The first two of these are theoretically possible with the current packaging branch, but involve directory names like ~/RDKit_test_install/lib/python2.6/dist-packages. The last doesn't really work at all. I just checked in a change on the packaging branch that, I believe, allows the old behavior again: if the cmake variable RDK_INSTALL_INTREE is set (it is not by default), then "make install" will show the original behavior (it will install things in the source tree). Please take a look and see what you think. Best Regards, -greg |
|
From: Riccardo V. <ric...@gm...> - 2010-08-05 16:54:41
|
Hi all,
On Tue, Aug 3, 2010 at 6:23 AM, Greg Landrum <gre...@gm...> wrote:
> Dear al,
>
> On Mon, Aug 2, 2010 at 10:41 PM, Gianluca Sforna <gi...@gm...> wrote:
>> On Mon, Aug 2, 2010 at 9:20 PM, Greg Landrum <gre...@gm...> wrote:
>>>
>>> A comment about the way this is going: it currently seems to be pretty
>>> easy to get a system-wide installation (i.e. something in /opt or
>>> /usr/local), which is great of packaging. But as a developer, I would
>>> like to be able to have multiple copies of the code, so it would be
>>> very nice to have current (trunk) "make install" behavior available:
>>> i.e. copy the binaries back up into the current source tree. I will
>>> look into this a bit over the next couple of days, but if anyone has
>>> tips on how this can be accomplished I'd love to hear them.
>>
>> I don't know if that's easily doable. However, rdkit is the only
>> project I worked on with a that kind of behaviour for "make install";
>> how exactly is that useful to you?
>
I agree it's good to have the older behaviour (and therefore the
development layout) supported, but I agree with Gianluca that rdkit is
somewhat unusual in this aspect. In the last days I tried a couple of
approaches to making tests functional with an out-of-tree
build/installation, so that one could test the build prior to the
actual installation step, but the testing framework is also quite
coupled to the layout and resources available from inside the source
tree.
So I was evaluating a different option.. it might be feasible to
introduce changes and fixes to the CMakeLists.txt files so that the
produced binaries were put in directories matching their final
location in an in-tree installation (for example C++ shared libraries
might have the target's output directory set to something like
${CMAKE_BINARY_DIR}/bin). If this is doable, an in-source build could
put everything in place for a local development environment, right
after the build step, with no explicit in-tree installation required.
'make clean' might at the same time also become available to the
developer and this also looks like a nice thing in comparison to
overwriting the existing installation or the limited reliability of an
uninstall operation (where available). When packaging, one could
perform an in-source build, run the tests and then perform the staged
installation.
These changes and support for in-tree installation are not necessarily
mutually exclusive (if I didn't miss anything important from the
latest patch, in-tree installation works on a reconfiguration of the
installation paths, and this shouldn't depend on where the built
targets are saved), but I'm having the impression that reconfiguring
where the built targets are placed could result is some additional
flexibility. What do you think?
Best regards,
Riccardo
|
|
From: Riccardo V. <ric...@gm...> - 2010-08-07 22:15:18
Attachments:
insource.patch
|
Hi all, I tried to explore that idea a bit further, and it seems that the changes required to have an in-source build that doesn't require an installation step to be functional are more limited than I thought. I made a couple of tests, and on my linux box the attached patch supported both the "out-of-source build, in-tree install and test", and the "in-source build, test, out-of-source install" sequences. In both cases, testing requires the proper environment ($RDBASE and the other env variables, plus the sqlite database files), but I hope this is not a problem for the packaging process. Best regards, Riccardo |
|
From: Greg L. <gre...@gm...> - 2010-08-10 04:29:34
|
Dear Riccardo, On Sun, Aug 8, 2010 at 12:15 AM, Riccardo Vianello <ric...@gm...> wrote: > > I tried to explore that idea a bit further, and it seems that the > changes required to have an in-source build that doesn't require an > installation step to be functional are more limited than I thought. I > made a couple of tests, and on my linux box the attached patch > supported both the "out-of-source build, in-tree install and test", > and the "in-source build, test, out-of-source install" sequences. In > both cases, testing requires the proper environment ($RDBASE and the > other env variables, plus the sqlite database files), but I hope this > is not a problem for the packaging process. Thanks for the patch. I tested it this morning on my linux box and they look good: an in-source build and test followed by a standard install to /usr/local worked just fine. The changes are checked in. -greg |
|
From: Riccardo V. <ric...@gm...> - 2010-09-03 20:58:33
Attachments:
install_hdrs.patch
|
Dear Greg, thanks for including that last patch. I noticed that C++ header files were not yet considered by the installation procedure, so I prepared an additional patch that should take care of them (when the RDK_INSTALL_INTREE is not set, of course). It repeatedly uses a small macro (rdkit_headers) I added to RDKitUtils.cmake and that should be a no-op when installing intree. Given the number of source directories I can't exclude the possibility I might have missed any headers, but (assuming the patch is acceptable) it should be quite complete. One additional thing one might consider is that currently almost all files from the python subtree are installed together with the actual python modules (cmake list files, test code..), but I hope the current status of the branch could be quite close to what is needed to finalize the packaging process. Regards, Riccardo |
|
From: Greg L. <gre...@gm...> - 2010-09-04 14:20:15
|
Dear Riccardo, On Fri, Sep 3, 2010 at 10:58 PM, Riccardo Vianello <ric...@gm...> wrote: > > thanks for including that last patch. I noticed that C++ header files > were not yet considered by the installation procedure, so I prepared > an additional patch that should take care of them (when the > RDK_INSTALL_INTREE is not set, of course). It repeatedly uses a small > macro (rdkit_headers) I added to RDKitUtils.cmake and that should be a > no-op when installing intree. Given the number of source directories I > can't exclude the possibility I might have missed any headers, but > (assuming the patch is acceptable) it should be quite complete. Thanks for doing this; it was on my list, but I hadn't managed to find time yet. I applied the patch; verified that an install still works; and committed it. I haven't checked that all headers are copied yet, but I'll do that over the next few days. > One additional thing one might consider is that currently almost all > files from the python subtree are installed together with the actual > python modules (cmake list files, test code..), Good catch. I just checked in a modification to catch these (at least most of them). > but I hope the current > status of the branch could be quite close to what is needed to > finalize the packaging process. I think we're close! -greg |
|
From: Gianluca S. <gi...@gm...> - 2010-09-07 09:55:21
Attachments:
rpmlint.txt
|
On Sat, Sep 4, 2010 at 4:19 PM, Greg Landrum <gre...@gm...> wrote: > Dear Riccardo, > > On Fri, Sep 3, 2010 at 10:58 PM, Riccardo Vianello > <ric...@gm...> wrote: > >> but I hope the current >> status of the branch could be quite close to what is needed to >> finalize the packaging process. > > I think we're close! Indeed! I updated the code from the branch and worked a bit more on the packaging side. Right now, I have this split: rdkit-doc - documentation (/usr/share/RDkit/Doc) rdkit-devel - development files ( /usr/include/rdkit and /usr/lib/lib*.so python-rdkit - python bindings rdkit - everything else I'm still considering whether splitting out Data from the latter package, but overall it is quite a good achievement Running rpmlint on resulting rpms results in some warnings, the list is attached. Most of them relates to the executable bit being set on some source fields, I could fix it during packaging but it's probably a good idea to change them in the repository. Then it warns about /usr/include/rdkit/GraphMol/BondProps.h and /usr/include/rdkit/GraphMol/AtomProps.h being empty. I will have a look at running tests during build, then push packages somewhere for a wider exposure. Thanks a lot G. -- Gianluca Sforna http://morefedora.blogspot.com http://identi.ca/giallu - http://twitter.com/giallu |
|
From: Greg L. <gre...@gm...> - 2010-09-08 04:24:56
|
2010/9/7 Gianluca Sforna <gi...@gm...>: > > Running rpmlint on resulting rpms results in some warnings, the list > is attached. > Most of them relates to the executable bit being set on some source > fields, I could fix it during packaging but it's probably a good idea > to change them in the repository. > Then it warns about /usr/include/rdkit/GraphMol/BondProps.h and > /usr/include/rdkit/GraphMol/AtomProps.h being empty. These should now be taken care of. I also tested the build on windows and everything seems to work without problems. -greg |
|
From: Greg L. <gre...@gm...> - 2010-09-07 10:43:10
|
2010/9/7 Gianluca Sforna <gi...@gm...>: > On Sat, Sep 4, 2010 at 4:19 PM, Greg Landrum <gre...@gm...> wrote: >> Dear Riccardo, >> >> On Fri, Sep 3, 2010 at 10:58 PM, Riccardo Vianello >> <ric...@gm...> wrote: >> >>> but I hope the current >>> status of the branch could be quite close to what is needed to >>> finalize the packaging process. >> >> I think we're close! > > Indeed! > I updated the code from the branch and worked a bit more on the packaging side. > > Right now, I have this split: > > rdkit-doc - documentation (/usr/share/RDkit/Doc) > rdkit-devel - development files ( /usr/include/rdkit and /usr/lib/lib*.so > python-rdkit - python bindings > rdkit - everything else sounds like a sensible layout to me, but I think /usr/lib/lib*.so probably needs to be in the rdkit package. These shared libs are needed by the python modules and will (probably) be needed by any application that is built using the RDKit. > I'm still considering whether splitting out Data from the latter > package, but overall it is quite a good achievement Agreed. We'll need to think about dependencies (i.e. python-rdkit depends on rdkit), but I guess that's easy. > Running rpmlint on resulting rpms results in some warnings, the list > is attached. > Most of them relates to the executable bit being set on some source > fields, I could fix it during packaging but it's probably a good idea > to change them in the repository. > Then it warns about /usr/include/rdkit/GraphMol/BondProps.h and > /usr/include/rdkit/GraphMol/AtomProps.h being empty. I will go through these this evening. I will also take a look at the situation on Windows again this evening to make sure everything there is still behaving. > I will have a look at running tests during build, then push packages > somewhere for a wider exposure. Sounds like we're close to it being time to merge the changes from the branch back onto the trunk. It's very cool! Thanks to both of you for all the help, -greg |
|
From: Gianluca S. <gi...@gm...> - 2010-09-07 14:50:50
|
On Tue, Sep 7, 2010 at 12:42 PM, Greg Landrum <gre...@gm...> wrote: > sounds like a sensible layout to me, but I think /usr/lib/lib*.so > probably needs to be in the rdkit package. These shared libs are > needed by the python modules and will (probably) be needed by any > application that is built using the RDKit. The .so files providing python bindings are in the python-rdkit package; the versioned .so (lib*.so.something) are those required at runtime for stuff built against rdkit. At link time lib*.so files are used to actually perform the link, but those are not needed at runtime so they are kept with the headers, according to: https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages I believe all distros have the same arrangement. > >> I'm still considering whether splitting out Data from the latter >> package, but overall it is quite a good achievement > > Agreed. We'll need to think about dependencies (i.e. python-rdkit > depends on rdkit), but I guess that's easy. Yeah, that's easy and mostly automatic, I will just add a couple requures from -data and -docs for the main rdkit package. -- Gianluca Sforna http://morefedora.blogspot.com http://identi.ca/giallu - http://twitter.com/giallu |