Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lookup prerequisites with same name outside of scope instead of matching self #264

Conversation

svanderbleek
Copy link

Task#lookup_prerequisite finds self if looking up a prerequisite outside of the namespace with the same task name. Behavior in this case is debatable but at least there should be a way to specify to lookup the task outside of the scope instead of ending the chain with a circular reference. If this default behavior I have implemented looks good this solves the problem by convention. Otherwise notation will need to be introduced such as

task :b
namespace "a" do
  task :b => "global:b"
end

Where global signifies to lookup outside the namespace.

application[prerequisite_name, @scope]
scoped_prerequisite_task = application[prerequisite_name, @scope]
if scoped_prerequisite_task == self
unscoped_prerequisite_task = application[prerequisite_name]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned about this @scope-less lookup. Does it follow ruby's constant lookup rules, or does it always only look at top-level.

What prerequisite task is discovered with:

m = task 'm'
a_m = nil

namespace 'a' do
  a_m = task 'm'

  namespace 'b' do
    task x: 'm'
  end
end

Whichever behavior (m or a_m) this change causes should be tested and documented.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point ✨ I'll look into it and follow up. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking further, I think m should be the dependency for a:b:x, not a:m.

I think this is best because it prevents unintentional change in dependencies as you add new tasks in the namespaces between the root and current namespace.

@drbrain drbrain added this to the 10.3 milestone Apr 2, 2014
@drbrain drbrain modified the milestones: 10.4, 10.3 Apr 15, 2014
@drbrain
Copy link
Collaborator

drbrain commented Apr 15, 2014

Any updates on this @svanderbleek, or should I finish it?

@svanderbleek
Copy link
Author

I'll finish it, thanks for the ping. To finish we just need the two namespace test, correct?

@drbrain
Copy link
Collaborator

drbrain commented Apr 15, 2014

Correct

@svanderbleek
Copy link
Author

@drbrain rebased and updated with test_prerequisite_tasks_in_nested_namespaces

@drbrain drbrain modified the milestones: Future, 10.4 Apr 17, 2014
@svanderbleek
Copy link
Author

Any thoughts @drbrain?

@drbrain
Copy link
Collaborator

drbrain commented Apr 23, 2014

Since this changes the behavior of rake I have to wait until rake 11 to merge it. Barring any bugs or other commitments this will probably be within a month or so

@bhenderson
Copy link

according to the Rakefile Format https://github.com/jimweirich/rake/blob/master/doc/rakefile.rdoc, the way to do this is '^b' or 'rake:b'. this does seem to work for prerequisites, but not for task names.

task :test do
  puts 'hi test'
end

namespace :test do
  task :unit do
    puts 'hi units'
  end

  task :unit => '^test' # works
  task '^test' => :unit # does not work
end
$ rake --version
rake, version 10.3.1
$ rake test
hi test
$ rake test:unit
hi test
hi units
$ rake test:^test
hi test
hi units

Am I missing something?

@svanderbleek
Copy link
Author

@bhenderson Your example is off, but you are correct. Here is the problem I was talking about:

task :test do
  puts 'hi test'
end

namespace :unit do
  task :test do
    puts 'hi units'
  end

  task :test => '^test' # works
  task :test => 'test' # doesn't work
end

I didn't know about the syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants