Bug 549405

Scons won't use compatibility modules if corresponding system libraries exist.


See 
http://www.scons.org/doc/1.3.0.d20100501/HTML/scons-api/SCons.compat-module.html

(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.

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.

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.

Hi, I was just looking for an update on this.  Did you decide to rm -rf the compat packages?

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',
     'url'              : "http://www.scons.org/",
     'packages'         : ["SCons",
-                          "SCons.compat",
                           "SCons.Node",
                           "SCons.Options",
                           "SCons.Platform",
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
 done
+# remove bundled files. We don't use them and want to make sure they aren't.
+rm -rf engine/SCons/compat
+%patch1 -p1
 
 %build
 python setup.py build
@@ -56,6 +60,9 @@ rm -rf $RPM_BUILD_ROOT
 %{_mandir}/man?/*
 
 %changelog
+* Wed Oct 06 2010 Kevin Fenzi <kevin> - 2.0.1-2
+- Removed bundled compat files to make sure they are not used. 
+
 * Mon Aug 23 2010 Chen Lei <supercyper> - 2.0.1-1
 - new release 2.0.1

and a scratch build: 
http://koji.fedoraproject.org/koji/taskinfo?taskID=2519190

Any objections to applying?

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].

[1]http://www.scons.org/doc/2.0.1/HTML/scons-api/SCons.compat-pysrc.html
[2]http://www.scons.org/doc/2.0.1/HTML/scons-api/SCons.compat-module.html

(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.

(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.

scons compat is also needed for python 2.6.

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

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.o to start off discussion.

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

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.o.

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: 
http://fedoraproject.org/wiki/BugZappers/HouseKeeping

This is still the case in rawhide. ;)

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.