Could you guys review this patch? This seems to solve both cases. I'll attach it to the relvant issue if this is considered a good fix.

The patch adds a defined? method to Project. The post condition of Project.project then becomes that Project.defined? will return true. A project is considered defined if it's definition has been executed. What is not guaranteed is that the definition of each sub project has been executed. AFAICT this shouldn't be a problem and is most likely what we want in the first place.

Pepijn

On 13/6/2010 19:14, Rhett Sutphin wrote:
Hi Pepijn,

On Jun 13, 2010, at 5:34 AM, Pepijn Van Eeckhoudt wrote:

I haven't checked yet but it seems as if project() is checking that all 
projects in the hierarchy are defined (i.e., the define task has been invoked).

In a way -- it is invoking the associated rake task for each referenced 
project.  The project rake task executes #define and rake prevents any given 
task from being executed more than once.

Wouldn't it be sufficient to only ensure the leaf of the path is defined and 
ignore the state of any parents? That would solve the problem you're describing.

It would solve the particular example I laid out in the message, yes.  It would not solve 
the example in BUILDR-454, unless you also change the way buildr "ensure[s] the leaf 
of the path is defined."

Rhett



Pepijn

Op 13-jun-2010 om 05:12 heeft Rhett Sutphin<[email protected]>  het 
volgende geschreven:\

Hi,

(Note: very long.  Buildr committers: before you tl;dr, please consider the 
section after the ==== at the end.)

On Jun 12, 2010, at 2:26 PM, Pepijn Van Eeckhoudt wrote:

I suspected 399 might have something to do with the cycle stuff that's been 
popping up. I still think the fix for 399 itself is correct though; the only 
difference is cycles are now being correctly detected where they previously 
might not have been.

I guess it depends on how you define "correctly".  The way buildr 1.4.0 
behaves, it is more awkward/less clear to use subprojects as namespaces: you can't 
express sibling dependencies by qualified name.  The example I gave in BUILDR-454 was the 
minimal one I could reproduce, but this alternative example might show better why this is 
undesirable:

define "psc" do
define "authentication" do
   define "plugin-api" do
   end

   define "cas-plugin" do
     compile.with project("authentication:plugin-api")
   end
end

define "web" do
   compile.with project("authentication:plugin-api")
end
end

1.4.0 fails on loading this buildfile where 1.3.5 does not:

RuntimeError : Circular dependency detected: TOP =>  psc =>  psc:authentication => 
 psc:authentication:local-plugin =>  psc:authentication

There isn't actually a dependency from psc:authentication:local-plugin to 
psc:authentication, though.  psc:authentication doesn't even have any packages 
or tasks, so it's nonsense (and therefore frustrating) for buildr to claim that 
there is a circular dependency there.

As I indicated in the ticket for BUILDR-454, you can work around this by using 
project.parent.project('plugin-api'), but that's awkward.  You can also work 
around it by defining cas-plugin like so:

define "cas-plugin" do
compile.with project("plugin-api")
end

which is less awkward (though still potentially unclear: what if there are 
several plugin APIs in my project?), but it doesn't make sense that one works 
and the other one doesn't.  After all, they both resolve to the same project.

This could come across as a regression to a user of buildr though. Any idea how 
to fix it?

Between BUILDR-454, BUILDR-320, and the problems Antoine referred to earlier in 
the thread, it seems like buildr's circular dependency detection is flawed[1].  
Last time I looked into it, it seemed like it was inherited from rake.  Perhaps 
its a mistake to have the projects themselves defined as separate rake tasks?

The alternative I've come up with is also flawed, but I'll describe it in case 
if gives someone a better idea:

Instead of using rake tasks for #defines, build up a graph of project 
instances.  When a define is encountered:

* Add a disconnected node for it to the graph
* Evaluate its body (but not any included defines), turning #project references 
into edges in the graph
   - If the referenced project exists in the graph, return it
   - If it doesn't, return a lazy proxy
* Perform a topological sort of the graph at this point to determine if any 
cycles exist.  (RGL[2] has an implementation.)
* If there are no cycles, recurse into the child defines and evaluate them the 
same way

(Note in particular that the parent-child relationship does not become a graph edge in this scheme, 
unless there's an explicit dependency from one to the other.  However, that relationship would 
still need to be separately maintained in order for things like project.projects("foo", 
"bar") to work.)

I see one flaw in this approach: it precludes meaningful[3] forward references 
in #define.  However, buildr already has problems with those (see BUILDR-320) 
and no one but me is complaining, so maybe no one uses them.

