Bug 549405 - SCons bundles system libraries
Summary: SCons bundles system libraries
Alias: None
Product: Fedora
Classification: Fedora
Component: scons
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Gérard Milmeister
QA Contact: Fedora Extras Quality Assurance
Depends On:
Blocks: DuplicSysLibsTracker
TreeView+ depends on / blocked
Reported: 2009-12-21 16:05 UTC by Cristian Ciupitu
Modified: 2010-11-08 19:21 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-11-08 19:21:41 UTC
Type: ---

Attachments (Terms of Use)

Description Cristian Ciupitu 2009-12-21 16:05:27 UTC
Description of problem:
Scons bundles a couple of system libraries. They are installed under /usr/lib/scons/SCons/compat/ and duplicate optparse, shlex, subprocess, textwrap and other system libraries that come with the python package.

Version-Release number of selected component (if applicable):

Comment 1 Chen Lei 2010-05-16 04:01:11 UTC
Scons won't use compatibility modules if corresponding system libraries exist.


Comment 2 Cristian Ciupitu 2010-05-16 15:02:07 UTC
(In reply to comment #1)
> Scons won't use compatibility modules if corresponding system libraries exist.

This is good, but nevertheless they shouldn't be packaged anymore in order to save space and make sure they aren't used.

Comment 3 Toshio Ernie Kuratomi 2010-08-14 19:17:07 UTC
Are we able to rm -rf those modules in our rpm packaging (due to them not being used if the corresponding system library exists)?  If so, that would definitely resolve this bug.  If not, why not?

Looking over how scons has made their compatibility libraries, I'm pleased with the following things:
* they keep all such code in a a separate directory.
* they make the module private (leading underscore) so people know they are not supposed to use them.
* The code doesn't rely on coders doing an explicit try: except ImportError everywhere that the compat module might need to be used.  Instead, the compat module takes care of discovering whether the system module exists and only uses  the compat module if the system module does not exist.  This is much less error prone.

My reading of that code is that we should be able to rm -rf those modules as SCons/compat/__init__.py should import the system libraries and never touch the compat modules on modern python2.x.  (Note that there's also some work in there to deal with python3.  I'm less familiar with what the situation is there so some of that might be necessary when you're ready to target scons as running on the python3 interpreter).

Even without doing the rm -rf, it might make sense to add something to the Guidelines about being able to tell that the code will not be used.  However, it might also be that the rm -rf is better as:

1) Not every packager and reviewer will be capable of analyzing the code
2) When you import a new piece of upstream code, you'll need to analyze it to be sure it isn't now using some of the bundled code improperly.

Let me know if you're against doing the rm -rf and should look into writing up a guideline draft.

Comment 4 Chen Lei 2010-08-23 13:11:06 UTC
Personally, I don't object to delete compat packages from scons since those files are only used by python 2.4~2.5 though I don't know the opinion from gemi. 

I'd like to point out that the latest scons version 2.0.1 include fewer compat modules than scons 1.2.x. We should also consider the default installtion place for scons is /usr/lib/scons instead %{python_sitelib}, so theoretically the scons rpms in rawhide can even run on el5 without any modification if we don't delete those compat modules.

Comment 5 Patrick Dignan 2010-09-02 17:35:29 UTC
Hi, I was just looking for an update on this.  Did you decide to rm -rf the compat packages?

Comment 6 Kevin Fenzi 2010-10-07 05:06:01 UTC
Here's a small patch and spec diff that does this: 

