Hi,
I have forgot add the list to my reply.
Patrick
attached mail follows:
Hi Branko,
I am sure the some of my implementation is not ideal, because of my
limited knowledge and experience with python. I will try to improve my
implementation.
Branko Čibej wrote:
>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"?
>
>
Sorry about the typo. I will change the log message.
>
>
>>--- 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 :.-).
>
>
These are not sanity checks. There are DependencyNode's which never set
a filename, because they do not create a single file. I did not invent
those. Those could not be added to a map (or how this thing is called in
Python). Changing the filename of a DependencyNode after it has been
added to a map does not work at all. I have tried setting the name as
the filename, but that broke the linux build.
>
>
>>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.
>
>
I did not want to add the methods get_windows_custom_build and
get_windows_custom_target to Target. I can either do that or add the
value of needs_windows_custom_build to the parameters of the
constructor. In Java I would have used an interface for that. I did not
find something similar in Python.
>
>
>>@@ -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?
>
>
I am not shure but you mean here.
>
>
>>+ 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.
>
>
I do think that will be quite complex. The custom build step operation
is not something, one reads from build.conf and put it into the project
file.
1. There are custom build rules, which are for the whole project and
there those for every source file in the project. This mean atleast 2
different ways of specifying a custom build rule.
2. The custom build code depends on the generator. Quotes are different
for the msvc and vcnet generator.
3. The custom build code may depend on parameters to gen-make.py. That
could remove the requirement to put everything needed for the windows
build into the path.
4. The swig custom build code depends on other options in build.conf.
5. Generating the target to be put in project file is not trivial either.
I think there are realy 2 different kind of Target's, which need custom
build rules.
1. Target's which have complex build rules like the Swig and Java
Targets which have there own Target class already. They have code to
build their custom build rules.
2. Target's which have simple build rules and read them from build.conf
Managing both kinds of Target's with the same methods, one needs to
build a small interpreter for the build rule specification. I do not
think my Python knowledge and experience is good enough for doing this
kind of change in the build system.
>
>
>>@@ -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.
>
>
Your right, I will remove it in a future patch.
>
>
>>@@ -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.
>
>
No, I do not see what you mean here.
>
>
>>@@ -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.
>
>
As above, I do not see what you mean here.
I hope we can work out a way to get the javahl binding in the
intergrated build system. I hoped you had noticed my posts with the
patches to dev@subversion.tigris.org and the discussion with Justin
Erenkrantz about them earlier.
Patrick
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jun 8 08:52:08 2004