You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Russell Yanofsky <re...@columbia.edu> on 2004/07/23 00:47:10 UTC
Re: svn commit: r10381 - trunk/build/generator
<ma...@tigris.org> wrote in message
news:200407212231.i6LMVQ308385@morbius.ch.collab.net...
> Author: maxb
> Date: Wed Jul 21 17:31:26 2004
> New Revision: 10381
>
> Modified:
> trunk/build/generator/gen_base.py
> trunk/build/generator/gen_make.py
> trunk/build/generator/gen_win.py
> Log:
> Phase 1 of a series of refactorings of the build generator code, intended
to
> make it easier to understand the architecture of Target and Section
classes.
> ...
> class Target(DependencyNode):
> "A build target is a node in our dependency graph."
>
> - def __init__(self, name, options, cfg, extmap):
> + def __init__(self, name, options, gen_obj):
> self.name = name
> + self.gen_obj = gen_obj
I don't think targets should hold references to the generator object. The
generator is a singleton, there's only one instance of it created for the
entire program. It seems like a waste of space to create a bunch of target
objects that all point to the same generator.
All other target members contain information about the target itself, and
they are used by generor backends to generate build rules for the target.
But the gen_obj member contains no information specific to the target, and
it would never be used by any backend because backends can access the
generator singleton directly.
Hopefully, that argument makes some sense and is convincing. If not, I think
the "gen_obj" member should be renamed to "_generator." The leading
underscore is appropriate for a private member that is only used internally
by Target methods. And losing the "_obj" suffix isn't a big deal because it
tells little about how the member is used or what it means (I usually think
the same thing about "_var", "_int", "_string" suffixes when they aren't
there for disambiguation)
- Russ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
Russell Yanofsky wrote:
>Branko Čibej wrote:
>
>
>>Russell Yanofsky wrote:
>>
>>
>>
>>>Branko Čibej wrote:
>>>
>>>
>>>
>>>
>>>>In the end, the decision between
>>>>
>>>> t = Target(self, x, y, z)
>>>> t.add_dependencies()
>>>>
>>>>and
>>>>
>>>> t = Target(x, y, z)
>>>> t.add_dependencies(self)
>>>>
>>>>depends on how many target methods depend on the generator state. If
>>>>it's only a few, the second form is probably nicer.
>>>>
>>>>
>>>>
>>>>
>>>I don't think so.
>>>
>>>
>>>
>>I said the *second* form, which means I was agreeing with you.
>>
>>
>
>Heh. I agree that you were agreeing with me there. I just disagreed about
>the decision depending on the number of target methods using the generator
>object. I think the decision should depend on the conceptual relationship
>between the classes.
>
>
Point is, if there are more than a few such functions, that usually
implies some conceptual relationship between the classes. But I're
getting off-topic now.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Russell Yanofsky <re...@columbia.edu>.
Branko Čibej wrote:
> Russell Yanofsky wrote:
>
>> Branko Čibej wrote:
>>
>>
>>> In the end, the decision between
>>>
>>> t = Target(self, x, y, z)
>>> t.add_dependencies()
>>>
>>> and
>>>
>>> t = Target(x, y, z)
>>> t.add_dependencies(self)
>>>
>>> depends on how many target methods depend on the generator state. If
>>> it's only a few, the second form is probably nicer.
>>>
>>>
>>
>> I don't think so.
>>
> I said the *second* form, which means I was agreeing with you.
Heh. I agree that you were agreeing with me there. I just disagreed about
the decision depending on the number of target methods using the generator
object. I think the decision should depend on the conceptual relationship
between the classes.
- Russ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
Russell Yanofsky wrote:
>Branko Čibej wrote:
>
>
>>In the end, the decision between
>>
>> t = Target(self, x, y, z)
>> t.add_dependencies()
>>
>>and
>>
>> t = Target(x, y, z)
>> t.add_dependencies(self)
>>
>>depends on how many target methods depend on the generator state. If
>>it's only a few, the second form is probably nicer.
>>
>>
>
>I don't think so.
>
I said the *second* form, which means I was agreeing with you.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Russell Yanofsky <re...@columbia.edu>.
Branko Čibej wrote:
> Russell Yanofsky wrote:
>> I'm not saying we should use global variables either. We can pass the
>> generator object around wherever we need it. But we shouldn't make
>> the generator object an attribute of target objects because
>> conceptually, it isn't one.
>>
>>
> Well, yes, if the generator ever learns to create scripts for several
> targets at once (having more than one generator object live at the
> same time, with only one set of targets), then I'd say that's a valid
> argument. But right now, something like that can't work because the
> list of targets (or the targets' attributes) depends on the generator
> used.
That's the thing. The list of targets and the targets' attributes should not
depend on the generator used, and they didn't before this change.
> In the end, the decision between
>
> t = Target(self, x, y, z)
> t.add_dependencies()
>
> and
>
> t = Target(x, y, z)
> t.add_dependencies(self)
>
> depends on how many target methods depend on the generator state. If
> it's only a few, the second form is probably nicer.
I don't think so. To my mind, adding a pointer member to an object is
analogous to adding a foreign key column in a database table. It should be
avoided unless it represents a conceptual relationship between the two
entities.
- Russ.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
Russell Yanofsky wrote:
>Branko ÄOibej wrote:
>
>
>>Keeping a reference to a state object is better than relying on global
>>variables.
>>
>>
>I'm not saying we should use global variables either. We can pass the
>generator object around wherever we need it. But we shouldn't make the
>generator object an attribute of target objects because conceptually, it
>isn't one.
>
>
Well, yes, if the generator ever learns to create scripts for several
targets at once (having more than one generator object live at the same
time, with only one set of targets), then I'd say that's a valid
argument. But right now, something like that can't work because the list
of targets (or the targets' attributes) depends on the generator used.
In the end, the decision between
t = Target(self, x, y, z)
t.add_dependencies()
and
t = Target(x, y, z)
t.add_dependencies(self)
depends on how many target methods depend on the generator state. If
it's only a few, the second form is probably nicer.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Russell Yanofsky <re...@columbia.edu>.
Branko ÄOibej wrote:
> The generator scripts arent indended to be run on an embedded system.
I'm not making an efficiency argument. I'm saying we shouldn't start storing
redundant information without some good reason for doing so.
> Keeping a reference to a state object is better than relying on global
> variables.
I'm not saying we should use global variables either. We can pass the
generator object around wherever we need it. But we shouldn't make the
generator object an attribute of target objects because conceptually, it
isn't one.
- Russ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
Russell Yanofsky wrote:
>Max Bowsher wrote:
>
>
>>Russell Yanofsky wrote:
>>
>>
>>>I don't think targets should hold references to the generator
>>>object. The generator is a singleton, there's only one instance of
>>>it created for the entire program. It seems like a waste of space to
>>>create a bunch of target objects that all point to the same
>>>generator.
>>>
>>>
>>"waste of space"?
>>
>>
>
>Space, i.e. memory. You are adding a new class member to store redundant
>information.
>
>
The generator scripts arent indended to be run on an embedded system.
Keeping a reference to a state object is better than relying on global
variables.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Russell Yanofsky <re...@columbia.edu>.
Max Bowsher wrote:
> Russell Yanofsky wrote:
>> I don't think targets should hold references to the generator
>> object. The generator is a singleton, there's only one instance of
>> it created for the entire program. It seems like a waste of space to
>> create a bunch of target objects that all point to the same
>> generator.
>
> "waste of space"?
Space, i.e. memory. You are adding a new class member to store redundant
information.
> It seems sensible to me to give the Target objects a reference to the
> global state of the generation process - the generator object.
It is fine for target objects to get a reference to the generator object. We
can pass it as a parameter to any methods that need it. What I object to is
storing the reference as part of the state of Target objects because it
contains redudant information that isn't specific to any target.
>> All other target members contain information about the target
>> itself, and they are used by generor backends to generate build
>> rules for the target. But the gen_obj member contains no information
>> specific to the target, and it would never be used by any backend
>> because backends can access the generator singleton directly.
>
> It's purpose is not to be used by the backends, but to be used by the
> Target object during add_dependencies().
If it needs to be used by add_dependencies, then it should be passed as a
parameter to add_dependencies.
> BTW, does anyone else find it bizarre that you have to construct a
> Target object, and then manually call what is effectively a
> "more_init" function before it is useful?
Yes. It'd be good either merge the __init__ and add_dependencies methods or
else have the __init__ method invoke the add_dependencies method.
- Russ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10381 - trunk/build/generator
Posted by Max Bowsher <ma...@ukf.net>.
Russell Yanofsky wrote:
> <ma...@tigris.org> wrote in message
> news:200407212231.i6LMVQ308385@morbius.ch.collab.net...
>> Author: maxb
>> Date: Wed Jul 21 17:31:26 2004
>> New Revision: 10381
>>
>> Modified:
>> trunk/build/generator/gen_base.py
>> trunk/build/generator/gen_make.py
>> trunk/build/generator/gen_win.py
>> Log:
>> Phase 1 of a series of refactorings of the build generator code, intended
to
>> make it easier to understand the architecture of Target and Section
classes.
>> ...
>> class Target(DependencyNode):
>> "A build target is a node in our dependency graph."
>>
>> - def __init__(self, name, options, cfg, extmap):
>> + def __init__(self, name, options, gen_obj):
>> self.name = name
>> + self.gen_obj = gen_obj
>
> I don't think targets should hold references to the generator object. The
> generator is a singleton, there's only one instance of it created for the
> entire program. It seems like a waste of space to create a bunch of target
> objects that all point to the same generator.
"waste of space"?
It seems sensible to me to give the Target objects a reference to the global
state of the generation process - the generator object.
> All other target members contain information about the target itself, and
> they are used by generor backends to generate build rules for the target.
> But the gen_obj member contains no information specific to the target, and
> it would never be used by any backend because backends can access the
> generator singleton directly.
It's purpose is not to be used by the backends, but to be used by the Target
object during add_dependencies().
BTW, does anyone else find it bizarre that you have to construct a Target
object, and then manually call what is effectively a "more_init" function
before it is useful?
> Hopefully, that argument makes some sense and is convincing. If not, I
think
> the "gen_obj" member should be renamed to "_generator." The leading
> underscore is appropriate for a private member that is only used
internally
> by Target methods. And losing the "_obj" suffix isn't a big deal because
it
> tells little about how the member is used or what it means (I usually
think
> the same thing about "_var", "_int", "_string" suffixes when they aren't
> there for disambiguation)
That, I agree with. It is a private member.
Max.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org