The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #5027

Issue warnings when using dynamic scope

Added by Paul Berry almost 4 years ago. Updated over 2 years ago.

Status:ClosedStart date:04/14/2011
Priority:HighDue date:
Assignee:-% Done:

100%

Category:language
Target version:2.7.0
Affected Puppet version: Branch:http://github.com/MarkusQ/puppet/tree/ticket/next/5027
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

This ticket captures a decision made on the developer mailing list on Oct 12 (see email thread “Questions about variable scope”).

In Puppet version 2.7, a deprecation warning should be issued whenever a variable reference takes place or a resource default is applied as a result of dynamic scoping.

Dynamic scoping means that variables and resource defaults defined in class A also take effect in classes that A includes. The warning should advise users that this feature will be removed in a future version of Puppet, so they should start using other puppet features (such as parameterized classes) to achieve the desired effect.


Subtasks

Bug #7111: Modify scoping warning/deprecation messageClosed

Bug #13312: Dynamic scoping deprecation warnings should be given at t...ClosedAndrew Parker


Related issues

Related to Puppet - Refactor #5063: Internal global variable lookups should not be scoped Closed 10/20/2010

History

#1 Updated by Paul Berry almost 4 years ago

Here is an example of a .pp file that should produce the warning:

$foo = 'foo_value'

class a {
    $bar = 'bar_value'

    include b
}

class b inherits c {
    notify { $baz: } # should not generate a warning -- inherited from class c
    notify { $bar: } # should generate a warning -- uses dynamic scoping
    notify { $foo: } # should not generate a warning -- comes from top scope
}

class c {
    $baz = 'baz_value'
}

include a

This should produce three notify messages: “foo_value”, “bar_value”, and “baz_value”. Only the notify statement for $bar should produce a deprecation warning.

#2 Updated by Nick Lewis almost 4 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to http://github.com/nicklewis/puppet/tree/feature/next/5027

Scopes now track how they acquired their parents (dynamic scope from being included vs. inheritance). This allows us to simulate static scope when looking up variables, compare it to the result of the dynamic lookup, and issue a warning if the results differ. Such a warning is only issued once per compile.

#3 Updated by Markus Roberts almost 4 years ago

  • Assignee changed from Paul Berry to Markus Roberts

I’m working on an alternative implementation which I believe will be cleaner but may just serve to educate me.

#4 Updated by Markus Roberts over 3 years ago

  • Status changed from In Topic Branch Pending Review to Ready For Checkin
  • Branch changed from http://github.com/nicklewis/puppet/tree/feature/next/5027 to http://github.com/MarkusQ/puppet/tree/ticket/next/5027

Paul’s eventual acquiescence to my version on puppet-dev didn’t make it to the ticket.

#5 Updated by Nigel Kersten over 3 years ago

  • Status changed from Ready For Checkin to Needs More Information

Is it possible to get this rebased against the current master?

Will this solution also identify resource defaults ? (the example above doesn’t use them)

#6 Updated by Nigel Kersten over 3 years ago

  • Status changed from Needs More Information to In Topic Branch Pending Review
  • Assignee deleted (Markus Roberts)

Rebased here:

https://github.com/markusq/puppet/tree/ticket/next/5027 https://github.com/MarkusQ/puppet/commit/151092c94b696a190d9c93ba594c9c5a5e8b8efe

#7 Updated by Nick Lewis over 3 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

The branch on the ticket has been merged, with fixups to the warnings, to next in commit:2dfa0afb57ec80f451a54ef96341d413819c14c7.

#8 Updated by Dominic Maraglia over 3 years ago

  • Status changed from Merged - Pending Release to Closed

Fix verified in [[http://projects.puppetlabs.com/projects/puppet/repository/revisions/2dfa0afb57ec80f451a54ef96341d413819c14c7]]

Test checked to puppet-acceptance: git@github.com:puppetlabs/puppet-acceptance.git 18a26d9..8e12139 master –> master

#9 Updated by James Turnbull about 3 years ago

  • Target version changed from 2.7.x to 2.7.0

Also available in: Atom PDF