Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Python variables are not accessible when script is sourced from a file listed in buildfile.includes section #182

Closed
davido opened this issue Aug 19, 2014 · 5 comments

Comments

@davido
Copy link
Contributor

davido commented Aug 19, 2014

Found on 63e0e52.

Including a python file (VERSION) directly works as expected, particularly variables that defined in it are exposed to the caller:

Given this VERSION file:

 $ cat VERSION
 PLUGIN_VERSION = '1.0'

and gerrit_plugin.bucklet:

 $ cat gerrit_plugin.bucklet
def gerrit_plugin(
    name,
    visibility = ['PUBLIC']):
  from os import path
  ROOT = get_base_path()
  f = '//' + path.join(ROOT, 'VERSION')
  include_defs(f)
  genrule(
    name = name + '__manifest',
    cmd = 'echo ' + PLUGIN_VERSION + '>$OUT',
    out = 'MANIFEST.MF',
  )

And this BUCK build file:

include_defs('//gerrit_plugin.bucklet')
gerrit_plugin(
  name = 'wip',
)

All is just fine:

 $ buck build :wip__manifest
 $ cat buck-out/gen/MANIFEST.MF
 1.0

When we make use of .buckconfig and buildfile.includes section there and include bucklet from a file that listed in this section, it doesn't work, e. g.:

$ cat .buckconfig
[buildfile]
  includes = //bucklets.defs

And include gerrit_plugin.bucklet not directly, but from bucklets.defs that is specified in .buckconfig file:

$ cat bucklets.defs
bucklets = [
  'gerrit_plugin.bucklet',
]

for bucklet in bucklets:
  include_defs('//%s' % bucklet)

And remove the include from BUCK:

 $ cat BUCK
 gerrit_plugin(
  name = 'wip',
)

It doesn't work any more, PLUGIN_VERSION cannt be accessed, even though the file '//VERSION' seems to be correctly imported:

$ buck build :wip__manifest
Using buckd.
[+] PARSING BUILD FILES...0,1s
Traceback (most recent call last):
  File "/home/davido/projects/buck_reproducer/buck-out/tmp/buck_run.pGbINKP8kg/buck6118602169058355247.py", line 1215, in <module>
    main()
  File "/home/davido/projects/buck_reproducer/buck-out/tmp/buck_run.pGbINKP8kg/buck6118602169058355247.py", line 583, in main
    buildFileProcessor.process(build_file.rstrip())
  File "/home/davido/projects/buck_reproducer/buck-out/tmp/buck_run.pGbINKP8kg/buck6118602169058355247.py", line 482, in process
    build_env['BUILD_FILE_SYMBOL_TABLE'])
  File "/home/davido/projects/buck_reproducer/./BUCK", line 9, in <module>
    name = 'wip',
  File "/home/davido/projects/buck_reproducer/gerrit_plugin.bucklet", line 10, in gerrit_plugin
    cmd = 'echo ' + PLUGIN_VERSION + '>$OUT',
NameError: global name 'PLUGIN_VERSION' is not defined
BUILD FAILED: Parse error for BUCK file ./BUCK: End of input at line 1 column 1
@davido davido changed the title Python variables are not accessible when script is sourced from a file listed in builfile.includes section Python variables are not accessible when script is sourced from a file listed in buildfile.includes section Aug 19, 2014
@davido
Copy link
Contributor Author

davido commented Aug 19, 2014

If someone wonders why we would want to do that, here is the real change [1]

[1] https://gerrit-review.googlesource.com/58892

@davido
Copy link
Contributor Author

davido commented Aug 20, 2014

With this reproducer it's easy to see the problem and the workaround: [1].

[1] https://github.com/davido/buck_include_defs_repro

@shs96c
Copy link
Contributor

shs96c commented Sep 16, 2014

We've a patch landing for this soon. Thanks for the report and test.

@andrewjcg
Copy link
Contributor

Thanks for such a detailed bug report! This undesirable behavior (and a few others) is caused by how we currently handle "include_defs" and python globals. We're working on remodeling this around python modules, which should fix a number of issues (and I've confirmed it fixes this issue), and hope to have it pushed soon.

@andrewjcg
Copy link
Contributor

Heh, completely missed Simon's comment.

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Oct 29, 2014
This version fixes a critical bug [1] that prevents us from simplifying
bucklets intergration. In this version, Buck only allows defining new
rule functions in files included with include_defs, not actually
instantiating rules, so we need to reshuffle some rules.

After this commit [2] "$(macro ...)" syntax is preserved for buck
macros, we need to use a backslash to send commands directly to the
shell: "\$(macro ...)". It turns out this doesn't work yet, shell
backticks seem to work though [3].

[1] facebook/buck#182
[2] facebook/buck@d6f3252
[3] facebook/buck#212

Change-Id: Ie99757bafc626d4ac2c5a75a2983d91b0c4f6d24
openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Nov 25, 2014
The fix for [1] prevents from importing python variables __foo__,
as explained in [2]. Rename standalone mode flag correspondingly.

[1] facebook/buck#182
[2] facebook/buck#221

Change-Id: If8682278e7c91df3fc18b3d623eb43950e839448
davido added a commit to davido/bucklets that referenced this issue Mar 21, 2015
The fix for [1] prevents from importing python variables __foo__,
as explained in [2]. Rename standalone mode flag correspondingly.

[1] facebook/buck#182
[2] facebook/buck#221

Change-Id: I0ac05c80a1d5fc1b8f8bd2c5ca5dabdfeeecbe52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants