8000 Fix errors with incompatible_disallow_empty_glob by gergelyfabian · Pull Request #315 · bazel-contrib/rules_python · GitHub
[go: up one dir, main page]
Skip to content

Fix errors with incompatible_disallow_empty_glob #315

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 25, 2020
Merged

Fix errors with incompatible_disallow_empty_glob #315

merged 3 commits into from
Jun 25, 2020

Conversation

gergelyfabian
Copy link
Contributor

Allow py_library sources to be empty.
If --incompatible_disallow_empty_glob is set then generated py_library
targets will fail if there are no .py files.

Examples are pymssql==2.1.4 and cx-Oracle==7.2.3.

Set allow_empty = True for glob().

Bazel issue for incompatible_disallow_empty_glob:
bazelbuild/bazel#8195

@thundergolfer
Copy link

Hey @gergelyfabian,

The .par files are binary files and therefore unreviewable, so for security reasons we will regenerate the .par files for you when merging your pull request. see CONTRIBUTING.md.

you can remove them from your PR 🙂

@gergelyfabian
Copy link
Contributor Author
gergelyfabian commented May 21, 2020 via email

@gergelyfabian gergelyfabian force-pushed the allow_empty_glob_for_py_library branch from 7711c7b to eaccecf Compare May 21, 2020 03:49
@gergelyfabian
Copy link
Contributor Author

Thanks! Will do.

I have removed the .par files.

@gergelyfabian gergelyfabian mentioned this pull request May 28, 2020
Merged
@gergelyfabian
Copy link
Contributor Author

Just a comment: rules_python_external is not experiencing this problem, because it adds a __init__.py file to the generated Bazel structure:

rules_python_external (for pymssql):

.
├── BUILD
├── _mssql.cpython-36m-x86_64-linux-gnu.so
├── pymssql-2.1.4.dist-info
│   ├── __init__.py
│   ├── METADATA
│   ├── RECORD
│   ├── top_level.txt
│   └── WHEEL
└── pymssql.cpython-36m-x86_64-linux-gnu.so

While rules_python does (for pymssql):

.
├── BUILD
├── _mssql.cpython-36m-x86_64-linux-gnu.so
├── pymssql-2.1.4.dist-info
│   ├── METADATA
│   ├── RECORD
│   ├── top_level.txt
│   └── WHEEL
├── pymssql.cpython-36m-x86_64-linux-gnu.so
└── WORKSPACE

So maybe the fix should be to add the __init__.py file creation, rather then to add allow_empty = True?

@gergelyfabian gergelyfabian force-pushed the allow_empty_glob_for_py_library branch from eaccecf to c167096 Compare June 4, 2020 05:10
@gergelyfabian
Copy link
Contributor Author

Answering my own question on __init__.py. In rules_python_external the decision was made, that it should not depend on some generated code (done by the plugin), but rather allow for no Python sources if this is e.g. a pip package implemented in Cython (that means no Python sources in Bazel cache for that).

So I'd argument that the fix should be to add allow_empty = True.

@thundergolfer
Copy link

@gergelyfabian I think this change would be good to merge with a comment about which scenarios produce packages without py srcs. Like was done here -> 8000 https://github.com/dillon-giacoppo/rules_python_external/pull/40/files 👍

@gergelyfabian
Fix errors with incompatible_disallow_empty_glob
74a3eac 
Allow py_library sources to be empty.
If --incompatible_disallow_empty_glob is set then generated py_library
targets will fail if there are no .py files.

Examples are pymssql==2.1.4 and cx-Oracle==7.2.3.

Set `allow_empty = True` for glob().

Bazel issue for incompatible_disallow_empty_glob:
bazelbuild/bazel#8195
@gergelyfabian gergelyfabian force-pushed the allow_empty_glob_for_py_library branch from c167096 to 74a3eac Compare June 15, 2020 04:54
@gergelyfabian
Copy link
Contributor Author

@thundergolfer I've updated the PR.

@andyscott andyscott merged commit cd55272 into bazel-contrib:master Jun 25, 2020
thundergolfer pushed a commit to gmweaver/rules_python that referenced this pull request Jul 31, 2020
keep file permissions when unpacking
d6bbe55 
add zipextended.py to BUILD

fix import path

use pip internal unzip

use pip internal unzip tools

remove custom zipfile implementation