diff -Nur scons-2.0.1.orig/setup.py scons-2.0.1/setup.py
--- scons-2.0.1.orig/setup.py	2010-08-17 00:02:54.000000000 -0600
+++ scons-2.0.1/setup.py	2010-10-06 22:59:29.043981562 -0600
@@ -392,7 +392,6 @@
     'author_email'     : 'knight@baldmt.com',
     'url'              : "http://www.scons.org/",
     'packages'         : ["SCons",
-                          "SCons.compat",
diff --git a/scons.spec b/scons.spec
index 9ea0c23..040744d 100644
--- a/scons.spec
+++ b/scons.spec
@@ -2,12 +2,13 @@
 Name:		scons
 Version:	2.0.1
-Release:	1%{?posttag}%{?dist}
+Release:	2%{?posttag}%{?dist}
 Summary:	An Open Source software construction tool
 Group:		Development/Tools
 License:	MIT
 URL:		http://www.scons.org
 Source:		http://downloads.sourceforge.net/scons/scons-%{version}%{?posttag}.tar.gz
+Patch1:         scons-2.0.1-remove-compat.patch
 BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:	noarch
 BuildRequires:	python2-devel
@@ -36,6 +37,9 @@ for file in *.txt; do
     touch -r $file $file.new && \
     mv $file.new $file
+# remove bundled files. We don't use them and want to make sure they aren't.
+rm -rf engine/SCons/compat
+%patch1 -p1
 python setup.py build
@@ -56,6 +60,9 @@ rm -rf $RPM_BUILD_ROOT
+* Wed Oct 06 2010 Kevin Fenzi <kevin@tummy.com> - 2.0.1-2
+- Removed bundled compat files to make sure they are not used. 
 * Mon Aug 23 2010 Chen Lei <supercyper@163.com> - 2.0.1-1
 - new release 2.0.1

and a scratch build: 

Comment 7 Kevin Fenzi 2010-10-07 05:06:54 UTC
Any objections to applying?

Comment 8 Chen Lei 2010-10-07 08:27:01 UTC
I don't object to apply this change to scons. But I don't think we really need to delete those files, because scons only copy a few functions from python 2.6, those files are heaviley modilied and even not used by Fedora if python >2.5 is available[1]. Also, the compat modules is useful for regression testing[1].


Comment 9 Cristian Ciupitu 2010-10-07 11:33:00 UTC
(In reply to comment #7)
> Any objections to applying?
Is scons in EPEL? If so, shouldn't you remove those files only if you're building for Fedora >= 13?

(In reply to comment #8)
> Also, the compat modules is useful for regression testing[1].
I couldn't find anything mentioning that compat modules are useful for regression testing.

Comment 10 Chen Lei 2010-10-07 12:27:03 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > Also, the compat modules is useful for regression testing[1].
> I couldn't find anything mentioning that compat modules are useful for
> regression testing.

See http://www.scons.org/doc/2.0.1/HTML/scons-api/SCons.compat-pysrc.html

215  if os.environ.get('SCONS_HORRIBLE_REGRESSION_TEST_HACK') is not None: 
216      # We can't apply the 'callable' fixer until the floor is 2.6, but the 
217      # '-3' option to Python 2.6 and 2.7 generates almost ten thousand 
218      # warnings.  This hack allows us to run regression tests with the '-3' 
219      # option by replacing the callable() built-in function with a hack 
220      # that performs the same function but doesn't generate the warning. 
221      # Note that this hack is ONLY intended to be used for regression 
222      # testing, and should NEVER be used for real runs. 
223      from types import ClassType 
224 - def callable(obj):
225 if hasattr(obj, '__call__'): return True 226 if isinstance(obj, (ClassType, type)): return True 227 return False
228      import builtins 
229      builtins.callable = callable 
230      del callable 

scons only copys some necessary functions/classes from python standard libraries, so the bundled files are smaller than the corresponding files in python standard libraries. Deleting those files only save very few space.

Comment 11 Chen Lei 2010-10-07 12:33:29 UTC
scons compat is also needed for python 2.6.


Comment 12 Toshio Ernie Kuratomi 2010-10-07 18:20:55 UTC
I've opened a ticket to see about making  clear whether we need to delete the libraries or not: https://fedorahosted.org/fpc/ticket/19  and I've posted to packaging@lists.fp.o to start off discussion.

Comment 13 Chen Lei 2010-10-07 19:27:47 UTC
After taking a look at scons source, compat/__init__.py is widely used by other scons modules, so we should keep this file untouched.

It's safe to unbundle the following files:
compat/hashlib.py compat/set.py compat/collections.py compat/io.py compat/subprocess.py

Comment 14 Toshio Ernie Kuratomi 2010-10-13 19:12:52 UTC
The Packaging Committee drafted this this week: https://fedoraproject.org/wiki/PackagingDrafts/Treatment_Of_Bundled_Libraries

We'll likely vote on it next week.  If you have any input on it (clarification, need to know about whether specific things in scons fit under the exception or not, etc) please let us know on packaging@lists.fp.o.

Comment 15 Bug Zapper 2010-11-04 02:51:41 UTC
This message is a reminder that Fedora 12 is nearing its end of life.
Approximately 30 (thirty) days from now Fedora will stop maintaining
and issuing updates for Fedora 12.  It is Fedora's policy to close all
bug reports from releases that are no longer maintained.  At that time
this bug will be closed as WONTFIX if it remains open with a Fedora 
'version' of '12'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version prior to Fedora 12's end of life.

Bug Reporter: Thank you for reporting this issue and we are sorry that 
we may not be able to fix it before Fedora 12 is end of life.  If you 
would still like to see this bug fixed and are able to reproduce it 
against a later version of Fedora please change the 'version' of this 
bug to the applicable version.  If you are unable to change the version, 
please add a comment here and someone will do it for you.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events.  Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

The process we are following is described here: 

Comment 16 Kevin Fenzi 2010-11-04 04:37:19 UTC
This is still the case in rawhide. ;)

Comment 17 Patrick Dignan 2010-11-08 19:21:41 UTC
So it looks like SCons is ok according to the Conditional Functionality section in the draft here: https://fedoraproject.org/wiki/PackagingDrafts/Treatment_Of_Bundled_Libraries

That said, it would be nice to have a note in the spec file saying that the libraries are bundled libraries.

Note You need to log in before you can comment on or make changes to this bug.