[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r9935 - in trunk: . build/generator subversion/bindings/java/javahl subversion/bindings/java/javahl/native

From: Branko Čibej <brane_at_xbc.nu>
Date: 2004-06-08 05:55:58 CEST

pmayweg@tigris.org wrote:

>Author: pmayweg
>Date: Mon Jun 7 09:00:49 2004
>New Revision: 9935
>
>* build/generator/gen_base.py
> DependencyNode.__hash__
> use member name of member filename contains an empty string
> DependencyNode.__cmp__
> use member name of member filename contains an empty string
>
>
"if" instead of "of"?

>--- trunk/build/generator/gen_base.py (original)
>+++ trunk/build/generator/gen_base.py Mon Jun 7 09:00:49 2004
>@@ -255,10 +255,19 @@
> return self.filename
>
> def __cmp__(self, ob):
>- return cmp(self.filename, ob.filename)
>+ myname = self.filename
>+ if myname == '':
>+ myname = self.name
>+ othername = ob.filename
>+ if othername == '':
>+ othername = ob.name
>+ return cmp(myname, othername)
>
> def __hash__(self):
>- return hash(self.filename)
>+ myname = self.filename
>+ if myname == '':
>+ myname = self.name
>+ return hash(myname)
>
>
Why aren't these sanity checks performed in the class init function
where they belong? __hash__ and __cmp__ are definitely the wrong place
for them. If filename or nams can change after the object has been
initialised, then create setter functions that maintain object
consistency (you should know about that, coming from the Java world :.-).

>
> class ObjectFile(DependencyNode):
> def __init__(self, filename, compile_cmd = None):
>@@ -326,7 +335,8 @@
> self.path = options.get('path', '')
> self.add_deps = options.get('add-deps', '')
> self.msvc_name = options.get('msvc-name') # override project name
>-
>+ self.needs_windows_custom_build = None
>
>
Now why are you doing this? Python has class inheritance with virtual
methods, and the classes have constructors. You could use either of
those mechanisms, instead of forcing the top-level generator to know
about this feature.

>@@ -670,8 +680,8 @@
> self.headers = options.get('headers')
> self.classes = options.get('classes')
> self.package = options.get('package')
>- self.extra_classes = string.split(options.get('extra-classes'))
> self.output_dir = self.headers
>+ self.needs_windows_custom_build = "Yes"
>
>
1?

>+ def get_windows_custom_build(self, generator, source, rootpath):
>+ junit_path = "";
>+ if not generator.junit_path is None:
>+ junit_path = ";" + generator.junit_path
>+
>+ dirs = string.split(source, '\\')
>+ i = len(dirs) - 1 # Last element is the .java file name.
>+
>+ classname = self.package + "." + string.split(dirs[i],".")[0]
>+
>+ classes = os.path.join(rootpath, self.classes)
>+ classpath = generator.get_project_quote() + classes + junit_path
>+ classpath = classpath + generator.get_project_quote()
>+ headers = os.path.join(rootpath, self.headers)
>+ targetdir = generator.get_project_quote() + headers
>+ targetdir = targetdir + generator.get_project_quote()
>+ cbuild = "javah -verbose -force -classpath " + classpath + " -d " +targetdir
>+ cbuild = cbuild + " " +classname
>+ self.path = "../" + self.headers
>+ return cbuild
>+
>+
>+ def get_windows_custom_target(self, source, rootpath):
>+ classesdir = os.path.join(rootpath, self.classes)
>+ classpart = source[len(classesdir)+1:]
>+ headername = string.split(classpart,".")[0]+".h"
>+ headername = string.replace(headername,"\\","_")
>+ target = os.path.join(rootpath, self.headers, headername)
>+ return target
>
>
And I don't like this, either. The custom build rules should go into the
build.conf file, not into the code, and there should be exactly one
generic function in the base Target class that handles them.

>@@ -109,6 +116,9 @@
> depends = [ ]
> if not isinstance(target, gen_base.TargetI18N):
> depends = self.adjust_win_depends(target, name)
>+ #print name
>+ #for dep in depends:
>+ # print " ",dep.name
>
>
You overlooked this bit of debug code.

>@@ -240,12 +243,17 @@
> "Get the list of source files for each project"
> sources = [ ]
> if not isinstance(target, gen_base.TargetProject):
>+ cbuild = None
>+ ctarget = None
> for src, reldir in self.get_win_sources(target):
> rsrc = string.replace(os.path.join(rootpath, src), os.sep, '\\')
> if quote_path and '-' in rsrc:
> rsrc = '"%s"' % rsrc
>+ if target.needs_windows_custom_build is not None:
>+ cbuild = target.get_windows_custom_build(self, rsrc, rootpath)
>+ ctarget = target.get_windows_custom_target(rsrc, rootpath)
>
>
See what I mean? This is evil.

>@@ -280,14 +288,17 @@
> if not isinstance(iobj, gen_base.SWIGSource):
> user_deps.append(isrc)
> continue
>+ cbuild = "swig "+self.swig_options+" -"+target.lang
>+ for include in self.get_win_includes(target, rootpath):
>+ cbuild = cbuild + " -I" + self.get_project_quote() + include
>+ cbuild = cbuild + self.get_project_quote()
>+ cbuild = cbuild + " -o " + self.get_project_quote() + cout
>+ cbuild = cbuild + self.get_project_quote() + " $(InputPath)"
>
>
And this even more so.

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jun 8 05:56:47 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.