Add documentation of community / Bazel team ownership (bazel-contrib#308)

This adds a more nuanced CODEOWNERS and explains its purpose in CONTRIBUTING.md.

Fixes bazel-contrib#291.

point README readers to new 0.0.2 release (bazel-contrib#302)

update version in version.bzl (bazel-contrib#303)

Fix for when there are so many file arguments it creates the Command To Long error (bazel-contrib#320)

Fix failing build on CI by specifying pip package version (bazel-contrib#329)

`bazel build //...` was failing due to "googleapis-common-protos[grpc]"
pip package being unavailable.

It seems to be caused by latest googleapis-common-protos release.
Specify googleapis-common-protos in requirements.txt to be in the previous
version (1.51.0) to fix this.

Fixes bazel-contrib#321.

"Skylark" is an outdated name of the language, please use "starlark" instead (bazel-contrib#327)

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

Support python interpreter target in pip_import. (bazel-contrib#312)

* Support python interpreter target in pip_import.

This allows users to use a custom python interpreter that is built by
another repository rule instead of using a pre-built interpreter binary
that is checked-in.

This tangentially addresses bazel-contrib#257 since a common setup is to use the
custom built interpreter in the python toolchain.

For example, see: https://github.com/kku1993/bazel-hermetic-python

* Actually use interpreter path.

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

Address bazel-contrib#289 (bazel-contrib#328)

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

Fix errors with incompatible_disallow_empty_glob (bazel-contrib#315)

Allow py_library sources to be empty.
If --incompatible_disallow_empty_glob is set then generated py_library
targets will fail if there are no .py files.

Examples are pymssql==2.1.4 and cx-Oracle==7.2.3.

Set `allow_empty = True` for glob().

Bazel issue for incompatible_disallow_empty_glob:
bazelbuild/bazel#8195

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

rebuild piptool and whltool

update filename, add links to pip function definitions

remove typing checks

chore: github setup improvements (bazel-contrib#334)

Remove mention and usage of Bazel Federation (bazel-contrib#339)

It's currently a stalled project so it's not useful for us to direct new users there in our README.
Separately it is harder to develop on rules_python since it is currently not self-contained.
For example it's hard to find or adjust the version of rules_pkg without looking/editing in the federation repo.
Tony says this is an okay change: bazelbuild/bazel-federation@63f9746#commitcomment-40577834

leaner implementation of pip unzip

remove unzip.py

temp test with tools

2nd test with tools

final test with tools

replace with master tools

merge original tools

feat(examples): move examples to a nested WORKSPACE (bazel-contrib#337)

This lets users understand the example in isolation. They can copy/paste the example directory
and it works correctly.

This refactors the existing examples which are quite weak, only really demonstrating pip usage.
This makes room for examples demonstrating other features (like protocol buffers) or package
managers (like poetry).

In a later commit I'll add bazel-integration-testing so we get a test target that confirms
the examples build (including their WORKSPACE being self-contained)

warn against putting .par file changes in PR. (bazel-contrib#342)
thundergolfer pushed a commit to gmweaver/rules_python that referenced this pull request Jul 31, 2020
keep file permissions when unpacking
6742ef2 
add zipextended.py to BUILD

fix import path

use pip internal unzip

use pip internal unzip tools

remove custom zipfile implementation

Add documentation of community / Bazel team ownership (bazel-contrib#308)

This adds a more nuanced CODEOWNERS and explains its purpose in CONTRIBUTING.md.

Fixes bazel-contrib#291.

point README readers to new 0.0.2 release (bazel-contrib#302)

update version in version.bzl (bazel-contrib#303)

Fix for when there are so many file arguments it creates the Command To Long error (bazel-contrib#320)

Fix failing build on CI by specifying pip package version (bazel-contrib#329)

`bazel build //...` was failing due to "googleapis-common-protos[grpc]"
pip package being unavailable.

It seems to be caused by latest googleapis-common-protos release.
Specify googleapis-common-protos in requirements.txt to be in the previous
version (1.51.0) to fix this.

Fixes bazel-contrib#321.

"Skylark" is an outdated name of the language, please use "starlark" instead (bazel-contrib#327)

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

Support python interpreter target in pip_import. (bazel-contrib#312)

* Support python interpreter target in pip_import.

This allows users to use a custom python interpreter that is built by
another repository rule instead of using a pre-built interpreter binary
that is checked-in.

This tangentially addresses bazel-contrib#257 since a common setup is to use the
custom built interpreter in the python toolchain.

For example, see: https://github.com/kku1993/bazel-hermetic-python

* Actually use interpreter path.

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

Address bazel-contrib#289 (bazel-contrib#328)

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

Fix errors with incompatible_disallow_empty_glob (bazel-contrib#315)

Allow py_library sources to be empty.
If --incompatible_disallow_empty_glob is set then generated py_library
targets will fail if there are no .py files.

Examples are pymssql==2.1.4 and cx-Oracle==7.2.3.

Set `allow_empty = True` for glob().

Bazel issue for incompatible_disallow_empty_glob:
bazelbuild/bazel#8195

Co-authored-by: Andy Scott <andyscott@users.noreply.github.com>

rebuild piptool and whltool

update filename, add links to pip function definitions

remove typing checks

chore: github setup improvements (bazel-contrib#334)

Remove mention and usage of Bazel Federation (bazel-contrib#339)

It's currently a stalled project so it's not useful for us to direct new users there in our README.
Separately it is harder to develop on rules_python since it is currently not self-contained.
For example it's hard to find or adjust the version of rules_pkg without looking/editing in the federation repo.
Tony says this is an okay change: bazelbuild/bazel-federation@63f9746#commitcomment-40577834

leaner implementation of pip unzip

remove unzip.py

temp test with tools

2nd test with tools

final test with tools

replace with master tools

merge original tools

feat(examples): move examples to a nested WORKSPACE (bazel-contrib#337)

This lets users understand the example in isolation. They can copy/paste the example directory
and it works correctly.

This refactors the existing examples which are quite weak, only really demonstrating pip usage.
This makes room for examples demonstrating other features (like protocol buffers) or package
managers (like poetry).

In a later commit I'll add bazel-integration-testing so we get a test target that confirms
the examples build (including their WORKSPACE being self-contained)

warn against putting .par file changes in PR. (bazel-contrib#342)
@alexeagle alexeagle mentioned this pull request Aug 20, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thundergolfer thundergolfer thundergolfer approved these changes

@andyscott andyscott Awaiting requested review from andyscott

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gergelyfabian @thundergolfer @andyscott @googlebot
0