I haven't evaluated buildr itself to determine how hard this would be to 
implement.

====

Now, having gone on at length about possible solutions, I'd like to 
emphatically request that this not be solved before the next version of buildr 
is released.  Buildr has not worked with the current version of rubygems for 
the last four months.  As the developer of a two public java projects that use 
buildr, it is difficult enough for me to explain to people who want to compile 
them that there are build tools other than ant/maven/eclipse.  Since February, 
there's been the additional complexity that I have to carefully explain to them 
how to install ruby but not the current version of rubygems.  (And I'm not even 
mentioning the baroque RVM setup I have going personally so I can use rubygems 
1.3.7 with my ruby projects while still using a released version of buildr for 
my java projects.)

If you do decide to delay the 1.4.0 release further, may I suggest a buildr 
1.3.6 release that just fixes the rubygems incompatibility?  And as a mature 
project, perhaps buildr should maintain separate stable and feature branches to 
address these sorts of critical issues.

Thanks for reading all that,
Rhett

[1]: Indeed, I have so many problems with it that the other day when I (for the 
first time) encountered a valid cyclical dependency failure I initially assumed 
it was another buildr/rake bug.
[2]: http://rgl.rubyforge.org/rgl/index.html
[3]: by which I mean forward references which do anything with the attributes of the 
referenced project during project evaluation.  compile.with project("foo"), 
e.g., falls into the meaningful reference category.



Pepijn

Op 12-jun-2010 om 21:11 heeft Antoine Toulme<[email protected]>  het 
volgende geschreven:\

Commenting on my own email:

On Sat, Jun 12, 2010 at 01:01, Antoine Toulme<[email protected]>wrote:

I tried some functional testing of RC4 with Apache ODE.

It didn't play well.

First, here is still an issue with tag_name over git. I thought this was
fixed, but our API obviously is still not ready. We should at the very least
die with a deprecation message. I'll work on that this week-end.

Fixed now.


Second, with jruby, I got this annoying stacktrace:
http://jira.codehaus.org/browse/JRUBY-4867
Turns out our buildr script for jruby needed to be updated for version
1.5.1. Fixed now.

Third, I ran into issues with cycle detection.
Commenting out the Rake monkey-patching in application.rb fixes the issue -
I need to hack it some more.

Looks like BUILDR-399 causes the problem. Looks also like BUILDR-354 is
unrelated.
I'm wondering if the Rakefile of ODE is the problem - after all, it was one
of the first Rakefile and may contain some stale stuff.


Fourth, compilation fails because jmock and junit are not found.

Not there yet.


I'm not happy with this mess. I intend to start having rounds of functional
testing harnessed in Hudson.

Ongoing - I detailed my plan in BUILDR-456.


I'll probably sacrifice part of my week-end over this.

Thanks,

Antoine







--
Pepijn Van Eeckhoudt - Project Leader
T +32 16 23 95 91
F +32 16 29 34 22 | [email protected]

LUCIAD - high performance visualization
Wetenschapspark Arenberg | Gaston Geenslaan 11
3001 Leuven | Belgium | www.luciad.com

Index: lib/buildr/core/project.rb
===================================================================
--- lib/buildr/core/project.rb  (revision 946877)
+++ lib/buildr/core/project.rb  (revision )
@@ -254,7 +254,10 @@
         end
         project ||= @projects[name] # Not found in scope.
         raise "No such project #{name}" unless project
-        project.invoke
+
+        project_scope = project.name.split(':')
+        app_scope = Buildr.application.current_scope
+        project.invoke unless app_scope && project_scope == app_scope[0, 
project_scope.length]
         project
       end
 
Index: spec/core/project_spec.rb
===================================================================
--- spec/core/project_spec.rb   (revision 919303)
+++ spec/core/project_spec.rb   (revision )
@@ -83,8 +83,27 @@
     Buildr.define('foo') { define('bar') { project('baz:bar') } }
     lambda { project('foo') }.should raise_error(RuntimeError, /Circular 
dependency/)
   end
+
+  it 'should handle non-circular dependencies' do
+    Buildr.define "psc" do
+      define "authentication" do
+        define "plugin-api" do
-end
+        end
 
+        define "cas-plugin" do
+          compile.with project("authentication:plugin-api")
+        end
+      end
+
+      define "web" do
+        compile.with project("authentication:plugin-api")
+      end
+    end
+
+    lambda { project('psc:authentication:cas-plugin') }.should_not raise_error
+  end
+end
+
 describe Project, ' property' do
   it 'should be set if passed as argument' do
     define 'foo', 'version'=>'1.1'

Reply via email to