(PUP-2176)(PUP-2035) Followup on function API / Puppet Functions #2546
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes it possible to write functions in site.pp, or directly in apply -e. When encountered, the PopsBridge will define them in the appropriate loader. A new SimpleEnvironmentLoader was introduced (it is basically a loader that does not find anything, but can be given entries from an external source). The functionality to instantiate from a model was added to the PuppetFunctionInstantiator. The ability to produce a URI with the source location was added to SourcePosAdapter. The compiler initialization sequence is altered in this commit because the very early loading and parsing of site.pp (or equivalent), and the compilers context overrides (environment, injector, global_scope, etc) was done too late. Now, the overrides are constructed once, but used twice (in the initvars method, and in the compile method).
This makes it possible to store a Proc in the context. On lookup the Proc is called and the result is memoized for subseqent lookups.
This makes the compiler initialize its context lazily wrt. the injector and loader when using the future branch of the logic. This is required because loaders and injector would otherwise need to be initialized twice; once for the initial import and a second time when doing the compilation. Now the values are retained. The initial problem was that compiler's initial import is triggered before loaders and injector was initialized (from initvars). This is required for using logic from the loader subsystem. Specifically for being able to define a function on the command line using puppet apply.
The loaders expected an error from validation that qualified names for puppet functions were not allowed. This changed, and the error is now that the function in question has a mismatched name.
This updates the tree dumper to dump parsed type information for parameters. Tests are added that puppet functions can be parsed.
Without this change, loading functions is always done from the perspective of the environment, not the perspective of the loader that loaded the code that makes the call. This is an important feature not only for modularity, but also for being able to conveniently test with a test specific loader with test specific functions. This change simply searches for a LoaderAdapter and favors that loader over the environment loader. This mechanism will be used for everything loaded, but is not yet implemented in the loaders). This fix was on master, but was lost in a merge conflict.
The default loader was puppet_system_loader, and this would not see any functions loaded in the environment (i.e. functions that are not in a module).
During the review of the functionality for PUP-2035 it was discovered that it was not possible to call a function in a module from the environment. This was due to the initial configuration, and that the evaluator used the wrong loader. This commit fixes the logic so that: * the environment loader is now split into two parts, a public (sees only things in the environment), and private (the public + all modules). * a module loader is parented by the public_environment_loader (does not see all modules - only those it depends on). * functions defined outside of a module are added to the public loader (i.e. from the command line, or site.pp) This fix is needed to be able to evaluate PUP-2176
All puppet code that is loaded needs to be associated with the loader that loaded that code. This is important for subsequent requests to load things from the body of the loaded code as it should use the same loader that loaded that body of code. The evaluator uses this information when calls are made. Currently, if code lacks information about loader, the private environment loader is used and it has visibility into all modules. This commit takes a small step in the right direction of implementing this everywhere as it adds a loader adapter to autoloaded puppet functions.
|
CLA signed by all contributors. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes issues found when reviewing puppet functions / loader api. The issues are tangled - part function part loaders.
This also adds lazy evaluation capability to the context since this was needed to enable the compiler's initial import to also be executing in a configured context but without causing performance degradation in all cases where features from the context were not used, and if that they were used, that they do not have to be initialized more than once for the same compiler.