Skip to content

Commit

Permalink
Automated rollback of commit 0f9c6ea.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Breaks Kokoro and I accidentally submitted the change without presubmit checks.

*** Original change description ***

Make __init__.py files creation optional

Introduce a new attribute to py_binary and py_test to control whether to
create `__init__.py` or not.

Fixes bazelbuild/rules_python#55

Closes #4470.

PiperOrigin-RevId: 185676592
  • Loading branch information
lberki authored and philwo committed Feb 14, 2018
1 parent 0f9c6ea commit fa0fac2
Show file tree
Hide file tree
Showing 11 changed files with 27 additions and 89 deletions.
1 change: 0 additions & 1 deletion examples/py/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ py_library(
py_binary(
name = "bin",
srcs = ["bin.py"],
legacy_create_init = False,
deps = [":lib"],
)

Expand Down
2 changes: 1 addition & 1 deletion examples/py/bin.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import lib
from examples.py import lib

print("Fib(5)=%d" % lib.Fib(5))
3 changes: 0 additions & 3 deletions examples/py_native/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ filegroup(
py_binary(
name = "bin",
srcs = ["bin.py"],
legacy_create_init = False,
deps = [
":lib",
"//examples/py_native/fibonacci",
Expand All @@ -25,7 +24,6 @@ py_library(
py_test(
name = "test",
srcs = ["test.py"],
legacy_create_init = False,
deps = [
":lib",
"//examples/py_native/fibonacci",
Expand All @@ -35,6 +33,5 @@ py_test(
py_test(
name = "fail",
srcs = ["fail.py"],
legacy_create_init = False,
deps = [":lib"],
)
2 changes: 1 addition & 1 deletion examples/py_native/bin.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# pylint: disable=superfluous-parens
"""A tiny example binary for the native Python rules of Bazel."""
from examples.py_native.lib import GetNumber
from fib import Fib
from lib import GetNumber

print("The number is %d" % GetNumber())
print("Fib(5) == %d" % Fib(5))
2 changes: 1 addition & 1 deletion examples/py_native/fail.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""A tiny example binary for the native Python rules of Bazel."""
import unittest
from lib import GetNumber
from examples.py_native.lib import GetNumber


class TestGetNumber(unittest.TestCase):
Expand Down
2 changes: 1 addition & 1 deletion examples/py_native/test.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"""A tiny example binary for the native Python rules of Bazel."""

import unittest
from examples.py_native.lib import GetNumber
from fib import Fib
from lib import GetNumber


class TestGetNumber(unittest.TestCase):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
import static com.google.devtools.build.lib.packages.BuildType.TRISTATE;
import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
import static com.google.devtools.build.lib.syntax.Type.STRING;
import static com.google.devtools.build.lib.syntax.Type.STRING_LIST;

Expand Down Expand Up @@ -136,14 +135,14 @@ public static final class PyBinaryBaseRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironment env) {
return builder
/* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(data) -->
The list of files needed by this binary at runtime.
See general comments about <code>data</code> at
<a href="${link common-definitions#common-attributes}">
Attributes common to all build rules</a>.
Also see the <a href="${link py_library.data}"><code>data</code></a> argument of
the <a href="${link py_library}"><code>py_library</code></a> rule for details.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
/* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(data) -->
The list of files needed by this binary at runtime.
See general comments about <code>data</code> at
<a href="${link common-definitions#common-attributes}">
Attributes common to all build rules</a>.
Also see the <a href="${link py_library.data}"><code>data</code></a> argument of
the <a href="${link py_library}"><code>py_library</code></a> rule for details.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */

/* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(main) -->
The name of the source file that is the main entry point of the application.
Expand All @@ -158,13 +157,11 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
Valid values are <code>"PY2"</code> (default) or <code>"PY3"</code>.
Python 3 support is experimental.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr("default_python_version", STRING)
.value(PythonVersion.defaultTargetPythonVersion().toString())
.allowedValues(new AllowedValueSet(PythonVersion.getTargetPythonValues()))
.nonconfigurable(
"read by PythonUtils.getNewPythonVersion, which doesn't have access"
+ " to configuration keys"))
.add(attr("default_python_version", STRING)
.value(PythonVersion.defaultTargetPythonVersion().toString())
.allowedValues(new AllowedValueSet(PythonVersion.getTargetPythonValues()))
.nonconfigurable("read by PythonUtils.getNewPythonVersion, which doesn't have access"
+ " to configuration keys"))
/* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(srcs) -->
The list of source files that are processed to create the target.
This includes all your checked-in code and any
Expand All @@ -173,21 +170,11 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
probably belong in <code>srcs</code> and library targets probably belong
in <code>deps</code>, but don't worry about it too much.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(
attr("srcs", LABEL_LIST)
.mandatory()
.allowedFileTypes(PYTHON_SOURCE)
.direct_compile_time_input()
.allowedFileTypes(BazelPyRuleClasses.PYTHON_SOURCE))
/* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(legacy_create_init) -->
Whether to implicitly create empty __init__.py files in the runfiles tree.
These are created in every directory containing Python source code or
shared libraries, and every parent directory of those directories.
Default is true for backward compatibility. If false, the user is responsible
for creating __init__.py files (empty or not) and adding them to `srcs` or `deps`
of Python targets as required.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("legacy_create_init", BOOLEAN).value(true))
.add(attr("srcs", LABEL_LIST)
.mandatory()
.allowedFileTypes(PYTHON_SOURCE)
.direct_compile_time_input()
.allowedFileTypes(BazelPyRuleClasses.PYTHON_SOURCE))
/* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(stamp) -->
Enable link stamping.
Whether to encode build information into the binary. Possible values:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkParamsStore;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -141,11 +140,7 @@ private static Runfiles collectCommonRunfiles(RuleContext ruleContext, PyCommon
}
semantics.collectDefaultRunfiles(ruleContext, builder);
builder.add(ruleContext, PythonRunfilesProvider.TO_RUNFILES);

if (!ruleContext.attributes().has("legacy_create_init", Type.BOOLEAN)
|| ruleContext.attributes().get("legacy_create_init", Type.BOOLEAN)) {
builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
}
builder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
semantics.collectRunfilesForBinary(ruleContext, builder, common);
return builder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ protected void collect(CcLinkParams.Builder builder, boolean linkingStatically,
} else {
runfilesBuilder.addTransitiveArtifacts(filesToBuild);
}
runfilesBuilder.setEmptyFilesSupplier(PythonUtils.GET_INIT_PY_FILES);
runfilesBuilder.add(ruleContext, PythonRunfilesProvider.TO_RUNFILES);
runfilesBuilder.addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1449,9 +1449,9 @@ public void testAccessingRunfiles() throws Exception {
scratch.file(
"test/BUILD",
"load('//test:rule.bzl', 'skylark_rule')",
"py_binary(name = 'lib', srcs = ['lib.py', 'lib2.py'])",
"py_library(name = 'lib', srcs = ['a.py', 'b.py'])",
"skylark_rule(name = 'foo', dep = ':lib')",
"py_binary(name = 'lib_with_init', srcs = ['lib_with_init.py', 'lib2.py', '__init__.py'])",
"py_library(name = 'lib_with_init', srcs = ['a.py', 'b.py', '__init__.py'])",
"skylark_rule(name = 'foo_with_init', dep = ':lib_with_init')");

SkylarkRuleContext ruleContext = createRuleContext("//test:foo");
Expand All @@ -1460,7 +1460,7 @@ public void testAccessingRunfiles() throws Exception {
ruleContext, "[f.short_path for f in ruleContext.attr.dep.default_runfiles.files]");
assertThat(filenames).isInstanceOf(SkylarkList.class);
SkylarkList filenamesList = (SkylarkList) filenames;
assertThat(filenamesList).containsExactly("test/lib", "test/lib.py", "test/lib2.py");
assertThat(filenamesList).containsExactly("test/a.py", "test/b.py").inOrder();
Object emptyFilenames =
evalRuleContextCode(
ruleContext, "list(ruleContext.attr.dep.default_runfiles.empty_filenames)");
Expand Down
41 changes: 0 additions & 41 deletions src/test/py/bazel/py_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,46 +65,5 @@ def testRunfilesSymlinks(self):
.endswith('/a/b.py'))


class TestInitPyFiles(test_base.TestBase):

def createSimpleFiles(self, create_init=True):
self.ScratchFile('WORKSPACE')

self.ScratchFile('src/a/BUILD', [
'py_binary(name="a", srcs=["a.py"], deps=[":b"], legacy_create_init=%s)'
% create_init,
'py_library(name="b", srcs=["b.py"])',
])

self.ScratchFile('src/a/a.py', [
'from src.a import b',
'b.Hello()',
])

self.ScratchFile('src/a/b.py', [
'def Hello():',
' print("Hello, World")',
])

def testInitPyFilesCreated(self):
self.createSimpleFiles()
exit_code, _, stderr = self.RunBazel(['build', '//src/a:a'])
self.AssertExitCode(exit_code, 0, stderr)
self.assertTrue(
os.path.exists('bazel-bin/src/a/a.runfiles/__main__/src/__init__.py'))
self.assertTrue(
os.path.exists('bazel-bin/src/a/a.runfiles/__main__/src/a/__init__.py'))


def testInitPyFilesNotCreatedWhenLegacyCreateInitIsSet(self):
self.createSimpleFiles(create_init=False)
exit_code, _, stderr = self.RunBazel(['build', '//src/a:a'])
self.AssertExitCode(exit_code, 0, stderr)
self.assertFalse(
os.path.exists('bazel-bin/src/a/a.runfiles/__main__/src/__init__.py'))
self.assertFalse(
os.path.exists('bazel-bin/src/a/a.runfiles/__main__/src/a/__init__.py'))


if __name__ == '__main__':
unittest.main()

0 comments on commit fa0fac2

Please sign in to comment.